Merge pull request #1665 from ahoppen/another-asp

Migrate the remaining methods in `BuiltInBuildSystem` to be BSP-based

Author: ahoppen

Sources/BuildServerProtocol/BuildTargetsRequest.swift

@@ -81,7 +81,9 @@ public struct BuildTarget: Codable, Hashable, Sendable {
     tags: [BuildTargetTag],
     capabilities: BuildTargetCapabilities,
     languageIds: [Language],
-    dependencies: [BuildTargetIdentifier]
+    dependencies: [BuildTargetIdentifier],
+    dataKind: BuildTargetDataKind? = nil,
+    data: LSPAny? = nil
   ) {
     self.id = id
     self.displayName = displayName
@@ -90,6 +92,8 @@ public struct BuildTarget: Codable, Hashable, Sendable {
     self.capabilities = capabilities
     self.languageIds = languageIds
     self.dependencies = dependencies
+    self.dataKind = dataKind
+    self.data = data
   }
 }
 
@@ -177,22 +181,52 @@ public struct BuildTargetDataKind: RawRepresentable, Codable, Hashable, Sendable
   }
 
   /// `data` field must contain a CargoBuildTarget object.
-  public static let cargo = "cargo"
+  public static let cargo = BuildTargetDataKind(rawValue: "cargo")
 
   /// `data` field must contain a CppBuildTarget object.
-  public static let cpp = "cpp"
+  public static let cpp = BuildTargetDataKind(rawValue: "cpp")
 
   /// `data` field must contain a JvmBuildTarget object.
-  public static let jvm = "jvm"
+  public static let jvm = BuildTargetDataKind(rawValue: "jvm")
 
   /// `data` field must contain a PythonBuildTarget object.
-  public static let python = "python"
+  public static let python = BuildTargetDataKind(rawValue: "python")
 
   /// `data` field must contain a SbtBuildTarget object.
-  public static let sbt = "sbt"
+  public static let sbt = BuildTargetDataKind(rawValue: "sbt")
 
   /// `data` field must contain a ScalaBuildTarget object.
-  public static let scala = "scala"
+  public static let scala = BuildTargetDataKind(rawValue: "scala")
+
+  /// `data` field must contain a SourceKitBuildTarget object.
+  public static let sourceKit = BuildTargetDataKind(rawValue: "sourceKit")
+}
+
+public struct SourceKitBuildTarget: LSPAnyCodable, Codable {
+  /// The toolchain that should be used to build this target. The URI should point to the directory that contains the
+  /// `usr` directory. On macOS, this is typically a bundle ending in `.xctoolchain`. If the toolchain is installed to
+  /// `/` on Linux, the toolchain URI would point to `/`.
+  ///
+  /// If no toolchain is given, SourceKit-LSP will pick a toolchain to use for this target.
+  public var toolchain: URI?
+
+  public init(toolchain: URI? = nil) {
+    self.toolchain = toolchain
+  }
+
+  public init(fromLSPDictionary dictionary: [String: LanguageServerProtocol.LSPAny]) {
+    if case .string(let toolchain) = dictionary[CodingKeys.toolchain.stringValue] {
+      self.toolchain = try? URI(string: toolchain)
+    }
+  }
+
+  public func encodeToLSPAny() -> LanguageServerProtocol.LSPAny {
+    var result: [String: LSPAny] = [:]
+    if let toolchain {
+      result[CodingKeys.toolchain.stringValue] = .string(toolchain.stringValue)
+    }
+    return .dictionary(result)
+  }
 }
 
 /// The build target output paths request is sent from the client to the server

Sources/BuildServerProtocol/CMakeLists.txt

@@ -10,7 +10,8 @@ add_library(BuildServerProtocol STATIC
   PrepareTargetsRequest.swift
   RegisterForChangeNotifications.swift
   ShutdownBuild.swift
-  SourceKitOptionsRequest.swift)
+  SourceKitOptionsRequest.swift
+  WaitForBuildSystemUpdates.swift)
 set_target_properties(BuildServerProtocol PROPERTIES
   INTERFACE_INCLUDE_DIRECTORIES ${CMAKE_Swift_MODULE_DIRECTORY})
 target_link_libraries(BuildServerProtocol PRIVATE

Sources/BuildServerProtocol/InitializeBuild.swift

@@ -104,7 +104,7 @@ public struct InitializeBuildResponseDataKind: RawRepresentable, Hashable, Codab
   }
 
   /// `data` field must contain a `SourceKitInitializeBuildResponseData` object.
-  public static let sourceKit = InitializeBuildResponseDataKind(rawValue: "sourcekit")
+  public static let sourceKit = InitializeBuildResponseDataKind(rawValue: "sourceKit")
 }
 
 public struct SourceKitInitializeBuildResponseData: LSPAnyCodable, Codable, Sendable {

Sources/BuildServerProtocol/WaitForBuildSystemUpdates.swift

@@ -0,0 +1,25 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2024 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+
+import LanguageServerProtocol
+
+/// This request is a no-op and doesn't have any effects.
+///
+/// If the build system is currently updating the build graph, this request should return after those updates have
+/// finished processing.
+public struct WaitForBuildSystemUpdatesRequest: RequestType, Hashable {
+  public typealias Response = VoidResponse
+
+  public static let method: String = "workspace/waitForBuildSystemUpdates"
+
+  public init() {}
+}

Sources/BuildSystemIntegration/BuildServerBuildSystem.swift

@@ -330,15 +330,8 @@ extension BuildServerBuildSystem: BuiltInBuildSystem {
     )
   }
 
-  package func toolchain(for uri: DocumentURI, _ language: Language) async -> Toolchain? {
-    return nil
-  }
-
-  package func waitForUpToDateBuildGraph() async {}
-
-  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).
+  package func waitForUpBuildSystemUpdates(request: WaitForBuildSystemUpdatesRequest) async -> VoidResponse {
+    return VoidResponse()
   }
 }
 

Sources/BuildSystemIntegration/BuildSystemDelegate.swift

@@ -32,4 +32,7 @@ package protocol BuildSystemManagerDelegate: AnyObject, Sendable {
 
   /// Log the given message to the client's index log.
   func logMessageToIndexLog(taskID: IndexTaskID, message: String)
+
+  /// Notify the delegate that the list of source files in the build system might have changed.
+  func sourceFilesDidChange() async
 }

Sources/BuildSystemIntegration/BuildSystemManager.swift

@@ -25,15 +25,15 @@ import struct TSCBasic.AbsolutePath
 import os
 #endif
 
-fileprivate class RequestCache<Request: RequestType & Hashable> {
-  private var storage: [Request: Task<Request.Response, Error>] = [:]
+fileprivate class RequestCache<Request: RequestType & Hashable, Result: Sendable> {
+  private var storage: [Request: Task<Result, Error>] = [:]
 
   func get(
     _ key: Request,
     isolation: isolated any Actor = #isolation,
-    compute: @Sendable @escaping (Request) async throws(Error) -> Request.Response
-  ) async throws(Error) -> Request.Response {
-    let task: Task<Request.Response, Error>
+    compute: @Sendable @escaping (Request) async throws(Error) -> Result
+  ) async throws(Error) -> Result {
+    let task: Task<Result, Error>
     if let cached = storage[key] {
       task = cached
     } else {
@@ -110,15 +110,15 @@ package actor BuildSystemManager: BuiltInBuildSystemAdapterDelegate {
   /// Force-unwrapped optional because initializing it requires access to `self`.
   private var filesDependenciesUpdatedDebouncer: Debouncer<Set<DocumentURI>>! = nil
 
-  private var cachedTargetsForDocument = RequestCache<InverseSourcesRequest>()
+  private var cachedTargetsForDocument = RequestCache<InverseSourcesRequest, InverseSourcesResponse>()
 
-  private var cachedSourceKitOptions = RequestCache<SourceKitOptionsRequest>()
+  private var cachedSourceKitOptions = RequestCache<SourceKitOptionsRequest, SourceKitOptionsResponse?>()
 
-  private var cachedBuildTargets = RequestCache<BuildTargetsRequest>()
+  private var cachedBuildTargets = RequestCache<
+    BuildTargetsRequest, [BuildTargetIdentifier: (target: BuildTarget, depth: Int)]
+  >()
 
-  private var cachedTargetSources = RequestCache<BuildTargetSourcesRequest>()
-
-  private var cachedTargetDepths: (buildTargets: [BuildTarget], depths: [BuildTargetIdentifier: Int])? = nil
+  private var cachedTargetSources = RequestCache<BuildTargetSourcesRequest, BuildTargetSourcesResponse>()
 
   /// 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
@@ -176,6 +176,8 @@ package actor BuildSystemManager: BuiltInBuildSystemAdapterDelegate {
       }
       await delegate.filesDependenciesUpdated(changedWatchedFiles)
     }
+
+    // FIXME: (BSP migration) Forward file watch patterns from this initialize request to the client
     initializeResult = Task { () -> InitializeBuildResponse? in
       guard let buildSystem else {
         return nil
@@ -222,10 +224,9 @@ package actor BuildSystemManager: BuiltInBuildSystemAdapterDelegate {
     if !options.backgroundIndexingOrDefault,
       events.contains(where: { $0.uri.fileURL?.pathExtension == "swiftmodule" })
     {
-      let targets = await orLog("Getting build targets") {
-        try await self.buildTargets()
+      await orLog("Getting build targets") {
+        targetsWithUpdatedDependencies.formUnion(try await self.buildTargets().keys)
       }
-      targetsWithUpdatedDependencies.formUnion(targets?.map(\.id) ?? [])
     }
 
     var filesWithUpdatedDependencies: Set<DocumentURI> = []
@@ -271,9 +272,37 @@ package actor BuildSystemManager: BuiltInBuildSystemAdapterDelegate {
   }
 
   /// Returns the toolchain that should be used to process the given document.
-  package func toolchain(for uri: DocumentURI, _ language: Language) async -> Toolchain? {
-    if let toolchain = await buildSystem?.underlyingBuildSystem.toolchain(for: uri, language) {
-      return toolchain
+  package func toolchain(
+    for uri: DocumentURI,
+    in target: BuildTargetIdentifier?,
+    language: Language
+  ) async -> Toolchain? {
+    let toolchainPath = await orLog("Getting toolchain from build targets") { () -> AbsolutePath? in
+      guard let target else {
+        return nil
+      }
+      let targets = try await self.buildTargets()
+      guard let target = targets[target]?.target else {
+        logger.error("Failed to find target \(target.forLogging) to determine toolchain")
+        return nil
+      }
+      guard target.dataKind == .sourceKit, case .dictionary(let data) = target.data else {
+        return nil
+      }
+      guard let toolchain = SourceKitBuildTarget(fromLSPDictionary: data).toolchain else {
+        return nil
+      }
+      guard let toolchainUrl = toolchain.fileURL else {
+        logger.error("Toolchain is not a file URL")
+        return nil
+      }
+      return try AbsolutePath(validating: toolchainUrl.path)
+    }
+    if let toolchainPath {
+      if let toolchain = await self.toolchainRegistry.toolchain(withPath: toolchainPath) {
+        return toolchain
+      }
+      logger.error("Toolchain at \(toolchainPath) not registered in toolchain registry.")
     }
 
     switch language {
@@ -484,15 +513,14 @@ package actor BuildSystemManager: BuiltInBuildSystemAdapterDelegate {
   }
 
   package func waitForUpToDateBuildGraph() async {
-    await self.buildSystem?.underlyingBuildSystem.waitForUpToDateBuildGraph()
+    await orLog("Waiting for build system updates") {
+      let _: VoidResponse? = try await self.buildSystem?.send(WaitForBuildSystemUpdatesRequest())
+    }
   }
 
   /// 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)
@@ -511,7 +539,6 @@ package actor BuildSystemManager: BuiltInBuildSystemAdapterDelegate {
         }
       }
     }
-    cachedTargetDepths = (buildTargets, depths)
     return depths
   }
 
@@ -522,25 +549,25 @@ package actor BuildSystemManager: BuiltInBuildSystemAdapterDelegate {
   ///
   /// `nil` if the build system doesn't support topological sorting of targets.
   package func topologicalSort(of targets: [BuildTargetIdentifier]) async throws -> [BuildTargetIdentifier]? {
-    guard let workspaceTargets = await orLog("Getting build targets for topological sort", { try await buildTargets() })
+    guard let buildTargets = 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]
+      return (buildTargets[lhs]?.depth ?? 0) > (buildTargets[rhs]?.depth ?? 0)
     }
   }
 
   /// Returns the list of targets that might depend on the given target and that need to be re-prepared when a file in
   /// `target` is modified.
   package func targets(dependingOn targetIds: Set<BuildTargetIdentifier>) async -> [BuildTargetIdentifier] {
-    guard let buildTargets = await orLog("Getting build targets for dependencies", { try await self.buildTargets() })
+    guard
+      let buildTargets = await orLog("Getting build targets for dependencies", { try await self.buildTargets().values })
     else {
       return []
     }
-    return buildTargets.filter { $0.dependencies.contains(anyIn: targetIds) }.map { $0.id }
+    return buildTargets.filter { $0.target.dependencies.contains(anyIn: targetIds) }.map { $0.target.id }
   }
 
   package func prepare(
@@ -564,26 +591,46 @@ package actor BuildSystemManager: BuiltInBuildSystemAdapterDelegate {
     self.watchedFiles[uri] = nil
   }
 
-  package func buildTargets() async throws -> [BuildTarget] {
+  package func buildTargets() async throws -> [BuildTargetIdentifier: (target: BuildTarget, depth: Int)] {
     guard let buildSystem else {
-      return []
+      return [:]
     }
 
     let request = BuildTargetsRequest()
-    let response = try await cachedBuildTargets.get(request) { request in
-      try await buildSystem.send(request)
+    let result = try await cachedBuildTargets.get(request) { request in
+      let buildTargets = try await buildSystem.send(request).targets
+      let depths = await self.targetDepths(for: buildTargets)
+      var result: [BuildTargetIdentifier: (target: BuildTarget, depth: Int)] = [:]
+      result.reserveCapacity(buildTargets.count)
+      for buildTarget in buildTargets {
+        guard result[buildTarget.id] == nil else {
+          logger.error("Found two targets with the same ID \(buildTarget.id)")
+          continue
+        }
+        let depth: Int
+        if let d = depths[buildTarget.id] {
+          depth = d
+        } else {
+          logger.fault("Did not compute depth for target \(buildTarget.id)")
+          depth = 0
+        }
+        result[buildTarget.id] = (buildTarget, depth)
+      }
+      return result
     }
-    return response.targets
+    return result
   }
 
-  package func sourceFiles(in targets: [BuildTargetIdentifier]) async throws -> [SourcesItem] {
+  package func sourceFiles(in targets: some Sequence<BuildTargetIdentifier>) async throws -> [SourcesItem] {
     guard let buildSystem else {
       return []
     }
 
     // FIXME: (BSP migration) If we have a cached request for a superset of the targets, serve the result from that
     // cache entry.
-    let request = BuildTargetSourcesRequest.init(targets: targets)
+    // Sort targets to help cache hits if we have two calls to `sourceFiles` with targets in different orders.
+    let sortedTargets = targets.sorted { $0.uri.stringValue < $1.uri.stringValue }
+    let request = BuildTargetSourcesRequest(targets: sortedTargets)
     let response = try await cachedTargetSources.get(request) { request in
       try await buildSystem.send(request)
     }
@@ -595,9 +642,8 @@ package actor BuildSystemManager: BuiltInBuildSystemAdapterDelegate {
     // retrieving the source files for those targets.
     // FIXME: (BSP Migration) Handle source files that are in multiple targets
     let targets = try await self.buildTargets()
-    let targetsById = Dictionary(elements: targets, keyedBy: \.id)
-    let sourceFiles = try await self.sourceFiles(in: targets.map(\.id)).flatMap { sourcesItem in
-      let target = targetsById[sourcesItem.target]
+    let sourceFiles = try await self.sourceFiles(in: targets.keys).flatMap { sourcesItem in
+      let target = targets[sourcesItem.target]?.target
       return sourcesItem.sources.map { sourceItem in
         SourceFileInfo(
           uri: sourceItem.uri,
@@ -663,6 +709,7 @@ extension BuildSystemManager {
     // FIXME: (BSP Migration) Communicate that the build target has changed to the `BuildSystemManagerDelegate` and make
     // it responsible for figuring out which files are affected.
     await delegate?.fileBuildSettingsChanged(Set(watchedFiles.keys))
+    await self.delegate?.sourceFilesDidChange()
   }
 
   private func logMessage(notification: BuildServerProtocol.LogMessageNotification) async {