Skip to content

Commit 3318f29

Browse files
committed
Tooling: Stop clippy from forcing a crate rebuild every run
- clippy `touch`ed `src/lib.rs` each run to force a re-lint, but that mtime bump invalidated `cmdr_lib` for the next debug cargo invocation sharing `target/` - a measured ~22s spurious rebuild in rust-tests / bindings-fresh / integration, and ~30s wasted in clippy itself. - Removed it: with `-D warnings` a lint is a compile error, so warnings fail the build (not cached) and are re-surfaced every run until fixed - verified that warm re-runs of an injected warning all caught it, and clean stays clean. clippy drops ~32s -> ~1-2s warm; the other Rust checks keep a warm `target/`. - The `--fix` failure branch keeps its touch (different transition; local-only, on an already-failing clippy).
1 parent 3c00d73 commit 3318f29

2 files changed

Lines changed: 36 additions & 3 deletions

File tree

docs/notes/check-cpu-contention.md

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,4 +82,26 @@ over-budget check runs alone). Net effect: wall-clock stays bounded by the criti
8282
`--include-slow`, `clippy`-cold for the default suite) while peak oversubscription drops from ~2-3× to ~1×.
8383
Fast/unmeasured checks default to 1 and can be recalibrated later if the fast lane ever shows contention.
8484

85+
## Rust group: build-caching findings
86+
87+
The four heavy normal-suite Rust checks (clippy, bindings-fresh, rust-tests, rust-integration-tests) share one
88+
`target/`, so cache invalidation in one shows up as a rebuild in the others. Findings from a warm-tree measurement
89+
(`cargo nextest run --no-run` = build-only):
90+
91+
- **clippy used to `touch src/lib.rs` every run** to force a re-lint. That mtime bump invalidated `cmdr_lib` for the
92+
next debug cargo invocation: a warm test build is ~1-2 s, but **24 s right after the touch** (recompiles `cmdr` + the
93+
test binary). So `rust-tests`, `bindings-fresh`, and `integration` each ate a ~22 s spurious rebuild, and clippy paid
94+
it too. **Removed the touch** — with `-D warnings` a lint becomes a compile error, so warnings fail the build (not
95+
cached) and are re-surfaced on every run until fixed (verified: warm re-runs of an injected `needless_return` all
96+
caught it; clean stays clean). clippy dropped ~32 s → ~1-2 s warm, and the others stopped rebuilding. The `--fix`
97+
failure branch keeps its touch (different transition — `--fix` succeeds with unfixable warnings; only runs locally on
98+
an already-failing clippy, so it never poisons the shared warm cache).
99+
- **`integration-tests` builds in `--release`** — a separate profile from `rust-tests`' debug, so they share no
100+
artifacts and integration always pays a full release compile. Open question: if the SMB tests don't truly need `-O`,
101+
switching to debug would let them reuse the test build. Left as-is pending a check that the timing-sensitive SMB tests
102+
still pass in debug.
103+
- **`bindings-fresh` is content-hash cached** (not mtime) and runs the bindings export test in **debug** — the same
104+
build as `rust-tests`, so they share artifacts. On a warm tree with no `src-tauri` change it should return `<100 ms`
105+
("cached"); a non-cached run means the marker in `target/` didn't persist (e.g. a `cargo clean`).
106+
85107
Render the graph with weights + lanes: `./scripts/check.sh --graph` (also `--graph-format mermaid|dot`).

scripts/check/checks/desktop-rust-clippy.go

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,14 @@ func RunClippy(ctx *CheckContext) (CheckResult, error) {
2020
return CheckResult{}, fmt.Errorf("failed to prepare llama-server binaries\n%s", indentOutput(output))
2121
}
2222

23-
// Touch lib.rs to force clippy to re-lint (otherwise cached builds skip linting)
24-
libPath := filepath.Join(rustDir, "src", "lib.rs")
25-
_ = exec.Command("touch", libPath).Run()
23+
// No source touch here: clippy runs incrementally. With `-D warnings`, a
24+
// warning becomes a compile error, so a warning-laden build FAILS and cargo
25+
// does NOT cache it — it's re-surfaced on every run until fixed (verified:
26+
// warm re-runs of a warning all caught it). Touching lib.rs to force a
27+
// re-lint of unchanged-clean code only wasted ~22s rebuilding `cmdr_lib`
28+
// here AND, because the touch bumped a shared source mtime, forced the same
29+
// rebuild in rust-tests / bindings-fresh / integration (they share
30+
// `target/`). See docs/notes/check-cpu-contention.md.
2631

2732
// Run the enforcing check first. On the happy path (no warnings) this is
2833
// the only build pass we do. --fix is reserved for the failure branch
@@ -41,6 +46,12 @@ func RunClippy(ctx *CheckContext) (CheckResult, error) {
4146
fixCmd.Dir = rustDir
4247
_, _ = RunCommand(fixCmd, true)
4348

49+
// Force a re-lint for the re-check below: `--fix` runs without `-D`, so it
50+
// succeeds even with unfixable warnings and caches a clean-with-warnings
51+
// build; without this touch the `-D` re-check could reuse it and miss
52+
// them. Only reached locally on an already-failing clippy, so it never
53+
// touches the warm-path cache the other Rust checks share.
54+
libPath := filepath.Join(rustDir, "src", "lib.rs")
4455
_ = exec.Command("touch", libPath).Run()
4556
cmd = exec.Command("cargo", "clippy", "--all-targets", "--", "-D", "warnings")
4657
cmd.Dir = rustDir

0 commit comments

Comments
 (0)