Public API for getting information about build targets

[neonichu]

Until now, SourceKit-LSP has been using a couple of internal data structures of the build plan, which were incidentally public, for this. This changes exposes a new public API for accessing information about build targets, including those for plugins.

Note that this API is part of a new module which could be the start of a "proper" public API for SwiftPM. That said, right now it depends on several types of existing modules in its public interface and that isn't likely to be resolved as part of this particular change.

rdar://112120976

[neonichu]

This will probably need some iteration to get the API right for what sourcekit-lsp needs.

[neonichu]

@swift-ci please smoke test

[neonichu]

@ahoppen Using TargetBuildDescription for plugins wasn’t really feasible because it is very much geared towards “real” targets. So I introduced a new protocol that the existing target build description types and a type geared towards plugins can call implement.

We should probably also figure out a way for how to get the compiler arguments for manifest compilation into this, IIRC there's some ad-hoc way to handle them in sourcekit-lsp.

[ahoppen]

I have started to use this in swiftlang/sourcekit-lsp#793 and so far I have identified two problems:

• The compiler arguments don’t contain the paths of the source files (easily fixed by https://github.com/apple/sourcekit-lsp/pull/793/files#diff-04e6f1a156c7d65bd64e75fc44f76bed1926a35a7aef0c80aa06329befb739a0R44-R51)
• If we have a package with multiple modules and dependencies between them (like https://github.com/apple/sourcekit-lsp/blob/main/Sources/SKTestSupport/INPUTS/SwiftPMPackage/Package.swift) and I get the compiler arguments for exec/main.swift, the returned compiler arguments don’t contain the build path as an include path (-I). This is causing testOpenInterface to fail

[neonichu]

Moved the code to a new module, haven't addressed any feedback yet.

[neonichu]

@swift-ci please test

[neonichu]

@swift-ci please test windows

[neonichu]

@swift-ci please test windows

[neonichu]

@swift-ci please test windows

[neonichu]

¯\_(ツ)_/¯

[neonichu]

@swift-ci please test windows

[neonichu]

@swift-ci please test windows

[MaxDesiatov]

Would it be possible to also add a new tests target and cover the new API in this PR?

[neonichu]

Would it be possible to also add a new tests target and cover the new API in this PR?

Yes, definitely. First trying to figure out the right shape and behavior for the API.

[ahoppen]

I just updated swiftlang/sourcekit-lsp#793 to use the new API and the second issue I noted (about missing -I) is fixed.

The following issue still exists, though

The compiler arguments don’t contain the paths of the source files (easily fixed by https://github.com/apple/sourcekit-lsp/pull/793/files#diff-04e6f1a156c7d65bd64e75fc44f76bed1926a35a7aef0c80aa06329befb739a0R44-R51)

[neonichu]

Looks like I was returning URLs for the file paths, I think that may have been the issue?

[neonichu]

Oh, also for plugins the file paths aren't returned at all, that is true.

[neonichu]

I made some more fixes here and this is mostly passing the existing sourcekit-lsp tests in SwiftPMWorkspaceTests. There are two remaining issues:

• testBasicCXXArgs expects to receive arguments for header files which is not something SwiftPM's build system currently supports
• testSymlinkInWorkspaceCXX has some expectations around symlinks that we're currently not meeting. I'll be looking into these more

[neonichu]

This is the patch I was working with on top of the sourcekit-lsp branch Alex created:

diff --git a/Sources/SKSwiftPMWorkspace/SwiftPMWorkspace.swift b/Sources/SKSwiftPMWorkspace/SwiftPMWorkspace.swift
index f720676..9e04d64 100644
--- a/Sources/SKSwiftPMWorkspace/SwiftPMWorkspace.swift
+++ b/Sources/SKSwiftPMWorkspace/SwiftPMWorkspace.swift
@@ -50,14 +50,6 @@ public typealias SwiftBuildTarget = SwiftPM.BuildTarget
 /// A build target in `BuildServerProtocol`
 public typealias BuildServerTarget = BuildServerProtocol.BuildTarget
 
-extension SwiftBuildTarget {
-  func fixedCompileArguments() throws -> [String] {
-    var args = try self.compileArguments()
-    args += self.sources.map { $0.path }
-    return args
-  }
-}
-
 /// Swift Package Manager build system and workspace support.
 ///
 /// This class implements the `BuildSystem` interface to provide the build settings for a Swift
@@ -301,7 +293,7 @@ extension SwiftPMWorkspace: SKCore.BuildSystem {
 
     if let buildTarget = try buildTarget(for: path) {
       return FileBuildSettings(
-        compilerArguments: try buildTarget.fixedCompileArguments(),
+        compilerArguments: try buildTarget.compileArguments(for: path.asURL),
         workingDirectory: workspacePath.pathString
       )
     }
@@ -414,7 +406,8 @@ extension SwiftPMWorkspace {
       while !dir.isRoot {
         if let buildTarget = sourceDirToTarget[dir] {
           return FileBuildSettings(
-            compilerArguments: try buildTarget.fixedCompileArguments(),
+            // FIXME: how to determine the "right" file path here?
+            compilerArguments: try buildTarget.compileArguments(for: buildTarget.sources[0]),
             workingDirectory: workspacePath.pathString
           )
         }
@@ -430,73 +423,6 @@ extension SwiftPMWorkspace {
     let canonicalPath = try resolveSymlinks(path)
     return try canonicalPath == path ? nil : impl(canonicalPath)
   }
-
-  /// Retrieve settings for the given C-family language file, which is part of a known target build
-  /// description.
-  ///
-  /// - Note: language must be a C-family language.
-  public func settings(
-    forClangFile path: AbsolutePath,
-    _ language: Language,
-    _ td: ClangTargetBuildDescription
-  ) throws -> FileBuildSettings {
-    // FIXME: this is re-implementing things from swiftpm's createClangCompileTarget
-
-    var args = try td.basicArguments()
-
-    let nativePath: AbsolutePath =
-      try URL(fileURLWithPath: path.pathString).withUnsafeFileSystemRepresentation {
-        try AbsolutePath(validating: String(cString: $0!))
-      }
-    let compilePath = try td.compilePaths().first(where: { $0.source == nativePath })
-    if let compilePath = compilePath {
-      args += [
-        "-MD",
-        "-MT",
-        "dependencies",
-        "-MF",
-        compilePath.deps.pathString,
-      ]
-    }
-
-    switch language {
-    case .c:
-      if let std = td.clangTarget.cLanguageStandard {
-        args += ["-std=\(std)"]
-      }
-    case .cpp:
-      if let std = td.clangTarget.cxxLanguageStandard {
-        args += ["-std=\(std)"]
-      }
-    default:
-      break
-    }
-
-    if let compilePath = compilePath {
-      args += [
-        "-c",
-        compilePath.source.pathString,
-        "-o",
-        compilePath.object.pathString,
-      ]
-    } else if path.extension == "h" {
-      args += ["-c"]
-      if let xflag = language.xflagHeader {
-        args += ["-x", xflag]
-      }
-      args += [path.pathString]
-    } else {
-      args += [
-        "-c",
-        path.pathString,
-      ]
-    }
-
-    return FileBuildSettings(
-      compilerArguments: args,
-      workingDirectory: workspacePath.pathString
-    )
-  }
 }
 
 /// Find a Swift Package root directory that contains the given path, if any.
diff --git a/Sources/SKTestSupport/Utils.swift b/Sources/SKTestSupport/Utils.swift
index 471a61d..e3a523b 100644
--- a/Sources/SKTestSupport/Utils.swift
+++ b/Sources/SKTestSupport/Utils.swift
@@ -13,6 +13,7 @@
 import Foundation
 import LanguageServerProtocol
 import SKCore
+import TSCBasic
 
 extension Language {
   var fileExtension: String {
@@ -63,6 +64,25 @@ public func testScratchDir(testName: String = #function) throws -> URL {
   return url
 }
 
+/// Execute `body` with a path to a temporary scratch directory for the given
+/// test name.
+///
+/// The temporary directory will be deleted at the end of `directory` unless the
+/// `SOURCEKITLSP_KEEP_TEST_SCRATCH_DIR` environment variable is set.
+public func withTestScratchDir<T>(
+ _ body: (AbsolutePath) async throws -> T,
+ testName: String = #function
+) async throws -> T {
+ let scratchDirectory = try testScratchDir(testName: testName)
+ try FileManager.default.createDirectory(at: scratchDirectory, withIntermediateDirectories: true)
+ defer {
+  if cleanScratchDirectories {
+   try? FileManager.default.removeItem(at: scratchDirectory)
+  }
+ }
+ return try await body(try AbsolutePath(validating: scratchDirectory.path))
+}
+
 fileprivate extension URL {
   /// Assuming this is a file URL, resolves all symlinks in the path.
   ///
diff --git a/Tests/SKSwiftPMWorkspaceTests/SwiftPMWorkspaceTests.swift b/Tests/SKSwiftPMWorkspaceTests/SwiftPMWorkspaceTests.swift
index b5ded2e..fdf10d9 100644
--- a/Tests/SKSwiftPMWorkspaceTests/SwiftPMWorkspaceTests.swift
+++ b/Tests/SKSwiftPMWorkspaceTests/SwiftPMWorkspaceTests.swift
@@ -141,13 +141,14 @@ final class SwiftPMWorkspaceTests: XCTestCase {
       check(
         "-module-name",
         "lib",
-        "-incremental",
         "-emit-dependencies",
         "-emit-module",
         "-emit-module-path",
         arguments: arguments
       )
-      check("-parse-as-library", "-c", arguments: arguments)
+      check("-incremental", arguments: arguments)
+      check("-parse-as-library", arguments: arguments)
+      check("-c", arguments: arguments)
 
       check("-target", arguments: arguments)  // Only one!
       #if os(macOS)
@@ -168,7 +169,7 @@ final class SwiftPMWorkspaceTests: XCTestCase {
   func testBuildSetup() async throws {
     // FIXME: should be possible to use InMemoryFileSystem.
     let fs = localFileSystem
-    try await withTemporaryDirectory(removeTreeOnDeinit: true) { tempDir in
+    try await withTestScratchDir { tempDir in
       try fs.createFiles(
         root: tempDir,
         files: [
@@ -350,7 +351,7 @@ final class SwiftPMWorkspaceTests: XCTestCase {
   func testUnknownFile() async throws {
     // FIXME: should be possible to use InMemoryFileSystem.
     let fs = localFileSystem
-    try await withTemporaryDirectory(removeTreeOnDeinit: true) { tempDir in
+    try await withTestScratchDir { tempDir in
       try fs.createFiles(
         root: tempDir,
         files: [
@@ -480,7 +481,7 @@ final class SwiftPMWorkspaceTests: XCTestCase {
   func testDeploymentTargetSwift() async throws {
     // FIXME: should be possible to use InMemoryFileSystem.
     let fs = localFileSystem
-    try await withTemporaryDirectory(removeTreeOnDeinit: true) { tempDir in
+    try await withTestScratchDir { tempDir in
       try fs.createFiles(
         root: tempDir,
         files: [
@@ -523,7 +524,7 @@ final class SwiftPMWorkspaceTests: XCTestCase {
   func testSymlinkInWorkspaceSwift() async throws {
     // FIXME: should be possible to use InMemoryFileSystem.
     let fs = localFileSystem
-    try await withTemporaryDirectory(removeTreeOnDeinit: true) { tempDir in
+    try await withTestScratchDir { tempDir in
       try fs.createFiles(
         root: tempDir,
         files: [
@@ -698,7 +699,7 @@ final class SwiftPMWorkspaceTests: XCTestCase {
   func testPluginArgs() async throws {
     // FIXME: should be possible to use InMemoryFileSystem.
     let fs = localFileSystem
-    try await withTemporaryDirectory(removeTreeOnDeinit: true) { tempDir in
+    try await withTestScratchDir { tempDir in
       try fs.createFiles(
         root: tempDir,
         files: [

[neonichu]

@swift-ci please test

[neonichu]

@swift-ci please test

[neonichu]

@swift-ci please test windows

[neonichu]

@swift-ci please test

[neonichu]

@swift-ci please test windows

[neonichu]

@swift-ci please test windows

[neonichu]

@swift-ci please test windows

[neonichu]

/home/build-user/swiftpm/Sources/SPMTestSupport/MockBuildTestHelper.swift:13:18: error: module 'PackageModel' was not compiled for testing
@testable import PackageModel
~~~~~~~~~~       ^

Ugh, I guess I'll have to change or split up MockBuildTestHelper in some way.

[neonichu]

I don't really understand why I can't reproduce the failure locally, but it also seems like @testable wasn't needed anyway.

[neonichu]

@swift-ci please test

[neonichu]

@swift-ci please test windows

[neonichu]

@swift-ci please test windows

[neonichu]

@swift-ci please test windows

[neonichu]

@swift-ci please test windows

[neonichu]

Consistently seeing this again, I guess I'll wait a bit before retriggering Windows

======UPDATE FAILURES======
C:\Users\swift-ci\jenkins\workspace\swiftpm-PR-windows\swift-docc failed (ret=1): ['git', 'rebase', 'FETCH_HEAD']
b'error: cannot rebase: You have unstaged changes.\nerror: Please commit or stash them.\n'
update-checkout failed, fix errors and try again
Build step 'Execute Windows batch command' marked build as failure

[neonichu]

@swift-ci please test windows