Pass through the toolchain to use for manifest loading #7639
Conversation
bnbarham
requested review from
MaxDesiatov,
jakepetroules,
francescomikulis and
xedin
as code owners
| @@ -128,6 +129,7 @@ public struct BuildDescription { | |||
| return PluginTargetBuildDescription( | |||
| target: target, | |||
| toolsVersion: package.manifest.toolsVersion, | |||
| toolchain: buildPlan.toolsBuildParameters.toolchain as? UserToolchain, | |||
There was a problem hiding this comment.
I very much dislike this, but ManifestLoader has a bunch of uses of members in UserToolchain that are not in Toolchain. Should we add requirements for them all?
There was a problem hiding this comment.
If they make sense for Toolchain protocol, why not?
There was a problem hiding this comment.
Do we have any other implementations of Toolchain other than UserToolchain and MockToolchain? If not, maybe we should reconsider our approach to mocking that doesn't require maintaining a separate protocol?
There was a problem hiding this comment.
There is a FIXME right on top of UserToolchain, might be the time to make it right...
| let isPartOfRootPackage: Bool | ||
|
|
||
| init(target: ResolvedModule, toolsVersion: ToolsVersion, isPartOfRootPackage: Bool) { | ||
| init(target: ResolvedModule, toolsVersion: ToolsVersion, toolchain: UserToolchain?, isPartOfRootPackage: Bool) { |
There was a problem hiding this comment.
How about we pass BuildParameters here which should allow us make toolchain field non-Optional, we can assign it this way in the initializer:
self.toolchain = if let toolchain = buildParameters.toolchain as? UserToolchain {
toolchain
} else {
UserToolchain(swiftSDK: .hostSwiftSDK()))
}There was a problem hiding this comment.
Ended up adding a static method to get the arguments and then adding swiftPMLibrariesLocation to Toolchain, which then gets rid of this optional problem entirely.
Prefer the toolchain provided in the build plan rather than the default host toolchain so that the returned arguments align with the toolchain used to construct the build plan.
bnbarham
force-pushed
the
plugin-toolchain
branch
from
2c0ef73 to
495894a
Compare
|
@swift-ci please test |
Prefer the toolchain provided in the build plan rather than the default host toolchain so that the returned arguments align with the toolchain used to construct the build plan.