All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Babak M <babak@melon.com.au>, git <git@vger.kernel.org>
Subject: Re: Issuing warning when hook does not have execution permission
Date: Tue, 19 Aug 2014 09:52:23 -0700	[thread overview]
Message-ID: <xmqqegwcwhfc.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <20140819080002.GB7712@peff.net> (Jeff King's message of "Tue, 19 Aug 2014 04:00:02 -0400")

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").

  reply	other threads:[~2014-08-19 16:53 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-08-19  6:05 Issuing warning when hook does not have execution permission Babak M
2014-08-19  8:00 ` Jeff King
2014-08-19 16:52   ` Junio C Hamano [this message]
2014-08-20  8:55     ` Chris Packham
2014-08-20 10:19       ` Jeff King
2014-08-20 17:51         ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqegwcwhfc.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=babak@melon.com.au \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.