Bug 243759 - [GTK] Fix build on macOS
Summary: [GTK] Fix build on macOS
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: Mac (Apple Silicon) macOS 12
: P2 Normal
Assignee: Michael Catanzaro
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-08-09 18:05 PDT by echassiers.09-regards
Modified: 2022-08-11 23:24 PDT (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description echassiers.09-regards 2022-08-09 18:05:39 PDT
The Cocoa headers under Source/WTF/wtf/spi/cocoa are required to build WebKitGTK on macOS, but they are missing from the released tarballs.

It will also be convenient to package all headers under Source/WTF/wtf/spi.
Comment 1 Michael Catanzaro 2022-08-10 06:52:32 PDT
That can't be right because the spi is the Apple public API. That should not be required for building WebKitGTK, which has a different public API.

I'm afraid that you'll have to untangle whatever dependencies are involved here: no WebKit developers work on WebKitGTK for macOS, so it's up to the people who try to build and use it to figure out how to keep it working. Sorry....
Comment 2 Michael Catanzaro 2022-08-10 06:56:23 PDT
You can find the manifest used to build the tarballs in Tools/gtk/manifest.txt.in. Notice that all cocoa/ and even mac/ directories are excluded from the tarball. Should be somehow possible to continue to do that.
Comment 3 echassiers.09-regards 2022-08-10 08:00:28 PDT
Yeah, but Source/WTF/wtf/spi/darwin/ is included somehow...
Comment 4 Michael Catanzaro 2022-08-10 08:09:56 PDT
I think we do want to include darwin/ directories because those are required to build on macOS. We should probably add spi/ to the list of directories to exclude, though.
Comment 5 Michael Catanzaro 2022-08-10 08:10:59 PDT
(In reply to Michael Catanzaro from comment #4)
> I think we do want to include darwin/ directories because those are required
> to build on macOS. We should probably add spi/ to the list of directories to
> exclude, though.

(BTW, I'm just guessing about this. You're more familiar with macOS than I am. But I am fairly sure we don't want anything under spi/ to be built.)
Comment 6 echassiers.09-regards 2022-08-10 09:14:24 PDT
I found that in the files

Source/WTF/wtf/WTFConfig.cpp (Bug #222086)
Source/WTF/wtf/posix/OSAllocatorPOSIX.cpp (Bug #235720)

Cocoa headers are included with an #if OS(DARWIN) instead of #if PLATFORM(COCOA), and that seems problematic.
Comment 7 echassiers.09-regards 2022-08-10 14:23:55 PDT
(In reply to echassiers.09-regards from comment #6)
> I found that in the files
> 
> Source/WTF/wtf/WTFConfig.cpp (Bug #222086)
> Source/WTF/wtf/posix/OSAllocatorPOSIX.cpp (Bug #235720)
> 
> Cocoa headers are included with an #if OS(DARWIN) instead of #if
> PLATFORM(COCOA), and that seems problematic.

And these are effectively the only files that need to be fixed. Tested on my own machine.
Comment 8 Michael Catanzaro 2022-08-10 14:35:09 PDT
If you have a fix that works, want to try submitting a pull request?
Comment 9 echassiers.09-regards 2022-08-10 14:46:09 PDT
(In reply to Michael Catanzaro from comment #8)
> If you have a fix that works, want to try submitting a pull request?

I have a patch in my GitHub repo that fixes WTF errors when building WebKitGTK, which might seem too immature since I'm not aware of the Contributing Code.

https://github.com/wegank/nur-packages/blob/6f9b8a3e25157be742df286f93c3f0a3648cc4eb/pkgs/development/libraries/webkitgtk/fix-wtf-errors-on-darwin.patch
Comment 10 Michael Catanzaro 2022-08-10 14:50:54 PDT
That actually looks good to me. Seems you've disentangled things nicely.

If you aren't comfortable with submitting your own pull request on GitHub, I can submit it for you.
Comment 11 echassiers.09-regards 2022-08-10 15:00:48 PDT
(In reply to Michael Catanzaro from comment #10)
> That actually looks good to me. Seems you've disentangled things nicely.
> 
> If you aren't comfortable with submitting your own pull request on GitHub, I
> can submit it for you.

Oh, I'd really appreciate it if you could do me a favour.
Comment 12 Michael Catanzaro 2022-08-11 15:33:43 PDT
Pull request: https://github.com/WebKit/WebKit/pull/3246
Comment 13 Michael Catanzaro 2022-08-11 15:37:32 PDT
(In reply to Michael Catanzaro from comment #1)
> That can't be right because the spi is the Apple public API. That should not
> be required for building WebKitGTK, which has a different public API.

It turns out this was an altogether different SPI. Confusing. Whatever. :)

I guess "SPI" is just a different way of saying "API".
Comment 14 EWS 2022-08-11 23:24:17 PDT
Committed 253367@main (6bb3f1342f34): <https://commits.webkit.org/253367@main>

Reviewed commits have been landed. Closing PR #3246 and removing active labels.