Implement topologicalSort inside BuildSystemManager using dependency information

Author: ahoppen

Sources/BuildSystemIntegration/BuildServerBuildSystem.swift

@@ -336,10 +336,6 @@ extension BuildServerBuildSystem: BuiltInBuildSystem {
 
   package func waitForUpToDateBuildGraph() async {}
 
-  package func topologicalSort(of targets: [BuildTargetIdentifier]) async -> [BuildTargetIdentifier]? {
-    return nil
-  }
-
   package func addSourceFilesDidChangeCallback(_ callback: @escaping () async -> Void) {
     // BuildServerBuildSystem does not support syntactic test discovery or background indexing.
     // (https://github.com/swiftlang/sourcekit-lsp/issues/1173).

Sources/BuildSystemIntegration/BuildSystemManager.swift

@@ -118,6 +118,8 @@ package actor BuildSystemManager: BuiltInBuildSystemAdapterDelegate {
 
   private var cachedTargetSources = RequestCache<BuildTargetSourcesRequest>()
 
+  private var cachedTargetDepths: (buildTargets: [BuildTarget], depths: [BuildTargetIdentifier: Int])? = nil
+
   /// The root of the project that this build system manages. For example, for SwiftPM packages, this is the folder
   /// containing Package.swift. For compilation databases it is the root folder based on which the compilation database
   /// was found.
@@ -485,8 +487,50 @@ package actor BuildSystemManager: BuiltInBuildSystemAdapterDelegate {
     await self.buildSystem?.underlyingBuildSystem.waitForUpToDateBuildGraph()
   }
 
+  /// The root targets of the project have depth of 0 and all target dependencies have a greater depth than the target
+  // itself.
+  private func targetDepths(for buildTargets: [BuildTarget]) -> [BuildTargetIdentifier: Int] {
+    if let cachedTargetDepths, cachedTargetDepths.buildTargets == buildTargets {
+      return cachedTargetDepths.depths
+    }
+    var nonRoots: Set<BuildTargetIdentifier> = []
+    for buildTarget in buildTargets {
+      nonRoots.formUnion(buildTarget.dependencies)
+    }
+    let targetsById = Dictionary(elements: buildTargets, keyedBy: \.id)
+    var depths: [BuildTargetIdentifier: Int] = [:]
+    let rootTargets = buildTargets.filter { !nonRoots.contains($0.id) }
+    var worksList: [(target: BuildTargetIdentifier, depth: Int)] = rootTargets.map { ($0.id, 0) }
+    while let (target, depth) = worksList.popLast() {
+      depths[target] = max(depths[target, default: 0], depth)
+      for dependency in targetsById[target]?.dependencies ?? [] {
+        // Check if we have already recorded this target with a greater depth, in which case visiting it again will
+        // not increase its depth or any of its children.
+        if depths[target, default: 0] < depth + 1 {
+          worksList.append((dependency, depth + 1))
+        }
+      }
+    }
+    cachedTargetDepths = (buildTargets, depths)
+    return depths
+  }
+
+  /// Sort the targets so that low-level targets occur before high-level targets.
+  ///
+  /// This sorting is best effort but allows the indexer to prepare and index low-level targets first, which allows
+  /// index data to be available earlier.
+  ///
+  /// `nil` if the build system doesn't support topological sorting of targets.
   package func topologicalSort(of targets: [BuildTargetIdentifier]) async throws -> [BuildTargetIdentifier]? {
-    return await buildSystem?.underlyingBuildSystem.topologicalSort(of: targets)
+    guard let workspaceTargets = await orLog("Getting build targets for topological sort", { try await buildTargets() })
+    else {
+      return nil
+    }
+
+    let depths = targetDepths(for: workspaceTargets)
+    return targets.sorted { (lhs: BuildTargetIdentifier, rhs: BuildTargetIdentifier) -> Bool in
+      return depths[lhs, default: 0] > depths[rhs, default: 0]
+    }
   }
 
   /// Returns the list of targets that might depend on the given target and that need to be re-prepared when a file in

Sources/BuildSystemIntegration/BuiltInBuildSystem.swift

@@ -105,14 +105,6 @@ package protocol BuiltInBuildSystem: AnyObject, Sendable {
   /// Wait until the build graph has been loaded.
   func waitForUpToDateBuildGraph() async
 
-  /// Sort the targets so that low-level targets occur before high-level targets.
-  ///
-  /// This sorting is best effort but allows the indexer to prepare and index low-level targets first, which allows
-  /// index data to be available earlier.
-  ///
-  /// `nil` if the build system doesn't support topological sorting of targets.
-  func topologicalSort(of targets: [BuildTargetIdentifier]) async -> [BuildTargetIdentifier]?
-
   /// The toolchain that should be used to open the given document.
   ///
   /// If `nil` is returned, then the default toolchain for the given language is used.

Sources/BuildSystemIntegration/CompilationDatabaseBuildSystem.swift

@@ -159,10 +159,6 @@ extension CompilationDatabaseBuildSystem: BuiltInBuildSystem {
 
   package func waitForUpToDateBuildGraph() async {}
 
-  package func topologicalSort(of targets: [BuildTargetIdentifier]) -> [BuildTargetIdentifier]? {
-    return nil
-  }
-
   private func database(for uri: DocumentURI) -> CompilationDatabase? {
     if let url = uri.fileURL, let path = try? AbsolutePath(validating: url.path) {
       return database(for: path)

Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift

@@ -210,10 +210,8 @@ package actor SwiftPMBuildSystem {
   /// Maps source and header files to the target that include them.
   private var fileToTargets: [DocumentURI: Set<BuildTargetIdentifier>] = [:]
 
-  /// Maps target ids to their SwiftPM build target as well as the depth at which they occur in the build
-  /// graph. Top level targets on which no other target depends have a depth of `1`. Targets with dependencies have a
-  /// greater depth.
-  private var targets: [BuildTargetIdentifier: (buildTarget: SwiftBuildTarget, depth: Int)] = [:]
+  /// Maps target ids to their SwiftPM build target.
+  private var swiftPMTargets: [BuildTargetIdentifier: SwiftBuildTarget] = [:]
 
   private var targetDependencies: [BuildTargetIdentifier: Set<BuildTargetIdentifier>] = [:]
 
@@ -442,7 +440,7 @@ extension SwiftPMBuildSystem {
     /// properties because otherwise we might end up in an inconsistent state
     /// with only some properties modified.
 
-    self.targets = [:]
+    self.swiftPMTargets = [:]
     self.fileToTargets = [:]
     self.targetDependencies = [:]
 
@@ -451,10 +449,7 @@ extension SwiftPMBuildSystem {
       guard let targetIdentifier else {
         return
       }
-      var depth = depth
-      if let existingDepth = targets[targetIdentifier]?.depth {
-        depth = max(existingDepth, depth)
-      } else {
+      if swiftPMTargets[targetIdentifier] == nil {
         for source in buildTarget.sources + buildTarget.headers {
           fileToTargets[DocumentURI(source), default: []].insert(targetIdentifier)
         }
@@ -464,7 +459,7 @@ extension SwiftPMBuildSystem {
       {
         self.targetDependencies[parentIdentifier, default: []].insert(targetIdentifier)
       }
-      targets[targetIdentifier] = (buildTarget, depth)
+      swiftPMTargets[targetIdentifier] = buildTarget
     }
 
     await messageHandler?.sendNotificationToSourceKitLSP(DidChangeBuildTargetNotification(changes: nil))
@@ -523,9 +518,9 @@ extension SwiftPMBuildSystem: BuildSystemIntegration.BuiltInBuildSystem {
   }
 
   package func buildTargets(request: BuildTargetsRequest) async throws -> BuildTargetsResponse {
-    let targets = self.targets.map { (targetId, target) in
+    let targets = self.swiftPMTargets.map { (targetId, target) in
       var tags: [BuildTargetTag] = [.test]
-      if target.depth != 1 {
+      if !target.isPartOfRootPackage {
         tags.append(.dependency)
       }
       return BuildTarget(
@@ -547,10 +542,10 @@ extension SwiftPMBuildSystem: BuildSystemIntegration.BuiltInBuildSystem {
     // TODO: Query The SwiftPM build system for the document's language and add it to SourceItem.data
     // (https://github.com/swiftlang/sourcekit-lsp/issues/1267)
     for target in request.targets {
-      guard let swiftPMTarget = self.targets[target] else {
+      guard let swiftPMTarget = self.swiftPMTargets[target] else {
         continue
       }
-      let sources = swiftPMTarget.buildTarget.sources.map {
+      let sources = swiftPMTarget.sources.map {
         SourceItem(uri: DocumentURI($0), kind: .file, generated: false)
       }
       result.append(SourcesItem(target: target, sources: sources))
@@ -568,13 +563,13 @@ extension SwiftPMBuildSystem: BuildSystemIntegration.BuiltInBuildSystem {
       return try settings(forPackageManifest: path)
     }
 
-    guard let buildTarget = self.targets[request.target]?.buildTarget else {
+    guard let swiftPMTarget = self.swiftPMTargets[request.target] else {
       logger.fault("Did not find target \(request.target.forLogging)")
       return nil
     }
 
-    if !buildTarget.sources.lazy.map(DocumentURI.init).contains(request.textDocument.uri),
-      let substituteFile = buildTarget.sources.sorted(by: { $0.path < $1.path }).first
+    if !swiftPMTarget.sources.lazy.map(DocumentURI.init).contains(request.textDocument.uri),
+      let substituteFile = swiftPMTarget.sources.sorted(by: { $0.path < $1.path }).first
     {
       logger.info("Getting compiler arguments for \(url) using substitute file \(substituteFile)")
       // If `url` is not part of the target's source, it's most likely a header file. Fake compiler arguments for it
@@ -585,7 +580,7 @@ extension SwiftPMBuildSystem: BuildSystemIntegration.BuiltInBuildSystem {
       // getting its compiler arguments and then patching up the compiler arguments by replacing the substitute file
       // with the `.cpp` file.
       let buildSettings = FileBuildSettings(
-        compilerArguments: try await compilerArguments(for: DocumentURI(substituteFile), in: buildTarget),
+        compilerArguments: try await compilerArguments(for: DocumentURI(substituteFile), in: swiftPMTarget),
         workingDirectory: projectRoot.pathString
       ).patching(newFile: try resolveSymlinks(path).pathString, originalFile: substituteFile.absoluteString)
       return SourceKitOptionsResponse(
@@ -595,7 +590,7 @@ extension SwiftPMBuildSystem: BuildSystemIntegration.BuiltInBuildSystem {
     }
 
     return SourceKitOptionsResponse(
-      compilerArguments: try await compilerArguments(for: request.textDocument.uri, in: buildTarget),
+      compilerArguments: try await compilerArguments(for: request.textDocument.uri, in: swiftPMTarget),
       workingDirectory: projectRoot.pathString
     )
   }
@@ -635,14 +630,6 @@ extension SwiftPMBuildSystem: BuildSystemIntegration.BuiltInBuildSystem {
     await self.packageLoadingQueue.async {}.valuePropagatingCancellation
   }
 
-  package func topologicalSort(of targets: [BuildTargetIdentifier]) -> [BuildTargetIdentifier]? {
-    return targets.sorted { (lhs: BuildTargetIdentifier, rhs: BuildTargetIdentifier) -> Bool in
-      let lhsDepth = self.targets[lhs]?.depth ?? 0
-      let rhsDepth = self.targets[rhs]?.depth ?? 0
-      return lhsDepth > rhsDepth
-    }
-  }
-
   package func prepare(request: PrepareTargetsRequest) async throws -> VoidResponse {
     // TODO: Support preparation of multiple targets at once. (https://github.com/swiftlang/sourcekit-lsp/issues/1262)
     for target in request.targets {
@@ -804,21 +791,6 @@ extension SwiftPMBuildSystem: BuildSystemIntegration.BuiltInBuildSystem {
     }
   }
 
-  package func sourceFiles() -> [SourceFileInfo] {
-    var sourceFiles: [DocumentURI: SourceFileInfo] = [:]
-    for (buildTarget, depth) in self.targets.values {
-      for sourceFile in buildTarget.sources {
-        let uri = DocumentURI(sourceFile)
-        sourceFiles[uri] = SourceFileInfo(
-          uri: uri,
-          isPartOfRootProject: depth == 1 || (sourceFiles[uri]?.isPartOfRootProject ?? false),
-          mayContainTests: true
-        )
-      }
-    }
-    return sourceFiles.values.sorted { $0.uri.pseudoPath < $1.uri.pseudoPath }
-  }
-
   package func addSourceFilesDidChangeCallback(_ callback: @Sendable @escaping () async -> Void) async {
     testFilesDidChangeCallbacks.append(callback)
   }