ci: run cppcheck in lint container, document CI-faithful lint one-liner - #7533
ci: run cppcheck in lint container, document CI-faithful lint one-liner#7533PastaPastaPasta wants to merge 25 commits into
Conversation
WalkthroughAdded matching “Running Linters Exactly Like CI” sections to Updated Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
✅ Final review complete — no blockers (commit 0523bda) |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bf43d43bb7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ``` | ||
|
|
||
| ```bash | ||
| G="$(git rev-parse --path-format=absolute --git-common-dir)"; docker run --rm -v "$PWD":"$PWD" -v "$G":"$G" -w "$PWD" -e BUILD_TARGET=linux64 -e CHECK_DOC=1 -e PULL_REQUEST=true -e COMMIT_RANGE="$(git merge-base develop HEAD)..HEAD" dash-linter bash -c 'git config --global --add safe.directory "*" && ./ci/dash/lint.sh' |
There was a problem hiding this comment.
When the working tree contains non-conflicting uncommitted changes and COMMIT_RANGE includes a scripted-diff: commit, this command invokes test/lint/commit-script-check.sh, which checks out the commit's parent and runs git reset --hard HEAD; that can permanently discard the user's changes from the writable bind mount. Require a clean worktree or add a preflight guard before advertising this as a general local lint command.
Useful? React with 👍 / 👎.
| # One-time setup; rebuild only when ci/lint/ changes. | ||
| docker build -t dash-linter ci/lint |
There was a problem hiding this comment.
For C++ changes checked with this documented container, ci/lint/Dockerfile installs only the tools from ci/lint/04_install.sh and omits cppcheck, so lint-cppcheck-dash.py exits successfully after printing the skip mentioned below. The actual lint job uses contrib/containers/ci/ci-slim.Dockerfile (.github/workflows/build.yml:190-196), which builds cppcheck 2.17.1, meaning this command can pass locally while the supposedly reproduced CI job fails.
Useful? React with 👍 / 👎.
| ``` | ||
|
|
||
| ```bash | ||
| G="$(git rev-parse --path-format=absolute --git-common-dir)"; docker run --rm -v "$PWD":"$PWD" -v "$G":"$G" -w "$PWD" -e BUILD_TARGET=linux64 -e CHECK_DOC=1 -e PULL_REQUEST=true -e COMMIT_RANGE="$(git merge-base develop HEAD)..HEAD" dash-linter bash -c 'git config --global --add safe.directory "*" && ./ci/dash/lint.sh' |
There was a problem hiding this comment.
In a clone or worktree without a local develop branch, the host-side git merge-base develop HEAD fails before Docker starts; if that branch exists but is stale or divergent, the range also differs from CI. The inspected lint workflow fetches the target branch and computes its PR range from origin/develop (.github/workflows/lint.yml:31-49), so the documented command needs an equivalent fetch/ref strategy to reproduce that job reliably.
Useful? React with 👍 / 👎.
bf43d43 to
4de30bb
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4de30bb3a6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| ```bash | ||
| # One-time setup; rebuild only when ci/lint/ changes. | ||
| docker build -t dash-linter ci/lint |
There was a problem hiding this comment.
On native arm64 hosts, this unqualified build selects an arm64 base image, but ci/lint/04_install.sh:44-47 unconditionally installs the linux.x86_64 ShellCheck binary. test/lint/lint-shell.py:20-25 only treats a missing executable as skippable, so an incompatible binary can terminate the advertised lint run with an exec-format error. Either select linux/amd64 explicitly for the build/run or make the ShellCheck download follow the target architecture as ci-slim.Dockerfile does.
Useful? React with 👍 / 👎.
| ``` | ||
|
|
||
| ```bash | ||
| G="$(git rev-parse --path-format=absolute --git-common-dir)"; docker run --rm -v "$PWD":"$PWD" -v "$G":"$G" -w "$PWD" -e BUILD_TARGET=linux64 -e CHECK_DOC=1 -e PULL_REQUEST=true -e COMMIT_RANGE="$(git merge-base develop HEAD)..HEAD" dash-linter bash -c 'git config --global --add safe.directory "*" && ./ci/dash/lint.sh' |
There was a problem hiding this comment.
On native Linux Docker installations, this command runs as root because ci/lint/Dockerfile declares no USER, while test/lint/lint-python.py:18-20 writes its cache into the bind-mounted test/.mypy_cache. A successful lint can therefore leave root-owned cache directories that subsequent host-side lint or cleanup commands cannot update or remove without elevated privileges; pass through the host UID/GID with a writable HOME, or keep generated caches outside the source bind mount.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
AGENTS.md (1)
145-145: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winScope Git trust to the mounted worktree.
safe.directory "*"treats every repository in the container as safe. Scope the exception to the mounted worktree instead.
AGENTS.md#L145-L145: replacesafe.directory "*"withsafe.directory "$PWD".CLAUDE.md#L145-L145: replacesafe.directory "*"withsafe.directory "$PWD".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@AGENTS.md` at line 145, Scope Git’s safe-directory exception to the mounted worktree by replacing the wildcard with "$PWD" in the docker lint command; apply this change in AGENTS.md at lines 145-145 and CLAUDE.md at lines 145-145.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@AGENTS.md`:
- Line 145: Scope Git’s safe-directory exception to the mounted worktree by
replacing the wildcard with "$PWD" in the docker lint command; apply this change
in AGENTS.md at lines 145-145 and CLAUDE.md at lines 145-145.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b205ea4a-712a-4e42-8c0e-9d3f2aae14b9
📒 Files selected for processing (2)
AGENTS.mdCLAUDE.md
4de30bb to
97ad6fb
Compare
PastaPastaPasta
changed the title
|
Addressed all three Codex findings:
🤖 Posted autonomously by Claude on behalf of pasta. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ci/lint/Dockerfile (1)
21-26: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winVerify the Cppcheck archive before building it.
Both
ci/lint/Dockerfileandcontrib/containers/ci/ci-slim.Dockerfiledownloadcppcheck/${CPPCHECK_VERSION}.tar.gzinto a build container and run CMake without integrity or signature validation. Pin the archive to the intended commit and check the digest or signature beforetar, so a moved tag or compromised archive cannot execute build-time code and place a modifiedcppcheckbinary in the lint image.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ci/lint/Dockerfile` around lines 21 - 26, Update the Cppcheck download-and-build flow in the Dockerfile, and apply the same change to the corresponding ci-slim.Dockerfile flow: pin the archive to the intended immutable commit and verify its expected digest or signature before extracting it with tar or running CMake. Keep the existing build steps unchanged after validation succeeds.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@ci/lint/Dockerfile`:
- Around line 21-26: Update the Cppcheck download-and-build flow in the
Dockerfile, and apply the same change to the corresponding ci-slim.Dockerfile
flow: pin the archive to the intended immutable commit and verify its expected
digest or signature before extracting it with tar or running CMake. Keep the
existing build steps unchanged after validation succeeds.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a6b472b8-3f00-4890-991f-c04a6b5209de
📒 Files selected for processing (3)
AGENTS.mdCLAUDE.mdci/lint/Dockerfile
🚧 Files skipped from review as they are similar to previous changes (2)
- AGENTS.md
- CLAUDE.md
|
Prepared the follow-up for the new Codex/CodeRabbit findings, but the active Final two-commit branch, rebased onto current
Validation passed: docs remain identical, To update this PR while preserving the intended two-commit history, the branch owner can fetch |
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Final validation — Codex + Sonnet
The cppcheck builder correctly mirrors the existing CI builder and enables a lint that previously self-skipped in the local image. The documented workflow still needs adjustments for ARM hosts, bind-mount ownership, and placement of its destructive-operation warning, and the commits should be reordered so the documentation is accurate at every point in history. No blocking correctness defects were found.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (completed),gpt-5.6-sol— dash-core-commit-history (completed) - Verifier:
gpt-5.6-sol— final-verifier (fallback) - Sonnet reviewers:
claude-sonnet-5— general (failed),claude-sonnet-5— dash-core-commit-history (completed),claude-sonnet-5— general (failed),claude-sonnet-5— general (completed)
🟡 4 suggestion(s)
1 additional finding(s) omitted (not in diff).
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `AGENTS.md`:
- [SUGGESTION] AGENTS.md:141: Force the CI architecture for the lint image
This build selects the host architecture, while `ci/lint/04_install.sh` unconditionally downloads the x86_64 ShellCheck executable. On a native arm64 host without x86 binary emulation, the image builds successfully but `lint-shell.py` later fails with an exec-format error. Since the CI lint job runs on amd64, pin this workflow to `linux/amd64`, or make the installer select ShellCheck using the target architecture as `ci-slim.Dockerfile` already does. Apply the same correction to the mirrored section in `CLAUDE.md`.
- [SUGGESTION] AGENTS.md:145: Run the bind-mounted lint job as the host user
The lint image has no `USER` directive and this invocation supplies no `--user`, so it writes through the native Linux bind mount as UID 0. The lint suite creates `test/.mypy_cache` and `ci-cache-linux64/cppcheck`, leaving root-owned cache contents that subsequent host-side runs cannot update or remove recursively without elevated privileges. CI also runs its container as root, but its checkout is disposable rather than a contributor's live working tree. Run the local container with the host UID/GID and a writable `HOME`, and apply the same correction to `CLAUDE.md`.
- [SUGGESTION] AGENTS.md:139-155: Put the destructive-operation warning before the copy-pasteable command
With `PULL_REQUEST=true`, `ci/dash/lint.sh` invokes `commit-script-check.sh`. If the selected range contains a `scripted-diff:` commit, that script checks out the parent and executes `git reset --hard` against the live bind-mounted working tree, which can erase dirty tracked changes that survive the checkout. The documentation discloses this only after the command users and agents are expected to copy. Move the commit-or-stash warning above the command or add a clean-working-tree preflight to the command itself. Make the same change in `CLAUDE.md`.
In `<commit:e37b1d5>`:
- [SUGGESTION] <commit:e37b1d5>:1: Place the cppcheck implementation before documentation that depends on it
Commit `e37b1d5` states that the lint image builds cppcheck and reproduces CI with pinned tools, but the Dockerfile does not acquire cppcheck until its child commit `97ad6fb`. Checking out or bisecting to the documentation commit therefore leaves an inaccurate command whose cppcheck lint still self-skips. Reorder the cppcheck implementation before the documentation commit, or squash the two commits so every retained commit is internally accurate.
|
|
||
| ```bash | ||
| # One-time setup; rebuild only when ci/lint/ changes. | ||
| docker build -t dash-linter ci/lint |
There was a problem hiding this comment.
🟡 Suggestion: Force the CI architecture for the lint image
This build selects the host architecture, while ci/lint/04_install.sh unconditionally downloads the x86_64 ShellCheck executable. On a native arm64 host without x86 binary emulation, the image builds successfully but lint-shell.py later fails with an exec-format error. Since the CI lint job runs on amd64, pin this workflow to linux/amd64, or make the installer select ShellCheck using the target architecture as ci-slim.Dockerfile already does. Apply the same correction to the mirrored section in CLAUDE.md.
| docker build -t dash-linter ci/lint | |
| docker build --platform=linux/amd64 -t dash-linter ci/lint |
source: ['claude', 'codex']
There was a problem hiding this comment.
Resolved in this update — Force the CI architecture for the lint image no longer present.
Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.
| ``` | ||
|
|
||
| ```bash | ||
| G="$(git rev-parse --path-format=absolute --git-common-dir)"; docker run --rm -v "$PWD":"$PWD" -v "$G":"$G" -w "$PWD" -e BUILD_TARGET=linux64 -e CHECK_DOC=1 -e PULL_REQUEST=true -e COMMIT_RANGE="$(git merge-base develop HEAD)..HEAD" dash-linter bash -c 'git config --global --add safe.directory "*" && ./ci/dash/lint.sh' |
There was a problem hiding this comment.
🟡 Suggestion: Run the bind-mounted lint job as the host user
The lint image has no USER directive and this invocation supplies no --user, so it writes through the native Linux bind mount as UID 0. The lint suite creates test/.mypy_cache and ci-cache-linux64/cppcheck, leaving root-owned cache contents that subsequent host-side runs cannot update or remove recursively without elevated privileges. CI also runs its container as root, but its checkout is disposable rather than a contributor's live working tree. Run the local container with the host UID/GID and a writable HOME, and apply the same correction to CLAUDE.md.
| G="$(git rev-parse --path-format=absolute --git-common-dir)"; docker run --rm -v "$PWD":"$PWD" -v "$G":"$G" -w "$PWD" -e BUILD_TARGET=linux64 -e CHECK_DOC=1 -e PULL_REQUEST=true -e COMMIT_RANGE="$(git merge-base develop HEAD)..HEAD" dash-linter bash -c 'git config --global --add safe.directory "*" && ./ci/dash/lint.sh' | |
| G="$(git rev-parse --path-format=absolute --git-common-dir)"; docker run --rm --user "$(id -u):$(id -g)" -e HOME=/tmp -v "$PWD":"$PWD" -v "$G":"$G" -w "$PWD" -e BUILD_TARGET=linux64 -e CHECK_DOC=1 -e PULL_REQUEST=true -e COMMIT_RANGE="$(git merge-base develop HEAD)..HEAD" dash-linter bash -c 'git config --global --add safe.directory "*" && ./ci/dash/lint.sh' |
source: ['claude', 'codex']
There was a problem hiding this comment.
Resolved in this update — Run the bind-mounted lint job as the host user no longer present.
Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.
| ```bash | ||
| # One-time setup; rebuild only when ci/lint/ changes. | ||
| docker build -t dash-linter ci/lint | ||
| ``` | ||
|
|
||
| ```bash | ||
| G="$(git rev-parse --path-format=absolute --git-common-dir)"; docker run --rm -v "$PWD":"$PWD" -v "$G":"$G" -w "$PWD" -e BUILD_TARGET=linux64 -e CHECK_DOC=1 -e PULL_REQUEST=true -e COMMIT_RANGE="$(git merge-base develop HEAD)..HEAD" dash-linter bash -c 'git config --global --add safe.directory "*" && ./ci/dash/lint.sh' | ||
| ``` | ||
|
|
||
| Notes: | ||
|
|
||
| - The dual mount (`$PWD` plus the git common dir) makes this work from git | ||
| worktrees as well as normal checkouts; run it from the repo/worktree root. | ||
| - Commit or stash before running. With `PULL_REQUEST=true` the range is fed to | ||
| `test/lint/commit-script-check.sh`, which checks out commits and runs | ||
| `git reset --hard` while replaying `scripted-diff:` commits; through the | ||
| writable bind mount this can discard uncommitted changes. |
There was a problem hiding this comment.
🟡 Suggestion: Put the destructive-operation warning before the copy-pasteable command
With PULL_REQUEST=true, ci/dash/lint.sh invokes commit-script-check.sh. If the selected range contains a scripted-diff: commit, that script checks out the parent and executes git reset --hard against the live bind-mounted working tree, which can erase dirty tracked changes that survive the checkout. The documentation discloses this only after the command users and agents are expected to copy. Move the commit-or-stash warning above the command or add a clean-working-tree preflight to the command itself. Make the same change in CLAUDE.md.
source: ['claude']
The cppcheck linter has been silently analyzing nothing (see next commit), letting several warnings in non-backported files accumulate. Fix the ones that the linter's ALWAYS_ENABLED_WARNINGS patterns force-report: remove unused/dead locals, make single-argument constructors explicit (with an inline suppression for CBLSIdImplicit, whose implicit conversion is intentional), narrow benchmark counters to the scope they are used in, pass CSigBase and BlsCheck constructor arguments by reference/move, and inline-suppress a danglingTempReference false positive on a lifetime-extended range-for temporary.
The linter has been vacuous in two ways. First, without __GNUC__ defined, src/attributes.h hits '#error No known always_inline attribute', which aborts cppcheck's analysis of nearly every translation unit; the resulting preprocessorErrorDirective lines were then dropped by the output filter because they don't point at files from non-backported.txt. Second, even with preprocessing fixed, cppcheck 2.17.1 crashes with an assertion in TokenList::setLang under --check-level=exhaustive, and that crash was explicitly suppressed. Define __GNUC__ so preprocessing succeeds, bump cppcheck to 2.21.0 (which no longer crashes with exhaustive checking) and drop the crash suppression, and treat analysis failures (preprocessorErrorDirective, internal errors) as lint failures regardless of which file they point at so the linter can never silently go vacuous again. Filter out 'note:'/source-context lines, which don't carry the check id that suppressions match on and would leak through when their parent warning is suppressed. Finally, suppress the check ids with pre-existing violations in the tree so the linter can be enforced; these should be burned down and re-enabled over time.
The shadowMember re-enable left dangling parameter names after dashpay#7538/dashpay#7539 dropped those parameters (connman_in / signing_manager), which broke the build. Finish the ProcessVoteAndRelay cleanup by removing the now-unused CConnman plumbing from GovernanceSigner, governance RPC helpers, and node interfaces. Cap cppcheck jobs and treat abnormal cppcheck exits as failures so CI analysis cannot pass vacuous when the child process dies.
std::ranges algorithms and std::accumulate with lambdas lose lock analysis context in Clang -Wthread-safety-analysis when calling functions or accessing members that require mutexes (cs_wallet, cs_coinjoin). Revert these loops to traditional for loops and add lint suppressions for cppcheck.
…ashes std::ranges::copy_if with a lambda closure loses cs_store thread-safety analysis context in Clang -Wthread-safety-analysis when accessing mapObjects (which is GUARDED_BY(cs_store)). Revert to a traditional for loop and add lint suppression.
std::ranges::transform on Span<T> passes non-const element references to lambdas, causing ThreadSanitizer data races during concurrent BLS worker execution. Revert to raw const-reference loops and add lint suppression.
Without --error-exitcode, cppcheck diagnostics keep exit status 0, so any nonzero status means the analysis itself failed (bad arguments, unloadable config, OOM kill, crash) and must fail the lint instead of being ignored when its output does not match FATAL_ERRORS. Add syntaxError to FATAL_ERRORS: it aborts analysis of the affected translation unit, and when attributed to a header outside the non-backported list it was silently discarded by the path filter. This immediately surfaced a real instance: QT_VERSION_CHECK is a function-like macro cppcheck cannot evaluate, aborting analysis of Qt translation units, so define it on the command line. Also match the raw cppcheck severities (style/performance/portability) in the diagnostic filter; the gcc template currently renders them all as 'warning:' but this makes the filter robust to template changes.
The cap to 4 threads slipped into the rebase-repair commit unintentionally; develop runs cppcheck with one job per core and there is no reason to throttle it.
97ad6fb to
04d86ab
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Pushed
Since the claw had no Docker daemon, the scoped 🤖 Posted autonomously by Claude on behalf of pasta. |
Potential PR merge conflictsThis is advisory only. It does not block CI, but it marks PRs that will likely need a rebase depending on merge order. If this PR merges firstThese open PRs will likely need a rebase:
|
The CI lint job runs in the ci-slim container, which builds cppcheck, while the local ci/lint image had none, so lint-cppcheck-dash.py self-skipped locally and could pass while CI failed. Build the same cppcheck version in a builder stage, mirroring ci-slim.Dockerfile, and pin-and-verify the upstream archive's SHA-256 in both mirrored Dockerfiles.
Running test/lint scripts on the host uses local tool versions and the default ci/lint container entrypoint diverges from CI (merge-base against master, extra check-doc/subtree checks). Document a single docker run invocation that reproduces ci/dash/lint.sh with CI-pinned tools and works from git worktrees.
04d86ab to
0523bda
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Restacked on #7535 ( Changes in the restack:
Re-validated end-to-end on the stacked branch from a git worktree (macOS, Docker 29.4.0): digest check passes during build, The base stays 🤖 Posted autonomously by Claude on behalf of pasta. |
|
This pull request has conflicts, please rebase. |
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Final validation — Codex + Sonnet
At head 0523bda, the two PR-owned commits build cppcheck 2.21.0 (SHA-256 verified) in the ci/lint image mirroring ci-slim.Dockerfile, and document a CI-faithful docker lint one-liner in byte-identical AGENTS.md/CLAUDE.md sections. All four prior findings (arm64 platform pin, host-UID/GID execution, warning placement, commit ordering) are independently verified fixed in the current source. No new in-scope issues were found; the Codex commit-history suggestions target the inherited #7535 stack, which the PR description explicitly excludes from this PR's scope. Source: reviewer backends gpt-5.6-sol (codex, general + dash-core-commit-history) and claude-sonnet-5 (general + dash-core-commit-history); final verifier backend claude-sonnet-5.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (completed),gpt-5.6-sol— dash-core-commit-history (completed) - Verifier:
claude-sonnet-5— final-verifier - Sonnet reviewers:
claude-sonnet-5— general (completed),claude-sonnet-5— dash-core-commit-history (completed)
🟡 1 suggestion(s)
1 additional finding(s) omitted (not in diff).
3 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `<commit:e37b1d5>`:
- [SUGGESTION] <commit:e37b1d5>:1: Place the cppcheck implementation before documentation that depends on it
Commit `e37b1d5` states that the lint image builds cppcheck and reproduces CI with pinned tools, but the Dockerfile does not acquire cppcheck until its child commit `97ad6fb`. Checking out or bisecting to the documentation commit therefore leaves an inaccurate command whose cppcheck lint still self-skips. Reorder the cppcheck implementation before the documentation commit, or squash the two commits so every retained commit is internally accurate.
2 participants
Stacked on #7535 — only the last two commits (
ci:anddocs:) belong to this PR; review the rest there. GitHub can't set a cross-fork PR base, so the base staysdevelopuntil #7535 merges, after which this will be rebased to just the two commits.Issue being fixed or feature implemented
Local lint runs frequently disagree with CI, which slows down and confuses both human contributors and coding agents:
test/lint/*scripts on the host uses whatever tool versions happen to be installed locally, while CI pins specific versions of codespell/flake8/mypy/shellcheck etc.test/lint/README.md(baredocker run ... dash-linter) is not CI-faithful either: its default entrypoint merge-bases againstmasterinstead ofdevelop, and runscheck-doc.pyplus git-subtree checks that the CI lint job (ci/dash/lint.sh) has disabled.ci/lintimage had no cppcheck, while CI's lint job runs in theci-slimcontainer which builds cppcheck — solint-cppcheck-dash.pysilently self-skipped locally. This matters more now that fix(scripts): make lint-cppcheck-dash actually report warnings #7535 makes that lint actually report findings.The result is failures locally that CI never reports, and vice versa.
What was done?
Two commits on top of #7535 (ci first so the docs are accurate at every point in history):
ci/lint/Dockerfilevia a builder stage mirroringcontrib/containers/ci/ci-slim.Dockerfile(same pinned version — 2.21.0 after fix(scripts): make lint-cppcheck-dash actually report warnings #7535 — binary + cfg installed the same way), so the local container runs the cppcheck lint just like CI instead of self-skipping. Both mirrored Dockerfiles now pin and verify the SHA-256 of the cppcheck source archive before building it.CLAUDE.mdandAGENTS.md, kept identical per the guides' own convention). Host linters remain the first choice when they behave; the section documents a one-timedocker build --platform linux/amd64 -t dash-linter ci/lintplus a singledocker runone-liner that reproduces the CI lint job:ci/dash/lint.shwith the same environment CI sets (BUILD_TARGET=linux64,CHECK_DOC=1,PULL_REQUEST=true,COMMIT_RANGEfrommerge-base develop). The command pins the CI architecture (--platform linux/amd64, sinceci/lint/04_install.shinstalls x86_64 ShellCheck unconditionally), runs the container as the host UID/GID withHOME=/tmpso bind-mounted caches don't end up root-owned, and scopes git'ssafe.directorytrust to the mounted worktree rather than*. The section also covers the dual mount needed for git worktrees, an up-front commit-or-stash warning placed before the copy-pasteable command (commit-script-check.shhard-resets while replayingscripted-diff:commits), keeping localdevelopcurrent, why the baretest/lint/README.mdflow should not be used to reproduce CI, and theci-cache-*scratch directory the run creates.How Has This Been Tested?
Built the image and ran the final documented one-liner end-to-end from a git worktree on macOS (Apple Silicon, Docker 29.4.0), on this stacked branch: the archive digest check passes during build,
cppcheck --versionreports 2.21.0 inside the image, the run exits 0 with only the known non-fatal codespell output,lint-cppcheck-dash.py— with #7535's fixes active on this stacked branch — analyzes the commit range's 311 source files and reports "no issues found" (populating its--cppcheck-build-dir) instead of printing the skip message, the resulting caches are owned by the host user, and the worktree-scopedsafe.directoryproduces no dubious-ownership errors. Independently re-derived the cppcheck 2.21.0 archive SHA-256 from a fresh download; it matches the pinned digest. VerifiedCLAUDE.mdandAGENTS.mdsections are byte-identical viadiff.Breaking Changes
None.
Checklist: