Skip to content

Replace buggy bash commit-msg hook with Python implementation - #2640

Merged
blowekamp merged 1 commit into
SimpleITK:mainfrom
blowekamp:fix/kw-commit-msg-hook
Jul 20, 2026
Merged

blowekamp merged 1 commit into
SimpleITK:mainfrom
blowekamp:fix/kw-commit-msg-hook

Conversation

@blowekamp

Copy link
Copy Markdown
Member

The bash Utilities/Hooks/kw-commit-msg hook has had a regression since 2024-03-19 (commit 7994b76, "Update KW hooks. Remove gerrit and hook chaining"). That commit removed Gerrit review support and, as a side effect, also deleted the only state transition (else: state=rest) that allowed a commit message body after the mandatory blank second line. Since then, the hook has rejected any commit message with real body content beyond that blank line — every line after it gets re-validated by the "must be empty" check forever.

This explains why so many historical commits with proper multi-paragraph bodies exist in this repo's history: they were made via GitHub's server-side squash/merge (which never runs local hooks), with --no-verify, or in checkouts where pre-commit wasn't installed — never through this hook actually passing.

Fix

Replace the bash script with Utilities/Hooks/commit-msg.py, adapted from ITK's own Python commit-message hook implementation:

  • Validates subject line length (8-78 chars, configurable via git config hooks.commit-msg.maxLength) and no leading/trailing whitespace
  • Requires a blank second line
  • Allows a free-form body afterward (the actual fix)
  • Drops ITK's mandatory subject-line prefix convention (BUG:/ENH:/etc.), which SimpleITK does not use
  • Fixes a missing return in get_max_length() present in ITK's version
  • Drops the kw- prefix from the hook id/name/filename

.pre-commit-config.yaml is updated to invoke it with language: python instead of language: system.

Testing

Manually verified against sample commit messages (multi-paragraph body, subject-only, too-short subject, too-long subject, missing blank second line) — all pass/fail as expected. This commit's own message is a multi-paragraph body, validated by the new hook.

The bash Utilities/Hooks/kw-commit-msg script has had a regression
since 2024-03-19 (commit 7994b76): removing Gerrit support
accidentally also deleted the state transition that allowed a commit
message body after the mandatory blank second line, so any commit
message with real body content beyond that blank line was rejected.

Replace it with Utilities/Hooks/commit-msg.py, adapted from ITK's
Python commit-message hook, dropping ITK's mandatory subject-line
prefix convention (BUG:/ENH:/etc.) which SimpleITK does not use,
fixing a missing return in get_max_length(), and dropping the 'kw-'
prefix from the hook id/name/filename.

Update .pre-commit-config.yaml to invoke the new hook with
language: python instead of language: system.
@blowekamp
blowekamp marked this pull request as ready for review July 16, 2026 19:35
@blowekamp
blowekamp requested review from dave3d and zivy July 16, 2026 19:50
zivy
zivy previously requested changes Jul 16, 2026
Comment thread Utilities/Hooks/commit-msg.py
@blowekamp
blowekamp requested a review from zivy July 20, 2026 13:04
@blowekamp
blowekamp dismissed zivy’s stale review July 20, 2026 13:39

Not worth diverging from ITK for this suggestion.

@blowekamp
blowekamp merged commit 75e4ba5 into SimpleITK:main Jul 20, 2026
10 checks passed
@blowekamp
blowekamp deleted the fix/kw-commit-msg-hook branch September 9, 2026 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants