Fix duplicate modulemap errors with macro and plugin deps

Sources/Basics/Graph/GraphAlgorithms.swift

@@ -131,3 +131,50 @@ public func depthFirstSearch<T: Hashable>(
         }
     }
 }
+
+/// Implements a pre-order depth-first search that traverses the whole graph and
+/// doesn't distinguish between unique and duplicate nodes. The visitor can abort
+/// a path as needed to prune the tree.
+/// The method expects the graph to be acyclic but doesn't check that.
+///
+/// - Parameters:
+///   - nodes: The list of input nodes to sort.
+///   - successors: A closure for fetching the successors of a particular node.
+///   - onNext: A callback to indicate the node currently being processed
+///             including its parent (if any) and its depth. Returns whether to
+///             continue down the current path.
+///
+/// - Complexity: O(v + e) where (v, e) are the number of vertices and edges
+/// reachable from the input nodes via the relation.
+public enum DepthFirstContinue {
+    case `continue`
+    case abort
+}
+
+public func depthFirstSearch<T: Hashable>(
+    _ nodes: [T],
+    successors: (T) throws -> [T],
+    visitNext: (T, _ parent: T?) throws -> DepthFirstContinue
+) rethrows {
+    var stack = OrderedSet<TraversalNode<T>>()
+
+    for node in nodes {
+        precondition(stack.isEmpty)
+        stack.append(TraversalNode(parent: nil, curr: node))
+
+        while !stack.isEmpty {
+            let node = stack.removeLast()
+
+            if try visitNext(node.curr, node.parent) == .continue {
+                for succ in try successors(node.curr) {
+                    stack.append(
+                        TraversalNode(
+                            parent: node.curr,
+                            curr: succ
+                        )
+                    )
+                }
+            }
+        }
+    }
+}

Sources/Build/BuildDescription/ModuleBuildDescription.swift

@@ -187,8 +187,30 @@ extension ModuleBuildDescription {
         var dependencies: [Dependency] = []
         plan.traverseDependencies(of: self) { product, _, description in
             dependencies.append(.product(product, description))
+            return .continue
         } onModule: { module, _, description in
             dependencies.append(.module(module, description))
+            return .continue
+        }
+        return dependencies
+    }
+
+    package func recursiveLinkDependencies(using plan: BuildPlan) -> [Dependency] {
+        var dependencies: [Dependency] = []
+        plan.traverseDependencies(of: self) { product, _, description in
+            guard product.type != .macro && product.type != .plugin else {
+                return .abort
+            }
+
+            dependencies.append(.product(product, description))
+            return .continue
+        } onModule: { module, _, description in
+            guard module.type != .macro && module.type != .plugin else {
+                return .abort
+            }
+
+            dependencies.append(.module(module, description))
+            return .continue
         }
         return dependencies
     }

Sources/Build/BuildDescription/SwiftModuleBuildDescription.swift

@@ -1042,6 +1042,12 @@ extension SwiftModuleBuildDescription {
         ModuleBuildDescription.swift(self).dependencies(using: plan)
     }
 
+    package func recursiveLinkDependencies(
+        using plan: BuildPlan
+    ) -> [ModuleBuildDescription.Dependency] {
+        ModuleBuildDescription.swift(self).recursiveLinkDependencies(using: plan)
+    }
+
     package func recursiveDependencies(
         using plan: BuildPlan
     ) -> [ModuleBuildDescription.Dependency] {

Sources/Build/BuildPlan/BuildPlan+Swift.swift

@@ -19,7 +19,7 @@ import class PackageModel.SystemLibraryModule
 extension BuildPlan {
     func plan(swiftTarget: SwiftModuleBuildDescription) throws {
         // We need to iterate recursive dependencies because Swift compiler needs to see all the targets a target
-        // depends on.
+        // builds against
         for case .module(let dependency, let description) in swiftTarget.recursiveDependencies(using: self) {
             switch dependency.underlying {
             case let underlyingTarget as ClangModule where underlyingTarget.type == .library:
@@ -53,5 +53,4 @@ extension BuildPlan {
             }
         }
     }
-
 }

Sources/Build/BuildPlan/BuildPlan.swift

@@ -1154,8 +1154,8 @@ extension BuildPlan {
 
     package func traverseDependencies(
         of description: ModuleBuildDescription,
-        onProduct: (ResolvedProduct, BuildParameters.Destination, ProductBuildDescription?) -> Void,
-        onModule: (ResolvedModule, BuildParameters.Destination, ModuleBuildDescription?) -> Void
+        onProduct: (ResolvedProduct, BuildParameters.Destination, ProductBuildDescription?) -> DepthFirstContinue,
+        onModule: (ResolvedModule, BuildParameters.Destination, ModuleBuildDescription?) -> DepthFirstContinue
     ) {
         var visited = Set<TraversalNode>()
         func successors(
@@ -1196,16 +1196,16 @@ extension BuildPlan {
             case .package:
                 []
             }
-        } onNext: { module, _ in
+        } visitNext: { module, _ in
             switch module {
             case .package:
-                break
+                return .continue
 
             case .product(let product, let destination):
-                onProduct(product, destination, self.description(for: product, context: destination))
+                return onProduct(product, destination, self.description(for: product, context: destination))
 
             case .module(let module, let destination):
-                onModule(module, destination, self.description(for: module, context: destination))
+                return onModule(module, destination, self.description(for: module, context: destination))
             }
         }
     }

Tests/BuildTests/BuildPlanTests.swift

@@ -6909,6 +6909,72 @@ class BuildPlanTestCase: BuildSystemProviderTestCase {
         XCTAssertMatch(contents, .regex(#"args: \[.*"-I","/testpackagedep/SomeArtifact.xcframework/macos/Headers".*,"/testpackage/Sources/CLib/lib.c".*]"#))
         XCTAssertMatch(contents, .regex(#"args: \[.*"-module-name","SwiftLib",.*"-I","/testpackagedep/SomeArtifact.xcframework/macos/Headers".*]"#))
     }
+
+    func testMacroPluginDependencyLeakage() async throws {
+        // Make sure the include paths from macro and plugin executables don't leak into dependents
+        let observability = ObservabilitySystem.makeForTesting()
+        let fs = InMemoryFileSystem(emptyFiles: [
+            "/LeakTest/Sources/CLib/include/Clib.h",
+            "/LeakTest/Sources/CLib/Clib.c",
+            "/LeakTest/Sources/MyMacro/MyMacro.swift",
+            "/LeakTest/Sources/MyPluginTool/MyPluginTool.swift",
+            "/LeakTest/Plugins/MyPlugin/MyPlugin.swift",
+            "/LeakTest/Sources/MyLib/MyLib.swift",
+            "/LeakLib/Sources/CLib2/include/Clib.h",
+            "/LeakLib/Sources/CLib2/Clib.c",
+            "/LeakLib/Sources/MyMacro2/MyMacro.swift",
+            "/LeakLib/Sources/MyPluginTool2/MyPluginTool.swift",
+            "/LeakLib/Plugins/MyPlugin2/MyPlugin.swift",
+            "/LeakLib/Sources/MyLib2/MyLib.swift"
+        ])
+
+        let graph = try loadModulesGraph(fileSystem: fs, manifests: [
+            Manifest.createFileSystemManifest(
+                displayName: "LeakLib",
+                path: "/LeakLib",
+                products: [
+                    ProductDescription(name: "MyLib2", type: .library(.automatic), targets: ["MyLib2"]),
+                ],
+                targets: [
+                    TargetDescription(name: "CLib2"),
+                    TargetDescription(name: "MyMacro2", dependencies: ["CLib2"], type: .macro),
+                    TargetDescription(name: "MyPluginTool2", dependencies: ["CLib2"], type: .executable),
+                    TargetDescription(name: "MyPlugin2", dependencies: ["MyPluginTool2"], type: .plugin, pluginCapability: .buildTool),
+                    TargetDescription(name: "MyLib2", dependencies: ["CLib2", "MyMacro2"], pluginUsages: [.plugin(name: "MyPlugin2", package: nil)]),
+                ]
+            ),
+            Manifest.createRootManifest(
+                displayName: "LeakTest",
+                path: "/LeakTest",
+                dependencies: [
+                    .fileSystem(path: "/LeakLib")
+                ],
+                targets: [
+                    TargetDescription(name: "CLib"),
+                    TargetDescription(name: "MyMacro", dependencies: ["CLib"], type: .macro),
+                    TargetDescription(name: "MyPluginTool", dependencies: ["CLib"], type: .executable),
+                    TargetDescription(name: "MyPlugin", dependencies: ["MyPluginTool"], type: .plugin, pluginCapability: .buildTool),
+                    TargetDescription(
+                        name: "MyLib",
+                        dependencies: ["CLib", "MyMacro", .product(name: "MyLib2", package: "LeakLib")],
+                        pluginUsages: [.plugin(name: "MyPlugin", package: nil)]
+                    ),
+                ]
+            )
+        ], observabilityScope: observability.topScope)
+        XCTAssertNoDiagnostics(observability.diagnostics)
+
+        let plan = try await mockBuildPlan(
+            graph: graph,
+            fileSystem: fs,
+            observabilityScope: observability.topScope
+        )
+        XCTAssertNoDiagnostics(observability.diagnostics)
+
+        let myLib = try XCTUnwrap(plan.targets.first(where: { $0.module.name == "MyLib" })).swift()
+        print(myLib.additionalFlags)
+        XCTAssertFalse(myLib.additionalFlags.contains(where: { $0.contains("-tool/include")}), "flags shouldn't contain tools items")
+    }
 }
 
 class BuildPlanNativeTests: BuildPlanTestCase {

Tests/BuildTests/BuildPlanTraversalTests.swift

@@ -146,8 +146,10 @@ final class BuildPlanTraversalTests: XCTestCase {
             XCTAssertEqual(product.name, "SwiftSyntax")
             XCTAssertEqual(destination, .host)
             XCTAssertNil(description)
+            return .continue
         } onModule: { module, destination, description in
             moduleDependencies.append((module, destination, description))
+            return .continue
         }
 
         XCTAssertEqual(moduleDependencies.count, 2)