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

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Thu, 21 Apr 2022 20:50:25 +0200

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

> Ævar Arnfjörð Bjarmason  <[email-masked]> 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...