Subject: Re: [PATCH 7/9] hook: allow out-of-repo 'git hook' invocations

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Fri, 23 Jul 2021 11:18:22 +0200

On Thu, Jul 22 2021, Emily Shaffer wrote:

> On Fri, Jul 16, 2021 at 10:33:25AM +0200, Ævar Arnfjörð Bjarmason wrote:
>> 
>> 
>> On Thu, Jul 15 2021, Emily Shaffer wrote:
>> 
>> > Since hooks can now be supplied via the config, and a config can be
>> > present without a gitdir via the global and system configs, we can start
>> > to allow 'git hook run' to occur without a gitdir. This enables us to do
>> > things like run sendemail-validate hooks when running 'git send-email'
>> > from a nongit directory.
>> >
>> > It still doesn't make sense to look for hooks in the hookdir in nongit
>> > repos, though, as there is no hookdir.
>> 
>> Hrm, I haven't tested but re the discussion we had about
>> RUN_SETUP_GENTLY on my re-rolled base topic is this really just a
>> regression in my changes there?
>> 
>> I.e. I assumed we could do RUN_SETUP for the bug-for-bug compatibility
>> step, but send-email runs out of repo hooks and we just didn't have
>> tests for it, or am I missing something?
>
> I'm not sure. I could see a case for you including RUN_SETUP_GENTLY on
> your series and adding a test for sendemail-validate + core.hooksPath in
> global config. I think I also don't have support for that case here,
> actually....

The narrow goal of the base topic is to be a bug-for-bug compatible
version of what we have now on "master", just via a dispatch
command/API.

So yeah, that git-send-email.perl behavior looks bizarre, but let's fix
it in a separate set of behavior changing patches on top, no?

> Anyway, it looks like right now git-send-email.perl:validate_patch()
> doesn't bother if it's out-of-repo, so this wouldn't have worked before (and
> still won't work even after this change). So either I can add a patch to
> my series to allow that, or you can modify your patch converting
> sendemail-validate to 'git hook run' to drop the 'if $repo' line and
> teach your series to behave correctly with nongit+hooksPath. It looks
> like in my earlier attempt at the series, I did drop that check and run
> 'git hook run' no matter what kind of directory we're in.

I think this was one of the things that either needed a test change in
your series, or I saw tests for changes to existing but untested
behavior, I think this was the latter. 

I completely agree that the behavior is weird, probably undesired and
should be changed. I'm just saying that we should split up refactorings
& behavior changes.