Re: [PATCH 1/5] run-command: add preliminary support for multiple hooks

["brian m. carlson" <sandals@crustytoothpaste.net>]

[-- Attachment #1: Type: text/plain, Size: 1812 bytes --]

On Wed, Apr 24, 2019 at 11:27:59AM +0900, Junio C Hamano wrote:
> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
> > diff --git a/run-command.c b/run-command.c
> > index 3449db319b..669af5ebc7 100644
> > --- a/run-command.c
> > +++ b/run-command.c
> > @@ -1308,58 +1308,137 @@ int async_with_fork(void)
> >  #endif
> >  }
> >  
> > +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.

Unfortunately, we used to have Git versions that wrote an non-executable
file in place instead of a ".sample" file when initializing the
repository. We have a warning for that, but I have many repositories
that have lived long enough that they're still affected (and I've turned
off the warning for that reason). I feel like we'll be creating
surprising behavior for long-term users.

Also, if someone is using an old manager script that uses the ".d"
directory or some other workaround, it's easy enough for them to simply
clear the executable bit; they need not delete it, and can restore it at
any time simply by toggling the bit.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]