From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 23 Jan 2026 07:19:21 GMT
Subject: Re: [PATCH v7 06/12] hook: allow separate std[out|err] streams
Message-ID: <aXMg-SKKhYzIXvv8@pks.im>
In-Reply-To: <20260121215436.1473800-7-adrian.ratiu@collabora.com>

On Wed, Jan 21, 2026 at 11:54:30PM +0200, Adrian Ratiu wrote:
> The hook API assumed that all hooks merge stdout to stderr.

Tiny nit, not worth rerolling over: we typically write the observation
in past tense. So s/assumed/assumes/

> diff --git a/hook.c b/hook.c
> index 5ddd7678d1..fde1f88ce8 100644
> --- a/hook.c
> +++ b/hook.c
> @@ -81,7 +81,7 @@ static int pick_next_hook(struct child_process *cp,
>  		cp->in = -1;
>  	}
>  
> -	cp->stdout_to_stderr = 1;
> +	cp->stdout_to_stderr = hook_cb->options->stdout_to_stderr;
>  	cp->trace2_hook_name = hook_cb->hook_name;
>  	cp->dir = hook_cb->options->dir;

The implementation looks easy enough. We convert the static value we had
before into a configurable one, and...

> diff --git a/hook.h b/hook.h
> index 2169d4a6bd..7cbeef0a1e 100644
> --- a/hook.h
> +++ b/hook.h
> @@ -34,6 +34,11 @@ struct run_hooks_opt
>  	 */
>  	int *invoked_hook;
>  
> +	/**
> +	 * Send the hook's stdout to stderr.
> +	 */
> +	unsigned int stdout_to_stderr:1;
> +
>  	/**
>  	 * Path to file which should be piped to stdin for each hook.
>  	 */

Another tiny nit that is not worth a reroll: might be worth mentioning
that this is the default behaviour.

> @@ -80,6 +85,7 @@ struct run_hooks_opt
>  #define RUN_HOOKS_OPT_INIT { \
>  	.env = STRVEC_INIT, \
>  	.args = STRVEC_INIT, \
> +	.stdout_to_stderr = 1, \
>  }

... make the old behaviour the default.

Patrick