Skip to content

Commit 1c68725

Browse files
elshelsh
authored
Add diags for non-existing binary used by prebuild plugin (#5834)
Add diags for non-existing binary used by prebuild plugin. Add a test to verify a package with a plugin target no longer requires a non-plugin target. Resolves rdar://90442392, rdar://86787091
1 parent f94e5f8 commit 1c68725

File tree

2 files changed

+211
-4
lines changed

2 files changed

+211
-4
lines changed

‎Sources/SPMBuildCore/PluginInvocation.swift

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,6 @@ fileprivate extension PluginToHostMessage {
288288
}
289289
}
290290

291-
292291
extension PackageGraph {
293292

294293
/// Traverses the graph of reachable targets in a package graph, and applies plugins to targets as needed. Each
@@ -381,14 +380,16 @@ extension PackageGraph {
381380
// Set up a delegate to handle callbacks from the build tool plugin. We'll capture free-form text output as well as defined commands and diagnostics.
382381
let delegateQueue = DispatchQueue(label: "plugin-invocation")
383382
class PluginDelegate: PluginInvocationDelegate {
383+
let fileSystem: FileSystem
384384
let delegateQueue: DispatchQueue
385385
let toolPaths: [AbsolutePath]
386386
var outputData = Data()
387387
var diagnostics = [Basics.Diagnostic]()
388388
var buildCommands = [BuildToolPluginInvocationResult.BuildCommand]()
389389
var prebuildCommands = [BuildToolPluginInvocationResult.PrebuildCommand]()
390390

391-
init(delegateQueue: DispatchQueue, toolPaths: [AbsolutePath]) {
391+
init(fileSystem: FileSystem, delegateQueue: DispatchQueue, toolPaths: [AbsolutePath]) {
392+
self.fileSystem = fileSystem
392393
self.delegateQueue = delegateQueue
393394
self.toolPaths = toolPaths
394395
}
@@ -427,6 +428,11 @@ extension PackageGraph {
427428

428429
func pluginDefinedPrebuildCommand(displayName: String?, executable: AbsolutePath, arguments: [String], environment: [String : String], workingDirectory: AbsolutePath?, outputFilesDirectory: AbsolutePath) {
429430
dispatchPrecondition(condition: .onQueue(delegateQueue))
431+
// executable must exist before running prebuild command
432+
if !fileSystem.exists(executable) {
433+
diagnostics.append(.error("exectuable 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 "))
434+
return
435+
}
430436
prebuildCommands.append(.init(
431437
configuration: .init(
432438
displayName: displayName,
@@ -437,7 +443,7 @@ extension PackageGraph {
437443
outputFilesDirectory: outputFilesDirectory))
438444
}
439445
}
440-
let delegate = PluginDelegate(delegateQueue: delegateQueue, toolPaths: toolPaths)
446+
let delegate = PluginDelegate(fileSystem: fileSystem, delegateQueue: delegateQueue, toolPaths: toolPaths)
441447

442448
// Invoke the build tool plugin with the input parameters and the delegate that will collect outputs.
443449
let startTime = DispatchTime.now()
@@ -613,7 +619,6 @@ public enum PluginEvaluationError: Swift.Error {
613619
case decodingPluginOutputFailed(json: Data, underlyingError: Error)
614620
}
615621

616-
617622
public protocol PluginInvocationDelegate {
618623
/// Called before a plugin is compiled. This call is always followed by a `pluginCompilationEnded()`, but is mutually exclusive with `pluginCompilationWasSkipped()` (which is called if the plugin didn't need to be recompiled).
619624
func pluginCompilationStarted(commandLine: [String], environment: EnvironmentVariables)

‎Tests/SPMBuildCoreTests/PluginInvocationTests.swift

Lines changed: 202 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -583,4 +583,206 @@ class PluginInvocationTests: XCTestCase {
583583
}
584584
}
585585
}
586+
587+
func testPrebuildPluginShouldNotUseExecTarget() throws {
588+
try testWithTemporaryDirectory { tmpPath in
589+
// Create a sample package with a library target and a plugin.
590+
let packageDir = tmpPath.appending(components: "mypkg")
591+
try localFileSystem.createDirectory(packageDir, recursive: true)
592+
try localFileSystem.writeFileContents(packageDir.appending(component: "Package.swift"), string: """
593+
// swift-tools-version:5.7
594+
595+
import PackageDescription
596+
597+
let package = Package(
598+
name: "mypkg",
599+
products: [
600+
.library(
601+
name: "MyLib",
602+
targets: ["MyLib"])
603+
],
604+
targets: [
605+
.target(
606+
name: "MyLib",
607+
plugins: [
608+
.plugin(name: "X")
609+
]),
610+
.plugin(
611+
name: "X",
612+
capability: .buildTool(),
613+
dependencies: [ "Y" ]
614+
),
615+
.executableTarget(
616+
name: "Y",
617+
dependencies: []),
618+
]
619+
)
620+
""")
621+
622+
let libTargetDir = packageDir.appending(components: "Sources", "MyLib")
623+
try localFileSystem.createDirectory(libTargetDir, recursive: true)
624+
try localFileSystem.writeFileContents(libTargetDir.appending(component: "file.swift"), string: """
625+
public struct MyUtilLib {
626+
public let strings: [String]
627+
public init(args: [String]) {
628+
self.strings = args
629+
}
630+
}
631+
""")
632+
633+
let depTargetDir = packageDir.appending(components: "Sources", "Y")
634+
try localFileSystem.createDirectory(depTargetDir, recursive: true)
635+
try localFileSystem.writeFileContents(depTargetDir.appending(component: "main.swift"), string: """
636+
struct Y {
637+
func run() {
638+
print("You passed us two arguments, argumentOne, and argumentTwo")
639+
}
640+
}
641+
Y.main()
642+
""")
643+
644+
let pluginTargetDir = packageDir.appending(components: "Plugins", "X")
645+
try localFileSystem.createDirectory(pluginTargetDir, recursive: true)
646+
try localFileSystem.writeFileContents(pluginTargetDir.appending(component: "plugin.swift"), string: """
647+
import PackagePlugin
648+
@main struct X: BuildToolPlugin {
649+
func createBuildCommands(context: PluginContext, target: Target) async throws -> [Command] {
650+
[
651+
Command.prebuildCommand(
652+
displayName: "X: Running Y before the build...",
653+
executable: try context.tool(named: "Y").path,
654+
arguments: [ "ARGUMENT_ONE", "ARGUMENT_TWO" ],
655+
outputFilesDirectory: context.pluginWorkDirectory.appending("OUTPUT_FILES_DIRECTORY")
656+
)
657+
]
658+
}
659+
660+
}
661+
""")
662+
663+
// Load a workspace from the package.
664+
let observability = ObservabilitySystem.makeForTesting()
665+
let workspace = try Workspace(
666+
fileSystem: localFileSystem,
667+
forRootPackage: packageDir,
668+
customManifestLoader: ManifestLoader(toolchain: UserToolchain.default),
669+
delegate: MockWorkspaceDelegate()
670+
)
671+
672+
// Load the root manifest.
673+
let rootInput = PackageGraphRootInput(packages: [packageDir], dependencies: [])
674+
let rootManifests = try tsc_await {
675+
workspace.loadRootManifests(
676+
packages: rootInput.packages,
677+
observabilityScope: observability.topScope,
678+
completion: $0
679+
)
680+
}
681+
XCTAssert(rootManifests.count == 1, "\(rootManifests)")
682+
683+
// Load the package graph.
684+
let packageGraph = try workspace.loadPackageGraph(rootInput: rootInput, observabilityScope: observability.topScope)
685+
XCTAssertNoDiagnostics(observability.diagnostics)
686+
XCTAssert(packageGraph.packages.count == 1, "\(packageGraph.packages)")
687+
688+
// Find the build tool plugin.
689+
let buildToolPlugin = try XCTUnwrap(packageGraph.packages[0].targets.map(\.underlyingTarget).filter{ $0.name == "X" }.first as? PluginTarget)
690+
XCTAssertEqual(buildToolPlugin.name, "X")
691+
XCTAssertEqual(buildToolPlugin.capability, .buildTool)
692+
693+
// Create a plugin script runner for the duration of the test.
694+
let pluginCacheDir = tmpPath.appending(component: "plugin-cache")
695+
let pluginScriptRunner = DefaultPluginScriptRunner(
696+
fileSystem: localFileSystem,
697+
cacheDir: pluginCacheDir,
698+
toolchain: try UserToolchain.default
699+
)
700+
701+
// Invoke build tool plugin
702+
do {
703+
let outputDir = packageDir.appending(component: ".build")
704+
let builtToolsDir = outputDir.appending(component: "debug")
705+
let result = try packageGraph.invokeBuildToolPlugins(
706+
outputDir: outputDir,
707+
builtToolsDir: builtToolsDir,
708+
buildEnvironment: BuildEnvironment(platform: .macOS, configuration: .debug),
709+
toolSearchDirectories: [UserToolchain.default.swiftCompilerPath.parentDirectory],
710+
pluginScriptRunner: pluginScriptRunner,
711+
observabilityScope: observability.topScope,
712+
fileSystem: localFileSystem
713+
)
714+
715+
let diags = result.map{$0.value}.flatMap{$0}.map{$0.diagnostics}.flatMap{$0}
716+
testDiagnostics(diags) { result in
717+
let msg = "exectuable 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"
718+
result.check(diagnostic: .contains(msg), severity: .error)
719+
}
720+
}
721+
}
722+
}
723+
724+
func testShouldNotRequireNonPluginTarget() throws {
725+
try testWithTemporaryDirectory { tmpPath in
726+
// Create a sample package with a library target and a plugin.
727+
let packageDir = tmpPath.appending(components: "MyPackage")
728+
try localFileSystem.createDirectory(packageDir, recursive: true)
729+
try localFileSystem.writeFileContents(packageDir.appending(component: "Package.swift"), string: """
730+
// swift-tools-version: 5.7
731+
import PackageDescription
732+
let package = Package(
733+
name: "MyPackage",
734+
products: [
735+
.plugin(name: "Foo", targets: ["Foo"])
736+
],
737+
dependencies: [
738+
],
739+
targets: [
740+
.plugin(
741+
name: "Foo",
742+
capability: .command(
743+
intent: .custom(verb: "Foo", description: "Plugin example"),
744+
permissions: []
745+
)
746+
)
747+
]
748+
)
749+
""")
750+
751+
let myPluginTargetDir = packageDir.appending(components: "Plugins", "Foo")
752+
try localFileSystem.createDirectory(myPluginTargetDir, recursive: true)
753+
try localFileSystem.writeFileContents(myPluginTargetDir.appending(component: "plugin.swift"), string: """
754+
import PackagePlugin
755+
@main struct FooPlugin: BuildToolPlugin {
756+
func createBuildCommands(
757+
context: PluginContext,
758+
target: Target
759+
) throws -> [Command] { }
760+
}
761+
""")
762+
763+
// Load a workspace from the package.
764+
let observability = ObservabilitySystem.makeForTesting()
765+
let workspace = try Workspace(
766+
fileSystem: localFileSystem,
767+
forRootPackage: packageDir,
768+
customManifestLoader: ManifestLoader(toolchain: UserToolchain.default),
769+
delegate: MockWorkspaceDelegate()
770+
)
771+
772+
// Load the root manifest.
773+
let rootInput = PackageGraphRootInput(packages: [packageDir], dependencies: [])
774+
let rootManifests = try tsc_await {
775+
workspace.loadRootManifests(
776+
packages: rootInput.packages,
777+
observabilityScope: observability.topScope,
778+
completion: $0
779+
)
780+
}
781+
XCTAssert(rootManifests.count == 1, "\(rootManifests)")
782+
783+
// Load the package graph.
784+
let _ = try workspace.loadPackageGraph(rootInput: rootInput, observabilityScope: observability.topScope)
785+
XCTAssertNoDiagnostics(observability.diagnostics)
786+
}
787+
}
586788
}

0 commit comments

Comments
 (0)