Skip to content

Fix a race condition that could cause the build graph to not be generated when doing initial background indexing #1652

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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ extension BuildServerBuildSystem: BuildSystem {
return [ConfiguredTarget(targetID: "dummy", runDestinationID: "dummy")]
}

package func generateBuildGraph() {}
package func scheduleBuildGraphGeneration() {}

package func waitForUpToDateBuildGraph() async {}

Expand Down
6 changes: 4 additions & 2 deletions Sources/BuildSystemIntegration/BuildSystem.swift
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,10 @@ package protocol BuildSystem: AnyObject, Sendable {
/// Return the list of targets and run destinations that the given document can be built for.
func configuredTargets(for document: DocumentURI) async -> [ConfiguredTarget]

/// Re-generate the build graph.
func generateBuildGraph() async throws
/// Schedule a task that re-generates the build graph. The function may return before the build graph has finished
/// being generated. If clients need to wait for an up-to-date build graph, they should call
/// `waitForUpToDateBuildGraph` afterwards.
func scheduleBuildGraphGeneration() async throws

/// Wait until the build graph has been loaded.
func waitForUpToDateBuildGraph() async
Expand Down
4 changes: 2 additions & 2 deletions Sources/BuildSystemIntegration/BuildSystemManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -265,8 +265,8 @@ extension BuildSystemManager {
return settings
}

package func generateBuildGraph() async throws {
try await self.buildSystem?.generateBuildGraph()
package func scheduleBuildGraphGeneration() async throws {
try await self.buildSystem?.scheduleBuildGraphGeneration()
}

package func waitForUpToDateBuildGraph() async {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ extension CompilationDatabaseBuildSystem: BuildSystem {
throw PrepareNotSupportedError()
}

package func generateBuildGraph() {}
package func scheduleBuildGraphGeneration() {}

package func waitForUpToDateBuildGraph() async {}

Expand Down
13 changes: 7 additions & 6 deletions Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift
Original file line number Diff line number Diff line change
Expand Up @@ -365,10 +365,11 @@ package actor SwiftPMBuildSystem {
extension SwiftPMBuildSystem {
/// (Re-)load the package settings by parsing the manifest and resolving all the targets and
/// dependencies.
package func reloadPackage() async throws {
try await packageLoadingQueue.asyncThrowing {
@discardableResult
package func schedulePackageReload() -> Task<Void, Swift.Error> {
return packageLoadingQueue.asyncThrowing {
try await self.reloadPackageImpl()
}.valuePropagatingCancellation
}
}

/// - Important: Must only be called on `packageLoadingQueue`.
Expand Down Expand Up @@ -549,8 +550,8 @@ extension SwiftPMBuildSystem: BuildSystemIntegration.BuildSystem {
return []
}

package func generateBuildGraph() async throws {
try await self.reloadPackage()
package func scheduleBuildGraphGeneration() async throws {
self.schedulePackageReload()
}

package func waitForUpToDateBuildGraph() async {
Expand Down Expand Up @@ -743,7 +744,7 @@ extension SwiftPMBuildSystem: BuildSystemIntegration.BuildSystem {
if events.contains(where: { self.fileEventShouldTriggerPackageReload(event: $0) }) {
logger.log("Reloading package because of file change")
await orLog("Reloading package") {
try await self.reloadPackage()
try await self.schedulePackageReload().value
}
}

Expand Down
12 changes: 5 additions & 7 deletions Sources/SourceKitLSP/SourceKitLSPServer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -967,13 +967,11 @@ extension SourceKitLSPServer {
guard await condition(buildSystem) else {
return nil
}
Task {
await orLog("Initial build graph generation") {
// Schedule an initial generation of the build graph. Once the build graph is loaded, the build system will send
// call `fileHandlingCapabilityChanged`, which allows us to move documents to a workspace with this build
// system.
try await buildSystem?.generateBuildGraph()
}
await orLog("Initial build graph generation") {
// Schedule an initial generation of the build graph. Once the build graph is loaded, the build system will send
// call `fileHandlingCapabilityChanged`, which allows us to move documents to a workspace with this build
// system.
try await buildSystem?.scheduleBuildGraphGeneration()
}

let projectRoot = await buildSystem?.projectRoot.pathString
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,7 @@ class ManualBuildSystem: BuildSystem {
throw PrepareNotSupportedError()
}

package func generateBuildGraph() {}
package func scheduleBuildGraphGeneration() {}

package func waitForUpToDateBuildGraph() async {}

Expand Down
28 changes: 14 additions & 14 deletions Tests/BuildSystemIntegrationTests/SwiftPMBuildSystemTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ final class SwiftPMBuildSystemTests: XCTestCase {
options: SourceKitLSPOptions(),
testHooks: SwiftPMTestHooks()
)
await assertThrowsError(try await buildSystem.generateBuildGraph())
await assertThrowsError(try await buildSystem.schedulePackageReload().value)
}
}

Expand Down Expand Up @@ -144,7 +144,7 @@ final class SwiftPMBuildSystemTests: XCTestCase {
options: SourceKitLSPOptions(),
testHooks: SwiftPMTestHooks()
)
try await swiftpmBuildSystem.generateBuildGraph()
try await swiftpmBuildSystem.schedulePackageReload().value

let aswift = packageRoot.appending(components: "Sources", "lib", "a.swift")
let hostTriple = await swiftpmBuildSystem.destinationBuildParameters.triple
Expand Down Expand Up @@ -209,7 +209,7 @@ final class SwiftPMBuildSystemTests: XCTestCase {
options: SourceKitLSPOptions(),
testHooks: SwiftPMTestHooks()
)
try await swiftpmBuildSystem.generateBuildGraph()
try await swiftpmBuildSystem.schedulePackageReload().value

let aPlusSomething = packageRoot.appending(components: "Sources", "lib", "a+something.swift")
let hostTriple = await swiftpmBuildSystem.destinationBuildParameters.triple
Expand Down Expand Up @@ -272,7 +272,7 @@ final class SwiftPMBuildSystemTests: XCTestCase {
options: SourceKitLSPOptions(swiftPM: options),
testHooks: SwiftPMTestHooks()
)
try await swiftpmBuildSystem.generateBuildGraph()
try await swiftpmBuildSystem.schedulePackageReload().value

let aswift = packageRoot.appending(components: "Sources", "lib", "a.swift")
let hostTriple = await swiftpmBuildSystem.destinationBuildParameters.triple
Expand Down Expand Up @@ -354,7 +354,7 @@ final class SwiftPMBuildSystemTests: XCTestCase {
options: SourceKitLSPOptions(),
testHooks: SwiftPMTestHooks()
)
try await swiftpmBuildSystem.generateBuildGraph()
try await swiftpmBuildSystem.schedulePackageReload().value

let source = try resolveSymlinks(packageRoot.appending(component: "Package.swift"))
let arguments = try await unwrap(swiftpmBuildSystem.buildSettings(for: source.asURI, language: .swift))
Expand Down Expand Up @@ -391,7 +391,7 @@ final class SwiftPMBuildSystemTests: XCTestCase {
options: SourceKitLSPOptions(),
testHooks: SwiftPMTestHooks()
)
try await swiftpmBuildSystem.generateBuildGraph()
try await swiftpmBuildSystem.schedulePackageReload().value

let aswift = packageRoot.appending(components: "Sources", "lib", "a.swift")
let bswift = packageRoot.appending(components: "Sources", "lib", "b.swift")
Expand Down Expand Up @@ -440,7 +440,7 @@ final class SwiftPMBuildSystemTests: XCTestCase {
options: SourceKitLSPOptions(),
testHooks: SwiftPMTestHooks()
)
try await swiftpmBuildSystem.generateBuildGraph()
try await swiftpmBuildSystem.schedulePackageReload().value

let aswift = packageRoot.appending(components: "Sources", "libA", "a.swift")
let bswift = packageRoot.appending(components: "Sources", "libB", "b.swift")
Expand Down Expand Up @@ -495,7 +495,7 @@ final class SwiftPMBuildSystemTests: XCTestCase {
options: SourceKitLSPOptions(),
testHooks: SwiftPMTestHooks()
)
try await swiftpmBuildSystem.generateBuildGraph()
try await swiftpmBuildSystem.schedulePackageReload().value

let aswift = packageRoot.appending(components: "Sources", "libA", "a.swift")
let bswift = packageRoot.appending(components: "Sources", "libB", "b.swift")
Expand Down Expand Up @@ -539,7 +539,7 @@ final class SwiftPMBuildSystemTests: XCTestCase {
options: SourceKitLSPOptions(),
testHooks: SwiftPMTestHooks()
)
try await swiftpmBuildSystem.generateBuildGraph()
try await swiftpmBuildSystem.schedulePackageReload().value

let acxx = packageRoot.appending(components: "Sources", "lib", "a.cpp")
let bcxx = packageRoot.appending(components: "Sources", "lib", "b.cpp")
Expand Down Expand Up @@ -620,7 +620,7 @@ final class SwiftPMBuildSystemTests: XCTestCase {
options: SourceKitLSPOptions(),
testHooks: SwiftPMTestHooks()
)
try await swiftpmBuildSystem.generateBuildGraph()
try await swiftpmBuildSystem.schedulePackageReload().value

let aswift = packageRoot.appending(components: "Sources", "lib", "a.swift")
let arguments = try await unwrap(swiftpmBuildSystem.buildSettings(for: aswift.asURI, language: .swift))
Expand Down Expand Up @@ -672,7 +672,7 @@ final class SwiftPMBuildSystemTests: XCTestCase {
options: SourceKitLSPOptions(),
testHooks: SwiftPMTestHooks()
)
try await swiftpmBuildSystem.generateBuildGraph()
try await swiftpmBuildSystem.schedulePackageReload().value

let aswift1 = packageRoot.appending(components: "Sources", "lib", "a.swift")
let aswift2 =
Expand Down Expand Up @@ -740,7 +740,7 @@ final class SwiftPMBuildSystemTests: XCTestCase {
options: SourceKitLSPOptions(),
testHooks: SwiftPMTestHooks()
)
try await swiftpmBuildSystem.generateBuildGraph()
try await swiftpmBuildSystem.schedulePackageReload().value

for file in [acpp, ah] {
let args = try unwrap(
Expand Down Expand Up @@ -780,7 +780,7 @@ final class SwiftPMBuildSystemTests: XCTestCase {
options: SourceKitLSPOptions(),
testHooks: SwiftPMTestHooks()
)
try await swiftpmBuildSystem.generateBuildGraph()
try await swiftpmBuildSystem.schedulePackageReload().value

let aswift = packageRoot.appending(components: "Sources", "lib", "a.swift")
let arguments = try await unwrap(swiftpmBuildSystem.buildSettings(for: aswift.asURI, language: .swift))
Expand Down Expand Up @@ -856,7 +856,7 @@ final class SwiftPMBuildSystemTests: XCTestCase {
options: SourceKitLSPOptions(),
testHooks: SwiftPMTestHooks()
)
try await swiftpmBuildSystem.generateBuildGraph()
try await swiftpmBuildSystem.schedulePackageReload().value

let aswift = packageRoot.appending(components: "Plugins", "MyPlugin", "a.swift")
let hostTriple = await swiftpmBuildSystem.destinationBuildParameters.triple
Expand Down
2 changes: 1 addition & 1 deletion Tests/SourceKitLSPTests/BuildSystemTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ actor TestBuildSystem: BuildSystem {
throw PrepareNotSupportedError()
}

func generateBuildGraph() {}
func scheduleBuildGraphGeneration() {}

package func waitForUpToDateBuildGraph() async {}

Expand Down