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 clippy loads two different version of core #77309

Closed
Nicholas-Baron opened this issue Sep 28, 2020 · 14 comments · Fixed by #77351
Closed

./x.py clippy loads two different version of core #77309

Nicholas-Baron opened this issue Sep 28, 2020 · 14 comments · Fixed by #77351
Assignees
Labels
C-bug Category: This is a bug. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@Nicholas-Baron
Copy link
Contributor

I am used to cargo clippy providing many good hints in other Rust projects. However, the equivalent here (./x.py clippy) does not complete after around 64,000 lines of output.

The output of a single clippy run.

@jyn514
Copy link
Member

jyn514 commented Sep 29, 2020

How useful is it to have 64,000 lines of clippy output? Consider using x.py clippy library/std or some other specific crate instead of the whole source tree.

@jyn514 jyn514 added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Sep 29, 2020
@Nicholas-Baron
Copy link
Contributor Author

@jyn514 Running ./x.py clippy library/std also leads to the same amount of lines.

Additionally, running rg 'error\[' -c on the file shows around 500 errors.
Most seen to be E0277: the size of values of type 'foo' cannot be known at compilation time.
Removing these may help see other errors.

@jyn514
Copy link
Member

jyn514 commented Sep 29, 2020

That sounds like it's just broken in general then, those are the errors you get by compiling with cargo check instead of x.py. You're definitely using x.py clippy, right?

@jyn514 jyn514 added the C-bug Category: This is a bug. label Sep 29, 2020
@Nicholas-Baron
Copy link
Contributor Author

Yes, I am running ./x.py clippy and getting these errors.
I have mostly resorted to using ./x.py check, but I do want to get the clippy option working.
Are there any details of my setup that would be helpful in diagnosing the issue?

@jyn514
Copy link
Member

jyn514 commented Sep 29, 2020

Ok, I can reproduce this locally. cc @Mark-Simulacrum, it looks like x.py clippy has been broken for some time:

$ ./x.py clippy library/std -v
Checking std artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
running: "/home/joshua/rustc/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "clippy" "--target" "x86_64-unknown-linux-gnu" "-Zbinary-dep-depinfo" "-j" "8" "--release" "--features" "panic-unwind backtrace compiler-builtins-c" "--manifest-path" "/home/joshua/rustc/library/test/Cargo.toml" "--message-format" "json-render-diagnostics" "--" "--cap-lints" "warn"

... many thousands of lines of output ...

error: duplicate lang item in crate `core` (which `rustc_std_workspace_core` depends on): `bool`.
  |
  = note: the lang item is first defined in crate `core` (which `getrandom` depends on)
  = note: first definition in `core` loaded from /home/joshua/.local/lib/rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib/libcore-2675a9a46b5cec89.rlib
  = note: second definition in `core` loaded from /home/joshua/rustc/build/x86_64-unknown-linux-gnu/stage0-std/x86_64-unknown-linux-gnu/release/deps/libcore-04b26fe1c15aa843.rmeta

@jyn514 jyn514 changed the title ./x.py clippy does not run to completion ./x.py clippy loads two different version of core Sep 29, 2020
@Mark-Simulacrum
Copy link
Member

Yeah, this is because clippy isn't being sent through the sysroot shim. I don't myself plan to invest time into fixing this, FWIW, but if someone wants to dig in I would figure out why clippy is behaving differently from rustc itself here. I suspect it's because we're shimming rustc with src/bootstrap/bin/rustc.rs and we need to do the same for clippy.

@jyn514 jyn514 self-assigned this Sep 29, 2020
@jyn514
Copy link
Member

jyn514 commented Sep 29, 2020

@Mark-Simulacrum do you know why this requires a separate binary instead of passing --sysroot to cargo clippy?

running: "/home/joshua/rustc/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "clippy" "--target" "x86_64-unknown-linux-gnu" "-Zbinary-dep-depinfo" "-j" "8" "--release" "--features" "panic-unwind backtrace compiler-builtins-c" "--manifest-path" "/home/joshua/rustc/library/test/Cargo.toml" "--message-format" "json-render-diagnostics" "--" "--cap-lints" "warn" "--sysroot" "/home/joshua/rustc/build/x86_64-unknown-linux-gnu/stage0-sysroot"
error: failed to run `rustc` to learn about target-specific information

Caused by:
  process didn't exit successfully: `/home/joshua/.local/lib/rustup/toolchains/nightly-x86_64-unknown-linux-gnu/bin/clippy-driver /home/joshua/rustc/build/bootstrap/debug/rustc - --crate-name ___ --print=file-names --crate-type bin --crate-type rlib --crate-type dylib --crate-type cdylib --crate-type staticlib --crate-type proc-macro --print=sysroot --print=cfg` (exit code: 1)
  --- stderr
  error: Option 'sysroot' given more than once

@jyn514
Copy link
Member

jyn514 commented Sep 29, 2020

Also that approach won't work anyway, there's no CLIPPY analogue of RUSTDOC: https://doc.rust-lang.org/cargo/reference/environment-variables.html

@Mark-Simulacrum
Copy link
Member

Broadly speaking it probably doesn't but a separate binary usually makes things easier. We will want to add that env variable (I am surprised to hear it wasn't added initially), via a cargo patch.

@jyn514
Copy link
Member

jyn514 commented Sep 29, 2020

Broadly speaking it probably doesn't but a separate binary usually makes things easier

I'm surprised you say that, I find it makes things a lot more painful because it means you have to set all sorts of env variables if you want to debug an issue.

error: Option 'sysroot' given more than once

I found this is because rustc.rs is passing sysroot itself; fn args() is for arguments passed to all binaries, not just clippy:

if !args.iter().any(|arg| arg == "--sysroot") {
cmd.arg("--sysroot").arg(&sysroot);
}

@jyn514
Copy link
Member

jyn514 commented Sep 29, 2020

That wasn't actually the issue, clippy is also setting sysroot: https://github.com/rust-lang/rust-clippy/blob/master/src/driver.rs#L354

@jyn514
Copy link
Member

jyn514 commented Sep 29, 2020

Ok, setting SYSROOT to the proper sysroot got me a ton further:

error[E0463]: can't find crate for `std`

error: could not compile `std`

I'm not sure why clippy is trying to dynamically load std from the sysroot ... @rust-lang/clippy do you know what's going on here?

@jyn514
Copy link
Member

jyn514 commented Sep 29, 2020

For context, this is while running on std itself, so std isn't available yet - it hasn't been compiled.

@jyn514
Copy link
Member

jyn514 commented Sep 29, 2020

Possibly related: rust-lang/rust-clippy#2874

jyn514 added a commit to jyn514/rust-clippy that referenced this issue Sep 30, 2020
This keeps the sysroot consistent when there are multiple `rustc`
programs floating around.

Context: rust-lang/rust#77309
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Nov 6, 2020
…acrum

Fix `x.py clippy`

I don't think this ever worked.

Fixes rust-lang#77309. `--fix` support is a work in progress, but works for a very small subset of `libtest`.

This works by using the host `cargo-clippy` driver; it does not use `stage0.txt` at all. To mitigate confusion from this, it gives an error if you don't have `rustc +nightly` as the default rustc in `$PATH`. Additionally, it means that bootstrap can't set `RUSTC`; this makes it no longer possible for clippy to detect the sysroot itself. Instead, bootstrap passes the sysroot to cargo.

r? `@ghost`
@bors bors closed this as completed in dc06a36 Nov 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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