Move FileSystem to WTF
Created attachment 359556 [details] Patch
Created attachment 359559 [details] Patch
Attachment 359559 [details] did not pass style-queue: ERROR: Source/WTF/wtf/win/PathWalker.cpp:27: Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/WTF/wtf/win/PathWalker.cpp:28: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/FileSystem.cpp:28: Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/WTF/wtf/FileSystem.cpp:29: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/win/FileSystemWin.cpp:32: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/posix/FileSystemPOSIX.cpp:31: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/glib/FileSystemGlib.cpp:25: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/cf/FileSystemCF.cpp:31: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 8 in 183 files If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to Build Bot from comment #3) > Attachment 359559 [details] did not pass style-queue: > > > ERROR: Source/WTF/wtf/win/PathWalker.cpp:27: Found other header before a > header this file implements. Should be: config.h, primary header, blank > line, and then alphabetically sorted. [build/include_order] [4] > ERROR: Source/WTF/wtf/win/PathWalker.cpp:28: Alphabetical sorting problem. > [build/include_order] [4] > ERROR: Source/WTF/wtf/FileSystem.cpp:28: Found other header before a header > this file implements. Should be: config.h, primary header, blank line, and > then alphabetically sorted. [build/include_order] [4] > ERROR: Source/WTF/wtf/FileSystem.cpp:29: Alphabetical sorting problem. > [build/include_order] [4] > ERROR: Source/WTF/wtf/win/FileSystemWin.cpp:32: Alphabetical sorting > problem. [build/include_order] [4] > ERROR: Source/WTF/wtf/posix/FileSystemPOSIX.cpp:31: Alphabetical sorting > problem. [build/include_order] [4] > ERROR: Source/WTF/wtf/glib/FileSystemGlib.cpp:25: Alphabetical sorting > problem. [build/include_order] [4] > ERROR: Source/WTF/wtf/cf/FileSystemCF.cpp:31: Alphabetical sorting problem. > [build/include_order] [4] > Total errors found: 8 in 183 files > > > If any of these errors are false positives, please file a bug against > check-webkit-style. IIUC, these are all the canonical style for WTF.
Created attachment 359589 [details] Patch
Attachment 359589 [details] did not pass style-queue: ERROR: Source/WTF/wtf/win/PathWalker.cpp:27: Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/WTF/wtf/win/PathWalker.cpp:28: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/FileSystem.cpp:28: Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/WTF/wtf/FileSystem.cpp:29: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/win/FileSystemWin.cpp:32: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/posix/FileSystemPOSIX.cpp:31: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/glib/FileSystemGlib.cpp:25: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/cf/FileSystemCF.cpp:31: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 8 in 186 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 359718 [details] Patch
Attachment 359718 [details] did not pass style-queue: ERROR: Source/WTF/wtf/win/PathWalker.cpp:27: Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/WTF/wtf/win/PathWalker.cpp:28: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/FileSystem.cpp:28: Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/WTF/wtf/FileSystem.cpp:29: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/win/FileSystemWin.cpp:32: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/posix/FileSystemPOSIX.cpp:31: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/glib/FileSystemGlib.cpp:25: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/cf/FileSystemCF.cpp:31: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 8 in 186 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 359719 [details] Patch
Attachment 359719 [details] did not pass style-queue: ERROR: Source/WTF/wtf/win/PathWalker.cpp:27: Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/WTF/wtf/win/PathWalker.cpp:28: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/FileSystem.cpp:28: Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/WTF/wtf/FileSystem.cpp:29: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/win/FileSystemWin.cpp:32: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/posix/FileSystemPOSIX.cpp:31: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/glib/FileSystemGlib.cpp:25: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/cf/FileSystemCF.cpp:31: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 8 in 186 files If any of these errors are false positives, please file a bug against check-webkit-style.
It's really perplexing why GTK and WPE continue to have a problem here -- seems like the compiler doesn't understand namespace aliases: > In file included from ../../Source/WebCore/Modules/cache/DOMCacheEngine.h:34:0, > from ../../Source/WebCore/Modules/cache/CacheStorageConnection.h:29, > from ../../Source/WebCore/workers/WorkerGlobalScope.h:30, > from ../../Source/WebCore/inspector/InspectorInstrumentation.h:49, > from ../../Source/WebCore/dom/EventTarget.cpp:38, > from /home/ews/gtk-wk2-ews/WebKit/WebKitBuild/Release/DerivedSources/WebCore/unified-sources/UnifiedSource-be65d27a-8.cpp:5: > ../../Source/WebCore/platform/SharedBuffer.h:139:40: error: reference to ‘FileSystem’ is ambiguous > static Ref<DataSegment> create(FileSystem::MappedFileData&& data) { return adoptRef(*new DataSegment(WTFMove(data))); } > ^~~~~~~~~~ > In file included from ../../Source/WebCore/platform/SharedBuffer.h:30:0, > from ../../Source/WebCore/Modules/cache/DOMCacheEngine.h:34, > from ../../Source/WebCore/Modules/cache/CacheStorageConnection.h:29, > from ../../Source/WebCore/workers/WorkerGlobalScope.h:30, > from ../../Source/WebCore/inspector/InspectorInstrumentation.h:49, > from ../../Source/WebCore/dom/EventTarget.cpp:38, > from /home/ews/gtk-wk2-ews/WebKit/WebKitBuild/Release/DerivedSources/WebCore/unified-sources/UnifiedSource-be65d27a-8.cpp:5: > DerivedSources/ForwardingHeaders/wtf/FileSystem.h:226:39: note: candidates are: namespace FileSystem = WTF::WTF::FileSystem; > namespace FileSystem = WTF::FileSystem; > ^ > In file included from ../../Source/WebCore/platform/SharedBuffer.h:30:0, > from ../../Source/WebCore/Modules/cache/DOMCacheEngine.h:34, > from ../../Source/WebCore/Modules/cache/CacheStorageConnection.h:29, > from ../../Source/WebCore/workers/WorkerGlobalScope.h:30, > from ../../Source/WebCore/inspector/InspectorInstrumentation.h:49, > from ../../Source/WebCore/dom/EventTarget.cpp:38, > from /home/ews/gtk-wk2-ews/WebKit/WebKitBuild/Release/DerivedSources/WebCore/unified-sources/UnifiedSource-be65d27a-8.cpp:5: > DerivedSources/ForwardingHeaders/wtf/FileSystem.h:63:11: note: namespace WTF::FileSystem { } > namespace FileSystem { > ^~~~~~~~~~ EventTarget.cpp does have `using namespace WTF;` but removing it in the latest patch here didn't help. This isn't really surprising though, since it's sitting safely within the `namespace WebCore {}` block.
Created attachment 359827 [details] Patch
Attachment 359827 [details] did not pass style-queue: ERROR: Source/WTF/wtf/win/PathWalker.cpp:27: Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/WTF/wtf/win/PathWalker.cpp:28: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/FileSystem.cpp:28: Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/WTF/wtf/FileSystem.cpp:29: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/win/FileSystemWin.cpp:32: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/posix/FileSystemPOSIX.cpp:31: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/glib/FileSystemGlib.cpp:25: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/cf/FileSystemCF.cpp:31: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 8 in 185 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 359830 [details] Patch
Created attachment 359839 [details] Patch
Attachment 359839 [details] did not pass style-queue: ERROR: Source/WTF/wtf/win/PathWalker.cpp:27: Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/WTF/wtf/win/PathWalker.cpp:28: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/FileSystem.cpp:28: Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/WTF/wtf/FileSystem.cpp:29: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/win/FileSystemWin.cpp:32: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/posix/FileSystemPOSIX.cpp:31: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/glib/FileSystemGlib.cpp:25: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/cf/FileSystemCF.cpp:31: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 8 in 184 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 359862 [details] Patch * Fixed GTK+ builds.
Attachment 359862 [details] did not pass style-queue: ERROR: Source/WTF/wtf/win/PathWalker.cpp:27: Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/WTF/wtf/win/PathWalker.cpp:28: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/FileSystem.cpp:28: Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/WTF/wtf/FileSystem.cpp:29: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/win/FileSystemWin.cpp:32: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/posix/FileSystemPOSIX.cpp:31: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/glib/FileSystemGlib.cpp:25: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/cf/FileSystemCF.cpp:31: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 8 in 184 files If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to Ross Kirsling from comment #11) > It's really perplexing why GTK and WPE continue to have a problem here -- > seems like the compiler doesn't understand namespace aliases: > > ... > > EventTarget.cpp does have `using namespace WTF;` but removing it in the > latest patch here didn't help. This isn't really surprising though, since > it's sitting safely within the `namespace WebCore {}` block. Ugh, it seems to be a bug in GCC 6 and 7: https://godbolt.org/z/-00gZR Not sure what to do about this. I guess for WTF::Unicode we don't use a namespace alias and instead sprinkle `using namespace WTF::Unicode;` all over the place... The other possibility is to remove `using namespace WTF;` throughout WebCore. If we do the latter, I suppose it would make sense to split it out as its own patch first.
Created attachment 359913 [details] Patch
Attachment 359913 [details] did not pass style-queue: ERROR: Source/WTF/wtf/win/PathWalker.cpp:27: Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/WTF/wtf/win/PathWalker.cpp:28: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/FileSystem.cpp:28: Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/WTF/wtf/FileSystem.cpp:29: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/win/FileSystemWin.cpp:32: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/posix/FileSystemPOSIX.cpp:31: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/glib/FileSystemGlib.cpp:25: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/cf/FileSystemCF.cpp:31: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 8 in 184 files If any of these errors are false positives, please file a bug against check-webkit-style.
I guess pleasing GCC 6 was as simple as renaming the inner namespace to FileSystemImpl, following the precedent here: https://github.com/WebKit/webkit/blob/master/Source/WTF/wtf/JSONValues.h#L45 Now if I could just figure out why this patch causes API test timeouts for WinCairo...
Comment on attachment 359913 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=359913&action=review > Source/WTF/ChangeLog:3 > + Move FileSystem to WTF Why? To me it seems like PAL is the right place for it.
Created attachment 359945 [details] Patch
Attachment 359945 [details] did not pass style-queue: ERROR: Source/WTF/wtf/win/PathWalker.cpp:27: Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/WTF/wtf/win/PathWalker.cpp:28: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/FileSystem.cpp:28: Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/WTF/wtf/FileSystem.cpp:29: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/win/FileSystemWin.cpp:32: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/posix/FileSystemPOSIX.cpp:31: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/glib/FileSystemGlib.cpp:25: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/cf/FileSystemCF.cpp:31: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 8 in 184 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 359913 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=359913&action=review >> Source/WTF/ChangeLog:3 >> + Move FileSystem to WTF > > Why? To me it seems like PAL is the right place for it. I like the idea moving FileSystem to WTF because JSC shell starts testing persistent bytecode cache which resides in the FileSystem anyway. https://trac.webkit.org/changeset/240255/webkit' If some functions can be shared, it is nice.
(In reply to Ross Kirsling from comment #22) > Now if I could just figure out why this patch causes API test timeouts for > WinCairo... Scratch that; evidently this is an existing issue. (In reply to Yusuke Suzuki from comment #26) > >> Source/WTF/ChangeLog:3 > >> + Move FileSystem to WTF > > > > Why? To me it seems like PAL is the right place for it. > > I like the idea moving FileSystem to WTF because JSC shell starts testing > persistent bytecode cache which resides in the FileSystem anyway. > https://trac.webkit.org/changeset/240255/webkit' > If some functions can be shared, it is nice. Awesome. :D We're also intending to use FileSystem for PlayStation port's JSC shell.
Created attachment 359985 [details] Patch
Attachment 359985 [details] did not pass style-queue: ERROR: Source/WTF/wtf/win/PathWalker.cpp:27: Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/WTF/wtf/win/PathWalker.cpp:28: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/FileSystem.cpp:28: Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/WTF/wtf/FileSystem.cpp:29: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/win/FileSystemWin.cpp:32: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/posix/FileSystemPOSIX.cpp:31: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/glib/FileSystemGlib.cpp:25: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WTF/wtf/cf/FileSystemCF.cpp:31: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 8 in 184 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 359985 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=359985&action=review r=me. This is beneficial for JSC shell testing and Cached bytecodes implementations too. > Source/WTF/ChangeLog:17 > + * wtf/PlatformWin.cmake: Can you add PlatformJSCOnly.cmake changes too?
(In reply to Yusuke Suzuki from comment #30) > Comment on attachment 359985 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=359985&action=review > > r=me. This is beneficial for JSC shell testing and Cached bytecodes > implementations too. Thanks! > > Source/WTF/ChangeLog:17 > > + * wtf/PlatformWin.cmake: > > Can you add PlatformJSCOnly.cmake changes too? Hmm, do you think it's better to add them now, or should we wait until JSC actually consumes FileSystem?
(In reply to Ross Kirsling from comment #31) > (In reply to Yusuke Suzuki from comment #30) > > > Source/WTF/ChangeLog:17 > > > + * wtf/PlatformWin.cmake: > > > > Can you add PlatformJSCOnly.cmake changes too? > > Hmm, do you think it's better to add them now, or should we wait until JSC > actually consumes FileSystem? Since the current patch is pleasing EWS and JSCOnly will require separate verification, I'll land this one now and handle the rest in a separate patch this morning. :)
Comment on attachment 359985 [details] Patch Clearing flags on attachment: 359985 Committed r240437: <https://trac.webkit.org/changeset/240437>
All reviewed patches have been landed. Closing bug.
<rdar://problem/47518054>
Looks like the changes in https://trac.webkit.org/changeset/240437/webkit has caused an API failure on Mac Debug. Log: https://build.webkit.org/builders/Apple%20High%20Sierra%20Debug%20WK1%20%28Tests%29/builds/7412/steps/run-api-tests/logs/stdio reproduced with: run-api-tests --root debug-240437 --debug I received no failure on 240436.
(In reply to Truitt Savell from comment #36) > Looks like the changes in https://trac.webkit.org/changeset/240437/webkit > > has caused an API failure on Mac Debug. > > Log: > https://build.webkit.org/builders/ > Apple%20High%20Sierra%20Debug%20WK1%20%28Tests%29/builds/7412/steps/run-api- > tests/logs/stdio > > reproduced with: > run-api-tests --root debug-240437 --debug > > I received no failure on 240436. I don't understand how this patch can have any impact on TestWTF.WTF.StringOperators, but I will take a look... :(
(In reply to Ross Kirsling from comment #37) > (In reply to Truitt Savell from comment #36) > > Looks like the changes in https://trac.webkit.org/changeset/240437/webkit > > > > has caused an API failure on Mac Debug. > > > > Log: > > https://build.webkit.org/builders/ > > Apple%20High%20Sierra%20Debug%20WK1%20%28Tests%29/builds/7412/steps/run-api- > > tests/logs/stdio > > > > reproduced with: > > run-api-tests --root debug-240437 --debug > > > > I received no failure on 240436. > > I don't understand how this patch can have any impact on > TestWTF.WTF.StringOperators, but I will take a look... :( Evidently this is a notoriously fragile test file: https://bugs.webkit.org/show_bug.cgi?id=158623 https://bugs.webkit.org/show_bug.cgi?id=180211#c11 https://bugs.webkit.org/show_bug.cgi?id=192361
Apparently TestWTF.WTF.StringOperators has a monopoly on String::operator+. What a nightmare. Committed r240459: <https://trac.webkit.org/changeset/240459>