Re: [PATCH] Use dashless git commands in setgitperms.perl

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 20, 2008, 01:27 UTC
Message-ID: <7v4p4b7gjr.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <20080920004814.GM2939@inocybe.teonanacatl.org>

Todd Zullinger <tmz@pobox.com> writes:

Show 24 quoted lines

> Jakub Narebski wrote:
>> Todd Zullinger wrote:
>> 
>>>  # To save permissions/ownership data, place this script in your .git/hooks
>>>  # directory and enable a `pre-commit` hook with the following lines:
>>>  #      #!/bin/sh
>>> -#     SUBDIRECTORY_OK=1 . git-sh-setup
>>> +#     SUBDIRECTORY_OK=1 . git sh-setup
>>>  #     $GIT_DIR/hooks/setgitperms.perl -r
>>>  #
>>>  # To restore permissions/ownership data, place this script in your .git/hooks
>>>  # directory and enable a `post-merge` and `post-checkout` hook with the
>>>  # following lines:
>>>  #      #!/bin/sh
>>> -#     SUBDIRECTORY_OK=1 . git-sh-setup
>>> +#     SUBDIRECTORY_OK=1 . git sh-setup
>>>  #     $GIT_DIR/hooks/setgitperms.perl -w
>>>  #
>> 
>> The rest is probably O.K., but I don't think this change is good one.
>
> I don't see git-sh-setup in /usr/bin with a default install of git
> 1.6, so without this change, wouldn't the comments be suggesting a
> potentially broken hook configuration?

The patch is wrong on one point, and should be unnecessary.

First of all, you do not understand what "." does in shell ;-) That is the "wrong" part.

About the "unnecessary" part, a hook should run under an environment where exec-path is already added to the $PATH, so it should be able to find git-sh-setup just fine without your change.

But I think the politically correct way would be:

	#!/bin/sh
	PATH=$(git --exec-path):$PATH
        SUBDIRECTORY_OK=1 . git-sh-setup
	$GIT_DIR/hooks/setgitperms.perl -r

especially if we envision that somebody may run the script by itself, not from the hook.

The change to the perl script should not be strictly necessary (because this is expected to be run from a hook), but to set a better example, I think it is a good idea to do these s/git-/git / substitutions.