From: Patrick Steinhardt <ps@pks.im>
Date: Thu, 02 Oct 2025 06:34:36 GMT
Subject: Re: [PATCH 05/10] reference-transaction: use hook.h to run hooks
Message-ID: <aN4c_DWtqBBScKEh@pks.im>
In-Reply-To: <20250925125352.1728840-6-adrian.ratiu@collabora.com>

On Thu, Sep 25, 2025 at 03:53:48PM +0300, Adrian Ratiu wrote:
> diff --git a/refs.c b/refs.c
> index 4ff55cf24f..5a2b6ad1fc 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -2377,31 +2377,16 @@ static int ref_update_reject_duplicates(struct string_list *refnames,
>  	return 0;
>  }
>  
> -static int run_transaction_hook(struct ref_transaction *transaction,
> -				const char *state)
> +static int transaction_hook_feed_stdin(int hook_stdin_fd, void *pp_cb, void *pp_task_cb UNUSED)
>  {
> -	struct child_process proc = CHILD_PROCESS_INIT;
> +	struct hook_cb_data *hook_cb = pp_cb;
> +	struct run_hooks_opt *opt = hook_cb->options;
> +	struct ref_transaction *transaction = opt->feed_pipe_ctx;
>  	struct strbuf buf = STRBUF_INIT;
> -	const char *hook;
> -	int ret = 0, i;
> -
> -	hook = find_hook(transaction->ref_store->repo, "reference-transaction");
> -	if (!hook)
> -		return ret;
> -
> -	strvec_pushl(&proc.args, hook, state, NULL);
> -	proc.in = -1;
> -	proc.stdout_to_stderr = 1;
> -	proc.trace2_hook_name = "reference-transaction";
> -
> -	ret = start_command(&proc);
> -	if (ret)
> -		return ret;
> -
> -	sigchain_push(SIGPIPE, SIG_IGN);
>  
> -	for (i = 0; i < transaction->nr; i++) {
> +	for (int i = 0; i < transaction->nr; i++) {
>  		struct ref_update *update = transaction->updates[i];
> +		int ret;
>  
>  		if (update->flags & REF_LOG_ONLY)
>  			continue;

Hm. In the "pre-push" hook you converted the callback to process one ref
per invocation. Why don't we do the same over here, with one transaction
per invocation?

Not saying that either one of these is better, but it left me puzzled
why we use two different patterns now.

Patrick