Enforce -Dwarnings in CI instead of in the manifest - #3447
Open
plusky wants to merge 1 commit into
Open
Conversation
`warnings = "deny"` in [lints.rust] applies to every build of rtk, not just the ones CI runs. That is a problem for anyone building with a toolchain other than the one CI pins: a rustc newer than rust-version routinely adds or widens a lint, and the first release that does so turns a warning nobody has seen yet into a hard build failure for every downstream consumer at once, with no way to opt out short of patching Cargo.toml. Set RUSTFLAGS: -Dwarnings on the clippy and test jobs instead. CI keeps failing on warnings exactly as before -- and now covers the dependency build too -- while `cargo build` with a newer toolchain degrades to a warning that can be fixed on the normal schedule. unsafe_code = "deny" stays in the manifest: that one is a deliberate design constraint of the crate, not lint drift.
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Second of two packaging papercuts found while packaging rtk for openSUSE (the other is #3446).
[lints.rust] warnings = "deny"applies to every build of rtk, not just the ones CI runs. CI pins its toolchain withdtolnay/rust-toolchain@stable; distributions do not — openSUSE Tumbleweed is on rustc 1.97 against arust-version = "1.91"floor. Every rustc release adds or widens lints, so the first one that touches rtk turns a warning nobody has seen yet into a hard build failure for every downstream consumer simultaneously, and the only escape is patching Cargo.toml.This moves the enforcement to where it belongs:
warnings = "deny"from[lints.rust]RUSTFLAGS: -Dwarningson theclippyandtestjobs inci.ymlCI fails on warnings exactly as it does today — and now covers dependency builds too, which the manifest lint did not. A newer toolchain outside CI degrades to a warning that can be fixed on your normal schedule rather than breaking the build.
unsafe_code = "deny"stays in the manifest: that is a deliberate design constraint of the crate, not lint drift.Happy to put the env at workflow level instead if you would rather have it apply to every job — I kept it to the two that compile so the "Clippy security lints" step, which greps its own output for
warning:, keeps behaving as written.