Re: [PATCH v3 2/2] hook: make ungroup opt-out instead of opt-in

[Adrian Ratiu <adrian.ratiu@collabora.com>]

On Wed, 14 Jan 2026, Jeff King <peff@peff.net> wrote:
> On Wed, Jan 14, 2026 at 08:57:31PM +0200, Adrian Ratiu wrote:
>
>> In 857f047e40 (hook: allow overriding the ungroup option, 2025-12-26),
>> I accidentally made the ungroup option opt-in instead of opt-out and
>> despite my best efforts to set it for all API users, I missed a case
>> which requires it to be set: the pre-push hook which regressed.
>> 
>> The only thing I needed in that commit was a way to change the default,
>> to convert the remaining receive-pack hooks which require ungroup == 0
>> for sideband output, so it doesn't matter if it's on or off by default.
>> 
>> Bring back the original behavior by setting it for all hooks in the
>> struct run_hooks_opt initializer, which nicely allows changing the
>> default value only where needed, in receive-pack.c.
>
> I think this is an improvement overall to what's currently in 'seen',
> and the patch looks as I'd expect.

Thanks. :)

My intention for this series is to fix the two regressions reported by
Chris and Kristoffer ASAP and not touch receive-pack (yet!) because it's
a separate topic which deserves its own patch & review / discussion.

>
> I have doubts in general about the approach taken by c65f26fca4
> (receive-pack: convert receive hooks to hook API, 2025-12-26). We used
> to use an async muxer thread, and now we are buffering hook stderr,
> which to my mind is a regression (both in terms of real-time output, but
> also the deadlock issues mentioned earlier).
>
> I'd rather see us continue to set up a muxer thread, and then direct the
> hook API to attach the stderr of the hook processes to that descriptor.
> Then receive-pack would just work as before, without having to fiddle
> with the ungroup flag at all.

I am certainly open to try this other design, especially if we can
eliminate the risk of deadlocks. I will code something along these
lines then send it for you to review.

>
> You can take that with the appropriate size grain of salt from an
> observer who has not been following the series (and is not really
> interested in it, beyond making sure we do not introduce regressions).
> But it is also an observer who has dealt with many I/O deadlocks in Git. ;)

No worries, all feedback is welcome. 

I genuinely appreciate your ideas and suggestions. :)

Expect a receive-pack patch from me soon, in a separate topic.