Skip to content

Commit 45c31f1

Browse files
dschaefer2dschaefer2
authored
[6.1] Fix duplicate modulemap errors with macro and plugin deps (#8472) (#8491)
We were including flags to hook up modulemaps and include files to C library dependencies in macros and plugin tools through to the modules that depend on those. This adds the capability to prune the depth first searches through the dependencies to ensure these are skipped when crossing macro and plugin boundaries.
1 parent ceb3b87 commit 45c31f1

File tree

7 files changed

+426
-75
lines changed

7 files changed

+426
-75
lines changed

‎Sources/Build/BuildDescription/ModuleBuildDescription.swift‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,4 +192,14 @@ extension ModuleBuildDescription {
192192
}
193193
return dependencies
194194
}
195+
196+
package func recursiveLinkDependencies(using plan: BuildPlan) -> [Dependency] {
197+
var dependencies: [Dependency] = []
198+
plan.traverseLinkDependencies(of: self) { product, _, description in
199+
dependencies.append(.product(product, description))
200+
} onModule: { module, _, description in
201+
dependencies.append(.module(module, description))
202+
}
203+
return dependencies
204+
}
195205
}

‎Sources/Build/BuildDescription/SwiftModuleBuildDescription.swift‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1042,6 +1042,12 @@ extension SwiftModuleBuildDescription {
10421042
ModuleBuildDescription.swift(self).dependencies(using: plan)
10431043
}
10441044

1045+
package func recursiveLinkDependencies(
1046+
using plan: BuildPlan
1047+
) -> [ModuleBuildDescription.Dependency] {
1048+
ModuleBuildDescription.swift(self).recursiveLinkDependencies(using: plan)
1049+
}
1050+
10451051
package func recursiveDependencies(
10461052
using plan: BuildPlan
10471053
) -> [ModuleBuildDescription.Dependency] {

‎Sources/Build/BuildPlan/BuildPlan+Swift.swift‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ import class PackageModel.SystemLibraryModule
1919
extension BuildPlan {
2020
func plan(swiftTarget: SwiftModuleBuildDescription) throws {
2121
// We need to iterate recursive dependencies because Swift compiler needs to see all the targets a target
22-
// depends on.
23-
for case .module(let dependency, let description) in swiftTarget.recursiveDependencies(using: self) {
22+
// builds against
23+
for case .module(let dependency, let description) in swiftTarget.recursiveLinkDependencies(using: self) {
2424
switch dependency.underlying {
2525
case let underlyingTarget as ClangModule where underlyingTarget.type == .library:
2626
guard case let .clang(target)? = description else {
@@ -53,5 +53,4 @@ extension BuildPlan {
5353
}
5454
}
5555
}
56-
5756
}

‎Sources/Build/BuildPlan/BuildPlan.swift‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1186,6 +1186,82 @@ extension BuildPlan {
11861186
}
11871187
}
11881188

1189+
depthFirstSearch(successors(for: description.module, destination: description.destination)) {
1190+
switch $0 {
1191+
case .module(let module, let destination):
1192+
successors(for: module, destination: destination)
1193+
case .product(let product, let destination):
1194+
successors(for: product, destination: destination)
1195+
case .package:
1196+
[]
1197+
}
1198+
} onNext: { module, _ in
1199+
switch module {
1200+
case .package:
1201+
break
1202+
1203+
case .product(let product, let destination):
1204+
onProduct(product, destination, self.description(for: product, context: destination))
1205+
1206+
case .module(let module, let destination):
1207+
onModule(module, destination, self.description(for: module, context: destination))
1208+
}
1209+
}
1210+
}
1211+
1212+
// Only follow link time dependencies, i.e. skip dependencies on macros and plugins
1213+
// except for testTargets that depend on macros.
1214+
package func traverseLinkDependencies(
1215+
of description: ModuleBuildDescription,
1216+
onProduct: (ResolvedProduct, BuildParameters.Destination, ProductBuildDescription?) -> Void,
1217+
onModule: (ResolvedModule, BuildParameters.Destination, ModuleBuildDescription?) -> Void
1218+
) {
1219+
var visited = Set<TraversalNode>()
1220+
func successors(
1221+
for product: ResolvedProduct,
1222+
destination: Destination
1223+
) -> [TraversalNode] {
1224+
product.modules.map { module in
1225+
TraversalNode(module: module, context: destination)
1226+
}.filter {
1227+
visited.insert($0).inserted
1228+
}
1229+
}
1230+
1231+
func successors(
1232+
for parentModule: ResolvedModule,
1233+
destination: Destination
1234+
) -> [TraversalNode] {
1235+
parentModule
1236+
.dependencies(satisfying: description.buildParameters.buildEnvironment)
1237+
.reduce(into: [TraversalNode]()) { partial, dependency in
1238+
switch dependency {
1239+
case .product(let product, _):
1240+
guard product.type != .plugin else {
1241+
return
1242+
}
1243+
1244+
guard product.type != .macro || parentModule.type == .test else {
1245+
return
1246+
}
1247+
1248+
partial.append(.init(product: product, context: destination))
1249+
case .module(let childModule, _):
1250+
guard childModule.type != .plugin else {
1251+
return
1252+
}
1253+
1254+
guard childModule.type != .macro || parentModule.type == .test else {
1255+
return
1256+
}
1257+
1258+
partial.append(.init(module: childModule, context: destination))
1259+
}
1260+
}.filter {
1261+
visited.insert($0).inserted
1262+
}
1263+
}
1264+
11891265
depthFirstSearch(successors(for: description.module, destination: description.destination)) {
11901266
switch $0 {
11911267
case .module(let module, let destination):

‎Tests/BuildTests/BuildPlanTests.swift‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6936,4 +6936,86 @@ final class BuildPlanTests: XCTestCase {
69366936
XCTAssertMatch(contents, .regex(#"args: \[.*"-I","/testpackagedep/SomeArtifact.xcframework/macos/Headers".*,"/testpackage/Sources/CLib/lib.c".*]"#))
69376937
XCTAssertMatch(contents, .regex(#"args: \[.*"-module-name","SwiftLib",.*"-I","/testpackagedep/SomeArtifact.xcframework/macos/Headers".*]"#))
69386938
}
6939+
6940+
func testMacroPluginDependencyLeakage() async throws {
6941+
// Make sure the include paths from macro and plugin executables don't leak into dependents
6942+
let observability = ObservabilitySystem.makeForTesting()
6943+
let fs = InMemoryFileSystem(emptyFiles: [
6944+
"/LeakTest/Sources/CLib/include/Clib.h",
6945+
"/LeakTest/Sources/CLib/Clib.c",
6946+
"/LeakTest/Sources/MyMacro/MyMacro.swift",
6947+
"/LeakTest/Sources/MyPluginTool/MyPluginTool.swift",
6948+
"/LeakTest/Sources/MyLib/MyLib.swift",
6949+
"/LeakTest/Plugins/MyPlugin/MyPlugin.swift",
6950+
"/LeakTest/Tests/MyMacroTests/MyMacroTests.swift",
6951+
"/LeakTest/Tests/MyMacro2Tests/MyMacro2Tests.swift",
6952+
"/LeakLib/Sources/CLib2/include/Clib.h",
6953+
"/LeakLib/Sources/CLib2/Clib.c",
6954+
"/LeakLib/Sources/MyMacro2/MyMacro.swift",
6955+
"/LeakLib/Sources/MyPluginTool2/MyPluginTool.swift",
6956+
"/LeakLib/Sources/MyLib2/MyLib.swift",
6957+
"/LeakLib/Plugins/MyPlugin2/MyPlugin.swift",
6958+
])
6959+
6960+
let graph = try loadModulesGraph(fileSystem: fs, manifests: [
6961+
Manifest.createFileSystemManifest(
6962+
displayName: "LeakLib",
6963+
path: "/LeakLib",
6964+
products: [
6965+
ProductDescription(name: "MyLib2", type: .library(.automatic), targets: ["MyLib2"]),
6966+
ProductDescription(name: "MyMacros2", type: .macro, targets: ["MyMacro2"]),
6967+
],
6968+
targets: [
6969+
TargetDescription(name: "CLib2"),
6970+
TargetDescription(name: "MyMacro2", dependencies: ["CLib2"], type: .macro),
6971+
TargetDescription(name: "MyPluginTool2", dependencies: ["CLib2"], type: .executable),
6972+
TargetDescription(name: "MyPlugin2", dependencies: ["MyPluginTool2"], type: .plugin, pluginCapability: .buildTool),
6973+
TargetDescription(name: "MyLib2", dependencies: ["CLib2", "MyMacro2"], pluginUsages: [.plugin(name: "MyPlugin2", package: nil)]),
6974+
]
6975+
),
6976+
Manifest.createRootManifest(
6977+
displayName: "LeakTest",
6978+
path: "/LeakTest",
6979+
dependencies: [
6980+
.fileSystem(path: "/LeakLib")
6981+
],
6982+
targets: [
6983+
TargetDescription(name: "CLib"),
6984+
TargetDescription(name: "MyMacro", dependencies: ["CLib"], type: .macro),
6985+
TargetDescription(name: "MyPluginTool", dependencies: ["CLib"], type: .executable),
6986+
TargetDescription(name: "MyPlugin", dependencies: ["MyPluginTool"], type: .plugin, pluginCapability: .buildTool),
6987+
TargetDescription(
6988+
name: "MyLib",
6989+
dependencies: ["CLib", "MyMacro", .product(name: "MyLib2", package: "LeakLib")],
6990+
pluginUsages: [.plugin(name: "MyPlugin", package: nil)]
6991+
),
6992+
TargetDescription(name: "MyMacroTests", dependencies: ["MyMacro"], type: .test),
6993+
TargetDescription(
6994+
name: "MyMacro2Tests",
6995+
dependencies: [.product(name: "MyMacros2", package: "LeakLib")],
6996+
type: .test
6997+
)
6998+
]
6999+
)
7000+
], observabilityScope: observability.topScope)
7001+
XCTAssertNoDiagnostics(observability.diagnostics)
7002+
7003+
let plan = try await mockBuildPlan(
7004+
graph: graph,
7005+
fileSystem: fs,
7006+
observabilityScope: observability.topScope
7007+
)
7008+
XCTAssertNoDiagnostics(observability.diagnostics)
7009+
7010+
let myLib = try XCTUnwrap(plan.targets.first(where: { $0.module.name == "MyLib" })).swift()
7011+
XCTAssertFalse(myLib.additionalFlags.contains(where: { $0.contains("-tool")}), "flags shouldn't contain tools items")
7012+
7013+
// Make sure the tests do have the include path and the module map from the lib
7014+
let myMacroTests = try XCTUnwrap(plan.targets.first(where: { $0.module.name == "MyMacroTests" })).swift()
7015+
XCTAssertTrue(myMacroTests.additionalFlags.contains(where: { $0.contains("CLib/include") }))
7016+
XCTAssertTrue(myMacroTests.additionalFlags.contains(where: { $0.contains("CLib-tool.build/module.modulemap") }))
7017+
let myMacro2Tests = try XCTUnwrap(plan.targets.first(where: { $0.module.name == "MyMacro2Tests" })).swift()
7018+
XCTAssertTrue(myMacro2Tests.additionalFlags.contains(where: { $0.contains("CLib2/include") }))
7019+
XCTAssertTrue(myMacro2Tests.additionalFlags.contains(where: { $0.contains("CLib2-tool.build/module.modulemap") }))
7020+
}
69397021
}

0 commit comments

Comments
 (0)