Bug 188732 – [CMake] Sync unified build with Cocoa ports

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Michael Catanzaro]

Sync unified build with Cocoa ports. This enables unified build for WebKit/Platform and WebKit/Shared.

Lots of files need to be moved around since the existing Sources.txt was not copied from CMakeLists.txt. This replaces the Sources.txt with the sources list from CMakeList.txt. Files that are not built for Cocoa are moved to SourcesGTK.txt, SourcesWPE.txt, and PlatformWin.cmake. Files that are built only on Cocoa are moved to SourcesCocoa.txt. There's plenty of room to determine if many of these files really need to be platform-specific in the future, but let's not change that now.

[Michael Catanzaro]

Created attachment 347459 [details]
Patch

[Michael Catanzaro]

Created attachment 347460 [details]
Patch

[Michael Catanzaro]

Created attachment 347461 [details]
Patch

[Michael Catanzaro]

GTK will need a bit more work due to *grumble* PluginProcess2. I'll try again later today.

[EWS Watchlist]

Comment on attachment 347461 [details]
Patch

Attachment 347461 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/8910813

New failing tests:
http/tests/security/contentSecurityPolicy/video-with-https-url-allowed-by-csp-media-src-star.html

[EWS Watchlist]

Created attachment 347464 [details]
Archive of layout-test-results from ews202 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews202  Port: win-future  Platform: CYGWIN_NT-6.1-2.10.0-0.325-5-3-x86_64-64bit

[Michael Catanzaro]

Created attachment 347475 [details]
Patch

[Michael Catanzaro]

This patch should work for everything except Windows.

I have a linker error on the Windows EWS. I don't know what's wrong. I tried to ensure the files that are being built did not change. Help with Windows would be much appreciated.

[Michael Catanzaro]

Hm, actually the Apple Windows bot bot is fine, it's only WinCairo that's having trouble. That's quite odd.

[Michael Catanzaro]

Well something in that last version broke iOS again... I'll check it tomorrow.

[EWS Watchlist]

Comment on attachment 347475 [details]
Patch

Attachment 347475 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/8913193

New failing tests:
legacy-animation-engine/imported/blink/transitions/unprefixed-transform.html

[EWS Watchlist]

Created attachment 347479 [details]
Archive of layout-test-results from ews205 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews205  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[Tim Horton]

(In reply to Michael Catanzaro from comment #9)
> Hm, actually the Apple Windows bot bot is fine, it's only WinCairo that's
> having trouble. That's quite odd.

Apple Win port doesn’t build WebKit2, so I think this overstates the oddness :)

[Fujii Hironori]

Created attachment 347482 [details]
Patch

WEBKIT_COMPUTE_SOURCES wasn't called for Windows port.

* Call WEBKIT_COMPUTE_SOURCES for all ports

[Michael Catanzaro]

Ahhh, thanks Fujii!

Now I'd better investigate why all Cocoa ports are broken. I had not changed too much since I had them green....

[Michael Catanzaro]

I think the problem with iOS and macOS is that the Sources.txt has changed too much and now the results no longer match the XCode project. I had to un-unify several files under Shared/ (everything used by WebKitPluginProcess2) and all of Platform/ (it's all used by WebKitPluginProcess2) and those need to be restored to the XCode project. Tim, would you be able to help with this?

[Michael Catanzaro]

(In reply to Michael Catanzaro from comment #16)
> I think the problem with iOS and macOS is that the Sources.txt has changed
> too much and now the results no longer match the XCode project. I had to
> un-unify several files under Shared/ (everything used by
> WebKitPluginProcess2) and all of Platform/ (it's all used by
> WebKitPluginProcess2) and those need to be restored to the XCode project.
> Tim, would you be able to help with this?

The only other solution I can think of would be to move ALL of these files to the platform-specific SourcesCocoa.txt, SourcesWin.txt, SourcesWPE.txt, and SourcesGTK.txt, so they could remain unified for all ports except GTK. Then I could probably land this without needing help from someone with XCode. But that seems worse to me.

[Tim Horton]

(In reply to Michael Catanzaro from comment #17)
> (In reply to Michael Catanzaro from comment #16)
> > I think the problem with iOS and macOS is that the Sources.txt has changed
> > too much and now the results no longer match the XCode project. I had to
> > un-unify several files under Shared/ (everything used by
> > WebKitPluginProcess2) and all of Platform/ (it's all used by
> > WebKitPluginProcess2) and those need to be restored to the XCode project.
> > Tim, would you be able to help with this?
> 
> The only other solution I can think of would be to move ALL of these files
> to the platform-specific SourcesCocoa.txt, SourcesWin.txt, SourcesWPE.txt,
> and SourcesGTK.txt, so they could remain unified for all ports except GTK.
> Then I could probably land this without needing help from someone with
> XCode. But that seems worse to me.

Is that worse? Not sure why the other ports should leave things ununified just for GTK :P Even just unifying platform + shared was a (small) build time win. Also, we didn't have Sources.txt until very recently so having lots of things in the platform-specific ones would be semi-status-quo?

[EWS Watchlist]

Comment on attachment 347482 [details]
Patch

Attachment 347482 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/8919172

New failing tests:
http/tests/security/canvas-remote-read-remote-video-blocked-no-crossorigin.html

[EWS Watchlist]

Created attachment 347524 [details]
Archive of layout-test-results from ews205 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews205  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[Michael Catanzaro]

(In reply to Tim Horton from comment #18)
> Is that worse? Not sure why the other ports should leave things ununified
> just for GTK :P Even just unifying platform + shared was a (small) build
> time win. Also, we didn't have Sources.txt until very recently so having
> lots of things in the platform-specific ones would be semi-status-quo?

Benefits: very small (probably not noticeable) build speedup for non-GTK ports, and avoids needing to scatter those TODOs throughout the cross-platform Sources.txt

Costs: a bunch of cross-platform files need to be duplicated in every platform-specific Sources.txt (currently only a few files are duplicated), also makes it easier for GTK developers to break the build for other ports since we won't be able to reproduce unified build problems affecting these files

Either way is OK, of course, so we can do whatever you prefer. But I'd say the costs probably outweigh the benefits.

If this were a large number of files, then I would be pretty concerned about unifying different files on different ports, since unified build issues are already tricky enough to handle when they're not platform-specific. For this small number of files, it's probably OK. (On the other hand, if this were a large number of files, the potential benefit would also be much greater.)

[Tim Horton]

Created attachment 347548 [details]
Patch

[Michael Catanzaro]

Committed r235098: <https://trac.webkit.org/changeset/235098>

[Radar WebKit Bug Importer]

<rdar://problem/43534342>