feat(warnings): add build.warnings option #14388
Conversation
rustbot
added
A-build-execution
|
☔ The latest upstream changes (presumably #14423) made this pull request unmergeable. Please resolve the merge conflicts. |
tests/testsuite/warning_override.rs
Outdated
| [FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s | ||
|
|
||
| "#]] | ||
| }); | ||
|
|
||
| const ALLOW_CACHED: LazyLock<Inline> = LazyLock::new(|| { | ||
| str![[r#" | ||
| [WARNING] unused variable: `x` | ||
| ... | ||
| [WARNING] `foo` (bin "foo") generated 1 warning |
There was a problem hiding this comment.
Why is this line omitted and should we?
There was a problem hiding this comment.
I think we decided that we wanted allow to not print any warnings / warning summary.
tests/testsuite/warning_override.rs
Outdated
| "#]] | ||
| }); | ||
|
|
||
| fn make_project(main_src: &str) -> Project { |
There was a problem hiding this comment.
nit: I'm always a bit cautious of sharing code too much in tests. In this case, I think it works as the intent of the tests isn't obscure. The main question is the make_project call, it did take me a second when reading the code to think "yeah, let x = 3; is being meant for main. One option would be to move the warning into this and call it make_project_with_rustc_warning or something.
Hmm, thats a good point. When we add the checking of cargo warnings, we'll need tests for those as well. The names are fairly general in this code and it will all need to be modified in the follow up. Clarifying things now can make it clearer for now and make it easier to introduce new tests in the future.
There was a problem hiding this comment.
Renamed to make_project_with_rustc_warning
|
Thanks for the updates! |
What does this PR try to resolve?
Allows control over warnings via a new Cargo configuration option
build.warnings.The primary use case is wanting CI runs to deny warnings, while keeping them as warnings for local development. While this is currently possible by conditionally setting
RUSTFLAGSin CI, that approach is not available for projects that already use another mechanism for settingRUSTFLAGS, since the two would conflict. Additionally, we may in the future make this apply to more than warnings fromrustc.build.warningsconfig option.Option values:
denyemits an error if any warnings occurredwarnis the current existing behaviorallowhides the warningsThe option currently only applies to warnings coming from
rustc, however it's documented as allowing additional categories of warnings from Cargo to be added.Fixes #8424
Fixes #14258
How should we test and review this PR?
Tests are included in the PR. For local testing, after building
cargowith this PR, run