Skip to content

Local pre-commit hook (scripts/hooks/pre-commit -> make lint) blocks every commit: ~5,100 pre-existing clang-tidy findings across 113 files #1264

Closed

Description

@gsdali

What happened

Running git commit -s on a fresh main checkout (no changes of my own in
the affected files) fails at the scripts/hooks/pre-commit hook, which
runs make -j3 -f Makefile.cbm lint unconditionally before every commit.
lint-tidy (clang-tidy, WarningsAsErrors: '*') fails with 5,116
error:-level findings across 113 files
in src/, internal/cbm/, and
one vendored header pulled in via src/foundation/. None of the affected
files were touched by my change (PR #1222, pass_pkgmap.c /
tests/test_pipeline.c only) -- this reproduces on a clean checkout.

This means the pre-commit hook, as currently configured, blocks every
commit for every contributor who has clang-tidy on PATH (it fails
silent-success only when clang-tidy is absent, since CLANG_TIDY ?= $(shell [ -x "$(LLVM_BIN)/clang-tidy" ] && ... || echo clang-tidy) falls
back to a bare clang-tidy that then errors "command not found" -- which
make -j3 lint may or may not propagate depending on shell, so this can
silently pass for contributors without it installed and silently block
everyone else).

PR #1222's own verification section already flagged the milder version of
this: "Full local make lint (clang-tidy included) surfaces pre-existing
findings in unrelated files (internal/cbm/cbm.c, internal/cbm/extract_calls.c,
etc.) that are present on a clean main checkout too... This matches
_lint.yml's own comment that clang-tidy is 'enforced locally,' not in
CI." This issue is the follow-up: the "local-only, not CI-enforced" gap
between scripts/lint.sh --ci (cppcheck + clang-format + no-suppress,
CI-enforced, passes cleanly) and the full make lint (clang-tidy included,
hook-enforced, does not pass) is large enough that the hook itself is not
usable as shipped.

Repro

git config core.hooksPath scripts/hooks   # per CONTRIBUTING.md
make -j3 -f Makefile.cbm lint-tidy

Environment

  • clang-tidy --version: Homebrew LLVM 22.1.8 (/opt/homebrew/opt/llvm/bin/clang-tidy)
  • macOS arm64, Xcode Command Line Tools SDK (MacOSX26.sdk)
  • .clang-tidy: Checks: -*, bugprone-*, cert-*, ... readability-*, clang-analyzer-* with a short disabled-list, WarningsAsErrors: '*', HeaderFilterRegex: '^(src|tests)/.*\.h$'

Evidence

Total: 5,116 error: lines across 113 distinct project files. Breakdown by check (top entries):

Check Count
readability-magic-numbers 3,932
misc-include-cleaner 380
readability-function-cognitive-complexity 270
readability-braces-around-statements 179
bugprone-implicit-widening-of-multiplication-result 58
readability-math-missing-parentheses 54
readability-avoid-nested-conditional-operator 32
readability-uppercase-literal-suffix 23
cert-err33-c 19
misc-no-recursion 15
readability-function-size 13
bugprone-branch-clone 12
misc-use-internal-linkage 11
clang-analyzer-optin.core.EnumCastOutOfRange 11

Representative examples (all pre-existing, none touched by PR #1222):

internal/cbm/cbm.c:22:1: error: included header mimalloc.h is not used directly [misc-include-cleaner,-warnings-as-errors]
internal/cbm/cbm.c:479:12: error: function 'count_params_from_signature' has cognitive complexity of 29 (threshold 25) [readability-function-cognitive-complexity,-warnings-as-errors]
internal/cbm/cbm.c:604:12: error: Potential leak of memory pointed to by 'key' [clang-analyzer-unix.Malloc,-warnings-as-errors]
src/ui/httpd.h:54:17: error: 16 is a magic number; consider replacing it with a named constant [readability-magic-numbers,-warnings-as-errors]
src/pipeline/pass_lsp_cross.h:154:6: error: function 'cbm_pxc_run_one' has a definition with different parameter names [readability-inconsistent-declaration-parameter-name,-warnings-as-errors]

readability-magic-numbers alone (3,932 of 5,116) suggests either a
.clang-tidy config drift (this repo's own SKIP_ONE/PAIR_LEN-style
named-constant convention, visible throughout pass_pkgmap.c, was clearly
meant to satisfy this check -- but it's still firing thousands of times
elsewhere) or a version-driven behavior change in how the check treats
struct field array sizes (e.g. char method[16]; in httpd.h) versus
whatever clang-tidy version this hook was last verified against.

Suggested remediation (any of)

  1. Pin/document a known-good clang-tidy version the config was authored
    against, if this is a version-drift issue -- possibly resolves most of
    the 3,932 magic-number findings at once.
  2. Scope the pre-commit hook's lint-tidy invocation to staged/changed
    files only (a git diff --cached --name-only filter), matching how a
    PR's own diff is what's actually being reviewed -- rather than
    re-linting the entire tree on every commit.
  3. If the intent really is "every commit must leave the whole tree
    clang-tidy-clean," that's a large, separate cleanup effort (113 files)
    that should probably be tracked and paid down deliberately rather than
    discovered by contributors as a blocked git commit.
  4. At minimum, note in CONTRIBUTING.md that the hook is currently
    non-functional so contributors know --no-verify is expected until
    this is resolved, rather than each rediscovering 5,000+ unrelated
    errors per commit.

Context

Found while addressing review feedback on PR #1222 (SwiftPM
Package.swift manifest resolution, item 1 of #551) -- my actual changed
file (src/pipeline/pass_pkgmap.c) has zero clang-tidy findings on the
lines I touched, confirmed by running clang-tidy scoped to just that file;
the hook only blocks because of these ~5,100 pre-existing findings
elsewhere.

Activity

  1. added
    bugSomething isn't working
    priority/normalStandard review queue; useful PR with ordinary maintainer urgency.
    on Jul 28, 2026
  2. added this to the 0.9.1-rc milestone on Jul 28, 2026
  3. DeusData commented on Jul 28, 2026

    @DeusData
    Owner

    Thanks for measuring the pre-existing findings and tying the report to the contributor workflow.

    Current main's hook still invokes the repository-wide lint target rather than deriving a staged-file set, so unrelated historical findings can block a focused commit. PR #1310 is linked and queued for review; triage is not pre-approving its exact staged-file implementation.

    I have routed this contributor-workflow bug to 0.9.1-rc at priority/normal. Thank you for checking that the file you actually changed was clean.

  4. added a commit that references this issue on Aug 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingpriority/normalStandard review queue; useful PR with ordinary maintainer urgency.ux/behaviorDisplay bugs, docs, adoption UX

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions