From: Junio C Hamano <gitster@pobox.com>
Date: Thu, 30 Apr 2020 17:44:28 GMT
Subject: Re: [PATCH v3] bugreport: collect list of populated hooks
Message-ID: <xmqqv9lgswar.fsf@gitster.c.googlers.com>
In-Reply-To: <20200430015049.GA115238@google.com>

Jonathan Nieder <jrnieder@gmail.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.

>> +	write_script .git/hooks/applypatch-msg &&
>
> write_script looks for a script on its stdin.  test_eval_ redirects
> stdin to come from /dev/null, so we happen to get an empty script, but
> this is subtle.  How about something like
>
> 	write_script .git/hooks/applypatch-msg <<-\EOF &&
> 	echo >&2 "rejecting message in $1"
> 	exit 1
> 	EOF

Yes, that is good.

> or
>
> 	write_script .git/hooks/applypatch-msg </dev/null &&

This takes us back to the resuling "empty" hook we wanted to avoid
by switching from "use touch to create something" to "write some
meaningful contents" approach, no?

>> +	git bugreport -s hooks &&
>> +	grep applypatch-msg git-bugreport-hooks.txt &&
>> +	! grep prepare-commit-msg git-bugreport-hooks.txt
>> +'
>
> Thanks,

Thanks.