Re: [PATCH 1/5] run-command: add preliminary support for multiple hooks
- From
Johannes Sixt <j6t@kdbg.org>
- Date
- Apr 24, 2019, 18:48 UTC
- Message-ID
- <8f79d251-58d9-f63b-7171-7f1fbd11c6f9@kdbg.org>
- In-Reply-To
- <xmqqo94w2l3k.fsf@gitster-ct.c.googlers.com>
Am 24.04.19 um 04:27 schrieb Junio C Hamano:
Show 17 quoted lines
> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
>> +static int has_hook(struct strbuf *path, int strip)
>> +{
>> + if (access(path->buf, X_OK) < 0) {
>
> Does ".git/post-commit" that is not an executable exist?
>
> It was perfectly fine for find_hook() to say "there is no hook for
> post-commit" in the old world in such a case, because the
> unexecutable file it found is not going to be run anyway.
>
> But it is not clear if has_hook(), that affects "there is no single
> hook file for post-commit, so let's look at post-commit.d" decision
> made by find_hooks(), should behave that way. It somehow feels more
> intuitive if a post-commit file that is not executable, by merely
> existing, stops post-commit.d directory from being scanned, at least
> to me.Furthermore, basing a decision on whether a file is executable won't work on Windows as intended. So, it is better to aim for an existence check.
-- Hannes