Implement RFC3137 trim-paths sysroot changes - take 2

[Urgau]

This PR is a continuation of #118149. Nothing really changed, except for #129408 which I was able to trigger locally.

Original description:

Implement parts of #111540

Right now, backtraces into sysroot always shows /rustc/$hash in diagnostics, e.g.

thread 'main' panicked at 'hello world', map-panic.rs:2:50
stack backtrace:
   0: std::panicking::begin_panic
             at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/std/src/panicking.rs:616:12
   1: map_panic::main::{{closure}}
             at ./map-panic.rs:2:50
   2: core::option::Option<T>::map
             at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/core/src/option.rs:929:29
   3: map_panic::main
             at ./map-panic.rs:2:30
   4: core::ops::function::FnOnce::call_once
             at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/core/src/ops/function.rs:248:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

RFC 3127 said

We want to change this behaviour such that, when rust-src source files can be discovered, the virtual path is discarded and therefore the local path will be embedded, unless there is a --remap-path-prefix that causes this local path to be remapped in the usual way.

This PR implements this behaviour. When rust-src is present at compile time, rustc replaces /rustc/$hash with a real path into local rust-src with best effort. To sanitise this, users must explicitly supply --remap-path-prefix=<path to rust-src>=foo.

cc @cbeuw
Fix #105907
Fix #85463

try-job: dist-x86_64-linux
try-job: x86_64-msvc
try-job: dist-x86_64-msvc
try-job: armhf-gnu

[Urgau]

@bors try

[bors]

⌛ Trying commit cac4a30 with merge a600fe8...

[bors]

☀️ Try build successful - checks-actions
Build commit: a600fe8 (a600fe8b275a62c2973e9fc1b589e50302eb2540)

[Urgau]

Great, the extended try is successful! dist-x86_64-linux and x86_64-msvc were the builder that failed in the previous PR: #118149 (comment).

I've verified locally (with rustup-toolchain-install-master):

1. that the prebuild rlibs only contains /rustc/ paths ✔
2. that compiling tests/codegen/remap_path_prefix/issue-73167-remap-std.rs without the rust-src component only leads to /rustc/ paths ✔
3. that compiling tests/codegen/remap_path_prefix/issue-73167-remap-std.rs with the rust-src component leads to many (not all) paths redirected to /home/gh-Urgau/.rustup/toolchains/a600fe8b275a62c2973e9fc1b589e50302eb2540/lib/rustlib/src/rust/library/ ✔

Everything looks go.
r? compiler

[rustbot]

Some changes occurred in src/tools/compiletest

cc @jieyouxu

[jieyouxu]

Thanks, the changes themselves look good to me. We definitely should update rustc-dev-guide to mention existence and purpose of {{rust-src-base}}, would be very cool if you can open a follow-up dev-guide PR.

[jieyouxu]

Remark: I think this is fine, though it might be interesting if stdlib ever adds/removes/renames specific crates. I also can't immediately think of a good way to add a test to make sure this list is always in sync.

[jieyouxu]

Suggestion: we definitely want to update rustc-dev-guide's compiletest directive remarks to mention {{rust-src-base}}, which is not to be confused with {{src-base}}.

[Urgau]

Opened rust-lang/rustc-dev-guide#2079

[jieyouxu]

@bors r+ rollup

[bors]

📌 Commit cac4a30 has been approved by jieyouxu

It is now in the queue for this repository.

[jieyouxu]

I'm not exactly sure why opt-dist sysroot path prefixes becomes wonky...

I suspect we may need to modify opt-dist to pass some information to bootstrap -> compiletest to possibly repair the path prefix up to something like unpacked-dist/rustc-nightly-x86_64-unknown-linux-gnu/rustc/lib/rustlib/src/rust, but I haven't dug deeply into this yet.

We wouldn't want to skip these tests naively especially if it can possibly reveal bugs in PGO+LTO among other things.

[jieyouxu]

If the opt-dist tests steps never get reached in PR CI or try-jobs, let's rollup=never and send it off for full CI. r=me once you drop the CI commit.

@bors rollup=never (expected to fail)

[Urgau]

@bors r=@jieyouxu

[bors]

📌 Commit 3a6c6ee has been approved by jieyouxu

It is now in the queue for this repository.

[bors]

⌛ Testing commit 3a6c6ee with merge 1d9162b...

[bors]

☀️ Test successful - checks-actions
Approved by: jieyouxu
Pushing 1d9162b to master...

[rust-timer]

Finished benchmarking commit (1d9162b): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.2%	[0.2%, 0.2%]	1
Regressions ❌ (secondary)	0.7%	[0.2%, 1.2%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.2%	[0.2%, 0.2%]	1

Max RSS (memory usage)

Results (primary 2.8%, secondary -0.1%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	2.8%	[2.8%, 2.8%]	1
Regressions ❌ (secondary)	3.5%	[2.2%, 6.6%]	4
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.4%	[-4.0%, -1.4%]	6
All ❌✅ (primary)	2.8%	[2.8%, 2.8%]	1

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

Results (primary 0.2%, secondary 0.1%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.2%	[0.0%, 1.4%]	71
Regressions ❌ (secondary)	0.1%	[0.0%, 0.2%]	18
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.2%	[0.0%, 1.4%]	71

Bootstrap: 768.272s -> 768.678s (0.05%)
Artifact size: 341.40 MiB -> 341.38 MiB (-0.01%)

[oxalica]

It's unfortunate that in 1.83, we have this behavior change, but have not stabilized trim-paths. I'm expecting to have a simple constant flag like CARGO_TRIM_PATHS=all before introducing this change though. This introduced some unexpected runtime dependencies for binary artifacts on Nix (oxalica/rust-overlay#199).

In the release note, it's mentioned that:

replaces /rustc/$hash with a real path into the local rust-src component with best effort

users must explicitly supply --remap-path-prefix=<path to rust-src>=foo or not have the rust-src component installed.

So in our build system, we now need to find out the path to rust-src in the same way as rustc, and add a remap flag to avoid it. I guess checking and using $(rustc --print=sysroot)/lib/rustlib/src/rust should be enough? (as the {{rust-src-base}} impl) Or is there any overriding flags or envvars we also need to check?

[Urgau]

I guess checking and using $(rustc --print=sysroot)/lib/rustlib/src/rust should be enough?

Yes, I think it should be enough. As for potential env variables (which I don't think there are) or additional flags (only --sysroot?) they are reflected in the --print=sysroot.

Another possibility (which the release notes also alludes to) is to not have the rust-src component. I'm a bit surprised that your build system is having at all, I wouldn't expect it.

[oxalica]

Yes, I think it should be enough. As for potential env variables (which I don't think there are) or additional flags (only --sysroot?) they are reflected in the --print=sysroot.

Thanks for the info.

Another possibility (which the release notes also alludes to) is to not have the rust-src component. I'm a bit surprised that your build system is having at all, I wouldn't expect it.

I didn't expect it too. Apparently there are someone using a non-minimal toolchain for build system. Yeah I'll comment that it is not the typical toolchain for release build.