Merge branch 'main' into 6.0/merge-main-2024-05-28

# Conflicts:
#	Sources/SKSwiftPMWorkspace/SwiftPMBuildSystem.swift

Author: ahoppen

Package.swift

@@ -367,6 +367,7 @@ let package = Package(
       name: "SourceKitLSPTests",
       dependencies: [
         "BuildServerProtocol",
+        "CAtomics",
         "LSPLogging",
         "LSPTestSupport",
         "LanguageServerProtocol",

Sources/Diagnose/DiagnoseCommand.swift

@@ -352,6 +352,15 @@ public struct DiagnoseCommand: AsyncParsableCommand {
       """
     )
 
+    #if os(macOS)
+    // Reveal the bundle in Finder on macOS
+    do {
+      let process = try Process.launch(arguments: ["open", "-R", bundlePath.path], workingDirectory: nil)
+      try await process.waitUntilExitSendingSigIntOnTaskCancellation()
+    } catch {
+      // If revealing the bundle in Finder should fail, we don't care. We still printed the bundle path to stdout.
+    }
+    #endif
   }
 
   @MainActor

Sources/SKCore/BuildServerBuildSystem.swift

@@ -279,7 +279,7 @@ extension BuildServerBuildSystem: BuildSystem {
     return [ConfiguredTarget(targetID: "dummy", runDestinationID: "dummy")]
   }
 
-  public func generateBuildGraph() {}
+  public func generateBuildGraph(allowFileSystemWrites: Bool) {}
 
   public func topologicalSort(of targets: [ConfiguredTarget]) async -> [ConfiguredTarget]? {
     return nil

Sources/SKCore/BuildSystem.swift

@@ -135,9 +135,14 @@ 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
+  /// Re-generate the build graph.
+  ///
+  /// If `allowFileSystemWrites` is `true`, this should include all the tasks that are necessary for building the entire
+  /// build graph, like resolving package versions.
+  ///
+  /// If `allowFileSystemWrites` is `false`, no files must be written to disk. This mode is used to determine whether
+  /// the build system can handle a source file, and decide whether a workspace should be opened with this build system
+  func generateBuildGraph(allowFileSystemWrites: Bool) async throws
 
   /// Sort the targets so that low-level targets occur before high-level targets.
   ///

Sources/SKCore/BuildSystemManager.swift

@@ -219,8 +219,8 @@ extension BuildSystemManager {
     return settings
   }
 
-  public func generateBuildGraph() async throws {
-    try await self.buildSystem?.generateBuildGraph()
+  public func generateBuildGraph(allowFileSystemWrites: Bool) async throws {
+    try await self.buildSystem?.generateBuildGraph(allowFileSystemWrites: allowFileSystemWrites)
   }
 
   public func topologicalSort(of targets: [ConfiguredTarget]) async throws -> [ConfiguredTarget]? {

Sources/SKCore/CompilationDatabaseBuildSystem.swift

@@ -132,7 +132,7 @@ extension CompilationDatabaseBuildSystem: BuildSystem {
     throw PrepareNotSupportedError()
   }
 
-  public func generateBuildGraph() {}
+  public func generateBuildGraph(allowFileSystemWrites: Bool) {}
 
   public func topologicalSort(of targets: [ConfiguredTarget]) -> [ConfiguredTarget]? {
     return nil

Sources/SKCore/TaskScheduler.swift

@@ -13,13 +13,16 @@
 import CAtomics
 import Foundation
 import LSPLogging
+import SKSupport
 
 /// See comment on ``TaskDescriptionProtocol/dependencies(to:taskPriority:)``
 public enum TaskDependencyAction<TaskDescription: TaskDescriptionProtocol> {
   case waitAndElevatePriorityOfDependency(TaskDescription)
   case cancelAndRescheduleDependency(TaskDescription)
 }
 
+private let taskSchedulerSubsystem = "org.swift.sourcekit-lsp.task-scheduler"
+
 public protocol TaskDescriptionProtocol: Identifiable, Sendable, CustomLogStringConvertible {
   /// Execute the task.
   ///
@@ -123,10 +126,6 @@ public actor QueuedTask<TaskDescription: TaskDescriptionProtocol> {
   /// Every time `execute` gets called, a new task is placed in this continuation. See comment on `executionTask`.
   private let executionTaskCreatedContinuation: AsyncStream<Task<ExecutionTaskFinishStatus, Never>>.Continuation
 
-  /// Placing a new value in this continuation will cause `resultTask` to query its priority and set
-  /// `QueuedTask.priority`.
-  private let updatePriorityContinuation: AsyncStream<Void>.Continuation
-
   nonisolated(unsafe) private var _priority: AtomicUInt8
 
   /// The latest known priority of the task.
@@ -183,20 +182,14 @@ public actor QueuedTask<TaskDescription: TaskDescriptionProtocol> {
   private let executionStateChangedCallback: (@Sendable (QueuedTask, TaskExecutionState) async -> Void)?
 
   init(
-    priority: TaskPriority? = nil,
+    priority: TaskPriority,
     description: TaskDescription,
     executionStateChangedCallback: (@Sendable (QueuedTask, TaskExecutionState) async -> Void)?
   ) async {
-    self._priority = .init(initialValue: priority?.rawValue ?? Task.currentPriority.rawValue)
+    self._priority = AtomicUInt8(initialValue: priority.rawValue)
     self.description = description
     self.executionStateChangedCallback = executionStateChangedCallback
 
-    var updatePriorityContinuation: AsyncStream<Void>.Continuation!
-    let updatePriorityStream = AsyncStream {
-      updatePriorityContinuation = $0
-    }
-    self.updatePriorityContinuation = updatePriorityContinuation
-
     var executionTaskCreatedContinuation: AsyncStream<Task<ExecutionTaskFinishStatus, Never>>.Continuation!
     let executionTaskCreatedStream = AsyncStream {
       executionTaskCreatedContinuation = $0
@@ -205,31 +198,24 @@ public actor QueuedTask<TaskDescription: TaskDescriptionProtocol> {
 
     self.resultTask = Task.detached(priority: priority) {
       await withTaskCancellationHandler {
-        await withTaskGroup(of: Void.self) { taskGroup in
-          taskGroup.addTask {
-            for await _ in updatePriorityStream {
-              self.priority = Task.currentPriority
-            }
-          }
-          taskGroup.addTask {
-            for await task in executionTaskCreatedStream {
-              switch await task.valuePropagatingCancellation {
-              case .cancelledToBeRescheduled:
-                // Break the switch and wait for a new `executionTask` to be placed into `executionTaskCreatedStream`.
-                break
-              case .terminated:
-                // The task finished. We are done with this `QueuedTask`
-                return
-              }
+        await withTaskPriorityChangedHandler(initialPriority: self.priority) {
+          for await task in executionTaskCreatedStream {
+            switch await task.valuePropagatingCancellation {
+            case .cancelledToBeRescheduled:
+              // Break the switch and wait for a new `executionTask` to be placed into `executionTaskCreatedStream`.
+              break
+            case .terminated:
+              // The task finished. We are done with this `QueuedTask`
+              return
             }
           }
-          // The first (update priority) task never finishes, so this waits for the second (wait for execution) task
-          // to terminate.
-          // Afterwards we also cancel the update priority task.
-          for await _ in taskGroup {
-            taskGroup.cancelAll()
-            return
+        } taskPriorityChanged: {
+          withLoggingSubsystemAndScope(subsystem: taskSchedulerSubsystem, scope: nil) {
+            logger.debug(
+              "Updating priority of \(self.description.forLogging) from \(self.priority.rawValue) to \(Task.currentPriority.rawValue)"
+            )
           }
+          self.priority = Task.currentPriority
         }
       } onCancel: {
         self.resultTaskCancelled.value = true
@@ -282,20 +268,19 @@ public actor QueuedTask<TaskDescription: TaskDescriptionProtocol> {
     self.executionTask = nil
   }
 
-  /// Trigger `QueuedTask.priority` to be updated with the current priority of the underlying task.
-  ///
-  /// This is an asynchronous operation that makes no guarantees when the updated priority will be available.
-  ///
-  /// This is needed because tasks can't subscribe to priority updates (ie. there is no `withPriorityHandler` similar to
-  /// `withCancellationHandler`, https://github.com/apple/swift/issues/73367).
-  func triggerPriorityUpdate() {
-    updatePriorityContinuation.yield()
-  }
-
   /// If the priority of this task is less than `targetPriority`, elevate the priority to `targetPriority` by spawning
   /// a new task that depends on it. Otherwise a no-op.
   nonisolated func elevatePriority(to targetPriority: TaskPriority) {
     if priority < targetPriority {
+      withLoggingSubsystemAndScope(subsystem: taskSchedulerSubsystem, scope: nil) {
+        logger.debug(
+          "Elevating priority of \(self.description.forLogging) from \(self.priority.rawValue) to \(targetPriority.rawValue)"
+        )
+      }
+      // Awaiting the result task from a higher-priority task will eventually update `priority` through
+      // `withTaskPriorityChangedHandler` but that might take a while because `withTaskPriorityChangedHandler` polls.
+      // Since we know that the priority will be elevated, set it now. That way we don't try to elevate it again.
+      self.priority = targetPriority
       Task(priority: targetPriority) {
         await self.resultTask.value
       }
@@ -371,7 +356,7 @@ public actor TaskScheduler<TaskDescription: TaskDescriptionProtocol> {
     )? = nil
   ) async -> QueuedTask<TaskDescription> {
     let queuedTask = await QueuedTask(
-      priority: priority,
+      priority: priority ?? Task.currentPriority,
       description: taskDescription,
       executionStateChangedCallback: executionStateChangedCallback
     )
@@ -385,16 +370,6 @@ public actor TaskScheduler<TaskDescription: TaskDescriptionProtocol> {
     return queuedTask
   }
 
-  /// Trigger all queued tasks to update their priority.
-  ///
-  /// Should be called occasionally to elevate tasks in the queue whose underlying `Swift.Task` had their priority
-  /// elevated because a higher-priority task started depending on them.
-  private func triggerPriorityUpdateOfQueuedTasks() async {
-    for task in pendingTasks {
-      await task.triggerPriorityUpdate()
-    }
-  }
-
   /// Returns the maximum number of concurrent tasks that are allowed to execute at the given priority.
   private func maxConcurrentTasks(at priority: TaskPriority) -> Int {
     for (atPriority, maxConcurrentTasks) in maxConcurrentTasksByPriority {
@@ -417,9 +392,8 @@ public actor TaskScheduler<TaskDescription: TaskDescriptionProtocol> {
       {
         // We don't have any execution slots left. Thus, this poker has nothing to do and is done.
         // When the next task finishes, it calls `poke` again.
-        // If the low priority task's priority gets elevated, that will be picked up when the next task in the
-        // `TaskScheduler` finishes, which causes  `triggerPriorityUpdateOfQueuedTasks` to be called, which transfers
-        // the new elevated priority to `QueuedTask.priority` and which can then be picked up by the next `poke` call.
+        // If the low priority task's priority gets elevated that task's priority will get elevated and it will be
+        // picked up on the next `poke` call.
         return
       }
       let dependencies = task.description.dependencies(to: currentlyExecutingTasks.map(\.description))
@@ -428,13 +402,17 @@ public actor TaskScheduler<TaskDescription: TaskDescriptionProtocol> {
         case .cancelAndRescheduleDependency(let taskDescription):
           guard let dependency = self.currentlyExecutingTasks.first(where: { $0.description.id == taskDescription.id })
           else {
-            logger.fault(
-              "Cannot find task to wait for \(taskDescription.forLogging) in list of currently executing tasks"
-            )
+            withLoggingSubsystemAndScope(subsystem: taskSchedulerSubsystem, scope: nil) {
+              logger.fault(
+                "Cannot find task to wait for \(taskDescription.forLogging) in list of currently executing tasks"
+              )
+            }
             return nil
           }
           if !taskDescription.isIdempotent {
-            logger.fault("Cannot reschedule task '\(taskDescription.forLogging)' since it is not idempotent")
+            withLoggingSubsystemAndScope(subsystem: taskSchedulerSubsystem, scope: nil) {
+              logger.fault("Cannot reschedule task '\(taskDescription.forLogging)' since it is not idempotent")
+            }
             return dependency
           }
           if dependency.priority > task.priority {
@@ -445,9 +423,11 @@ public actor TaskScheduler<TaskDescription: TaskDescriptionProtocol> {
         case .waitAndElevatePriorityOfDependency(let taskDescription):
           guard let dependency = self.currentlyExecutingTasks.first(where: { $0.description.id == taskDescription.id })
           else {
-            logger.fault(
-              "Cannot find task to wait for '\(taskDescription.forLogging)' in list of currently executing tasks"
-            )
+            withLoggingSubsystemAndScope(subsystem: taskSchedulerSubsystem, scope: nil) {
+              logger.fault(
+                "Cannot find task to wait for '\(taskDescription.forLogging)' in list of currently executing tasks"
+              )
+            }
             return nil
           }
           return dependency
@@ -465,9 +445,11 @@ public actor TaskScheduler<TaskDescription: TaskDescriptionProtocol> {
         switch taskDependency {
         case .cancelAndRescheduleDependency(let taskDescription):
           guard let task = self.currentlyExecutingTasks.first(where: { $0.description.id == taskDescription.id }) else {
-            logger.fault(
-              "Cannot find task to reschedule \(taskDescription.forLogging) in list of currently executing tasks"
-            )
+            withLoggingSubsystemAndScope(subsystem: taskSchedulerSubsystem, scope: nil) {
+              logger.fault(
+                "Cannot find task to reschedule \(taskDescription.forLogging) in list of currently executing tasks"
+              )
+            }
             return nil
           }
           return task
@@ -478,6 +460,9 @@ public actor TaskScheduler<TaskDescription: TaskDescriptionProtocol> {
       if !rescheduleTasks.isEmpty {
         Task.detached(priority: task.priority) {
           for task in rescheduleTasks {
+            withLoggingSubsystemAndScope(subsystem: taskSchedulerSubsystem, scope: nil) {
+              logger.debug("Suspending \(task.description.forLogging)")
+            }
             await task.cancelToBeRescheduled()
           }
         }
@@ -510,7 +495,6 @@ public actor TaskScheduler<TaskDescription: TaskDescriptionProtocol> {
     case .terminated: break
     case .cancelledToBeRescheduled: pendingTasks.append(task)
     }
-    await self.triggerPriorityUpdateOfQueuedTasks()
     self.poke()
   }
 }

Sources/SKSupport/AsyncUtils.swift

@@ -166,3 +166,28 @@ extension Collection where Element: Sendable {
     }
   }
 }
+
+public struct TimeoutError: Error, CustomStringConvertible {
+  public var description: String { "Timed out" }
+}
+
+/// Executes `body`. If it doesn't finish after `duration`, throws a `TimeoutError`.
+public func withTimeout<T: Sendable>(
+  _ duration: Duration,
+  _ body: @escaping @Sendable () async throws -> T
+) async throws -> T {
+  try await withThrowingTaskGroup(of: T.self) { taskGroup in
+    taskGroup.addTask {
+      try await Task.sleep(for: duration)
+      throw TimeoutError()
+    }
+    taskGroup.addTask {
+      return try await body()
+    }
+    for try await value in taskGroup {
+      taskGroup.cancelAll()
+      return value
+    }
+    throw CancellationError()
+  }
+}

Sources/SKSupport/CMakeLists.txt

@@ -17,6 +17,7 @@ add_library(SKSupport STATIC
   Result.swift
   Sequence+AsyncMap.swift
   SwitchableProcessResultExitStatus.swift
+  Task+WithPriorityChangedHandler.swift
   ThreadSafeBox.swift
   WorkspaceType.swift
 )