Re: [PATCH v3] bugreport: collect list of populated hooks

From: Emily Shaffer <emilyshaffer@google.com>
Date: May 7, 2020, 21:08 UTC
Message-ID: <20200507210849.GH77802@google.com>
In-Reply-To: <xmqqwo5wpqvg.fsf@gitster.c.googlers.com>

On Thu, Apr 30, 2020 at 03:09:55PM -0700, Junio C Hamano wrote:

Show 25 quoted lines

> 
> Junio C Hamano <gitster@pobox.com> writes:
> 
> >> When we do that, this "mkdir .git/hooks" will fail because the
> >> directory already exists.  Ideas:
> >>
> >>  A. Include a preparatory patch in this series that removes that "mv"
> >>     command.  That way, this test can do
> >
> > While I do not think it is realistic to anticipate that the "test"
> > repository may someday come with a hooks/ directory, even if we did
> > so, we would not enable any hook by default in there.  So "move away
> > and restore" feels way overkill.
> >
> >>   B. Run "git init" ourselves so we know what we're getting:
> >
> > That is certainly safer, and simpler.  But perhaps the simplest
> > would be
> >
> >     C. Use "mkdir -p .git/hooks" so we won't get affected.
> 
> In the meantime, I added this SQUASH on top.  I do not claim that
> this is the best solution, but the idea is to refuse to be affected
> by what is left in .git/hooks either by the test framework or
> earlier tests in the same test script file.

Thanks for the patience with the reply - like many of us, my attention is being pulled many directions right now :)

This solution looks very neat to me, minus one comment. I can send a squash with the change I mention below.

 - Emily

Show 14 quoted lines

> 
>  t/t0091-bugreport.sh | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/t/t0091-bugreport.sh b/t/t0091-bugreport.sh
> index 612c12a918..9450cc02e3 100755
> --- a/t/t0091-bugreport.sh
> +++ b/t/t0091-bugreport.sh
> @@ -58,11 +58,14 @@ test_expect_success 'can create leading directories outside of a git dir' '
>  '
>  
>  test_expect_success 'indicates populated hooks' '
> -	test_when_finished rm git-bugreport-hooks.txt &&
> -	test_when_finished rm -fr .git/hooks &&

I'm not sure it's necessary to lose these two lines. Especially the generated bugreport I'd like to clean up.

Show 10 quoted lines

> +	rm -fr .git/hooks &&
>  	mkdir .git/hooks &&
> -	write_script .git/hooks/applypatch-msg &&
> -	write_script .git/hooks/prepare-commit-msg.sample &&
> +	for hook in applypatch-msg prepare-commit-msg.sample
> +	do
> +		write_script ".git/hooks/$hook" <<-\EOF || return 1
> +		echo "hook $hook exists"
> +		EOF
> +	done &&

I like this placeholder script a lot.

Show 6 quoted lines

>  	git bugreport -s hooks &&
>  	grep applypatch-msg git-bugreport-hooks.txt &&
>  	! grep prepare-commit-msg git-bugreport-hooks.txt
> -- 
> 2.26.2-447-gd61d20c9b4
>