fix: use pre-push stdin for push file detection#1368
fix: use pre-push stdin for push file detection#1368mrexox merged 5 commits intoevilmartians:masterfrom
Conversation
mrexox
left a comment
There was a problem hiding this comment.
I see this PR provides two things:
- Fixes issue for SHA256 repos
- Adds custom PushFiles
I don't see why you're passing PushFiles and usePushFiles when the idea is to get the push files on pre-push hook in the command/build.go. Could you please explain why this approach does not work?
|
I went with The reason for both fields is that a simple slice is ambiguous. An empty slice could mean "the pushed file set is empty," or it could mean "no override was provided." I'm happy to clean up the plumbing if this feels too explicit. A few ideas:
I ain't married with it, tho. Just a approach. Let me know if you have any approach on it or if my approach is wrong in anyway. |
|
I took a swing at a more minimal approach, and honestly, it’s much cleaner. You can check it out here: [fix: simplify pre-push sha256 fallback](5892b2b). I realized I was probably over-engineering things with all that return r.FindExistingFiles(append(cmdPushFilesTreeBase, "HEAD"), "")This keeps the fix focused on the actual bug and saves us from wrestling with I ran the usual |
| reVersion = regexp.MustCompile(`\d+\.\d+\.(\d+|\w+)`) | ||
| cmdPushFilesBase = []string{"git", "diff", "--name-only", "HEAD", "@{push}"} | ||
| cmdPushFilesHead = []string{"git", "diff", "--name-only", "HEAD"} | ||
| cmdPushFilesTreeBase = []string{"git", "ls-tree", "-r", "--name-only"} |
|
Awesome! Thank you for cleaning up the PR. I thin I can merge it and address my comment myself |
Closes #1367
Context
pre-pushchanged-file detection fails on the initial push in repositories usingsha256object format.Before this change, Lefthook tried to infer
{push_files}fromHEAD @{push}and, when no upstream existed yet, fell back to a hardcoded SHA-1 empty-tree object:That object does not exist in
sha256repositories, so Lefthook failed before running configuredpre-pushcommands.I validated this both with automated coverage and against a real empty
sha256GitLab repository:Real-world comparison:
issue-1367branch reproduces the failure:fixes-1367branch succeeds against the same empty remote:Changes
pre-pushstdin ref updates as the source of truth for{push_files}when available.remote oidis all zeroes, resolve pushed files with:pre-pushstdin, but replace the hardcoded SHA-1 empty-tree fallback with:sha256repository performing its first push.Validation performed:
mise x go@1.26.0 -- make testpre_push_sha256_initialsha256GitLab remote using binaries built from:issue-1367fixes-1367