fix(rustflags): Respect rustflags when probing rustc for the sysroot

[adamgemmell]

What does this PR try to resolve?

Fixes #17351 by restoring the ability for RUSTFLAGS, CARGO_ENCODED_RUSTFLAGS and build.rustflags to override the sysroot.

As discussed in the issue, this PR does not restore the ability for rustflags in the target cfg to override the sysroot as it's very difficult for build-std to support this and it's not clear if anyone has a need for the functionality. Users surveyed include cargo careful and cargo miri in the issue, and xargo.

This PR also cleans up a leftover comment introduced in the PR that caused this issue.

How to test and review this PR?

The PR introduces unit tests and implements the new behaviour in a sequence of atomic commits. I'd recommend reading each one turn to understand the change. The behaviour can be tested by invoking cargo with RUSTFLAGS="--sysroot=dir" or by setting build.rustflags.

🤖 LLM disclosure: I asked an LLM whether my tests would pass on Windows as a sanity check, and used the information returned to rewrite them. All code was hand-written and wasn't generated.

[rustbot]

r? @epage

rustbot has assigned @epage.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: @epage, @weihanglo
• @epage, @weihanglo expanded to epage, weihanglo
• Random selection from epage, weihanglo

[weihanglo]

This change itself looks good, though since it kinda introduces a new mechanism of rustflags passing. I wonder if any other on @rust-lang/cargo would love to take a second look.

Also, have we checked this fix the failure in cargo-careful end to end?

View changes since this review

[weihanglo]

We probably want to beta backport if this merges.

[weihanglo]

FWIW in a proc-macro package, this fails:

$ ./target/debug/cargo careful  test --target aarch64-apple-darwin # (or you host-tuple)
    Finished `test` profile [unoptimized + debuginfo] target(s) in 0.34s
     Running unittests src/lib.rs (target/debug/build/pm-repro/427d198c513387be/out/pm_repro-427d198c513387be)
dyld[18345]: Library not loaded: @rpath/libstd-8f8b98e820f5fd49.dylib
  Referenced from: <7A872C53-8582-3C68-9FA4-1F09EC6A1478> /private/tmp/pm-repro/target/debug/build/pm-repro/427d198c513387be/out/pm_repro-427d198c513387be
  Reason: tried: '/private/tmp/pm-repro/target/debug/libstd-8f8b98e820f5fd49.dylib' (no such file), '/Users/whlo/Library/Caches/de.ralfj.cargo-careful/lib/rustlib/aarch64-apple-darwin/lib/libstd-8f8b98e820f5fd49.dylib' (no such file), '/Users/me/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/libstd-8f8b98e820f5fd49.dylib' (no such file), '/Users/me/lib/libstd-8f8b98e820f5fd49.dylib' (no such file), '/usr/local/lib/libstd-8f8b98e820f5fd49.dylib' (no such file), '/usr/lib/libstd-8f8b98e820f5fd49.dylib' (no such file, not in dyld cache)
error: test failed, to rerun pass `--lib`

Caused by:
  process didn't exit successfully: `/private/tmp/pm-repro/target/debug/build/pm-repro/427d198c513387be/out/pm_repro-427d198c513387be` (signal: 6, SIGABRT: process abort signal)

I am still trying to figure out what is the correct behavior, though just FYI after the refactor before this PR, this command works.

[adamgemmell]

Thanks very much for testing, it's a good spot. I think it's because host sysroot lookups didn't used to respect RUSTFLAGS, but with this PR they do.

cargo points the test executable to the built sysroot, but proc macro tests are built as a host artifact and so aren't built with RUSTFLAGS, and so expect the host std which doesn't match the one in the sysroot.

I think the assumption in this PR that it's only useful to have one sysroot per Cargo invocation is broken, so we could do option 2. or 4..

[weihanglo]

I think the assumption in this PR that it's only useful to have one sysroot per Cargo invocation is broken, so we could do option 2. or 4..

That is my main "concern". I am not sure if --target mode is an interesting enough use case that we should care. Need to look at xargo and other cross-compilation cases to decide I guess.
(Of course we can say RUSTFLAGS support doesn't guarantee stability but 🤷🏾‍♂️😞)

[adamgemmell]

I'm fine with reverting by the way, coming up with some plumbing sounds like it might be easier than working out the intended behaviour here.

[weihanglo]

Sounds good. Let's revert to buy us more time to think about it. Thank you for taking care of it