Skip to content

Commit 0151a0f

Browse files
committed
Don’t block the generation of a build system by build graph generation
We currently load the entire package before generating a `SwiftPMBuildSystem`. That means that the initialize request to SourceKit-LSP is blocked until the package has been loaded, preventing us from offering any sort of functionality, including syntactic functionality like formatting. Decouple build system creation and build graph generation (aka. package loading for SwiftPM). We can operate with fallback build settings until the build graph has been loaded and reopen the document once the proper build settings are available. rdar://126644596
1 parent 1f6bfce commit 0151a0f

14 files changed

+248
-144
lines changed

Sources/BuildSystemIntegration/BuildServerBuildSystem.swift

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,9 @@ extension BuildServerBuildSystem: BuildSystem {
281281
return [ConfiguredTarget(targetID: "dummy", runDestinationID: "dummy")]
282282
}
283283

284-
package func generateBuildGraph(allowFileSystemWrites: Bool) {}
284+
package func generateBuildGraph() {}
285+
286+
package func waitForUpToDateBuildGraph() async {}
285287

286288
package func topologicalSort(of targets: [ConfiguredTarget]) async -> [ConfiguredTarget]? {
287289
return nil

Sources/BuildSystemIntegration/BuildSystem.swift

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -138,13 +138,10 @@ package protocol BuildSystem: AnyObject, Sendable {
138138
func configuredTargets(for document: DocumentURI) async -> [ConfiguredTarget]
139139

140140
/// Re-generate the build graph.
141-
///
142-
/// If `allowFileSystemWrites` is `true`, this should include all the tasks that are necessary for building the entire
143-
/// build graph, like resolving package versions.
144-
///
145-
/// If `allowFileSystemWrites` is `false`, no files must be written to disk. This mode is used to determine whether
146-
/// the build system can handle a source file, and decide whether a workspace should be opened with this build system
147-
func generateBuildGraph(allowFileSystemWrites: Bool) async throws
141+
func generateBuildGraph() async throws
142+
143+
/// Wait until the build graph has been loaded.
144+
func waitForUpToDateBuildGraph() async
148145

149146
/// Sort the targets so that low-level targets occur before high-level targets.
150147
///

Sources/BuildSystemIntegration/BuildSystemManager.swift

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -265,8 +265,12 @@ extension BuildSystemManager {
265265
return settings
266266
}
267267

268-
package func generateBuildGraph(allowFileSystemWrites: Bool) async throws {
269-
try await self.buildSystem?.generateBuildGraph(allowFileSystemWrites: allowFileSystemWrites)
268+
package func generateBuildGraph() async throws {
269+
try await self.buildSystem?.generateBuildGraph()
270+
}
271+
272+
package func waitForUpToDateBuildGraph() async {
273+
await self.buildSystem?.waitForUpToDateBuildGraph()
270274
}
271275

272276
package func topologicalSort(of targets: [ConfiguredTarget]) async throws -> [ConfiguredTarget]? {

Sources/BuildSystemIntegration/CompilationDatabaseBuildSystem.swift

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,9 @@ extension CompilationDatabaseBuildSystem: BuildSystem {
133133
throw PrepareNotSupportedError()
134134
}
135135

136-
package func generateBuildGraph(allowFileSystemWrites: Bool) {}
136+
package func generateBuildGraph() {}
137+
138+
package func waitForUpToDateBuildGraph() async {}
137139

138140
package func topologicalSort(of targets: [ConfiguredTarget]) -> [ConfiguredTarget]? {
139141
return nil

Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,10 @@ package actor SwiftPMBuildSystem {
119119

120120
private let testHooks: SwiftPMTestHooks
121121

122+
/// The queue on which we reload the package to ensure we don't reload it multiple times concurrently, which can cause
123+
/// issues in SwiftPM.
124+
private let packageLoadingQueue = AsyncQueue<Serial>()
125+
122126
/// Delegate to handle any build system events.
123127
package weak var delegate: BuildSystemIntegration.BuildSystemDelegate? = nil
124128

@@ -363,7 +367,14 @@ package actor SwiftPMBuildSystem {
363367
extension SwiftPMBuildSystem {
364368
/// (Re-)load the package settings by parsing the manifest and resolving all the targets and
365369
/// dependencies.
366-
package func reloadPackage(forceResolvedVersions: Bool) async throws {
370+
package func reloadPackage() async throws {
371+
try await packageLoadingQueue.asyncThrowing {
372+
try await self.reloadPackageImpl()
373+
}.valuePropagatingCancellation
374+
}
375+
376+
/// - Important: Must only be called on `packageLoadingQueue`.
377+
private func reloadPackageImpl() async throws {
367378
await reloadPackageStatusCallback(.start)
368379
await testHooks.reloadPackageDidStart?()
369380
defer {
@@ -375,7 +386,7 @@ extension SwiftPMBuildSystem {
375386

376387
let modulesGraph = try await self.workspace.loadPackageGraph(
377388
rootInput: PackageGraphRootInput(packages: [AbsolutePath(projectRoot)]),
378-
forceResolvedVersions: forceResolvedVersions,
389+
forceResolvedVersions: !isForIndexBuild,
379390
observabilityScope: observabilitySystem.topScope
380391
)
381392

@@ -540,8 +551,12 @@ extension SwiftPMBuildSystem: BuildSystemIntegration.BuildSystem {
540551
return []
541552
}
542553

543-
package func generateBuildGraph(allowFileSystemWrites: Bool) async throws {
544-
try await self.reloadPackage(forceResolvedVersions: !isForIndexBuild || !allowFileSystemWrites)
554+
package func generateBuildGraph() async throws {
555+
try await self.reloadPackage()
556+
}
557+
558+
package func waitForUpToDateBuildGraph() async {
559+
await self.packageLoadingQueue.async {}.valuePropagatingCancellation
545560
}
546561

547562
package func topologicalSort(of targets: [ConfiguredTarget]) -> [ConfiguredTarget]? {

Sources/SemanticIndex/SemanticIndexManager.swift

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -266,9 +266,7 @@ package final actor SemanticIndexManager {
266266
signposter.endInterval("Preparing", state)
267267
}
268268
await testHooks.buildGraphGenerationDidStart?()
269-
await orLog("Generating build graph") {
270-
try await self.buildSystemManager.generateBuildGraph(allowFileSystemWrites: true)
271-
}
269+
await self.buildSystemManager.waitForUpToDateBuildGraph()
272270
// Ensure that we have an up-to-date indexstore-db. Waiting for the indexstore-db to be updated is cheaper than
273271
// potentially not knowing about unit files, which causes the corresponding source files to be re-indexed.
274272
index.pollForUnitChangesAndWait()

0 commit comments

Comments
 (0)