cargo icon indicating copy to clipboard operation
cargo copied to clipboard

Tracking Issue for warnings (config build.warnings)

Open epage opened this issue 1 year ago • 7 comments

Summary

Original issue: #8424, #14258 Implementation: #14388 Documentation: https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#warnings

Allows control over warnings via a new Cargo configuration option build.warnings.

Unresolved Issues

  • [ ] What operations should this apply to and is [build] the right home for this?
  • [ ] Should we turn rustc hard warnings into errors? See #16213
  • [ ] What non-rustc lints are in scope? See also https://github.com/rust-lang/cargo/issues/8424#issuecomment-1915272203
    • Including build script warnings? See https://github.com/rust-lang/cargo/issues/8424#issuecomment-2323952554

Future Extensions

  • CLI support
    • If or where we have individual lint level control (cargo clippy), these looking odd next to each other
    • See also https://github.com/rust-lang/cargo/issues/8424#issuecomment-1915265827
  • If we add a new level, config/cli to control it

About tracking issues

Tracking issues are used to record the overall progress of implementation. They are also used as hubs connecting to other relevant issues, e.g., bugs or open design questions. A tracking issue is however not meant for large scale discussion, questions, or bug reports about a feature. Instead, open a dedicated issue for the specific matter and add the relevant feature gate label.

epage avatar Nov 09 '24 02:11 epage

This config currently applies to all warnings, including hard warnings, not just lints. This is unfortunate, because the whole point of hard warnings is not to fail your build, and there is no way to allow or deny them. Here is an example of a build that failed due to build.warnings=deny firing on a hard warning:

Building stage1 library artifacts (stage1:x86_64-unknown-linux-gnu -> stage1:wasm32-wasip1)
     Compiling std v0.0.0 (/checkout/library/std)
  warning: dropping unsupported crate type `dylib` for target `wasm32-wasip1`
  warning: `std` (lib) generated 1 warning
  error: warnings are denied by `build.warnings` configuration
  Bootstrap failed while executing `--stage 2 test --host= --target wasm32-wasip1 tests/run-make tests/run-make-cargo tests/ui tests/mir-opt tests/codegen-units tests/codegen-llvm tests/assembly-llvm library/core`

Fixing the warning here is extremely non-trivial (override and remove -C crate-type=dylib in bootstrap's rustc shim when targetting wasi? change lib.crate-type = ["dylib"] in library/std/Cargo.toml to be passed through RUSTFLAGS? probably that will mess with cargo's own caching though). Allowing it is not possible because it's not a lint.

I think the right behavior here is for cargo to only fail the build if at least one lint warning was emitted, and ignore hard warnings. I believe Cargo can detect that by whether there's a code key in the JSON output, but I haven't rigorously tested that; also seems ok to make JSON changes on the compiler side if it makes cargo's life easier.


to reproduce this warning, add lib.crate-type=["dylib"] to any Cargo project and then run cargo check --target wasm32-wasip1.

jyn514 avatar Nov 04 '25 17:11 jyn514

This is unfortunate, because the whole point of hard warnings is not to fail your build,

All I'm finding on this is from https://rustc-dev-guide.rust-lang.org/diagnostics.html#lints-versus-fixed-diagnostics

Hard-coded warnings (those using methods like span_warn) should be avoided for normal code, preferring to use lints instead. Some cases, such as warnings with CLI flags, will require the use of hard-coded warnings.

It looks like we also include these hard warnings in the warnings count and I worry people might be confused by why these warnings aren't included.

However, you could say that Cargo's warnings are in a similar boat and we're not checking those, waiting on us turning them into lints. However, part of the motivation for that is for us to even evaluate whether the warning belongs as a warning.

Also, if we sell this as "like -Dwarnings", then yes we shouldn't deal with hard warnings.

to reproduce this warning, add lib.crate-type=["dylib"] to any Cargo project and then run cargo check --target wasm32-wasip1.

Do you have an example that uses a regular target or one of our cross-compile targets we use in tests?

epage avatar Nov 04 '25 17:11 epage

Do you have an example that uses a regular target or one of our cross-compile targets we use in tests?

All cross-compile targets you test support all crate types and thus won't ever trigger this warning.

Another example of a hard warning would be the linker warnings once we re-enable them again. Those can environment specific and not at all the fault of the project that gets compiled. We can't really distinguish between linker warnings caused by the environment and caused by the project we are compiling.

bjorn3 avatar Nov 04 '25 17:11 bjorn3

Another example of a hard warning would be the linker warnings once we re-enable them again

I had changed those to a lint, precisely because they're unpredictable.

jyn514 avatar Nov 04 '25 18:11 jyn514

here is a hard warning that can be reproduced on any target:

$ rustc src/lib.rs -C extra-filename=x -o x.rs --crate-type lib
warning: ignoring -C extra-filename flag due to -o flag

jyn514 avatar Nov 04 '25 18:11 jyn514

because the whole point of hard warnings is not to fail your build, and there is no way to allow or deny them.

My memory and understanding is that is not the intent. Hard warnings are only there because it was too difficult to wire them into the linting system. https://github.com/rust-lang/rust/issues/21204 is tracking that problem. Generally hard-warnings should be avoided.

ehuss avatar Nov 04 '25 18:11 ehuss

I'm leaning towards us not erroring on hard warnings since they can't be resolved by the user overriding them, only by removing the cause.

epage avatar Nov 05 '25 21:11 epage