-
Notifications
You must be signed in to change notification settings - Fork 14k
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? |
Kobzol
left a comment
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
|
@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````
Rollup merge of #144782 - jieyouxu:compiletest-selftests, r=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 #144675, as I didn't realize this until Zalathar pointed it out in [#t-infra/bootstrap > Building vs testing `compiletest` @ 💬](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 iscompiletestself-tests against stage 0 rustc. This makes it very annoying for PRs wanting to change target spec JSON format, whichcompiletestdepends on for target information, as otherwisecompiletestwould have to know how to handle 2 different target spec JSON formats and know when to pick which.Instead of doing that, we change
compiletestself-tests to reject running against stage 0rustcunless explicitly allowed withbuild.compiletest-allow-stage0=true.build.compiletest-allow-stage0is a proper bootstrap config option in favor of the ad-hocCOMPILETEST_FORCE_STAGE0env var. This means that:./x test src/tools/compiletest --stage=0is not allowed, unlessbuild.compiletest-allow-stage0=trueis set. In this scenario,compiletestself-tests should be expected to fail unless the stage 0rustcas provided is like codegen_cranelift where it's actually built from in-treerustcsources../x test src/tools/compiletest --stage=0to./x test src/tools/compiletest --stage=1, and move it topr-check-2. Yes, this involves building the stage 1 compiler, butpr-check-2already has to build stage 1 compiler to test stage 1 library crates.compiletestis only intended to support one target spec JSON format, namely the one corresponding to the in-treerustc.compiletest-use-stage0-libtestUX optimization where changingcompiler/tree should still not require rebuildingcompiletestas long asbuild.compiletest-use-stage0-libtest=true, as that should remain orthogonal. This is completely unlike my previous attempt at Considercompiletesta stagedToolStdtool #144563 that tries to do a way more invasive change which would cause the rebuild problem.Best reviewed commit-by-commit.
r? @Kobzol