Subject: Re: [PATCH v2 00/10] Convert remaining hooks to hook.h

From: Patrick Steinhardt <[email-masked]>
Date: Tue, 21 Oct 2025 09:40:44 +0200

On Fri, Oct 17, 2025 at 05:15:34PM +0300, Adrian Ratiu wrote:
> Hello everyone,
> 
> This is v2 of the series which converts the remaining hooks to the new API.
> 
> I addressed all the feedback received in v1, with two small exceptions
> (the ones starting with "Opted not to" in the below Changes list).
> 
> I had a minor conflict with an upstream change [1] which was trivial to fix.
> 
> I added 1 new commit and squashed together two commits (the simplified update
> hooks), so in total it's still 10 patches.
> 
> The plan is to follow this up with another series which enables config-based
> hooks and parallel hook execution, where possible.
> 
> As always this is based on the latest master branch, I've pushed it to GitHub
> and ran the CI pipeline [3]. The Win+Meson "missing libgitcore.a" and doc
> "invalid escape sequence" failures seem to be unrelated, since I get them
> without these patches.
> 
> 1: https://github.com/git/git/commit/22e7bc801cd9c5e5b5c4489b631be28e506fec42
> 2: https://github.com/10ne1/git/tree/dev/aratiu/hooks-conversion-v2
> 3: https://github.com/10ne1/git/actions/runs/18593709082
> 
> Changes between v1 -> v2:
> * Added a new commit with a mechanism to override ungroup options (Junio)
> * Addded a BUG if hook path_to_stdin and feed_pipe are both provided (Junio)
> * The feed_pipe cb can be set independently from path_to_stdin (Junio)
> * Simplified the post-rewrite callback (Patrick, Phillip and Junio)
> * Document that hook caller owns the feed_pipe_ctx (Junio)
> * Removed unnecessary "child" -> "notes_cp" renames (Phillip)
> * Reuse strbuf inside pre-push cb to avoid multiple alloc (Phillip)
> * Simplified pre-push hook cb logic (Phillip)
> * Rewrote reference-transaction cb logic to mirror pre-push (Patrick)
> * Simplified the update hook cb by removing the keepalive logic (Emily)
> * Squashed the simplified update and post-update conversions
> * Iterator types, if conditions and other small fixes (Patrick)
> * Fixed a conflict in refs.c with an upstream for loop sign compare check
> * Opted not to use -1 to signify no fd value instead of 0, because I'd have to
>   significantly rework the run-command.h .in/.out/.err API (Patrick)
> * Opted not to move sigchain_push(SIGPIPE, SIG_IGN); into pp_buffer_stdin())
>   because it will called too many times inside the process loop (Patrick)
> * Added Helped-by: Emily Shaffer tag to the reference-transaction coversion
> * Comments, typos, stray lines, commit rewordings (Ben, Patrick, Emily, Junio)

By the way, it would have helped to have a range-diff here so that it's
easy to see what exactly changed between the two different versions.
Could you maybe include that in subsequent iterations?

Thanks!

Patrick