Implement reloadPackageStatusCallback using BSP messages

Author: ahoppen

Sources/BuildServerProtocol/CMakeLists.txt

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

Sources/BuildServerProtocol/WorkDoneProgress.swift

@@ -0,0 +1,16 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2022 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
+
+public typealias CreateWorkDoneProgressRequest = LanguageServerProtocol.CreateWorkDoneProgressRequest
+public typealias WorkDoneProgress = LanguageServerProtocol.WorkDoneProgress

Sources/BuildSystemIntegration/BuildServerBuildSystem.swift

@@ -67,7 +67,7 @@ package actor BuildServerBuildSystem: MessageHandler {
   package private(set) var indexDatabasePath: AbsolutePath?
   package private(set) var indexStorePath: AbsolutePath?
 
-  package weak var messageHandler: BuiltInBuildSystemMessageHandler?
+  package let connectionToSourceKitLSP: any Connection
 
   /// The build settings that have been received from the build server.
   private var buildSettings: [DocumentURI: SourceKitOptionsResponse] = [:]
@@ -76,7 +76,7 @@ package actor BuildServerBuildSystem: MessageHandler {
 
   package init(
     projectRoot: AbsolutePath,
-    messageHandler: BuiltInBuildSystemMessageHandler?,
+    connectionToSourceKitLSP: any Connection,
     fileSystem: FileSystem = localFileSystem
   ) async throws {
     let configPath = projectRoot.appending(component: "buildServer.json")
@@ -96,18 +96,18 @@ package actor BuildServerBuildSystem: MessageHandler {
     #endif
     self.projectRoot = projectRoot
     self.serverConfig = config
-    self.messageHandler = messageHandler
+    self.connectionToSourceKitLSP = connectionToSourceKitLSP
     try await self.initializeBuildServer()
   }
 
   /// Creates a build system using the Build Server Protocol config.
   ///
   /// - Returns: nil if `projectRoot` has no config or there is an error parsing it.
-  package init?(projectRoot: AbsolutePath?, messageHandler: BuiltInBuildSystemMessageHandler?) async {
+  package init?(projectRoot: AbsolutePath?, connectionToSourceKitLSP: any Connection) async {
     guard let projectRoot else { return nil }
 
     do {
-      try await self.init(projectRoot: projectRoot, messageHandler: messageHandler)
+      try await self.init(projectRoot: projectRoot, connectionToSourceKitLSP: connectionToSourceKitLSP)
     } catch is FileSystemError {
       // config file was missing, no build server for this workspace
       return nil
@@ -218,8 +218,8 @@ package actor BuildServerBuildSystem: MessageHandler {
     reply(.failure(ResponseError.methodNotFound(R.method)))
   }
 
-  func handleBuildTargetsChanged(_ notification: DidChangeBuildTargetNotification) async {
-    await self.messageHandler?.sendNotificationToSourceKitLSP(notification)
+  func handleBuildTargetsChanged(_ notification: DidChangeBuildTargetNotification) {
+    connectionToSourceKitLSP.send(notification)
   }
 
   func handleFileOptionsChanged(_ notification: FileOptionsChangedNotification) async {
@@ -238,7 +238,7 @@ package actor BuildServerBuildSystem: MessageHandler {
     // FIXME: (BSP migration) When running in the legacy mode where teh BSP server pushes build settings to us, we could
     // consider having a separate target for each source file so that we can update individual targets instead of having
     // to send an update for all targets.
-    await self.messageHandler?.sendNotificationToSourceKitLSP(DidChangeBuildTargetNotification(changes: nil))
+    connectionToSourceKitLSP.send(DidChangeBuildTargetNotification(changes: nil))
   }
 }
 

Sources/BuildSystemIntegration/BuildSystemDelegate.swift

@@ -31,8 +31,20 @@ package protocol BuildSystemManagerDelegate: AnyObject, Sendable {
   func buildTargetsChanged(_ changes: [BuildTargetEvent]?) async
 
   /// Log the given message to the client's index log.
+  // FIXME: (BSP Migration) Use `sendNotificationToClient`
   func logMessageToIndexLog(taskID: IndexTaskID, message: String)
 
+  /// Whether the client can handle `WorkDoneProgress` requests.
+  var clientSupportsWorkDoneProgress: Bool { get async }
+
+  func sendNotificationToClient(_ notification: some NotificationType)
+
+  func sendRequestToClient<R: RequestType>(_ request: R) async throws -> R.Response
+
+  /// Wait until the connection to the client has been initialized, ie. wait until `SourceKitLSPServer` has replied
+  /// to the `initialize` request.
+  func waitUntilInitialized() async
+
   /// Notify the delegate that the list of source files in the build system might have changed.
   func sourceFilesDidChange() async
 }

Sources/BuildSystemIntegration/BuildSystemManager.swift

@@ -12,6 +12,7 @@
 
 import BuildServerProtocol
 import Dispatch
+import Foundation
 import LanguageServerProtocol
 import SKLogging
 import SKOptions
@@ -148,8 +149,7 @@ package actor BuildSystemManager: MessageHandler {
     buildSystemKind: BuildSystemKind?,
     toolchainRegistry: ToolchainRegistry,
     options: SourceKitLSPOptions,
-    buildSystemTestHooks: BuildSystemTestHooks,
-    reloadPackageStatusCallback: @Sendable @escaping (ReloadPackageStatus) async -> Void
+    buildSystemTestHooks: BuildSystemTestHooks
   ) async {
     self.fallbackBuildSystem = FallbackBuildSystem(options: options.fallbackBuildSystemOrDefault)
     self.toolchainRegistry = toolchainRegistry
@@ -162,8 +162,7 @@ package actor BuildSystemManager: MessageHandler {
       toolchainRegistry: toolchainRegistry,
       options: options,
       buildSystemTestHooks: buildSystemTestHooks,
-      connectionToSourceKitLSP: connectionFromBuildSystemToSourceKitLSP,
-      reloadPackageStatusCallback: reloadPackageStatusCallback
+      connectionToSourceKitLSP: connectionFromBuildSystemToSourceKitLSP
     )
     if let buildSystem {
       let connectionFromSourceKitLSPToBuildSystem = LocalConnection(receiverName: "\(type(of: buildSystem))")
@@ -286,6 +285,8 @@ package actor BuildSystemManager: MessageHandler {
       await self.didChangeBuildTarget(notification: notification)
     case let notification as BuildServerProtocol.LogMessageNotification:
       await self.logMessage(notification: notification)
+    case let notification as BuildServerProtocol.WorkDoneProgress:
+      await self.workDoneProgress(notification: notification)
     default:
       logger.error("Ignoring unknown notification \(type(of: notification).method)")
     }
@@ -294,12 +295,87 @@ package actor BuildSystemManager: MessageHandler {
   /// Implementation of `MessageHandler`, handling requests from the build system.
   ///
   /// - Important: Do not call directly.
-  nonisolated package func handle<Request: RequestType>(
+  nonisolated package func handle<R: RequestType>(
+    _ params: R,
+    id: RequestID,
+    reply: @Sendable @escaping (LSPResult<R.Response>) -> Void
+  ) {
+    let signposter = Logger(subsystem: LoggingScope.subsystem, category: "build-system-message-handling")
+      .makeSignposter()
+    let signpostID = signposter.makeSignpostID()
+    let state = signposter.beginInterval("Request", id: signpostID, "\(R.self)")
+
+    messageHandlingQueue.async {
+      signposter.emitEvent("Start handling", id: signpostID)
+      await withTaskCancellationHandler {
+        await self.handleImpl(params, id: id, reply: reply)
+        signposter.endInterval("Request", state, "Done")
+      } onCancel: {
+        signposter.emitEvent("Cancelled", id: signpostID)
+      }
+    }
+  }
+
+  private func handleImpl<Request: RequestType>(
     _ request: Request,
     id: RequestID,
     reply: @escaping @Sendable (LSPResult<Request.Response>) -> Void
-  ) {
-    reply(.failure(ResponseError.methodNotFound(Request.method)))
+  ) async {
+    let startDate = Date()
+
+    let request = RequestAndReply(request) { result in
+      reply(result)
+      let endDate = Date()
+      Task {
+        switch result {
+        case .success(let response):
+          logger.log(
+            """
+            Succeeded (took \(endDate.timeIntervalSince(startDate) * 1000, privacy: .public)ms)
+            \(Request.method, privacy: .public)
+            \(response.forLogging)
+            """
+          )
+        case .failure(let error):
+          logger.log(
+            """
+            Failed (took \(endDate.timeIntervalSince(startDate) * 1000, privacy: .public)ms)
+            \(Request.method, privacy: .public)(\(id, privacy: .public))
+            \(error.forLogging, privacy: .private)
+            """
+          )
+        }
+      }
+    }
+
+    switch request {
+    case let request as RequestAndReply<BuildServerProtocol.CreateWorkDoneProgressRequest>:
+      await request.reply { try await self.createWorkDoneProgress(request: request.params) }
+    default:
+      await request.reply { throw ResponseError.methodNotFound(Request.method) }
+    }
+  }
+
+  private func createWorkDoneProgress(
+    request: BuildServerProtocol.CreateWorkDoneProgressRequest
+  ) async throws -> BuildServerProtocol.CreateWorkDoneProgressRequest.Response {
+    guard let delegate else {
+      throw ResponseError.unknown("Connection to client closed")
+    }
+    guard await delegate.clientSupportsWorkDoneProgress else {
+      throw ResponseError.unknown("Client does not support work done progress")
+    }
+    await delegate.waitUntilInitialized()
+    return try await delegate.sendRequestToClient(request as LanguageServerProtocol.CreateWorkDoneProgressRequest)
+  }
+
+  private func workDoneProgress(notification: BuildServerProtocol.WorkDoneProgress) async {
+    guard let delegate else {
+      logger.fault("Ignoring work done progress form build system because connection to client closed")
+      return
+    }
+    await delegate.waitUntilInitialized()
+    delegate.sendNotificationToClient(notification as LanguageServerProtocol.WorkDoneProgress)
   }
 
   /// - Note: Needed so we can set the delegate from a different isolation context.

Sources/BuildSystemIntegration/BuiltInBuildSystemAdapter.swift

@@ -22,13 +22,6 @@ import ToolchainRegistry
 import struct TSCBasic.AbsolutePath
 import struct TSCBasic.RelativePath
 
-// FIXME: (BSP Migration) This should be a MessageHandler once we have migrated all build system queries to BSP and can use
-// LocalConnection for the communication.
-package protocol BuiltInBuildSystemMessageHandler: AnyObject, Sendable {
-  func sendNotificationToSourceKitLSP(_ notification: some NotificationType) async
-  func sendRequestToSourceKitLSP<R: RequestType>(_ request: R) async throws -> R.Response
-}
-
 package enum BuildSystemKind {
   case buildServer(projectRoot: AbsolutePath)
   case compilationDatabase(projectRoot: AbsolutePath)
@@ -51,37 +44,35 @@ private func createBuildSystem(
   options: SourceKitLSPOptions,
   buildSystemTestHooks: BuildSystemTestHooks,
   toolchainRegistry: ToolchainRegistry,
-  messageHandler: BuiltInBuildSystemMessageHandler,
-  reloadPackageStatusCallback: @Sendable @escaping (ReloadPackageStatus) async -> Void
+  connectionToSourceKitLSP: any Connection
 ) async -> BuiltInBuildSystem? {
   switch buildSystemKind {
   case .buildServer(let projectRoot):
-    return await BuildServerBuildSystem(projectRoot: projectRoot, messageHandler: messageHandler)
+    return await BuildServerBuildSystem(projectRoot: projectRoot, connectionToSourceKitLSP: connectionToSourceKitLSP)
   case .compilationDatabase(let projectRoot):
     return CompilationDatabaseBuildSystem(
       projectRoot: projectRoot,
       searchPaths: (options.compilationDatabaseOrDefault.searchPaths ?? []).compactMap {
         try? RelativePath(validating: $0)
       },
-      messageHandler: messageHandler
+      connectionToSourceKitLSP: connectionToSourceKitLSP
     )
   case .swiftPM(let projectRoot):
     return await SwiftPMBuildSystem(
       projectRoot: projectRoot,
       toolchainRegistry: toolchainRegistry,
       options: options,
-      messageHandler: messageHandler,
-      reloadPackageStatusCallback: reloadPackageStatusCallback,
+      connectionToSourceKitLSP: connectionToSourceKitLSP,
       testHooks: buildSystemTestHooks.swiftPMTestHooks
     )
   case .testBuildSystem(let projectRoot):
-    return TestBuildSystem(projectRoot: projectRoot, messageHandler: messageHandler)
+    return TestBuildSystem(projectRoot: projectRoot, connectionToSourceKitLSP: connectionToSourceKitLSP)
   }
 }
 
 /// A type that outwardly acts as a build server conforming to the Build System Integration Protocol and internally uses
 /// a `BuiltInBuildSystem` to satisfy the requests.
-package actor BuiltInBuildSystemAdapter: BuiltInBuildSystemMessageHandler, MessageHandler {
+package actor BuiltInBuildSystemAdapter: MessageHandler {
   /// The underlying build system
   // FIXME: (BSP Migration) This should be private, all messages should go through BSP. Only accessible from the outside for transition
   // purposes.
@@ -96,8 +87,7 @@ package actor BuiltInBuildSystemAdapter: BuiltInBuildSystemMessageHandler, Messa
     toolchainRegistry: ToolchainRegistry,
     options: SourceKitLSPOptions,
     buildSystemTestHooks: BuildSystemTestHooks,
-    connectionToSourceKitLSP: LocalConnection,
-    reloadPackageStatusCallback: @Sendable @escaping (ReloadPackageStatus) async -> Void
+    connectionToSourceKitLSP: LocalConnection
   ) async {
     guard let buildSystemKind else {
       return nil
@@ -109,8 +99,7 @@ package actor BuiltInBuildSystemAdapter: BuiltInBuildSystemMessageHandler, Messa
       options: options,
       buildSystemTestHooks: buildSystemTestHooks,
       toolchainRegistry: toolchainRegistry,
-      messageHandler: self,
-      reloadPackageStatusCallback: reloadPackageStatusCallback
+      connectionToSourceKitLSP: connectionToSourceKitLSP
     )
     guard let buildSystem else {
       return nil
@@ -160,6 +149,7 @@ package actor BuiltInBuildSystemAdapter: BuiltInBuildSystemMessageHandler, Messa
     id: RequestID,
     reply: @Sendable @escaping (LSPResult<R.Response>) -> Void
   ) {
+    // FIXME: Can we share this between the different message handler implementations?
     let signposter = Logger(subsystem: LoggingScope.subsystem, category: "build-system-message-handling")
       .makeSignposter()
     let signpostID = signposter.makeSignpostID()
@@ -227,12 +217,4 @@ package actor BuiltInBuildSystemAdapter: BuiltInBuildSystemMessageHandler, Messa
       await request.reply { throw ResponseError.methodNotFound(Request.method) }
     }
   }
-
-  package func sendNotificationToSourceKitLSP(_ notification: some LanguageServerProtocol.NotificationType) async {
-    connectionToSourceKitLSP.send(notification)
-  }
-
-  package func sendRequestToSourceKitLSP<R: RequestType>(_ request: R) async throws -> R.Response {
-    return try await connectionToSourceKitLSP.send(request)
-  }
 }

Sources/BuildSystemIntegration/CompilationDatabaseBuildSystem.swift

@@ -38,10 +38,7 @@ package actor CompilationDatabaseBuildSystem {
     }
   }
 
-  /// Callbacks that should be called if the list of possible test files has changed.
-  package var testFilesDidChangeCallbacks: [() async -> Void] = []
-
-  package weak var messageHandler: BuiltInBuildSystemMessageHandler?
+  package let connectionToSourceKitLSP: any Connection
 
   package let projectRoot: AbsolutePath
 
@@ -72,13 +69,13 @@ package actor CompilationDatabaseBuildSystem {
   package init?(
     projectRoot: AbsolutePath,
     searchPaths: [RelativePath],
-    messageHandler: (any BuiltInBuildSystemMessageHandler)?,
+    connectionToSourceKitLSP: any Connection,
     fileSystem: FileSystem = localFileSystem
   ) {
     self.fileSystem = fileSystem
     self.projectRoot = projectRoot
     self.searchPaths = searchPaths
-    self.messageHandler = messageHandler
+    self.connectionToSourceKitLSP = connectionToSourceKitLSP
     if let compdb = tryLoadCompilationDatabase(directory: projectRoot, additionalSearchPaths: searchPaths, fileSystem) {
       self.compdb = compdb
     } else {
@@ -129,9 +126,9 @@ extension CompilationDatabaseBuildSystem: BuiltInBuildSystem {
     return BuildTargetSourcesResponse(items: [SourcesItem(target: .dummy, sources: sources)])
   }
 
-  package func didChangeWatchedFiles(notification: BuildServerProtocol.DidChangeWatchedFilesNotification) async {
+  package func didChangeWatchedFiles(notification: BuildServerProtocol.DidChangeWatchedFilesNotification) {
     if notification.changes.contains(where: { self.fileEventShouldTriggerCompilationDatabaseReload(event: $0) }) {
-      await self.reloadCompilationDatabase()
+      self.reloadCompilationDatabase()
     }
   }
 
@@ -194,16 +191,13 @@ extension CompilationDatabaseBuildSystem: BuiltInBuildSystem {
 
   /// The compilation database has been changed on disk.
   /// Reload it and notify the delegate about build setting changes.
-  private func reloadCompilationDatabase() async {
+  private func reloadCompilationDatabase() {
     self.compdb = tryLoadCompilationDatabase(
       directory: projectRoot,
       additionalSearchPaths: searchPaths,
       self.fileSystem
     )
 
-    await messageHandler?.sendNotificationToSourceKitLSP(DidChangeBuildTargetNotification(changes: nil))
-    for testFilesDidChangeCallback in testFilesDidChangeCallbacks {
-      await testFilesDidChangeCallback()
-    }
+    connectionToSourceKitLSP.send(DidChangeBuildTargetNotification(changes: nil))
   }
 }