From 3a118980b4152e84ef503a746f7ff54e2b3ef1da Mon Sep 17 00:00:00 2001 From: Alex Hoppen Date: Sat, 24 Aug 2024 14:06:24 -0700 Subject: [PATCH] Migrate `BuildSystem.prepare` to a BSP request --- Sources/BuildServerProtocol/CMakeLists.txt | 2 + .../LogMessageNotification.swift | 70 +++++++++++++++++++ .../PrepareTargetsRequest.swift | 30 ++++++++ .../RegisterForChangeNotifications.swift | 1 + .../BuildServerBuildSystem.swift | 5 +- .../BuildSystemDelegate.swift | 3 + .../BuildSystemManager.swift | 14 +++- .../BuiltInBuildSystem.swift | 5 +- .../BuiltInBuildSystemAdapter.swift | 2 + .../CompilationDatabaseBuildSystem.swift | 5 +- .../BuildSystemIntegration/IndexTaskID.swift | 19 +++++ .../SwiftPMBuildSystem.swift | 34 +++++---- .../TestBuildSystem.swift | 5 +- Sources/SourceKitLSP/SourceKitLSPServer.swift | 2 +- Sources/SourceKitLSP/Workspace.swift | 8 +++ .../BuildSystemManagerTests.swift | 2 + 16 files changed, 176 insertions(+), 31 deletions(-) create mode 100644 Sources/BuildServerProtocol/LogMessageNotification.swift create mode 100644 Sources/BuildServerProtocol/PrepareTargetsRequest.swift diff --git a/Sources/BuildServerProtocol/CMakeLists.txt b/Sources/BuildServerProtocol/CMakeLists.txt index ae84ee3b..adbdb832 100644 --- a/Sources/BuildServerProtocol/CMakeLists.txt +++ b/Sources/BuildServerProtocol/CMakeLists.txt @@ -4,7 +4,9 @@ add_library(BuildServerProtocol STATIC DidChangeWatchedFilesNotification.swift InitializeBuild.swift InverseSourcesRequest.swift + LogMessageNotification.swift Messages.swift + PrepareTargetsRequest.swift RegisterForChangeNotifications.swift ShutdownBuild.swift SourceKitOptionsRequest.swift) diff --git a/Sources/BuildServerProtocol/LogMessageNotification.swift b/Sources/BuildServerProtocol/LogMessageNotification.swift new file mode 100644 index 00000000..37fca2cc --- /dev/null +++ b/Sources/BuildServerProtocol/LogMessageNotification.swift @@ -0,0 +1,70 @@ +//===----------------------------------------------------------------------===// +// +// 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 + +public enum LogMessageType: Int, Sendable, Codable { + /// An error message. + case error = 1 + + /// A warning message. + case warning = 2 + + /// An information message. + case info = 3 + + /// A log message. + case log = 4 +} + +public typealias TaskIdentifier = String + +public struct TaskId: Sendable, Codable { + /// A unique identifier + public var id: TaskIdentifier + + /// The parent task ids, if any. A non-empty parents field means + /// this task is a sub-task of every parent task id. The child-parent + /// relationship of tasks makes it possible to render tasks in + /// a tree-like user interface or inspect what caused a certain task + /// execution. + /// OriginId should not be included in the parents field, there is a separate + /// field for that. + public var parents: [TaskIdentifier]? + + public init(id: TaskIdentifier, parents: [TaskIdentifier]? = nil) { + self.id = id + self.parents = parents + } +} + +/// The log message notification is sent from a server to a client to ask the client to log a particular message in its console. +/// +/// A `build/logMessage`` notification is similar to LSP's `window/logMessage``, except for a few additions like id and originId. +public struct LogMessageNotification: NotificationType { + public static let method: String = "build/logMessage" + + /// The message type. + public var type: LogMessageType + + /// The task id if any. + public var task: TaskId? + + /// The actual message. + public var message: String + + public init(type: LogMessageType, task: TaskId? = nil, message: String) { + self.type = type + self.task = task + self.message = message + } +} diff --git a/Sources/BuildServerProtocol/PrepareTargetsRequest.swift b/Sources/BuildServerProtocol/PrepareTargetsRequest.swift new file mode 100644 index 00000000..6d0bf573 --- /dev/null +++ b/Sources/BuildServerProtocol/PrepareTargetsRequest.swift @@ -0,0 +1,30 @@ +//===----------------------------------------------------------------------===// +// +// 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 + +public typealias OriginId = String + +public struct PrepareTargetsRequest: RequestType, Hashable { + public static let method: String = "buildTarget/prepare" + public typealias Response = VoidResponse + + /// A list of build targets to prepare. + public var targets: [BuildTargetIdentifier] + + public var originId: OriginId? + + public init(targets: [BuildTargetIdentifier], originId: OriginId? = nil) { + self.targets = targets + self.originId = originId + } +} diff --git a/Sources/BuildServerProtocol/RegisterForChangeNotifications.swift b/Sources/BuildServerProtocol/RegisterForChangeNotifications.swift index 0c1d2aaf..49b91696 100644 --- a/Sources/BuildServerProtocol/RegisterForChangeNotifications.swift +++ b/Sources/BuildServerProtocol/RegisterForChangeNotifications.swift @@ -9,6 +9,7 @@ // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors // //===----------------------------------------------------------------------===// + import LanguageServerProtocol /// The register for changes request is sent from the language diff --git a/Sources/BuildSystemIntegration/BuildServerBuildSystem.swift b/Sources/BuildSystemIntegration/BuildServerBuildSystem.swift index 1148caaf..6b00d82c 100644 --- a/Sources/BuildSystemIntegration/BuildServerBuildSystem.swift +++ b/Sources/BuildSystemIntegration/BuildServerBuildSystem.swift @@ -329,10 +329,7 @@ extension BuildServerBuildSystem: BuiltInBuildSystem { return nil } - package func prepare( - targets: [BuildTargetIdentifier], - logMessageToIndexLog: @Sendable (_ taskID: IndexTaskID, _ message: String) -> Void - ) async throws { + package func prepare(request: PrepareTargetsRequest) async throws -> VoidResponse { throw PrepareNotSupportedError() } diff --git a/Sources/BuildSystemIntegration/BuildSystemDelegate.swift b/Sources/BuildSystemIntegration/BuildSystemDelegate.swift index eb477ff2..9726cdaf 100644 --- a/Sources/BuildSystemIntegration/BuildSystemDelegate.swift +++ b/Sources/BuildSystemIntegration/BuildSystemDelegate.swift @@ -40,4 +40,7 @@ package protocol BuildSystemManagerDelegate: AnyObject, Sendable { /// Notify the delegate that some information about the given build targets has changed and that it should recompute /// any information based on top of it. func buildTargetsChanged(_ changes: [BuildTargetEvent]?) async + + /// Log the given message to the client's index log. + func logMessageToIndexLog(taskID: IndexTaskID, message: String) } diff --git a/Sources/BuildSystemIntegration/BuildSystemManager.swift b/Sources/BuildSystemIntegration/BuildSystemManager.swift index dd48b4ba..5827e664 100644 --- a/Sources/BuildSystemIntegration/BuildSystemManager.swift +++ b/Sources/BuildSystemIntegration/BuildSystemManager.swift @@ -146,6 +146,8 @@ package actor BuildSystemManager: BuiltInBuildSystemAdapterDelegate { switch notification { case let notification as DidChangeBuildTargetNotification: await self.didChangeBuildTarget(notification: notification) + case let notification as BuildServerProtocol.LogMessageNotification: + await self.logMessage(notification: notification) default: logger.error("Ignoring unknown notification \(type(of: notification).method)") } @@ -369,7 +371,7 @@ package actor BuildSystemManager: BuiltInBuildSystemAdapterDelegate { targets: [BuildTargetIdentifier], logMessageToIndexLog: @escaping @Sendable (_ taskID: IndexTaskID, _ message: String) -> Void ) async throws { - try await buildSystem?.underlyingBuildSystem.prepare(targets: targets, logMessageToIndexLog: logMessageToIndexLog) + let _: VoidResponse? = try await buildSystem?.send(PrepareTargetsRequest(targets: targets)) } package func registerForChangeNotifications(for uri: DocumentURI, language: Language) async { @@ -449,6 +451,16 @@ extension BuildSystemManager: BuildSystemDelegate { // it responsible for figuring out which files are affected. await delegate?.fileBuildSettingsChanged(Set(watchedFiles.keys)) } + + private func logMessage(notification: BuildServerProtocol.LogMessageNotification) async { + // FIXME: (BSP Integration) Remove the restriction that task IDs need to have a raw value that can be parsed by + // `IndexTaskID.init`. + guard let task = notification.task, let taskID = IndexTaskID(rawValue: task.id) else { + logger.error("Ignoring log message notification with unknown task \(notification.task?.id ?? "")") + return + } + delegate?.logMessageToIndexLog(taskID: taskID, message: notification.message) + } } extension BuildSystemManager { diff --git a/Sources/BuildSystemIntegration/BuiltInBuildSystem.swift b/Sources/BuildSystemIntegration/BuiltInBuildSystem.swift index dbb6e4b0..05f98345 100644 --- a/Sources/BuildSystemIntegration/BuiltInBuildSystem.swift +++ b/Sources/BuildSystemIntegration/BuiltInBuildSystem.swift @@ -128,10 +128,7 @@ package protocol BuiltInBuildSystem: AnyObject, Sendable { /// Prepare the given targets for indexing and semantic functionality. This should build all swift modules of target /// dependencies. - func prepare( - targets: [BuildTargetIdentifier], - logMessageToIndexLog: @escaping @Sendable (_ taskID: IndexTaskID, _ message: String) -> Void - ) async throws + func prepare(request: PrepareTargetsRequest) async throws -> VoidResponse /// If the build system has knowledge about the language that this document should be compiled in, return it. /// diff --git a/Sources/BuildSystemIntegration/BuiltInBuildSystemAdapter.swift b/Sources/BuildSystemIntegration/BuiltInBuildSystemAdapter.swift index f74c976d..006f471d 100644 --- a/Sources/BuildSystemIntegration/BuiltInBuildSystemAdapter.swift +++ b/Sources/BuildSystemIntegration/BuiltInBuildSystemAdapter.swift @@ -140,6 +140,8 @@ package actor BuiltInBuildSystemAdapter: BuiltInBuildSystemMessageHandler { switch request { case let request as InverseSourcesRequest: return try await handle(request, underlyingBuildSystem.inverseSources) + case let request as PrepareTargetsRequest: + return try await handle(request, underlyingBuildSystem.prepare) case let request as SourceKitOptionsRequest: return try await handle(request, underlyingBuildSystem.sourceKitOptions) default: diff --git a/Sources/BuildSystemIntegration/CompilationDatabaseBuildSystem.swift b/Sources/BuildSystemIntegration/CompilationDatabaseBuildSystem.swift index 022b54f1..55375ef6 100644 --- a/Sources/BuildSystemIntegration/CompilationDatabaseBuildSystem.swift +++ b/Sources/BuildSystemIntegration/CompilationDatabaseBuildSystem.swift @@ -130,10 +130,7 @@ extension CompilationDatabaseBuildSystem: BuiltInBuildSystem { return InverseSourcesResponse(targets: [BuildTargetIdentifier.dummy]) } - package func prepare( - targets: [BuildTargetIdentifier], - logMessageToIndexLog: @Sendable (_ taskID: IndexTaskID, _ message: String) -> Void - ) async throws { + package func prepare(request: PrepareTargetsRequest) async throws -> VoidResponse { throw PrepareNotSupportedError() } diff --git a/Sources/BuildSystemIntegration/IndexTaskID.swift b/Sources/BuildSystemIntegration/IndexTaskID.swift index 3d01f706..d4795e22 100644 --- a/Sources/BuildSystemIntegration/IndexTaskID.swift +++ b/Sources/BuildSystemIntegration/IndexTaskID.swift @@ -44,4 +44,23 @@ package enum IndexTaskID: Sendable { return Self.numberToEmojis((id * 2 + 1).hashValue, numEmojis: numEmojis) } } + + package var rawValue: String { + switch self { + case .preparation(id: let id): return "preparation-\(id)" + case .updateIndexStore(id: let id): return "updateIndexStore-\(id)" + } + } + + package init?(rawValue: String) { + let split = rawValue.split(separator: "-", maxSplits: 2) + guard split.count == 2, let id = UInt32(split[1]) else { + return nil + } + switch split[0] { + case "preparation": self = .preparation(id: id) + case "updateIndexStore": self = .updateIndexStore(id: id) + default: return nil + } + } } diff --git a/Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift b/Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift index 271bdba7..9ba4515c 100644 --- a/Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift +++ b/Sources/BuildSystemIntegration/SwiftPMBuildSystem.swift @@ -655,22 +655,30 @@ extension SwiftPMBuildSystem: BuildSystemIntegration.BuiltInBuildSystem { } } - package func prepare( - targets: [BuildTargetIdentifier], - logMessageToIndexLog: @escaping @Sendable (_ taskID: IndexTaskID, _ message: String) -> Void - ) async throws { + 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 targets { - await orLog("Preparing") { try await prepare(singleTarget: target, logMessageToIndexLog: logMessageToIndexLog) } + for target in request.targets { + await orLog("Preparing") { try await prepare(singleTarget: target) } } - let filesInPreparedTargets = targets.flatMap { self.targets[$0]?.buildTarget.sources ?? [] } + let filesInPreparedTargets = request.targets.flatMap { self.targets[$0]?.buildTarget.sources ?? [] } await fileDependenciesUpdatedDebouncer.scheduleCall(Set(filesInPreparedTargets.map(DocumentURI.init))) + return VoidResponse() } - private func prepare( - singleTarget target: BuildTargetIdentifier, - logMessageToIndexLog: @escaping @Sendable (_ taskID: IndexTaskID, _ message: String) -> Void - ) async throws { + private nonisolated func logMessageToIndexLog(_ taskID: IndexTaskID, _ message: String) { + // FIXME: When `messageHandler` is a Connection, we don't need to go via Task anymore + Task { + await self.messageHandler?.sendNotificationToSourceKitLSP( + BuildServerProtocol.LogMessageNotification( + type: .info, + task: TaskId(id: taskID.rawValue), + message: message + ) + ) + } + } + + private func prepare(singleTarget target: BuildTargetIdentifier) async throws { if target == .forPackageManifest { // Nothing to prepare for package manifests. return @@ -720,8 +728,8 @@ extension SwiftPMBuildSystem: BuildSystemIntegration.BuiltInBuildSystem { \(arguments.joined(separator: " ")) """ ) - let stdoutHandler = PipeAsStringHandler { logMessageToIndexLog(logID, $0) } - let stderrHandler = PipeAsStringHandler { logMessageToIndexLog(logID, $0) } + let stdoutHandler = PipeAsStringHandler { self.logMessageToIndexLog(logID, $0) } + let stderrHandler = PipeAsStringHandler { self.logMessageToIndexLog(logID, $0) } let result = try await Process.run( arguments: arguments, diff --git a/Sources/BuildSystemIntegration/TestBuildSystem.swift b/Sources/BuildSystemIntegration/TestBuildSystem.swift index e93ff61b..bfc9afba 100644 --- a/Sources/BuildSystemIntegration/TestBuildSystem.swift +++ b/Sources/BuildSystemIntegration/TestBuildSystem.swift @@ -70,10 +70,7 @@ package actor TestBuildSystem: BuiltInBuildSystem { return InverseSourcesResponse(targets: [BuildTargetIdentifier.dummy]) } - package func prepare( - targets: [BuildTargetIdentifier], - logMessageToIndexLog: @escaping @Sendable (_ taskID: IndexTaskID, _ message: String) -> Void - ) async throws { + package func prepare(request: PrepareTargetsRequest) async throws -> VoidResponse { throw PrepareNotSupportedError() } diff --git a/Sources/SourceKitLSP/SourceKitLSPServer.swift b/Sources/SourceKitLSP/SourceKitLSPServer.swift index 078f74d8..93d2d9bf 100644 --- a/Sources/SourceKitLSP/SourceKitLSPServer.swift +++ b/Sources/SourceKitLSP/SourceKitLSPServer.swift @@ -832,7 +832,7 @@ extension SourceKitLSPServer: MessageHandler { } extension SourceKitLSPServer { - nonisolated func logMessageToIndexLog(taskID: IndexTaskID, message: String) { + nonisolated package func logMessageToIndexLog(taskID: IndexTaskID, message: String) { var message: Substring = message[...] while message.last?.isNewline ?? false { message = message.dropLast(1) diff --git a/Sources/SourceKitLSP/Workspace.swift b/Sources/SourceKitLSP/Workspace.swift index 2202ae19..296517bf 100644 --- a/Sources/SourceKitLSP/Workspace.swift +++ b/Sources/SourceKitLSP/Workspace.swift @@ -83,6 +83,9 @@ package final class Workspace: Sendable, BuildSystemManagerDelegate { /// `nil` if background indexing is not enabled. let semanticIndexManager: SemanticIndexManager? + /// A callback that should be called when the build system wants to log a message to the index log. + private let logMessageToIndexLogCallback: @Sendable (_ taskID: IndexTaskID, _ message: String) -> Void + /// A callback that should be called when the file handling capability (ie. the presence of a target for a source /// files) of this workspace changes. private let fileHandlingCapabilityChangedCallback: @Sendable () async -> Void @@ -106,6 +109,7 @@ package final class Workspace: Sendable, BuildSystemManagerDelegate { self.options = options self._uncheckedIndex = ThreadSafeBox(initialValue: uncheckedIndex) self.buildSystemManager = buildSystemManager + self.logMessageToIndexLogCallback = logMessageToIndexLog self.fileHandlingCapabilityChangedCallback = fileHandlingCapabilityChanged if options.backgroundIndexingOrDefault, let uncheckedIndex, await buildSystemManager.supportsPreparation { self.semanticIndexManager = SemanticIndexManager( @@ -321,6 +325,10 @@ package final class Workspace: Sendable, BuildSystemManagerDelegate { package func buildTargetsChanged(_ changes: [BuildTargetEvent]?) async { await self.fileHandlingCapabilityChangedCallback() } + + package func logMessageToIndexLog(taskID: IndexTaskID, message: String) { + self.logMessageToIndexLogCallback(taskID, message) + } } /// Wrapper around a workspace that isn't being retained. diff --git a/Tests/BuildSystemIntegrationTests/BuildSystemManagerTests.swift b/Tests/BuildSystemIntegrationTests/BuildSystemManagerTests.swift index 1da74e4d..bfe91c29 100644 --- a/Tests/BuildSystemIntegrationTests/BuildSystemManagerTests.swift +++ b/Tests/BuildSystemIntegrationTests/BuildSystemManagerTests.swift @@ -450,4 +450,6 @@ private actor BSMDelegate: BuildSystemManagerDelegate { } func buildTargetsChanged(_ changes: [BuildTargetEvent]?) async {} + + nonisolated func logMessageToIndexLog(taskID: BuildSystemIntegration.IndexTaskID, message: String) {} }