Skip to content

command plugins: Build command plugin dependencies for the host, not the target #7280

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

euanh
Copy link
Contributor

@euanh euanh commented Jan 22, 2024

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.

Result:

Command plugins can be used again when cross-compiling.

@euanh
Copy link
Contributor Author

euanh commented Jan 22, 2024

@swift-ci test

@euanh
Copy link
Contributor Author

euanh commented Jan 22, 2024

It's surprisingly tricky to write an automated test for this. On macOS, a Swift installation can typically build for either x86_64 or arm64, so the test can target the 'opposite' architecture and make sure that the plugin dependencies are build for the host. On Linux, without cross-compilation SDKs installed, it's typically only possible to build for the host triple. Trying to target an alternative triple will result in a build failure because SwiftPM still tries to build the dependency for host and target architectures, but doesn't have the standard library available.

[root@e39215bc76c5 workspace]# swift package --triple arm64-unknown-linux-gnu plugin test
Building for debugging...
error: emit-module command failed with exit code 1 (use -v to see invocation)
<unknown>:0: error: could not find module '_Concurrency' for target 'arm64-unknown-linux-gnu'; found: x86_64-unknown-linux-gnu, at: /usr/lib/swift/linux/_Concurrency.swiftmodule
<unknown>:0: error: could not find module '_Concurrency' for target 'arm64-unknown-linux-gnu'; found: x86_64-unknown-linux-gnu, at: /usr/lib/swift/linux/_Concurrency.swiftmodule
error: fatalError

error: fatalError

If BuildOperation.plan handled command plugin dependencies in the same way as it handles build plugin dependencies, we could test it in a unit test. I haven't yet been able to make a suitable change in BuildOperation.plan, so this PR just makes command plugins work again in the short term.

@euanh euanh force-pushed the cross-compilation-with-plugin-dependencies branch from 9483c12 to 81f0f8b Compare January 22, 2024 11:45
…the target

Since swiftlang#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 swiftlang#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.

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.
@euanh euanh force-pushed the cross-compilation-with-plugin-dependencies branch from 81f0f8b to 8d7a047 Compare January 22, 2024 11:48
@euanh
Copy link
Contributor Author

euanh commented Jan 22, 2024

@swift-ci test

@MaxDesiatov
Copy link
Contributor

@swift-ci test windows

@MaxDesiatov MaxDesiatov self-assigned this Jan 22, 2024
Copy link
Contributor

@MaxDesiatov MaxDesiatov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you be able to come up with a test that prevents possible future regressions in this area?

@MaxDesiatov MaxDesiatov added plugins cross-compilation needs tests This change needs test coverage labels Jan 22, 2024
@MaxDesiatov MaxDesiatov assigned euanh and unassigned MaxDesiatov Jan 22, 2024
@euanh
Copy link
Contributor Author

euanh commented Jan 22, 2024

Would you be able to come up with a test that prevents possible future regressions in this area?

As I mentioned in the comment above (#7280 (comment)), I think it's possible to have a macOS-only integration test, but I can't see a way to do the same for Linux.

@MaxDesiatov
Copy link
Contributor

Ah, sorry I missed that. Yes, macOS-only test is fine. There are at least a few tests that are macOS-only already.

This 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.
@euanh
Copy link
Contributor Author

euanh commented Jan 23, 2024

@swift-ci test

@euanh euanh requested a review from MaxDesiatov January 23, 2024 12:41
@euanh
Copy link
Contributor Author

euanh commented Jan 23, 2024

@swift-ci test windows

Copy link
Contributor

@MaxDesiatov MaxDesiatov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@euanh euanh enabled auto-merge (squash) January 23, 2024 14:20
@MaxDesiatov MaxDesiatov added bug and removed needs tests This change needs test coverage labels Jan 23, 2024
@euanh euanh merged commit ca2fe64 into swiftlang:main Jan 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants