From 847a73b0da4bfceaa1f0328ad48dba14e7154306 Mon Sep 17 00:00:00 2001 From: Euan Harris Date: Wed, 17 Jan 2024 15:36:51 +0000 Subject: [PATCH 1/2] Tests: Rename CommandPluginDiagrosticsStub to be a generic test fixture --- .../Package.swift | 0 .../Plugins/diagnostics-stub}/diagnostics_stub.swift | 0 .../Sources/main.swift | 0 Tests/CommandsTests/PackageToolTests.swift | 2 +- 4 files changed, 1 insertion(+), 1 deletion(-) rename Fixtures/Miscellaneous/Plugins/{CommandPluginDiagnosticsStub => CommandPluginTestStub}/Package.swift (100%) rename Fixtures/Miscellaneous/Plugins/{CommandPluginDiagnosticsStub/Plugins => CommandPluginTestStub/Plugins/diagnostics-stub}/diagnostics_stub.swift (100%) rename Fixtures/Miscellaneous/Plugins/{CommandPluginDiagnosticsStub => CommandPluginTestStub}/Sources/main.swift (100%) diff --git a/Fixtures/Miscellaneous/Plugins/CommandPluginDiagnosticsStub/Package.swift b/Fixtures/Miscellaneous/Plugins/CommandPluginTestStub/Package.swift similarity index 100% rename from Fixtures/Miscellaneous/Plugins/CommandPluginDiagnosticsStub/Package.swift rename to Fixtures/Miscellaneous/Plugins/CommandPluginTestStub/Package.swift diff --git a/Fixtures/Miscellaneous/Plugins/CommandPluginDiagnosticsStub/Plugins/diagnostics_stub.swift b/Fixtures/Miscellaneous/Plugins/CommandPluginTestStub/Plugins/diagnostics-stub/diagnostics_stub.swift similarity index 100% rename from Fixtures/Miscellaneous/Plugins/CommandPluginDiagnosticsStub/Plugins/diagnostics_stub.swift rename to Fixtures/Miscellaneous/Plugins/CommandPluginTestStub/Plugins/diagnostics-stub/diagnostics_stub.swift diff --git a/Fixtures/Miscellaneous/Plugins/CommandPluginDiagnosticsStub/Sources/main.swift b/Fixtures/Miscellaneous/Plugins/CommandPluginTestStub/Sources/main.swift similarity index 100% rename from Fixtures/Miscellaneous/Plugins/CommandPluginDiagnosticsStub/Sources/main.swift rename to Fixtures/Miscellaneous/Plugins/CommandPluginTestStub/Sources/main.swift diff --git a/Tests/CommandsTests/PackageToolTests.swift b/Tests/CommandsTests/PackageToolTests.swift index 6ea628d70f7..b66c69f0553 100644 --- a/Tests/CommandsTests/PackageToolTests.swift +++ b/Tests/CommandsTests/PackageToolTests.swift @@ -1890,7 +1890,7 @@ final class PackageToolTests: CommandsTestCase { let containsWarning = StringPattern.contains("command plugin: Diagnostics.warning") let containsError = StringPattern.contains("command plugin: Diagnostics.error") - try fixture(name: "Miscellaneous/Plugins/CommandPluginDiagnosticsStub") { fixturePath in + try fixture(name: "Miscellaneous/Plugins/CommandPluginTestStub") { fixturePath in func runPlugin(flags: [String], diagnostics: [String], completion: (String, String) -> Void) throws { let (stdout, stderr) = try SwiftPM.Package.execute(flags + ["print-diagnostics"] + diagnostics, packagePath: fixturePath) completion(stdout, stderr) From 8656b7cd7db211cc9eaee0141bd4d6d91bb6d72a Mon Sep 17 00:00:00 2001 From: Euan Harris Date: Tue, 16 Jan 2024 14:51:03 +0000 Subject: [PATCH 2/2] command plugins: Inherit SwiftPM's --configuration flag in packageManager.build This commit makes it possible for a build run on behalf of a command plugin to inherit the build configuration (debug or release) set for the whole `swift package` run using the `--configuration` flag. ### Motivation: When a command plugin asks for a target to be built by calling packageManager.build, it must specify a release or debug build. If no configuration is given, debug is the default. This overrides any configuration specified by the user with `swift package -c `. A command plugin might often be used as an alterative entry point to Swift PM, responsible for building a target and then processing it in some way to generate the final product. The user might run `swift package -c release some-plugin --target some-target`, expecting a release build of target to be made, but currently the result will be a debug binary if the plugin uses the default options. ### Modifications: * Added a new `.inherit` option for packageManager.build's `configuration` argument.; .debug remains the default, as before. * Added a test to verify that plugin-initiated builds are of the correct type. ### Result: A command plugin can request a target build matching the configuration specified on the SwiftPM command line. The default is still to make a debug build, however in future we might change this to inherit the overall SwiftPM configuration. ### Alternatives: A command plugin does not currently seem to have access to SwiftPM's build configuration. We could pass this information to the plugin, allowing the plugin author to pass it back in the packageManager.build() call. This would be a less invasive change to SwiftPM, however the approach in this commit makes it easier to change the default to .inherit in the future. --- .../CommandPluginTestStub/Package.swift | 7 +++ .../targetbuild-stub/targetbuild_stub.swift | 25 ++++++++ .../Commands/Utilities/PluginDelegate.swift | 4 ++ .../PackagePlugin/PackageManagerProxy.swift | 4 +- Sources/PackagePlugin/PluginMessages.swift | 2 +- .../Plugins/PluginInvocation.swift | 4 +- Tests/CommandsTests/PackageToolTests.swift | 62 +++++++++++++++++++ 7 files changed, 105 insertions(+), 3 deletions(-) create mode 100644 Fixtures/Miscellaneous/Plugins/CommandPluginTestStub/Plugins/targetbuild-stub/targetbuild_stub.swift diff --git a/Fixtures/Miscellaneous/Plugins/CommandPluginTestStub/Package.swift b/Fixtures/Miscellaneous/Plugins/CommandPluginTestStub/Package.swift index 716c2fce219..fe69a01f0b5 100644 --- a/Fixtures/Miscellaneous/Plugins/CommandPluginTestStub/Package.swift +++ b/Fixtures/Miscellaneous/Plugins/CommandPluginTestStub/Package.swift @@ -12,6 +12,13 @@ let package = Package( description: "Writes diagnostic messages for testing" )) ), + .plugin( + name: "targetbuild-stub", + capability: .command(intent: .custom( + verb: "build-target", + description: "Build a target for testing" + )) + ), .executableTarget( name: "placeholder" ), diff --git a/Fixtures/Miscellaneous/Plugins/CommandPluginTestStub/Plugins/targetbuild-stub/targetbuild_stub.swift b/Fixtures/Miscellaneous/Plugins/CommandPluginTestStub/Plugins/targetbuild-stub/targetbuild_stub.swift new file mode 100644 index 00000000000..84d07337066 --- /dev/null +++ b/Fixtures/Miscellaneous/Plugins/CommandPluginTestStub/Plugins/targetbuild-stub/targetbuild_stub.swift @@ -0,0 +1,25 @@ +import Foundation +import PackagePlugin + +@main +struct targetbuild_stub: CommandPlugin { + // This is a helper for testing target builds performed on behalf of plugins. + // It sends asks SwiftPM to build a target with different options depending on its arguments. + func performCommand(context: PluginContext, arguments: [String]) async throws { + // Build a target + var parameters = PackageManager.BuildParameters() + if arguments.contains("build-debug") { + parameters.configuration = .debug + } else if arguments.contains("build-release") { + parameters.configuration = .release + } else if arguments.contains("build-inherit") { + parameters.configuration = .inherit + } + // If no 'build-*' argument is present, the default (.debug) will be used. + + let _ = try packageManager.build( + .product("placeholder"), + parameters: parameters + ) + } +} \ No newline at end of file diff --git a/Sources/Commands/Utilities/PluginDelegate.swift b/Sources/Commands/Utilities/PluginDelegate.swift index fb87deef308..e98cff936ca 100644 --- a/Sources/Commands/Utilities/PluginDelegate.swift +++ b/Sources/Commands/Utilities/PluginDelegate.swift @@ -77,6 +77,10 @@ final class PluginDelegate: PluginInvocationDelegate { buildParameters.configuration = .debug case .release: buildParameters.configuration = .release + case .inherit: + // The top level argument parser set buildParameters.configuration according to the + // --configuration command line parameter. We don't need to do anything to inherit it. + break } buildParameters.flags.cCompilerFlags.append(contentsOf: parameters.otherCFlags) buildParameters.flags.cxxCompilerFlags.append(contentsOf: parameters.otherCxxFlags) diff --git a/Sources/PackagePlugin/PackageManagerProxy.swift b/Sources/PackagePlugin/PackageManagerProxy.swift index 61116602482..5d7ee53ee99 100644 --- a/Sources/PackagePlugin/PackageManagerProxy.swift +++ b/Sources/PackagePlugin/PackageManagerProxy.swift @@ -81,7 +81,7 @@ public struct PackageManager { /// Represents an overall purpose of the build, which affects such things /// as optimization and generation of debug symbols. public enum BuildConfiguration: String { - case debug, release + case debug, release, inherit } /// Represents the amount of detail in a build log. @@ -328,6 +328,8 @@ fileprivate extension PluginToHostMessage.BuildParameters.Configuration { self = .debug case .release: self = .release + case .inherit: + self = .inherit } } } diff --git a/Sources/PackagePlugin/PluginMessages.swift b/Sources/PackagePlugin/PluginMessages.swift index 5e4bf372aed..be17ed0bd19 100644 --- a/Sources/PackagePlugin/PluginMessages.swift +++ b/Sources/PackagePlugin/PluginMessages.swift @@ -297,7 +297,7 @@ enum PluginToHostMessage: Codable { struct BuildParameters: Codable { var configuration: Configuration enum Configuration: String, Codable { - case debug, release + case debug, release, inherit } var logging: LogVerbosity enum LogVerbosity: String, Codable { diff --git a/Sources/SPMBuildCore/Plugins/PluginInvocation.swift b/Sources/SPMBuildCore/Plugins/PluginInvocation.swift index c6a7391c174..f6c92fc834d 100644 --- a/Sources/SPMBuildCore/Plugins/PluginInvocation.swift +++ b/Sources/SPMBuildCore/Plugins/PluginInvocation.swift @@ -861,7 +861,7 @@ public enum PluginInvocationBuildSubset { public struct PluginInvocationBuildParameters { public var configuration: Configuration public enum Configuration: String { - case debug, release + case debug, release, inherit } public var logging: LogVerbosity public enum LogVerbosity: String { @@ -993,6 +993,8 @@ fileprivate extension PluginInvocationBuildParameters.Configuration { self = .debug case .release: self = .release + case .inherit: + self = .inherit } } } diff --git a/Tests/CommandsTests/PackageToolTests.swift b/Tests/CommandsTests/PackageToolTests.swift index b66c69f0553..4e770167975 100644 --- a/Tests/CommandsTests/PackageToolTests.swift +++ b/Tests/CommandsTests/PackageToolTests.swift @@ -1987,6 +1987,68 @@ final class PackageToolTests: CommandsTestCase { } } + // Test target builds requested by a command plugin + func testCommandPluginTargetBuilds() throws { + // Only run the test if the environment in which we're running actually supports Swift concurrency (which the plugin APIs require). + try XCTSkipIf(!UserToolchain.default.supportsSwiftConcurrency(), "skipping because test environment doesn't support concurrency") + + let debugTarget = [".build", "debug", "placeholder"] + let releaseTarget = [".build", "release", "placeholder"] + + func AssertIsExecutableFile(_ fixturePath: AbsolutePath, file: StaticString = #filePath, line: UInt = #line) { + XCTAssert( + localFileSystem.isExecutableFile(fixturePath), + "\(fixturePath) does not exist", + file: file, + line: line + ) + } + + func AssertNotExists(_ fixturePath: AbsolutePath, file: StaticString = #filePath, line: UInt = #line) { + XCTAssertFalse( + localFileSystem.exists(fixturePath), + "\(fixturePath) should not exist", + file: file, + line: line + ) + } + + // By default, a plugin-requested build produces a debug binary + try fixture(name: "Miscellaneous/Plugins/CommandPluginTestStub") { fixturePath in + let _ = try SwiftPM.Package.execute(["-c", "release", "build-target"], packagePath: fixturePath) + AssertIsExecutableFile(fixturePath.appending(components: debugTarget)) + AssertNotExists(fixturePath.appending(components: releaseTarget)) + } + + // If the plugin specifies a debug binary, that is what will be built, regardless of overall configuration + try fixture(name: "Miscellaneous/Plugins/CommandPluginTestStub") { fixturePath in + let _ = try SwiftPM.Package.execute(["-c", "release", "build-target", "build-debug"], packagePath: fixturePath) + AssertIsExecutableFile(fixturePath.appending(components: debugTarget)) + AssertNotExists(fixturePath.appending(components: releaseTarget)) + } + + // If the plugin requests a release binary, that is what will be built, regardless of overall configuration + try fixture(name: "Miscellaneous/Plugins/CommandPluginTestStub") { fixturePath in + let _ = try SwiftPM.Package.execute(["-c", "debug", "build-target", "build-release"], packagePath: fixturePath) + AssertNotExists(fixturePath.appending(components: debugTarget)) + AssertIsExecutableFile(fixturePath.appending(components: releaseTarget)) + } + + // If the plugin inherits the overall build configuration, that is what will be built + try fixture(name: "Miscellaneous/Plugins/CommandPluginTestStub") { fixturePath in + let _ = try SwiftPM.Package.execute(["-c", "debug", "build-target", "build-inherit"], packagePath: fixturePath) + AssertIsExecutableFile(fixturePath.appending(components: debugTarget)) + AssertNotExists(fixturePath.appending(components: releaseTarget)) + } + + // If the plugin inherits the overall build configuration, that is what will be built + try fixture(name: "Miscellaneous/Plugins/CommandPluginTestStub") { fixturePath in + let _ = try SwiftPM.Package.execute(["-c", "release", "build-target", "build-inherit"], packagePath: fixturePath) + AssertNotExists(fixturePath.appending(components: debugTarget)) + AssertIsExecutableFile(fixturePath.appending(components: releaseTarget)) + } + } + func testCommandPluginNetworkingPermissions(permissionsManifestFragment: String, permissionError: String, reason: String, remedy: [String]) throws { // Only run the test if the environment in which we're running actually supports Swift concurrency (which the plugin APIs require). try XCTSkipIf(!UserToolchain.default.supportsSwiftConcurrency(), "skipping because test environment doesn't support concurrency")