Force sysroot to have different hashes

[roblabla]

A terrible workaround a problem I found: If the sysroot and project share a dependency, it might cause conflicts, leading to confusing cargo errors.

[RalfJung]

Sorry for the long silence. I am slowly going over our backlog of PRs. ;)

How can sysroot and project share a dependency? Even when libstd depends on a crate, that should be completely separate from the crate that the project itself uses.

I also don't understand what all those changes about dirs and message-format have to do with the PR description.

[roblabla]

I also don't understand what all those changes about dirs and message-format have to do with the PR description.

They're unrelated, I accidentally made the PR with the master branch of my fork. I'll rebase this banch so it only has the sysroot fix later, and open PRs for the other commits.

How can sysroot and project share a dependency? Even when libstd depends on a crate, that should be completely separate from the crate that the project itself uses.

Well, when I was working on megaton-hammer, I had a custom libstd with direct crates.io dependencies before the libstd developed real support for it. So xargo would compile and link this dependency for the libstd, and then compile and link it again when it was used for the "final" crate. This would cause cargo to produce an error about how the crates conflict:

   Compiling megaton-example v0.1.0 (file:///home/roblabla/Dropbox/dev/src/rust/megatonhammer/megaton-example)
error[E0523]: found two different crates with name `spin` that are not distinguished by differing `-C metadata`. This will result in symbol conflicts between the two.
 --> megaton-example/src/main.rs:3:1
  |
3 | extern crate megaton_hammer;
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

If you want more information on this error, try using "rustc --explain E0523"
error: Could not compile `megaton-example`.

This would only happen if the libstd and crate are built in the same mode (e.g. both debug or both release) and the featureset used in both sides end up being the same.

The reason why this isn't a problem in Rust's libstd is that all dependencies are built with a rustc-dep-of-std feature, giving them a different hash. I'm pretty sure such a conflict would occur if a crate tried to build hashbrown with rustc-dep-of-std.

[RalfJung]

The reason why this isn't a problem in Rust's libstd is that all dependencies are built with a rustc-dep-of-std feature, giving them a different hash. I'm pretty sure such a conflict would occur if a crate tried to build hashbrown with rustc-dep-of-std.

That sounds like a wrong use of hashbrown though. These features specifically exist to let libstd depend on crates, and depending on crates in another way is not supported.

[roblabla]

It absolutely is a wrong use. I'm just pointing out that the same underlying "issue" of crates sharing the same featureset on both side failing a build is likely to exist with the current implementation, and is simply being worked around by having a "private" feature that shouldn't be used by anyone.

[RalfJung]

Interesting. I thought the feature mostly served for dependency tracking, and that not picking up the "wrong" crate is done through the sysroot handling (the rustc sysroot also includes a whole bunch of other crates, ones that rustc depends on, that do not have a special feature; AFAIK rustc treats those crates specially to make sure they do not get picked).

The question remains, though, why unsupported/incorrect ways of adding crate dependencies to libstd should work with xargo? We barely have the capacity to keep "normal" uses of xargo working.

[jethrogb]

The question remains, though, why unsupported/incorrect ways of adding crate dependencies to libstd should work with xargo?

There could be dependencies of std that don't require special features. Those would then conflict with uses of that crate by the user. This is clearly not incorrect, but maybe you'd say it's unsupported?

[RalfJung]

There could be dependencies of std that don't require special features.

Those would have to be #![no_core] to actually work -- the feature is needed for dependency tracking.

[RalfJung]

@jethrogb if you think this is a worthwhile feature and want to do the review, I won't block it. I just don't think I want to spend the time needed to review support for this use case.

However, if we land anything like this, it should come with a test.

[roblabla]

Xargo has its own system to work around this, which is build stages. Without this fix, build stages are basically broken in some configurations. With build stages, you can use crates that depend on core or alloc by building them in a later stage. See megaton-hammer's Xargo.toml: https://github.com/MegatonHammer/megaton-hammer/blob/master/Xargo.toml

We build std in a later stage, so all previous sysroot crates are available as a dependency to libstd and its dep.

[RalfJung]

Without this fix, build stages are completely useless.

That's not true -- Miri used them for years and they were needed. This only recently changed with rust-lang/rust#63637.

[RalfJung]

But you have a point that the README basically indicates this as a supported use-case.

[jethrogb]

@jethrogb if you think this is a worthwhile feature and want to do the review, I won't block it. I just don't think I want to spend the time needed to review support for this use case.

Ok but you're the one who resurrected this issue 😄

[RalfJung]

Well I want to either close or merge all our PRs. I don't like them lingering. ;)

So we might want to close this saying "sorry, xargo is in maintenance mode", or we actually review it. I don't see a good argument for just leaving it sitting here if we don't intend to review it.

[RalfJung]

Or we might be able to coerce @roblabla into helping us with maintenance :D

[roblabla]

Also, I'd like to point out that this specific use-case is how many console homebrew toolchains use xargo for this feature. The ability to build a custom libstd with a custom crate graph has been a huge help for ctru-rs (a 3ds homebrew toolchain), rusty-horizon (another switch homebrew toolchain) and a few others. They currently all work around this problem in various ways (Using #[feature(rustc_private]) or adding an extra unused feature to all crates built under std).

I'll try to add a test, split the commits and rebase later today.

[FenrirWolf]

Yeah, I was also bitten by this in ctru-rs. There's a custom C library that both std and user crates want to use to interface with the system, so the -sys crate for that library appears both inside and outside the sysroot. I created a dummy feature for the crate only used inside the sysroot to make the hashes unique, but it would be nice if that weren't necessary.

[RalfJung]

All right, I was not aware how much xargo is used to compile alternative libstds. That's useful information.

The request for help in maintaining xargo still stands, though. ;)

[RalfJung]

Cc rust-lang/wg-cargo-std-aware#34

[roblabla]

I tried writing a simple test by adding spin as a dependency to alloc, and then again as a dependency to the final binary crate, but I haven't been able to get the issue to reproduce the issue at all with xargo 0.3.16 and rustc 1.38.0-nightly (9703ef666 2019-08-10). The repo can be found here. When running with xargo build --release, I expected a fail, but it builds correctly. 🤷‍♂

Unfortunately I don't really have a whole lot of time to dedicate to this issue, and don't currently maintain a custom libstd myself, so I'm not going to punt on this issue. Feel free to close, and I guess I'll reopen if I ever hit the issue again, or if somebody else manages to get a reproducer.

---

I asked some of the people working on homebrew toolchains (Rust with Blackjack server) if they'd be interested in helping with the maintenance. I can't really dedicate much time to xargo maintenance myself, I'm stretched a bit too thin right now.

[RalfJung]

Indeed I'd prefer we have some way to reproduce this in a minimal example before adding an undocumented env var. Help with that would be welcome. :)

I opened #261 to track this problem, and will close the PR. Thanks!