Bug 198752 - [WPE][GTK] Fix build with unified sources disabled
Summary: [WPE][GTK] Fix build with unified sources disabled
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: Other
Hardware: Unspecified Linux
: P2 Normal
Assignee: Adrian Perez
URL:
Keywords: InRadar
Depends on: 198722
Blocks:
  Show dependency treegraph
 
Reported: 2019-06-11 05:47 PDT by Adrian Perez
Modified: 2019-06-19 10:43 PDT (History)
7 users (show)

See Also:


Attachments
List of build errors fixed (19.18 KB, text/plain)
2019-06-11 07:53 PDT, Adrian Perez
no flags Details
Patch (26.89 KB, patch)
2019-06-11 07:53 PDT, Adrian Perez
no flags Details | Formatted Diff | Diff
Patch for landing (26.87 KB, patch)
2019-06-11 08:18 PDT, Adrian Perez
no flags Details | Formatted Diff | Diff
Patch v2 (31.46 KB, patch)
2019-06-19 05:39 PDT, Adrian Perez
no flags Details | Formatted Diff | Diff
Patch v3 (31.39 KB, patch)
2019-06-19 07:13 PDT, Adrian Perez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adrian Perez 2019-06-11 05:47:05 PDT
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.
Comment 1 Adrian Perez 2019-06-11 07:53:30 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”.
Comment 2 Adrian Perez 2019-06-11 07:53:57 PDT
Created attachment 371845 [details]
Patch
Comment 3 Adrian Perez 2019-06-11 07:55:55 PDT
(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 4 Michael Catanzaro 2019-06-11 07:59:40 PDT
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.
Comment 5 Adrian Perez 2019-06-11 08:09:33 PDT
(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;
  ^
Comment 6 Adrian Perez 2019-06-11 08:18:52 PDT
Created attachment 371846 [details]
Patch for landing
Comment 7 Adrian Perez 2019-06-11 09:54:33 PDT
(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
Comment 8 Adrian Perez 2019-06-19 05:39:28 PDT
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.
Comment 9 Adrian Perez 2019-06-19 07:13:40 PDT
Created attachment 372459 [details]
Patch v3


Another stab at fixing the Mac build issue.
Comment 10 Adrian Perez 2019-06-19 10:18:26 PDT
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 11 WebKit Commit Bot 2019-06-19 10:42:12 PDT
Comment on attachment 372459 [details]
Patch v3

Clearing flags on attachment: 372459

Committed r246596: <https://trac.webkit.org/changeset/246596>
Comment 12 WebKit Commit Bot 2019-06-19 10:42:14 PDT
All reviewed patches have been landed.  Closing bug.
Comment 13 Radar WebKit Bug Importer 2019-06-19 10:43:13 PDT
<rdar://problem/51902752>