Skip to content

Commit 944553b

Browse files
committed
Let the build system determine which toolchain to use for a document
This allows us to fix a toolchain when using a `SwiftPMBuildSystem`, which is critical to ensure that a target gets prepared using the same toolchain that is used to index it and that is used for sourcekitd.
1 parent 42c9b79 commit 944553b

File tree

9 files changed

+68
-39
lines changed

9 files changed

+68
-39
lines changed

Sources/SKCore/BuildServerBuildSystem.swift

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,10 @@ extension BuildServerBuildSystem: BuildSystem {
278278
return nil
279279
}
280280

281+
public func toolchain(for uri: DocumentURI, _ language: Language) async -> SKCore.Toolchain? {
282+
return nil
283+
}
284+
281285
public func configuredTargets(for document: DocumentURI) async -> [ConfiguredTarget] {
282286
return [ConfiguredTarget(targetID: "dummy", runDestinationID: "dummy")]
283287
}

Sources/SKCore/BuildSystem.swift

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,11 @@ public protocol BuildSystem: AnyObject, Sendable {
179179
/// If `nil` is returned, the language based on the file's extension.
180180
func defaultLanguage(for document: DocumentURI) async -> Language?
181181

182+
/// The toolchain that should be used to open the given document.
183+
///
184+
/// If `nil` is returned, then the default toolchain for the given language is used.
185+
func toolchain(for uri: DocumentURI, _ language: Language) async -> Toolchain?
186+
182187
/// Register the given file for build-system level change notifications, such
183188
/// as command line flag changes, dependency changes, etc.
184189
///

Sources/SKCore/BuildSystemManager.swift

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,9 +104,18 @@ extension BuildSystemManager {
104104

105105
/// Returns the toolchain that should be used to process the given document.
106106
public func toolchain(for uri: DocumentURI, _ language: Language) async -> Toolchain? {
107-
// To support multiple toolchains within a single workspace, we need to ask the build system which toolchain to use
108-
// for this document.
109-
return await toolchainRegistry.defaultToolchain(for: language)
107+
if let toolchain = await buildSystem?.toolchain(for: uri, language) {
108+
return toolchain
109+
}
110+
111+
switch language {
112+
case .swift:
113+
return await toolchainRegistry.preferredToolchain(containing: [\.sourcekitd, \.swift, \.swiftc])
114+
case .c, .cpp, .objective_c, .objective_cpp:
115+
return await toolchainRegistry.preferredToolchain(containing: [\.clang, \.clangd])
116+
default:
117+
return nil
118+
}
110119
}
111120

112121
/// - Note: Needed so we can set the delegate from a different isolation context.

Sources/SKCore/CompilationDatabaseBuildSystem.swift

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,10 @@ extension CompilationDatabaseBuildSystem: BuildSystem {
119119
return nil
120120
}
121121

122+
public func toolchain(for uri: DocumentURI, _ language: Language) async -> SKCore.Toolchain? {
123+
return nil
124+
}
125+
122126
public func configuredTargets(for document: DocumentURI) async -> [ConfiguredTarget] {
123127
return [ConfiguredTarget(targetID: "dummy", runDestinationID: "dummy")]
124128
}

Sources/SKCore/ToolchainRegistry.swift

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -246,26 +246,14 @@ public final actor ToolchainRegistry {
246246
return darwinToolchainOverride ?? ToolchainRegistry.darwinDefaultToolchainIdentifier
247247
}
248248

249-
/// The toolchain to use for a document in the given language if the build system doesn't override it.
250-
func defaultToolchain(for language: Language) -> Toolchain? {
251-
let supportsLang = { (toolchain: Toolchain) -> Bool in
252-
// FIXME: the fact that we're looking at clangd/sourcekitd instead of the compiler indicates this method needs a parameter stating what kind of tool we're looking for.
253-
switch language {
254-
case .swift:
255-
return toolchain.sourcekitd != nil
256-
case .c, .cpp, .objective_c, .objective_cpp:
257-
return toolchain.clangd != nil
258-
default:
259-
return false
260-
}
261-
}
262-
263-
if let toolchain = self.default, supportsLang(toolchain) {
249+
/// Returns the preferred toolchain that contains all the tools at the given key paths.
250+
public func preferredToolchain(containing requiredTools: [KeyPath<Toolchain, AbsolutePath?>]) -> Toolchain? {
251+
if let toolchain = self.default, requiredTools.allSatisfy({ toolchain[keyPath: $0] != nil }) {
264252
return toolchain
265253
}
266254

267255
for toolchain in toolchains {
268-
if supportsLang(toolchain) {
256+
if requiredTools.allSatisfy({ toolchain[keyPath: $0] != nil }) {
269257
return toolchain
270258
}
271259
}

Sources/SKSwiftPMWorkspace/SwiftPMBuildSystem.swift

Lines changed: 30 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -59,10 +59,12 @@ public typealias BuildServerTarget = BuildServerProtocol.BuildTarget
5959

6060
/// Same as `toolchainRegistry.default`.
6161
///
62-
/// Needed to work around a compiler crash that prevents us from accessing `toolchainRegistry.default` in
62+
/// Needed to work around a compiler crash that prevents us from accessing `toolchainRegistry.preferredToolchain` in
6363
/// `SwiftPMWorkspace.init`.
64-
private func getDefaultToolchain(_ toolchainRegistry: ToolchainRegistry) async -> SKCore.Toolchain? {
65-
return await toolchainRegistry.default
64+
private func preferredToolchain(_ toolchainRegistry: ToolchainRegistry) async -> SKCore.Toolchain? {
65+
return await toolchainRegistry.preferredToolchain(containing: [
66+
\.clang, \.clangd, \.sourcekitd, \.swift, \.swiftc,
67+
])
6668
}
6769

6870
fileprivate extension BuildTriple {
@@ -123,7 +125,7 @@ public actor SwiftPMBuildSystem {
123125
@_spi(Testing) public let toolsBuildParameters: BuildParameters
124126
@_spi(Testing) public let destinationBuildParameters: BuildParameters
125127
private let fileSystem: FileSystem
126-
private let toolchainRegistry: ToolchainRegistry
128+
private let toolchain: SKCore.Toolchain
127129

128130
private let swiftBuildSupportsPrepareForIndexingTask = SwiftExtensions.ThreadSafeBox<Task<Bool, Never>?>(
129131
initialValue: nil
@@ -184,7 +186,11 @@ public actor SwiftPMBuildSystem {
184186
) async throws {
185187
self.workspacePath = workspacePath
186188
self.fileSystem = fileSystem
187-
self.toolchainRegistry = toolchainRegistry
189+
guard let toolchain = await preferredToolchain(toolchainRegistry) else {
190+
throw Error.cannotDetermineHostToolchain
191+
}
192+
193+
self.toolchain = toolchain
188194
self.experimentalFeatures = experimentalFeatures
189195

190196
guard let packageRoot = findPackageDirectory(containing: workspacePath, fileSystem) else {
@@ -193,12 +199,12 @@ public actor SwiftPMBuildSystem {
193199

194200
self.projectRoot = try resolveSymlinks(packageRoot)
195201

196-
guard let destinationToolchainBinDir = await getDefaultToolchain(toolchainRegistry)?.swiftc?.parentDirectory else {
202+
guard let destinationToolchainBinDir = toolchain.swiftc?.parentDirectory else {
197203
throw Error.cannotDetermineHostToolchain
198204
}
199205

200206
let swiftSDK = try SwiftSDK.hostSwiftSDK(AbsolutePath(destinationToolchainBinDir))
201-
let toolchain = try UserToolchain(swiftSDK: swiftSDK)
207+
let swiftPMToolchain = try UserToolchain(swiftSDK: swiftSDK)
202208

203209
var location = try Workspace.Location(
204210
forRootPackage: AbsolutePath(packageRoot),
@@ -217,7 +223,7 @@ public actor SwiftPMBuildSystem {
217223
fileSystem: fileSystem,
218224
location: location,
219225
configuration: configuration,
220-
customHostToolchain: toolchain
226+
customHostToolchain: swiftPMToolchain
221227
)
222228

223229
let buildConfiguration: PackageModel.BuildConfiguration
@@ -230,17 +236,21 @@ public actor SwiftPMBuildSystem {
230236

231237
self.toolsBuildParameters = try BuildParameters(
232238
destination: .host,
233-
dataPath: location.scratchDirectory.appending(component: toolchain.targetTriple.platformBuildPathComponent),
239+
dataPath: location.scratchDirectory.appending(
240+
component: swiftPMToolchain.targetTriple.platformBuildPathComponent
241+
),
234242
configuration: buildConfiguration,
235-
toolchain: toolchain,
243+
toolchain: swiftPMToolchain,
236244
flags: buildSetup.flags
237245
)
238246

239247
self.destinationBuildParameters = try BuildParameters(
240248
destination: .target,
241-
dataPath: location.scratchDirectory.appending(component: toolchain.targetTriple.platformBuildPathComponent),
249+
dataPath: location.scratchDirectory.appending(
250+
component: swiftPMToolchain.targetTriple.platformBuildPathComponent
251+
),
242252
configuration: buildConfiguration,
243-
toolchain: toolchain,
253+
toolchain: swiftPMToolchain,
244254
flags: buildSetup.flags
245255
)
246256

@@ -411,7 +421,7 @@ extension SwiftPMBuildSystem: SKCore.BuildSystem {
411421
#endif
412422
// Fix up compiler arguments that point to a `/Modules` subdirectory if the Swift version in the toolchain is less
413423
// than 6.0 because it places the modules one level higher up.
414-
let toolchainVersion = await orLog("Getting Swift version") { try await toolchainRegistry.default?.swiftVersion }
424+
let toolchainVersion = await orLog("Getting Swift version") { try await toolchain.swiftVersion }
415425
guard let toolchainVersion, toolchainVersion < SwiftVersion(6, 0) else {
416426
return compileArguments
417427
}
@@ -471,6 +481,10 @@ extension SwiftPMBuildSystem: SKCore.BuildSystem {
471481
return nil
472482
}
473483

484+
public func toolchain(for uri: DocumentURI, _ language: Language) async -> SKCore.Toolchain? {
485+
return toolchain
486+
}
487+
474488
public func configuredTargets(for uri: DocumentURI) -> [ConfiguredTarget] {
475489
guard let url = uri.fileURL, let path = try? AbsolutePath(validating: url.path) else {
476490
// We can't determine targets for non-file URIs.
@@ -535,7 +549,7 @@ extension SwiftPMBuildSystem: SKCore.BuildSystem {
535549
// TODO (indexing): Support preparation of multiple targets at once.
536550
// https://github.com/apple/sourcekit-lsp/issues/1262
537551
for target in targets {
538-
try await prepare(singleTarget: target, logMessageToIndexLog: logMessageToIndexLog)
552+
await orLog("Preparing") { try await prepare(singleTarget: target, logMessageToIndexLog: logMessageToIndexLog) }
539553
}
540554
let filesInPreparedTargets = targets.flatMap { self.targets[$0]?.buildTarget.sources ?? [] }
541555
await fileDependenciesUpdatedDebouncer.scheduleCall(Set(filesInPreparedTargets.map(DocumentURI.init)))
@@ -552,16 +566,13 @@ extension SwiftPMBuildSystem: SKCore.BuildSystem {
552566

553567
// TODO (indexing): Add a proper 'prepare' job in SwiftPM instead of building the target.
554568
// https://github.com/apple/sourcekit-lsp/issues/1254
555-
guard let toolchain = await toolchainRegistry.default else {
556-
logger.error("Not preparing because not toolchain exists")
557-
return
558-
}
559569
guard let swift = toolchain.swift else {
560570
logger.error(
561-
"Not preparing because toolchain at \(toolchain.identifier) does not contain a Swift compiler"
571+
"Not preparing because toolchain at \(self.toolchain.identifier) does not contain a Swift compiler"
562572
)
563573
return
564574
}
575+
logger.debug("Preparing '\(target.targetID)' using \(self.toolchain.identifier)")
565576
var arguments = [
566577
swift.pathString, "build",
567578
"--package-path", workspacePath.pathString,

Sources/SourceKitLSP/SourceKitLSPServer.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -521,7 +521,7 @@ public actor SourceKitLSPServer {
521521
return nil
522522
}
523523

524-
logger.log("Using toolchain \(toolchain.displayName) (\(toolchain.identifier)) for \(uri.forLogging)")
524+
logger.log("Using toolchain \(toolchain.identifier) (\(toolchain.identifier)) for \(uri.forLogging)")
525525

526526
return workspace.documentService.withLock { documentService in
527527
if let concurrentlySetService = documentService[uri] {

Tests/SKCoreTests/BuildSystemManagerTests.swift

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -465,6 +465,10 @@ class ManualBuildSystem: BuildSystem {
465465
return nil
466466
}
467467

468+
public func toolchain(for uri: DocumentURI, _ language: Language) async -> SKCore.Toolchain? {
469+
return nil
470+
}
471+
468472
public func configuredTargets(for document: DocumentURI) async -> [ConfiguredTarget] {
469473
return [ConfiguredTarget(targetID: "dummy", runDestinationID: "dummy")]
470474
}

Tests/SourceKitLSPTests/BuildSystemTests.swift

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,10 @@ actor TestBuildSystem: BuildSystem {
5757
return nil
5858
}
5959

60+
public func toolchain(for uri: DocumentURI, _ language: Language) async -> SKCore.Toolchain? {
61+
return nil
62+
}
63+
6064
public func configuredTargets(for document: DocumentURI) async -> [ConfiguredTarget] {
6165
return [ConfiguredTarget(targetID: "dummy", runDestinationID: "dummy")]
6266
}

0 commit comments

Comments
 (0)