Allow overriding default runtime CC and AR commands at compile time

[Keruspe]

No description provided.

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @pcwalton (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[Aatch]

Why is this change being made? What problem are you solving here? This needs to go into the commit message/PR description. Right now you appear to making a change to something that has worked fine for quite a while.

[alexcrichton]

I agree that I'd like to see some more motivation here. I generally think environment variables for something like this isn't the greatest solution because it doesn't handle cross compilation well. You don't actually want CC to be inherited to all compiler invocations because they may not all target the same platform (especially when invoked through Cargo)

[Keruspe]

I realize thatthis comes indeed a little bit our of nowhere, let me add some background to it.

The rationale is actually very tightly linked to cross compilation.

I'm one of the exherbo (a source-based distribution) developpers, and we recently added support for cross-compilation at the core of uour distribution. As we wanted everything to be homogeneous, even the native host is considered as being cross-compiled.

To achieve that, all of our toolchain binaries are prefixed with the architecture they target.

this means that binaries such as "cc" or "ar" don't exist at all. "x86_64-pc-linux-gnu-cc" and friends exist though.

Making these defaults configurable at build time would really help (and every language we packages so far allowed to do that)

The env var way was just the simplest solution I came with, but I'm all ears if something better could be done. As for the env method, I'm totally ok not to use CC and AR directly but some other env variables which only aim is that.

[Aatch]

Using variables from the configuration system seems like a better way to go. Right now it seems more like somebody is using the option_env! macro incorrectly. At least, that was my first thought when I saw the change. Seeing option_env!("CFG_CC") would be more clear.

[Keruspe]

Will make some tests with configuration variables.

[alexcrichton]

Note that you probably don't need to configure ar at all as LLVM already has a cross-platform-aware implementation which we use, so the only thing which needs configuring here is the linker. This can already be modified via the -C linker flag to the compiler or via Cargo configuration.

One of the problems with something like CC for Rust code is that it's not always cross compiled that frequently. Most Cargo builds nowadays end up having a build script somewhere in the dependency tree (dealing with a native library or code generation) which needs to be compiled for the host platform, not the target platform, meaning that the value of CC would be wrong. This is why in Cargo you specify the linker per platform rather than a global linker.

[Keruspe]

Here is another attempt, using CFG_CC and a newly defined CFG_AR

Note that ar is needed otherwise libraries compilation fail with "could not execute ar"

I know that these are runtime configurable, the point here is not to change that. All the cross compilation toolchains specifying them explicitely will obviously still work.

The point here is to get a rustc with working defaults if cc and ar don't exist.

[alexcrichton]

The current master branch of the compiler almost never falls down to the system ar, that only happens for nonstandard targets where we don't know the archive format. For that it may be best to just expose archive_format in the custom target json so the system doesn't need to have an ar tool.

This also means that CFG_CC is only read when the compiler is built, not when it's run, which seems especially odd? These sorts of defaults sound like they should be in the custom target fields rather than for all targets.

[Keruspe]

When you say that the current master branch almost never falls down to the system ar, is it something new in 1.3/1.4 or is it supposed to already be the case for 1.2 too?

The fact that it's read only at compile time and not at runtime is exactly the point of these changes.

At runtime neither "cc" nor "ar" are available and we do not want our users to have to export some env vars or pass specific options to cargo/rustc for it to work. During compilation, the package manager exports stuff like CC and AR to the correct values which is why I want those to be interpreted at compile time.

For now, we have some dirty sed in the package, while we try to stay as vanilla as possible. That's why I'm trying to do something upstream here.

What would you suggest? If this way is not acceptable, maybe specific configure switch would be ok? like --with-default-runtime-cc=foo or such?

When you speak of the custom target json, what are you referring to exactly? (I must admit I'm quite new to rust, and even newer to the rustc source base). I might be missing an obvious solution

[alexcrichton]

Do you know what target you're compiling by default? Is it the standard x86_64-unknown-linux-gnu or is it more specialized?

When you say that the current master branch almost never falls down to the system ar, is it something new in 1.3/1.4 or is it supposed to already be the case for 1.2 too?

This landed relatively recently in 1.3 I believe, it's not in 1.2.

The fact that it's read only at compile time and not at runtime is exactly the point of these changes.

Hm, ok. One of the differences between rustc and gcc is that the Rust compiler is more likely to be a cross compiler than any old build of gcc. For example all Rust compilers can target any platform, whereas (to the best of my knowledge) gcc generally targets only one or two platforms at a time. This means that a compile-time decision about the "default linker" isn't actually quite right because the compiler could be used to target any number of platforms.

For now, we have some dirty sed in the package, while we try to stay as vanilla as possible. That's why I'm trying to do something upstream here.

I totally agree! Just trying to get a handle on what's going on here!

What would you suggest? If this way is not acceptable, maybe specific configure switch would be ok? like --with-default-runtime-cc=foo or such?

When you speak of the custom target json, what are you referring to exactly? (I must admit I'm quite new to rust, and even newer to the rustc source base). I might be missing an obvious solution

If you're using your own custom target triple then you each target can indicate its own custom linker (which sounds like what you want). If you're using the standard triples, though, it looks like we're gonna need a solution that looks something along the lines of this.

[Keruspe]

Here is what a configure call by our package looks like (some enable/disable might change as they're configurable, for docs for example, the release channel is configurable too)

./configure --target=x86_64-unknown-linux-gnu --prefix=/usr/x86_64-pc-linux-gnu --datadir=/usr/share --mandir=/usr/share/man --release-channel=dev --disable-llvm-assertions --disable-docs --llvm-root=/usr/x86_64-pc-linux-gnu

I believe that's the correct way for building for x86_64-unknown-linux-gnu

As for the ar stuff, I'm currently testing against 1.2, that would explain why I'm hitting this, if that's changed in 1.3, that fine then.

I didn't know that a single rustc binary could handle cross compilation that well, that's really nice to hear! To clarify things, I just want the default linker for a 64-bits ELF to be the x86_64 linker, hope that makes sense, even with cross-compilation considered.

Maybe we should switch to some x86_64-pc-linux-gnu target and provide a specific json for that?

If we want to stick with the standard triple, given the travis failure, I guess we should either support the linker to be command args and not just command or switch to come specific configure switch validating the value.

[alexcrichton]

Hm ok, interesting! Using the default Linux triple makes it a little more difficult to solve, but certainly not insurmountable! I feel that for packaging and distribution purposes that it's generally pretty unlikely for the standard compiler to be used in many cross-compilation situations, so it may not be too bad to just blanket set the default...

Out of curiosity, is the ld program prefixed as well? Or does it exist as just vanilla ld? Also, how often do you cross compile from 32 to 64-bit or vice versa?

cc @brson, this will probably end up adding another ./configure option I think

[brson]

@alexcrichton Word. I don't see a better alternative, though I am enduring constant deep despair at the state of our configure script.

Thank god they are not trying to change --libdir!

[brson]

@eddyb pointed out this patch for NixOS, which needs this same thing.

[Keruspe]

@brson seems to have the same problem that the current patch here though.

Anyway, I hope to be able to update this in a couple of hours

[Keruspe]

@alexcrichton does something like this sounds ok ?

Should I squash the commits?

[alexcrichton]

Ok, this looks good to me, thanks @Keruspe! Could you also squash the commits together?

[Keruspe]

@alexcrichton squashed and rebased

Any chance for this to be in 1.3 or will it have to wait 1.4?

[alexcrichton]

@bors: r+ c977596

Thanks! We generally don't backport anything to beta unless absolutely necessary, so this should make its way into 1.4

[bors]

⌛ Testing commit c977596 with merge adb423f...

[bors]

💔 Test failed - auto-mac-32-opt

[eddyb]

@bors retry

[bors]

⌛ Testing commit c977596 with merge e565e44...

[bors]

💔 Test failed - auto-linux-64-opt

[eddyb]

Since when do we get spurious segfaults on linux?!

[alexcrichton]

@bors: retry

[bors]

⌛ Testing commit c977596 with merge 2fa14da...

[bors]

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

[alexcrichton]

@bors: retry

On Fri, Aug 21, 2015 at 2:41 PM, bors [email protected] wrote:

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

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

[bors]

⚡ Previous build results for auto-linux-64-opt, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-msvc-32-opt are reusable. Rebuilding only auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-nopt-t, 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, auto-win-msvc-64-opt...

[bors]

⌛ Testing commit c977596 with merge a03ec1a...

[bors]

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, 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