Fix incorrect paths for library products with explicit linkage #7519
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Library products declared with `.static` or `.dynamic` 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. rdar://127170225
MaxDesiatov
added
bug
cross-compilation
build system
|
@swift-ci test |
bnbarham
approved these changes
May 1, 2024
| @@ -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)") | |||
There was a problem hiding this comment.
How likely is it that we're missing other suffix's 😅? Is there any decent way to look for all of them?
There was a problem hiding this comment.
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.
furby-tm
pushed a commit
to wabiverse/swift-package-manager
that referenced
this pull request
May 15, 2024
…lang#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
furby-tm
pushed a commit
to wabiverse/swift-package-manager
that referenced
this pull request
May 15, 2024
…lang#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
MaxDesiatov
added a commit
that referenced
this pull request
Jun 6, 2024
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 (cherry picked from commit 8db2401) # Conflicts: # Tests/BuildTests/CrossCompilationBuildPlanTests.swift
xedin
added a commit
that referenced
this pull request
Jun 10, 2024
**Explanation**: Macros cross-compiled by SwiftPM with Swift SDKs should be correctly built, loaded, and evaluated for the host triple. **Scope**: isolated to modules dependency resolution and llbuild, does not impact code related to XCBuild. **Risk**: medium, known issues were addressed on `main` and are cherry-picked here, with no new issues reported for a few weeks now. **Testing**: added unit tests, manual end-to-end testing done with existing Swift SDKs. **Issue**: rdar://105991372 **Reviewers**: @bnbarham @xedin @neonichu (cherry picked from commit cb3b085, #7353) ``` # Conflicts: # CHANGELOG.md # Sources/Build/BuildDescription/SwiftTargetBuildDescription.swift # Sources/Build/BuildManifest/LLBuildManifestBuilder.swift # Sources/Build/BuildPlan/BuildPlan.swift # Sources/Commands/SwiftTestCommand.swift # Sources/Commands/Utilities/PluginDelegate.swift # Sources/Commands/Utilities/TestingSupport.swift # Sources/PackageGraph/ModulesGraph+Loading.swift # Sources/PackageGraph/ModulesGraph.swift # Sources/SPMTestSupport/MockBuildTestHelper.swift # Sources/SPMTestSupport/MockPackageGraphs.swift # Sources/SPMTestSupport/PackageGraphTester.swift # Sources/SPMTestSupport/ResolvedTarget+Mock.swift # Tests/BuildTests/ClangTargetBuildDescriptionTests.swift # Tests/BuildTests/CrossCompilationBuildPlanTests.swift # Tests/SourceKitLSPAPITests/SourceKitLSPAPITests.swift ``` (cherry picked from commit b9eb3c1, #7493) ``` # Conflicts: # Sources/PackageGraph/ModulesGraph+Loading.swift # Sources/PackageGraph/Resolution/ResolvedPackage.swift ``` (cherry picked from commit 5a4c024, #7508) ``` # Conflicts: # Sources/Commands/SwiftBuildCommand.swift # Sources/Commands/SwiftTestCommand.swift # Sources/Commands/Utilities/TestingSupport.swift ``` (cherry picked from commit 8db2401, #7519) ``` # Conflicts: # Tests/BuildTests/CrossCompilationBuildPlanTests.swift ``` --------- Co-authored-by: Jonathan Grynspan <jgrynspan@apple.com> Co-authored-by: Ben Barham <ben_barham@apple.com> Co-authored-by: Yuta Saito <kateinoigakukun@gmail.com> Co-authored-by: Pavel Yaskevich <xedin@apache.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Library products declared with
.staticor.dynamiclinkage didn't have a-toolssuffix 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-assertdepending on an old version ofswift-syntaxwith explicit.staticlinkage for its products.rdar://127170225