From: Emily Shaffer <nasamuffin@google.com>
Date: Fri, 10 Oct 2025 19:57:17 GMT
Subject: Re: [PATCH 08/10] receive-pack: convert 'update' hook to hook.h
Message-ID: <CAJoAoZ=HRKjjU-N6y+kHo6vpOY6jN4Q7nDdDRpT=cv0k0PtxGg@mail.gmail.com>
In-Reply-To: <20250925125352.1728840-9-adrian.ratiu@collabora.com>

On Thu, Sep 25, 2025 at 5:54 AM Adrian Ratiu <adrian.ratiu@collabora.com> wrote:
>
> From: Emily Shaffer <emilyshaffer@google.com>
>
> This makes use of the new sideband API in hook.h added in the
> preceding commit.
>
> Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  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.


>
> -       strvec_push(&proc.args, hook_path);
> -       strvec_push(&proc.args, cmd->ref_name);
> -       strvec_push(&proc.args, oid_to_hex(&cmd->old_oid));
> -       strvec_push(&proc.args, oid_to_hex(&cmd->new_oid));
> +       /* send a keepalive if there is no data to write */
> +       if (keepalive_active && !output->len) {
> +               static const char buf[] = "0005\1";
> +               write_or_die(1, buf, sizeof(buf) - 1);
> +               return;
> +       }
>
> -       proc.no_stdin = 1;
> -       proc.stdout_to_stderr = 1;
> -       proc.err = use_sideband ? -1 : 0;
> -       proc.trace2_hook_name = "update";
> +       if (use_keepalive == KEEPALIVE_AFTER_NUL && !keepalive_active) {
> +               const char *first_null = memchr(output->buf, '\0', output->len);
> +               if (first_null) {
> +                       /* The null bit is excluded. */
> +                       size_t before_null = first_null - output->buf;
> +                       size_t after_null = output->len - (before_null + 1);
> +                       keepalive_active = 1;
> +                       send_sideband(1, 2, output->buf, before_null, use_sideband);
> +                       send_sideband(1, 2, first_null + 1, after_null, use_sideband);
> +
> +                       return;
> +               }
> +       }
> +
> +       send_sideband(1, 2, output->buf, output->len, use_sideband);
> +}
> +
> +static int run_update_hook(struct command *cmd)
> +{
> +       struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;
> +
> +       strvec_pushl(&opt.args,
> +                    cmd->ref_name,
> +                    oid_to_hex(&cmd->old_oid),
> +                    oid_to_hex(&cmd->new_oid),
> +                    NULL);
>
> -       code = start_command(&proc);
> -       if (code)
> -               return code;
>         if (use_sideband)
> -               copy_to_sideband(proc.err, -1, NULL);
> -       return finish_command(&proc);
> +               opt.consume_sideband = hook_output_to_sideband;
> +
> +       return run_hooks_opt(the_repository, "update", &opt);
>  }
>
>  static struct command *find_command_by_refname(struct command *list,
> --
> 2.49.1
>