Bug 240175 - WebKit has a broken module in Mac Catalyst
Summary: WebKit has a broken module in Mac Catalyst
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified macOS 12
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-05-06 10:54 PDT by Ian Anderson
Modified: 2022-05-09 12:12 PDT (History)
10 users (show)

See Also:

Attachments
Patch (5.05 KB, patch)
2022-05-06 10:57 PDT, Ian Anderson 		no flags Details | Formatted Diff | Diff
Patch (5.07 KB, patch)
2022-05-06 13:05 PDT, Ian Anderson 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Ian Anderson 2022-05-06 10:54:18 PDT 
WebKit has a broken module in Mac Catalyst
Comment 1 Ian Anderson 2022-05-06 10:57:38 PDT 
Created attachment 458964 [details]
Patch
Comment 2 Alexey Proskuryakov 2022-05-06 12:04:52 PDT 
Comment on attachment 458964 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=458964&action=review

> Source/WebKit/ChangeLog:4
> +        https://bugs.webkit.org/show_bug.cgi?id=240175

Is there a radar for this already? Please post the link (no title) to the bug, and also add it to ChangeLog if so.

I'll leave reviewing the actual change to experts.
Comment 3 Tim Horton 2022-05-06 12:23:31 PDT 
Comment on attachment 458964 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=458964&action=review

> Source/WebKit/Configurations/WebKit.xcconfig:35
> +MODULEMAP_FILE = $(MODULEMAP_FILE_$(WK_COCOA_TOUCH)_$(WK_IS_CATALYST));
> +MODULEMAP_FILE_cocoatouch_NO = Modules/iOS.modulemap;
> +MODULEMAP_FILE_cocoatouch_YES = Modules/MacCatalyst.modulemap;
> +MODULEMAP_FILE__NO = Modules/OSX.modulemap;

This is fine, though I think usually when we want to make this distinction we fall back to WK_PLATFORM_NAME instead (but, I admit, that would involve a lot more duplication).
Comment 4 Ian Anderson 2022-05-06 13:05:26 PDT 
Created attachment 458970 [details]
Patch
Comment 5 Alexey Proskuryakov 2022-05-06 13:06:31 PDT 
rdar://92703419
Comment 6 Ian Anderson 2022-05-06 13:07:07 PDT 
(In reply to Alexey Proskuryakov from comment #2)
> Is there a radar for this already? Please post the link (no title) to the
> bug, and also add it to ChangeLog if so.

Yep, I was wondering how to associate the two! It's rdar://92703419 and I added it to the change log as well.
Comment 7 Ian Anderson 2022-05-06 13:10:21 PDT 
(In reply to Tim Horton from comment #3)
> Comment on attachment 458964 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=458964&action=review
> 
> > Source/WebKit/Configurations/WebKit.xcconfig:35
> > +MODULEMAP_FILE = $(MODULEMAP_FILE_$(WK_COCOA_TOUCH)_$(WK_IS_CATALYST));
> > +MODULEMAP_FILE_cocoatouch_NO = Modules/iOS.modulemap;
> > +MODULEMAP_FILE_cocoatouch_YES = Modules/MacCatalyst.modulemap;
> > +MODULEMAP_FILE__NO = Modules/OSX.modulemap;
> 
> This is fine, though I think usually when we want to make this distinction
> we fall back to WK_PLATFORM_NAME instead (but, I admit, that would involve a
> lot more duplication).

Yeah, I couldn't decide which way was less gross. Let me know if you change your mind.
Comment 8 EWS 2022-05-09 12:11:57 PDT 
Committed r293984 (250421@main): <https://commits.webkit.org/250421@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 458970 [details].