Skip to content

Commit 7ea21e1

Browse files
committed
[Build] Fix a bug where computeLLBuildTargetName used wrong test product to infer destination
When there are multiple packages involved, they are going to introduce mutliple `<package-name>PackageTests` products. While inferring a destination for a module we need to find the product it belongs to and infer based on whether that product has any direct macro dependencies or not.
1 parent 37aa266 commit 7ea21e1

File tree

5 files changed

+52
-16
lines changed

5 files changed

+52
-16
lines changed

Sources/Build/BuildOperation.swift

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -573,11 +573,17 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
573573

574574
/// Compute the llbuild target name using the given subset.
575575
func computeLLBuildTargetName(for subset: BuildSubset) async throws -> String {
576-
func inferTestsDestination(graph: ModulesGraph) -> BuildParameters.Destination {
577-
if graph.allProducts.filter({ $0.type == .test }).first(where: \.hasDirectMacroDependencies) != nil {
578-
return .host
576+
func inferTestDestination(
577+
testModule: ResolvedModule,
578+
graph: ModulesGraph
579+
) throws -> BuildParameters.Destination {
580+
for product in graph.allProducts where product.type == .test {
581+
if product.modules.contains(where: { $0.id == testModule.id }) {
582+
return product.hasDirectMacroDependencies ? .host : .target
583+
}
579584
}
580-
return .target
585+
586+
throw InternalError("Could not find a product for test module: \(testModule)")
581587
}
582588

583589
switch subset {
@@ -601,7 +607,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
601607
} else if product.type == .macro || product.type == .plugin {
602608
config.buildParameters(for: .host)
603609
} else if product.type == .test {
604-
config.buildParameters(for: inferTestsDestination(graph: graph))
610+
config.buildParameters(for: product.hasDirectMacroDependencies ? .host : .target)
605611
} else {
606612
config.buildParameters(for: .target)
607613
}
@@ -632,7 +638,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
632638
} else if module.type == .macro || module.type == .plugin {
633639
config.buildParameters(for: .host)
634640
} else if module.type == .test {
635-
config.buildParameters(for: inferTestsDestination(graph: graph))
641+
try config.buildParameters(for: inferTestDestination(testModule: module, graph: graph))
636642
} else {
637643
config.buildParameters(for: .target)
638644
}

Sources/_InternalTestSupport/MockPackageGraphs.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ package func macrosTestsPackageGraph() throws -> MockPackageGraph {
212212
)
213213
]
214214
),
215-
Manifest.createFileSystemManifest(
215+
Manifest.createRootManifest(
216216
displayName: "swift-syntax",
217217
path: "/swift-syntax",
218218
products: [

Tests/BuildTests/BuildOperationTests.swift

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -137,19 +137,37 @@ final class BuildOperationTests: XCTestCase {
137137
func testHostProductsAndTargetsWithoutExplicitDestination() async throws {
138138
let mock = try macrosTestsPackageGraph()
139139

140+
let hostParameters = mockBuildParameters(destination: .host)
141+
let targetParameters = mockBuildParameters(destination: .target)
140142
let op = mockBuildOperation(
141-
productsBuildParameters: mockBuildParameters(destination: .target),
142-
toolsBuildParameters: mockBuildParameters(destination: .host),
143+
productsBuildParameters: targetParameters,
144+
toolsBuildParameters: hostParameters,
143145
packageGraphLoader: { mock.graph },
144146
scratchDirectory: AbsolutePath("/.build/\(hostTriple)"),
145147
fs: mock.fileSystem,
146148
observabilityScope: mock.observabilityScope
147149
)
148150

149-
let result = try await op.computeLLBuildTargetName(for: .product("MMIOMacros"))
151+
let mmioMacrosProductName = try await op.computeLLBuildTargetName(for: .product("MMIOMacros"))
150152
XCTAssertEqual(
151153
"MMIOMacros-\(hostTriple)-debug-tool.exe",
152-
result
154+
mmioMacrosProductName
155+
)
156+
157+
let mmioTestsProductName = try await op.computeLLBuildTargetName(
158+
for: .product("swift-mmioPackageTests")
159+
)
160+
XCTAssertEqual(
161+
"swift-mmioPackageTests-\(hostTriple)-debug-tool.test",
162+
mmioTestsProductName
163+
)
164+
165+
let swiftSyntaxTestsProductName = try await op.computeLLBuildTargetName(
166+
for: .product("swift-syntaxPackageTests")
167+
)
168+
XCTAssertEqual(
169+
"swift-syntaxPackageTests-\(targetParameters.triple)-debug.test",
170+
swiftSyntaxTestsProductName
153171
)
154172

155173
for target in ["MMIOMacros", "MMIOPlugin", "MMIOMacrosTests", "MMIOMacro+PluginTests"] {
@@ -160,6 +178,14 @@ final class BuildOperationTests: XCTestCase {
160178
)
161179
}
162180

181+
let swiftSyntaxTestsTarget = try await op.computeLLBuildTargetName(
182+
for: .target("SwiftSyntaxTests")
183+
)
184+
XCTAssertEqual(
185+
"SwiftSyntaxTests-\(targetParameters.triple)-debug.module",
186+
swiftSyntaxTestsTarget
187+
)
188+
163189
let dependencies = try BuildSubset.target("MMIOMacro+PluginTests").recursiveDependencies(
164190
for: mock.graph,
165191
observabilityScope: mock.observabilityScope

Tests/BuildTests/CrossCompilationBuildPlanTests.swift

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -295,14 +295,17 @@ final class CrossCompilationBuildPlanTests: XCTestCase {
295295
observabilityScope: scope
296296
)
297297

298-
// Make sure that build plan doesn't have any "target" tests.
298+
// Make sure that build plan doesn't have any "target" tests except SwiftSyntax ones.
299299
for description in plan.targetMap where description.module.underlying.type == .test {
300-
XCTAssertEqual(description.buildParameters.destination, .host)
300+
XCTAssertEqual(
301+
description.buildParameters.destination,
302+
description.module.name == "SwiftSyntaxTests" ? .target : .host
303+
)
301304
}
302305

303306
let result = try BuildPlanResult(plan: plan)
304-
result.checkProductsCount(2)
305-
result.checkTargetsCount(17)
307+
result.checkProductsCount(3)
308+
result.checkTargetsCount(20)
306309

307310
XCTAssertTrue(try result.allTargets(named: "SwiftSyntax")
308311
.map { try $0.swift() }
@@ -313,6 +316,7 @@ final class CrossCompilationBuildPlanTests: XCTestCase {
313316
try result.check(destination: .host, triple: toolsTriple, for: "MMIOMacros")
314317
try result.check(destination: .target, triple: destinationTriple, for: "MMIO")
315318
try result.check(destination: .host, triple: toolsTriple, for: "MMIOMacrosTests")
319+
try result.check(destination: .target, triple: destinationTriple, for: "swift-syntaxPackageTests")
316320

317321
let macroProducts = result.allProducts(named: "MMIOMacros")
318322
XCTAssertEqual(macroProducts.count, 1)

Tests/PackageGraphTests/CrossCompilationPackageGraphTests.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ final class CrossCompilationPackageGraphTests: XCTestCase {
9191
"SwiftSyntaxMacrosTestSupport"
9292
)
9393

94-
result.check(testModules: "MMIOMacrosTests", "MMIOMacro+PluginTests", "NOOPTests")
94+
result.check(testModules: "MMIOMacrosTests", "MMIOMacro+PluginTests", "NOOPTests", "SwiftSyntaxTests")
9595
result.checkTarget("MMIO") { result in
9696
result.check(dependencies: "MMIOMacros")
9797
}

0 commit comments

Comments
 (0)