Consider compiletest a staged ToolStd tool
#144563
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rustbot
added
A-CI
This comment has been minimized.
This comment has been minimized.
jieyouxu
changed the title
compiletest a staged ToolStd tool
|
r? @Kobzol |
|
|
rustbot
added
S-waiting-on-review
|
This PR modifies If appropriate, please update This PR modifies If appropriate, please update This PR modifies If appropriate, please update |
|
Argh, forgot to update PR number. |
… tool Instead, consider this as an `ToolStd` tool until `compiletest`'s dependency in the unstable `#![feature(internal_output_capture)]` is replaced by a stable implementation.
But remove `python3 ../x.py check compiletest --set build.compiletest-use-stage0-libtest=true` from `pr-check-1` and move `./x test compiletest` to the last (and test `compiletest` unit tests against staged compiler/library), as this will require a stage 1 (compiler, library) build to build `compiletest` itself as well.
.. or stabilize some subset of the JSON output, which would be good for many reasons. :) |
Instead of the ad-hoc `COMPILETEST_FORCE_STAGE0` env var. However, explicitly do not test this configuration under CI, as tests are most certainly not guaranteed to pass against stage 0 (compiler, library).
jieyouxu
force-pushed
the
drop-stage0-libtest
branch
from
fede36f to
1a97eec
Compare
Sure, but even then, it can depend on what kind of changes are permitted (even if stabilized) between compiler versions :) Or I suppose "beyond the guaranteed subset of stability". |
|
Would this still work with stage0 local-rebuild testing like cg_clif does in it's CI? For those compiletest has to be built with stage0. https://github.com/rust-lang/rustc_codegen_cranelift/blob/812388a717cbc1311aa7ab3f321d6503b56ecd16/scripts/test_rustc_tests.sh#L172 Edit: I see you still have |
Oh sorry, I was going to ping you about this, but I completely forgor: the answer is yes, it should, but the incantation is slightly different. This use case I believe should be preserved by the third commit 372e25d, but instead of |
Yeah, I was going to ask you about it if I didn't forgor... |
Building compiletest with stage0 vs stage1 has absolutely no effect on which target spec versions compiletest can read.
This is unlikely to change any time soon. Using stage1 to compile compiletest means that each test cycle gets slower by 30s+ due to having to rebuild compiletest. |
Kobzol
reviewed
Jul 28, 2025
|
|
||
| builder.ensure(ToolBuild { | ||
| build_compiler: self.compiler, | ||
| target: self.target, | ||
| tool: $tool_name, | ||
| mode: if is_unstable && !compiletest_wants_stage0 { | ||
| mode: if is_unstable || !builder.config.compiletest_force_stage0 { |
There was a problem hiding this comment.
This is doing something weird, it compiles compiletest using the stage0 compiler, but uses Mode::ToolStd. We should have an assert against that.
There was a problem hiding this comment.
Ugh, yeah this is unintended.
|
Hm yeah. I'll need to think about this. |
Yeah, this is IMO a big issue and a blocker for this PR landing in the current shape. |
Right...
|
|
When it comes to the target JSON issue, I don't understand what any of this has to do with the stage that compiletest is built with. What matters is which rustc it invokes to get the target spec when running the tests. That should obviously be the same rustc that actually runs the tests, and is independent of how it was built. If we support running ui tests with the bootstrap rustc, then we need to be able to parse both the bootstrap rustc output and the in-tree rustc output. But why would we run the tests with the bootstrap rustc...? |
rustbot
added
S-waiting-on-author
|
Discussing this in #t-infra/bootstrap > Building vs testing `compiletest` |
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
Closing this PR, because this approach does not work. |
rustbot
removed
the
S-waiting-on-author
samueltardieu
added a commit
to samueltardieu/rust
that referenced
this pull request
Jul 30, 2025
…bzol 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 rust-lang#144563 that tries to do a way more invasive change which would cause the rebuild problem. Best reviewed commit-by-commit. --- r? `@Kobzol`
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`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Before this PR (i.e. currently),$\left\langle \text{compiler}, \text{library} \right\rangle$ :
compiletestwas in a weird state where it exhibits behavior of bothToolBootstraptools andToolStdtoolsuntil you observe it. At the time, this is becausecompiletestdepended on libtest, but we felt like it was nice to support "faster" checks forcompiletestby allowing it to be built with and thus test against stage 0 libtest. However, this scheme turns out problematic.compiletest, even after removing dependencing on libtest via the new executor, very much depends on unstable features of both#![feature(internal_output_capture)]ofAnd this influences the unit tests of
compiletestitself. Thus,compiletestis very much a staged tool (host-only).This PR tries to reconcile this situation by being faithful to$\left\langle \text{compiler}, \text{library} \right\rangle$ , if we can manage to:
compiletest's nature: thatcompiletestin its current formulation is very much a staged tool. It's possible forcompiletestto be a bootstrap tool and testable with the stage 0compiletest's dependency on#![feature(internal_output_capture)]library feature, andcompiletestand the compiler that is properly synchronized.Commits
build.compiletest-use-stage0-libtestconfig, this was no longer relevant after we removed the dependency on libtest entirely.build.compiletest-use-stage0-libtestor./x test compiletest --stage=0. Forpr-check-1specifically, I moved./x test compiletestto the last, because this will necessarily require buildingcompiletest. Unfortunately, this does mean we lose the ability to "fast-reject". However, I argue that the old "fast-reject" was actually wrong -- that meant testingcompiletest's own unit tests was againstCOMPILETEST_FORCE_STAGE0env var previously that did not go through bootstrap's config handling at all, this PR adds a first-classbuild.compiletest-force-stage0option to preserve the ability to runcompiletest-managed test suites andcompiletest's own unit tests against a providedCOMPILETEST_FORCE_STAGE0=1 ./x test run-make --stage=0, but with this PR, it will be./x test run-make --stage=0 --set build.compiletest-force-stage0=true. In any case, running bothcompiletest-managed test suites (e.g.tests/run-make) orcompiletest's own unit/self tests againstrust-lang/rustCI.This should unblock #144443.