Skip to content
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

x.py test --stage 0 src/tools/clippy does not work #78717

Open
jyn514 opened this issue Nov 4, 2020 · 12 comments
Open

x.py test --stage 0 src/tools/clippy does not work #78717

jyn514 opened this issue Nov 4, 2020 · 12 comments
Labels
A-clippy Area: Clippy A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself C-bug Category: This is a bug. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@jyn514
Copy link
Member

jyn514 commented Nov 4, 2020

$ x.py test --stage 0 src/tools/clippy 
Updating only changed submodules
Submodules updated in 0.01 seconds
    Finished dev [unoptimized + debuginfo] target(s) in 0.09s
Building stage0 std artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
    Finished release [optimized] target(s) in 0.09s
Copying stage0 std from stage0 (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu / x86_64-unknown-linux-gnu)
Building stage0 compiler artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
    Finished release [optimized] target(s) in 0.13s
Copying stage0 rustc from stage0 (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu / x86_64-unknown-linux-gnu)
Building stage0 tool clippy-driver (x86_64-unknown-linux-gnu)
   Compiling clippy v0.0.212 (/home/joshua/rustc/src/tools/clippy)
    Finished release [optimized] target(s) in 1.54s
   Compiling clippy v0.0.212 (/home/joshua/rustc/src/tools/clippy)
    Finished release [optimized] target(s) in 1.49s
     Running build/x86_64-unknown-linux-gnu/stage0-tools/x86_64-unknown-linux-gnu/release/deps/clippy_driver-c0ddca9b2ddf2dba

running 1 test
test test_arg_value ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

     Running build/x86_64-unknown-linux-gnu/stage0-tools/x86_64-unknown-linux-gnu/release/deps/compile_test-93629616c1644211

running 1 test
test compile_test ... FAILED

failures:

---- compile_test stdout ----
thread 'compile_test' panicked at 'Found multiple rlibs for crate `serde`: `"/home/joshua/rustc/build/x86_64-unknown-linux-gnu/stage0-tools/x86_64-unknown-linux-gnu/release/deps/libserde-0cbb27a77c795012.rlib"` and `"/home/joshua/rustc/build/x86_64-unknown-linux-gnu/stage0-tools/x86_64-unknown-linux-gnu/release/deps/libserde-d748cd52d3612bbb.rlib"', src/tools/clippy/tests/compile-test.rs:46:25
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Meta

3478d7c

@jyn514 jyn514 added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) C-bug Category: This is a bug. labels Nov 4, 2020
@jyn514
Copy link
Member Author

jyn514 commented Nov 4, 2020

The really strange thing here is that clippy is built twice:

Building stage0 tool clippy-driver (x86_64-unknown-linux-gnu)
   Compiling clippy v0.0.212 (/home/joshua/rustc/src/tools/clippy)
    Finished release [optimized] target(s) in 1.54s
   Compiling clippy v0.0.212 (/home/joshua/rustc/src/tools/clippy)
    Finished release [optimized] target(s) in 1.49s
     Running build/x86_64-unknown-linux-gnu/stage0-tools/x86_64-unknown-linux-gnu/release/deps/clippy_driver-c0ddca9b2ddf2dba

@jyn514
Copy link
Member Author

jyn514 commented Nov 4, 2020

This does work fine with --stage 1.

@jyn514
Copy link
Member Author

jyn514 commented May 6, 2022

Ok, I dug quite a bit into this. The main issue is that clippy is trying to run tests against the sysroot of the host compiler, not the target compiler. In particular, test --stage 0 is failing because it's trying to use the .rlibs in stage0-sysroot, instead of stage1.

I fixed that. The problem now is that clippy wants to link the test files to its own dependencies. That doesn't work while bootstrapping, because its dependencies were built by a different toolchain than the currently running clippy (stage0, not stage1).

@rust-lang/clippy why does clippy need to use its own dependencies in tests? Is it possible to write the tests without those? I can fix this, but it will make --stage 0 essentially useless because it will build the compiler twice (once to build clippy itself, and once with the fresh toolchain to build its dependencies).

If it's not possible to do that, can we instead add some dummy package that only depends on TEST_DEPENDENCIES, so we don't have to rebuild the whole compiler?

@Manishearth
Copy link
Member

why does clippy need to use its own dependencies in tests

Because clippy contains lints for working on clippy itself, and they need to be tested too

@Manishearth
Copy link
Member

Manishearth commented May 6, 2022

Also tbh my experience with rustc is that stage0 is basically always useless and should never be used, so it doesn't seem too off for stage 0 to be useless for working on clippy. Maybe we can just disallow this for clippy.

thread 'compile_test' panicked at 'Found multiple rlibs for crate `serde`: `"/home/joshua/rustc/build/x86_64-unknown-linux-gnu/stage0-tools/x86_64-unknown-linux-gnu/release/deps/libserde-0cbb27a77c795012.rlib"` and `"/home/joshua/rustc/build/x86_64-unknown-linux-gnu/stage0-tools/x86_64-unknown-linux-gnu/release/deps/libserde-d748cd52d3612bbb.rlib"', src/tools/clippy/tests/compile-test.rs:46:25
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

worth noting, this error is kinda also because compiletest doesn't quite know what crates to link to, this happens pretty often in many different scenarios.

@jyn514
Copy link
Member Author

jyn514 commented May 6, 2022

@Manishearth the numbering for clippy is inconsistent with the numbering for the rest of the compiler. test --stage 0 builds the compiler once; test --stage 1 builds it twice. That's very painful when trying to iterate quickly.

That error up there is outdated, it was fixed by rust-lang/rust-clippy#7631. The new error is

error[E0514]: found crate `std` compiled by an incompatible version of rustc
  |
  = help: please recompile that crate using this compiler (rustc 1.61.0-beta.5 (cf5fd8947 2022-05-04)) (consider running `cargo clean` first)
  = note: the following crate versions were found:
          crate `std` compiled by rustc 1.62.0-nightly (e7575f967 2022-04-14): /home/callie/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libstd_detect-939385b13e387309.rlib
          crate `std` compiled by rustc 1.62.0-nightly (e7575f967 2022-04-14): /home/callie/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libstd-443e20b28eeed844.rlib

or something similar - I don't have the exact output on hand, but it's related to stage0 vs stage1 sysroot.

@jyn514
Copy link
Member Author

jyn514 commented May 6, 2022

If it's not possible to do that, can we instead add some dummy package that only depends on TEST_DEPENDENCIES, so we don't have to rebuild the whole compiler?

This is hard to do, because cargo only allows one library per package, and doesn't allow building binaries without first building the library :( I guess we could hard-code the dependencies clippy uses in another package somewhere? nested workspaces are still a pipe dream unfortunately ...

@RalfJung
Copy link
Member

RalfJung commented Aug 31, 2022

Also tbh my experience with rustc is that stage0 is basically always useless and should never be used

FWIW Miri tests work fine in stage 0, and generally there are very few things that require building the compiler twice these days -- testing clippy is one of them. This makes clippy failures more painful to deal with than most other issues one runs into when working on rustc.

@RalfJung
Copy link
Member

RalfJung commented Aug 3, 2024

Unfortunately this issue is still present, which means that having to bless clippy tests takes about 2x to 3x as long (and >2x the disk space) as it takes to bless any other part of rustc I've had to bless in the last years -- clippy is literally the only thing that forces me to do a 2nd-stage build.

Because clippy contains lints for working on clippy itself, and they need to be tested too

I assume those tests fail fairly rarely due to rustc changes, so it'd be good if it were possible to at least bless all the other tests without having to wait for rustc bootstrap to finish.

@RalfJung RalfJung added A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself A-clippy Area: Clippy labels Aug 3, 2024
@jyn514
Copy link
Member Author

jyn514 commented Aug 3, 2024

I assume those tests fail fairly rarely due to rustc changes, so it'd be good if it were possible to at least bless all the other tests without having to wait for rustc bootstrap to finish.

@RalfJung x test --bless --exclude clippy

@jyn514
Copy link
Member Author

jyn514 commented Aug 3, 2024

(unless things have radically changed while i'm gone i suspect there will still be other things using stage 2, like linkchecker)

@RalfJung
Copy link
Member

RalfJung commented Aug 4, 2024

@jyn514 I meant all the clippy tests that don't need to depend on clippy themselves. For the majority of clippy tests, I don't see why they would need a stage 2, so maybe we can require the higher stage only for the tests that actually need it.

(unless things have radically changed while i'm gone i suspect there will still be other things using stage 2, like linkchecker)

Yeah there probably are, but at least with the kind of compiler changes I am doing, you need to re-bless them even less often than clippy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-clippy Area: Clippy A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself C-bug Category: This is a bug. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants