fix(hooks): gate --allow-unknown-hook-name on git 2.54+#728
fix(hooks): gate --allow-unknown-hook-name on git 2.54+#728aviator-app[bot] merged 1 commit intomasterfrom
Conversation
The flag was introduced in git 2.54 (not earlier), so only include it when the detected version is >= 2.54. Older gits reject unrecognized flags, so we must not pass it there.
Current Aviator status
This PR was merged using Aviator.
See the real-time status of this PR on the
Aviator webapp.
Use the Aviator Chrome Extension
to see the status of your PR within GitHub.
|
✅ FlexReview StatusCommon Owner:
Review SLO: |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the Git version check logic for the --allow-unknown-hook-name flag, changing the threshold from version 2.44 to 2.54. Review feedback suggests that 2.54 is a typo for 2.45, which is the version where the flag was actually introduced; using 2.54 would cause the flag to be incorrectly omitted on versions 2.45 through 2.53, potentially leading to hook execution failures.
| // 2.54+, which introduced it and simultaneously began rejecting non-native | ||
| // hook names without it. Older gits don't recognize the flag. |
There was a problem hiding this comment.
The version 2.54 appears to be a typo for 2.45. Git 2.44 is the current stable release, and the --allow-unknown-hook-name flag was introduced in Git 2.45. Using 2.54 will cause the flag to be omitted on versions 2.45 through 2.53, which will likely lead to hook execution failures on those versions due to strict hook name validation.
| // 2.54+, which introduced it and simultaneously began rejecting non-native | |
| // hook names without it. Older gits don't recognize the flag. | |
| // 2.45+, which introduced it and simultaneously began rejecting non-native | |
| // hook names without it. Older Git versions don't recognize the flag. |
| return true | ||
| } | ||
| return major > 2 || (major == 2 && minor >= 44) | ||
| return major > 2 || (major == 2 && minor >= 54) |
There was a problem hiding this comment.
As noted above, the minor version threshold should be 45 instead of 54 to correctly support Git 2.45+.
| return major > 2 || (major == 2 && minor >= 54) | |
| return major > 2 || (major == 2 && minor >= 45) |
2 participants
Summary
Follow-up to #727. That PR gated
--allow-unknown-hook-nameat git 2.44 based on incorrect version information. The flag was actually introduced in git 2.54 alongside the stricter hook-name validation, so passing it on 2.44–2.53 would be rejected as an unknown flag.Change
internal/git/version.go: bump the threshold insupportsAllowUnknownHookNamefrom>= 2.44to>= 2.54, and update the surrounding comments.Behavior now:
< 2.54— flag omitted (no unknown-flag errors on older git).>= 2.54— flag included; required to allow non-native hook names likepre-av-pr/pre-av-sync.If version detection fails, we still include the flag so modern git keeps working.
Test plan
go build ./...go tool golangci-lint run ./internal/git/... ./cmd/av/...go test --vet=all ./internal/git/... ./cmd/av/...av syncnow invokes thepre-av-synchook instead of failing withunknown hook event.