Re: [PATCH v3 6/6] hook: allow out-of-repo 'git hook' invocations

> 
> 
> On Wed, Aug 18 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.
> 
> Sensible goal. Perhaps we should note in an earlier commit when
> config-based hooks are introduced something like:
> 
>     Even though we've added config-based hooks, they currently only work
>     if we can find a .git directory, even though certain commands such
>     as "git send-email" (or only that command?) can be run outside of a
>     git directory. A subsequent commit will address that edge-case.

> 
> > [...]
> > Notes:
> >     For hookdir hooks, do we want to run them in nongit dir when core.hooksPath
> >     is set? For example, if someone set core.hooksPath in their global config and
> >     then ran 'git hook run sendemail-validate' in a nongit dir?
> > [...]
> >  git.c           |  2 +-
> >  hook.c          |  2 +-
> >  t/t1800-hook.sh | 20 +++++++++++++++-----
> >  3 files changed, 17 insertions(+), 7 deletions(-)
> >
> > diff --git a/git.c b/git.c
> > index 540909c391..39988ee3b0 100644
> > --- a/git.c
> > +++ b/git.c
> > @@ -538,7 +538,7 @@ static struct cmd_struct commands[] = {
> >  	{ "grep", cmd_grep, RUN_SETUP_GENTLY },
> >  	{ "hash-object", cmd_hash_object },
> >  	{ "help", cmd_help },
> > -	{ "hook", cmd_hook, RUN_SETUP },
> > +	{ "hook", cmd_hook, RUN_SETUP_GENTLY },
> >  	{ "index-pack", cmd_index_pack, RUN_SETUP_GENTLY | NO_PARSEOPT },
> >  	{ "init", cmd_init_db },
> >  	{ "init-db", cmd_init_db },
> > diff --git a/hook.c b/hook.c
> > index 581d87cbbd..2e08156546 100644
> > --- a/hook.c
> > +++ b/hook.c
> > @@ -218,7 +218,7 @@ struct list_head *list_hooks_gently(const char *hookname)
> >  
> >  	/* Add the hook from the hookdir. The placeholder makes it easier to
> >  	 * allocate work in pick_next_hook. */
> > -	if (find_hook_gently(hookname))
> > +	if (have_git_dir() && find_hook_gently(hookname))
> >  		append_or_move_hook(hook_head, NULL);
> >  
> >  	return hook_head;
> > diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh
> > index ef2432f53a..a7e45c0d16 100755
> > --- a/t/t1800-hook.sh
> > +++ b/t/t1800-hook.sh
> > @@ -118,15 +118,25 @@ test_expect_success 'git hook run -- pass arguments' '
> >  	test_cmp expect actual
> >  '
> >  
> > -test_expect_success 'git hook run -- out-of-repo runs excluded' '
> > -	write_script .git/hooks/test-hook <<-EOF &&
> > -	echo Test hook
> > -	EOF
> > +test_expect_success 'git hook run: out-of-repo runs execute global hooks' '
> > +	test_config_global hook.global-hook.event test-hook --add &&
> > +	test_config_global hook.global-hook.command "echo no repo no problems" --add &&
> > +
> > +	echo "global-hook" >expect &&
> > +	nongit git hook list test-hook >actual &&
> > +	test_cmp expect actual &&
> > +
> > +	echo "no repo no problems" >expect &&
> >  
> > -	nongit test_must_fail git hook run test-hook
> > +	nongit git hook run test-hook 2>actual &&
> > +	test_cmp expect actual
> >  '
> >  
> >  test_expect_success 'git -c core.hooksPath=<PATH> hook run' '
> > +	write_script .git/hooks/test-hook <<-EOF &&
> > +	echo Test hook
> > +	EOF
> > +
> >  	mkdir my-hooks &&
> >  	write_script my-hooks/test-hook <<-\EOF &&
> >  	echo Hook ran $1 >>actual
> 
> If the only user of this is git-send-email, let's have tests for this in
> t/t9001-send-email.sh. That should also address your "Notes" above,
> i.e. let's just test it with core.hooksPath and see what the interaction
> looks like.