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

From: Junio C Hamano <gitster@pobox.com>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	git@vger.kernel.org, "Anthony Sottile" <asottile@umich.edu>,
	"Emily Shaffer" <emilyshaffer@google.com>,
	"Phillip Wood" <phillip.wood123@gmail.com>
Subject: Re: [PATCH v2 0/8] hook API: connect hooks to the TTY again, fixes a v2.36.0 regression
Date: Wed, 25 May 2022 18:10:33 -0700	[thread overview]
Message-ID: <xmqqczg13xpy.fsf@gitster.g> (raw)
In-Reply-To: <xmqqbkvl8s88.fsf@gitster.g> (Junio C. Hamano's message of "Wed, 25 May 2022 09:57:59 -0700")

Junio C Hamano <gitster@pobox.com> writes:

> Absolutely.  I wonder how involved is would be to revert the merge
> of the whole thing from 'master'.  It may give us a clean slate to
> rethink the whole mess and redo it without breaking the existing
> users' hooks.

I tried the revert, and the result compiled and tested OK, but I am
tempted to say that it looks as if the topic was deliberately
designed to make it hard to revert by taking as much stuff hostage
as possible.

At least one fix that depends on the run_hooks_opt structure
introduced by c70bc338 (Merge branch 'ab/config-based-hooks-2',
2022-02-09) needs to be discarded.  7431379a (Merge branch
'ab/racy-hooks', 2022-03-16) did address an issue worth addressing,
so even if we revert the whole c70bc338, we would want to redo the
fix, possibly in some other way.  But it also needed an "oops that
was wrong, here is an attempt to fix it again" by cb3b3974 (Merge
branch 'ab/racy-hooks', 2022-03-30).  The situation is quite ugly.

As you hinted in the message I responded to in the message I am
responding to, if we can make a surgical fix to make the new and
improved run_hooks_opt() API build on top of run_command(), instead
on top of run_processes_parallel(), that would give us a cleaner way
out than discarding everything and redoing them "the right way".  At
least, the external interface into the API (read: the impression you
would get by "less hook.h") does not look too bad.

Thanks.