Skip compiling plugins when skipping building (#6696)

neonichu · MaxDesiatov · web-flow · commit dd76ad49271f · 2023-07-31T15:51:45.000-07:00
Sink `shouldSkipBuilding` down into build parameters, so we can respect it whereever we would be trying to build, e.g. when trying to build plugins during build planning. resolves #6683 Co-authored-by: Max Desiatov <m_desiatov@apple.com>
diff --git a/Fixtures/Miscellaneous/TestDiscovery/Simple/Package.swift b/Fixtures/Miscellaneous/TestDiscovery/Simple/Package.swift
@@ -1,10 +1,11 @@
-// swift-tools-version:4.2
+// swift-tools-version:5.8
 import PackageDescription
 
 let package = Package(
     name: "Simple",
     targets: [
-        .target(name: "Simple"),
+        .target(name: "Simple", plugins: ["SimplePlugin"]),
         .testTarget(name: "SimpleTests", dependencies: ["Simple"]),
+        .plugin(name: "SimplePlugin", capability: .buildTool()),
     ]
 )
diff --git a/Fixtures/Miscellaneous/TestDiscovery/Simple/Plugins/SimplePlugin/plugin.swift b/Fixtures/Miscellaneous/TestDiscovery/Simple/Plugins/SimplePlugin/plugin.swift
@@ -0,0 +1,8 @@
+import PackagePlugin
+
+@main
+struct Simple: BuildToolPlugin {
+    func createBuildCommands(context: PackagePlugin.PluginContext, target: PackagePlugin.Target) async throws -> [PackagePlugin.Command] {
+        return []
+    }
+}
diff --git a/Sources/Build/BuildOperation.swift b/Sources/Build/BuildOperation.swift
@@ -237,6 +237,9 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
 
     /// Perform a build using the given build description and subset.
     public func build(subset: BuildSubset) throws {
+        guard !buildParameters.shouldSkipBuilding else {
+            return
+        }
 
         let buildStartTime = DispatchTime.now()
 
@@ -422,7 +425,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
         let buildToolPluginInvocationResults: [ResolvedTarget: [BuildToolPluginInvocationResult]]
         let prebuildCommandResults: [ResolvedTarget: [PrebuildCommandResult]]
         // Invoke any build tool plugins in the graph to generate prebuild commands and build commands.
-        if let pluginConfiguration {
+        if let pluginConfiguration, !self.buildParameters.shouldSkipBuilding {
             let buildOperationForPluginDependencies = try BuildOperation(
                 buildParameters: self.buildParameters.forTriple(self.buildParameters.hostTriple),
                 cacheBuildManifest: false,
diff --git a/Sources/Commands/SwiftTestTool.swift b/Sources/Commands/SwiftTestTool.swift
@@ -187,7 +187,7 @@ public struct SwiftTestTool: SwiftCommand {
         } else if !self.options.shouldRunInParallel {
             let toolchain = try swiftTool.getTargetToolchain()
             let testProducts = try buildTestsIfNeeded(swiftTool: swiftTool)
-            let buildParameters = try swiftTool.buildParametersForTest(options: self.options)
+            let buildParameters = try swiftTool.buildParametersForTest(options: self.options, sharedOptions: self.sharedOptions)
 
             // Clean out the code coverage directory that may contain stale
             // profraw files from a previous run of the code coverage tool.
@@ -216,6 +216,7 @@ public struct SwiftTestTool: SwiftCommand {
                     in: testProducts,
                     swiftTool: swiftTool,
                     enableCodeCoverage: options.enableCodeCoverage,
+                    shouldSkipBuilding: sharedOptions.shouldSkipBuilding,
                     sanitizers: globalOptions.build.sanitizers
                 )
                 let tests = try testSuites
@@ -267,12 +268,13 @@ public struct SwiftTestTool: SwiftCommand {
                 in: testProducts,
                 swiftTool: swiftTool,
                 enableCodeCoverage: options.enableCodeCoverage,
+                shouldSkipBuilding: sharedOptions.shouldSkipBuilding,
                 sanitizers: globalOptions.build.sanitizers
             )
             let tests = try testSuites
                 .filteredTests(specifier: options.testCaseSpecifier)
                 .skippedTests(specifier: options.skippedTests(fileSystem: swiftTool.fileSystem))
-            let buildParameters = try swiftTool.buildParametersForTest(options: self.options)
+            let buildParameters = try swiftTool.buildParametersForTest(options: self.options, sharedOptions: self.sharedOptions)
 
             // If there were no matches, emit a warning and exit.
             if tests.isEmpty {
@@ -338,7 +340,7 @@ public struct SwiftTestTool: SwiftCommand {
         // Merge all the profraw files to produce a single profdata file.
         try mergeCodeCovRawDataFiles(swiftTool: swiftTool)
 
-        let buildParameters = try swiftTool.buildParametersForTest(options: self.options)
+        let buildParameters = try swiftTool.buildParametersForTest(options: self.options, sharedOptions: self.sharedOptions)
         for product in testProducts {
             // Export the codecov data as JSON.
             let jsonPath = buildParameters.codeCovAsJSONPath(packageName: rootManifest.displayName)
@@ -352,7 +354,7 @@ public struct SwiftTestTool: SwiftCommand {
         let llvmProf = try swiftTool.getTargetToolchain().getLLVMProf()
 
         // Get the profraw files.
-        let buildParameters = try swiftTool.buildParametersForTest(options: self.options)
+        let buildParameters = try swiftTool.buildParametersForTest(options: self.options, sharedOptions: self.sharedOptions)
         let codeCovFiles = try swiftTool.fileSystem.getDirectoryContents(buildParameters.codeCovPath)
 
         // Construct arguments for invoking the llvm-prof tool.
@@ -372,7 +374,7 @@ public struct SwiftTestTool: SwiftCommand {
     private func exportCodeCovAsJSON(to path: AbsolutePath, testBinary: AbsolutePath, swiftTool: SwiftTool) throws {
         // Export using the llvm-cov tool.
         let llvmCov = try swiftTool.getTargetToolchain().getLLVMCov()
-        let buildParameters = try swiftTool.buildParametersForTest(options: self.options)
+        let buildParameters = try swiftTool.buildParametersForTest(options: self.options, sharedOptions: self.sharedOptions)
         let args = [
             llvmCov.pathString,
             "export",
@@ -392,13 +394,11 @@ public struct SwiftTestTool: SwiftCommand {
     ///
     /// - Returns: The paths to the build test products.
     private func buildTestsIfNeeded(swiftTool: SwiftTool) throws -> [BuiltTestProduct] {
-        let buildParameters = try swiftTool.buildParametersForTest(options: self.options)
+        let buildParameters = try swiftTool.buildParametersForTest(options: self.options, sharedOptions: self.sharedOptions)
         let buildSystem = try swiftTool.createBuildSystem(customBuildParameters: buildParameters)
 
-        if !self.sharedOptions.shouldSkipBuilding {
-            let subset = self.sharedOptions.testProduct.map(BuildSubset.product) ?? .allIncludingTests
-            try buildSystem.build(subset: subset)
-        }
+        let subset = self.sharedOptions.testProduct.map(BuildSubset.product) ?? .allIncludingTests
+        try buildSystem.build(subset: subset)
 
         // Find the test product.
         let testProducts = buildSystem.builtTestProducts
@@ -498,6 +498,7 @@ extension SwiftTestTool {
                 in: testProducts,
                 swiftTool: swiftTool,
                 enableCodeCoverage: false,
+                shouldSkipBuilding: sharedOptions.shouldSkipBuilding,
                 sanitizers: globalOptions.build.sanitizers
             )
 
@@ -508,13 +509,11 @@ extension SwiftTestTool {
         }
 
         private func buildTestsIfNeeded(swiftTool: SwiftTool) throws -> [BuiltTestProduct] {
-            let buildParameters = try swiftTool.buildParametersForTest(enableCodeCoverage: false)
+            let buildParameters = try swiftTool.buildParametersForTest(enableCodeCoverage: false, shouldSkipBuilding: sharedOptions.shouldSkipBuilding)
             let buildSystem = try swiftTool.createBuildSystem(customBuildParameters: buildParameters)
 
-            if !self.sharedOptions.shouldSkipBuilding {
-                let subset = self.sharedOptions.testProduct.map(BuildSubset.product) ?? .allIncludingTests
-                try buildSystem.build(subset: subset)
-            }
+            let subset = self.sharedOptions.testProduct.map(BuildSubset.product) ?? .allIncludingTests
+            try buildSystem.build(subset: subset)
 
             // Find the test product.
             let testProducts = buildSystem.builtTestProducts
@@ -1041,10 +1040,11 @@ final class XUnitGenerator {
 }
 
 extension SwiftTool {
-    func buildParametersForTest(options: TestToolOptions) throws -> BuildParameters {
+    func buildParametersForTest(options: TestToolOptions, sharedOptions: SharedOptions) throws -> BuildParameters {
         try self.buildParametersForTest(
             enableCodeCoverage: options.enableCodeCoverage,
-            enableTestability: options.enableTestableImports
+            enableTestability: options.enableTestableImports,
+            shouldSkipBuilding: sharedOptions.shouldSkipBuilding
         )
     }
 }
diff --git a/Sources/Commands/Utilities/PluginDelegate.swift b/Sources/Commands/Utilities/PluginDelegate.swift
@@ -198,6 +198,7 @@ final class PluginDelegate: PluginInvocationDelegate {
                 fromTestAt: testProduct.bundlePath,
                 swiftTool: swiftTool,
                 enableCodeCoverage: parameters.enableCodeCoverage,
+                shouldSkipBuilding: false,
                 sanitizers: swiftTool.options.build.sanitizers
             )
             for testSuite in testSuites {
diff --git a/Sources/Commands/Utilities/TestingSupport.swift b/Sources/Commands/Utilities/TestingSupport.swift
@@ -62,14 +62,21 @@ enum TestingSupport {
         throw InternalError("XCTestHelper binary not found, tried \(triedPaths.map { $0.pathString }.joined(separator: ", "))")
     }
 
-    static func getTestSuites(in testProducts: [BuiltTestProduct], swiftTool: SwiftTool, enableCodeCoverage: Bool, sanitizers: [Sanitizer]) throws -> [AbsolutePath: [TestSuite]] {
+    static func getTestSuites(
+        in testProducts: [BuiltTestProduct],
+        swiftTool: SwiftTool,
+        enableCodeCoverage: Bool,
+        shouldSkipBuilding: Bool,
+        sanitizers: [Sanitizer]
+    ) throws -> [AbsolutePath: [TestSuite]] {
         let testSuitesByProduct = try testProducts
             .map {(
                 $0.bundlePath,
                 try Self.getTestSuites(
                     fromTestAt: $0.bundlePath,
                     swiftTool: swiftTool,
                     enableCodeCoverage: enableCodeCoverage,
+                    shouldSkipBuilding: shouldSkipBuilding,
                     sanitizers: sanitizers
                 )
             )}
@@ -86,15 +93,22 @@ enum TestingSupport {
     /// - Throws: TestError, SystemError, TSCUtility.Error
     ///
     /// - Returns: Array of TestSuite
-    static func getTestSuites(fromTestAt path: AbsolutePath, swiftTool: SwiftTool, enableCodeCoverage: Bool, sanitizers: [Sanitizer]) throws -> [TestSuite] {
+    static func getTestSuites(
+        fromTestAt path: AbsolutePath,
+        swiftTool: SwiftTool,
+        enableCodeCoverage: Bool,
+        shouldSkipBuilding: Bool,
+        sanitizers: [Sanitizer]
+    ) throws -> [TestSuite] {
         // Run the correct tool.
         #if os(macOS)
         let data: String = try withTemporaryFile { tempFile in
             let args = [try Self.xctestHelperPath(swiftTool: swiftTool).pathString, path.pathString, tempFile.path.pathString]
             var env = try Self.constructTestEnvironment(
                 toolchain: try swiftTool.getTargetToolchain(),
                 buildParameters: swiftTool.buildParametersForTest(
-                    enableCodeCoverage: enableCodeCoverage
+                    enableCodeCoverage: enableCodeCoverage,
+                    shouldSkipBuilding: shouldSkipBuilding
                 ),
                 sanitizers: sanitizers
             )
@@ -113,7 +127,8 @@ enum TestingSupport {
         let env = try Self.constructTestEnvironment(
             toolchain: try swiftTool.getTargetToolchain(),
             buildParameters: swiftTool.buildParametersForTest(
-                enableCodeCoverage: enableCodeCoverage
+                enableCodeCoverage: enableCodeCoverage,
+                shouldSkipBuilding: shouldSkipBuilding
             ),
             sanitizers: sanitizers
         )
@@ -184,13 +199,15 @@ enum TestingSupport {
 extension SwiftTool {
     func buildParametersForTest(
         enableCodeCoverage: Bool,
-        enableTestability: Bool? = nil
+        enableTestability: Bool? = nil,
+        shouldSkipBuilding: Bool = false
     ) throws -> BuildParameters {
         var parameters = try self.buildParameters()
         parameters.enableCodeCoverage = enableCodeCoverage
         // for test commands, we normally enable building with testability
         // but we let users override this with a flag
         parameters.enableTestability = enableTestability ?? true
+        parameters.shouldSkipBuilding = shouldSkipBuilding
         return parameters
     }
 }
diff --git a/Sources/SPMBuildCore/BuildParameters.swift b/Sources/SPMBuildCore/BuildParameters.swift
@@ -270,6 +270,8 @@ public struct BuildParameters: Encodable {
 
     public var debugInfoFormat: DebugInfoFormat
 
+    public var shouldSkipBuilding: Bool
+
     @available(*, deprecated, message: "use `init` overload with `targetTriple` parameter name instead")
     @_disfavoredOverload
     public init(
@@ -302,7 +304,8 @@ public struct BuildParameters: Encodable {
         colorizedOutput: Bool = false,
         verboseOutput: Bool = false,
         linkTimeOptimizationMode: LinkTimeOptimizationMode? = nil,
-        debugInfoFormat: DebugInfoFormat = .dwarf
+        debugInfoFormat: DebugInfoFormat = .dwarf,
+        shouldSkipBuilding: Bool = false
     ) throws {
         try self.init(
             dataPath: dataPath,
@@ -334,7 +337,8 @@ public struct BuildParameters: Encodable {
             colorizedOutput: colorizedOutput,
             verboseOutput: verboseOutput,
             linkTimeOptimizationMode: linkTimeOptimizationMode,
-            debugInfoFormat: debugInfoFormat
+            debugInfoFormat: debugInfoFormat,
+            shouldSkipBuilding: shouldSkipBuilding
         )
     }
 
@@ -368,7 +372,8 @@ public struct BuildParameters: Encodable {
         colorizedOutput: Bool = false,
         verboseOutput: Bool = false,
         linkTimeOptimizationMode: LinkTimeOptimizationMode? = nil,
-        debugInfoFormat: DebugInfoFormat = .dwarf
+        debugInfoFormat: DebugInfoFormat = .dwarf,
+        shouldSkipBuilding: Bool = false
     ) throws {
         let targetTriple = try targetTriple ?? .getHostTriple(usingSwiftCompiler: toolchain.swiftCompilerPath)
 
@@ -439,6 +444,7 @@ public struct BuildParameters: Encodable {
         self.verboseOutput = verboseOutput
         self.linkTimeOptimizationMode = linkTimeOptimizationMode
         self.debugInfoFormat = debugInfoFormat
+        self.shouldSkipBuilding = shouldSkipBuilding
     }
 
     @available(*, deprecated, renamed: "forTriple()")
@@ -487,7 +493,8 @@ public struct BuildParameters: Encodable {
             linkerDeadStrip: self.linkerDeadStrip,
             colorizedOutput: self.colorizedOutput,
             verboseOutput: self.verboseOutput,
-            linkTimeOptimizationMode: self.linkTimeOptimizationMode
+            linkTimeOptimizationMode: self.linkTimeOptimizationMode,
+            shouldSkipBuilding: self.shouldSkipBuilding
         )
     }
 
diff --git a/Sources/XCBuildSupport/XcodeBuildSystem.swift b/Sources/XCBuildSupport/XcodeBuildSystem.swift
@@ -103,6 +103,10 @@ public final class XcodeBuildSystem: SPMBuildCore.BuildSystem {
     }
 
     public func build(subset: BuildSubset) throws {
+        guard !buildParameters.shouldSkipBuilding else {
+            return
+        }
+
         let pifBuilder = try getPIFBuilder()
         let pif = try pifBuilder.generatePIF()
         try self.fileSystem.writeIfChanged(path: buildParameters.pifManifest, string: pif)

Original file line number	Diff line number	Diff line change
`@@ -1,10 +1,11 @@`
`1`		`-// swift-tools-version:4.2`
	`1`	`+// swift-tools-version:5.8`
`2`	`2`	`import PackageDescription`
`3`	`3`
`4`	`4`	`let package = Package(`
`5`	`5`	`name: "Simple",`
`6`	`6`	`targets: [`
`7`		`- .target(name: "Simple"),`
	`7`	`+ .target(name: "Simple", plugins: ["SimplePlugin"]),`
`8`	`8`	`.testTarget(name: "SimpleTests", dependencies: ["Simple"]),`
	`9`	`+ .plugin(name: "SimplePlugin", capability: .buildTool()),`
`9`	`10`	`]`
`10`	`11`	`)`
Original file line number	Diff line number	Diff line change
`@@ -198,6 +198,7 @@ final class PluginDelegate: PluginInvocationDelegate {`
`198`	`198`	`fromTestAt: testProduct.bundlePath,`
`199`	`199`	`swiftTool: swiftTool,`
`200`	`200`	`enableCodeCoverage: parameters.enableCodeCoverage,`
	`201`	`+ shouldSkipBuilding: false,`
`201`	`202`	`sanitizers: swiftTool.options.build.sanitizers`
`202`	`203`	`)`
`203`	`204`	`for testSuite in testSuites {`