configure: Enable -C rpath by default

[alexcrichton]

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

[rust-highfive]

r? @brson

(rust_highfive has picked a reviewer for you, use r? to override)

[nagisa]

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 --disable-rpath for prebuilt binaries.

[pnkfelix]

@alexcrichton I am thrilled to see us go in this direction.

@nagisa can you indicate why having rpath-enabled on rustc (but not on its build products, at least not be default) is problematic for your use case? Is it something specific to Arch, or is this an instance of a problem that other distributions have? (There's lots of other issues involved here that are elsewhere on this repo; its possible you might be able to answer my question by just linking one or more of them.)

[nagisa]

@pnkfelix because

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.

While it might work well for cases where install scripts simply do an equivalent of mv -r ./bin/ ./lib/ /usr/, it is likely to stop working for anything else that used to work before (where a system was configured properly, no less!)… or perhaps I do not know something about rpath and dynamic linkers do fallback to searching for libraries by non-rpath means if they can’t find any at provided rpath?

[alexcrichton]

@nagisa

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.

[brson]

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.

[geofft]

As a user, +1 for this change for rustc itself (and rustdoc, cargo, etc.), for the official binary distribution, so that rustup --prefix=/home/$USER works. As far as I can tell, the previous issues have been about setting the rpath in generated binaries (which seems unnecessary given that the default is statically linking libstd), and about unconditionally setting it without a ./configure option, which is no longer the case.

@nagisa rpath is essentially just another source of search paths, alongside LD_LIBRARY_PATH, /etc/ld.so.conf, the hard-coded defaults. Linux's man page for ld.so has a list of where it looks. Providing an rpath is harmless, unless there's a library at the rpath location that's unwanted.

It is true that we use DT_RPATH instead of DT_RUNPATH, so you can't override it with LD_LIBRARY_PATH. I'll file a bug about that.

Also, on current stable rust, the rpath I get from -C rpath is $ORIGIN/../usr/local/lib/rustlib/x86_64-unknown-linux-gnu/lib:/usr/local/lib/rustlib/x86_64-unknown-linux-gnu/lib. Is that... correct? I don't think it'll cause a prefixed compiler to find its own bits. And it seems like /usr/local shouldn't be included, and only the $ORIGIN-relative rpath should be included.

[geofft]

Filed #30378 to switch to DT_RUNPATH, to give LD_LIBRARY_PATH preference over the rpath, though in my opinion it shouldn't block this PR.

Also is #17219 "-C rpath does not work on OS X" still an issue and does it affect this PR?

[alexcrichton]

@geofft I think there may be a longstanding bug or two about precisely what -C rpath does, but at least for me whenever using the compiler the ones we've generated have always worked.

[brson]

@bors r+

[bors]

📌 Commit 9bff8b0 has been approved by brson

[bors]

⌛ Testing commit 9bff8b0 with merge 45e52da...

[bors]

💔 Test failed - auto-linux-64-nopt-t

[alexcrichton]

@bors: retry

On Tue, Dec 22, 2015 at 6:42 PM, bors notifications@github.com wrote:

[image: 💔] Test failed - auto-linux-64-nopt-t
http://buildbot.rust-lang.org/builders/auto-linux-64-nopt-t/builds/7468

—
Reply to this email directly or view it on GitHub
#30353 (comment).

[bors]

⚡ 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...

[bors]

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-debug-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-linux-cross-opt, auto-linux-musl-64-opt, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-gnu-32-nopt-t, auto-win-gnu-32-opt, auto-win-gnu-64-nopt-t, auto-win-gnu-64-opt, auto-win-msvc-32-opt, auto-win-msvc-64-opt

[glandium]

Sorry, I didn't notice this PR before it actually got merged, but...

so it is expected that all those builds of Rust will want to pass --disable-rpath to the configure
script to preserve that behavior.

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.

[alexcrichton]

@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.

[comex]

rpath is still broken on OS X, right?

[pnkfelix]

@alexcrichton silly question: Did this patch actually work ?

I was just reviewing what ends up in the config.mk and the logic in the makefiles; it looks to me like in addition to changing the argument in opt rpath 0 to opt rpath 1, one also needs to change the main.mk to use ifndef CFG_DISABLE_RPATH (from the current ifdef CFG_ENABLE_RPATH)

[pnkfelix]

(in fact, the awesome thing is that it looks like this PR on its own breaks --enable-rpath, since it causes the CFG_ENABLE_RPATH=1 to no longer be emitted into the config.mk.)

Maybe we should change the generation of config.mk to include all opt's, with their default values if not overriden, rather than just the options supplied by the user.

[alexcrichton]

@pnkfelix oh dear I thought I checked to make sure but apparently I did not, thanks for catching that and submitting #30739!