configure: Enable -C rpath by default #30353
Conversation
This commit changes our distribution and in-tree sources to pass the `-C rpath` flag by default during compiles. This means that from-source builds, including our release channels, will have this option enabled as well. Motivated by rust-lang#29941, this change means that the compiler should be usable as-is on all platforms just after extraction or installation. This experience is already true on Windows but on Unixes you still need to set up LD_LIBRARY_PATH or the equivalent, which can often be unfortunate. This option was originally turned off by default for Linux distributions who tend to take care of these sorts of details themselves, so it is expected that all those builds of Rust will want to pass `--disable-rpath` to the configure script to preserve that behavior. Closes rust-lang#29941
|
r? @brson (rust_highfive has picked a reviewer for you, use r? to override) |
|
Even though I use --enable-rpath almost exclusively when developing… I do not think this is a correct thing to do for our tarballs distributed at static.rust-lang.org. Namely, I use the rust-nightly-bin package from AUR which extracts and installs prebuilt rustc from static.rust-lang.org and there’s no way to pass |
|
@alexcrichton I am thrilled to see us go in this direction. @nagisa can you indicate why having rpath-enabled on |
|
@pnkfelix because
While it might work well for cases where install scripts simply do an equivalent of |
|
I would expect it to be pretty rare for an official distribution to just download our nightlies and repackage them. For example in doing this you can't change the way we link to native libraries like LLVM which I also suspect most distributions will want to do. Along those lines I don't think that we should necessarily support the use case of having our nightlies be ready for distribution in other package managers as-is. I also believe there are tools (patchelf maybe?) which can modify this sort of information on built artifacts. The high-order bit here I believe is not what distributions are doing but rather what we are doing. We go to great lengths to ensure that rustc is usable as-is on Windows, and the same should be true of Linux/OSX as well. |
|
I posted a link to this in the packaging thread and will give it a few days to percolate, see how packagers might feel about it. |
|
As a user, +1 for this change for rustc itself (and rustdoc, cargo, etc.), for the official binary distribution, so that @nagisa rpath is essentially just another source of search paths, alongside It is true that we use Also, on current stable rust, the rpath I get from |
|
@geofft I think there may be a longstanding bug or two about precisely what |
|
@bors r+ |
|
📌 Commit 9bff8b0 has been approved by |
brson
added
the
relnotes
This commit changes our distribution and in-tree sources to pass the `-C rpath` flag by default during compiles. This means that from-source builds, including our release channels, will have this option enabled as well. Motivated by #29941, this change means that the compiler should be usable as-is on all platforms just after extraction or installation. This experience is already true on Windows but on Unixes you still need to set up LD_LIBRARY_PATH or the equivalent, which can often be unfortunate. This option was originally turned off by default for Linux distributions who tend to take care of these sorts of details themselves, so it is expected that all those builds of Rust will want to pass `--disable-rpath` to the configure script to preserve that behavior. Closes #29941
|
💔 Test failed - auto-linux-64-nopt-t |
|
@bors: retry On Tue, Dec 22, 2015 at 6:42 PM, bors notifications@github.com wrote:
|
|
⚡ Previous build results for auto-linux-32-opt, auto-linux-cross-opt, auto-linux-musl-64-opt, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-msvc-32-opt, auto-win-msvc-64-opt are reusable. Rebuilding only auto-linux-32-nopt-t, auto-linux-64-debug-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-win-gnu-32-nopt-t, auto-win-gnu-32-opt, auto-win-gnu-64-nopt-t, auto-win-gnu-64-opt... |
|
Sorry, I didn't notice this PR before it actually got merged, but...
I'm not sure this is the right move. Surely, those that care about the rpaths will notice their builds of newer versions will get one, and will look at how to remove it. But why not just change whatever script is used to produce the official builds to add --enable-rpath instead of changing the defaults for everyone? That being said, I can understand that rustc developers also benefit from not having to set LD_LIBRARY_PATH for themselves, which is why I have doubts. |
|
@glandium that is indeed a possible alternative! The case against that, however, is that almost all builds from source will want this option set (e.g. local development, custom installations, etc). Only builds by distributions will want to set this, and that seems like a small enough set that it's not necessarily the right default. |
|
rpath is still broken on OS X, right? |
|
@alexcrichton silly question: Did this patch actually work ? I was just reviewing what ends up in the |
|
(in fact, the awesome thing is that it looks like this PR on its own breaks Maybe we should change the generation of |
…dash finish enabling `-C rpath` by default in rustc. See #30353.
This commit changes our distribution and in-tree sources to pass the
-C rpathflag by default during compiles. This means that from-source builds, including
our release channels, will have this option enabled as well. Motivated
by #29941, this change means that the compiler should be usable as-is on all
platforms just after extraction or installation. This experience is already true
on Windows but on Unixes you still need to set up LD_LIBRARY_PATH or the
equivalent, which can often be unfortunate.
This option was originally turned off by default for Linux distributions who
tend to take care of these sorts of details themselves, so it is expected that
all those builds of Rust will want to pass
--disable-rpathto the configurescript to preserve that behavior.
Closes #29941