Merge pull request #1366 from ahoppen/fix-modules-include-directory

Support running tests that require building with a Swift 5.10 toolchain

Author: ahoppen

Sources/SKCore/Toolchain.swift

@@ -12,13 +12,51 @@
 
 import LSPLogging
 import LanguageServerProtocol
+import RegexBuilder
 import SKSupport
 
 import enum PackageLoading.Platform
 import struct TSCBasic.AbsolutePath
 import protocol TSCBasic.FileSystem
+import class TSCBasic.Process
 import var TSCBasic.localFileSystem
 
+/// A Swift version consisting of the major and minor component.
+public struct SwiftVersion: Sendable, Comparable, CustomStringConvertible {
+  public let major: Int
+  public let minor: Int
+
+  public static func < (lhs: SwiftVersion, rhs: SwiftVersion) -> Bool {
+    return (lhs.major, lhs.minor) < (rhs.major, rhs.minor)
+  }
+
+  public init(_ major: Int, _ minor: Int) {
+    self.major = major
+    self.minor = minor
+  }
+
+  public var description: String {
+    return "\(major).\(minor)"
+  }
+}
+
+fileprivate enum SwiftVersionParsingError: Error, CustomStringConvertible {
+  case failedToFindSwiftc
+  case failedToParseOutput(output: String?)
+
+  var description: String {
+    switch self {
+    case .failedToFindSwiftc:
+      return "Default toolchain does not contain a swiftc executable"
+    case .failedToParseOutput(let output):
+      return """
+        Failed to parse Swift version. Output of swift --version:
+        \(output ?? "<empty>")
+        """
+    }
+  }
+}
+
 /// A Toolchain is a collection of related compilers and libraries meant to be used together to
 /// build and edit source code.
 ///
@@ -63,6 +101,47 @@ public final class Toolchain: Sendable {
   /// The path to the indexstore library if available.
   public let libIndexStore: AbsolutePath?
 
+  private let swiftVersionTask = ThreadSafeBox<Task<SwiftVersion, any Error>?>(initialValue: nil)
+
+  /// The Swift version installed in the toolchain. Throws an error if the version could not be parsed or if no Swift
+  /// compiler is installed in the toolchain.
+  public var swiftVersion: SwiftVersion {
+    get async throws {
+      let task = swiftVersionTask.withLock { task in
+        if let task {
+          return task
+        }
+        let newTask = Task { () -> SwiftVersion in
+          guard let swiftc else {
+            throw SwiftVersionParsingError.failedToFindSwiftc
+          }
+
+          let process = Process(args: swiftc.pathString, "--version")
+          try process.launch()
+          let result = try await process.waitUntilExit()
+          let output = String(bytes: try result.output.get(), encoding: .utf8)
+          let regex = Regex {
+            "Swift version "
+            Capture { OneOrMore(.digit) }
+            "."
+            Capture { OneOrMore(.digit) }
+          }
+          guard let match = output?.firstMatch(of: regex) else {
+            throw SwiftVersionParsingError.failedToParseOutput(output: output)
+          }
+          guard let major = Int(match.1), let minor = Int(match.2) else {
+            throw SwiftVersionParsingError.failedToParseOutput(output: output)
+          }
+          return SwiftVersion(major, minor)
+        }
+        task = newTask
+        return newTask
+      }
+
+      return try await task.value
+    }
+  }
+
   public init(
     identifier: String,
     displayName: String,

Sources/SKSwiftPMWorkspace/SwiftPMBuildSystem.swift

@@ -358,11 +358,30 @@ extension SwiftPMBuildSystem: SKCore.BuildSystem {
 
   public var indexPrefixMappings: [PathPrefixMapping] { return [] }
 
+  /// Return the compiler arguments for the given source file within a target, making any necessary adjustments to
+  /// account for differences in the SwiftPM versions being linked into SwiftPM and being installed in the toolchain.
+  private func compilerArguments(for file: URL, in buildTarget: any SwiftBuildTarget) async throws -> [String] {
+    let compileArguments = try buildTarget.compileArguments(for: file)
+
+    // Fix up compiler arguments that point to a `/Modules` subdirectory if the Swift version in the toolchain is less
+    // than 6.0 because it places the modules one level higher up.
+    let toolchainVersion = await orLog("Getting Swift version") { try await toolchainRegistry.default?.swiftVersion }
+    guard let toolchainVersion, toolchainVersion < SwiftVersion(6, 0) else {
+      return compileArguments
+    }
+    return compileArguments.map { argument in
+      if argument.hasSuffix("/Modules"), argument.contains(self.workspace.location.scratchDirectory.pathString) {
+        return String(argument.dropLast(8))
+      }
+      return argument
+    }
+  }
+
   public func buildSettings(
     for uri: DocumentURI,
     in configuredTarget: ConfiguredTarget,
     language: Language
-  ) throws -> FileBuildSettings? {
+  ) async throws -> FileBuildSettings? {
     guard let url = uri.fileURL, let path = try? AbsolutePath(validating: url.path) else {
       // We can't determine build settings for non-file URIs.
       return nil
@@ -388,13 +407,13 @@ extension SwiftPMBuildSystem: SKCore.BuildSystem {
       // getting its compiler arguments and then patching up the compiler arguments by replacing the substitute file
       // with the `.cpp` file.
       return FileBuildSettings(
-        compilerArguments: try buildTarget.compileArguments(for: substituteFile),
+        compilerArguments: try await compilerArguments(for: substituteFile, in: buildTarget),
         workingDirectory: workspacePath.pathString
       ).patching(newFile: try resolveSymlinks(path).pathString, originalFile: substituteFile.absoluteString)
     }
 
     return FileBuildSettings(
-      compilerArguments: try buildTarget.compileArguments(for: url),
+      compilerArguments: try await compilerArguments(for: url, in: buildTarget),
       workingDirectory: workspacePath.pathString
     )
   }

Sources/SKTestSupport/SkipUnless.swift

@@ -65,11 +65,7 @@ public actor SkipUnless {
       // Never skip tests in CI. Toolchain should be up-to-date
       checkResult = .featureSupported
     } else {
-      guard let swiftc = await ToolchainRegistry.forTesting.default?.swiftc else {
-        throw SwiftVersionParsingError.failedToFindSwiftc
-      }
-
-      let toolchainSwiftVersion = try await getSwiftVersion(swiftc)
+      let toolchainSwiftVersion = try await unwrap(ToolchainRegistry.forTesting.default).swiftVersion
       let requiredSwiftVersion = SwiftVersion(swiftVersion.major, swiftVersion.minor)
       if toolchainSwiftVersion < requiredSwiftVersion {
         checkResult = .featureUnsupported(
@@ -174,10 +170,6 @@ public actor SkipUnless {
 
   /// SwiftPM moved the location where it stores Swift modules to a subdirectory in
   /// https://github.com/apple/swift-package-manager/pull/7103.
-  ///
-  /// sourcekit-lsp uses the built-in SwiftPM to synthesize compiler arguments and cross-module tests fail if the host
-  /// toolchain’s SwiftPM stores the Swift modules on the top level but we synthesize compiler arguments expecting the
-  /// modules to be in a `Modules` subdirectory.
   public static func swiftpmStoresModulesInSubdirectory(
     file: StaticString = #filePath,
     line: UInt = #line
@@ -297,60 +289,3 @@ fileprivate extension String {
     }
   }
 }
-
-/// A Swift version consisting of the major and minor component.
-fileprivate struct SwiftVersion: Comparable, CustomStringConvertible {
-  let major: Int
-  let minor: Int
-
-  static func < (lhs: SwiftVersion, rhs: SwiftVersion) -> Bool {
-    return (lhs.major, lhs.minor) < (rhs.major, rhs.minor)
-  }
-
-  init(_ major: Int, _ minor: Int) {
-    self.major = major
-    self.minor = minor
-  }
-
-  var description: String {
-    return "\(major).\(minor)"
-  }
-}
-
-fileprivate enum SwiftVersionParsingError: Error, CustomStringConvertible {
-  case failedToFindSwiftc
-  case failedToParseOutput(output: String?)
-
-  var description: String {
-    switch self {
-    case .failedToFindSwiftc:
-      return "Default toolchain does not contain a swiftc executable"
-    case .failedToParseOutput(let output):
-      return """
-        Failed to parse Swift version. Output of swift --version:
-        \(output ?? "<empty>")
-        """
-    }
-  }
-}
-
-/// Return the major and minor version of Swift for a `swiftc` compiler at `swiftcPath`.
-private func getSwiftVersion(_ swiftcPath: AbsolutePath) async throws -> SwiftVersion {
-  let process = Process(args: swiftcPath.pathString, "--version")
-  try process.launch()
-  let result = try await process.waitUntilExit()
-  let output = String(bytes: try result.output.get(), encoding: .utf8)
-  let regex = Regex {
-    "Swift version "
-    Capture { OneOrMore(.digit) }
-    "."
-    Capture { OneOrMore(.digit) }
-  }
-  guard let match = output?.firstMatch(of: regex) else {
-    throw SwiftVersionParsingError.failedToParseOutput(output: output)
-  }
-  guard let major = Int(match.1), let minor = Int(match.2) else {
-    throw SwiftVersionParsingError.failedToParseOutput(output: output)
-  }
-  return SwiftVersion(major, minor)
-}

Tests/SKSwiftPMWorkspaceTests/SwiftPMBuildSystemTests.swift

@@ -28,11 +28,11 @@ import struct PackageModel.BuildFlags
 #endif
 
 fileprivate extension SwiftPMBuildSystem {
-  func buildSettings(for uri: DocumentURI, language: Language) throws -> FileBuildSettings? {
+  func buildSettings(for uri: DocumentURI, language: Language) async throws -> FileBuildSettings? {
     guard let target = self.configuredTargets(for: uri).only else {
       return nil
     }
-    return try buildSettings(for: uri, in: target, language: language)
+    return try await buildSettings(for: uri, in: target, language: language)
   }
 }
 
@@ -116,6 +116,7 @@ final class SwiftPMBuildSystemTests: XCTestCase {
   }
 
   func testBasicSwiftArgs() async throws {
+    try await SkipUnless.swiftpmStoresModulesInSubdirectory()
     let fs = localFileSystem
     try await withTestScratchDir { tempDir in
       try fs.createFiles(

Tests/SourceKitLSPTests/BackgroundIndexingTests.swift

@@ -104,7 +104,6 @@ final class BackgroundIndexingTests: XCTestCase {
   }
 
   func testBackgroundIndexingOfMultiModuleProject() async throws {
-    try await SkipUnless.swiftpmStoresModulesInSubdirectory()
     let project = try await SwiftPMTestProject(
       files: [
         "LibA/MyFile.swift": """
@@ -212,7 +211,6 @@ final class BackgroundIndexingTests: XCTestCase {
   }
 
   func testBackgroundIndexingOfPackageDependency() async throws {
-    try await SkipUnless.swiftpmStoresModulesInSubdirectory()
     let dependencyContents = """
       public func 1️⃣doSomething() {}
       """
@@ -541,7 +539,6 @@ final class BackgroundIndexingTests: XCTestCase {
   }
 
   func testPrepareTargetAfterEditToDependency() async throws {
-    try await SkipUnless.swiftpmStoresModulesInSubdirectory()
     var serverOptions = SourceKitLSPServer.Options.testDefault
     let expectedPreparationTracker = ExpectedIndexTaskTracker(expectedPreparations: [
       [
@@ -652,7 +649,6 @@ final class BackgroundIndexingTests: XCTestCase {
     let libBStartedPreparation = self.expectation(description: "LibB started preparing")
     let libDPreparedForEditing = self.expectation(description: "LibD prepared for editing")
 
-    try await SkipUnless.swiftpmStoresModulesInSubdirectory()
     var serverOptions = SourceKitLSPServer.Options.testDefault
     let expectedPreparationTracker = ExpectedIndexTaskTracker(expectedPreparations: [
       // Preparation of targets during the initial of the target
@@ -750,8 +746,6 @@ final class BackgroundIndexingTests: XCTestCase {
   }
 
   func testIndexingHappensInParallel() async throws {
-    try await SkipUnless.swiftpmStoresModulesInSubdirectory()
-
     let fileAIndexingStarted = self.expectation(description: "FileA indexing started")
     let fileBIndexingStarted = self.expectation(description: "FileB indexing started")
 

Tests/SourceKitLSPTests/DefinitionTests.swift

@@ -440,7 +440,6 @@ class DefinitionTests: XCTestCase {
 
   func testDependentModuleGotBuilt() async throws {
     try SkipUnless.longTestsEnabled()
-    try await SkipUnless.swiftpmStoresModulesInSubdirectory()
     let project = try await SwiftPMTestProject(
       files: [
         "LibA/LibA.swift": """

Tests/SourceKitLSPTests/DependencyTrackingTests.swift

@@ -16,7 +16,6 @@ import XCTest
 
 final class DependencyTrackingTests: XCTestCase {
   func testDependenciesUpdatedSwift() async throws {
-    try await SkipUnless.swiftpmStoresModulesInSubdirectory()
     let project = try await SwiftPMTestProject(
       files: [
         "LibA/LibA.swift": """

Tests/SourceKitLSPTests/IndexTests.swift

@@ -16,7 +16,6 @@ import XCTest
 
 final class IndexTests: XCTestCase {
   func testIndexSwiftModules() async throws {
-    try await SkipUnless.swiftpmStoresModulesInSubdirectory()
     let project = try await SwiftPMTestProject(
       files: [
         "LibA/LibA.swift": """

Tests/SourceKitLSPTests/PublishDiagnosticsTests.swift

@@ -154,7 +154,6 @@ final class PublishDiagnosticsTests: XCTestCase {
 
   func testDiagnosticUpdatedAfterDependentModuleIsBuilt() async throws {
     try SkipUnless.longTestsEnabled()
-    try await SkipUnless.swiftpmStoresModulesInSubdirectory()
 
     let project = try await SwiftPMTestProject(
       files: [

Tests/SourceKitLSPTests/PullDiagnosticsTests.swift

@@ -185,7 +185,6 @@ final class PullDiagnosticsTests: XCTestCase {
 
   func testDiagnosticUpdatedAfterDependentModuleIsBuilt() async throws {
     try SkipUnless.longTestsEnabled()
-    try await SkipUnless.swiftpmStoresModulesInSubdirectory()
 
     let project = try await SwiftPMTestProject(
       files: [
@@ -251,8 +250,6 @@ final class PullDiagnosticsTests: XCTestCase {
   }
 
   func testDiagnosticsWaitForDocumentToBePrepared() async throws {
-    try await SkipUnless.swiftpmStoresModulesInSubdirectory()
-
     nonisolated(unsafe) var diagnosticRequestSent = AtomicBool(initialValue: false)
     var serverOptions = SourceKitLSPServer.Options.testDefault
     serverOptions.indexTestHooks.preparationTaskDidStart = { @Sendable taskDescription in