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

Weird behavior of clippy in qualification script #14045

Open
emabee opened this issue Jan 20, 2025 · 9 comments
Open

Weird behavior of clippy in qualification script #14045

emabee opened this issue Jan 20, 2025 · 9 comments
Labels
C-bug Category: Clippy is not doing the correct thing

Comments

@emabee
Copy link

emabee commented Jan 20, 2025

Summary

We recognized recently that our qualification script runs into issues when working with nightly.
The script runs various cargo commands to build and test.

With current nightly versions, it has issues to run cargo +nightly clippy --package hdbconnect -- -D warnings:

It fails with errors like this:

warning: unexpected `cfg` condition name: `test`
  --> <some_file_name>.rs:84:7
   |
84 | #[cfg(test)]
   |       ^^^^
   |
   = help: expected names are: `clippy`, `debug_assertions`, `doc`, `docsrs`, `doctest`, `feature`, `fmt_debug`, `miri`, `overflow_checks`, `panic`, `proc_macro`, `relocation_model`, `rustfmt`, `sanitize`, `sanitizer_cfi_generalize_pointers`, `sanitizer_cfi_normalize_integers`, `target_abi`, `target_arch`, `target_endian`, `target_env`, `target_family`, `target_feature`, `target_has_atomic`, `target_has_atomic_equal_alignment`, `target_has_atomic_load_store`, `target_os`, `target_pointer_width`, `target_thread_local`, `target_vendor`, `ub_checks`, `unix`, and `windows`
   = help: consider using a Cargo feature instead
   = help: or consider adding in `Cargo.toml` the `check-cfg` lint config for the lint:
            [lints.rust]
            unexpected_cfgs = { level = "warn", check-cfg = ['cfg(test)'] }
   = help: or consider adding `println!("cargo::rustc-check-cfg=cfg(test)");` to the top of the `build.rs`
   = note: see <https://doc.rust-lang.org/nightly/rustc/check-cfg/cargo-specifics.html> for more information about checking conditional configuration
   = note: `#[warn(unexpected_cfgs)]` on by default

This did not happen until some weeks ago. The source code was not touched in that timeframe.

Executing the command cargo +nightly clippy --package hdbconnect -- -D warnings directly on the shell is successful!

Reproducer

Here is a minimized version of the script to reproduce this issue:

#!/usr/bin/env run-cargo-script
//! ```cargo
//! [dependencies]
//! yansi = "1.0"
//! ```
extern crate yansi;
use std::{process::Command, time::Instant};

fn main() {
    macro_rules! run_command {
        ($cmd:expr) => {
            let mut command = command!($cmd);
            let mut child = command.spawn().unwrap();
            let status = child.wait().unwrap();
            if !status.success() {
                print!("> {}", yansi::Paint::red("qualify terminates due to error"));
                std::process::exit(-1);
            }
        };
    }

    macro_rules! command {
        ($cmd:expr) => {{
            println!("\n> {}", yansi::Paint::yellow($cmd));
            let mut chips = $cmd.split(' ');
            let mut command = Command::new(chips.next().unwrap());
            for chip in chips {
                command.arg(chip);
            }
            command
        }};
    }

    // works:
    run_command!("cargo clippy --package hdbconnect -- -D warnings");

    // fails:
    run_command!("cargo +nightly clippy --package hdbconnect -- -D warnings");
}

It fails, but should run successfully.

Version

>> rustc --version
rustc 1.83.0 (90b35a623 2024-11-26)
>> rustc +nightly --version
rustc 1.86.0-nightly (9a1d156f3 2025-01-19)

Additional Labels

No response

@emabee emabee added the C-bug Category: Clippy is not doing the correct thing label Jan 20, 2025
@y21
Copy link
Member

y21 commented Jan 22, 2025

@emabee does this also happen if you replace clippy with check (or build) in that command? The warning from the unexpected_cfgs lint you're seeing is a builtin compiler lint, so that leads me to believe it's probably not a bug in clippy and should be moved to the rust repo, but want to confirm first.

@emabee
Copy link
Author

emabee commented Jan 22, 2025

does this also happen if you replace clippy with check (or build) in that command?

In fact, this only happens with clippy. I double-checked also in a smaller project without workspaces:

// works:
run_command!("cargo build");

// works:
run_command!("cargo clippy -- -D warnings");

// does not abort, but reports inappropriate warnings:
run_command!("cargo +nightly clippy");

// aborts because of the inappropriate warnings:
run_command!("cargo +nightly clippy -- -D warnings");

@emabee
Copy link
Author

emabee commented Jan 22, 2025

It seems to complain about any #[cfg(test)] in the code.

@smoelius
Copy link
Contributor

smoelius commented Jan 22, 2025

@emabee Please try adding .env_remove("CARGO") to each command.

I do not understand the root cause (personal failing), but I ran into a similar problem and this was the fix.

The root cause has something to do with rust-lang/rust#131729 and rust-lang/cargo#14963.

EDIT: I think it has something to do with this line:

let mut cmd = Command::new(env::var("CARGO").unwrap_or("cargo".into()));

@Alexendoo
Copy link
Member

Yeah it's down to both of those

It goes something like: cargo run sets the CARGO environment variable to the stable version of cargo while executing the script with run_command! etc in it

The cargo +nightly clippy invocation in the script inherits this value of CARGO and uses it when it internally runs cargo check (set to use clippy-driver rather than rustc)

So you're in a position where nightly clippy-driver is being executed by stable cargo, which doesn't yet pass test as a well known cfg

Running $CARGO is useful to fix some issues though (#11943, rust-lang/cargo#12656), it might be worth opening an issue on the Cargo issue tracker to see if there's a different approach they would recommend

@smoelius
Copy link
Contributor

it might be worth opening an issue on the Cargo issue tracker to see if there's a different approach they would recommend

You mean a different approach to invoking nightly Clippy as a subcommand?

@Alexendoo
Copy link
Member

For how cargo-clippy or other external subcommands should determine which cargo to run

@emabee
Copy link
Author

emabee commented Jan 23, 2025

@emabee Please try adding .env_remove("CARGO") to each command.

Yes, this helps! Thanks a lot.

@Alexendoo, @smoelius
would one of you follow up with an issue on Cargo? I'm not sure I could really be of help here. Thanks a lot!

@Alexendoo
Copy link
Member

Yep, opened rust-lang/cargo#15099

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing
Projects
None yet
Development

No branches or pull requests

4 participants