From: Emily Shaffer Date: Fri, 10 Oct 2025 19:57:20 GMT Subject: Re: [PATCH 00/10] Convert remaining hooks to hook.h Message-ID: In-Reply-To: <20250925125352.1728840-1-adrian.ratiu@collabora.com> On Thu, Sep 25, 2025 at 5:54 AM Adrian Ratiu wrote: > > Hello everyone, > > This is a continuation of Emily and Aevar's work to convert remaining hooks > to the hook.h interface, by adding and using two new run-command/hook APIs: > * feeding hook stdin via a callback > * capturing server-side collated outputs > > I've tried to keep the implementations as simple as possible and avoid any > unnecessary copying by feeding the data directly to the hook stdin fds and > even batching the writes of pre/post-receive so we achieve similar perf/data/ > syscall efficiency as we had before the callback conversion. > > As suggested by Aevar [1], I've removed the string_list API, the extra copies > and the $'\n' assumptions on the data, however I did not go the full zero-copy > route with mmap-ing because I think that will break backwards compatbility. We > could explore that in a future series as an efficientization of the current IPC, > this patch series basically aims for parity with the existing implementation. > > This series also unblocks config-based hooks and hooks parallelization which will > follow up in a separate series. > > The patch series is based on the master branch, I've pushed it to github [2] and > it also passes CI runs. [3]. Also merged and tested against next with no conflicts. > > 1: https://lore.kernel.org/git/230209.86y1p7y4fa.gmgdl@evledraar.gmail.com/ > 2: https://github.com/10ne1/git/tree/dev/aratiu/hooks-conversion-v1 > 3: https://github.com/10ne1/git/actions/runs/18006589297 Thanks Adrian. For the most part I have only small concerns - not surprising as these patches are mostly (not entirely) logically equivalent to patches I wrote a few years ago and which we have been running for Googlers since that time. Others covered some of the comments I had to send, but I have a few comments of my own too. The only one I'm truly stumped on is the `update` hook conversion, we can discuss on that patch. Otherwise, I'll look forward to the v2. - Emily > > Big warm thank you, > Adrian > > Adrian Ratiu (1): > reference-transaction: use hook.h to run hooks > > Emily Shaffer (9): > run-command: add stdin callback for parallelization > hook: provide stdin via callback > hook: convert 'post-rewrite' hook in sequencer.c to hook.h > transport: convert pre-push hook to hook.h > run-command: allow capturing of collated output > hooks: allow callers to capture output > receive-pack: convert 'update' hook to hook.h > post-update: use hook.h library > receive-pack: convert receive hooks to hook.h > > builtin/fetch.c | 2 +- > builtin/receive-pack.c | 310 +++++++++++++++++++----------------- > builtin/submodule--helper.c | 2 +- > hook.c | 11 +- > hook.h | 30 ++++ > refs.c | 61 ++++--- > run-command.c | 115 +++++++++++-- > run-command.h | 44 ++++- > sequencer.c | 62 +++++--- > submodule.c | 2 +- > t/helper/test-run-command.c | 67 +++++++- > t/t0061-run-command.sh | 37 +++++ > transport.c | 79 ++++----- > 13 files changed, 547 insertions(+), 275 deletions(-) > > -- > 2.49.1 >