[Build/PackageGraph] Sink fallback logic for macro/plugin/test products and packages identification into ModulesGraph

Sources/Build/BuildOperation.swift

@@ -516,7 +516,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
     }
 
     /// Compute the llbuild target name using the given subset.
-    private func computeLLBuildTargetName(for subset: BuildSubset) throws -> String {
+    func computeLLBuildTargetName(for subset: BuildSubset) throws -> String {
         switch subset {
         case .allExcludingTests:
             return LLBuildManifestBuilder.TargetKind.main.targetName
@@ -526,36 +526,22 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
             // FIXME: This is super unfortunate that we might need to load the package graph.
             let graph = try getPackageGraph()
 
-            var product = graph.product(
+            let product = graph.product(
                 for: productName,
                 destination: destination == .host ? .tools : .destination
             )
 
-            var buildParameters = if destination == .host {
-                self.toolsBuildParameters
-            } else {
-                self.productsBuildParameters
-            }
-
-            // It's possible to request a build of a macro or a plugin via `swift build`
-            // which won't have the right destination set because it's impossible to indicate it.
-            //
-            // Same happens with `--test-product` - if one of the test targets directly references
-            // a macro then all if its targets and the product itself become `host`.
-            if product == nil && destination == .target {
-                if let toolsProduct = graph.product(for: productName, destination: .tools),
-                   toolsProduct.type == .macro || toolsProduct.type == .plugin || toolsProduct.type == .test
-                {
-                    product = toolsProduct
-                    buildParameters = self.toolsBuildParameters
-                }
-            }
-
             guard let product else {
                 observabilityScope.emit(error: "no product named '\(productName)'")
                 throw Diagnostics.fatalError
             }
 
+            let buildParameters = if product.buildTriple == .tools {
+                self.toolsBuildParameters
+            } else {
+                self.productsBuildParameters
+            }
+
             // If the product is automatic, we build the main target because automatic products
             // do not produce a binary right now.
             if product.type == .library(.automatic) {
@@ -570,33 +556,22 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
             // FIXME: This is super unfortunate that we might need to load the package graph.
             let graph = try getPackageGraph()
 
-            var target = graph.target(
+            let target = graph.target(
                 for: targetName,
                 destination: destination == .host ? .tools : .destination
             )
 
-            var buildParameters = if destination == .host {
-                self.toolsBuildParameters
-            } else {
-                self.productsBuildParameters
-            }
-
-            // It's possible to request a build of a macro or a plugin via `swift build`
-            // which won't have the right destination because it's impossible to indicate it.
-            if target == nil && destination == .target {
-                if let toolsTarget = graph.target(for: targetName, destination: .tools),
-                   toolsTarget.type == .macro || toolsTarget.type == .plugin
-                {
-                    target = toolsTarget
-                    buildParameters = self.toolsBuildParameters
-                }
-            }
-
             guard let target else {
                 observabilityScope.emit(error: "no target named '\(targetName)'")
                 throw Diagnostics.fatalError
             }
 
+            let buildParameters = if target.buildTriple == .tools {
+                self.toolsBuildParameters
+            } else {
+                self.productsBuildParameters
+            }
+
             return target.getLLBuildTargetName(buildParameters: buildParameters)
         }
     }

Sources/Commands/Utilities/PluginDelegate.swift

@@ -389,8 +389,19 @@ final class PluginDelegate: PluginInvocationDelegate {
             throw StringError("could not find a target named “\(targetName)”")
         }
 
+        // FIXME: This is currently necessary because `target(for:destination:)` can
+        // produce a module that is targeting host when `targetName`` corresponds to
+        // a macro, plugin, or a test. Ideally we'd ask a build system for a`BuildSubset`
+        // and get the destination from there but there are other places that need
+        // refactoring in that way as well.
+        let buildParameters = if target.buildTriple == .tools {
+                try swiftCommandState.toolsBuildParameters
+            } else {
+                try swiftCommandState.productsBuildParameters
+            }
+
         // Build the target, if needed.
-        try buildSystem.build(subset: .target(target.name))
+        try buildSystem.build(subset: .target(target.name, for: buildParameters.destination))
 
         // Configure the symbol graph extractor.
         var symbolGraphExtractor = try SymbolGraphExtract(
@@ -419,7 +430,7 @@ final class PluginDelegate: PluginInvocationDelegate {
         guard let package = packageGraph.package(for: target) else {
             throw StringError("could not determine the package for target “\(target.name)”")
         }
-        let outputDir = try buildSystem.buildPlan.toolsBuildParameters.dataPath.appending(
+        let outputDir = try buildParameters.dataPath.appending(
             components: "extracted-symbols",
             package.identity.description,
             target.name
@@ -430,7 +441,7 @@ final class PluginDelegate: PluginInvocationDelegate {
         let result = try symbolGraphExtractor.extractSymbolGraph(
             module: target,
             buildPlan: try buildSystem.buildPlan,
-            buildParameters: buildSystem.buildPlan.destinationBuildParameters,
+            buildParameters: buildParameters,
             outputRedirection: .collect,
             outputDirectory: outputDir,
             verboseOutput: self.swiftCommandState.logLevel <= .info

Sources/PackageGraph/ModulesGraph.swift

@@ -167,11 +167,57 @@ public struct ModulesGraph {
     }
 
     public func product(for name: String, destination: BuildTriple) -> ResolvedProduct? {
-        self.allProducts.first { $0.name == name && $0.buildTriple == destination }
+        func findProduct(name: String, destination: BuildTriple) -> ResolvedProduct? {
+            self.allProducts.first { $0.name == name && $0.buildTriple == destination }
+        }
+
+        if let product = findProduct(name: name, destination: destination) {
+            return product
+        }
+
+        // FIXME: This is a temporary workaround and needs to be handled by the callers.
+
+        // It's possible to request a build of a macro, a plugin, or a test via `swift build`
+        // which won't have the right destination set because it's impossible to indicate it.
+        //
+        // Same happens with `--test-product` - if one of the test targets directly references
+        // a macro then all if its targets and the product itself become `host`.
+        if destination == .destination {
+            if let toolsProduct = findProduct(name: name, destination: .tools),
+                toolsProduct.type == .macro || toolsProduct.type == .plugin || toolsProduct.type == .test
+            {
+                return toolsProduct
+            }
+        }
+
+        return nil
     }
 
     public func target(for name: String, destination: BuildTriple) -> ResolvedModule? {
-        self.allTargets.first { $0.name == name && $0.buildTriple == destination }
+        func findModule(name: String, destination: BuildTriple) -> ResolvedModule? {
+            self.allTargets.first { $0.name == name && $0.buildTriple == destination }
+        }
+
+        if let module = findModule(name: name, destination: destination) {
+            return module
+        }
+
+        // FIXME: This is a temporary workaround and needs to be handled by the callers.
+
+        // It's possible to request a build of a macro, a plugin or a test via `swift build`
+        // which won't have the right destination set because it's impossible to indicate it.
+        //
+        // Same happens with `--test-product` - if one of the test targets directly references
+        // a macro then all if its targets and the product itself become `host`.
+        if destination == .destination {
+            if let toolsModule = findModule(name: name, destination: .tools),
+                toolsModule.type == .macro || toolsModule.type == .plugin || toolsModule.type == .test
+            {
+                return toolsModule
+            }
+        }
+
+        return nil
     }
 
     /// All root and root dependency packages provided as input to the graph.

Sources/SPMTestSupport/MockPackageGraphs.swift

@@ -135,9 +135,11 @@ public func macrosPackageGraph() throws -> MockPackageGraph {
 @_spi(SwiftPMInternal)
 public func macrosTestsPackageGraph() throws -> MockPackageGraph {
     let fs = InMemoryFileSystem(emptyFiles:
+        "/swift-mmio/Plugins/MMIOPlugin/source.swift",
         "/swift-mmio/Sources/MMIO/source.swift",
         "/swift-mmio/Sources/MMIOMacros/source.swift",
         "/swift-mmio/Sources/MMIOMacrosTests/source.swift",
+        "/swift-mmio/Sources/MMIOMacro+PluginTests/source.swift",
         "/swift-syntax/Sources/SwiftSyntax/source.swift",
         "/swift-syntax/Sources/SwiftSyntaxMacrosTestSupport/source.swift",
         "/swift-syntax/Sources/SwiftSyntaxMacros/source.swift",
@@ -164,6 +166,11 @@ public func macrosTestsPackageGraph() throws -> MockPackageGraph {
                         name: "MMIO",
                         type: .library(.automatic),
                         targets: ["MMIO"]
+                    ),
+                    ProductDescription(
+                        name: "MMIOPlugin",
+                        type: .plugin,
+                        targets: ["MMIOPlugin"]
                     )
                 ],
                 targets: [
@@ -179,13 +186,26 @@ public func macrosTestsPackageGraph() throws -> MockPackageGraph {
                         ],
                         type: .macro
                     ),
+                    TargetDescription(
+                        name: "MMIOPlugin",
+                        type: .plugin,
+                        pluginCapability: .buildTool
+                    ),
                     TargetDescription(
                         name: "MMIOMacrosTests",
                         dependencies: [
                             .target(name: "MMIOMacros"),
                             .product(name: "SwiftSyntaxMacrosTestSupport", package: "swift-syntax")
                         ],
                         type: .test
+                    ),
+                    TargetDescription(
+                        name: "MMIOMacro+PluginTests",
+                        dependencies: [
+                            .target(name: "MMIOPlugin"),
+                            .target(name: "MMIOMacros")
+                        ],
+                        type: .test
                     )
                 ]
             ),

Tests/BuildTests/BuildOperationTests.swift

@@ -18,6 +18,7 @@ import LLBuildManifest
 @_spi(DontAdoptOutsideOfSwiftPMExposedForBenchmarksAndTestsOnly)
 import PackageGraph
 import SPMBuildCore
+@_spi(SwiftPMInternal)
 import SPMTestSupport
 import XCTest
 
@@ -179,4 +180,41 @@ final class BuildOperationTests: XCTestCase {
             }
         }
     }
+
+    func testHostProductsAndTargetsWithoutExplicitDestination() throws {
+        let mock  = try macrosTestsPackageGraph()
+
+        let op = mockBuildOperation(
+            productsBuildParameters: mockBuildParameters(destination: .target),
+            toolsBuildParameters: mockBuildParameters(destination: .host),
+            packageGraphLoader: { mock.graph },
+            scratchDirectory: AbsolutePath("/.build/\(hostTriple)"),
+            fs: mock.fileSystem,
+            observabilityScope: mock.observabilityScope
+        )
+
+        XCTAssertEqual(
+            "MMIOMacros-\(hostTriple)-debug-tool.exe",
+            try op.computeLLBuildTargetName(for: .product("MMIOMacros"))
+        )
+
+        for target in ["MMIOMacros", "MMIOPlugin", "MMIOMacrosTests", "MMIOMacro+PluginTests"] {
+            XCTAssertEqual(
+                "\(target)-\(hostTriple)-debug-tool.module",
+                try op.computeLLBuildTargetName(for: .target(target))
+            )
+        }
+
+        let dependencies = try BuildSubset.target("MMIOMacro+PluginTests").recursiveDependencies(
+            for: mock.graph,
+            observabilityScope: mock.observabilityScope
+        )
+
+        XCTAssertNotNil(dependencies)
+        XCTAssertTrue(dependencies!.count > 0)
+
+        for dependency in dependencies! {
+            XCTAssertEqual(dependency.buildTriple, .tools)
+        }
+    }
 }

Tests/BuildTests/CrossCompilationBuildPlanTests.swift

@@ -302,7 +302,7 @@ final class CrossCompilationBuildPlanTests: XCTestCase {
         )
         let result = try BuildPlanResult(plan: plan)
         result.checkProductsCount(2)
-        result.checkTargetsCount(15)
+        result.checkTargetsCount(16)
 
         XCTAssertTrue(try result.allTargets(named: "SwiftSyntax")
             .map { try $0.swiftTarget() }

Tests/PackageGraphTests/CrossCompilationPackageGraphTests.swift

@@ -83,6 +83,10 @@ final class CrossCompilationPackageGraphTests: XCTestCase {
                 }
             }
 
+            result.checkProduct("MMIOMacros", destination: .destination) { result in
+                result.check(buildTriple: .tools)
+            }
+
             result.checkTargets("SwiftSyntax") { results in
                 XCTAssertEqual(results.count, 2)
 
@@ -100,6 +104,7 @@ final class CrossCompilationPackageGraphTests: XCTestCase {
             result.check(
                 targets: "MMIO",
                 "MMIOMacros",
+                "MMIOPlugin",
                 "SwiftCompilerPlugin",
                 "SwiftCompilerPlugin",
                 "SwiftCompilerPluginMessageHandling",
@@ -111,14 +116,15 @@ final class CrossCompilationPackageGraphTests: XCTestCase {
                 "SwiftSyntaxMacrosTestSupport",
                 "SwiftSyntaxMacrosTestSupport"
             )
-            result.check(testModules: "MMIOMacrosTests")
+            result.check(testModules: "MMIOMacrosTests", "MMIOMacro+PluginTests")
             result.checkTarget("MMIO") { result in
                 result.check(buildTriple: .destination)
                 result.check(dependencies: "MMIOMacros")
             }
             result.checkTargets("MMIOMacros") { results in
                 XCTAssertEqual(results.count, 1)
             }
+
             result.checkTarget("MMIOMacrosTests", destination: .tools) { result in
                 result.check(buildTriple: .tools)
                 result.checkDependency("MMIOMacros") { result in
@@ -146,6 +152,14 @@ final class CrossCompilationPackageGraphTests: XCTestCase {
                 }
             }
 
+            result.checkTarget("MMIOMacros", destination: .destination) { result in
+                result.check(buildTriple: .tools)
+            }
+
+            result.checkTarget("MMIOMacrosTests", destination: .destination) { result in
+                result.check(buildTriple: .tools)
+            }
+
             result.checkTargets("SwiftSyntax") { results in
                 XCTAssertEqual(results.count, 2)
 
@@ -168,4 +182,35 @@ final class CrossCompilationPackageGraphTests: XCTestCase {
             }
         }
     }
+
+    func testPlugins() throws {
+        let graph = try macrosTestsPackageGraph().graph
+        PackageGraphTester(graph) { result in
+            result.checkProduct("MMIOPlugin", destination: .tools) { result in
+                result.check(buildTriple: .tools)
+            }
+
+            result.checkProduct("MMIOPlugin", destination: .destination) { result in
+                result.check(buildTriple: .tools)
+            }
+
+            result.checkTarget("MMIOPlugin", destination: .tools) { result in
+                result.check(buildTriple: .tools)
+            }
+
+            result.checkTarget("MMIOPlugin", destination: .destination) { result in
+                result.check(buildTriple: .tools)
+            }
+
+            result.checkTarget("MMIOMacro+PluginTests", destination: .tools) { result in
+                result.check(buildTriple: .tools)
+                result.check(dependencies: "MMIOPlugin", "MMIOMacros")
+            }
+
+            result.checkTarget("MMIOMacro+PluginTests", destination: .destination) { result in
+                result.check(buildTriple: .tools)
+                result.check(dependencies: "MMIOPlugin", "MMIOMacros")
+            }
+        }
+    }
 }