do not build additional stage on compiler paths #137882

onur-ozkan · 2025-03-02T08:32:46Z

When calling x build compiler (or rustc) --stage N bootstrap builds stage N+1 compiler, which is clearly not what we requested. This doesn't happen when running x build --stage N without explicitly targeting the compiler.

The changes applied fix this issue.

r? ghost

When calling `x build compiler (or rustc) --stage N` bootstrap builds stage N+1 compiler, which is clearly not what we requested. This doesn't happen when running `x build --stage N` without explicitly targeting the compiler. The changes applied fix this issue. Signed-off-by: onur-ozkan <work@onurozkan.dev>

onur-ozkan · 2025-03-02T09:23:23Z

src/bootstrap/src/core/builder/tests.rs

+            first(builder.cache.all::<compile::Assemble>()),
            &[
-                rustc!(TEST_TRIPLE_1 => TEST_TRIPLE_1, stage = 0),
-                rustc!(TEST_TRIPLE_1 => TEST_TRIPLE_1, stage = 1),
-                rustc!(TEST_TRIPLE_1 => TEST_TRIPLE_1, stage = 2),
-                rustc!(TEST_TRIPLE_1 => TEST_TRIPLE_2, stage = 1),
-                rustc!(TEST_TRIPLE_1 => TEST_TRIPLE_2, stage = 2),
+                compile::Assemble {
+                    target_compiler: Compiler {
+                        host: TargetSelection::from_user(TEST_TRIPLE_1),
+                        stage: 0
+                    }
+                },
+                compile::Assemble {
+                    target_compiler: Compiler {
+                        host: TargetSelection::from_user(TEST_TRIPLE_1),
+                        stage: 1
+                    }
+                },
+                compile::Assemble {
+                    target_compiler: Compiler {
+                        host: TargetSelection::from_user(TEST_TRIPLE_1),
+                        stage: 2
+                    }
+                },
+                compile::Assemble {
+                    target_compiler: Compiler {
+                        host: TargetSelection::from_user(TEST_TRIPLE_2),
+                        stage: 2
+                    }
+                },


This would include stage3 compiler on master.

onur-ozkan · 2025-03-02T09:23:49Z

@rustbot ready

r? bootstrap

Signed-off-by: onur-ozkan <work@onurozkan.dev>

Kobzol · 2025-03-03T10:01:53Z

One of those PRs where --dry-run is really useful :) Looks good.

@bors r+

bors · 2025-03-03T10:01:56Z

📌 Commit cfb475c has been approved by Kobzol

It is now in the queue for this repository.

bors · 2025-03-03T10:01:57Z

🌲 The tree is currently closed for pull requests below priority 100. This pull request will be tested once the tree is reopened.

onur-ozkan · 2025-03-03T10:12:42Z

One of those PRs where --dry-run is really useful :)

Yeah, it has saved me lots of hours in my recent works..

Zalathar · 2025-03-04T05:10:20Z

I'm a bit confused by x build --stage=1 apparently not having the same behaviour. Is there some other stage adjustment that applies in that case?

onur-ozkan · 2025-03-04T05:38:26Z

Can you say more about not having the same behaviour?

Zalathar · 2025-03-04T05:51:21Z

So, my understanding is that --stage=1 typically means to use the stage 1 compiler. That’s why x build compiler --stage=1 (before this PR) builds a fully working stage 1 compiler, then uses that to build the “stage 1 compiler artifacts”, which are part of the stage 2 compiler.

My confusion is that x build --stage=1, from what I can tell, doesn’t use the stage 1 compiler to build compiler/. So I don’t understand what was causing x build and x build compiler to interpret the stage differently.

onur-ozkan · 2025-03-04T06:05:35Z

x build compiler --stage 1 was literally building stage 2 compiler and it now just builds fully working stage 1 compiler but I guess I am missing something here?

Zalathar · 2025-03-04T07:07:15Z

Before this change, x build compiler --stage=1 meant “use the stage 1 compiler to build the compiler”, and the compiler you build with the stage 1 compiler is the stage 2 compiler. So the old behaviour was logically consistent (but unintuitive).

After this change, x build compiler --stage=1 means “use the stage 0 compiler to build the stage 1 compiler”. That's honestly more useful, so I'm not sad about it, but it made me wonder why x build --stage=1 already builds the stage 1 compiler and not the stage 2 compiler.

Also note that this new behaviour is now inconsistent with x check compiler and x test compiler, which still interpret --stage=1 to mean “build a working stage 1 compiler and use it to check/test the stage 2 compiler”. So perhaps those commands should be adjusted too.

onur-ozkan · 2025-03-04T08:44:24Z

To make it clear, this change makes x build and x build compiler work the same way. Before, x build without a path gave you the compiler for the configured stage, but x build compiler didn't. If I run x build --stage 2, I don't expect a stage 3 compiler, so why should it give that when I put "compiler" next to x build ? Therefore we fixed x build compiler --stage=N to match x build.

x check and x test work differently, they don't build extra compiler sysroots (you won't see stage N+1 in the logs). But those can be improved too.

Zalathar · 2025-03-04T09:45:58Z

Ah, I think the reason x build --stage=1 doesn't build the stage 2 compiler (even before this PR) is that compile::Rustc isn't a default step, so x build --stage=1 does not include x build compiler --stage=1.

It still builds the stage 1 compiler, of course, but only because having a working stage 1 compiler is required for other things (e.g. x build library --stage=1, which uses the stage 1 compiler to build the standard library).

Kobzol · 2025-03-04T09:59:26Z

This broke the rustc-perf bootstrap benchmark, because it does x build --stage 0 compiler/rustc, so it now finishes instantly xD
Using x build --stage 1 compiler/rustc should be enough as a fix, right?

onur-ozkan · 2025-03-04T10:31:11Z

This broke the rustc-perf bootstrap benchmark, because it does x build --stage 0 compiler/rustc, so it now finishes instantly xD Using x build --stage 1 compiler/rustc should be enough as a fix, right?

Yeah, previous x build --stage 0 compiler/rustc logic does what x build --stage 1 compiler/rustc does right now (it was magically incrementing the stage, now we don't do that).

Zalathar · 2025-03-05T03:34:27Z

If I run x build --stage 2, I don't expect a stage 3 compiler, so why should it give that when I put "compiler" next to x build? Therefore we fixed x build compiler --stage=N to match x build.

This is not a bug fix; it's an ad-hoc breaking change to how stage numbering works for compiler specifically.

There are arguments for and against that, but I'm concerned that nobody involved seems to understand or acknowledge that this is what's happening.

After this PR, x build and x build compiler are inconsistent in how they handle stage numbering.

The reason x build --stage=N doesn't build the stage N+1 compiler, but x build compiler --stage=N did (before this PR), is that compile::Rustc and compile::Assemble are not default steps, so x build does not include x build compiler.

(it was magically incrementing the stage, now we don't do that).

It was not magically incrementing the stage. It was using the stage N compiler, as requested, to build the compiler. The result of doing that is the stage N+1 compiler. That's why the +1 was deliberately included.

After this PR, x build compiler --stage=N now magically uses the stage N.saturating_sub(1) compiler (instead of the requested stage N) to build the stage N.saturating_sub(1) + 1 compiler. It then assembles something, though at this point the stage logic has become so ad-hoc that it's hard to tell what is being assembled or why.

onur-ozkan · 2025-03-05T05:23:53Z

The reason x build --stage=N doesn't build the stage N+1 compiler, but x build compiler --stage=N did (before this PR), is that compile::Rustc and compile::Assemble are not default steps, so x build does not include x build compiler.

Yeah I am aware that, but why do you think it was not a default Step? Why do you think bootstrap didn't invoke it by default and would that make sense from the user perspective (they will see it's building stageN+1 compiler everytime they call x build --stage=N)?

After this PR, x build compiler --stage=N now magically uses the stage N.saturating_sub(1) compiler (instead of the requested stage N) to build the stage N.saturating_sub(1) + 1 compiler. It then assembles something, though at this point the stage logic has become so ad-hoc that it's hard to tell what is being assembled or why.

That's exactly what was the problem with Rustc tools and many people was arguing about it for years, which is fixed with #137215.

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Mar 2, 2025

onur-ozkan commented Mar 2, 2025

View reviewed changes

onur-ozkan marked this pull request as ready for review March 2, 2025 09:23

rustbot assigned Kobzol Mar 2, 2025

extend scope of build_all test

cfb475c

Signed-off-by: onur-ozkan <work@onurozkan.dev>

onur-ozkan force-pushed the remove-extra-compiler-stage branch from bd24ae8 to cfb475c Compare March 2, 2025 11:42

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 3, 2025

matthiaskrgr mentioned this pull request Mar 3, 2025

Rollup of 12 pull requests #137959

Merged

bors merged commit a4b6181 into rust-lang:master Mar 4, 2025
6 checks passed

rustbot added this to the 1.87.0 milestone Mar 4, 2025

onur-ozkan deleted the remove-extra-compiler-stage branch March 4, 2025 05:50

Kobzol mentioned this pull request Mar 4, 2025

Fix bootstrap rustc benchmark rust-lang/rustc-perf#2051

Merged

Uh oh!

do not build additional stage on compiler paths #137882

do not build additional stage on compiler paths #137882

Uh oh!

Conversation

onur-ozkan commented Mar 2, 2025

Uh oh!

onur-ozkan Mar 2, 2025

Choose a reason for hiding this comment

Uh oh!

onur-ozkan commented Mar 2, 2025

Uh oh!

Kobzol commented Mar 3, 2025

Uh oh!

bors commented Mar 3, 2025

Uh oh!

bors commented Mar 3, 2025

Uh oh!

onur-ozkan commented Mar 3, 2025

Uh oh!

Zalathar commented Mar 4, 2025

Uh oh!

onur-ozkan commented Mar 4, 2025

Uh oh!

Uh oh!

Zalathar commented Mar 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

onur-ozkan commented Mar 4, 2025

Uh oh!

Zalathar commented Mar 4, 2025

Uh oh!

onur-ozkan commented Mar 4, 2025

Uh oh!

Zalathar commented Mar 4, 2025

Uh oh!

Kobzol commented Mar 4, 2025

Uh oh!

onur-ozkan commented Mar 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Zalathar commented Mar 5, 2025

Uh oh!

onur-ozkan commented Mar 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Zalathar commented Mar 4, 2025 •

edited

Loading

onur-ozkan commented Mar 4, 2025 •

edited

Loading