Skip to content

Commit 41405ac

Browse files
committed
Pass through the toolchain to use for manifest loading
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.
1 parent 844bd13 commit 41405ac

File tree

6 files changed

+40
-44
lines changed

6 files changed

+40
-44
lines changed

Sources/PackageLoading/ManifestLoader.swift

Lines changed: 12 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -289,7 +289,6 @@ public final class ManifestLoader: ManifestLoaderProtocol {
289289
public var delegate: Delegate?
290290

291291
private let databaseCacheDir: AbsolutePath?
292-
private let sdkRootCache = ThreadSafeBox<AbsolutePath>()
293292

294293
private let useInMemoryCache: Bool
295294
private let memoryCache = ThreadSafeKeyValueStore<CacheKey, ManifestJSONParser.Result>()
@@ -1114,35 +1113,12 @@ public final class ManifestLoader: ManifestLoaderProtocol {
11141113
}
11151114
}
11161115

1117-
/// Returns path to the sdk, if possible.
1118-
private func sdkRoot() -> AbsolutePath? {
1119-
if let sdkRoot = self.sdkRootCache.get() {
1120-
return sdkRoot
1121-
}
1122-
1123-
var sdkRootPath: AbsolutePath? = nil
1124-
// Find SDKROOT on macOS using xcrun.
1125-
#if os(macOS)
1126-
let foundPath = try? AsyncProcess.checkNonZeroExit(
1127-
args: "/usr/bin/xcrun", "--sdk", "macosx", "--show-sdk-path")
1128-
guard let sdkRoot = foundPath?.spm_chomp(), !sdkRoot.isEmpty else {
1129-
return nil
1130-
}
1131-
if let path = try? AbsolutePath(validating: sdkRoot) {
1132-
sdkRootPath = path
1133-
self.sdkRootCache.put(path)
1134-
}
1135-
#endif
1136-
1137-
return sdkRootPath
1138-
}
1139-
1140-
/// Returns the interpreter flags for a manifest.
1141-
public func interpreterFlags(
1142-
for toolsVersion: ToolsVersion
1116+
package static func interpreterFlags(
1117+
for toolsVersion: ToolsVersion,
1118+
toolchain: some Toolchain
11431119
) -> [String] {
11441120
var cmd = [String]()
1145-
let modulesPath = self.toolchain.swiftPMLibrariesLocation.manifestModulesPath
1121+
let modulesPath = toolchain.swiftPMLibrariesLocation.manifestModulesPath
11461122
cmd += ["-swift-version", toolsVersion.swiftLanguageVersion.rawValue]
11471123
// if runtimePath is set to "PackageFrameworks" that means we could be developing SwiftPM in Xcode
11481124
// which produces a framework for dynamic package products.
@@ -1152,14 +1128,21 @@ public final class ManifestLoader: ManifestLoaderProtocol {
11521128
cmd += ["-I", modulesPath.pathString]
11531129
}
11541130
#if os(macOS)
1155-
if let sdkRoot = self.toolchain.sdkRootPath ?? self.sdkRoot() {
1131+
if let sdkRoot = toolchain.sdkRootPath {
11561132
cmd += ["-sdk", sdkRoot.pathString]
11571133
}
11581134
#endif
11591135
cmd += ["-package-description-version", toolsVersion.description]
11601136
return cmd
11611137
}
11621138

1139+
/// Returns the interpreter flags for a manifest.
1140+
public func interpreterFlags(
1141+
for toolsVersion: ToolsVersion
1142+
) -> [String] {
1143+
return Self.interpreterFlags(for: toolsVersion, toolchain: toolchain)
1144+
}
1145+
11631146
/// Returns path to the manifest database inside the given cache directory.
11641147
private static func manifestCacheDBPath(_ cacheDir: AbsolutePath) -> AbsolutePath {
11651148
return cacheDir.appending("manifest.db")

Sources/PackageModel/Toolchain.swift

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,9 @@ public protocol Toolchain {
4343
/// The root path to the Swift SDK used by this toolchain.
4444
var sdkRootPath: AbsolutePath? { get }
4545

46+
/// The manifest and library locations used by this toolchain.
47+
var swiftPMLibrariesLocation: ToolchainConfiguration.SwiftPMLibrariesLocation { get }
48+
4649
/// Path of the `clang` compiler.
4750
func getClangCompiler() throws -> AbsolutePath
4851

Sources/SourceKitLSPAPI/BuildDescription.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import class Build.SwiftModuleBuildDescription
2424
import struct PackageGraph.ResolvedModule
2525
import struct PackageGraph.ModulesGraph
2626
import enum PackageGraph.BuildTriple
27+
internal import class PackageModel.UserToolchain
2728

2829
public typealias BuildTriple = PackageGraph.BuildTriple
2930

@@ -133,6 +134,7 @@ public struct BuildDescription {
133134
return PluginTargetBuildDescription(
134135
target: target,
135136
toolsVersion: package.manifest.toolsVersion,
137+
toolchain: buildPlan.toolsBuildParameters.toolchain,
136138
isPartOfRootPackage: modulesGraph.rootPackages.map(\.id).contains(package.id)
137139
)
138140
}

Sources/SourceKitLSPAPI/PluginTargetBuildDescription.swift

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,18 +18,20 @@ import struct PackageGraph.ResolvedModule
1818

1919
private import class PackageLoading.ManifestLoader
2020
internal import struct PackageModel.ToolsVersion
21-
internal import class PackageModel.UserToolchain
21+
internal import protocol PackageModel.Toolchain
2222
import enum PackageGraph.BuildTriple
2323

2424
struct PluginTargetBuildDescription: BuildTarget {
2525
private let target: ResolvedModule
2626
private let toolsVersion: ToolsVersion
27+
private let toolchain: any Toolchain
2728
let isPartOfRootPackage: Bool
2829

29-
init(target: ResolvedModule, toolsVersion: ToolsVersion, isPartOfRootPackage: Bool) {
30+
init(target: ResolvedModule, toolsVersion: ToolsVersion, toolchain: any Toolchain, isPartOfRootPackage: Bool) {
3031
assert(target.type == .plugin)
3132
self.target = target
3233
self.toolsVersion = toolsVersion
34+
self.toolchain = toolchain
3335
self.isPartOfRootPackage = isPartOfRootPackage
3436
}
3537

@@ -47,16 +49,7 @@ struct PluginTargetBuildDescription: BuildTarget {
4749

4850
func compileArguments(for fileURL: URL) throws -> [String] {
4951
// FIXME: This is very odd and we should clean this up by merging `ManifestLoader` and `DefaultPluginScriptRunner` again.
50-
let environment = Environment.current
51-
let loader = ManifestLoader(
52-
toolchain: try UserToolchain(
53-
swiftSDK: .hostSwiftSDK(
54-
environment: environment
55-
),
56-
environment: environment
57-
)
58-
)
59-
var args = loader.interpreterFlags(for: self.toolsVersion)
52+
var args = ManifestLoader.interpreterFlags(for: self.toolsVersion, toolchain: toolchain)
6053
// Note: we ignore the `fileURL` here as the expectation is that we get a commandline for the entire target in case of Swift. Plugins are always assumed to only consist of Swift files.
6154
args += sources.map { $0.path }
6255
return args

Sources/_InternalTestSupport/MockBuildTestHelper.swift

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,9 @@ public struct MockToolchain: PackageModel.Toolchain {
4040
public let extraFlags = PackageModel.BuildFlags()
4141
public let installedSwiftPMConfiguration = InstalledSwiftPMConfiguration.default
4242
public let providedLibraries = [ProvidedLibrary]()
43+
public let swiftPMLibrariesLocation = ToolchainConfiguration.SwiftPMLibrariesLocation(
44+
manifestLibraryPath: AbsolutePath("/fake/manifestLib/path"), pluginLibraryPath: AbsolutePath("/fake/pluginLibrary/path")
45+
)
4346

4447
public func getClangCompiler() throws -> AbsolutePath {
4548
"/fake/path/to/clang"

Tests/SourceKitLSPAPITests/SourceKitLSPAPITests.swift

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,8 @@ final class SourceKitLSPAPITests: XCTestCase {
2525
func testBasicSwiftPackage() throws {
2626
let fs = InMemoryFileSystem(emptyFiles:
2727
"/Pkg/Sources/exe/main.swift",
28-
"/Pkg/Sources/lib/lib.swift"
28+
"/Pkg/Sources/lib/lib.swift",
29+
"/Pkg/Plugins/plugin/plugin.swift"
2930
)
3031

3132
let observability = ObservabilitySystem.makeForTesting()
@@ -38,6 +39,7 @@ final class SourceKitLSPAPITests: XCTestCase {
3839
targets: [
3940
TargetDescription(name: "exe", dependencies: ["lib"]),
4041
TargetDescription(name: "lib", dependencies: []),
42+
TargetDescription(name: "plugin", type: .plugin, pluginCapability: .buildTool)
4143
]),
4244
],
4345
observabilityScope: observability.topScope
@@ -81,6 +83,15 @@ final class SourceKitLSPAPITests: XCTestCase {
8183
],
8284
isPartOfRootPackage: true
8385
)
86+
try description.checkArguments(
87+
for: "plugin",
88+
graph: graph,
89+
partialArguments: [
90+
"-I", "/fake/manifestLib/path"
91+
],
92+
isPartOfRootPackage: true,
93+
destination: .tools
94+
)
8495
}
8596
}
8697

@@ -89,9 +100,10 @@ extension SourceKitLSPAPI.BuildDescription {
89100
for targetName: String,
90101
graph: ModulesGraph,
91102
partialArguments: [String],
92-
isPartOfRootPackage: Bool
103+
isPartOfRootPackage: Bool,
104+
destination: BuildTriple = .destination
93105
) throws -> Bool {
94-
let target = try XCTUnwrap(graph.module(for: targetName, destination: .destination))
106+
let target = try XCTUnwrap(graph.module(for: targetName, destination: destination))
95107
let buildTarget = try XCTUnwrap(self.getBuildTarget(for: target, in: graph))
96108

97109
guard let file = buildTarget.sources.first else {

0 commit comments

Comments
 (0)