-
Notifications
You must be signed in to change notification settings - Fork 13.3k
rustbuild: Fix LC_ID_DYLIB directives on OSX #38440
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
r? @eddyb (rust_highfive has picked a reviewer for you, use r? to override) |
r? @brson |
I'm also going to preemptively tag this as beta-nominated. If this misses the beta branch for 1.15 we'll want to be sure to backport this to 1.15. |
wtf @bors r+ p=1 |
📌 Commit 1472ed1 has been approved by |
@bors r- Actually, could you add more comments explaining that this is a hack and should not exist and wtf. |
Currently libraries installed by rustbuild on OSX have an incorrect `LC_ID_DYLIB` directive located in the dynamic libraries that are installed. The directive we expect looks like: @rpath/libstd.dylib Which means that if you want to find that dynamic library you should look at the dylib's other `@rpath` directives. Typically our `@rpath` directives look like `@loader_path/../lib` for the compiler as that's where the installed libraries will be located. Currently, though, rustbuild produces dylibs with the directive that looks like: /Users/rustbuild/src/rust-buildbot/slave/nightly-dist-rustc-mac/build/build/x86_64-apple-darwin/stage1-std/x86_64-apple-darwin/release/deps/libstd-713ad88203512705.dylib In other words, the build directory is encoded erroneously. The compiler already [knows how] to change this directive, but it only passes that argument when `-C rpath` is also passed. The rustbuild system, however, explicitly [does not pass] this option explicitly and instead bakes its own. This logic then also erroneously didn't pass `-Wl,-install_name` like the compiler. [knows how]: https://github.com/rust-lang/rust/blob/4a008cccaabc8b3fe65ccf5868b9d16319c9ac58/src/librustc_trans/back/linker.rs#L210-L214 [does not pass]: https://github.com/rust-lang/rust/blob/4a008cccaabc8b3fe65ccf5868b9d16319c9ac58/src/bootstrap/bin/rustc.rs#L133-L158 To fix this regression this patch introduces a new `-Z` flag, `-Z osx-rpath-install-name` which basically just forces the compiler to take the previous `-install_name` branch when creating a dynamic library. Hopefully we can sort out a better rpath story in the future, but for now this "hack" should suffice in getting our nightly builds back to the same state as before. Closes rust-lang#38430
1472ed1
to
1b8e6c1
Compare
@bors: r=brson |
📌 Commit 1b8e6c1 has been approved by |
rustbuild: Fix LC_ID_DYLIB directives on OSX Currently libraries installed by rustbuild on OSX have an incorrect `LC_ID_DYLIB` directive located in the dynamic libraries that are installed. The directive we expect looks like: @rpath/libstd.dylib Which means that if you want to find that dynamic library you should look at the dylib's other `@rpath` directives. Typically our `@rpath` directives look like `@loader_path/../lib` for the compiler as that's where the installed libraries will be located. Currently, though, rustbuild produces dylibs with the directive that looks like: /Users/rustbuild/src/rust-buildbot/slave/nightly-dist-rustc-mac/build/build/x86_64-apple-darwin/stage1-std/x86_64-apple-darwin/release/deps/libstd-713ad88203512705.dylib In other words, the build directory is encoded erroneously. The compiler already [knows how] to change this directive, but it only passes that argument when `-C rpath` is also passed. The rustbuild system, however, explicitly [does not pass] this option explicitly and instead bakes its own. This logic then also erroneously didn't pass `-Wl,-install_name` like the compiler. [knows how]: https://github.com/rust-lang/rust/blob/4a008cccaabc8b3fe65ccf5868b9d16319c9ac58/src/librustc_trans/back/linker.rs#L210-L214 [does not pass]: https://github.com/rust-lang/rust/blob/4a008cccaabc8b3fe65ccf5868b9d16319c9ac58/src/bootstrap/bin/rustc.rs#L133-L158 To fix this regression this patch introduces a new `-Z` flag, `-Z osx-rpath-install-name` which basically just forces the compiler to take the previous `-install_name` branch when creating a dynamic library. Hopefully we can sort out a better rpath story in the future, but for now this "hack" should suffice in getting our nightly builds back to the same state as before. Closes #38430
This made it to beta, de-nominating |
Currently libraries installed by rustbuild on OSX have an incorrect
LC_ID_DYLIB
directive located in the dynamic libraries that areinstalled. The directive we expect looks like:
Which means that if you want to find that dynamic library you should
look at the dylib's other
@rpath
directives. Typically our@rpath
directives look like
@loader_path/../lib
for the compiler as that'swhere the installed libraries will be located. Currently, though,
rustbuild produces dylibs with the directive that looks like:
In other words, the build directory is encoded erroneously. The compiler
already knows how to change this directive, but it only passes that
argument when
-C rpath
is also passed. The rustbuild system, however,explicitly does not pass this option explicitly and instead bakes its
own. This logic then also erroneously didn't pass
-Wl,-install_name
like the compiler.
To fix this regression this patch introduces a new
-Z
flag,-Z osx-rpath-install-name
which basically just forces the compiler to takethe previous
-install_name
branch when creating a dynamic library.Hopefully we can sort out a better rpath story in the future, but for
now this "hack" should suffice in getting our nightly builds back to the
same state as before.
Closes #38430