Issuing warning when hook does not have execution permission

> I saw that if a hook file is present in .git/hooks and it does not
> have execution permissions it is silently ignored.
> 
> I thought it might be worthwhile issuing a warning such as "Warning:
> pre-commit hook exists but it cannot be executed due to insufficient
> permissions".
> 
> Not sure if this has been discussed before. I searched the archive but
> didn't see anything.
> 
> Thoughts, suggestions? Is there anything like that already?

> On Tue, Aug 19, 2014 at 04:05:21PM +1000, Babak M wrote:
>
>> I saw that if a hook file is present in .git/hooks and it does not
>> have execution permissions it is silently ignored.
>> 
>> I thought it might be worthwhile issuing a warning such as "Warning:
>> pre-commit hook exists but it cannot be executed due to insufficient
>> permissions".
>> 
>> Not sure if this has been discussed before. I searched the archive but
>> didn't see anything.
>> 
>> Thoughts, suggestions? Is there anything like that already?
>
> Once upon a time we shipped sample hooks with their execute bits turned
> off, and such a warning would have been very bad.
>
> These days we give them a ".sample" extension (because Windows installs
> had trouble with the execute bit :) ), so I think it should be OK in
> theory. Installing a new version of git on top of an old one with "make
> install" does not clean up old files. So somebody who has continuously
> upgraded their git via "make install" to the same directory would have
> the old-style sample files. Under your proposal, they would get a lot of
> warnings.
>
> However, that change came in v1.6.0, just over 6 years ago. We can
> probably discount that (and if it does happen, maybe it is time for that
> someone to clean up the other leftover cruft from past git installs).

> Jeff King <peff@peff.net> writes:
>
>> On Tue, Aug 19, 2014 at 04:05:21PM +1000, Babak M wrote:
>>
>>> I saw that if a hook file is present in .git/hooks and it does not
>>> have execution permissions it is silently ignored.
>>>
>>> I thought it might be worthwhile issuing a warning such as "Warning:
>>> pre-commit hook exists but it cannot be executed due to insufficient
>>> permissions".
>>>
>>> Not sure if this has been discussed before. I searched the archive but
>>> didn't see anything.
>>>
>>> Thoughts, suggestions? Is there anything like that already?
>>
>> Once upon a time we shipped sample hooks with their execute bits turned
>> off, and such a warning would have been very bad.
>>
>> These days we give them a ".sample" extension (because Windows installs
>> had trouble with the execute bit :) ), so I think it should be OK in
>> theory. Installing a new version of git on top of an old one with "make
>> install" does not clean up old files. So somebody who has continuously
>> upgraded their git via "make install" to the same directory would have
>> the old-style sample files. Under your proposal, they would get a lot of
>> warnings.
>>
>> However, that change came in v1.6.0, just over 6 years ago. We can
>> probably discount that (and if it does happen, maybe it is time for that
>> someone to clean up the other leftover cruft from past git installs).
>
> The above all sounds very sensible.
>
> We have another code path that looks for an executable, finds one
> with no execute permission and decides not to execute it, and I
> wonder if we should use the same criteria to decide to give (or not
> give) a warning as the one used in the other code path (i.e. looking
> up "git-foo" executable when the user runs "git foo").
> --

> On Wed, Aug 20, 2014 at 08:55:52PM +1200, Chris Packham wrote:
>
>> I actually find the existing behaviour useful. If I want to disable a
>> hook to I can just chmod -x .git/hook/... and I then chmod +x it when
>> I want to re-enable it. I guess I could live with an extra warning as
>> long as the command still succeeds.
>
> You could do the same thing "mv $hook $hook.disabled" but it involves
> retraining your fingers. I kind of agree that the existing system of
> respecting the executable bit is nice, though: it does what you told it
> to do, and a misconfiguration is your problem, not the system's. It's
> perhaps worth changing if people frequently get the executable-bit thing
> wrong, but I don't know whether they do or not.
>
> I kind of feel like we had a similar discussion around items in PATH,
> but I don't remember how it resolved.