Merge pull request #1632 from ahoppen/lazy-workspace-loading

Don’t block the generation of a build system by build graph generation

Author: ahoppen

Sources/BuildSystemIntegration/BuildServerBuildSystem.swift

@@ -281,7 +281,9 @@ extension BuildServerBuildSystem: BuildSystem {
     return [ConfiguredTarget(targetID: "dummy", runDestinationID: "dummy")]
   }
 
-  package func generateBuildGraph(allowFileSystemWrites: Bool) {}
+  package func generateBuildGraph() {}
+
+  package func waitForUpToDateBuildGraph() async {}
 
   package func topologicalSort(of targets: [ConfiguredTarget]) async -> [ConfiguredTarget]? {
     return nil

Sources/BuildSystemIntegration/BuildSystem.swift

@@ -138,13 +138,10 @@ package protocol BuildSystem: AnyObject, Sendable {
   func configuredTargets(for document: DocumentURI) async -> [ConfiguredTarget]
 
   /// Re-generate the build graph.
-  ///
-  /// If `allowFileSystemWrites` is `true`, this should include all the tasks that are necessary for building the entire
-  /// build graph, like resolving package versions.
-  ///
-  /// If `allowFileSystemWrites` is `false`, no files must be written to disk. This mode is used to determine whether
-  /// the build system can handle a source file, and decide whether a workspace should be opened with this build system
-  func generateBuildGraph(allowFileSystemWrites: Bool) async throws
+  func generateBuildGraph() async throws
+
+  /// Wait until the build graph has been loaded.
+  func waitForUpToDateBuildGraph() async
 
   /// Sort the targets so that low-level targets occur before high-level targets.
   ///

Sources/BuildSystemIntegration/BuildSystemManager.swift

@@ -265,8 +265,12 @@ extension BuildSystemManager {
     return settings
   }
 
-  package func generateBuildGraph(allowFileSystemWrites: Bool) async throws {
-    try await self.buildSystem?.generateBuildGraph(allowFileSystemWrites: allowFileSystemWrites)
+  package func generateBuildGraph() async throws {
+    try await self.buildSystem?.generateBuildGraph()
+  }
+
+  package func waitForUpToDateBuildGraph() async {
+    await self.buildSystem?.waitForUpToDateBuildGraph()
   }
 
   package func topologicalSort(of targets: [ConfiguredTarget]) async throws -> [ConfiguredTarget]? {

Sources/BuildSystemIntegration/CompilationDatabaseBuildSystem.swift

@@ -133,7 +133,9 @@ extension CompilationDatabaseBuildSystem: BuildSystem {
     throw PrepareNotSupportedError()
   }
 
-  package func generateBuildGraph(allowFileSystemWrites: Bool) {}
+  package func generateBuildGraph() {}
+
+  package func waitForUpToDateBuildGraph() async {}
 
   package func topologicalSort(of targets: [ConfiguredTarget]) -> [ConfiguredTarget]? {
     return nil

Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift

@@ -119,6 +119,10 @@ package actor SwiftPMBuildSystem {
 
   private let testHooks: SwiftPMTestHooks
 
+  /// The queue on which we reload the package to ensure we don't reload it multiple times concurrently, which can cause
+  /// issues in SwiftPM.
+  private let packageLoadingQueue = AsyncQueue<Serial>()
+
   /// Delegate to handle any build system events.
   package weak var delegate: BuildSystemIntegration.BuildSystemDelegate? = nil
 
@@ -361,7 +365,14 @@ package actor SwiftPMBuildSystem {
 extension SwiftPMBuildSystem {
   /// (Re-)load the package settings by parsing the manifest and resolving all the targets and
   /// dependencies.
-  package func reloadPackage(forceResolvedVersions: Bool) async throws {
+  package func reloadPackage() async throws {
+    try await packageLoadingQueue.asyncThrowing {
+      try await self.reloadPackageImpl()
+    }.valuePropagatingCancellation
+  }
+
+  /// - Important: Must only be called on `packageLoadingQueue`.
+  private func reloadPackageImpl() async throws {
     await reloadPackageStatusCallback(.start)
     await testHooks.reloadPackageDidStart?()
     defer {
@@ -373,7 +384,7 @@ extension SwiftPMBuildSystem {
 
     let modulesGraph = try await self.workspace.loadPackageGraph(
       rootInput: PackageGraphRootInput(packages: [AbsolutePath(projectRoot)]),
-      forceResolvedVersions: forceResolvedVersions,
+      forceResolvedVersions: !isForIndexBuild,
       observabilityScope: observabilitySystem.topScope
     )
 
@@ -538,8 +549,12 @@ extension SwiftPMBuildSystem: BuildSystemIntegration.BuildSystem {
     return []
   }
 
-  package func generateBuildGraph(allowFileSystemWrites: Bool) async throws {
-    try await self.reloadPackage(forceResolvedVersions: !isForIndexBuild || !allowFileSystemWrites)
+  package func generateBuildGraph() async throws {
+    try await self.reloadPackage()
+  }
+
+  package func waitForUpToDateBuildGraph() async {
+    await self.packageLoadingQueue.async {}.valuePropagatingCancellation
   }
 
   package func topologicalSort(of targets: [ConfiguredTarget]) -> [ConfiguredTarget]? {
@@ -728,7 +743,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(forceResolvedVersions: !isForIndexBuild)
+        try await self.reloadPackage()
       }
     }
 

Sources/SemanticIndex/SemanticIndexManager.swift

@@ -266,9 +266,7 @@ package final actor SemanticIndexManager {
           signposter.endInterval("Preparing", state)
         }
         await testHooks.buildGraphGenerationDidStart?()
-        await orLog("Generating build graph") {
-          try await self.buildSystemManager.generateBuildGraph(allowFileSystemWrites: true)
-        }
+        await self.buildSystemManager.waitForUpToDateBuildGraph()
         // Ensure that we have an up-to-date indexstore-db. Waiting for the indexstore-db to be updated is cheaper than
         // potentially not knowing about unit files, which causes the corresponding source files to be re-indexed.
         index.pollForUnitChangesAndWait()