[prev in list] [next in list] [prev in thread] [next in thread] 

List:       git
Subject:    Re: [PATCH] Use dashless git commands in setgitperms.perl
From:       Todd Zullinger <tmz () pobox ! com>
Date:       2008-09-22 11:30:08
Message-ID: 20080922113008.GO2939 () inocybe ! teonanacatl ! org
[Download RAW message or body]

Signed-off-by: Todd Zullinger <tmz@pobox.com>
---
 contrib/hooks/setgitperms.perl |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

Junio C Hamano wrote:
> 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.

Ugh, that was embarrassing to miss (bad enough to miss it on my own,
but worse to not notice it after Jakub questioned it).

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

Since that chunk is only in a comment explaining what to add to the
hook the user wants to run this script from, it seems like that really
is unnecessary.

I was in fact running .git/hooks/setgitperms.perl directly (to create
an initial .gitmeta file) which is why I noticed it used git-*
commands and was failing.  Using "git " makes that work for me.

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

Sounds good.  Here is a patch with just those changes.

diff --git a/contrib/hooks/setgitperms.perl b/contrib/hooks/setgitperms.perl
index dab7c8e..a577ad0 100644
--- a/contrib/hooks/setgitperms.perl
+++ b/contrib/hooks/setgitperms.perl
@@ -50,7 +50,7 @@ if ((@ARGV < 0) || !GetOptions(
 			      )) { die $usage; }
 die $usage unless ($read_mode xor $write_mode);
 
-my $topdir = `git-rev-parse --show-cdup` or die "\n"; chomp $topdir;
+my $topdir = `git rev-parse --show-cdup` or die "\n"; chomp $topdir;
 my $gitdir = $topdir . '.git';
 my $gitmeta = $topdir . '.gitmeta';
 
@@ -155,7 +155,7 @@ elsif ($read_mode) {
 	open (OUT, ">$gitmeta.tmp") or die "Could not open $gitmeta.tmp for writing: $!\n";
     }
 
-    my @files = `git-ls-files`;
+    my @files = `git ls-files`;
     my %dirs;
 
     foreach my $path (@files) {
-- 
1.6.0.2

-- 
Todd        OpenPGP -> KeyID: 0xBEAF0CE3 | URL: www.pobox.com/~tmz/pgp
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Careful. We don't want to learn from this.
    -- Calvin

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
[prev in list] [next in list] [prev in thread] [next in thread]