Bug 198752 – [WPE][GTK] Fix build with unified sources disabled

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Adrian Perez]

Both the WPE and GTK WebKit ports cannot be built when passing
the “-DENABLE_UNIFIED_BUILDS=OFF” option to CMake. Fixing the
build with that will avoid mystery build failures when usage
of unified sources is enabled as well.

[Adrian Perez]

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”.

[Adrian Perez]

Created attachment 371845 [details]
Patch

[Adrian Perez]

(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”.

[Michael Catanzaro]

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.

[Adrian Perez]

(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;
  ^

[Adrian Perez]

Created attachment 371846 [details]
Patch for landing

[Adrian Perez]

(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

[Adrian Perez]

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.

[Adrian Perez]

Created attachment 372459 [details]
Patch v3


Another stab at fixing the Mac build issue.

[Adrian Perez]

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 :)

[WebKit Commit Bot]

Comment on attachment 372459 [details]
Patch v3

Clearing flags on attachment: 372459

Committed r246596: <https://trac.webkit.org/changeset/246596>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/51902752>