Repository navigation
feat(hooks): run the pre-push hook on every push - #85
ChrisLauinger77 wants to merge 2 commits into
Conversation
cheat2001
left a comment
There was a problem hiding this comment.
Read the pre-push part too. Running it in push_negotiation is a nice call, the remote SHAs come from the server and a failing hook aborts before anything is sent. #17 still holds. I checked --to-stdin against git's source, it's missing at v2.39.0 and there at v2.40.0, so the gate is right.
One thing I'd like fixed here. capture_child joins the stdout and stderr readers after git exits, so a hook that starts something in the background without redirecting it keeps the pipe open and the push never finishes. The commit hooks from #84 share it. Joining with a short grace period after exit and then moving on would do it.
I'm also going to try a slow pre-push over HTTPS. The hook runs between the info/refs GET and the receive-pack POST, and I'm not sure libgit2's keep-alive socket survives a two minute hook.
Later: a timed-out hook comes back as a plain error rather than hook_failed, so there's no "Push without hooks" button, and Force push after "Push without hooks" runs the hook again.
|
I merged the reviewed #84 branch into this one (
Until #84 is merged, this PR also shows #84's commits. The pre-push work itself is |
|
#84 is in (squashed as f662f1e), and the background process fix came with it, so that one's done. The push request change looks good too, keeping the branch and the hook choice from the click is the right call. Since #84 got squashed this branch now conflicts with main. Could you rebase it onto main so only the pre-push commits are left? I'll give it a last look in the installed app after that. |
libgit2 never runs hooks, so every push from the app behaved like git push --no-verify. Every push path now runs pre-push first, with the same arguments and ref lines git gives it. - The hook runs in libgit2's push_negotiation callback, after the remote's refs are read and before anything is sent, so it sees the remote's real SHAs and a failing hook sends nothing. - Stdin follows git: up-to-date refs and rejected non-fast-forward updates are left out, deletes read "(delete)", annotated tags send the tag object, and the hook also runs when nothing needs pushing. - Input goes through git hook run --to-stdin on Git 2.40 or newer. On Git 2.36 to 2.39 macOS and Linux run the hook file directly, in its own process group like the git runner; Windows skips it with a warning. - The direct runner retries an ENOEXEC spawn as "/bin/sh <hook> <args>" with the same stdin, environment and working folder, as git's run-command does for a hook without a shebang. - push, push_tag and the new push_specs share one push routine, so a refused tag now fails the push too. - Hook failures from commit, amend and every push path share one dialog and the per-repository failure store, and the retry re-runs the original operation in its own repository. The Push menu and the palette offer "Push without hooks" when the repository has a pre-push hook. Refs cheat2001#82
…push Toolbar and palette pushes passed no branch, so the engine resolved HEAD again on every call. If another branch was checked out while a pre-push hook failed, "Push without hooks" published that other branch without running the hook. Toolbar pushes now build one request at click time with the branch, the remote and the hook choice, and the hook retry re-runs it with the hook skipped. A non-fast-forward rejection keeps the request, so the rejected push dialog's Force push continues it instead of starting fresh, and a "Push without hooks" no longer runs the hook again. The palette passes the branch explicitly too. Refs cheat2001#82
f569c02 to
0af1ddf
Compare
|
Rebased onto
|
What & why
libgit2 never runs hooks, so every push from AngKorGit behaved like
git push --no-verify. This is part 2 of #82:pre-pushnow runs before every push, using the hook runner from #84.In scope
remote::pushandremote::push_tag, plus a generalpush_specs.pushurl, elseurl, afterinsteadOf/pushInsteadOf).<local ref> <local sha> <remote ref> <remote sha>.(delete) <zeros>; an annotated tag sends the tag object's SHA.push_negotiationcallback, which receives the remote's advertised refs and runs before any object or ref update is sent. A failing hook aborts the push there, so nothing reaches the remote.git hook run --to-stdin, which exists since Git 2.40 (absent frombuiltin/hook.cat v2.39.0, present at v2.40.0).proc::hidden./bin/sh <hook> <args>when the exec fails withENOEXEC. The direct runner does the same, keeping arguments, stdin, environment and working folder. (On macOS Rust'sposix_spawnpalready retries throughsh; Linux's does not.)sh.exe, which CI can't exercise.pre-pushhook: the push takes exactly the same path as before, with no extra process.pre-pushhook.push_tagnow goes through the same push routine, so a tag the server refuses fails the push instead of looking like it succeeded.Deferred
prepare-commit-msg,post-commit,post-checkout,post-merge,post-rewrite.post-merge(Start page: group context menu with fetch all and fast-forward-only pull #69), agreed in Run local Git hooks (pre-commit, commit-msg, pre-push) #82 for the next PR:post-mergeruns per repository only where something merged or fast-forwarded, one repository after another, a failure shows as a warning on that repository's row, and Skip hooks applies.group_pull_matches_toolbar_pull_with_active_hooksstill asserts that no post-hook runs until then.pre-pushtoo, including the direct runner and itsshfallback.Tests
tests/git_engine.rs, against a local bare repo:push_tagruns the hookpush_tagcore.hooksPathis respectedsh(unix). This one can only fail without the fix on Linux, since macOS retries throughshanyway; the Ubuntu CI job covers it.ENOEXECtriggers theshretryScreenshots
Taken on demo mode, dark theme; to be attached.
Checklist
pnpm typecheck && pnpm testpasscargo fmt --check && cargo clippy --all-targets -- -D warnings && cargo testpass (if Rust changed). Threesign.rsmodule tests fail locally with a globalcommit.gpgsign=trueand fail onmaintoo; everything passes with a clean HOME.tests/git_engine.rstracked()wrapper where undoableCLAUDE.mdupdated if conventions/architecture changedCo-authored-by: <tool>,Generated with …)