We create /System/Library/LaunchDaemons/ for builds that don't install plists in that path #1866
Conversation
msaboff
added
Platform
|
Looks good to me. I think it would also be possible to resolve this by giving the script phases an output path that's defined by one build setting that evaluates to an empty string when the plists won't be installed. But as you mentioned, that isn't really important since the plists are "leaf nodes", and no other operation will ever need to block on creating them. |
There was a problem hiding this comment.
If we remove all output paths, the script always runs. I know this script is relatively fast, but as I'm trying to improve incremental build performance, this would be a (small) step in the wrong direction.
I would eventually like to see these scripts rewritten as a build rule, because then—among other benefits—we could set EXCLUDED_SOURCE_FILE_NAMES = com.apple.webpushd.plist and prevent the script from ever executing. But in the meantime, could we do something like what @davidquesada is suggesting?
webkit-early-warning-system
force-pushed
the
eng/We-create-SystemLibraryLaunchDaemons-for-builds-that-dont-install-products-in-the-path
branch
from
a6204dd
to
b0e1099
Compare
|
Testing a version where the output path is defined by one build setting that evaluates to an empty string when the plists won't be installed as @davidquesada suggested. |
msaboff
force-pushed
the
eng/We-create-SystemLibraryLaunchDaemons-for-builds-that-dont-install-products-in-the-path
branch
from
b0e1099
to
7a16e68
Compare
|
@emw-apple and @davidquesada Updated the patch as suggested and tested it locally. |
| @@ -16153,7 +16153,7 @@ | |||
| outputFileListPaths = ( | |||
| ); | |||
| outputPaths = ( | |||
| "$(DSTROOT)$(LAUNCHD_PLIST_INSTALL_PATH)/$(LAUNCHD_PLIST_FILE_NAME)", | |||
| "$(DERIVED_FILE_DIR)/newOutputFile", | |||
There was a problem hiding this comment.
I think this is supposed to be:
| "$(DERIVED_FILE_DIR)/newOutputFile", | |
| "$(LAUNCHD_PLIST_OUTPUT_FILE)", |
There was a problem hiding this comment.
Agreed. I made that change, so I don't know why it didn't take. I'll post an updated PR.
There was a problem hiding this comment.
r=me once this is updated!
msaboff
force-pushed
the
eng/We-create-SystemLibraryLaunchDaemons-for-builds-that-dont-install-products-in-the-path
branch
from
7a16e68
to
400985a
Compare
…l plists in that path https://bugs.webkit.org/show_bug.cgi?id=242081 <rdar://96013133> Reviewed by Saam Barati. The Output Files for the Copy launchd plist and create symlink script phases for the adattributed and webpushd targets will create the containing folder of any of the files listed there, even though the script creates the containing directory and file when needed. Therefore we changed the Output Files specified to have a computed path value when installing a plist, or an empty value when we aren't. This output path value is based on the SKIP_INSTALL configuration variable. * Source/WebKit/Configurations/adattributiond.xcconfig: * Source/WebKit/Configurations/webpushd.xcconfig: * Source/WebKit/Scripts/copy-launchd-plist-and-create-symlink.sh: * Source/WebKit/WebKit.xcodeproj/project.pbxproj: Canonical link: https://commits.webkit.org/251969@main
webkit-early-warning-system
force-pushed
the
eng/We-create-SystemLibraryLaunchDaemons-for-builds-that-dont-install-products-in-the-path
branch
from
400985a
to
d0b2da3
Compare
|
Committed 251969@main (d0b2da3): https://commits.webkit.org/251969@main Reviewed commits have been landed. Closing PR #1866 and removing active labels. |
webkit-commit-queue
removed
the
merge-queue
d0b2da3