Merge pull request #1273 from ahoppen/background-preparation

Author: ahoppen

Sources/SKCore/BuildServerBuildSystem.swift

@@ -279,6 +279,16 @@ extension BuildServerBuildSystem: BuildSystem {
     return [ConfiguredTarget(targetID: "dummy", runDestinationID: "dummy")]
   }
 
+  public func generateBuildGraph() {}
+
+  public func topologicalSort(of targets: [ConfiguredTarget]) async -> [ConfiguredTarget]? {
+    return nil
+  }
+
+  public func prepare(targets: [ConfiguredTarget]) async throws {
+    throw PrepareNotSupportedError()
+  }
+
   public func registerForChangeNotifications(for uri: DocumentURI) {
     let request = RegisterForChanges(uri: uri, action: .register)
     _ = self.buildServer?.send(request) { result in

Sources/SKCore/BuildSystem.swift

@@ -31,14 +31,19 @@ public struct SourceFileInfo: Sendable {
   /// The URI of the source file.
   public let uri: DocumentURI
 
+  /// `true` if this file belongs to the root project that the user is working on. It is false, if the file belongs
+  /// to a dependency of the project.
+  public let isPartOfRootProject: Bool
+
   /// Whether the file might contain test cases. This property is an over-approximation. It might be true for files
   /// from non-test targets or files that don't actually contain any tests. Keeping this list of files with
   /// `mayContainTets` minimal as possible helps reduce the amount of work that the syntactic test indexer needs to
   /// perform.
   public let mayContainTests: Bool
 
-  public init(uri: DocumentURI, mayContainTests: Bool) {
+  public init(uri: DocumentURI, isPartOfRootProject: Bool, mayContainTests: Bool) {
     self.uri = uri
+    self.isPartOfRootProject = isPartOfRootProject
     self.mayContainTests = mayContainTests
   }
 }
@@ -64,6 +69,13 @@ public struct ConfiguredTarget: Hashable, Sendable {
   }
 }
 
+/// An error build systems can throw from `prepare` if they don't support preparation of targets.
+public struct PrepareNotSupportedError: Error, CustomStringConvertible {
+  public init() {}
+
+  public var description: String { "Preparation not supported" }
+}
+
 /// Provider of FileBuildSettings and other build-related information.
 ///
 /// The primary role of the build system is to answer queries for
@@ -114,6 +126,22 @@ public 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 including all the tasks that are necessary for building the entire build graph, like
+  /// resolving package versions.
+  func generateBuildGraph() async throws
+
+  /// 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: [ConfiguredTarget]) async -> [ConfiguredTarget]?
+
+  /// Prepare the given targets for indexing and semantic functionality. This should build all swift modules of target
+  /// dependencies.
+  func prepare(targets: [ConfiguredTarget]) async throws
+
   /// If the build system has knowledge about the language that this document should be compiled in, return it.
   ///
   /// This is used to determine the language in which a source file should be background indexed.
@@ -146,5 +174,3 @@ public protocol BuildSystem: AnyObject, Sendable {
   /// The callback might also be called without an actual change to `sourceFiles`.
   func addSourceFilesDidChangeCallback(_ callback: @Sendable @escaping () async -> Void) async
 }
-
-public let buildTargetsNotSupported = ResponseError.methodNotFound(BuildTargets.method)

Sources/SKCore/BuildSystemManager.swift

@@ -208,6 +208,18 @@ extension BuildSystemManager {
     return settings
   }
 
+  public func generateBuildGraph() async throws {
+    try await self.buildSystem?.generateBuildGraph()
+  }
+
+  public func topologicalSort(of targets: [ConfiguredTarget]) async throws -> [ConfiguredTarget]? {
+    return await buildSystem?.topologicalSort(of: targets)
+  }
+
+  public func prepare(targets: [ConfiguredTarget]) async throws {
+    try await buildSystem?.prepare(targets: targets)
+  }
+
   public func registerForChangeNotifications(for uri: DocumentURI, language: Language) async {
     logger.debug("registerForChangeNotifications(\(uri.forLogging))")
     let mainFile = await mainFile(for: uri, language: language)
@@ -247,7 +259,7 @@ extension BuildSystemManager {
 
   public func testFiles() async -> [DocumentURI] {
     return await sourceFiles().compactMap { (info: SourceFileInfo) -> DocumentURI? in
-      guard info.mayContainTests else {
+      guard info.isPartOfRootProject, info.mayContainTests else {
         return nil
       }
       return info.uri

Sources/SKCore/CompilationDatabaseBuildSystem.swift

@@ -125,6 +125,16 @@ extension CompilationDatabaseBuildSystem: BuildSystem {
     return [ConfiguredTarget(targetID: "dummy", runDestinationID: "dummy")]
   }
 
+  public func prepare(targets: [ConfiguredTarget]) async throws {
+    throw PrepareNotSupportedError()
+  }
+
+  public func generateBuildGraph() {}
+
+  public func topologicalSort(of targets: [ConfiguredTarget]) -> [ConfiguredTarget]? {
+    return nil
+  }
+
   public func registerForChangeNotifications(for uri: DocumentURI) async {
     self.watchedFiles.insert(uri)
   }
@@ -208,7 +218,7 @@ extension CompilationDatabaseBuildSystem: BuildSystem {
       return []
     }
     return compdb.allCommands.map {
-      SourceFileInfo(uri: DocumentURI($0.url), mayContainTests: true)
+      SourceFileInfo(uri: DocumentURI($0.url), isPartOfRootProject: true, mayContainTests: true)
     }
   }
 

Sources/SKSupport/Collection+PartitionIntoBatches.swift

@@ -10,7 +10,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-public extension Collection {
+public extension Collection where Index == Int {
   /// Partition the elements of the collection into `numberOfBatches` roughly equally sized batches.
   ///
   /// Elements are assigned to the batches round-robin. This ensures that elements that are close to each other in the
@@ -32,4 +32,18 @@ public extension Collection {
     }
     return batches.filter { !$0.isEmpty }
   }
+
+  /// Partition the collection into batches that have a maximum size of `batchSize`.
+  ///
+  /// The last batch will contain the remainder elements.
+  func partition(intoBatchesOfSize batchSize: Int) -> [[Element]] {
+    var batches: [[Element]] = []
+    batches.reserveCapacity(self.count / batchSize)
+    var lastIndex = self.startIndex
+    for index in stride(from: self.startIndex, to: self.endIndex, by: batchSize).dropFirst() + [self.endIndex] {
+      batches.append(Array(self[lastIndex..<index]))
+      lastIndex = index
+    }
+    return batches
+  }
 }

Sources/SKSwiftPMWorkspace/SwiftPMBuildSystem.swift

@@ -100,7 +100,11 @@ public actor SwiftPMBuildSystem {
 
   var fileToTarget: [AbsolutePath: SwiftBuildTarget] = [:]
   var sourceDirToTarget: [AbsolutePath: SwiftBuildTarget] = [:]
-  var targets: [SwiftBuildTarget] = []
+
+  /// Maps target ids (aka. `ConfiguredTarget.targetID`) to their SwiftPM build target as well as an index in their
+  /// topological sorting. Targets with lower index are more low level, ie. targets with higher indices depend on
+  /// targets with lower indices.
+  var targets: [String: (index: Int, buildTarget: SwiftBuildTarget)] = [:]
 
   /// The URIs for which the delegate has registered for change notifications,
   /// mapped to the language the delegate specified when registering for change notifications.
@@ -119,6 +123,18 @@ public actor SwiftPMBuildSystem {
   /// Force-unwrapped optional because initializing it requires access to `self`.
   var fileDependenciesUpdatedDebouncer: Debouncer<Set<DocumentURI>>! = nil
 
+  /// A `ObservabilitySystem` from `SwiftPM` that logs.
+  private let observabilitySystem = ObservabilitySystem({ scope, diagnostic in
+    logger.log(level: diagnostic.severity.asLogLevel, "SwiftPM log: \(diagnostic.description)")
+  })
+
+  /// Whether the SwiftPMBuildSystem may modify `Package.resolved` or not.
+  ///
+  /// This is `false` if the `SwiftPMBuildSystem` is pointed at a `.index-build` directory that's independent of the
+  /// user's build. In this case `SwiftPMBuildSystem` is allowed to clone repositories even if no `Package.resolved`
+  /// exists.
+  private let forceResolvedVersions: Bool
+
   /// Creates a build system using the Swift Package Manager, if this workspace is a package.
   ///
   /// - Parameters:
@@ -132,11 +148,13 @@ public actor SwiftPMBuildSystem {
     toolchainRegistry: ToolchainRegistry,
     fileSystem: FileSystem = localFileSystem,
     buildSetup: BuildSetup,
+    forceResolvedVersions: Bool,
     reloadPackageStatusCallback: @escaping (ReloadPackageStatus) async -> Void = { _ in }
   ) async throws {
     self.workspacePath = workspacePath
     self.fileSystem = fileSystem
     self.toolchainRegistry = toolchainRegistry
+    self.forceResolvedVersions = forceResolvedVersions
 
     guard let packageRoot = findPackageDirectory(containing: workspacePath, fileSystem) else {
       throw Error.noManifest(workspacePath: workspacePath)
@@ -204,7 +222,6 @@ public actor SwiftPMBuildSystem {
       }
       await delegate.filesDependenciesUpdated(filesWithUpdatedDependencies)
     }
-
     try await reloadPackage()
   }
 
@@ -217,6 +234,7 @@ public actor SwiftPMBuildSystem {
     url: URL,
     toolchainRegistry: ToolchainRegistry,
     buildSetup: BuildSetup,
+    forceResolvedVersions: Bool,
     reloadPackageStatusCallback: @escaping (ReloadPackageStatus) async -> Void
   ) async {
     do {
@@ -225,6 +243,7 @@ public actor SwiftPMBuildSystem {
         toolchainRegistry: toolchainRegistry,
         fileSystem: localFileSystem,
         buildSetup: buildSetup,
+        forceResolvedVersions: forceResolvedVersions,
         reloadPackageStatusCallback: reloadPackageStatusCallback
       )
     } catch Error.noManifest {
@@ -237,6 +256,9 @@ public actor SwiftPMBuildSystem {
 }
 
 extension SwiftPMBuildSystem {
+  public func generateBuildGraph() async throws {
+    try await self.reloadPackage()
+  }
 
   /// (Re-)load the package settings by parsing the manifest and resolving all the targets and
   /// dependencies.
@@ -248,13 +270,9 @@ extension SwiftPMBuildSystem {
       }
     }
 
-    let observabilitySystem = ObservabilitySystem({ scope, diagnostic in
-      logger.log(level: diagnostic.severity.asLogLevel, "SwiftPM log: \(diagnostic.description)")
-    })
-
     let modulesGraph = try self.workspace.loadPackageGraph(
       rootInput: PackageGraphRootInput(packages: [AbsolutePath(projectRoot)]),
-      forceResolvedVersions: true,
+      forceResolvedVersions: forceResolvedVersions,
       observabilityScope: observabilitySystem.topScope
     )
 
@@ -272,7 +290,15 @@ extension SwiftPMBuildSystem {
     /// with only some properties modified.
     self.modulesGraph = modulesGraph
 
-    self.targets = try buildDescription.allTargetsInTopologicalOrder(in: modulesGraph)
+    self.targets = Dictionary(
+      try buildDescription.allTargetsInTopologicalOrder(in: modulesGraph).enumerated().map { (index, target) in
+        return (key: target.name, (index, target))
+      },
+      uniquingKeysWith: { first, second in
+        logger.fault("Found two targets with the same name \(first.buildTarget.name)")
+        return second
+      }
+    )
 
     self.fileToTarget = [AbsolutePath: SwiftBuildTarget](
       modulesGraph.allTargets.flatMap { target in
@@ -343,14 +369,8 @@ extension SwiftPMBuildSystem: SKCore.BuildSystem {
       return try settings(forPackageManifest: path)
     }
 
-    let buildTargets = self.targets.filter({ $0.name == configuredTarget.targetID })
-    if buildTargets.count > 1 {
-      logger.error("Found multiple targets with name \(configuredTarget.targetID). Picking the first one")
-    }
-    guard let buildTarget = buildTargets.first else {
-      if buildTargets.isEmpty {
-        logger.error("Did not find target with name \(configuredTarget.targetID)")
-      }
+    guard let buildTarget = self.targets[configuredTarget.targetID]?.buildTarget else {
+      logger.error("Did not find target with name \(configuredTarget.targetID)")
       return nil
     }
 
@@ -368,7 +388,8 @@ extension SwiftPMBuildSystem: SKCore.BuildSystem {
   }
 
   public func defaultLanguage(for document: DocumentURI) async -> Language? {
-    // TODO (indexing): Query The SwiftPM build system for the document's language
+    // TODO (indexing): Query The SwiftPM build system for the document's language.
+    // https://github.com/apple/sourcekit-lsp/issues/1267
     return nil
   }
 
@@ -395,6 +416,77 @@ extension SwiftPMBuildSystem: SKCore.BuildSystem {
     return []
   }
 
+  public func topologicalSort(of targets: [ConfiguredTarget]) -> [ConfiguredTarget]? {
+    return targets.sorted { (lhs: ConfiguredTarget, rhs: ConfiguredTarget) -> Bool in
+      let lhsIndex = self.targets[lhs.targetID]?.index ?? self.targets.count
+      let rhsIndex = self.targets[lhs.targetID]?.index ?? self.targets.count
+      return lhsIndex < rhsIndex
+    }
+  }
+
+  public func prepare(targets: [ConfiguredTarget]) async throws {
+    // TODO (indexing): Support preparation of multiple targets at once.
+    // https://github.com/apple/sourcekit-lsp/issues/1262
+    for target in targets {
+      try await prepare(singleTarget: target)
+    }
+  }
+
+  private func prepare(singleTarget target: ConfiguredTarget) async throws {
+    // TODO (indexing): Add a proper 'prepare' job in SwiftPM instead of building the target.
+    // https://github.com/apple/sourcekit-lsp/issues/1254
+    guard let toolchain = await toolchainRegistry.default else {
+      logger.error("Not preparing because not toolchain exists")
+      return
+    }
+    guard let swift = toolchain.swift else {
+      logger.error(
+        "Not preparing because toolchain at \(toolchain.identifier) does not contain a Swift compiler"
+      )
+      return
+    }
+    let arguments = [
+      swift.pathString, "build",
+      "--scratch-path", self.workspace.location.scratchDirectory.pathString,
+      "--disable-index-store",
+      "--target", target.targetID,
+    ]
+    let process = Process(
+      arguments: arguments,
+      workingDirectory: workspacePath
+    )
+    try process.launch()
+    let result = try await process.waitUntilExitSendingSigIntOnTaskCancellation()
+    switch result.exitStatus.exhaustivelySwitchable {
+    case .terminated(code: 0):
+      break
+    case .terminated(code: let code):
+      // This most likely happens if there are compilation errors in the source file. This is nothing to worry about.
+      let stdout = (try? String(bytes: result.output.get(), encoding: .utf8)) ?? "<no stderr>"
+      let stderr = (try? String(bytes: result.stderrOutput.get(), encoding: .utf8)) ?? "<no stderr>"
+      logger.debug(
+        """
+        Preparation of target \(target.targetID) terminated with non-zero exit code \(code)
+        Stderr:
+        \(stderr)
+        Stdout:
+        \(stdout)
+        """
+      )
+    case .signalled(signal: let signal):
+      if !Task.isCancelled {
+        // The indexing job finished with a signal. Could be because the compiler crashed.
+        // Ignore signal exit codes if this task has been cancelled because the compiler exits with SIGINT if it gets
+        // interrupted.
+        logger.error("Preparation of target \(target.targetID) signaled \(signal)")
+      }
+    case .abnormal(exception: let exception):
+      if !Task.isCancelled {
+        logger.error("Preparation of target \(target.targetID) exited abnormally \(exception)")
+      }
+    }
+  }
+
   public func registerForChangeNotifications(for uri: DocumentURI) async {
     self.watchedFiles.insert(uri)
   }
@@ -489,14 +581,11 @@ extension SwiftPMBuildSystem: SKCore.BuildSystem {
 
   public func sourceFiles() -> [SourceFileInfo] {
     return fileToTarget.compactMap { (path, target) -> SourceFileInfo? in
-      guard target.isPartOfRootPackage else {
-        // Don't consider files from package dependencies as possible test files.
-        return nil
-      }
       // We should only set mayContainTests to `true` for files from test targets
       // (https://github.com/apple/sourcekit-lsp/issues/1174).
       return SourceFileInfo(
         uri: DocumentURI(path.asURL),
+        isPartOfRootProject: target.isPartOfRootPackage,
         mayContainTests: true
       )
     }