Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix incorrect paths for library products with explicit linkage #7519

Merged
merged 1 commit into from
May 1, 2024
Merged

Fix incorrect paths for library products with explicit linkage #7519

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Sources/SPMBuildCore/BuildParameters/BuildParameters.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How likely is it that we're missing other suffix's 😅? Is there any decent way to look for all of them?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that we're constructing these paths in many places in an ad-hoc way, it is still somewhat likely. I'll do an audit of this code and also generalize path handling to avoid ad-hocness in a follow-up PR not to stall this one.

}

/// Returns the path to the binary of a product for the current build parameters, relative to the build directory.
Expand All @@ -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:
Expand Down
68 changes: 66 additions & 2 deletions Sources/SPMTestSupport/MockPackageGraphs.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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)
}
53 changes: 47 additions & 6 deletions Tests/BuildTests/CrossCompilationBuildPlanTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion Tests/PackageGraphTests/CrossCompilationPackageGraphTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down