-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[linux] remove absolute rpath of /usr/lib/swift/linux added to many shared libraries #34023
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
Conversation
…hared libraries This was presumably added as a backup, in case the libraries in a toolchain couldn't be found, but will not work well, so take it out.
As we discussed offline, can you create a PR against swift-integration-test that validates that the snapshots maintain this property? Then I think you can do cross-repo testing to test it with this PR. Then first merge this and then the integration test one. |
Will do. |
…rom the RUNPATH of several Swift libraries, stays that way.
…rom the RUNPATH of several Swift libraries, stays that way.
@drexin, let me know what you think, I'd like to get this into the 5.3.1 patch release. |
@swift-ci smoke test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thanks, sometimes adding a comment seems to get the CI to run? |
I think the CI needs a swift kick. 😉 |
@swift-ci smoke test |
Passes CI, ready to merge. |
I validated in the CI job that swift-ci found the appropriate PR on swift-integration-tests. |
Confirmed that the CI checked out the new integration test and that those tests were run, but the log doesn't list them individually so my new test isn't listed. The Windows CI failure appears unrelated. |
Make sure swiftlang/swift#34023, which removes /usr/lib/swift/linux from the RUNPATH of several Swift libraries, stays that way
This was presumably added as a backup, in case the libraries in a toolchain couldn't be found, but will not work well, so take it out.
Here's a full list of shared library runpaths from the just-released official Swift 5.3 toolchain for linux:
If there's any difference between the toolchain being used and the one installed in
/usr/lib/swift/linux
, say if you download this 5.3 toolchain for linux into your home directory but 5.2.5 is installed in/usr/lib/swift/linux
, using that system runpath won't work properly. I checked and all these libraries already have a runpath relative to $ORIGIN, ie locally, so all this absolute path will do is supply the system Swift library if the local one is missing (obviously if the used toolchain is installed in the system, ie/usr/lib/swift/linux
, this absolute path is redundant). But since using the system Swift libraries will not work properly if the Swift versions don't match, causing subtle bugs in the worst case, I think it's better not to add this absolute runpath at all.It was added five years ago by @bitjammer, ad95b5f, and copied to SourceKit a couple years later, a93bddf. I brought up this issue earlier this year, but got no response at the time, so I let it go. I think this is worth getting in and into the 5.3 branch for the next patch release.
@gottesmm and @compnerd, I think this should be removed, similar to the other ELF runpath issues we discussed. @tachoknight, let us know what you think from your Fedora packaging perspective.