git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Adrian Ratiu <adrian.ratiu@collabora.com>
Date
Jan 14, 2026, 22:45 UTC
Message-ID
<87qzrrlspa.fsf@collabora.com>
In-Reply-To
<20260114212718.GB1010080@coredump.intra.peff.net>
On Wed, 14 Jan 2026, Jeff King <peff@peff.net> wrote:
Show 17 quoted lines
> 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.

Show 11 quoted lines
>
> 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.

Show 5 quoted lines
>
> 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.
Previous: Jeff KingNext: Kristoffer Haugsbakk
Message 22 of 30 in “hook: make stdout_to_stderr optional”
  1. hook: make stdout_to_stderr optionalAdrian Ratiu, Jan 13, 2026
  2. Patrick SteinhardtJan 13, 2026
  3. Adrian RatiuJan 13, 2026
  4. Junio C HamanoJan 13, 2026
  5. Junio C HamanoJan 13, 2026
  6. Adrian RatiuJan 13, 2026
  7. Junio C HamanoJan 13, 2026
  8. Adrian RatiuJan 13, 2026
  9. Adrian RatiuJan 13, 2026
  10. hook: allow hooks to disable stdout_to_stderrAdrian Ratiu, Jan 13, 2026
  11. Jeff KingJan 14, 2026
  12. Adrian RatiuJan 14, 2026
  13. Adrian RatiuJan 14, 2026
  14. Kristoffer HaugsbakkJan 14, 2026
  15. Jeff KingJan 14, 2026
  16. Jeff KingJan 14, 2026
  17. Adrian RatiuJan 14, 2026
  18. Kristoffer HaugsbakkJan 14, 2026
  19. 0/2 Fix two hook conversion regressionsAdrian Ratiu, Jan 14, 2026
  20. 2/2 hook: make ungroup opt-out instead of opt-inAdrian Ratiu, Jan 14, 2026
  21. Jeff KingJan 14, 2026
  22. Adrian RatiuJan 14, 2026
  23. Kristoffer HaugsbakkJan 18, 2026
  24. 1/2 hook: allow hooks to disable stdout_to_stderrAdrian Ratiu, Jan 14, 2026
  25. Junio C HamanoJan 15, 2026
  26. Adrian RatiuJan 15, 2026
  27. Junio C HamanoJan 15, 2026
  28. Adrian RatiuJan 15, 2026
  29. Junio C HamanoJan 15, 2026
  30. Adrian RatiuJan 15, 2026

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.