-
Notifications
You must be signed in to change notification settings - Fork 83
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
Check the runpaths for more ELF binaries #113
Conversation
# CHECK-SG-NOT: {{.*}} {{\(RPATH\)|\(RUNPATH\)}} {{.*}}:/usr/lib/swift/linux | ||
# | ||
# RUN: %{readelf} -d %{package_path}/usr/lib/swift/linux/libswiftSwiftOnoneSupport.so | %{FileCheck} --check-prefix CHECK-SON %s | ||
# RUN: %{readelf} -d %{package_path}/usr/lib/swift/linux/%{linux_arch}/libswiftSwiftOnoneSupport.so | %{FileCheck} --check-prefix CHECK-SON %s | ||
# CHECK-SON: {{.*}} {{\(RPATH\)|\(RUNPATH\)}} {{.*}}$ORIGIN{{[^/]}} |
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.
As long as I'm updating this file, I added these $ORIGIN
checks too, which are needed for the runtime libraries to find each other.
@bnbarham, now that this passed the CI on the linked compiler pull, looking for review so I can get this in before the upcoming 5.9 branch. Please don't merge though, as all 11 pulls will have to be merged together. |
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.
I'm not sure I'm the best to review these, but assuming your main PR is reasonable than this seems to match that 👍.
Make sure they include $ORIGIN, so they can find each other in the same toolchain.
Removed the changes to add the architecture to the runpath since the Swift devs want to take a different approach, leaving only the other runpath checks I added. One last CI run and this can go in, @bnbarham. |
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.
Looks reasonable to me.
Not sure about the check-string names. SG
, SON
, and RM
don't communicate a lot, but they are prior art, so ¯_(ツ)_/¯.
Passed CI, checked the linux log to make sure this test was run:
This can be merged now. |
Ping @bnbarham, ready for merge. |
Sorry about the delay there, thanks for the ping. The last comment got lost amongst many other emails 😅 |
This is needed for swiftlang/swift#63782, which changes the Unix toolchain to look for libraries in architecture-specific directories.Make sure they include
$ORIGIN
, so they can find each other in the same toolchain.