Fix incorrect paths for library products with explicit linkage (#7519)

Library products declared with `.static` or `.dynamic` linkage didn't have a `-tools` suffix in their file names when built as dependencies of macros. This change resolves the inconsistency and adds corresponding build plan tests. The regression in question broke the compatibility test suite, which had [an old version of `swift-power-assert`](https://github.com/kishikawakatsumi/swift-power-assert/blob/a60cb50/Package.swift) depending on an old version of `swift-syntax` with explicit `.static` linkage for its products. rdar://127170225
swiftlang · May 1, 2024 · 8db2401 · 8db2401
1 parent fe28c77
commit 8db2401
Show file tree

Hide file tree

Showing 4 changed files with 116 additions and 11 deletions.
diff --git a/Sources/SPMBuildCore/BuildParameters/BuildParameters.swift b/Sources/SPMBuildCore/BuildParameters/BuildParameters.swift
@@ -243,7 +243,7 @@ public struct BuildParameters: Encodable {
 
     /// Returns the path to the dynamic library of a product for the current build parameters.
     func potentialDynamicLibraryPath(for product: ResolvedProduct) throws -> RelativePath {
-        try RelativePath(validating: "\(self.triple.dynamicLibraryPrefix)\(product.name)\(self.triple.dynamicLibraryExtension)")
+        try RelativePath(validating: "\(self.triple.dynamicLibraryPrefix)\(product.name)\(self.suffix(triple: product.buildTriple))\(self.triple.dynamicLibraryExtension)")
     }
 
     /// Returns the path to the binary of a product for the current build parameters, relative to the build directory.
@@ -254,7 +254,7 @@ public struct BuildParameters: Encodable {
         case .executable, .snippet:
             return potentialExecutablePath
         case .library(.static):
-            return try RelativePath(validating: "lib\(product.name)\(self.triple.staticLibraryExtension)")
+            return try RelativePath(validating: "lib\(product.name)\(self.suffix(triple: product.buildTriple))\(self.triple.staticLibraryExtension)")
         case .library(.dynamic):
             return try potentialDynamicLibraryPath(for: product)
         case .library(.automatic), .plugin:

diff --git a/Sources/SPMTestSupport/MockPackageGraphs.swift b/Sources/SPMTestSupport/MockPackageGraphs.swift
@@ -21,6 +21,7 @@ import func PackageGraph.loadModulesGraph
 
 import class PackageModel.Manifest
 import struct PackageModel.ProductDescription
+import enum PackageModel.ProductType
 import struct PackageModel.TargetDescription
 import protocol TSCBasic.FileSystem
 import class TSCBasic.InMemoryFileSystem
@@ -258,7 +259,7 @@ package func macrosTestsPackageGraph() throws -> MockPackageGraph {
     return (graph, fs, observability.topScope)
 }
 
-package func trivialPackageGraph(pkgRootPath: AbsolutePath) throws -> MockPackageGraph {
+package func trivialPackageGraph() throws -> MockPackageGraph {
     let fs = InMemoryFileSystem(
         emptyFiles:
         "/Pkg/Sources/app/main.swift",
@@ -288,7 +289,7 @@ package func trivialPackageGraph(pkgRootPath: AbsolutePath) throws -> MockPackag
     return (graph, fs, observability.topScope)
 }
 
-package func embeddedCxxInteropPackageGraph(pkgRootPath: AbsolutePath) throws -> MockPackageGraph {
+package func embeddedCxxInteropPackageGraph() throws -> MockPackageGraph {
     let fs = InMemoryFileSystem(
         emptyFiles:
         "/Pkg/Sources/app/main.swift",
@@ -329,3 +330,66 @@ package func embeddedCxxInteropPackageGraph(pkgRootPath: AbsolutePath) throws ->
 
     return (graph, fs, observability.topScope)
 }
+
+package func toolsExplicitLibrariesGraph(linkage: ProductType.LibraryType) throws -> MockPackageGraph {
+    let fs = InMemoryFileSystem(emptyFiles:
+        "/swift-mmio/Sources/MMIOMacros/source.swift",
+        "/swift-mmio/Sources/MMIOMacrosTests/source.swift",
+        "/swift-syntax/Sources/SwiftSyntax/source.swift"
+    )
+
+    let observability = ObservabilitySystem.makeForTesting()
+    let graph = try loadModulesGraph(
+        fileSystem: fs,
+        manifests: [
+            Manifest.createRootManifest(
+                displayName: "swift-mmio",
+                path: "/swift-mmio",
+                dependencies: [
+                    .localSourceControl(
+                        path: "/swift-syntax",
+                        requirement: .upToNextMajor(from: "1.0.0")
+                    )
+                ],
+                targets: [
+                    TargetDescription(
+                        name: "MMIOMacros",
+                        dependencies: [
+                            .product(name: "SwiftSyntax", package: "swift-syntax"),
+                        ],
+                        type: .macro
+                    ),
+                    TargetDescription(
+                        name: "MMIOMacrosTests",
+                        dependencies: [
+                            .target(name: "MMIOMacros"),
+                        ],
+                        type: .test
+                    )
+                ]
+            ),
+            Manifest.createFileSystemManifest(
+                displayName: "swift-syntax",
+                path: "/swift-syntax",
+                products: [
+                    ProductDescription(
+                        name: "SwiftSyntax",
+                        type: .library(linkage),
+                        targets: ["SwiftSyntax"]
+                    ),
+                ],
+                targets: [
+                    TargetDescription(
+                        name: "SwiftSyntax",
+                        dependencies: []
+                    ),
+                ]
+            ),
+        ],
+        observabilityScope: observability.topScope
+    )
+
+    XCTAssertNoDiagnostics(observability.diagnostics)
+
+    return (graph, fs, observability.topScope)
+}
diff --git a/Tests/BuildTests/CrossCompilationBuildPlanTests.swift b/Tests/BuildTests/CrossCompilationBuildPlanTests.swift
@@ -20,12 +20,14 @@ import struct Basics.Triple
 import enum PackageGraph.BuildTriple
 import class PackageModel.Manifest
 import struct PackageModel.TargetDescription
+import enum PackageModel.ProductType
 import func SPMTestSupport.loadPackageGraph
 
 import func SPMTestSupport.embeddedCxxInteropPackageGraph
 import func SPMTestSupport.macrosPackageGraph
 import func SPMTestSupport.macrosTestsPackageGraph
 import func SPMTestSupport.mockBuildParameters
+import func SPMTestSupport.toolsExplicitLibrariesGraph
 import func SPMTestSupport.trivialPackageGraph
 
 import struct SPMTestSupport.BuildPlanResult
@@ -37,7 +39,7 @@ import XCTest
 
 final class CrossCompilationBuildPlanTests: XCTestCase {
     func testEmbeddedWasmTarget() throws {
-        var (graph, fs, observabilityScope) = try trivialPackageGraph(pkgRootPath: "/Pkg")
+        var (graph, fs, observabilityScope) = try trivialPackageGraph()
 
         let triple = try Triple("wasm32-unknown-none-wasm")
         var parameters = mockBuildParameters(triple: triple)
@@ -68,7 +70,7 @@ final class CrossCompilationBuildPlanTests: XCTestCase {
             ]
         )
 
-        (graph, fs, observabilityScope) = try embeddedCxxInteropPackageGraph(pkgRootPath: "/Pkg")
+        (graph, fs, observabilityScope) = try embeddedCxxInteropPackageGraph()
 
         result = try BuildPlanResult(plan: BuildPlan(
             buildParameters: parameters,
@@ -98,9 +100,7 @@ final class CrossCompilationBuildPlanTests: XCTestCase {
     }
 
     func testWasmTargetRelease() throws {
-        let pkgPath = AbsolutePath("/Pkg")
-
-        let (graph, fs, observabilityScope) = try trivialPackageGraph(pkgRootPath: pkgPath)
+        let (graph, fs, observabilityScope) = try trivialPackageGraph()
 
         var parameters = mockBuildParameters(
             config: .release, triple: .wasi, linkerDeadStrip: true
@@ -133,7 +133,7 @@ final class CrossCompilationBuildPlanTests: XCTestCase {
     func testWASITarget() throws {
         let pkgPath = AbsolutePath("/Pkg")
 
-        let (graph, fs, observabilityScope) = try trivialPackageGraph(pkgRootPath: pkgPath)
+        let (graph, fs, observabilityScope) = try trivialPackageGraph()
 
         var parameters = mockBuildParameters(triple: .wasi)
         parameters.linkingParameters.shouldLinkStaticSwiftStdlib = true
@@ -306,6 +306,47 @@ final class CrossCompilationBuildPlanTests: XCTestCase {
             ]
         )
     }
+
+    func testToolsExplicitLibraries() throws {
+        let destinationTriple = Triple.arm64Linux
+        let toolsTriple = Triple.x86_64MacOS
+
+        for (linkage, productFileName) in [(ProductType.LibraryType.static, "libSwiftSyntax-tool.a"), (.dynamic, "libSwiftSyntax-tool.dylib")] {
+            let (graph, fs, scope) = try toolsExplicitLibrariesGraph(linkage: linkage)
+            let plan = try BuildPlan(
+                destinationBuildParameters: mockBuildParameters(shouldLinkStaticSwiftStdlib: true, triple: destinationTriple),
+                toolsBuildParameters: mockBuildParameters(triple: toolsTriple),
+                graph: graph,
+                fileSystem: fs,
+                observabilityScope: scope
+            )
+            let result = try BuildPlanResult(plan: plan)
+            result.checkProductsCount(4)
+            result.checkTargetsCount(6)
+
+            XCTAssertTrue(try result.allTargets(named: "SwiftSyntax")
+                .map { try $0.swiftTarget() }
+                .contains { $0.target.buildTriple == .tools })
+
+            try result.check(buildTriple: .tools, triple: toolsTriple, for: "swift-mmioPackageTests")
+            try result.check(buildTriple: .tools, triple: toolsTriple, for: "swift-mmioPackageDiscoveredTests")
+            try result.check(buildTriple: .tools, triple: toolsTriple, for: "MMIOMacros")
+            try result.check(buildTriple: .tools, triple: toolsTriple, for: "MMIOMacrosTests")
+
+            let macroProducts = result.allProducts(named: "MMIOMacros")
+            XCTAssertEqual(macroProducts.count, 1)
+            let macroProduct = try XCTUnwrap(macroProducts.first)
+            XCTAssertEqual(macroProduct.buildParameters.triple, toolsTriple)
+
+            let swiftSyntaxProducts = result.allProducts(named: "SwiftSyntax")
+            XCTAssertEqual(swiftSyntaxProducts.count, 2)
+            let swiftSyntaxToolsProduct = try XCTUnwrap(swiftSyntaxProducts.first { $0.product.buildTriple == .tools })
+            let archiveArguments = try swiftSyntaxToolsProduct.archiveArguments()
+
+            // Verify that produced library file has a correct name
+            XCTAssertMatch(archiveArguments, [.contains(productFileName)])
+        }
+    }
 }
 
 extension BuildPlanResult {

diff --git a/Tests/PackageGraphTests/CrossCompilationPackageGraphTests.swift b/Tests/PackageGraphTests/CrossCompilationPackageGraphTests.swift
@@ -20,7 +20,7 @@ import XCTest
 
 final class CrossCompilationPackageGraphTests: XCTestCase {
     func testTrivialPackage() throws {
-        let graph = try trivialPackageGraph(pkgRootPath: "/Pkg").graph
+        let graph = try trivialPackageGraph().graph
         try PackageGraphTester(graph) { result in
             result.check(packages: "Pkg")
             // "SwiftSyntax" is included for both host and target triples and is not pruned on this level