Bug 195523 – [Cocoa] Code signing fails because services are copied into XPCServices after the framework is signed

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Darin Adler]

[Cocoa] Code signing frameworks because services are copied into XPCServices after the framework is signed

[Darin Adler]

Created attachment 364140 [details]
Patch

[Darin Adler]

I’m trying what Dan suggested when I asked people at Apple about this problem. This is supposed to solve the problem where we get code signing errors in some builds.

[Darin Adler]

Comment on attachment 364140 [details]
Patch

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

> Source/WebKit/ChangeLog:3
> +        [Cocoa] Code signing frameworks because services are copied into XPCServices after the framework is signed

I meant to say "code signing fails", not "code signing frameworks".

Comment on attachment 364140 [details]
Patch

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

Looks good overall. Should the new script build phase’s output paths be populated with the paths of the symlinks? Maybe there’s no harm in having the script run even if they exist.

Another concern is whether the script needs to check if there exists a directory where it wants to create the symlink, and delete it, so that the transition to this new way of building doesn’t require people to manually delete some of the built product.

> Source/WebKit/WebKit.xcodeproj/project.pbxproj:10675
> +			shellScript = "if [[ \"${CONFIGURATION}\" == \"Production\" ]]; then\n    exit\nfi\n\nif [[ ${WK_PLATFORM_NAME} != \"macosx\" ]]; then\n    XPC_SERVICES_PATH=\"${BUILT_PRODUCTS_DIR}/WebKit.framework/XPCServices\"\n    BUILT_PRODUCTS_DIR_RELATIVE_PATH_FROM_XPC_SERVICES=\"../..\"\nelse\n    XPC_SERVICES_PATH=\"${BUILT_PRODUCTS_DIR}/WebKit.framework/Versions/A/XPCServices\"\n    BUILT_PRODUCTS_DIR_RELATIVE_PATH_FROM_XPC_SERVICES=\"../../../..\"\nfi\n\nmkdir -p \"${XPC_SERVICES_PATH}\"\nln -sfh \"${BUILT_PRODUCTS_DIR_RELATIVE_PATH_FROM_XPC_SERVICES}/com.apple.WebKit.WebContent.xpc\" \"${XPC_SERVICES_PATH}/com.apple.WebKit.WebContent.xpc\"\nln -sfh \"${BUILT_PRODUCTS_DIR_RELATIVE_PATH_FROM_XPC_SERVICES}/com.apple.WebKit.Networking.xpc\" \"${XPC_SERVICES_PATH}/com.apple.WebKit.Networking.xpc\"\nln -sfh \"${BUILT_PRODUCTS_DIR_RELATIVE_PATH_FROM_XPC_SERVICES}/${WK_STORAGE_SERVICE_PRODUCT_NAME}.xpc\" \"${XPC_SERVICES_PATH}/${WK_STORAGE_SERVICE_PRODUCT_NAME}.xpc\"\n\nif [[ ${WK_PLATFORM_NAME} == macosx ]]; then\n    ln -sfh \"${BUILT_PRODUCTS_DIR_RELATIVE_PATH_FROM_XPC_SERVICES}/com.apple.WebKit.Plugin.32.xpc\" \"${XPC_SERVICES_PATH}/com.apple.WebKit.Plugin.xpc\"\n    ln -sfh \"${BUILT_PRODUCTS_DIR_RELATIVE_PATH_FROM_XPC_SERVICES}/com.apple.WebKit.Plugin.64.xpc\" \"${XPC_SERVICES_PATH}/com.apple.WebKit.Plugin.64.xpc\"\nfi\n";

We can remove references to the 32-bit plug-in service, which no longer exists.

[EWS Watchlist]

Comment on attachment 364140 [details]
Patch

Attachment 364140 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/11441126

Number of test failures exceeded the failure limit.

[EWS Watchlist]

Created attachment 364141 [details]
Archive of layout-test-results from ews106 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6

[Darin Adler]

(In reply to mitz from comment #4)
> Should the new script build phase’s output paths be
> populated with the paths of the symlinks? Maybe there’s no harm in having
> the script run even if they exist.

I like the idea of having the script not run if it doesn’t need to. I don’t know how to make sure the build system does run the script whenever it does need to. I think the times it needs to run are:

1) When the symlinks, and perhaps the entire XPCServices directory, haven’t yet been created.

2) When the directory is there and populated with frameworks rather than symlinks to frameworks. I think that I can switch from the "-f" option in the ln command to the "-F" option so that it will delete even entire framework directories when writing the symlinks. I have two ideas about how to have the build system run the script in such cases: a) Do not list the output files so that the script runs every time, b) list one additional output file that the old script never created, a "dummy" one if necessary.

3) Hypothetically in the future, when symlinks are there, but have incorrect paths or are otherwise incorrect because they were created by an earlier version of the script. I believe the script will overwrite such links because I used the "-f" option in the ln command. Again, I need to get the build system to run the script. One idea is to keep changing the name of the dummy file each time the script needs to be re-run.

> Another concern is whether the script needs to check if there exists a
> directory where it wants to create the symlink, and delete it, so that the
> transition to this new way of building doesn’t require people to manually
> delete some of the built product.

I think passing the "-F" option to the ln command should take care of it, but I’m not sure how to test.

> We can remove references to the 32-bit plug-in service, which no longer
> exists.

Thanks. I will do that.

[Darin Adler]

Looks like all the tests crashed on the bots. I don’t know why.

[Darin Adler]

Here are the things that worry me about putting names in outputPaths:

1) I don’t understand why it’s OK to use the same output paths on macOS as on iOS. Perhaps the symlinks already created as part of the bundle structure make the paths work fine on both, even though they are not the same as the paths we use in the script on macOS. But if so then I don’t understand why we need the XPC_SERVICES_PATH environment variable inside the build phase script itself.

2) I don’t understand why it’s OK to list macOS-only paths that will never be created on iOS. It seems that would lead to the script being executed every time on iOS.

3) I wonder if I will actually need to add a dummy as described above to help people successfully compile with their existing trees, since all the frameworks will be there, they just will be old versions. It’s quite important that those get replaced with symlinks!

Because of those three, I didn’t add outputPaths yet in this upcoming patch.

[Darin Adler]

Created attachment 364143 [details]
Patch

[Darin Adler]

Committed r242686: <https://trac.webkit.org/changeset/242686>

[Radar WebKit Bug Importer]

<rdar://problem/48750841>

[Alexey Proskuryakov]

This caused rdar://problem/49542244.