Skip to content

[5.4][CMake] fix runpath for ELF platforms #321

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
Mar 12, 2021

Conversation

finagolfin
Copy link
Member

  • Explanation

Remove the absolute path to the host toolchain's stdlib from libXCTest.so.

This is a cherry-pick of #303, which was just approved for trunk and already merged for the 5.3 branch in #308.

  • Scope

Only affects the rpath on ELF platforms, removing a path that is unused.

  • SR issue

Resolves point 2. in SR-1650

  • Risk

None, this should be completely safe for the 5.4 branch, as it's just removing the build toolchain's stdlib rpath, which should be unused by this library.

  • Testing

This is in the 5.3 branch and the other corelibs had similar pulls merged before the 5.3 release and have had no complaints.

  • Reviewer

I discussed these rpath issues in detail across the entire toolchain with @gottesmm and @compnerd.

Remove the absolute path to the host toolchain's stdlib from libXCTest.so.
@finagolfin
Copy link
Member Author

Ping @compnerd, mind approving this too?

@finagolfin
Copy link
Member Author

@briancroom, this makes sure 5.4 doesn't regress from 5.3, please review and merge.

@briancroom
Copy link
Contributor

@swift-ci please test

@finagolfin
Copy link
Member Author

Linux CI failure is unrelated to this pull.

@finagolfin
Copy link
Member Author

Ping, another linux CI run and this can be merged.

@gottesmm
Copy link
Contributor

@swift-ci smoke test linux platform

@gottesmm
Copy link
Contributor

@swift-ci please test

@finagolfin
Copy link
Member Author

@briancroom, this is ready to merge, think we can get it in before the 5.4 release? It's already in the 5.3 branch without a problem.

@briancroom
Copy link
Contributor

Yes, let's get this merged. It looks like the checks are in a bit of a confused state, so I'll go ahead and merge it manually.

@briancroom
Copy link
Contributor

Looks like the release branch isn't configured to allow a direct push from me, actually. @shahmishal, can you help us merge this PR?

@shahmishal
Copy link
Member

@briancroom Branch requirements updated

@briancroom
Copy link
Contributor

Thank you!

@briancroom briancroom merged commit 8ad2cec into swiftlang:release/5.4 Mar 12, 2021
@finagolfin
Copy link
Member Author

Thanks all.

@finagolfin finagolfin deleted the fix-rpath branch March 12, 2021 02:57
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.

4 participants