Bug 200242

Summary: macCatalyst build fails the first attempt, requires a second build
Product: WebKit Reporter: Keith Rollin <krollin>
Component: Tools / TestsAssignee: Keith Rollin <krollin>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, bfulgham, commit-queue, dino, mitz, pvollan, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=193799
Attachments:
Description Flags
Patch none

Keith Rollin
Reported 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>
Attachments
Patch (37.00 KB, patch)
2019-07-29 14:21 PDT, Keith Rollin
no flags
Radar WebKit Bug Importer
Comment 1 2019-07-29 14:12:30 PDT
Keith Rollin
Comment 2 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.
Keith Rollin
Comment 3 2019-07-29 14:21:29 PDT
Keith Rollin
Comment 4 2019-07-31 08:50:32 PDT
Adding a couple more reviewers who may be able to help with entitlements and the build system.
Alex Christensen
Comment 5 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.
Keith Rollin
Comment 6 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.
Brent Fulgham
Comment 7 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?
Keith Rollin
Comment 8 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;
Brent Fulgham
Comment 9 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.
Brent Fulgham
Comment 10 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.
WebKit Commit Bot
Comment 11 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>
WebKit Commit Bot
Comment 12 2019-08-02 11:39:46 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.