Bug 200242 - macCatalyst build fails the first attempt, requires a second build
Summary: macCatalyst build fails the first attempt, requires a second build
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Keith Rollin
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-07-29 14:11 PDT by Keith Rollin
Modified: 2019-08-02 11:39 PDT (History)
8 users (show)

See Also:


Attachments
Patch (37.00 KB, patch)
2019-07-29 14:21 PDT, Keith Rollin
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Keith Rollin 2019-07-29 14:11:49 PDT
macCatalyst builds fail the first time with an error like:

Code Signing Error: The file "/Users/tim_cook/Build/Debug-maccatalyst/DerivedSources/WebKit2/WebContent-macCatalyst-no-sandbox.entitlements" could not be opened. Verify the value of the CODE_SIGN_ENTITLEMENTS build setting for target "WebContent" is correct and that the file exists on disk.

This problem is caused by the file referenced by CODE_SIGN_ENTITLEMENTS changing during the build process. For macCatalyst builds, we start with the iOS entitlements files and then tweak them for macCatalyst. When this occurs during a clean build, Xcode sees the entitlements file being generated and complains about it. Restarting the build does so with the file already existing, and so Xcode does not complain about it.

The approach of generating or tweaking entitlement files may have worked in the past, but the fact is that Xcode doesn't support it.

We had a similar problem with macOS builds. The entitlements files used to be generated on the fly with scripts like WebKit/Scripts/process-network-sandbox-entitlements.sh. That process was reworked to avoid the issue with Xcode not allowing the files to be generated (see r241135). In short:

o The various process-*-entitlements.sh scripts were consolidated into a single process-entitlements file
o CODE_SIGN_ENTITLEMENTS, which contains the name of the entitlements file to use, was de-initialized so that Xcode would not try to access our generated entitlements file
o CODE_SIGN_INJECT_BASE_ENTITLEMENTS (which injects some base entitlements) was set to NO. If it were left set to YES, Xcode would create its own entitlements file and use it as if it were specified in CODE_SIGN_ENTITLEMENTS
o WK_LIBRARY_VALIDATION_CODE_SIGN_FLAGS was updated with an "--entitlements <generated_file>" option. WK_LIBRARY_VALIDATION_CODE_SIGN_FLAGS was then used to initialize OTHER_CODE_SIGN_FLAGS. By specifying the entitlements file this way, we avoid Xcode complaining about it.

This approach works well for macOS, and so we now also use it to address the issue with macCatalyst. While we're at it, convert the rest of the platforms to use the same approach and also generate their entitlements from the process-entitlements script.


<rdar://problem/52976416>
Comment 1 Radar WebKit Bug Importer 2019-07-29 14:12:30 PDT
<rdar://problem/53678481>
Comment 2 Keith Rollin 2019-07-29 14:18:52 PDT
The new process was validated by performing a build with the old process and the new process, and then comparing the entitlements of the resulting XPC services to make sure they were the same. Builds were performed for all platforms, and for Engineering and Production builds.
Comment 3 Keith Rollin 2019-07-29 14:21:29 PDT
Created attachment 375103 [details]
Patch
Comment 4 Keith Rollin 2019-07-31 08:50:32 PDT
Adding a couple more reviewers who may be able to help with entitlements and the build system.
Comment 5 Alex Christensen 2019-07-31 09:59:57 PDT
Does it not work if there's a separate build target that processes the entitlements before the other webkit build targets start? I would imagine Xcode wouldn't even look at the value of CODE_SIGN_ENTITLEMENTS until it starts building that target, and if a previous target had already done the processing, then the entitlements file would exist and not need to be modified any more.
Comment 6 Keith Rollin 2019-07-31 10:29:06 PDT
That might work with the old build system. I haven't tried it. But it's less likely to work with XCBuild, which takes a larger overview when building and would likely expect/require that the entitlements files exist even across build targets.

Even if we were to consider that approach and it worked, it would still leave us with two different mechanisms for managing entitlements files. This patch not only addresses the build problem, but it also unifies the approaches into just a single approach. This single approach is based on a single process-entitlements.sh script file, which better allows us to see what entitlements are used in what builds and which better allows us to verify and adjust the entitlements. In the old approach, everything was spread out across many entitlements files -- some of which got tweaked -- which were hard to validate.
Comment 7 Brent Fulgham 2019-08-01 17:11:15 PDT
Comment on attachment 375103 [details]
Patch

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

I think this looks right, but please confirm we aren't missing 'seatbelt-profile' stuff in the generated build.

> Source/WebKit/Scripts/process-entitlements.sh:77
> +    plistbuddy Add :com.apple.security.cs.allow-jit bool YES

Does this need a seatbelt-profiles array? It was present in the original 'entitlements' file.

    plistbuddy Add :seatbelt-profiles array
    plistbuddy Add :seatbelt-profiles:0 string com.apple.WebKit.WebContent

> Source/WebKit/Scripts/process-entitlements.sh:83
> +    plistbuddy Add :com.apple.security.network.client bool YES

Does this need a seatbelt-profiles array? It was present in the original 'entitlements' file.

    plistbuddy Add :seatbelt-profiles array
    plistbuddy Add :seatbelt-profiles:0 string com.apple.WebKit.Networking

> Source/WebKit/Scripts/process-entitlements.sh:142
> +    plistbuddy Add :com.apple.security.print                               bool YES

I don't believe we allow plugin process on iOS. Was there an entitlements file for it?
Comment 8 Keith Rollin 2019-08-02 08:22:07 PDT
(In reply to Brent Fulgham from comment #7)
> Comment on attachment 375103 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=375103&action=review
> 
> I think this looks right, but please confirm we aren't missing
> 'seatbelt-profile' stuff in the generated build.

I'm pretty sure it's right. I should elaborate. In the ChangeLog, I say:

    The new process was validated by performing a build with the old
    process and the new process, and then comparing the entitlements of
    the resulting XPC services to make sure they were the same. Builds
    were performed for all platforms, and for Engineering and Production
    builds.

This comparison was not eyeballed. I built all products using both the old and new methods. I then wrote a script that dumped the entitlements and compared them using `diff`. I'll double-check this approach by eyeballing the results and looking for the differences you ask about, but -- initially -- I think the results consistent.


> > Source/WebKit/Scripts/process-entitlements.sh:77
> > +    plistbuddy Add :com.apple.security.cs.allow-jit bool YES
> 
> Does this need a seatbelt-profiles array? It was present in the original
> 'entitlements' file.
> 
>     plistbuddy Add :seatbelt-profiles array
>     plistbuddy Add :seatbelt-profiles:0 string com.apple.WebKit.WebContent
> 
> > Source/WebKit/Scripts/process-entitlements.sh:83
> > +    plistbuddy Add :com.apple.security.network.client bool YES
> 
> Does this need a seatbelt-profiles array? It was present in the original
> 'entitlements' file.
> 
>     plistbuddy Add :seatbelt-profiles array
>     plistbuddy Add :seatbelt-profiles:0 string com.apple.WebKit.Networking
> 
> > Source/WebKit/Scripts/process-entitlements.sh:142
> > +    plistbuddy Add :com.apple.security.print                               bool YES


The "Derive Entitlements for Manual Sandboxing" Build Phase removes seatbelt-profiles if WK_MANUAL_SANDBOXING_ENABLED is YES. WK_MANUAL_SANDBOXING_ENABLED is set to YES only for the macOS SDK. macCatalyst projects are built against the macOS SDK so WK_MANUAL_SANDBOXING_ENABLED  is YES for for macCatalyst. Thus, seatbelt-profiles gets removed for macCatalyst.


> I don't believe we allow plugin process on iOS. Was there an entitlements
> file for it?


This was in Source/WebKit/Configurations/PluginService.64.xcconfig:

CODE_SIGN_ENTITLEMENTS_COCOA_TOUCH_YES = Configurations/PluginService.entitlements;
Comment 9 Brent Fulgham 2019-08-02 09:32:35 PDT
Comment on attachment 375103 [details]
Patch

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

>>> Source/WebKit/Scripts/process-entitlements.sh:77
>>> +    plistbuddy Add :com.apple.security.cs.allow-jit bool YES
>> 
>> Does this need a seatbelt-profiles array? It was present in the original 'entitlements' file.
>> 
>>     plistbuddy Add :seatbelt-profiles array
>>     plistbuddy Add :seatbelt-profiles:0 string com.apple.WebKit.WebContent
> 
> The "Derive Entitlements for Manual Sandboxing" Build Phase removes seatbelt-profiles if WK_MANUAL_SANDBOXING_ENABLED is YES. WK_MANUAL_SANDBOXING_ENABLED is set to YES only for the macOS SDK. macCatalyst projects are built against the macOS SDK so WK_MANUAL_SANDBOXING_ENABLED  is YES for for macCatalyst. Thus, seatbelt-profiles gets removed for macCatalyst.

Got it. Looks good.
Comment 10 Brent Fulgham 2019-08-02 09:33:05 PDT
This looks correct, and you've answered all my questions. Please get it landed when you have a chance.
Comment 11 WebKit Commit Bot 2019-08-02 11:39:44 PDT
Comment on attachment 375103 [details]
Patch

Clearing flags on attachment: 375103

Committed r248164: <https://trac.webkit.org/changeset/248164>
Comment 12 WebKit Commit Bot 2019-08-02 11:39:46 PDT
All reviewed patches have been landed.  Closing bug.