Bug 237394 – Copy WebKit frameworks and XPC processes to Secondary Path

• Component: Platform
• Status: RESOLVED FIXED

[Michael Saboff]

Copy the various WebKit Frameworks, XPC services and other dylibs to the SYSTEM_SECONDARY_CONTENT_PATH when it is set.

For the copied executables, we also need to update the binaries to use the frameworks copied to SYSTEM_SECONDARY_CONTENT_PATH.

[Michael Saboff]

<rdar://89053248>
<rdar://86558002>

[Michael Saboff]

Created attachment 453681 [details]
Patch

[EWS Watchlist]

Note that there are important steps to take when updating ANGLE. See https://trac.webkit.org/wiki/UpdatingANGLE

[Elliott Williams]

Comment on attachment 453681 [details]
Patch

Some high-level feedback:

At first glance, it looks like you could do this with less code by adding Copy Files phases to the Xcode targets, which would copy their target's product to $(SECONDARY_STAGED_FRAMEWORK_DIRECTORY). These can be configured to be install-only and Mac-only. It might be worth trying this out before we commit to adding a new rsync script.

If you do stick with a script, please put it in WTF's Scripts/ directory. Those scripts get copied during WTF's installhdrs and are available for other projects to use, to save us from checking in duplicate copies :)

[Saam Barati]

Comment on attachment 453681 [details]
Patch

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

> Source/WebGPU/Scripts/copy-frameworks-to-secondary-path.sh:1
> +#!/bin/sh

Do we really need all the different versions of this script? There must be a better way.

[Alexey Proskuryakov]

Comment on attachment 453681 [details]
Patch

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

> Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj:11931
> +			inputFileListPaths = (

I think that we are going to need inputs for script phases for the modern build system.

[Richard Houle]

As the original implementor of mach_o.py, this patch looks good to me.

[Michael Saboff]

(In reply to Saam Barati from comment #5)
> Comment on attachment 453681 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=453681&action=review
> 
> > Source/WebGPU/Scripts/copy-frameworks-to-secondary-path.sh:1
> > +#!/bin/sh
> 
> Do we really need all the different versions of this script? There must be a
> better way.

Our internal build system builds one project at a time.  Therefore only the files in that projects tree are available.

[Michael Saboff]

(In reply to Elliott Williams from comment #4)
> Comment on attachment 453681 [details]
> Patch
> 
> Some high-level feedback:
> 
> At first glance, it looks like you could do this with less code by adding
> Copy Files phases to the Xcode targets, which would copy their target's
> product to $(SECONDARY_STAGED_FRAMEWORK_DIRECTORY). These can be configured
> to be install-only and Mac-only. It might be worth trying this out before we
> commit to adding a new rsync script.

This was patterned after similar scripts in the Safari source tree that do the same types of copying and modifying.

That may work for the copy-frameworks-to-secondary-path.sh script, but for copy-xpc-services-to-secondary-path.sh  we'd still need a script to update DYLD_VERSIONED_FRAMEWORK_PATH for the XPC services.

Also, we read the symlink found at ${BUILT_PRODUCTS_DIR}/${PRODUCT_COMPONENT} to get the real file or directory to copy over.

> If you do stick with a script, please put it in WTF's Scripts/ directory.
> Those scripts get copied during WTF's installhdrs and are available for
> other projects to use, to save us from checking in duplicate copies :)

We don't copy the WTF scripts to the public SDK.  This would probably break Xcode builds for open source contributors.

[Elliott Williams]

(In reply to Michael Saboff from comment #9)
> (In reply to Elliott Williams from comment #4)
> > Comment on attachment 453681 [details]
> > Patch
> > 
> > Some high-level feedback:
> > 
> > At first glance, it looks like you could do this with less code by adding
> > Copy Files phases to the Xcode targets, which would copy their target's
> > product to $(SECONDARY_STAGED_FRAMEWORK_DIRECTORY). These can be configured
> > to be install-only and Mac-only. It might be worth trying this out before we
> > commit to adding a new rsync script.
> 
> This was patterned after similar scripts in the Safari source tree that do
> the same types of copying and modifying.
> 
> That may work for the copy-frameworks-to-secondary-path.sh script, but for
> copy-xpc-services-to-secondary-path.sh  we'd still need a script to update
> DYLD_VERSIONED_FRAMEWORK_PATH for the XPC services.
>
> Also, we read the symlink found at
> ${BUILT_PRODUCTS_DIR}/${PRODUCT_COMPONENT} to get the real file or directory
> to copy over.

Thanks for the explanation. I still think it would be worth trying at some point (in both trees), but not so high-value that we should block now. FWIW, I believe Xcode's copy phases resolve symlinks, so that particular need would be met.


> > If you do stick with a script, please put it in WTF's Scripts/ directory.
> > Those scripts get copied during WTF's installhdrs and are available for
> > other projects to use, to save us from checking in duplicate copies :)
> 
> We don't copy the WTF scripts to the public SDK.  This would probably break
> Xcode builds for open source contributors.

WTF scripts don't make it to the public SDK, but they are always available in engineering builds at $(BUILT_PRODUCTS_DIR)/usr/local/include/wtf/Scripts. This is how, for instance, generate-unified-source-bundles.rb works. When WTF builds, Source/WTF/Scripts/generate-unified-source-bundles.rb is copied to /usr/local/include/wtf/Scripts. Build phases in WebCore and JSC execute it from that path to create their respective unified sources.

AFAICT, this script would be used in the same way, so it should be able to live in Source/WTF/Scripts/ too. Is there something I'm missing here?

[Elliott Williams]

(In reply to Alexey Proskuryakov from comment #6)
> Comment on attachment 453681 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=453681&action=review
> 
> > Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj:11931
> > +			inputFileListPaths = (
> 
> I think that we are going to need inputs for script phases for the modern
> build system.

IMO it is fine to omit them for now. XCBuild will run the script unconditionally if it has no inputs, and since this script is only run for `install` actions, running it unconditionally does not affect typical incremental builds.

We would need to add them if and when we start sandboxing our scripts, though.

[Saam Barati]

Comment on attachment 453681 [details]
Patch

r=me

[Saam Barati]

(In reply to Elliott Williams from comment #10)
> (In reply to Michael Saboff from comment #9)
> > (In reply to Elliott Williams from comment #4)
> > > Comment on attachment 453681 [details]
> > > Patch
> > > 
> > > Some high-level feedback:
> > > 
> > > At first glance, it looks like you could do this with less code by adding
> > > Copy Files phases to the Xcode targets, which would copy their target's
> > > product to $(SECONDARY_STAGED_FRAMEWORK_DIRECTORY). These can be configured
> > > to be install-only and Mac-only. It might be worth trying this out before we
> > > commit to adding a new rsync script.
> > 
> > This was patterned after similar scripts in the Safari source tree that do
> > the same types of copying and modifying.
> > 
> > That may work for the copy-frameworks-to-secondary-path.sh script, but for
> > copy-xpc-services-to-secondary-path.sh  we'd still need a script to update
> > DYLD_VERSIONED_FRAMEWORK_PATH for the XPC services.
> >
> > Also, we read the symlink found at
> > ${BUILT_PRODUCTS_DIR}/${PRODUCT_COMPONENT} to get the real file or directory
> > to copy over.
> 
> Thanks for the explanation. I still think it would be worth trying at some
> point (in both trees), but not so high-value that we should block now. FWIW,
> I believe Xcode's copy phases resolve symlinks, so that particular need
> would be met.
> 
> 
> > > If you do stick with a script, please put it in WTF's Scripts/ directory.
> > > Those scripts get copied during WTF's installhdrs and are available for
> > > other projects to use, to save us from checking in duplicate copies :)
> > 
> > We don't copy the WTF scripts to the public SDK.  This would probably break
> > Xcode builds for open source contributors.
> 
> WTF scripts don't make it to the public SDK, but they are always available
> in engineering builds at
> $(BUILT_PRODUCTS_DIR)/usr/local/include/wtf/Scripts. This is how, for
> instance, generate-unified-source-bundles.rb works. When WTF builds,
> Source/WTF/Scripts/generate-unified-source-bundles.rb is copied to
> /usr/local/include/wtf/Scripts. Build phases in WebCore and JSC execute it
> from that path to create their respective unified sources.
> 
> AFAICT, this script would be used in the same way, so it should be able to
> live in Source/WTF/Scripts/ too. Is there something I'm missing here?

Michael, maybe you can file a FIXME for this, and we can improve it in the future? Would be nice to remove the duplication.

[Michael Saboff]

Committed r290805 (248044@trunk): <https://commits.webkit.org/248044@trunk>