Cargo caches failed `cargo clippy -- --bad-arg`

[Alexendoo]

Problem

From rust-lang/rust-clippy#12623 (comment):

cargo new --lib demo && cd demo

cargo check
cargo clippy -- -x
cargo clippy

• The first clippy/check creates the target dir including target/.rustc_info.json, it seems failures aren't recorded in .rustc_info.json if it doesn't exist yet
• In the second command cargo records the result of the target info acquisition under a different hash
• The third command uses the same hash to retrieve the previously failed result

The issue being CLIPPY_ARGS is not taken into account when creating the hash

Output:

    Creating library `demo` package
note: see more `Cargo.toml` keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
    Checking demo v0.1.0 (/.../demo)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.14s
error: process didn't exit successfully: `/.../rustc -vV` (exit status: 1)
--- stderr
error: Unrecognized option: 'x'


error: process didn't exit successfully: `/.../clippy-driver /.../rustc -vV` (exit status: 1)
--- stderr
error: Unrecognized option: 'x'

cargo clippy -- -x is expected to fail but the following cargo clippy is not

In normal operation clippy-driver gets far enough along to register CLIPPY_ARGS into the env depinfo but that doesn't happen here

Steps

No response

Possible Solution(s)

CLIPPY_ARGS could be added to the fingerprint as a special case, but the issue could also exist for other wrappers

Notes

No response

Version

No response

[epage]

Note that cargo has no knowledge of CLIPPY_ARGS, one way or the other, to be able to fingerprint it.

[Alexendoo]

Would not caching failed invocations in .rustc_info.json be an option?

[epage]

The command being invoked is

$ /home/epage/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/clippy-driver /home/epage/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/rustc -vV

So CLIPPY_ARGS is being passed through to this which is causing it to fail.

Trying 1.75, this wasn't a problem. Looks like this might have been broken by #13659

CC @RalfJung

[epage]

@RalfJung , I'm not seeing a problem statement in #10885. Any further details on that?

[epage]

Would not caching failed invocations in .rustc_info.json be an option?

Caching of failures was intentionally added in #9112 (cc5e9df)

This commit updates the rustc info cache to cache failures to execute
rustc as well as successes. This fixes a weird issue where if you're
probing for flags the rustc_info_cache test fails on channels which
don't have the flag since previously a failure to execute rustc resulted
in never caching the result.

The problem in that case isn't too clear to me and would need further investigation to go down that route.

[RalfJung]

@RalfJung , I'm not seeing a problem statement in #10885. Any further details on that?

That issue explains the motivation, not sure what to add. Also, what good is a "wrapper" that doesn't actually get invoked for all rustc invocation? If the goal of the wrapper is to e.g. swap out which rustc is being called, it is crucial that all invocations are made through the wrapper.

Seems like the clippy wrapper is just blindly adding extra arguments to all invocations. Note that rustc -vV is not the only such "query invocation" cargo does, it also does a rustc invocation to print all available targets, and that has always gone through the wrapper. So something must be treating that query different -- either that doesn't get cached for some reason (seems unlikely) or clippy is already aware of it and avoids adding potentially faulty arguments, in which case that logic needs to be extended to recognize other "queries" as well.

[epage]

That issue explains the motivation, not sure what to add.

From my looking it came down to

This is causing us a bunch of pain right now since we need to figure out how to best intercept this -vV call.

without an explanation of why that call needs to be intercepted

Seems like the clippy wrapper is just blindly adding extra arguments to all invocations. Note that rustc -vV is not the only such "query invocation" cargo does, it also does a rustc invocation to print all available targets, and that has always gone through the wrapper. So something must be treating that query different -- either that doesn't get cached for some reason (seems unlikely) or clippy is already aware of it and avoids adding potentially faulty arguments, in which case that logic needs to be extended to recognize other "queries" as well.

This would be a good next step to investigate when considering all possible directions for resolving this.

[RalfJung]

Please also read the rest of the issue. :)

The context is that Miri and bootstrap use RUSTC_WRAPPER to completely replace the rustc being invoked; there is no a priori relationship between $RUSTC_WRAPPER rustc and the rustc in the path. Why they need to do that is not super relevant, it's exactly the kind of usecase that RUSTC_WRAPPER is designed for and they have done this for many years. Because $RUSTC_WRAPPER rustc invokes a completely different binary, the result cargo gets by invoking rustc -vV without the wrapper is obviously bogus since it is talking to the wrong compiler.

[Alexendoo]

It was an issue in 1.75 also

export RUSTUP_TOOLCHAIN=1.75
cargo new --lib demo && cd demo

cargo check
cargo clippy -- -x
cargo clippy

error: failed to run `rustc` to learn about target-specific information

Caused by:
  process didn't exit successfully: `...\clippy-driver.exe '...\rustc.exe' 
- --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=split-debuginfo --print=crate-name --print=cfg` (exit code: 1)
  --- stderr
  error: Unrecognized option: 'x'

error: failed to run `rustc` to learn about target-specific information

Caused by:
  process didn't exit successfully: `...\clippy-driver.exe '...\rustc.exe' 
- --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=split-debuginfo --print=crate-name --print=cfg` (exit code: 1)
  --- stderr
  error: Unrecognized option: 'x'

Skipping some steps if we encounter --print/etc is something I'll try on the clippy driver side

[RalfJung]

Yeah that is exactly the other "query invocation" I mentioned above. #13659 just made it so that both query invocations are fed through the wrapper consistently, but clippy's wrapper behavior in combination with caching was a problem even when just one of the queries is fed through the wrapper.