Re: [PATCH 0/6] hook API: connect hooks to the TTY again, fixes a v2.36.0 regression

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Apr 21, 2022, 18:50 UTC
Message-ID: <220421.86bkwu6zd2.gmgdl@evledraar.gmail.com>
In-Reply-To: <xmqqmtgejq4o.fsf@gitster.g>

On Thu, Apr 21 2022, Junio C Hamano wrote:

Show 10 quoted lines

> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
>> This fixes the regression reported by Anthony Sottile[1] with hooks
>> not being connected to a TTY. See 6/6 for details.
>
> It is surprising that it takes 6 patches that rewrites ~100 lines
> and adds ~200 new lines, which would need to be treated as a new
> development with its own risk of regressions (hence going through
> the normal review cycle, starting out of 'next', and gradually
> getting merged down), instead of being able to be fast-tracked.

Yes, it's unfortunate. Perhaps there's some way I'm missing in coming up with a smaller isolated fix, but I wasn't able to come up with one.

It's not just the pre-commit hook, that's just where the problem was discovered, but the dozen or so other run_hook*() users (not all of whom may be practically impacted).

Rewriting the hook.c API to use another "runner" would probably be the smallest change, but even if that's going to be smaller by line-count I'd think it's much bigger in terms of review time. Most of this series is boilerplate changes, or changes where it's easy to review that no caller of run-command.[ch] that isn't hook.c will be impacted by them...