Re: [PATCH] hook: make stdout_to_stderr optional

From: Patrick Steinhardt <ps@pks.im>
Date: Jan 13, 2026, 13:36 UTC
Message-ID: <aWZKYAxhavFc1ZaH@pks.im>
In-Reply-To: <20260113115633.230479-1-adrian.ratiu@collabora.com>

On Tue, Jan 13, 2026 at 01:56:33PM +0200, Adrian Ratiu wrote:

Show 8 quoted lines

> The last batch of hooks converted to the hook.[ch] API introduced
> a regression because pick_next_hook() always sets stdout_to_stderr
> for its child processes.
> 
> Pre-push is the only hook API user which requires stdout_to_stderr
> to be 0, so it can be argued that pre-push needs fixing, however
> this will likely break many pre-push hooks, so it's better to allow
> it to be 0, i.e. to match the previous behavior.

Okay. Do you happen to know whether we've got test coverage for those other hooks? Would be great to verify whether changing `stodut_to_stderr` to default-disabled causes at least one test to fail for every hook we've got.

> We can introduce an extension for the breaking change of all hooks
> sending stdout to stderr, however this just fixes the regression.

Is it really necessary to change this though? I wouldn't really want to go there without a good reason.

Show 11 quoted lines

> diff --git a/transport.c b/transport.c
> index 6d0f02be5d..8f0e5987ab 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -1372,6 +1372,7 @@ static int run_pre_push_hook(struct transport *transport,
>  
>  	opt.feed_pipe = pre_push_hook_feed_stdin;
>  	opt.feed_pipe_cb_data = &data;
> +	opt.stdout_to_stderr = 0;
>  
>  	ret = run_hooks_opt(the_repository, "pre-push", &opt);

The fact that this was able to sneak in without anybody noticing shows that we have a test gap. Can we maybe have a test that verifies that the hook output goes to the correct standard stream?

Thanks!

Patrick