public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: "Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail•com>
To: "Adrian Ratiu" <adrian.ratiu@collabora•com>, git@vger•kernel.org
Cc: "Junio C Hamano" <gitster@pobox•com>,
	"Patrick Steinhardt" <ps@pks•im>,
	"Emily Shaffer" <emilyshaffer@google•com>,
	"Jeff King" <peff@peff•net>, "Chris Darroch" <chrisd@apache•org>
Subject: Re: [PATCH v3 2/2] hook: make ungroup opt-out instead of opt-in
Date: Sun, 18 Jan 2026 09:44:53 +0100	[thread overview]
Message-ID: <a2408c8c-db6e-4632-8fd8-7ac888bd3fa2@app.fastmail.com> (raw)
In-Reply-To: <20260114185731.2381550-3-adrian.ratiu@collabora.com>

On Wed, Jan 14, 2026, at 19:57, 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.
>
> While at it add a few hook tests which exercise receive-pack sideband
> output since they are the only ungroup=0 exceptions and there are no
> other tests exercising this functionality.

This description looks okay given that the regression was never
released. Only those who go out of their way build on top of `master`
(for some reason) could have observed it. However if this was a bug in
some release then the description is very technical and play-by-play. As
a Git user reading this isolation, I see nothing that links these
concrete code discussions back to something that might have been weird
in my hook scripts.

This would be especially relevant given that this bug is so weird. It’s
not a crash with some error text that can be googled; everything works
(eventually) the same as before, only that *if* you read from standard
input you’ll have to wait a minute or so for something to time out and
unstuck the whole process.

Again. I think this is okay since it was never a bug in any release.

>
> Fixes: 857f047e40f7 ("hook: allow overriding the ungroup option")
> Reported-by: Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail•com>
> Suggested-by: Jeff King <peff@peff•net>
> Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora•com>
> ---
>[snip]

  parent reply	other threads:[~2026-01-18  8:45 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-13 11:56 [PATCH] hook: make stdout_to_stderr optional Adrian Ratiu
2026-01-13 13:36 ` Patrick Steinhardt
2026-01-13 13:55   ` Adrian Ratiu
2026-01-13 14:00 ` Junio C Hamano
2026-01-13 14:06   ` Junio C Hamano
2026-01-13 14:59     ` Adrian Ratiu
2026-01-13 15:22       ` Junio C Hamano
2026-01-13 15:37         ` Adrian Ratiu
2026-01-13 14:11   ` Adrian Ratiu
2026-01-13 23:45 ` [PATCH v2] hook: allow hooks to disable stdout_to_stderr Adrian Ratiu
2026-01-14  3:12   ` Jeff King
2026-01-14  8:46     ` Adrian Ratiu
2026-01-14  8:59       ` Adrian Ratiu
2026-01-14  9:36       ` Kristoffer Haugsbakk
2026-01-14 17:08       ` Jeff King
2026-01-14 17:19         ` Jeff King
2026-01-14 17:56           ` Adrian Ratiu
2026-01-14  6:13   ` Kristoffer Haugsbakk
2026-01-14 18:57 ` [PATCH v3 0/2] Fix two hook conversion regressions Adrian Ratiu
2026-01-14 18:57   ` [PATCH v3 1/2] hook: allow hooks to disable stdout_to_stderr Adrian Ratiu
2026-01-14 18:57   ` [PATCH v3 2/2] hook: make ungroup opt-out instead of opt-in Adrian Ratiu
2026-01-14 21:27     ` Jeff King
2026-01-14 22:45       ` Adrian Ratiu
2026-01-18  8:44     ` Kristoffer Haugsbakk [this message]
2026-01-15 14:15   ` [PATCH v3 0/2] Fix two hook conversion regressions Junio C Hamano
2026-01-15 17:19     ` Adrian Ratiu
2026-01-15 17:33       ` Junio C Hamano
2026-01-15 17:53         ` Adrian Ratiu
2026-01-15 20:27           ` Junio C Hamano
2026-01-15 21:24             ` Adrian Ratiu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a2408c8c-db6e-4632-8fd8-7ac888bd3fa2@app.fastmail.com \
    --to=kristofferhaugsbakk@fastmail$(echo .)com \
    --cc=adrian.ratiu@collabora$(echo .)com \
    --cc=chrisd@apache$(echo .)org \
    --cc=emilyshaffer@google$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=gitster@pobox$(echo .)com \
    --cc=peff@peff$(echo .)net \
    --cc=ps@pks$(echo .)im \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox