Skip to content

Reject running compiletest self-tests against stage 0 rustc unless explicitly allowed #144675

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jul 30, 2025

Conversation

jieyouxu
Copy link
Member

@jieyouxu jieyouxu commented Jul 30, 2025

Currently, in pr-check-1, we run python3 ../x.py test --stage 0 src/tools/compiletest, which is compiletest self-tests against stage 0 rustc. This makes it very annoying for PRs wanting to change target spec JSON format, which compiletest depends on for target information, as otherwise compiletest would have to know how to handle 2 different target spec JSON formats and know when to pick which.

Instead of doing that, we change compiletest self-tests to reject running against stage 0 rustc unless explicitly allowed with build.compiletest-allow-stage0=true. build.compiletest-allow-stage0 is a proper bootstrap config option in favor of the ad-hoc COMPILETEST_FORCE_STAGE0 env var. This means that:

  • ./x test src/tools/compiletest --stage=0 is not allowed, unless build.compiletest-allow-stage0=true is set. In this scenario, compiletest self-tests should be expected to fail unless the stage 0 rustc as provided is like codegen_cranelift where it's actually built from in-tree rustc sources.
  • In CI, we change ./x test src/tools/compiletest --stage=0 to ./x test src/tools/compiletest --stage=1, and move it to pr-check-2. Yes, this involves building the stage 1 compiler, but pr-check-2 already has to build stage 1 compiler to test stage 1 library crates.
  • Crucially, this means that compiletest is only intended to support one target spec JSON format, namely the one corresponding to the in-tree rustc.
  • This should preserve the compiletest-use-stage0-libtest UX optimization where changing compiler/ tree should still not require rebuilding compiletest as long as build.compiletest-use-stage0-libtest=true, as that should remain orthogonal. This is completely unlike my previous attempt at Consider compiletest a staged ToolStd tool #144563 that tries to do a way more invasive change which would cause the rebuild problem.

Best reviewed commit-by-commit.


r? @Kobzol

@rustbot rustbot added A-CI Area: Our Github Actions CI A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Jul 30, 2025
@jieyouxu jieyouxu force-pushed the compiletest-staging branch from 925f797 to 0539cc1 Compare July 30, 2025 08:46
@jieyouxu
Copy link
Member Author

jieyouxu commented Jul 30, 2025

cc @bjorn3 on the cg_clif changes (commit 2 mostly)

@jieyouxu jieyouxu marked this pull request as ready for review July 30, 2025 08:46
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 30, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jul 30, 2025

This PR modifies bootstrap.example.toml.

If appropriate, please update CONFIG_CHANGE_HISTORY in src/bootstrap/src/utils/change_tracker.rs.

This PR modifies src/bootstrap/src/core/config.

If appropriate, please update CONFIG_CHANGE_HISTORY in src/bootstrap/src/utils/change_tracker.rs.

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

@jieyouxu jieyouxu force-pushed the compiletest-staging branch from 0539cc1 to 89f67a4 Compare July 30, 2025 08:56
@jieyouxu
Copy link
Member Author

0 checks

Hello?

@jieyouxu jieyouxu closed this Jul 30, 2025
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 30, 2025
@jieyouxu jieyouxu reopened this Jul 30, 2025
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 30, 2025
Copy link
Member

@Kobzol Kobzol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

opt-dist and cg_gcc will also need to be updated (you can grep for usage of COMPILETEST_FORCE_STAGE0).

@jieyouxu jieyouxu added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 30, 2025
@jieyouxu jieyouxu force-pushed the compiletest-staging branch from 89f67a4 to 450c589 Compare July 30, 2025 11:15
@rustbot
Copy link
Collaborator

rustbot commented Jul 30, 2025

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

Some changes occurred in src/tools/opt-dist

cc @Kobzol

@jieyouxu
Copy link
Member Author

Changes since last review:

  • Also updated cg_gcc and opt-dist (I forgor).
  • Updated comment re. intended use case.

@jieyouxu jieyouxu added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 30, 2025
@jieyouxu jieyouxu force-pushed the compiletest-staging branch from 450c589 to 2e92be1 Compare July 30, 2025 11:41
@jieyouxu jieyouxu changed the title Reject running compiletest self-tests against stage 0 rustc unless forced Reject running compiletest self-tests against stage 0 rustc unless explicitly allowed Jul 30, 2025
@jieyouxu jieyouxu force-pushed the compiletest-staging branch from dc2a43a to 2d0e721 Compare July 30, 2025 11:47
@jieyouxu
Copy link
Member Author

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 30, 2025
@jieyouxu jieyouxu force-pushed the compiletest-staging branch from 2d0e721 to 274e067 Compare July 30, 2025 11:49
jieyouxu added 5 commits July 30, 2025 19:54
In favor of the adhoc `COMPILETEST_FORCE_STAGE0` env var.
…explicitly allowed

Otherwise, `compiletest` would have to know e.g. how to parse two
different target spec, if target spec format was changed between beta
`rustc` and in-tree `rustc`.
And move `./x test compiletest --stage=1` to `pr-check-2`, where testing
library artifacts already requires building the stage 1 compiler.
@jieyouxu jieyouxu force-pushed the compiletest-staging branch from 274e067 to e954253 Compare July 30, 2025 11:55
@jieyouxu
Copy link
Member Author

@rustbot review

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 30, 2025
@Kobzol
Copy link
Member

Kobzol commented Jul 30, 2025

Thanks!

@bors r+

@bors
Copy link
Collaborator

bors commented Jul 30, 2025

📌 Commit e954253 has been approved by Kobzol

It is now in the queue for this repository.

@bors 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 Jul 30, 2025
bors added a commit that referenced this pull request Jul 30, 2025
Rollup of 4 pull requests

Successful merges:

 - #143465 (Support multiple crate versions in --extern-html-root-url)
 - #144308 ([rustdoc] Display total time and compilation time of merged doctests)
 - #144655 (clean up codegen fn attrs)
 - #144675 (Reject running `compiletest` self-tests against stage 0 rustc unless explicitly allowed)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 202d1f5 into rust-lang:master Jul 30, 2025
10 checks passed
rust-timer added a commit that referenced this pull request Jul 30, 2025
Rollup merge of #144675 - jieyouxu:compiletest-staging, r=Kobzol

Reject running `compiletest` self-tests against stage 0 rustc unless explicitly allowed

Currently, in `pr-check-1`, we run `python3 ../x.py test --stage 0 src/tools/compiletest`, which is `compiletest` self-tests against stage 0 rustc. This makes it very annoying for PRs wanting to change target spec JSON format, which `compiletest` depends on for target information, as otherwise `compiletest` would have to know how to handle 2 different target spec JSON formats and know when to pick which.

Instead of doing that, we change `compiletest` self-tests to reject running against stage 0 `rustc` *unless* explicitly allowed with `build.compiletest-allow-stage0=true`. `build.compiletest-allow-stage0` is a proper bootstrap config option in favor of the ad-hoc `COMPILETEST_FORCE_STAGE0` env var. This means that:

- `./x test src/tools/compiletest --stage=0` is not allowed, unless `build.compiletest-allow-stage0=true` is set. In this scenario, `compiletest` self-tests should be expected to fail unless the stage 0 `rustc` as provided is like codegen_cranelift where it's *actually* built from in-tree `rustc` sources.
- In CI, we change `./x test src/tools/compiletest --stage=0` to `./x test src/tools/compiletest --stage=1`, and move it to `pr-check-2`. Yes, this involves building the stage 1 compiler, but `pr-check-2` already has to build stage 1 compiler to test stage 1 library crates.
- Crucially, this means that **`compiletest` is only intended to support one target spec JSON format**, namely the one corresponding to the in-tree `rustc`.
- This should preserve the `compiletest-use-stage0-libtest` UX optimization where changing `compiler/` tree should still not require rebuilding `compiletest` as long as `build.compiletest-use-stage0-libtest=true`, as that should remain orthogonal. This is completely unlike my previous attempt at #144563 that tries to do a way more invasive change which would cause the rebuild problem.

Best reviewed commit-by-commit.

---

r? `@Kobzol`
@rustbot rustbot added this to the 1.90.0 milestone Jul 30, 2025
@jieyouxu jieyouxu deleted the compiletest-staging branch July 31, 2025 04:39
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Aug 1, 2025
…Kobzol

Properly pass path to staged `rustc` to `compiletest` self-tests

Otherwise, this can do weird things like use a global rustc, or try to use stage 0 rustc. This must be properly configured, because `compiletest` is intended to only support one compiler target spec JSON format (of the in-tree compiler).

Historically, this was probably done so before `bootstrap` was really its own thing, and `compiletest` had to be runnable as a much more "self-standing" tool.

Follow-up to rust-lang#144675, as I didn't realize this until Zalathar pointed it out in [#t-infra/bootstrap > Building vs testing &rust-lang#96;compiletest&rust-lang#96; @ 💬](https://rust-lang.zulipchat.com/#narrow/channel/326414-t-infra.2Fbootstrap/topic/Building.20vs.20testing.20.60compiletest.60/near/532040838).

r? `@Kobzol`
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Aug 1, 2025
…Kobzol

Properly pass path to staged `rustc` to `compiletest` self-tests

Otherwise, this can do weird things like use a global rustc, or try to use stage 0 rustc. This must be properly configured, because `compiletest` is intended to only support one compiler target spec JSON format (of the in-tree compiler).

Historically, this was probably done so before `bootstrap` was really its own thing, and `compiletest` had to be runnable as a much more "self-standing" tool.

Follow-up to rust-lang#144675, as I didn't realize this until Zalathar pointed it out in [#t-infra/bootstrap > Building vs testing &rust-lang#96;compiletest&rust-lang#96; @ 💬](https://rust-lang.zulipchat.com/#narrow/channel/326414-t-infra.2Fbootstrap/topic/Building.20vs.20testing.20.60compiletest.60/near/532040838).

r? ``@Kobzol``
samueltardieu added a commit to samueltardieu/rust that referenced this pull request Aug 2, 2025
…Kobzol

Properly pass path to staged `rustc` to `compiletest` self-tests

Otherwise, this can do weird things like use a global rustc, or try to use stage 0 rustc. This must be properly configured, because `compiletest` is intended to only support one compiler target spec JSON format (of the in-tree compiler).

Historically, this was probably done so before `bootstrap` was really its own thing, and `compiletest` had to be runnable as a much more "self-standing" tool.

Follow-up to rust-lang#144675, as I didn't realize this until Zalathar pointed it out in [#t-infra/bootstrap > Building vs testing &rust-lang#96;compiletest&rust-lang#96; @ 💬](https://rust-lang.zulipchat.com/#narrow/channel/326414-t-infra.2Fbootstrap/topic/Building.20vs.20testing.20.60compiletest.60/near/532040838).

r? ```@Kobzol```
samueltardieu added a commit to samueltardieu/rust that referenced this pull request Aug 2, 2025
…Kobzol

Properly pass path to staged `rustc` to `compiletest` self-tests

Otherwise, this can do weird things like use a global rustc, or try to use stage 0 rustc. This must be properly configured, because `compiletest` is intended to only support one compiler target spec JSON format (of the in-tree compiler).

Historically, this was probably done so before `bootstrap` was really its own thing, and `compiletest` had to be runnable as a much more "self-standing" tool.

Follow-up to rust-lang#144675, as I didn't realize this until Zalathar pointed it out in [#t-infra/bootstrap > Building vs testing &rust-lang#96;compiletest&rust-lang#96; @ 💬](https://rust-lang.zulipchat.com/#narrow/channel/326414-t-infra.2Fbootstrap/topic/Building.20vs.20testing.20.60compiletest.60/near/532040838).

r? ````@Kobzol````
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-CI Area: Our Github Actions CI A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants