Run make prepush's checks concurrently - #2
Draft
ppannuto-claude wants to merge 3 commits into
Draft
Conversation
ppannuto-claude
force-pushed
the
prepush-parallel-checks
branch
4 times, most recently
from
August 27, 2026 23:11
3944e3f to
0558438
Compare
2 tasks
ppannuto-claude
force-pushed
the
prepush-parallel-checks
branch
5 times, most recently
from
August 28, 2026 07:12
9bf8dd3 to
4416e71
Compare
…et-spec qemu_i486_q35's target is a bare JSON filename, not a path, which cargo only resolves as a custom target spec (rather than requiring a builtin triple name) via the unstable json-target-spec mechanism. Previously this was passed as a `-Zjson-target-spec` CLI flag baked into the board's own Makefile (`CARGO = cargo -Zjson-target-spec`), the only board doing so. Set it instead via `[unstable] json-target-spec = true` in the board's own .cargo/config.toml -- the same persistent-config mechanism ../cargo/riscv_flags.toml already uses for the RISC-V boards' JSON specs (those are referenced by relative path instead, so didn't need it spelled out explicitly). That makes a plain `cargo clippy`/`cargo check`/`cargo build` all work with no special-casing in the board Makefile, so the now-redundant CARGO override is dropped. Also removed a stale `[env] RUST_TARGET_PATH` + TODO comment that predated the current include-based config layout. Verified: a full `make -C boards/qemu_i486_q35` build still produces a working kernel binary with the CARGO override removed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
ppannuto-claude
force-pushed
the
prepush-parallel-checks
branch
2 times, most recently
from
August 28, 2026 16:52
225cbb9 to
5b7e235
Compare
The hand-picked board list (nrf52840dk, raspberry_pi_pico, hifive1, qemu_i486_q35) silently missed arch/* crates with no board maintaining the list ever noticed: cortexm7, cortexm33, and rv64i had no clippy coverage with a real target. tools/build/list_arch_boards.sh picks one board per arch/* crate, so the set self-updates as boards and archs are added or removed instead of drifting stale. It iterates tools/build/list_archs.sh's small, already-existing arch list on the outside and boards on the inside, stopping at the first (alphabetically, since boards are sorted first) match per arch -- rather than scanning every board regardless of whether a match was already found. Every chip that depends on an arch crate does so directly (one hop; verified against every chips/*/Cargo.toml), and a few board crates also depend on an arch crate directly alongside their chip (e.g. hail, the apollo3 boards) -- so for each candidate board, check both its own Cargo.toml and its chip's for an arch/ path dependency. An arch crate with no board using it directly is only an error if no *other* arch crate depends on it either (checked with one more grep over arch/*/Cargo.toml): e.g. cortex-v7m, cortex-m, cortex-m0, and riscv all have zero boards depending on them directly today, but each is a sibling dependency of some other arch crate that a board does use directly (cortex-m4f -> cortex-v7m -> cortex-m, rv32i -> riscv, etc.), so building that board compiles -- and so checks -- them too as a side effect, with no separate pick needed. cortex-m3 is currently the only arch crate genuinely unreachable either way; the script correctly hard-fails on it (verified: `make ci-job-clippy` now fails specifically and only on cortex-m3, everything else still resolves to the same 7 boards as before). This should self-resolve once a board using cortex-m3 lands. Results are collected into a variable and only printed after the full loop succeeds, rather than printed as each arch resolves: a failure partway through then leaves stdout completely empty instead of an incomplete board list, which matters because a `$(shell ...)`-based caller (as opposed to a direct invocation checked with `|| exit 1`) can only observe empty-vs-nonempty output, not the exit status. An earlier version used `cargo metadata` to compute each board's whole transitive dependency graph, which turned out to be solving a problem that doesn't exist here -- the codebase just doesn't have deep chip->chip->arch chains that would need it. A cold-context review caught a real bug in the regex: `path = "..."` requires exact spacing around `=`, but two chips (psoc62xa, rp2040) write `path="..."` with none, so their arch dependency silently didn't match. Harmless today only by coincidence -- both boards using those chips also happen to redeclare the same arch crate directly, so the board-level lookup covered for it -- but a future cleanup removing that "redundant" direct dependency would silently drop cortex-m0p from clippy coverage with nothing to flag it. Loosened the regex to tolerate any whitespace around `=`. Verified: `make prepush` passes except for ci-job-clippy, which fails specifically and only on the cortex-m3 gap described above; output for every other arch is identical to the earlier board-first version's. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
`prepush` ran format-check, clippy, syntax-check, and licensecheck back-to-back. Each already builds into an isolated cargo state (a distinct target triple, or for licensecheck an entirely separate workspace/target dir), verified empirically: alternating `cargo clippy` and `cargo check --config deny_warnings.toml` on the same workspace does not invalidate each other's incremental cache on this toolchain, and concurrent cargo invocations against the same target directory complete correctly. So there's no ordering-based fingerprint thrashing to fix, but there's nothing stopping these jobs from running in parallel either. Split ci-job-clippy's per-board `cargo clippy` invocations (workspace plus one board per `arch/*` crate, per the prior commit) into separate phony sub-targets, and have prepush re-invoke make so its scheduler fans all of these (and ci-job-syntax/licensecheck) across available cores, plus `--output-sync=target` so each job's output still prints as one block instead of interleaving. `make ci-job-clippy` and `make prepush` keep their existing behavior for other callers (e.g. ci-runner-github-clippy); this only changes how the work is scheduled. The sub-target set is generated from CLIPPY_ARCH_BOARDS (the previous commit's dynamic board list) via `$(eval)`, so it stays correct as that list changes instead of hand-tracking a fixed board count. `$(shell)` doesn't fail the build on a nonzero exit, so a failure in list_arch_boards.sh here would silently produce an empty CLIPPY_ARCH_BOARDS and generate zero board-specific clippy rules -- guarded against in ci-job-clippy's own recipe, not a top-level `$(error)`: that assignment runs on every `make` invocation regardless of target (Make expands all rule headers before deciding what to build), so a parse-time error there would take down unrelated targets like `make clean` too. ci-job-syntax now calls `cargo check --config <path>` directly instead of through `$(MAKE) allcheck`: a recursive $(MAKE) inside a synced target's recipe escapes --output-sync's buffering (the child make manages its own output independently), so under a parallel `prepush` the check's compile output could land in the middle of another job's printed block. Reproduced directly; this was the only one of prepush's three parallel jobs using that pattern. The recursive `make` only defaults to `-j$(nproc)` when the outer invocation didn't already set one. GNU Make submake `-j` always overrides (not merges with) a parent's -- verified with a synthetic nested-make test: an unconditional `-j` in the recipe ran all jobs at once even under `make -j2 prepush`, silently ignoring the user's own limit and printing "warning: -jN forced in submake: resetting jobserver mode". A MAKEFLAGS-based guard (`$(filter -j% --jobserver%,$(MAKEFLAGS))`) now skips forcing our own -j whenever the outer invocation already set one, so `make -j2 prepush` really does run at 2-way concurrency; a plain `make prepush` still defaults to parallel. `--output-sync=target` itself requires GNU Make >= 4.0 (2013) -- it's the only 4.0+ feature anywhere in this Makefile, checked directly. An older make (e.g. macOS's bundled 3.81 -- Tock's Getting_Started.md macOS setup never mentions installing a newer one, implying reliance on the system make, and Apple has shipped 3.81 indefinitely since Make moved to GPLv3 at 3.82) doesn't degrade gracefully on an unrecognized flag: it's a hard parse-time error before any recipe runs, breaking `make prepush` entirely. Gated it behind `$(MAKE_VERSION)`'s major version using make-function primitives old enough to be safe on 3.81 itself. One real serialization point does exist: these jobs share one workspace `target/` directory (separate `<triple>/` subdirectories, not separate target dirs). `-Zbuild-std` (every board rebuilds `core`/ `compiler_builtins` from source, see boards/cargo/unstable_flags.toml) takes what appears to be an exclusive lock on the whole directory while building an uncached triple's sysroot. Confirmed with timestamped concurrent runs on a cold cache: a board needing build-std was blocked on "file lock on build directory" for the *entire* duration of any other concurrent cargo invocation against the same target dir, resuming within ~0.1s of the other one finishing -- both against the root workspace check/clippy and against another board's clippy. This only costs anything on a cold cache (fresh checkout or after `make clean`); once every triple's build-std output is cached, which is the normal case for repeated `make prepush` runs, the lock is never taken again and all jobs run fully concurrently. Also folded in: an `nproc` -> `sysctl -n hw.ncpu` -> `4` fallback chain (stock macOS has no `nproc`, which was silently degrading `-j$(shell nproc)` to unbounded `-j`), and running format-check synchronously before the parallel batch so a trivial `cargo fmt` miss still fails in ~4s instead of waiting behind the full multi-minute clippy/check batch. Verified via a forced worst-case invalidation (touching kernel/src/lib.rs, which invalidates every variant): the parallel run completes correctly and faster than an equivalent serial run, a deliberately broken build now fails in ~4s via the synchronous format-check step, a cold-cache `make -j2 prepush` run completes cleanly with no submake-override warnings, and a full cold `make prepush` run passes with all of the above in place. Also verified directly: `make list` (and other targets unrelated to clippy) still succeeds when list_arch_boards.sh is broken, while `make ci-job-clippy` correctly fails with the intended error message; and `make -n prepush` does show the real recursive-make work. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
ppannuto-claude
force-pushed
the
prepush-parallel-checks
branch
from
August 28, 2026 18:16
5b7e235 to
5957391
Compare
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
1 participant
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.
Pull Request Overview
make prepushran format-check, clippy (workspace + one board per arch), syntax-check, and licensecheck strictly back-to-back. This parallelizes the independent parts, without dropping or narrowing any check.cargo clippyandcargo check --config deny_warnings.tomldon't invalidate each other's incremental cache once warm (tested directly) — no fingerprint-thrashing to fix by reordering.ci-job-clippy's per-boardcargo clippyinvocations into separate.PHONYsub-targets, generated via$(eval)from Make ci-job-clippy's board selection cover all archs, dynamically #3'sCLIPPY_ARCH_BOARDSlist (now sourced fromtools/build/list_arch_boards.sh, Make ci-job-clippy's board selection cover all archs, dynamically #3's simplified bash version) — Make can only parallelize separate targets, not recipe lines within one target, and the sub-target set needs to track that list dynamically rather than assuming a fixed board count.$(shell ...)doesn't fail the build on a nonzero exit, soCLIPPY_ARCH_BOARDScoming back empty (the board-selection script failing, or some future change leaving it with nothing to say) would otherwise silently generate zero board-specific clippy rules instead of erroring — guarded against inci-job-clippy's own recipe (not a top-level$(error): that assignment runs on everymakeinvocation regardless of target, so a parse-time error there took down unrelated targets likemake cleantoo — a second cold review caught this in an earlier version of this fix). No board's per-board clippy invocation needs-Zjson-target-specanymore either, since Make ci-job-clippy's board selection cover all archs, dynamically #3 fixed that at the source (qemu_i486_q35's own.cargo/config.toml) instead of passing the unstable flag to every board.ci-job-syntaxnow callscargo check --config <path>directly instead of through$(MAKE) allcheck— the same review found a recursive$(MAKE)inside a synced target's recipe escapes--output-sync's buffering (the child make manages its own output independently), so under a parallelprepushthe check's compile output could land mid-block of another job's output. Reproduced directly; this was the only one of prepush's three parallel jobs using that pattern.prepushrunsformat-checksynchronously first (cheap, fails fast — ~4s instead of waiting behind a multi-minute clippy run on a trivialcargo fmtmiss), then re-invokesmakewith-j/--output-sync=targetfor the remaining 3 (which fans out to clippy's per-board sub-jobs too).-jonly defaults to-j$(nproc)(with asysctl/4fallback, since macOS lacksnproc) when the user didn't already pass one — a submake's own-jalways overrides, not merges with, a parent's (verified with a synthetic nested-make test), somake -j2 prepushnow genuinely runs at 2-way concurrency.--output-sync=targetrequires make >= 4.0, gated behind$(MAKE_VERSION)— macOS's bundled make (3.81) hard-fails on an unrecognized flag rather than degrading gracefully (confirmed directly), and Tock'sGetting_Started.mdmacOS setup never installs a newer one.Depends on #3 — this branch is stacked on top of it, since the
per-board sub-targets here are generated from #3's dynamic board list
rather than a fixed set. Rebase this PR onto master once #3 lands.
Known limitation
These jobs share one workspace
target/dir (separate<triple>/subdirectories, not separate target dirs).-Zbuild-std(every board rebuildscore/compiler_builtinsfrom source) takes what looks like an exclusive lock on the whole dir while building an uncached triple's sysroot — confirmed with timestamped concurrent runs: a board needing build-std blocked on "file lock on build directory" for the entire duration of any other concurrent job against the same target dir (both a non-build-std root job and another board's build-std job), resuming within ~0.1s of that job finishing. Only costs anything on a cold cache/aftermake clean; once every triple's build-std output is cached (the normal case), the lock isn't taken again.Testing Strategy
cargo clippy+cargo check --config deny_warnings.tomlrun concurrently without corrupting caches or serializing (37.5s real vs. 59.7s summed).kernel/src/lib.rs, invalidating every variant): parallelprepushcompletes correctly and faster than serial; a deliberately broken build fails with the correct per-target error and nonzero exit; a cold-cachemake -j2 prepushcompletes with no submake-override warnings; a full coldmake prepushpasses with the version-gated--output-sync.$(MAKE_VERSION)gate and-j/nprocfallback dry-run-verified against real make 4.4.1 and simulated old-make/missing-nprocenvironments.make prepush, cold and warm) after rebasing this branch on top of Make ci-job-clippy's board selection cover all archs, dynamically #3's dynamic board list.CLIPPY_ARCH_BOARDSempty-list guard actually fires by temporarily stubbing the board-selection script with one that exits 1.cargo-metadata) version.CLIPPY_ARCH_BOARDSstill resolves to the same 7-board set.make -n prepush"no longer shows the real work" —-ndoes propagate correctly through the recursive$(MAKE)call and the dry-run output is complete (removed that claim below). It also confirmedmake list(and other clippy-unrelated targets) still succeed when the board-selection script is broken, thatmake ci-job-clippycorrectly fails in that case, and that the-j/nproc/MAKE_VERSIONlogic is sound (a synthetic nested-make test showed a real jobserver-bounded-j2run stays at 2-way concurrency, not unlimited).TODO or Help Wanted
One accepted cosmetic trade-off: output completion order (which job's banner prints first) is no longer deterministic — inherent to running independent jobs concurrently, not fixable without giving up parallel-by-default.
ci-help's job listing is also longer now (7 per-board clippy sub-targets instead of 4), which is arguably useful (each is independently runnable, e.g.make ci-job-clippy-arty_e21) rather than pure noise.Checklist
make prepush.PR Contents
Documentation