Subject: Re: [PATCH 08/10] receive-pack: convert 'update' hook to hook.h

From: Adrian Ratiu <[email-masked]>
Date: Fri, 17 Oct 2025 11:27:37 +0300

Hi Emily and sorry for the delayed response

On Fri, 10 Oct 2025, Emily Shaffer <[email-masked]> wrote:
> On Thu, Sep 25, 2025 at 5:54 AM Adrian Ratiu 
> <[email-masked]> wrote: 
>> 
>> From: Emily Shaffer <[email-masked]> 
>> 
>> This makes use of the new sideband API in hook.h added in the 
>> preceding commit. 
>> 
>> Signed-off-by: Emily Shaffer <[email-masked]> 
>> Signed-off-by: Ævar Arnfjörð Bjarmason <[email-masked]> --- 
>>  builtin/receive-pack.c | 60 
>>  +++++++++++++++++++++++++++++------------- 1 file changed, 41 
>>  insertions(+), 19 deletions(-) 
>> 
>> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c 
>> index 1113137a6f..d5192ce132 100644 --- 
>> a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ 
>> -939,31 +939,53 @@ static int run_receive_hook(struct command 
>> *commands, 
>>         return status; 
>>  } 
>> 
>> -static int run_update_hook(struct command *cmd) +static void 
>> hook_output_to_sideband(struct strbuf *output, void *cb_data 
>> UNUSED) 
>>  { 
>> -       struct child_process proc = CHILD_PROCESS_INIT; - 
>> int code; -       const char *hook_path = 
>> find_hook(the_repository, "update"); +       int 
>> keepalive_active = 0; 
>> 
>> -       if (!hook_path) -               return 0; +       if 
>> (keepalive_in_sec <= 0) +               use_keepalive = 
>> KEEPALIVE_NEVER; +       if (use_keepalive == KEEPALIVE_ALWAYS) 
>> +               keepalive_active = 1; 
> 
> This hook wasn't using the keepalive at all before, right? 
> What's the reason to use it now? I am worried it might be going 
> to a sideband consumer who wasn't expecting it because it's not 
> documented in githooks. 

Indeed, I just picked this up from the branch I'm basing my work 
on [1] and haven't thought this through enough in v1. There was no 
keepalive before the hook conversion and really there should not 
be any need for it AFAICT (it's a short lived hook).

You raise an excellent point about the behavior change, so I'm 
inclined to remove it in v2. I will obviously test to confirm 
before posting v2.

[1] 
https://github.com/steadmon/git/commit/6d80376bea4e476b1af1d8649fe054cdfd9295dd