Add a new BuildDescription.load API for use in SourceKit-LSP

bnbarham · bnbarham · commit 71ab073b5eb3 · 2025-02-18T15:52:56.000-08:00
Build tool plugins need to be built and run as part of the build plan
generation. Rather than relying on clients to do so, add a new API that
does both. This does unfortunately expose much more of the SwiftPM API
than previously, but given it is all required to a generate a
`BuildPlan`, this was really already the case.

Long term this module should be replaced by a long-running BSP server
that can prepare and provide build settings in the one process.

Resolves rdar://102242345.
diff --git a/Package.swift b/Package.swift
@@ -169,7 +169,11 @@ let package = Package(
         .target(
             name: "SourceKitLSPAPI",
             dependencies: [
+                "Basics",
                 "Build",
+                "PackageGraph",
+                "PackageLoading",
+                "PackageModel",
                 "SPMBuildCore",
             ],
             exclude: ["CMakeLists.txt"],
diff --git a/Sources/Build/BuildOperation.swift b/Sources/Build/BuildOperation.swift
@@ -309,12 +309,12 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
                 }
             }
             // We need to perform actual planning if we reach here.
-            return try await self.plan(subset: subset).description
+            return try await self.generateDescription(subset: subset).description
         }
     }
 
     public func getBuildManifest() async throws -> LLBuildManifest {
-        try await self.plan().manifest
+        try await self.generateDescription().manifest
     }
 
     /// Cancel the active build operation.
@@ -647,9 +647,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
         }
     }
 
-    /// Create the build plan and return the build description.
-    private func plan(subset: BuildSubset? = nil) async throws -> (description: BuildDescription, manifest: LLBuildManifest) {
-        // Load the package graph.
+    package func generatePlan() async throws -> BuildPlan {
         let graph = try await getPackageGraph()
 
         let pluginTools: [ResolvedModule.ID: [String: PluginTool]]
@@ -672,7 +670,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
         }
 
         // Create the build plan based on the modules graph and any information from plugins.
-        let plan = try await BuildPlan(
+        return try await BuildPlan(
             destinationBuildParameters: self.config.destinationBuildParameters,
             toolsBuildParameters: self.config.buildParameters(for: .host),
             graph: graph,
@@ -684,23 +682,29 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
             fileSystem: self.fileSystem,
             observabilityScope: self.observabilityScope
         )
+
+    }
+
+    /// Create the build plan and return the build description.
+    private func generateDescription(subset: BuildSubset? = nil) async throws -> (description: BuildDescription, manifest: LLBuildManifest) {
+        let plan = try await generatePlan()
         self._buildPlan = plan
 
         // Emit warnings about any unhandled files in authored packages. We do this after applying build tool plugins, once we know what files they handled.
         // rdar://113256834 This fix works for the plugins that do not have PreBuildCommands.
         let targetsToConsider: [ResolvedModule]
         if let subset = subset, let recursiveDependencies = try
-            subset.recursiveDependencies(for: graph, observabilityScope: observabilityScope) {
+            subset.recursiveDependencies(for: plan.graph, observabilityScope: observabilityScope) {
             targetsToConsider = recursiveDependencies
         } else {
-            targetsToConsider = Array(graph.reachableModules)
+            targetsToConsider = Array(plan.graph.reachableModules)
         }
 
         for module in targetsToConsider {
             // Subtract out any that were inputs to any commands generated by plugins.
             if let pluginResults = plan.buildToolPluginInvocationResults[module.id] {
                 diagnoseUnhandledFiles(
-                    modulesGraph: graph,
+                    modulesGraph: plan.graph,
                     module: module,
                     buildToolPluginInvocationResults: pluginResults
                 )
@@ -851,7 +855,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
 
     public func packageStructureChanged() async -> Bool {
         do {
-            _ = try await self.plan()
+            _ = try await self.generateDescription()
         }
         catch Diagnostics.fatalError {
             return false
@@ -947,7 +951,7 @@ extension BuildOperation {
                 // Determine the tools to which this plugin has access, and create a name-to-path mapping from tool
                 // names to the corresponding paths. Built tools are assumed to be in the build tools directory.
                 let accessibleTools = try await plugin.preparePluginTools(
-                    fileSystem: fileSystem,
+                    fileSystem: config.fileSystem,
                     environment: config.buildEnvironment(for: .host),
                     for: hostTriple
                 ) { name, path in
diff --git a/Sources/CoreCommands/Options.swift b/Sources/CoreCommands/Options.swift
@@ -654,7 +654,7 @@ package struct TraitOptions: ParsableArguments {
     /// The traits to enable for the package.
     @Option(
         name: .customLong("traits"),
-        help: "Enables the passed traits of the package. Multiple traits can be specified by providing a space separated list e.g. `--traits Trait1 Trait2`. When enabling specific traits the defaults traits need to explictily enabled as well by passing `defaults` to this command."
+        help: "Enables the passed traits of the package. Multiple traits can be specified by providing a comma separated list e.g. `--traits Trait1,Trait2`. When enabling specific traits the defaults traits need to explictily enabled as well by passing `defaults` to this command."
     )
     package var _enabledTraits: String?
 
diff --git a/Sources/PackageGraph/TraitConfiguration.swift b/Sources/PackageGraph/TraitConfiguration.swift
@@ -18,7 +18,7 @@ public struct TraitConfiguration: Codable, Hashable {
     /// Enables all traits of the package.
     package var enableAllTraits: Bool
 
-    package init(
+    public init(
         enabledTraits: Set<String>? = nil,
         enableAllTraits: Bool = false
     ) {
diff --git a/Sources/SourceKitLSPAPI/BuildDescription.swift b/Sources/SourceKitLSPAPI/BuildDescription.swift
@@ -10,20 +10,17 @@
 //
 //===----------------------------------------------------------------------===//
 
-import struct Foundation.URL
-
-private import struct Basics.AbsolutePath
-private import func Basics.resolveSymlinks
-
-internal import SPMBuildCore
-
-// FIXME: should import these module with `private` or `internal` access control
-import class Build.BuildPlan
-import class Build.ClangModuleBuildDescription
-import class Build.SwiftModuleBuildDescription
-import struct PackageGraph.ResolvedModule
-import struct PackageGraph.ModulesGraph
-internal import class PackageModel.UserToolchain
+import Foundation
+import TSCBasic
+
+// Ideally wouldn't expose these (it defeats the purpose of this module), but we should replace this entire API with
+// a BSP server, so this is good enough for now (and LSP is using all these types internally anyway).
+import Basics
+import Build
+import PackageGraph
+internal import PackageLoading
+internal import PackageModel
+import SPMBuildCore
 
 public enum BuildDestination {
     case host
@@ -90,7 +87,13 @@ private struct WrappedClangTargetBuildDescription: BuildTarget {
     }
 
     var others: [URL] {
-        return description.others.map(\.asURL)
+        var others = Set(description.others)
+        for pluginResult in description.buildToolPluginInvocationResults {
+            for buildCommand in pluginResult.buildCommands {
+                others.formUnion(buildCommand.inputFiles)
+            }
+        }
+        return others.map(\.asURL)
     }
 
     public var name: String {
@@ -102,7 +105,7 @@ private struct WrappedClangTargetBuildDescription: BuildTarget {
     }
 
     public func compileArguments(for fileURL: URL) throws -> [String] {
-        let filePath = try resolveSymlinks(try AbsolutePath(validating: fileURL.path))
+        let filePath = try resolveSymlinks(try Basics.AbsolutePath(validating: fileURL.path))
         let commandLine = try description.emitCommandLine(for: filePath)
         // First element on the command line is the compiler itself, not an argument.
         return Array(commandLine.dropFirst())
@@ -143,7 +146,13 @@ private struct WrappedSwiftTargetBuildDescription: BuildTarget {
     }
 
     var others: [URL] {
-        return description.others.map(\.asURL)
+        var others = Set(description.others)
+        for pluginResult in description.buildToolPluginInvocationResults {
+            for buildCommand in pluginResult.buildCommands {
+                others.formUnion(buildCommand.inputFiles)
+            }
+        }
+        return others.map(\.asURL)
     }
 
     func compileArguments(for fileURL: URL) throws -> [String] {
@@ -160,14 +169,52 @@ public struct BuildDescription {
 
     /// The inputs of the build plan so we don't need to re-compute them  on every call to
     /// `fileAffectsSwiftOrClangBuildSettings`.
-    private let inputs: [BuildPlan.Input]
+    private let inputs: [Build.BuildPlan.Input]
 
-    // FIXME: should not use `BuildPlan` in the public interface
+    /// Wrap an already constructed build plan.
     public init(buildPlan: Build.BuildPlan) {
         self.buildPlan = buildPlan
         self.inputs = buildPlan.inputs
     }
 
+    /// Construct a build description, compiling build tool plugins and generating their output when necessary.
+    public static func load(
+        destinationBuildParameters: BuildParameters,
+        toolsBuildParameters: BuildParameters,
+        packageGraph: ModulesGraph,
+        pluginConfiguration: PluginConfiguration,
+        traitConfiguration: TraitConfiguration,
+        disableSandbox: Bool,
+        scratchDirectory: URL,
+        fileSystem: any FileSystem,
+        observabilityScope: ObservabilityScope
+    ) async throws -> (description: BuildDescription, errors: String) {
+        let bufferedOutput = BufferedOutputByteStream()
+        let threadSafeOutput = ThreadSafeOutputByteStream(bufferedOutput)
+
+        // This is quite an abuse of `BuildOperation`, building plugins should really be refactored out of it. Though
+        // even better would be to have a BSP server that handles both preparing and getting settings.
+        // https://github.com/swiftlang/swift-package-manager/issues/8287
+        let operation = BuildOperation(
+            productsBuildParameters: destinationBuildParameters,
+            toolsBuildParameters: toolsBuildParameters,
+            cacheBuildManifest: true,
+            packageGraphLoader: { packageGraph },
+            pluginConfiguration: pluginConfiguration,
+            scratchDirectory: try Basics.AbsolutePath(validating: scratchDirectory.path),
+            traitConfiguration: traitConfiguration,
+            additionalFileRules: FileRuleDescription.swiftpmFileTypes,
+            pkgConfigDirectories: [],
+            outputStream: threadSafeOutput,
+            logLevel: .error,
+            fileSystem: fileSystem,
+            observabilityScope: observabilityScope
+        )
+
+        let plan = try await operation.generatePlan()
+        return (BuildDescription(buildPlan: plan), bufferedOutput.bytes.description)
+    }
+
     func getBuildTarget(
         for module: ResolvedModule,
         destination: BuildParameters.Destination
@@ -219,7 +266,7 @@ public struct BuildDescription {
     /// Returns `true` if the file at the given path might influence build settings for a `swiftc` or `clang` invocation
     /// generated by SwiftPM.
     public func fileAffectsSwiftOrClangBuildSettings(_ url: URL) -> Bool {
-        guard let filePath = try? AbsolutePath(validating: url.path) else {
+        guard let filePath = try? Basics.AbsolutePath(validating: url.path) else {
             return false
         }
 
diff --git a/Sources/SourceKitLSPAPI/CMakeLists.txt b/Sources/SourceKitLSPAPI/CMakeLists.txt
@@ -10,8 +10,13 @@ add_library(SourceKitLSPAPI STATIC
   BuildDescription.swift
   PluginTargetBuildDescription.swift)
 target_link_libraries(SourceKitLSPAPI PUBLIC
+  Basics
   Build
+  PackageGraph
   SPMBuildCore)
+target_link_libraries(SourceKitLSPAPI PRIVATE
+  PackageLoading
+  PackageModel)
 
 # NOTE(compnerd) workaround for CMake not setting up include flags yet
 set_target_properties(SourceKitLSPAPI PROPERTIES
diff --git a/Sources/SourceKitLSPAPI/PluginTargetBuildDescription.swift b/Sources/SourceKitLSPAPI/PluginTargetBuildDescription.swift
@@ -10,15 +10,12 @@
 //
 //===----------------------------------------------------------------------===//
 
-private import Basics
+import Foundation
 
-import struct Foundation.URL
-
-import struct PackageGraph.ResolvedModule
-
-private import class PackageLoading.ManifestLoader
-internal import struct PackageModel.ToolsVersion
-internal import protocol PackageModel.Toolchain
+import Basics
+import PackageGraph
+internal import PackageLoading
+internal import PackageModel
 
 struct PluginTargetBuildDescription: BuildTarget {
     private let target: ResolvedModule
diff --git a/Tests/SourceKitLSPAPITests/SourceKitLSPAPITests.swift b/Tests/SourceKitLSPAPITests/SourceKitLSPAPITests.swift
@@ -18,7 +18,7 @@ import PackageGraph
 
 import PackageModel
 @testable import SourceKitLSPAPI
-import struct SPMBuildCore.BuildParameters
+import SPMBuildCore
 import _InternalTestSupport
 import XCTest
 
@@ -255,6 +255,70 @@ final class SourceKitLSPAPITests: XCTestCase {
             ]
         )
     }
+
+    func testLoadPackage() async throws {
+        let fs = InMemoryFileSystem(emptyFiles:
+            "/Pkg/Sources/lib/lib.swift"
+        )
+
+        let observability = ObservabilitySystem.makeForTesting()
+        let graph = try loadModulesGraph(
+            fileSystem: fs,
+            manifests: [
+                Manifest.createRootManifest(
+                    displayName: "Pkg",
+                    path: "/Pkg",
+                    toolsVersion: .v5_10,
+                    targets: [
+                        TargetDescription(
+                            name: "lib",
+                            dependencies: []
+                        )
+                    ]),
+            ],
+            observabilityScope: observability.topScope
+        )
+        XCTAssertNoDiagnostics(observability.diagnostics)
+
+        let destinationBuildParameters = mockBuildParameters(destination: .target)
+        try await withTemporaryDirectory { tmpDir in
+            let pluginConfiguration = PluginConfiguration(
+                scriptRunner: DefaultPluginScriptRunner(
+                    fileSystem: fs,
+                    cacheDir: tmpDir.appending("cache"),
+                    toolchain: try UserToolchain.default
+                ),
+                workDirectory: tmpDir.appending("work"),
+                disableSandbox: false
+            )
+            let scratchDirectory = tmpDir.appending(".build")
+
+            let loaded = try await BuildDescription.load(
+                destinationBuildParameters: destinationBuildParameters,
+                toolsBuildParameters: mockBuildParameters(destination: .host),
+                packageGraph: graph,
+                pluginConfiguration: pluginConfiguration,
+                traitConfiguration: TraitConfiguration(),
+                disableSandbox: false,
+                scratchDirectory: scratchDirectory.asURL,
+                fileSystem: fs,
+                observabilityScope: observability.topScope
+            )
+
+            try loaded.description.checkArguments(
+                for: "lib",
+                graph: graph,
+                partialArguments: [
+                    "-module-name", "lib",
+                    "-package-name", "pkg",
+                    "-emit-dependencies",
+                    "-emit-module",
+                    "-emit-module-path", "/path/to/build/\(destinationBuildParameters.triple)/debug/Modules/lib.swiftmodule"
+                ],
+                isPartOfRootPackage: true
+            )
+        }
+    }
 }
 
 extension SourceKitLSPAPI.BuildDescription {

Original file line number	Diff line number	Diff line change
`@@ -654,7 +654,7 @@ package struct TraitOptions: ParsableArguments {`
`654`	`654`	`/// The traits to enable for the package.`
`655`	`655`	`@Option(`
`656`	`656`	`name: .customLong("traits"),`
`657`		- help: "Enables the passed traits of the package. Multiple traits can be specified by providing a space separated list e.g. `--traits Trait1 Trait2`. When enabling specific traits the defaults traits need to explictily enabled as well by passing `defaults` to this command."
	`657`	+ help: "Enables the passed traits of the package. Multiple traits can be specified by providing a comma separated list e.g. `--traits Trait1,Trait2`. When enabling specific traits the defaults traits need to explictily enabled as well by passing `defaults` to this command."
`658`	`658`	`)`
`659`	`659`	`package var _enabledTraits: String?`
`660`	`660`