Skip to content

Commit 835bdac

Browse files
neonichuelsh
andauthored
Error and exit early if prebuild command references built tool (#5947)
* Error and exit early if prebuild command references built tool rdar://94712361 Co-authored-by: Ellie Shin <[email protected]>
1 parent 2319c92 commit 835bdac

File tree

2 files changed

+31
-14
lines changed

2 files changed

+31
-14
lines changed

Sources/SPMBuildCore/PluginInvocation.swift

Lines changed: 30 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,9 @@ extension PluginTarget {
133133

134134
/// Whether at least one error has been reported; this is used to make sure there is at least one error if the plugin fails.
135135
var hasReportedError = false
136+
137+
/// If this is true, we exited early with an error.
138+
var exitEarly = false
136139

137140
init(invocationDelegate: PluginInvocationDelegate, observabilityScope: ObservabilityScope) {
138141
self.invocationDelegate = invocationDelegate
@@ -191,14 +194,19 @@ extension PluginTarget {
191194
outputFiles: try outputFiles.map{ try AbsolutePath(validating: $0) })
192195

193196
case .definePrebuildCommand(let config, let outputFilesDir):
194-
self.invocationDelegate.pluginDefinedPrebuildCommand(
197+
let success = self.invocationDelegate.pluginDefinedPrebuildCommand(
195198
displayName: config.displayName,
196199
executable: try AbsolutePath(validating: config.executable),
197200
arguments: config.arguments,
198201
environment: config.environment,
199202
workingDirectory: try config.workingDirectory.map{ try AbsolutePath(validating: $0) },
200203
outputFilesDirectory: try AbsolutePath(validating: outputFilesDir))
201204

205+
if !success {
206+
exitEarly = true
207+
hasReportedError = true
208+
}
209+
202210
case .buildOperationRequest(let subset, let parameters):
203211
self.invocationDelegate.pluginRequestedBuildOperation(subset: .init(subset), parameters: .init(parameters)) {
204212
do {
@@ -264,10 +272,9 @@ extension PluginTarget {
264272
delegate: runnerDelegate) { result in
265273
dispatchPrecondition(condition: .onQueue(callbackQueue))
266274
completion(result.map { exitCode in
267-
// Return a result based on the exit code. If the plugin
268-
// exits with an error but hasn't already emitted an error,
269-
// we do so for it.
270-
let exitedCleanly = (exitCode == 0)
275+
// Return a result based on the exit code or the `exitEarly` parameter. If the plugin
276+
// exits with an error but hasn't already emitted an error, we do so for it.
277+
let exitedCleanly = (exitCode == 0) && !runnerDelegate.exitEarly
271278
if !exitedCleanly && !runnerDelegate.hasReportedError {
272279
delegate.pluginEmittedDiagnostic(
273280
.error("Plugin ended with exit code \(exitCode)")
@@ -366,6 +373,12 @@ extension PackageGraph {
366373
dict[name] = path
367374
}
368375
})
376+
let builtToolNames = accessibleTools.compactMap { (accTool) -> String? in
377+
if case .builtTool(let name, _) = accTool {
378+
return name
379+
}
380+
return nil
381+
}
369382

370383
// Determine additional input dependencies for any plugin commands, based on any executables the plugin target depends on.
371384
let toolPaths = toolNamesToPaths.values.sorted()
@@ -393,15 +406,17 @@ extension PackageGraph {
393406
let fileSystem: FileSystem
394407
let delegateQueue: DispatchQueue
395408
let toolPaths: [AbsolutePath]
409+
let builtToolNames: [String]
396410
var outputData = Data()
397411
var diagnostics = [Basics.Diagnostic]()
398412
var buildCommands = [BuildToolPluginInvocationResult.BuildCommand]()
399413
var prebuildCommands = [BuildToolPluginInvocationResult.PrebuildCommand]()
400414

401-
init(fileSystem: FileSystem, delegateQueue: DispatchQueue, toolPaths: [AbsolutePath]) {
415+
init(fileSystem: FileSystem, delegateQueue: DispatchQueue, toolPaths: [AbsolutePath], builtToolNames: [String]) {
402416
self.fileSystem = fileSystem
403417
self.delegateQueue = delegateQueue
404418
self.toolPaths = toolPaths
419+
self.builtToolNames = builtToolNames
405420
}
406421

407422
func pluginCompilationStarted(commandLine: [String], environment: EnvironmentVariables) {
@@ -436,12 +451,12 @@ extension PackageGraph {
436451
outputFiles: outputFiles))
437452
}
438453

439-
func pluginDefinedPrebuildCommand(displayName: String?, executable: AbsolutePath, arguments: [String], environment: [String : String], workingDirectory: AbsolutePath?, outputFilesDirectory: AbsolutePath) {
454+
func pluginDefinedPrebuildCommand(displayName: String?, executable: AbsolutePath, arguments: [String], environment: [String : String], workingDirectory: AbsolutePath?, outputFilesDirectory: AbsolutePath) -> Bool {
440455
dispatchPrecondition(condition: .onQueue(delegateQueue))
441456
// executable must exist before running prebuild command
442-
if !fileSystem.exists(executable) {
443-
diagnostics.append(.error("executable target '\(executable.basename)' is not pre-built; a plugin running a prebuild command should only rely on an existing binary; as a workaround, build '\(executable.basename)' first and then run the plugin "))
444-
return
457+
if builtToolNames.contains(executable.basename) {
458+
diagnostics.append(.error("a prebuild command cannot use executables built from source, including executable target '\(executable.basename)'"))
459+
return false
445460
}
446461
prebuildCommands.append(.init(
447462
configuration: .init(
@@ -451,9 +466,10 @@ extension PackageGraph {
451466
environment: environment,
452467
workingDirectory: workingDirectory),
453468
outputFilesDirectory: outputFilesDirectory))
469+
return true
454470
}
455471
}
456-
let delegate = PluginDelegate(fileSystem: fileSystem, delegateQueue: delegateQueue, toolPaths: toolPaths)
472+
let delegate = PluginDelegate(fileSystem: fileSystem, delegateQueue: delegateQueue, toolPaths: toolPaths, builtToolNames: builtToolNames)
457473

458474
// Invoke the build tool plugin with the input parameters and the delegate that will collect outputs.
459475
let startTime = DispatchTime.now()
@@ -655,7 +671,7 @@ public protocol PluginInvocationDelegate {
655671
func pluginDefinedBuildCommand(displayName: String?, executable: AbsolutePath, arguments: [String], environment: [String: String], workingDirectory: AbsolutePath?, inputFiles: [AbsolutePath], outputFiles: [AbsolutePath])
656672

657673
/// Called when a plugin defines a prebuild command through the PackagePlugin APIs.
658-
func pluginDefinedPrebuildCommand(displayName: String?, executable: AbsolutePath, arguments: [String], environment: [String: String], workingDirectory: AbsolutePath?, outputFilesDirectory: AbsolutePath)
674+
func pluginDefinedPrebuildCommand(displayName: String?, executable: AbsolutePath, arguments: [String], environment: [String: String], workingDirectory: AbsolutePath?, outputFilesDirectory: AbsolutePath) -> Bool
659675

660676
/// Called when a plugin requests a build operation through the PackagePlugin APIs.
661677
func pluginRequestedBuildOperation(subset: PluginInvocationBuildSubset, parameters: PluginInvocationBuildParameters, completion: @escaping (Result<PluginInvocationBuildResult, Error>) -> Void)
@@ -779,7 +795,8 @@ public struct PluginInvocationTestResult {
779795
public extension PluginInvocationDelegate {
780796
func pluginDefinedBuildCommand(displayName: String?, executable: AbsolutePath, arguments: [String], environment: [String : String], workingDirectory: AbsolutePath?, inputFiles: [AbsolutePath], outputFiles: [AbsolutePath]) {
781797
}
782-
func pluginDefinedPrebuildCommand(displayName: String?, executable: AbsolutePath, arguments: [String], environment: [String : String], workingDirectory: AbsolutePath?, outputFilesDirectory: AbsolutePath) {
798+
func pluginDefinedPrebuildCommand(displayName: String?, executable: AbsolutePath, arguments: [String], environment: [String : String], workingDirectory: AbsolutePath?, outputFilesDirectory: AbsolutePath) -> Bool {
799+
return true
783800
}
784801
func pluginRequestedBuildOperation(subset: PluginInvocationBuildSubset, parameters: PluginInvocationBuildParameters, completion: @escaping (Result<PluginInvocationBuildResult, Error>) -> Void) {
785802
DispatchQueue.sharedConcurrent.async { completion(Result.failure(StringError("unimplemented"))) }

Tests/SPMBuildCoreTests/PluginInvocationTests.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -893,7 +893,7 @@ class PluginInvocationTests: XCTestCase {
893893

894894
let diags = result.map{$0.value}.flatMap{$0}.map{$0.diagnostics}.flatMap{$0}
895895
testDiagnostics(diags) { result in
896-
let msg = "executable target 'Y' is not pre-built; a plugin running a prebuild command should only rely on an existing binary; as a workaround, build 'Y' first and then run the plugin"
896+
let msg = "a prebuild command cannot use executables built from source, including executable target 'Y'"
897897
result.check(diagnostic: .contains(msg), severity: .error)
898898
}
899899
}

0 commit comments

Comments
 (0)