Summary: | [WPE][GTK] Fix build with unified sources disabled | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Adrian Perez <aperez> | ||||||||||||
Component: | Tools / Tests | Assignee: | Adrian Perez <aperez> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | bfulgham, commit-queue, mcatanzaro, megan_gardner, webkit-bug-importer, wenson_hsieh, zalan | ||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||
Version: | Other | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Linux | ||||||||||||||
Bug Depends on: | 198722 | ||||||||||||||
Bug Blocks: | |||||||||||||||
Attachments: |
|
Description
Adrian Perez
2019-06-11 05:47:05 PDT
Created attachment 371844 [details]
List of build errors fixed
I am attaching a (cleaned up) build log of the errors that
prevent building with “-DENABLE_UNIFIED_BUILDS=OFF”.
Created attachment 371845 [details]
Patch
(In reply to Adrian Perez from comment #2) > Created attachment 371845 [details] > Patch This patch makes non-unified builds of the WPE and GTK ports work for me with “trunk” using the default build options, when the build is configured directly by invoking CMake. Unified builds still work fine for the two ports; checked using “build-webkit”. Comment on attachment 371845 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=371845&action=review It looks like you considered forward-declarations in headers whenever possible, which was the only thing that really needed to be reviewed here. Good, and thanks! > Source/WebKit/WebProcess/WebPage/wpe/WebPageWPE.cpp:32 > +#if HAVE(ACCESSIBILITY) > +#include "WebPageProxy.h" > +#endif Just #include it always, adding guards only increases the odds of configuration-dependent build failures and has almost zero benefit. If it really deserves guards (it doesn't) then it would need to be #included in a separate block, at the bottom. (In reply to Michael Catanzaro from comment #4) > Comment on attachment 371845 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=371845&action=review > > It looks like you considered forward-declarations in headers whenever > possible, which was the only thing that really needed to be reviewed here. > Good, and thanks! Yes, I tried to avoid blindly adding #includes to avoid increasing the build time =) > > Source/WebKit/WebProcess/WebPage/wpe/WebPageWPE.cpp:32 > > +#if HAVE(ACCESSIBILITY) > > +#include "WebPageProxy.h" > > +#endif > > Just #include it always, adding guards only increases the odds of > configuration-dependent build failures and has almost zero benefit. > > If it really deserves guards (it doesn't) then it would need to be #included > in a separate block, at the bottom. Makes sense. I will apply your suggestion before landing. Also, I have to fix this build issue caught by the Mac EWSs: In file included from ./Modules/indexeddb/server/MemoryObjectStore.cpp:39: ./Modules/indexeddb/server/UniqueIDBDatabase.h:54:1: error: class 'IDBGetRecordData' was previously declared as a struct [-Werror,-Wmismatched-tags] class IDBGetRecordData; ^ Created attachment 371846 [details]
Patch for landing
(In reply to Adrian Perez from comment #6) > Created attachment 371846 [details] > Patch for landing …aaaand another Mac failure to take care of: In file included from /Volumes/Data/EWS/WebKit/WebKitBuild/Debug/DerivedSources/WebKit2/unified-sources/UnifiedSource6.cpp:2: In file included from /Volumes/Data/EWS/WebKit/Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp:40: In file included from /Volumes/Data/EWS/WebKit/Source/WebKit/UIProcess/WebFrameProxy.h:32: /Volumes/Data/EWS/WebKit/Source/WebKit/UIProcess/WebPageProxy.h:1162:95: error: unknown type name 'DataCallback'; did you mean 'Messages::WebPageProxy::DataCallback'? void drawPagesToPDF(WebFrameProxy*, const PrintInfo&, uint32_t first, uint32_t count, Ref<DataCallback>&&); ^~~~~~~~~~~~ Messages::WebPageProxy::DataCallback Created attachment 372454 [details]
Patch v2
Should fix the Mac build issue, and adds a few more fixes needed
for non-unified builds to complete successfully.
Created attachment 372459 [details]
Patch v3
Another stab at fixing the Mac build issue.
Comment on attachment 372459 [details]
Patch v3
Setting cq+ — I'll keep an eye on the bots, but it looks
quite safe to land this now that all the EWS bots are green :)
Comment on attachment 372459 [details] Patch v3 Clearing flags on attachment: 372459 Committed r246596: <https://trac.webkit.org/changeset/246596> All reviewed patches have been landed. Closing bug. |