Skip to content

ci(format): pin cppcheck via uvx and make it blocking #28

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sqr00t wants to merge 1 commit into from
Open

ci(format): pin cppcheck via uvx and make it blocking #28

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 27 additions & 20 deletions .github/workflows/format.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,14 @@ name: Format

# Static checks for both the Python and C/C++ sources.
#
# Python (ruff) is blocking. clang-format is now blocking too, but only over
# the sources we own — src/shim.hpp and src/bindings.cpp — because the rest of
# src/ is kept byte-identical to upstream INSP-RH/bw. cppcheck runs over all of
# src/ and is advisory until its version is pinned.
# All three checks block, and all three are version-pinned so that a runner
# image or upstream release cannot turn them red on its own.
#
# Python: ruff, pinned to 0.14.10.
# C/C++: clang-format 22.1.8 over src/shim.hpp and src/bindings.cpp only —
# the rest of src/ is kept byte-identical to upstream INSP-RH/bw.
# cppcheck 2.17.1 over all of src/, since analysis does not modify the
# sources and so does not threaten that byte-identity.
on:
push:
branches: [main]
Expand Down Expand Up @@ -42,21 +46,24 @@ jobs:
# check on a runner-image bump alone. Style lives in .clang-format.
- name: clang-format (dry-run)
run: uvx clang-format@22.1.8 --dry-run --Werror src/shim.hpp src/bindings.cpp
# Both cppcheck steps are advisory AND time-boxed. The apt install has
# wedged for over an hour on three separate runs, and without these
# guards it takes the whole job — including the blocking clang-format
# result above — down with it. clang-format runs first for the same
# reason: the check that gates the merge must not depend on apt.
- name: Install cppcheck
continue-on-error: true
timeout-minutes: 5
run: sudo apt-get update && sudo apt-get install -y --no-install-recommends cppcheck
# Advisory: the apt version is not pinned, so a runner-image bump can
# introduce new diagnostics that are not a regression in our code.
# Passing clean under cppcheck 2.21 locally — promote to blocking once
# the CI version is pinned and installed from something other than apt
# (roadmap task 3.4).
# cppcheck is pinned and blocking, for the same two reasons as
# clang-format above.
#
# Pinned: an unpinned checker fails the build on a version bump alone,
# with no change to our code. It comes from PyPI via uvx rather than apt
# — the apt install wedged for over an hour on three separate runs and
# has no version guarantee either way.
#
# --check-level=exhaustive analyses every branch. Without it cppcheck
# emits an informational normalCheckLevelMaxBranches notice on
# energy_build.cpp, which --error-exitcode counts as a failure. Analysing
# properly is the honest fix; suppressing the notice is not.
- name: cppcheck
continue-on-error: true
timeout-minutes: 5
run: cppcheck --enable=warning,portability --suppress=missingIncludeSystem --error-exitcode=1 src/
run: >
uvx --from cppcheck==1.5.1 cppcheck
--enable=warning,portability
--check-level=exhaustive
--suppress=missingIncludeSystem
--error-exitcode=1
src/
14 changes: 7 additions & 7 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,13 @@ prek run --all-files # everything, before a PR

Three workflows run (see [`docs/runbooks/ci-triage.md`](docs/runbooks/ci-triage.md) for diagnosing a red check).

| Check | Blocking? | Notes |
| ------------------------------------------------------------------------------------------------------------------------------ | ----------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `tests.yml` — `uvx --with tox-uv tox run` on `ubuntu-latest`, `ubuntu-24.04-arm`, `macos-latest`, each across Python 3.10–3.14 | **Yes** | `fail-fast: false`, so you see all three legs. The Linux legs guard the GCC/libstdc++ build path — never "fix" a red Linux leg by removing it |
| `format.yml` job `python` — `ruff check .` and `ruff format --check .`, pinned to 0.14.10 | **Yes** | Same version as `.pre-commit-config.yaml`, so `prek run` locally is equivalent. If you bump one, bump the other |
| `format.yml` job `cpp` — `clang-format` pinned to 22.1.8, over `src/shim.hpp` and `src/bindings.cpp` only | **Yes** | Style is `.clang-format` at the repo root. The three upstream files are deliberately excluded — see [ADR 0010](docs/adr/0010-scope-clang-format-to-owned-sources.md). Run `prek run clang-format --all-files` before pushing; the hook is pinned to the same version |
| `format.yml` job `cpp` — `cppcheck` over all of `src/` | No (step `continue-on-error`) | Advisory only because the apt version is not pinned, so a runner-image bump can add diagnostics that are not a regression in our code. It passes clean today — **open the log** if you touched C++ |
| `publish-codeartifact.yml` | n/a | Only fires on a published GitHub Release |
| Check | Blocking? | Notes |
| ------------------------------------------------------------------------------------------------------------------------------ | --------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
| `tests.yml` — `uvx --with tox-uv tox run` on `ubuntu-latest`, `ubuntu-24.04-arm`, `macos-latest`, each across Python 3.10–3.14 | **Yes** | `fail-fast: false`, so you see all three legs. The Linux legs guard the GCC/libstdc++ build path — never "fix" a red Linux leg by removing it |
| `format.yml` job `python` — `ruff check .` and `ruff format --check .`, pinned to 0.14.10 | **Yes** | Same version as `.pre-commit-config.yaml`, so `prek run` locally is equivalent. If you bump one, bump the other |
| `format.yml` job `cpp` — `clang-format` pinned to 22.1.8, over `src/shim.hpp` and `src/bindings.cpp` only | **Yes** | Style is `.clang-format` at the repo root. The three upstream files are deliberately excluded — see [ADR 0010](docs/adr/0010-scope-clang-format-to-owned-sources.md). Run `prek run clang-format --all-files` before pushing; the hook is pinned to the same version |
| `format.yml` job `cpp` — `cppcheck` pinned to 2.17.1, over all of `src/` | **Yes** | Runs `--check-level=exhaustive`. Reproduce locally with the identical pin: `uvx --from cppcheck==1.5.1 cppcheck --enable=warning,portability --check-level=exhaustive --suppress=missingIncludeSystem --error-exitcode=1 src/`. Scope is all of `src/` — unlike formatting, analysis does not rewrite the upstream files |
| `publish-codeartifact.yml` | n/a | Only fires on a published GitHub Release |

Ruff config lives in `pyproject.toml`: line length 120, `select = ["ANN", "B", "C", "E", "F", "I", "N", "W"]`, Google docstring convention. Tests are exempt from `ANN`, `D100` and `D103`.

Expand Down
8 changes: 5 additions & 3 deletions ONBOARDING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,10 @@ See `examples/reproduce_bw_vignette.py` for a fuller worked example with plots.
`src/shim.hpp` and `src/bindings.cpp` only — style in `.clang-format` at the root.
The three upstream-derived files are deliberately excluded to keep them
byte-identical; see [ADR 0010](docs/adr/0010-scope-clang-format-to-owned-sources.md).
`cppcheck` still covers all of `src/` but stays **advisory** until its version is
pinned too.
`cppcheck` is **blocking** too, pinned to 2.17.1 and run with
`--check-level=exhaustive`. It covers all of `src/`, not just the two owned files —
analysis reads the sources without rewriting them, so it does not threaten the
byte-identity that limits clang-format's scope. Neither tool comes from apt.
- **pre-commit** (`.pre-commit-config.yaml`): ruff-format/check, standard hygiene
hooks, `nbstripout`/`jupytext` pairing, prettier, and `no-commit-to-branch`
for `dev`/`main`. Run `prek run --all-files` before pushing (`prek`, not `pre-commit`).
Expand All @@ -150,7 +152,7 @@ See `examples/reproduce_bw_vignette.py` for a fuller worked example with plots.
| Workflow | Trigger | What it does |
| -------------------------- | ----------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `tests.yml` | push to `main`, PRs | OS matrix (`ubuntu-latest`, `ubuntu-24.04-arm`, `macos-latest`) x Python 3.10-3.14 driven by `tox-uv`. Linux legs guard the GCC/libstdc++ build. |
| `format.yml` | push to `main`, PRs | `ruff` check + format (blocking); `clang-format` over the two owned sources (blocking, pinned 22.1.8); `cppcheck` over all of `src/` (advisory). |
| `format.yml` | push to `main`, PRs | All blocking and all version-pinned: `ruff` 0.14.10 check + format; `clang-format` 22.1.8 over the two owned sources; `cppcheck` 2.17.1 over all of `src/`. |
| `publish-codeartifact.yml` | GitHub Release published; manual dispatch | `uv build` compiles a platform wheel + sdist, authenticates to AWS via **OIDC** (no long-lived keys), mints a short-lived CodeArtifact token, and `uv publish`es to the private CodeArtifact PyPI. Pre-releases are skipped. |

Two things to know before you touch the publish workflow:
Expand Down
4 changes: 2 additions & 2 deletions docs/ROADMAP.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ raises a Python exception with a useful message rather than returning `NaN`.
| 3.1 | Pin the build-time requirements and the C++ standard | `scikit-build-core` and `pybind11` constrained in `[build-system] requires`, and `CMAKE_CXX_STANDARD` set explicitly in `CMakeLists.txt` | Neither build requirement appears in `uv.lock`, so both resolve fresh at every build — the largest reproducibility gap in a compiled package. No standard is specified anywhere, so the effective one is the compiler default, which is precisely the axis along which GCC and Clang already diverged once (`bf87e07`). Needs Phase 2 in place so that a pin that changes numerics is detected rather than assumed benign | 1 | A pybind11 major release breaks the build with no change to any committed file; a runner image upgrade silently changes the C++ standard in use |
| ~~3.2~~ | ~~Decide and record how the upstream sources are exempted from formatting~~ **Done — PR #21** | A checked-in `.clang-format`, and the exemption expressed as an explicit file list in `format.yml` rather than a glob | Was the blocker for 3.3. Decided in favour of the explicit file list over `rust-port.md` item (c3) — pinning the upstream commit per file and diffing in CI — on cost, not merit. Recorded in [ADR 0010](adr/0010-scope-clang-format-to-owned-sources.md), which also states the cost: the list is manual and nothing detects a first-party file left off it | 0.5 — spent | — |
| ~~3.3~~ | ~~Run the clang-format pass over the first-party C++ only, then drop `continue-on-error`~~ **Done — PR #21** | `shim.hpp` and `bindings.cpp` are formatted and `clang-format` blocks on them, pinned to 22.1.8 | Delivered together with 3.2. The pass also unblocked `cppcheck`, which had never run because clang-format failed first — it was hiding two real `uninitMemberVarNoCtor` warnings, fixed in `85a398c` | 1–2 — spent | — |
| 3.4 | Pin the `cppcheck` version and make it blocking | `cppcheck` runs at a fixed version and fails the build on new diagnostics | The remaining half of 3.3. It is advisory today only because the apt version floats, so a runner-image bump could redden `main` with no change to our code. It passes clean now, which is the moment to pin — the longer it stays advisory, the more likely findings accumulate unread again | 0.5 | The job goes back to being one nobody reads, which is precisely how the two `shim.hpp` warnings survived |
| ~~3.4~~ | ~~Pin the `cppcheck` version and make it blocking~~ **Done — PR #28** | `cppcheck` 2.17.1 from PyPI via `uvx`, `--check-level=exhaustive`, blocking, no apt | Completed 3.3. Moved off apt rather than merely pinned: the apt install wedged for over an hour on three separate CI runs, taking the blocking clang-format result down with it. Exhaustive analysis is required because the default level emits an informational notice that `--error-exitcode` treats as failure | 0.5 — spent | — |

**Done looks like:** two independent builds of the same commit resolve the same build dependencies
and compile against the same standard; `format.yml`'s `cpp` job is blocking for `shim.hpp` and
Expand All @@ -91,7 +91,7 @@ fidelity is checked instead.

**Status:** 3.2 and 3.3 landed early, out of phase order, because the `cpp` job's failure was
blocking nothing but hiding `cppcheck` output — see [ADR 0010](adr/0010-scope-clang-format-to-owned-sources.md).
3.1 and 3.4 remain, and 3.1 still depends on Phase 2.
3.1 remains, and still depends on Phase 2.

---

Expand Down
8 changes: 4 additions & 4 deletions docs/adr/0007-multiplatform-ci-and-gcc-compat.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,10 @@ carried two further decisions:
the build on the ported code's pre-existing formatting … until a dedicated clang-format pass
lands". Python ruff stays blocking, pinned to `0.14.10` to match `.pre-commit-config.yaml`.

> **Superseded for clang-format** by [ADR 0010](0010-scope-clang-format-to-owned-sources.md).
> That pass has now landed: clang-format is pinned, scoped to `shim.hpp` and `bindings.cpp`, and
> blocking. `cppcheck` remains advisory. This ADR keeps its number and status; only this
> consequence is out of date.
> **Superseded** by [ADR 0010](0010-scope-clang-format-to-owned-sources.md). Both checks now
> block and both are version-pinned: clang-format 22.1.8 over `shim.hpp` and `bindings.cpp`,
> cppcheck 2.17.1 over all of `src/`, neither installed from apt. This ADR keeps its number and
> status; only this consequence is out of date.

**3. Sequence the two changes separately.** Issue #7 records that the C++ change was "Deferred out of
the matrix-CI work so that C++ source changes are reviewed separately from CI config" — hence PR #8
Expand Down
Loading
Loading