[Build/Commands/PackageGraph] Remove ModulesGraph.updateBuildTripleRecursively

Plugins don't actually need whole graph to be switched to use
`.tools` triple because `PluginTarget.processAccessibleTools`
is able to find targets/products that have to be built for the
host, it just needs a way to signal that to the build system.

`Buildset` has been extended to support a destination which
is defaulted to `.target` because really only plugin (command) tools
have to set it to `.host`, this helps to propagate the information
about expected destination down through `BuildOperation.build(subset:)`
and find correct llbuild names and build products.

Author: xedin

Sources/Build/BuildOperation.swift

@@ -378,9 +378,9 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
 
         let subsetDescriptor: String?
         switch subset {
-        case .product(let productName):
+        case .product(let productName, _):
             subsetDescriptor = "product '\(productName)'"
-        case .target(let targetName):
+        case .target(let targetName, _):
             subsetDescriptor = "target: '\(targetName)'"
         case .allExcludingTests, .allIncludingTests:
             subsetDescriptor = nil
@@ -433,10 +433,10 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
         case .allExcludingTests, .allIncludingTests:
             pluginsToCompile = allPlugins
             continueBuilding = true
-        case .product(let productName):
+        case .product(let productName, _):
             pluginsToCompile = allPlugins.filter{ $0.productNames.contains(productName) }
             continueBuilding = pluginsToCompile.isEmpty
-        case .target(let targetName):
+        case .target(let targetName, _):
             pluginsToCompile = allPlugins.filter{ $0.targetName == targetName }
             continueBuilding = pluginsToCompile.isEmpty
         }
@@ -522,12 +522,14 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
             return LLBuildManifestBuilder.TargetKind.main.targetName
         case .allIncludingTests:
             return LLBuildManifestBuilder.TargetKind.test.targetName
-        default:
+        case .product(_, let destination), .target(_, let destination):
             // FIXME: This is super unfortunate that we might need to load the package graph.
             let graph = try getPackageGraph()
             if let result = subset.llbuildTargetName(
                 for: graph,
-                buildParameters: self.productsBuildParameters,
+                using: destination == .host
+                            ? self.toolsBuildParameters
+                            : self.productsBuildParameters,
                 observabilityScope: self.observabilityScope
             ) {
                 return result
@@ -550,15 +552,15 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
             // done, which makes it hard to realign them all at once.
             var pluginsBuildParameters = self.toolsBuildParameters
             pluginsBuildParameters.dataPath = pluginsBuildParameters.dataPath.parentDirectory.appending(components: ["plugins", "tools"])
-            var buildToolsGraph = graph
-            try buildToolsGraph.updateBuildTripleRecursively(.tools)
+
+            var targetBuildParameters = pluginsBuildParameters
+            targetBuildParameters.destination = .target
 
             let buildOperationForPluginDependencies = BuildOperation(
-                // FIXME: this doesn't maintain the products/tools split cleanly
-                productsBuildParameters: pluginsBuildParameters,
+                productsBuildParameters: targetBuildParameters,
                 toolsBuildParameters: pluginsBuildParameters,
                 cacheBuildManifest: false,
-                packageGraphLoader: { buildToolsGraph },
+                packageGraphLoader: { graph },
                 scratchDirectory: pluginsBuildParameters.dataPath,
                 additionalFileRules: self.additionalFileRules,
                 pkgConfigDirectories: self.pkgConfigDirectories,
@@ -569,7 +571,8 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
                 fileSystem: self.fileSystem,
                 observabilityScope: self.observabilityScope
             )
-            buildToolPluginInvocationResults = try buildToolsGraph.invokeBuildToolPlugins(
+
+            buildToolPluginInvocationResults = try graph.invokeBuildToolPlugins(
                 outputDir: pluginConfiguration.workDirectory.appending("outputs"),
                 buildParameters: pluginsBuildParameters,
                 additionalFileRules: self.additionalFileRules,
@@ -579,8 +582,10 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
                 observabilityScope: self.observabilityScope,
                 fileSystem: self.fileSystem
             ) { name, path in
-                try buildOperationForPluginDependencies.build(subset: .product(name))
-                if let builtTool = try buildOperationForPluginDependencies.buildPlan.buildProducts.first(where: { $0.product.name == name}) {
+                try buildOperationForPluginDependencies.build(subset: .product(name, for: .host))
+                if let builtTool = try buildOperationForPluginDependencies.buildPlan.buildProducts.first(where: {
+                    $0.product.name == name && $0.product.buildTriple == .tools
+                }) {
                     return try builtTool.binaryPath
                 } else {
                     return nil
@@ -893,13 +898,13 @@ extension BuildSubset {
             return Array(graph.reachableTargets)
         case .allExcludingTests:
             return graph.reachableTargets.filter { $0.type != .test }
-        case .product(let productName):
+        case .product(let productName, _):
             guard let product = graph.product(for: productName) else {
                 observabilityScope.emit(error: "no product named '\(productName)'")
                 return nil
             }
             return try product.recursiveTargetDependencies()
-        case .target(let targetName):
+        case .target(let targetName, _):
             guard let target = graph.target(for: targetName) else {
                 observabilityScope.emit(error: "no target named '\(targetName)'")
                 return nil
@@ -911,15 +916,17 @@ extension BuildSubset {
     /// Returns the name of the llbuild target that corresponds to the build subset.
     func llbuildTargetName(
         for graph: ModulesGraph,
-        buildParameters: BuildParameters,
+        using buildParameters: BuildParameters,
         observabilityScope: ObservabilityScope
     ) -> String? {
         switch self {
         case .allExcludingTests:
             return LLBuildManifestBuilder.TargetKind.main.targetName
         case .allIncludingTests:
             return LLBuildManifestBuilder.TargetKind.test.targetName
-        case .product(let productName):
+        case .product(let productName, let destination):
+            precondition(buildParameters.destination == destination)
+
             guard let product = graph.product(for: productName) else {
                 observabilityScope.emit(error: "no product named '\(productName)'")
                 return nil
@@ -936,7 +943,9 @@ extension BuildSubset {
             return observabilityScope.trap {
                 try product.getLLBuildTargetName(buildParameters: buildParameters)
             }
-        case .target(let targetName):
+        case .target(let targetName, let destination):
+            precondition(buildParameters.destination == destination)
+
             guard let target = graph.target(for: targetName) else {
                 observabilityScope.emit(error: "no target named '\(targetName)'")
                 return nil

Sources/Commands/PackageCommands/PluginCommand.swift

@@ -317,30 +317,27 @@ struct PluginCommand: SwiftCommand {
         let toolSearchDirs = [try swiftCommandState.getTargetToolchain().swiftCompilerPath.parentDirectory]
             + getEnvSearchPaths(pathString: ProcessEnv.path, currentWorkingDirectory: .none)
 
-        var buildToolsGraph = packageGraph
-        try buildToolsGraph.updateBuildTripleRecursively(.tools)
-
         let buildParameters = try swiftCommandState.toolsBuildParameters
         // Build or bring up-to-date any executable host-side tools on which this plugin depends. Add them and any binary dependencies to the tool-names-to-path map.
         let buildSystem = try swiftCommandState.createBuildSystem(
             explicitBuildSystem: .native,
             cacheBuildManifest: false,
-            // Force all dependencies to be built for the host, to work around the fact that BuildOperation.plan
-            // knows to compile build tool plugin dependencies for the host but does not do the same for command
-            // plugins.
-            productsBuildParameters: buildParameters,
-            packageGraphLoader: { buildToolsGraph }
+            productsBuildParameters: swiftCommandState.productsBuildParameters,
+            toolsBuildParameters: buildParameters,
+            packageGraphLoader: { packageGraph }
         )
 
         let accessibleTools = try plugin.processAccessibleTools(
-            packageGraph: buildToolsGraph,
+            packageGraph: packageGraph,
             fileSystem: swiftCommandState.fileSystem,
             environment: buildParameters.buildEnvironment,
             for: try pluginScriptRunner.hostTriple
         ) { name, _ in
             // Build the product referenced by the tool, and add the executable to the tool map. Product dependencies are not supported within a package, so if the tool happens to be from the same package, we instead find the executable that corresponds to the product. There is always one, because of autogeneration of implicit executables with the same name as the target if there isn't an explicit one.
-            try buildSystem.build(subset: .product(name))
-            if let builtTool = try buildSystem.buildPlan.buildProducts.first(where: { $0.product.name == name }) {
+            try buildSystem.build(subset: .product(name, for: .host))
+            if let builtTool = try buildSystem.buildPlan.buildProducts.first(where: {
+                $0.product.name == name && $0.product.buildTriple == .tools
+            }) {
                 return try builtTool.binaryPath
             } else {
                 return nil

Sources/Commands/SwiftTestCommand.swift

@@ -1381,7 +1381,12 @@ private func buildTestsIfNeeded(
         toolsBuildParameters: toolsBuildParameters
     )
 
-    let subset = testProduct.map(BuildSubset.product) ?? .allIncludingTests
+    let subset: BuildSubset = if let testProduct {
+        .product(testProduct)
+    } else {
+        .allIncludingTests
+    }
+
     try buildSystem.build(subset: subset)
 
     // Find the test product.

Sources/PackageGraph/ModulesGraph.swift

@@ -277,30 +277,6 @@ public struct ModulesGraph {
         self.allProducts = allProducts
     }
 
-    package mutating func updateBuildTripleRecursively(_ buildTriple: BuildTriple) throws {
-        self.reachableTargets = IdentifiableSet(self.reachableTargets.map {
-            var target = $0
-            target.buildTriple = buildTriple
-            return target
-        })
-        self.reachableProducts = IdentifiableSet(self.reachableProducts.map {
-            var product = $0
-            product.buildTriple = buildTriple
-            return product
-        })
-
-        self.allTargets = IdentifiableSet(self.allTargets.map {
-            var target = $0
-            target.buildTriple = buildTriple
-            return target
-        })
-        self.allProducts = IdentifiableSet(self.allProducts.map {
-            var product = $0
-            product.buildTriple = buildTriple
-            return product
-        })
-    }
-
     /// Computes a map from each executable target in any of the root packages to the corresponding test targets.
     @_spi(SwiftPMInternal)
     public func computeTestTargetsForExecutableTargets() throws -> [ResolvedModule.ID: [ResolvedModule]] {

Sources/SPMBuildCore/BuildParameters/BuildParameters.swift

@@ -36,7 +36,7 @@ public struct BuildParameters: Encodable {
     }
 
     /// The destination these parameters are going to be used for.
-    public let destination: Destination
+    public var destination: Destination
 
     /// The path to the data directory.
     public var dataPath: AbsolutePath

Sources/SPMBuildCore/BuildSystem/BuildSystem.swift

@@ -25,10 +25,10 @@ public enum BuildSubset {
     case allIncludingTests
 
     /// Represents a specific product.
-    case product(String)
+    case product(String, for: BuildParameters.Destination = .target)
 
     /// Represents a specific target.
-    case target(String)
+    case target(String, for: BuildParameters.Destination = .target)
 }
 
 /// A protocol that represents a build system used by SwiftPM for all build operations. This allows factoring out the

Sources/XCBuildSupport/XcodeBuildSystem.swift

@@ -382,9 +382,9 @@ extension PIFBuilderParameters {
 extension BuildSubset {
     var pifTargetName: String {
         switch self {
-        case .product(let name):
+        case .product(let name, _):
             PackagePIFProjectBuilder.targetName(for: name)
-        case .target(let name):
+        case .target(let name, _):
             name
         case .allExcludingTests:
             PIFBuilder.allExcludingTestsTargetName