-
Notifications
You must be signed in to change notification settings - Fork 13.6k
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
Conversation
925f797
to
0539cc1
Compare
cc @bjorn3 on the cg_clif changes (commit 2 mostly) |
This PR modifies If appropriate, please update This PR modifies If appropriate, please update Some changes occurred in compiler/rustc_codegen_cranelift cc @bjorn3 |
0539cc1
to
89f67a4
Compare
Hello? |
There was a problem hiding this 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
).
89f67a4
to
450c589
Compare
Some changes occurred in compiler/rustc_codegen_gcc Some changes occurred in src/tools/opt-dist cc @Kobzol |
Changes since last review:
|
450c589
to
2e92be1
Compare
compiletest
self-tests against stage 0 rustc unless forcedcompiletest
self-tests against stage 0 rustc unless explicitly allowed
dc2a43a
to
2d0e721
Compare
@rustbot author |
2d0e721
to
274e067
Compare
In favor of the adhoc `COMPILETEST_FORCE_STAGE0` env var.
…test-allow-stage0`
…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.
274e067
to
e954253
Compare
@rustbot review |
Thanks! @bors r+ |
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
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`
…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`
…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``
…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```
…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````
Currently, in
pr-check-1
, we runpython3 ../x.py test --stage 0 src/tools/compiletest
, which iscompiletest
self-tests against stage 0 rustc. This makes it very annoying for PRs wanting to change target spec JSON format, whichcompiletest
depends on for target information, as otherwisecompiletest
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 0rustc
unless explicitly allowed withbuild.compiletest-allow-stage0=true
.build.compiletest-allow-stage0
is a proper bootstrap config option in favor of the ad-hocCOMPILETEST_FORCE_STAGE0
env var. This means that:./x test src/tools/compiletest --stage=0
is not allowed, unlessbuild.compiletest-allow-stage0=true
is set. In this scenario,compiletest
self-tests should be expected to fail unless the stage 0rustc
as provided is like codegen_cranelift where it's actually built from in-treerustc
sources../x test src/tools/compiletest --stage=0
to./x test src/tools/compiletest --stage=1
, and move it topr-check-2
. Yes, this involves building the stage 1 compiler, butpr-check-2
already has to build stage 1 compiler to test stage 1 library crates.compiletest
is only intended to support one target spec JSON format, namely the one corresponding to the in-treerustc
.compiletest-use-stage0-libtest
UX optimization where changingcompiler/
tree should still not require rebuildingcompiletest
as long asbuild.compiletest-use-stage0-libtest=true
, as that should remain orthogonal. This is completely unlike my previous attempt at Considercompiletest
a stagedToolStd
tool #144563 that tries to do a way more invasive change which would cause the rebuild problem.Best reviewed commit-by-commit.
r? @Kobzol