Repository navigation
Replace buggy bash commit-msg hook with Python implementation - #2640
Merged
Merged
Conversation
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
marked this pull request as ready for review
July 16, 2026 19:35
dave3d
approved these changes
Jul 16, 2026
zivy
previously requested changes
Jul 16, 2026
blowekamp
dismissed
zivy’s stale review
July 20, 2026 13:39
Not worth diverging from ITK for this suggestion.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bash
Utilities/Hooks/kw-commit-msghook 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:git config hooks.commit-msg.maxLength) and no leading/trailing whitespaceBUG:/ENH:/etc.), which SimpleITK does not usereturninget_max_length()present in ITK's versionkw-prefix from the hook id/name/filename.pre-commit-config.yamlis updated to invoke it withlanguage: pythoninstead oflanguage: 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.