Skip to content

Commit 442f20e

Browse files
authored
Split out compilation of package plugins so it can be done separately from invoking them (typically for collecting diagnostics up-front) and add a PluginCompilationResult type that can be returned to clients. (#3841)
1 parent 4edaa39 commit 442f20e

File tree

2 files changed

+130
-23
lines changed

2 files changed

+130
-23
lines changed

Sources/Workspace/DefaultPluginScriptRunner.swift

Lines changed: 57 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -31,32 +31,37 @@ public struct DefaultPluginScriptRunner: PluginScriptRunner {
3131
self.toolchain = toolchain
3232
self.enableSandbox = enableSandbox
3333
}
34+
35+
public func compilePluginScript(sources: Sources, toolsVersion: ToolsVersion) throws -> PluginCompilationResult {
36+
return try self.compile(sources: sources, toolsVersion: toolsVersion, cacheDir: self.cacheDir)
37+
}
3438

3539
/// Public protocol function that compiles and runs the plugin as a subprocess. The tools version controls the availability of APIs in PackagePlugin, and should be set to the tools version of the package that defines the plugin (not of the target to which it is being applied).
3640
public func runPluginScript(sources: Sources, inputJSON: Data, toolsVersion: ToolsVersion, writableDirectories: [AbsolutePath], observabilityScope: ObservabilityScope, fileSystem: FileSystem) throws -> (outputJSON: Data, stdoutText: Data) {
37-
let compiledExec = try self.compile(sources: sources, toolsVersion: toolsVersion, cacheDir: self.cacheDir)
38-
return try self.invoke(compiledExec: compiledExec, toolsVersion: toolsVersion, writableDirectories: writableDirectories, input: inputJSON)
41+
// FIXME: We should only compile the plugin script again if needed.
42+
let result = try self.compile(sources: sources, toolsVersion: toolsVersion, cacheDir: self.cacheDir)
43+
guard let compiledExecutable = result.compiledExecutable else {
44+
throw DefaultPluginScriptRunnerError.compilationFailed(result)
45+
}
46+
return try self.invoke(compiledExec: compiledExecutable, toolsVersion: toolsVersion, writableDirectories: writableDirectories, input: inputJSON)
3947
}
4048

4149
public var hostTriple: Triple {
4250
return Self._hostTriple.memoize {
4351
Triple.getHostTriple(usingSwiftCompiler: self.toolchain.swiftCompilerPath)
4452
}
4553
}
46-
47-
/// Helper function that compiles a plugin script as an executable and returns the path to it.
48-
fileprivate func compile(sources: Sources, toolsVersion: ToolsVersion, cacheDir: AbsolutePath) throws -> AbsolutePath {
54+
55+
/// Helper function that compiles a plugin script as an executable and returns the path of the executable, any emitted diagnostics, etc. This function only throws an error if it wasn't even possible to start compiling the plugin — any regular compilation errors or warnings will be reflected in the returned compilation result.
56+
fileprivate func compile(sources: Sources, toolsVersion: ToolsVersion, cacheDir: AbsolutePath) throws -> PluginCompilationResult {
4957
// FIXME: Much of this is copied from the ManifestLoader and should be consolidated.
5058

59+
// Get access to the path containing the PackagePlugin module and library.
5160
let runtimePath = self.toolchain.swiftPMLibrariesLocation.pluginAPI
5261

53-
// Compile the package plugin script.
62+
// We use the toolchain's Swift compiler for compiling the plugin.
5463
var command = [self.toolchain.swiftCompilerPath.pathString]
5564

56-
// FIXME: Workaround for the module cache bug that's been haunting Swift CI
57-
// <rdar://problem/48443680>
58-
let moduleCachePath = ProcessEnv.vars["SWIFTPM_MODULECACHE_OVERRIDE"] ?? ProcessEnv.vars["SWIFTPM_TESTS_MODULECACHE"]
59-
6065
let macOSPackageDescriptionPath: AbsolutePath
6166
// if runtimePath is set to "PackageFrameworks" that means we could be developing SwiftPM in Xcode
6267
// which produces a framework for dynamic package products.
@@ -95,7 +100,12 @@ public struct DefaultPluginScriptRunner: PluginScriptRunner {
95100
// Add any extra flags required as indicated by the ManifestLoader.
96101
command += self.toolchain.swiftCompilerFlags
97102

103+
// Add the Swift language version implied by the package tools version.
98104
command += ["-swift-version", toolsVersion.swiftLanguageVersion.rawValue]
105+
106+
// Add the PackageDescription version specified by the package tools version, which controls what PackagePlugin API is seen.
107+
command += ["-package-description-version", toolsVersion.description]
108+
99109
// if runtimePath is set to "PackageFrameworks" that means we could be developing SwiftPM in Xcode
100110
// which produces a framework for dynamic package products.
101111
if runtimePath.extension == "framework" {
@@ -108,26 +118,33 @@ public struct DefaultPluginScriptRunner: PluginScriptRunner {
108118
command += ["-sdk", sdkRoot.pathString]
109119
}
110120
#endif
111-
command += ["-package-description-version", toolsVersion.description]
121+
122+
// Honor any module cache override that's set in the environment.
123+
let moduleCachePath = ProcessEnv.vars["SWIFTPM_MODULECACHE_OVERRIDE"] ?? ProcessEnv.vars["SWIFTPM_TESTS_MODULECACHE"]
112124
if let moduleCachePath = moduleCachePath {
113125
command += ["-module-cache-path", moduleCachePath]
114126
}
115127

116128
// Parse the plugin as a library so that `@main` is supported even though there might be only a single source file.
117129
command += ["-parse-as-library"]
118-
130+
131+
// Add options to create a .dia file containing any diagnostics emitted by the compiler.
132+
let diagnosticsFile = cacheDir.appending(component: "diagnostics.dia")
133+
command += ["-Xfrontend", "-serialize-diagnostics-path", "-Xfrontend", diagnosticsFile.pathString]
134+
135+
// Add all the source files that comprise the plugin scripts.
119136
command += sources.paths.map { $0.pathString }
120-
let compiledExec = cacheDir.appending(component: "compiled-plugin")
121-
command += ["-o", compiledExec.pathString]
122-
123-
let result = try Process.popen(arguments: command, environment: toolchain.swiftCompilerEnvironment)
124-
let output = try (result.utf8Output() + result.utf8stderrOutput()).spm_chuzzle() ?? ""
125-
if result.exitStatus != .terminated(code: 0) {
126-
// TODO: Make this a proper error.
127-
throw StringError("failed to compile package plugin:\n\(command)\n\n\(output)")
128-
}
129-
130-
return compiledExec
137+
138+
// Add the path of the compiled executable.
139+
let executableFile = cacheDir.appending(component: "compiled-plugin")
140+
command += ["-o", executableFile.pathString]
141+
142+
// Invoke the compiler and get back the result.
143+
let compilerResult = try Process.popen(arguments: command, environment: toolchain.swiftCompilerEnvironment)
144+
145+
// Finally return the result. We return the path of the compiled executable only if the compilation succeeded.
146+
let compiledExecutable = (compilerResult.exitStatus == .terminated(code: 0)) ? executableFile : nil
147+
return PluginCompilationResult(compiledExecutable: compiledExecutable, diagnosticsFile: diagnosticsFile, compilerResult: compilerResult)
131148
}
132149

133150
/// Returns path to the sdk, if possible.
@@ -210,9 +227,24 @@ public struct DefaultPluginScriptRunner: PluginScriptRunner {
210227
}
211228
}
212229

230+
/// The result of compiling a plugin. The executable path will only be present if the compilation succeeds, while the other properties are present in all cases.
231+
public struct PluginCompilationResult {
232+
/// Path of the compiled executable, or .none if compilation failed.
233+
public var compiledExecutable: AbsolutePath?
234+
235+
/// Path of the libClang diagnostics file emitted by the compiler (even if compilation succeded, it might contain warnings).
236+
public var diagnosticsFile: AbsolutePath
237+
238+
/// Process result of invoking the Swift compiler to produce the executable (contains command line, environment, exit status, and any output).
239+
public var compilerResult: ProcessResult
240+
}
241+
213242

214243
/// An error encountered by the default plugin runner.
215244
public enum DefaultPluginScriptRunnerError: Error {
245+
/// Failed to compile the plugin script, so it cannot be run.
246+
case compilationFailed(PluginCompilationResult)
247+
216248
/// Failed to start running the compiled plugin script as a subprocess. The message describes the error, and the
217249
/// command is the full command line that the runner tried to launch.
218250
case subprocessDidNotStart(_ message: String, command: [String])
@@ -230,6 +262,8 @@ public enum DefaultPluginScriptRunnerError: Error {
230262
extension DefaultPluginScriptRunnerError: CustomStringConvertible {
231263
public var description: String {
232264
switch self {
265+
case .compilationFailed(let result):
266+
return "could not compile plugin script: \(result)"
233267
case .subprocessDidNotStart(let message, _):
234268
return "could not run plugin script: \(message)"
235269
case .subprocessFailed(let message, _, let output):

Tests/SPMBuildCoreTests/PluginInvocationTests.swift

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
import Basics
1212
import PackageGraph
13+
import PackageLoading
1314
import PackageModel
1415
@testable import SPMBuildCore
1516
import SPMTestSupport
@@ -185,4 +186,76 @@ class PluginInvocationTests: XCTestCase {
185186

186187
XCTAssertEqual(evalFirstResult.textOutput, "Hello Plugin!")
187188
}
189+
190+
func testCompilationDiagnostics() throws {
191+
try testWithTemporaryDirectory { tmpPath in
192+
// Create a sample package with a library target and a plugin.
193+
let packageDir = tmpPath.appending(components: "MyPackage")
194+
try localFileSystem.writeFileContents(packageDir.appending(component: "Package.swift")) {
195+
$0 <<< """
196+
// swift-tools-version: 5.6
197+
import PackageDescription
198+
let package = Package(
199+
name: "MyPackage",
200+
targets: [
201+
.target(
202+
name: "MyLibrary",
203+
plugins: [
204+
"MyPlugin",
205+
]
206+
),
207+
.plugin(
208+
name: "MyPlugin",
209+
capability: .buildTool()
210+
),
211+
]
212+
)
213+
"""
214+
}
215+
try localFileSystem.writeFileContents(packageDir.appending(components: "Sources", "MyLibrary", "library.swift")) {
216+
$0 <<< "public func Foo() { }\n"
217+
}
218+
try localFileSystem.writeFileContents(packageDir.appending(components: "Plugins", "MyPlugin", "plugin.swift")) {
219+
$0 <<< "syntax error\n"
220+
}
221+
222+
// Load a workspace from the package.
223+
let observability = ObservabilitySystem.makeForTesting()
224+
let workspace = try Workspace(
225+
fileSystem: localFileSystem,
226+
location: .init(forRootPackage: packageDir, fileSystem: localFileSystem),
227+
customManifestLoader: ManifestLoader(toolchain: ToolchainConfiguration.default),
228+
delegate: MockWorkspaceDelegate()
229+
)
230+
231+
// Load the root manifest.
232+
let rootInput = PackageGraphRootInput(packages: [packageDir], dependencies: [])
233+
let rootManifests = try tsc_await {
234+
workspace.loadRootManifests(
235+
packages: rootInput.packages,
236+
observabilityScope: observability.topScope,
237+
completion: $0
238+
)
239+
}
240+
XCTAssert(rootManifests.count == 1, "\(rootManifests)")
241+
242+
// Load the package graph.
243+
let packageGraph = try workspace.loadPackageGraph(rootInput: rootInput, observabilityScope: observability.topScope)
244+
XCTAssert(observability.diagnostics.isEmpty, "\(observability.diagnostics)")
245+
XCTAssert(packageGraph.packages.count == 1, "\(packageGraph.packages)")
246+
247+
// Find the build tool plugin.
248+
let buildToolPlugin = try XCTUnwrap(packageGraph.packages[0].targets.first{ $0.type == .plugin })
249+
XCTAssertEqual(buildToolPlugin.name, "MyPlugin")
250+
251+
let pluginCacheDir = tmpPath.appending(component: "plugin-cache")
252+
let pluginScriptRunner = DefaultPluginScriptRunner(cacheDir: pluginCacheDir, toolchain: ToolchainConfiguration.default)
253+
let result = try pluginScriptRunner.compilePluginScript(sources: buildToolPlugin.sources, toolsVersion: .currentToolsVersion)
254+
255+
// Expect a failure since our input code is intentionally broken.
256+
XCTAssert(result.compilerResult.exitStatus == .terminated(code: 1), "\(result.compilerResult.exitStatus)")
257+
XCTAssert(result.compiledExecutable == .none, "\(result.compiledExecutable?.pathString ?? "-")")
258+
XCTAssert(result.diagnosticsFile.suffix == ".dia", "\(result.diagnosticsFile.pathString)")
259+
}
260+
}
188261
}

0 commit comments

Comments
 (0)