Skip to content

Commit 5b15d68

Browse files
committed
build: Tweak make validate-rust
This took me some thinking and experimenting. Basically we want: - Hard deny some warnings (this is covered by the Cargo.toml workspace.lints.rust) - Gate merging to main in CI on an exact set of warnings we want to forbid, but *without* also using a blanket -Dwarning deny policy because that could break our build when the compiler revs. - A corollary to the previous: allow developing locally without killing the build just because you have an unused import or some dead code (for example). So we don't want to add `dead_code = deny` into the Cargo.toml. - Be able to easily reproduce locally what CI is gating on in an efficient way. We already had `make validate-rust` which was intending to navigate this, but what was missing was the "deny extended set of warnings" so we got code committed to git main which hit `unused_imports`. Clippy upstream docs recommend the `RUSTFLAGS = -Dwarnings` approach in e.g. https://doc.rust-lang.org/clippy/continuous_integration/github_actions.html but again I think this is a problem because it can break with updated Rust/clippy versions (unless you pin on those, but that becomes a pain in and of itself). The problem also with doing `RUSTFLAGS = -Dwarnings` *locally* is it blows out the cargo cache. So here's the solution I came to: We run `cargo clippy -A clippy:all`, and then deny some specific clippy lints *and* the core Rust warnings we want (`unused_imports` type things) at this stage. The advantage is this doesn't blow out the main Cargo cache, and I can easily reproduce locally exactly what CI would gate on. Also while we're here, add `make fix-rust` which is a handy way to use the existing `clippy --fix` to locally fix things like unused imports as well as other machine-applicable things that are in e.g. `clippy::suspicious`. Signed-off-by: Colin Walters <[email protected]>
1 parent 859bf9e commit 5b15d68

File tree

1 file changed

+11
-4
lines changed

1 file changed

+11
-4
lines changed

Makefile

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,15 +59,22 @@ test-bin-archive: all
5959
test-tmt:
6060
cargo xtask test-tmt
6161

62-
# Checks extra rust things (formatting, a few extra rust warnings, and select clippy lints)
62+
# This gates CI by default. Note that for clippy, we gate on
63+
# only the clippy correctness and suspicious lints, plus a select
64+
# set of default rustc warnings.
65+
# We intentionally don't gate on this for local builds in cargo.toml
66+
# because it impedes iteration speed.
67+
CLIPPY_CONFIG = -A clippy::all -D clippy::correctness -D clippy::suspicious -Dunused_imports -Ddead_code
6368
validate-rust:
6469
cargo fmt -- --check -l
65-
cargo check
66-
(cd lib && cargo check --no-default-features)
6770
cargo test --no-run
68-
cargo clippy -- -D clippy::correctness -D clippy::suspicious
71+
(cd lib && cargo check --no-default-features)
72+
cargo clippy -- $(CLIPPY_CONFIG)
6973
env RUSTDOCFLAGS='-D warnings' cargo doc --lib
7074
.PHONY: validate-rust
75+
fix-rust:
76+
cargo clippy --fix --allow-dirty -- $(CLIPPY_CONFIG)
77+
.PHONY: fix-rust
7178

7279
validate: validate-rust
7380
ruff check

0 commit comments

Comments
 (0)