Make rustbuild force_alloc_system rather than relying on stage0

[aidanhs]

This 'fixes' jemalloc-less local rebuilds, where we tell cargo that we're actually stage1 (this only fixes the rustbuild path, since I wasn't enthusiastic to dive into the makefiles).

There should be one effect from this PR: --enable-local-rebuild --disable-jemalloc will successfully build a stage0 std (rather than erroring). Ideally I think it'd be nice to specify an allocator preference in Cargo.toml/cargo command line (used when an allocator must be picked i.e. dylibs, not rlibs), but since that's not possible we can make do with a force_alloc_system feature. Sadly this locks you into a single allocator in the build libstd, making any eventual implementation of #38575 not quite right in this edge case, but clearly not many people exercise the combination of these two flags.

This PR is also a substitute for #37975 I think. The crucial difference is that the feature name here is distinct from the jemalloc feature (reused in the previous PR) - we don't want someone to be forced into alloc_system just for disabling jemalloc!

Fixes #39054

r? @alexcrichton

[aidanhs]

I've tweaked this PR a bit as I wasn't happy with it.

Previously I moved all the detection of whether to use liballoc_system into rustbuild. The fallout was that all stage0 libstds, whether they were local rebuilds or not, ended up with a hard dependency on liballoc_system, making #38575 much more painful.

I've now stripped back the PR to make it less invasive, just handling the specific case of --enable-local-rebuild --disable-jemalloc to move the needle and make it not error. In addition, it exposes a feature that libstd consumers can make use of. I'm still fairly convinced that there's no ideal solution here until users can select an allocator to be the default without changing source code and creating hard dependencies. Is there anything available along these lines that I've missed? Or perhaps it's been discussed somewhere?

[aidanhs]

Yes, this has been discussed before - see #27389. As part of that, there's currently an implementation of picking jemalloc or system based on what's being produced -

rust/src/librustc_metadata/creader.rs

Line 822 in ff591b6

// * Binaries use jemalloc

So all that would be necessary is some way to alter/override the properties of the target you're compiling to, then you can pick the allocator at your leisure, which would permit making this section of code somewhat more robust. Perhaps something I can pick up at a later date.

[alexcrichton]

Thanks for the PR! I must admit though that I'm pretty distanced from all the issues here, but I'm also not super enthusiastic about this. If the standard library forcibly links to an allocator, then it blocks out all rust programs from bringing in a custom allocator, which wasn't intended.

Can you describe in a bit more the motivation for this issue? What toolchain doesn't have jemalloc?

[aidanhs]

If the standard library forcibly links to an allocator, then it blocks out all rust programs from bringing in a custom allocator, which wasn't intended.

This happens already in stage0. This PR just changes that so it also happens in a local rebuild stage0 (where we pretend that we're stage1 as far as cargo etc are concerned).

Before this, a local rebuild stage0 would say "hey, I'm stage1" and so libstd would link the default allocator of the rust being used to build it. This normally works fine (if you've disabled jemalloc in ./configure, then the stage1 compiler will default to system) but in a local rebuild you're using the local compiler which will typically default to jemalloc - this won't work if you've disabled jemalloc in ./configure, because the jemalloc crate hasn't been compiled!

The only build combination this will effect is a stage0 local rebuild with jemalloc disabled - previously it wouldn't work at all, now it will successfully build but with a hardcoded allocator.

As I mentioned, I think the better solution is to allow overriding target.json fields on-the-fly at compile time, but it'll require significantly more effort.

What toolchain doesn't have jemalloc?

This is more about me just not wanting jemalloc in a particular use case, but according to #37975 it seems like a useful feature for cross compilation as well.

[alexcrichton]

@aidanhs ah yes sorry I missed that part earlier, thanks for the clarification! Also for the motivation I'm curious but where is a local rebuild coming from that doesn't have jemalloc?

Also would it be possible to implement this with modifying the set of features? We've had weird bugs in the past where recomiples happened where they shouldn't and they always point back to changing features between stages.

[aidanhs]

@alexcrichton I'm not sure if this is your question, but the problem precisely is that local rustc does have jemalloc (i.e. you can observe the problem with any nightly) - the stage0 being built does not have jemalloc because I configured with --disable-jemalloc, so when the local rustc attempts to link to alloc_jemalloc (which it does by default), it fails.

If you're asking why I'm building with --disable-jemalloc, it's because I want to be able to swap out the system malloc easily at runtime (in libc) without jemalloc getting in the way.

Hmm, interesting question. I don't think this is possible to do without adding a lever to pull (in the form of a feature) because it's something that cuts across the concerns of other flags. Do you have any example issues off the top of your head so I can get a feel for what the spurious rebuilds looked like and how they were resolved? One thing that may be mildly reassuring is that the build path this affects didn't work at all before - spurious rebuilds are a step forward!

[alexcrichton]

Ah ok that all makes sense to me, thanks for clearing it up!

I can't recall a specific issue off the top of my head, and in general --disable-jemalloc builds are pretty rare (especially in dev) so this is probably not going to cause too many problems. In that case...

@bors: r+

[bors]

📌 Commit 70d2372 has been approved by alexcrichton

[bors]

⌛ Testing commit 70d2372 with merge 633f38a...

[bors]

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 633f38a to master...