Subject: Re: [PATCH] hook: make stdout_to_stderr optional

From: Adrian Ratiu <[email-masked]>
Date: Tue, 13 Jan 2026 17:37:59 +0200

On Tue, 13 Jan 2026, Junio C Hamano <[email-masked]> wrote:
> Adrian Ratiu <[email-masked]> writes:
>
>> On Tue, 13 Jan 2026, Junio C Hamano <[email-masked]> wrote:
>>> Junio C Hamano <[email-masked]> writes:
>>>
>>>> What was the previous behaviour of code paths that ran other hooks?
>>>> Was pre-push the only one that didn't divert standard output to
>>>> standard error?  This patch does look like a proper regression fix
>>>> in that case.  I browsed "git log -p 1627809eef..c65f26fca4" (i.e.,
>>>> the change for "Merge branch 'ar/run-command-hook'") and random
>>>> sampling (like run_receive_hook() that used run_and_feed_hook(),
>>>> which set stdout_to_stderr to 1) seems to indicate that it is the
>>>> case.
>>>
>>> By the way, if stdout_to_stderr is by default set to true, but tnis
>>> regression fix allows specific callers to opt out of it, then the
>>> title "make stdout_to_stderr optional" is a bit misleaing.  It makes
>>> it sound as if it is false by default and optionally turned on.
>>>
>>> Perhaps like "hook: allow stdout_to_stderr optionally off" or
>>> something?
>>
>> Ack. Will rename in v2.
>>
>> Please wait for v2 because, while writing the tests, I noticed pre-push
>> needs 1 additional line (ungroup output) to function as before.
>
> Understood.  Thanks.
>
> Writing these tests would take particular care, I imagine.  Apply
> the test to the tip of the 'master' before ar/run-commmand-hook was
> merged, to verify that the tests expect the behaviour before these
> series, and then merge the result up in more recent 'master' to see
> that the changes in ar/run-commmand-hook did not negatively change
> the behaviour, or something like that?

Yes, that is an excellent idea. The tests should work the same before
and after the conversion. Will do.