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

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.

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