stabilize stage management for rustc tools

[onur-ozkan]

#135990 got out of control due to excessive complexity. This PR aims to achieve the same goal with a simpler approach, likely through multiple smaller PRs. I will keep the other one read-only and open as a reference for future work.

This work stabilizes the staging logic for ToolRustc programs, so you no longer need to handle build and target compilers separately in steps. Previously, most tools didn't do this correctly, which was causing the compiler to be built twice (e.g., x test cargo --stage 1 would compile the stage 2 compiler before, but now it only compiles the stage 1 compiler).

I also tried to document how we should write ToolRustc steps as they are quite different and require more attention than other tools.

Next goal is to stabilize how stages are handled for the rustc itself. Currently, x build --stage 1 builds the stage 1 compiler which is fine, but x build compiler --stage 1 builds stage 2 compiler.

for now, r? ghost

[jieyouxu]

Feel free to r/? me if you want help with testing / becomes ready for review.

[rustbot]

The rustc-dev-guide subtree was changed. If this PR only touches the dev guide consider submitting a PR directly to rust-lang/rustc-dev-guide otherwise thank you for updating the dev guide with your changes.

cc @BoxyUwU, @jieyouxu, @Kobzol

[onur-ozkan]

r? jieyouxu

@rustbot ready

just in case @bors rollup=never

[onur-ozkan]

p.s.: Sorry for the dirty commit history. It was being painful to work on this change.

[jieyouxu]

p.s.: Sorry for the dirty commit history. It was being painful to work on this change.

No worries, making this less confusing is way more important

[onur-ozkan]

I get it but it was buggy even for cargo and clippy that's what I'm saying. When you pass the stage flag it never works as expected (it always compiles rustc and cargo for stage +1).

[Kobzol]

Right, so if I understand it correctly:

Before:
./x test src/tools/cargo => correct
./x test src/tools/cargo --stage=N => incorrect

After:
./x test src/tools/cargo => incorrect, warns
./x test src/tools/cargo --stage=N => correct

And I propose:

./x test src/tools/cargo => correct (automatically selects the lowest supported stage)
./x test src/tools/cargo --stage=N => correct

[onur-ozkan]

Yeah, we could do that. It's not hard — we just need to add a check for whether the stage was explicitly set. I can send a follow-up PR after this.

[BoxyUwU]

Sorry for the drive by comment, but does this PR make x test tests/ui build a stage2 rustc? That would be really bad for contributors, I can't see how that wouldn't more than double the time it takes to get to UI tests whenever you make a change to the compiler 😦

Very sympathetic to the fact that generally setting stages for various x commands does inconsistent or confusing things though and its good to see work happen here :3

[bors]

⌛ Testing commit 76063a6 with merge bb2cc59...

[onur-ozkan]

Sorry for the drive by comment, but does this PR make x test tests/ui build a stage2 rustc?

No, it doesn't. This PR either reduces compilation or keeps it the same as before. It doesn't add any extra compilation on any invocation.

[bors]

☀️ Test successful - checks-actions
Approved by: jieyouxu,Kobzol
Pushing bb2cc59 to master...

[rust-timer]

Finished benchmarking commit (bb2cc59): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.3%	[-0.3%, -0.3%]	2
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary -7.9%)

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
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-7.9%	[-7.9%, -7.9%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-7.9%	[-7.9%, -7.9%]	1

Cycles

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

Binary size

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

Bootstrap: 770.304s -> 770.69s (0.05%)
Artifact size: 359.67 MiB -> 359.83 MiB (0.04%)

[RalfJung]

I don't think this is a good default -- it should be considered a bug if the tests do not work in stage 1. Certainly, they do work for Miri, so this can lead to Miri contributors waiting unnecessarily long for their builds to finish.

[onur-ozkan]

#137522 should be enough for the stage1 problems happen on clippy and cargo tests, so this should be revertable.

[RalfJung]

So what that does is just bump the stage to 2? I guess that hides the bugs with those tools...

I have #78717 somewhere in my TODO list, so I'll have to remember to revert that then. I don't know why cargo needs stage 2.

[onur-ozkan]

I didn't get how #78717 is being related to test-stage = 2 in the config file.

So what that does is just bump the stage to 2? I guess that hides the bugs with those tools...

Actually I don't know if it's a bug or not. All other tools work fine but cargo and clippy fail on stage 1 tests.

[RalfJung]

#78717 tracks the bug that you need stage 2 for clippy. Once that is fixed, clippy can be tested on stage 1.

[RalfJung]

Actually I don't know if it's a bug or not.

I am arguing that we should consider it a bug. We should have the expectation that tools can be tested in stage 1 (except when they really need the bootstrap loop to have completed, which is quite rare).