-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Add a new BuildDescription.load
API for use in SourceKit-LSP
#8286
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. question: The single test added validate a single
The test is only validation a single case for the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
} | ||
|
||
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 | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here I think. |
||
import PackageGraph | ||
internal import PackageLoading | ||
internal import PackageModel | ||
|
||
struct PluginTargetBuildDescription: BuildTarget { | ||
private let target: ResolvedModule | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. question: Is there value in adding additional tests that will validate the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is just calling into |
||
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 { | ||
|
Uh oh!
There was an error while loading. Please reload this page.