SwiftPM does not build all dependency executables before invoking plugin build command #5636
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@swift-ci please smoke test |
neonichu
approved these changes
Jul 1, 2022
|
Looks like there is at least one test to update (because it codified the previous dependency) in addition to adding the new one. |
abertelrud
force-pushed
the
eng/95759599-plugin-executable-dependencies-not-being-built
branch
from
b16a83a to
c46337c
Compare
|
@swift-ci please smoke test |
abertelrud
force-pushed
the
eng/95759599-plugin-executable-dependencies-not-being-built
branch
from
c46337c to
d653002
Compare
…gin build command Make the list of input dependencies of an llbuild command that runs a plugin-provided build tool also have dependencies on the paths of all the tools on which the plugin has declared a dependency, not just the one that appears as an executable in the build command. Also adds a mildly elaborate unit test to make sure this works. rdar://93679015
abertelrud
force-pushed
the
eng/95759599-plugin-executable-dependencies-not-being-built
branch
from
d653002 to
e26c128
Compare
abertelrud
changed the title
|
@swift-ci please smoke test |
abertelrud
added
the
ready
abertelrud
commented
Jul 4, 2022
| @@ -395,7 +400,7 @@ extension PackageGraph { | |||
| arguments: arguments, | |||
| environment: environment, | |||
| workingDirectory: workingDirectory), | |||
| inputFiles: inputFiles, | |||
| inputFiles: toolPaths + inputFiles, | |||
There was a problem hiding this comment.
This is the key change. Nothing particularly risky or anything that requires extra scrutiny, I think, but it seemed worth calling out amid all the more mundane changes.
tomerd
approved these changes
Jul 5, 2022
abertelrud
added a commit
to abertelrud/swift-package-manager
that referenced
this pull request
Jul 5, 2022
…ng plugin build command (swiftlang#5636) Make the list of input dependencies of an llbuild command that runs a plugin-provided build tool also have dependencies on the paths of all the tools on which the plugin has declared a dependency, not just the one that appears as an executable in the build command. Also adds a mildly elaborate unit test to make sure this works. rdar://93679015 (cherry picked from commit a6ac80f)
abertelrud
deleted the
eng/95759599-plugin-executable-dependencies-not-being-built
branch
abertelrud
added a commit
that referenced
this pull request
Jul 5, 2022
…ng plugin build command (#5636) (#5642) Make the list of input dependencies of an llbuild command that runs a plugin-provided build tool also have dependencies on the paths of all the tools on which the plugin has declared a dependency, not just the one that appears as an executable in the build command. Also adds a mildly elaborate unit test to make sure this works. rdar://93679015 (cherry picked from commit a6ac80f)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Background & Motivation
Plugins can specify dependencies on executables. Currently, those executables get built before running build tool commands returned by the plugin if those dependencies were either:
This could potentially be more efficient than building all dependencies, since a plugin could depend on two separate executables and then return commands that only used one or the other of them. This would result in SwiftPM only building the executables that were actually mentioned by build commands.
However: This led to confusion and multiple bug reports of failures resulting from not building the executables before trying to use them. One common case is with SwiftProtobuf, which uses
protocas the executable and then a SwiftPM-built executable as one of the parameters toprotoc. If the plugin didn't explicitly include the code generation executable in the list of input dependencies in the build tools command it returns, the build would fail and it wouldn't be clear why.Changes
Make the list of input dependencies of an llbuild command that runs a plugin-provided build tool include the paths of all the tools on which the plugin has declared a dependency, not just the one that appears as an executable in the build command.
This can cause more command line tools to be built but is less confusing overall since a dependency on an executable by a plugin understandably implies that the executable should be built no matter what.
Note that if the plugin doesn't return any build commands, none of the dependencies need to be built. That behavior is preserved.
Specific Modifications:
Results
The executables declared by a plugin are built before any build command returned by the plugin (though not if the plugin doesn't return any commands, which is a no-op).
Note also that this new behavior of building the executables up-front more closely matches the behavior in Xcode 14.
rdar://93679015