Skip to content

Commit c8454f5

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 dd9c0af commit c8454f5

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
@@ -129,6 +129,10 @@ package actor SwiftPMBuildSystem {
129129

130130
private let testHooks: SwiftPMTestHooks
131131

132+
/// The queue on which we reload the package to ensure we don't reload it multiple times concurrently, which can cause
133+
/// issues in SwiftPM.
134+
private let packageLoadingQueue = AsyncQueue<Serial>()
135+
132136
/// Delegate to handle any build system events.
133137
package weak var delegate: BuildSystemIntegration.BuildSystemDelegate? = nil
134138

@@ -370,7 +374,14 @@ package actor SwiftPMBuildSystem {
370374
extension SwiftPMBuildSystem {
371375
/// (Re-)load the package settings by parsing the manifest and resolving all the targets and
372376
/// dependencies.
373-
package func reloadPackage(forceResolvedVersions: Bool) async throws {
377+
package func reloadPackage() async throws {
378+
try await packageLoadingQueue.asyncThrowing {
379+
try await self.reloadPackageImpl()
380+
}.valuePropagatingCancellation
381+
}
382+
383+
/// - Important: Must only be called on `packageLoadingQueue`.
384+
private func reloadPackageImpl() async throws {
374385
await reloadPackageStatusCallback(.start)
375386
await testHooks.reloadPackageDidStart?()
376387
defer {
@@ -382,7 +393,7 @@ extension SwiftPMBuildSystem {
382393

383394
let modulesGraph = try await self.workspace.loadPackageGraph(
384395
rootInput: PackageGraphRootInput(packages: [AbsolutePath(projectRoot)]),
385-
forceResolvedVersions: forceResolvedVersions,
396+
forceResolvedVersions: !isForIndexBuild,
386397
observabilityScope: observabilitySystem.topScope
387398
)
388399

@@ -547,8 +558,12 @@ extension SwiftPMBuildSystem: BuildSystemIntegration.BuildSystem {
547558
return []
548559
}
549560

550-
package func generateBuildGraph(allowFileSystemWrites: Bool) async throws {
551-
try await self.reloadPackage(forceResolvedVersions: !isForIndexBuild || !allowFileSystemWrites)
561+
package func generateBuildGraph() async throws {
562+
try await self.reloadPackage()
563+
}
564+
565+
package func waitForUpToDateBuildGraph() async {
566+
await self.packageLoadingQueue.async {}.valuePropagatingCancellation
552567
}
553568

554569
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)