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

On Thu, Jul 22 2021, Emily Shaffer wrote:

On Fri, Jul 16, 2021 at 10:33:25AM +0200, Ævar Arnfjörð Bjarmason wrote:

quoted

On Thu, Jul 15 2021, Emily Shaffer wrote:

quoted

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.