{"thread":{"id":"15602","subject":"[PATCH] Use dashless git commands in setgitperms.perl","startedAt":"2008-09-19T23:42:34Z","lastAt":"2008-09-22T11:30:08Z","messageCount":5,"participants":["Todd Zullinger","Jakub Narebski","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"91154","messageId":"20080919234234.GL2939@inocybe.teonanacatl.org","threadId":"15602","inReplyTo":null,"subject":"[PATCH] Use dashless git commands in setgitperms.perl","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2008-09-19T23:42:34Z","receivedAt":"2008-09-19T23:42:34Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"Signed-off-by: Todd Zullinger <tmz@pobox.com>\n---\n contrib/hooks/setgitperms.perl |    8 ++++----\n 1 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/contrib/hooks/setgitperms.perl b/contrib/hooks/setgitperms.perl\nindex dab7c8e..5c1fa91 100644\n--- a/contrib/hooks/setgitperms.perl\n+++ b/contrib/hooks/setgitperms.perl\n@@ -8,14 +8,14 @@\n # To save permissions/ownership data, place this script in your .git/hooks\n # directory and enable a `pre-commit` hook with the following lines:\n #      #!/bin/sh\n-#     SUBDIRECTORY_OK=1 . git-sh-setup\n+#     SUBDIRECTORY_OK=1 . git sh-setup\n #     $GIT_DIR/hooks/setgitperms.perl -r\n #\n # To restore permissions/ownership data, place this script in your .git/hooks\n # directory and enable a `post-merge` and `post-checkout` hook with the\n # following lines:\n #      #!/bin/sh\n-#     SUBDIRECTORY_OK=1 . git-sh-setup\n+#     SUBDIRECTORY_OK=1 . git sh-setup\n #     $GIT_DIR/hooks/setgitperms.perl -w\n #\n use strict;\n@@ -50,7 +50,7 @@ if ((@ARGV < 0) || !GetOptions(\n \t\t\t      )) { die $usage; }\n die $usage unless ($read_mode xor $write_mode);\n \n-my $topdir = `git-rev-parse --show-cdup` or die \"\\n\"; chomp $topdir;\n+my $topdir = `git rev-parse --show-cdup` or die \"\\n\"; chomp $topdir;\n my $gitdir = $topdir . '.git';\n my $gitmeta = $topdir . '.gitmeta';\n \n@@ -155,7 +155,7 @@ elsif ($read_mode) {\n \topen (OUT, \">$gitmeta.tmp\") or die \"Could not open $gitmeta.tmp for writing: $!\\n\";\n     }\n \n-    my @files = `git-ls-files`;\n+    my @files = `git ls-files`;\n     my %dirs;\n \n     foreach my $path (@files) {\n-- \n1.6.0.2\n\n-- \nTodd        OpenPGP -> KeyID: 0xBEAF0CE3 | URL: www.pobox.com/~tmz/pgp\n~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\nWhat a terrible thing to have lost one's mind. Or not to have a mind\nat all. How true that is.\n    -- Dan Quayle, speaking to the United Negro College Fund\n"},{"id":"91157","messageId":"gb1ej9$bg2$1@ger.gmane.org","threadId":"15602","inReplyTo":"20080919234234.GL2939@inocybe.teonanacatl.org","subject":"Re: [PATCH] Use dashless git commands in setgitperms.perl","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-09-20T00:02:53Z","receivedAt":"2008-09-20T00:02:53Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Todd Zullinger wrote:\n\n>  # To save permissions/ownership data, place this script in your .git/hooks\n>  # directory and enable a `pre-commit` hook with the following lines:\n>  #      #!/bin/sh\n> -#     SUBDIRECTORY_OK=1 . git-sh-setup\n> +#     SUBDIRECTORY_OK=1 . git sh-setup\n>  #     $GIT_DIR/hooks/setgitperms.perl -r\n>  #\n>  # To restore permissions/ownership data, place this script in your .git/hooks\n>  # directory and enable a `post-merge` and `post-checkout` hook with the\n>  # following lines:\n>  #      #!/bin/sh\n> -#     SUBDIRECTORY_OK=1 . git-sh-setup\n> +#     SUBDIRECTORY_OK=1 . git sh-setup\n>  #     $GIT_DIR/hooks/setgitperms.perl -w\n>  #\n\nThe rest is probably O.K., but I don't think this change is good one.\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n"},{"id":"91160","messageId":"20080920004814.GM2939@inocybe.teonanacatl.org","threadId":"15602","inReplyTo":"gb1ej9$bg2$1@ger.gmane.org","subject":"Re: [PATCH] Use dashless git commands in setgitperms.perl","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2008-09-20T00:48:14Z","receivedAt":"2008-09-20T00:48:14Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"Jakub Narebski wrote:\n> Todd Zullinger wrote:\n> \n>>  # To save permissions/ownership data, place this script in your .git/hooks\n>>  # directory and enable a `pre-commit` hook with the following lines:\n>>  #      #!/bin/sh\n>> -#     SUBDIRECTORY_OK=1 . git-sh-setup\n>> +#     SUBDIRECTORY_OK=1 . git sh-setup\n>>  #     $GIT_DIR/hooks/setgitperms.perl -r\n>>  #\n>>  # To restore permissions/ownership data, place this script in your .git/hooks\n>>  # directory and enable a `post-merge` and `post-checkout` hook with the\n>>  # following lines:\n>>  #      #!/bin/sh\n>> -#     SUBDIRECTORY_OK=1 . git-sh-setup\n>> +#     SUBDIRECTORY_OK=1 . git sh-setup\n>>  #     $GIT_DIR/hooks/setgitperms.perl -w\n>>  #\n> \n> The rest is probably O.K., but I don't think this change is good one.\n\nI don't see git-sh-setup in /usr/bin with a default install of git\n1.6, so without this change, wouldn't the comments be suggesting a\npotentially broken hook configuration?\n\n-- \nTodd        OpenPGP -> KeyID: 0xBEAF0CE3 | URL: www.pobox.com/~tmz/pgp\n~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\nSuppose I were a member of Congress, and suppose I were an idiot. But,\nI repeat myself.\n    -- Mark Twain\n\n"},{"id":"91162","messageId":"7v4p4b7gjr.fsf@gitster.siamese.dyndns.org","threadId":"15602","inReplyTo":"20080920004814.GM2939@inocybe.teonanacatl.org","subject":"Re: [PATCH] Use dashless git commands in setgitperms.perl","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-09-20T01:27:20Z","receivedAt":"2008-09-20T01:27:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Todd Zullinger <tmz@pobox.com> writes:\n\n> Jakub Narebski wrote:\n>> Todd Zullinger wrote:\n>> \n>>>  # To save permissions/ownership data, place this script in your .git/hooks\n>>>  # directory and enable a `pre-commit` hook with the following lines:\n>>>  #      #!/bin/sh\n>>> -#     SUBDIRECTORY_OK=1 . git-sh-setup\n>>> +#     SUBDIRECTORY_OK=1 . git sh-setup\n>>>  #     $GIT_DIR/hooks/setgitperms.perl -r\n>>>  #\n>>>  # To restore permissions/ownership data, place this script in your .git/hooks\n>>>  # directory and enable a `post-merge` and `post-checkout` hook with the\n>>>  # following lines:\n>>>  #      #!/bin/sh\n>>> -#     SUBDIRECTORY_OK=1 . git-sh-setup\n>>> +#     SUBDIRECTORY_OK=1 . git sh-setup\n>>>  #     $GIT_DIR/hooks/setgitperms.perl -w\n>>>  #\n>> \n>> The rest is probably O.K., but I don't think this change is good one.\n>\n> I don't see git-sh-setup in /usr/bin with a default install of git\n> 1.6, so without this change, wouldn't the comments be suggesting a\n> potentially broken hook configuration?\n\nThe patch is wrong on one point, and should be unnecessary.\n\nFirst of all, you do not understand what \".\"  does in shell ;-)  That is\nthe \"wrong\" part.\n\nAbout the \"unnecessary\" part, a hook should run under an environment where\nexec-path is already added to the $PATH, so it should be able to find\ngit-sh-setup just fine without your change.\n\nBut I think the politically correct way would be:\n\n\t#!/bin/sh\n\tPATH=$(git --exec-path):$PATH\n        SUBDIRECTORY_OK=1 . git-sh-setup\n\t$GIT_DIR/hooks/setgitperms.perl -r\n\nespecially if we envision that somebody may run the script by itself, not\nfrom the hook.\n\nThe change to the perl script should not be strictly necessary (because\nthis is expected to be run from a hook), but to set a better example, I\nthink it is a good idea to do these s/git-/git / substitutions.\n"},{"id":"91303","messageId":"20080922113008.GO2939@inocybe.teonanacatl.org","threadId":"15602","inReplyTo":"7v4p4b7gjr.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Use dashless git commands in setgitperms.perl","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2008-09-22T11:30:08Z","receivedAt":"2008-09-22T11:30:08Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"Signed-off-by: Todd Zullinger <tmz@pobox.com>\n---\n contrib/hooks/setgitperms.perl |    4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\nJunio C Hamano wrote:\n> The patch is wrong on one point, and should be unnecessary.\n> \n> First of all, you do not understand what \".\"  does in shell ;-)\n> That is the \"wrong\" part.\n\nUgh, that was embarrassing to miss (bad enough to miss it on my own,\nbut worse to not notice it after Jakub questioned it).\n\n> About the \"unnecessary\" part, a hook should run under an environment\n> where exec-path is already added to the $PATH, so it should be able\n> to find git-sh-setup just fine without your change.\n> \n> But I think the politically correct way would be:\n> \n> \t#!/bin/sh\n> \tPATH=$(git --exec-path):$PATH\n>       SUBDIRECTORY_OK=1 . git-sh-setup\n> \t$GIT_DIR/hooks/setgitperms.perl -r\n> \n> especially if we envision that somebody may run the script by\n> itself, not from the hook.\n\nSince that chunk is only in a comment explaining what to add to the\nhook the user wants to run this script from, it seems like that really\nis unnecessary.\n\nI was in fact running .git/hooks/setgitperms.perl directly (to create\nan initial .gitmeta file) which is why I noticed it used git-*\ncommands and was failing.  Using \"git \" makes that work for me.\n\n> The change to the perl script should not be strictly necessary\n> (because this is expected to be run from a hook), but to set a\n> better example, I think it is a good idea to do these s/git-/git /\n> substitutions.\n\nSounds good.  Here is a patch with just those changes.\n\ndiff --git a/contrib/hooks/setgitperms.perl b/contrib/hooks/setgitperms.perl\nindex dab7c8e..a577ad0 100644\n--- a/contrib/hooks/setgitperms.perl\n+++ b/contrib/hooks/setgitperms.perl\n@@ -50,7 +50,7 @@ if ((@ARGV < 0) || !GetOptions(\n \t\t\t      )) { die $usage; }\n die $usage unless ($read_mode xor $write_mode);\n \n-my $topdir = `git-rev-parse --show-cdup` or die \"\\n\"; chomp $topdir;\n+my $topdir = `git rev-parse --show-cdup` or die \"\\n\"; chomp $topdir;\n my $gitdir = $topdir . '.git';\n my $gitmeta = $topdir . '.gitmeta';\n \n@@ -155,7 +155,7 @@ elsif ($read_mode) {\n \topen (OUT, \">$gitmeta.tmp\") or die \"Could not open $gitmeta.tmp for writing: $!\\n\";\n     }\n \n-    my @files = `git-ls-files`;\n+    my @files = `git ls-files`;\n     my %dirs;\n \n     foreach my $path (@files) {\n-- \n1.6.0.2\n\n-- \nTodd        OpenPGP -> KeyID: 0xBEAF0CE3 | URL: www.pobox.com/~tmz/pgp\n~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\nCareful. We don't want to learn from this.\n    -- Calvin\n"}]}