-
Notifications
You must be signed in to change notification settings - Fork 13.5k
clippy: make tests work in stage 1 #144027
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
base: master
Are you sure you want to change the base?
Conversation
rustbot has assigned @Mark-Simulacrum. Use |
Some changes occurred in src/tools/clippy cc @rust-lang/clippy |
if stage < 2 { | ||
eprintln!("WARNING: clippy tests on stage {stage} may not behave well."); | ||
eprintln!("HELP: consider using stage 2"); | ||
} |
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.
There is some logic somewhere that changes the default stage for ./x test clippy
to 2... that can be removed now, but I couldn't find the logic.
210e667
to
ec30b29
Compare
// set `--sysroot`, so we need to use bootstrap's rustc wrapper. That wrapper | ||
// however has some staging logic that is hurting us here, so to work around | ||
// that we set both the "real" and "staging" rustc to TEST_RUSTC, including the | ||
// associated library paths. |
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.
It should be possible to use a different approach here... somehow, aux builds are working for clippy tests. It seems to be using clippy itself as the compiler for that, and that's somehow okay? So, we could try to do the same here, instead of all this TEST_RUSTC stuff. (That would also be closer to what Miri does.)
But TBH I found something that works so I am not overly inclined to debug a different approach. ;)
This finally fixes #78717 :)
Similar to what Miri already does, the clippy test step needs to carefully consider which compiler is used to build clippy and which compiler is linked into clippy (and thus must be used to build the test dependencies). On top of that we have some extra complications that Miri avoided by using
cargo-miri
for building its test dependencies: we need cargo to use the right rustc and the right sysroot, but rust-lang/cargo#4423 makes this quite hard to do. See the long comment insrc/tools/clippy/tests/compile-test.rs
for details.Some clippy tests tried to import rustc crates; that fundamentally requires a full bootstrap loop so it cannot work in stage 1. I had to kind of guess what those tests were doing so I don't know if my changes there make any sense.
Cc @flip1995 @Kobzol