Subject: Re: [PATCH v2 2/4] git-send-email: refactor duplicate $? checks into a function

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Fri, 09 Apr 2021 01:54:45 +0200

On Fri, Apr 09 2021, Junio C Hamano wrote:

> Junio C Hamano <[email-masked]> writes:
>
>> Junio C Hamano <[email-masked]> writes:
>>
>>> One big thing that is different between this version and the one in
>>> Emily's "config hook" topic is that this is still limited to the
>>> case where $repo exists.  In the new world order, it will not matter
>>> in what directory the command runs, as long as "git hook" finds the
>>> hook, and details of the invocation is hidden behind the command.
>>>
>>> I presume that Emily's series is expected to be updated soonish?
>>> Please figure out who to go first and other details to work well
>>> together between you two.
>>
>> Since I didn't hear from either of you, I'll queue with this
>> possibly bogus conflict resolution for now.
>>
>
> Well, I retract it.  This makes many steps in send-email tests
> fail.  For now, es/config-hooks topic is excluded from 'seen'.

Sorry about not replying earlier upthread. FWIW I didn't look deeply
into how the chdir etc. might interact with Emily's topic. I figured
we'd want the $? etc. cleanup first, and that just deleting most of that
code once we had some hook runner to shell out to would be easy.

> What's the status of that topic, if there weren't other topics in
> flight that interfere with it, by the way?  Is it otherwise a good
> enough shape to be given priority and stable enough to get other
> topics rebased on top of it?

I see I've mentioned [1] in passing to you before, but in summary I have
some major qualms about parts of it, but very much like the overall
direction/goal of having hooks in config.

Elevator pitch summary of the lengthy [1]: hooks in config: good, but
having a "git hook" command introduce some nascent UI for managing a
subset of git-config: somewhere between "meh" / "bad idea" (see security
concerns in [1]) / "not needed". I.e. I demonstrated that we can replace
it with a trivial git-config wrapper, if the series doesn't go out of
its way to make it difficult (i.e. we can/should stick all config for a
given hook in the same <prefix>, and not re-invent the
"sendemail.identity" special-case).

I'd very much like the author to respond to that :) And/or for others to
chime in with what they think.

1. https://lore.kernel.org/git/[email-masked]/