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.
