- 
                Notifications
    
You must be signed in to change notification settings  - Fork 13.9k
 
          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
      | 
           @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