Skip to content

[5.9] On ELF platforms, only add runpaths as needed (#6321) #6409

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.

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

Merged
merged 1 commit into from
Apr 12, 2023

Conversation

finagolfin
Copy link
Member

Cherrypick of #6321

Explanation: The bootstrap script currently adds two relative runpaths for ELF platforms, one for the PackageDescription and other libraries and another for the single swift-package executable, but since they're applied on the command line, both runpaths are applied to all binaries. This pull separates out the runpaths and only applies each where needed. It also shifts much of the bootstrap script over to applying build flags based on the build target, not on checking the build host platform in Python.

Scope: Should only affect the SPM build on ELF platforms like linux

Issue: None

Risk: low, as it only affects building SPM for ELF platforms

Testing: Passed all CI, including a trunk toolchain build that worked well

Reviewer: @neonichu

As long as I'm modifying the Python bootstrap script, clean up some incorrect
checks of the host `platform.system()` and replace some unnecessary regexes.
@neonichu
Copy link
Contributor

@swift-ci please smoke test

@finagolfin
Copy link
Member Author

Windows CI failure when testing Foundation is unrelated.

@neonichu
Copy link
Contributor

Right, Windows doesn't even use bootstrap, so this PR should be a no op there.

@tomerd could you force merge this?

@tomerd tomerd merged commit 3162d37 into swiftlang:release/5.9 Apr 12, 2023
@finagolfin
Copy link
Member Author

Thanks for the quick review.

@tomerd, should I submit for 5.8 also?

@finagolfin finagolfin deleted the rpaths branch April 13, 2023 05:45
@tomerd
Copy link
Contributor

tomerd commented Apr 13, 2023

sure, but it will have to wait until the next release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants