Fix overlapping plugins/products build database access #7273
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
This regression was introduced in https://github.com/apple/swift-package-manager/pull/7164/files#diff-7d5950f89d75bc14a591deb8270ebd32308c18248892c7abcb9d0ee3c6b14fd5L201. We can restore the previous behavior in a hacky way by making sure that build parameters passed to plugin builds and invocations have the database path set in the same way as previously. This somewhat defeats the point of tools/products build parameters isolation, but we're able to isolate it only to a few lines in a single function. I've added an explicit test case for this in new `PluginsBuildPlanTests.swift` file, so we can be sure it doesn't regress again. Resolves rdar://120560817
MaxDesiatov
added
bug
plugins
build system
|
@swift-ci test |
2 tasks
|
@swift-ci test linux |
|
@swift-ci test linux |
bnbarham
approved these changes
Jan 19, 2024
euanh
added a commit
that referenced
this pull request
Jan 23, 2024
…the target (#7280) Always build command line plugin dependencies for the host triple. ### Motivation: Since #7164, dependencies of command plugins are once again being built for the _target_ rather than the host. This causes problem when cross compiling because the host needs to be able to run the plugin dependencies, but finds target binaries instead. This problem was fixed before in #6791 by forcing command plugin dependencies to be built for the host by overriding the default build parameters in swiftTool.createBuildSystem(). The same solution still works in this commit, but a better long-term option would be to rework BuildOperation.plan() to handle command plugin dependencies specially, as it already does for build plugin dependencies. ### Modifications: At present, BuildOperation.plan calls graph.invokeBuildToolPlugins to process sources. invokeBuildToolPlugins finds all build tool dependecies and builds them separately, using a specially-created BuildOperation instance: https://github.com/apple/swift-package-manager/blob/34efc0bfe9d40d9a019644ac8fcd0b852c491dfe/Sources/SPMBuildCore/Plugins/PluginInvocation.swift#L409 There is no equivalent step for command plugin dependencies, so they are built for the host architecture. Ideally we should rework BuildOperation.plan to build command and build plugin dependencies in the same way. This commit forces all plugin dependencies to be built for the host - this is similar to what was done in #6791 and #7273. ### Testing: An integration test checks that any targets depended on by a command plugin are built for the host, not for the target. * A new CommandPluginTestStub plugin has a dependency on a target executable which will be built automatically when the plugin is run. The test checks that the dependency is built for the host architecture, no matter which target architecture is selected using '--triple'. * The plugin also asks SwiftPM to build the 'placeholder' main target. The test checks that the dependency is built for the target architecture. The test is restricted to macOS because we can be sure of having a viable cross-compilation environment (arm64 to x86_64 and vice versa). The standard Linux build environments can't cross compile to other architectures. ### Result: Command plugins can be used again when cross-compiling.
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.
This regression was introduced in https://github.com/apple/swift-package-manager/pull/7164/files#diff-7d5950f89d75bc14a591deb8270ebd32308c18248892c7abcb9d0ee3c6b14fd5L201.
We can restore the previous behavior in a hacky way by making sure that build parameters passed to plugin builds and invocations have the database path set in the same way as previously. This somewhat defeats the point of tools/products build parameters isolation, but we're able to isolate it only to a few lines in a single function.
I've added an explicit test case for this in new
PluginsBuildPlanTests.swiftfile, so we can be sure it doesn't regress again.Resolves rdar://120560817