{"thread":{"id":"11189","subject":"[PATCH] Calculate $commitsha1 in update() only when needed","startedAt":"2007-12-08T05:07:46Z","lastAt":"2007-12-08T08:48:21Z","messageCount":3,"participants":["Pavel Roskin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"62375","messageId":"20071208050745.29462.74137.stgit@dv.roinet.com","threadId":"11189","inReplyTo":null,"subject":"[PATCH] Calculate $commitsha1 in update() only when needed","fromName":"Pavel Roskin","fromEmail":"proski@gnu.org","sentAt":"2007-12-08T05:07:46Z","receivedAt":"2007-12-08T05:07:46Z","isPatch":true,"sender":{"key":"proski@gnu.org","avatar":null},"body":"This suppresses unhelpful error messages from git rev-parse during\ncheckout if the module doesn't exist.\n\nSigned-off-by: Pavel Roskin <proski@gnu.org>\n---\n\n git-cvsserver.perl |   12 +++++++-----\n 1 files changed, 7 insertions(+), 5 deletions(-)\n\n\ndiff --git a/git-cvsserver.perl b/git-cvsserver.perl\nindex ecded3b..409b301 100755\n--- a/git-cvsserver.perl\n+++ b/git-cvsserver.perl\n@@ -2427,9 +2427,6 @@ sub update\n     # first lets get the commit list\n     $ENV{GIT_DIR} = $self->{git_path};\n \n-    my $commitsha1 = `git rev-parse $self->{module}`;\n-    chomp $commitsha1;\n-\n     my $commitinfo = `git cat-file commit $self->{module} 2>&1`;\n     unless ( $commitinfo =~ /tree\\s+[a-zA-Z0-9]{40}/ )\n     {\n@@ -2440,8 +2437,13 @@ sub update\n     my $git_log;\n     my $lastcommit = $self->_get_prop(\"last_commit\");\n \n-    if (defined $lastcommit && $lastcommit eq $commitsha1) { # up-to-date\n-         return 1;\n+    if (defined $lastcommit) {\n+        my $commitsha1 = `git rev-parse $self->{module}`;\n+        chomp $commitsha1;\n+\n+        if ($lastcommit eq $commitsha1) { # up-to-date\n+            return 1;\n+        }\n     }\n \n     # Start exclusive lock here...\n"},{"id":"62387","messageId":"7vtzmtwqff.fsf@gitster.siamese.dyndns.org","threadId":"11189","inReplyTo":"20071208050745.29462.74137.stgit@dv.roinet.com","subject":"Re: [PATCH] Calculate $commitsha1 in update() only when needed","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-08T08:17:56Z","receivedAt":"2007-12-08T08:17:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pavel Roskin <proski@gnu.org> writes:\n\n> diff --git a/git-cvsserver.perl b/git-cvsserver.perl\n> index ecded3b..409b301 100755\n> --- a/git-cvsserver.perl\n> +++ b/git-cvsserver.perl\n> @@ -2427,9 +2427,6 @@ sub update\n>      # first lets get the commit list\n>      $ENV{GIT_DIR} = $self->{git_path};\n>  \n> -    my $commitsha1 = `git rev-parse $self->{module}`;\n> -    chomp $commitsha1;\n> -\n>      my $commitinfo = `git cat-file commit $self->{module} 2>&1`;\n>      unless ( $commitinfo =~ /tree\\s+[a-zA-Z0-9]{40}/ )\n>      {\n\nHmm.  The first rev-parse could be squelched with 2>/dev/null and then\nyou can check if it does not match [a-f0-9]{40} and die early before\nrunning \"cat-file commit\", can't you?\n\nAlso the regexp to check \"tree\" object name above seems quite wrong ;-)\nIf the purpose of this check is to make sure if the ref points at a\ncommit object, perhaps...\n\n\tmy $commitsha1 = `git rev-parse --verify $self->{module}^0 2>&1`;\n\tchomp($commitsha1);\n        if ($commitsha1 !~ /^[0-9a-f]{40}$/) {\n        \tdie \"no such module $self->{module}\";\n\t}\n\nThen the other hunk below would become unnecessary, I think.\n\n> @@ -2440,8 +2437,13 @@ sub update\n>      my $git_log;\n>      my $lastcommit = $self->_get_prop(\"last_commit\");\n>  \n> -    if (defined $lastcommit && $lastcommit eq $commitsha1) { # up-to-date\n> -         return 1;\n> +    if (defined $lastcommit) {\n> +        my $commitsha1 = `git rev-parse $self->{module}`;\n> +        chomp $commitsha1;\n> +\n> +        if ($lastcommit eq $commitsha1) { # up-to-date\n> +            return 1;\n> +        }\n>      }\n>  \n>      # Start exclusive lock here...\n"},{"id":"62391","messageId":"20071208034821.8icn2cflr4ksc0kw@webmail.spamcop.net","threadId":"11189","inReplyTo":"7vtzmtwqff.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Calculate $commitsha1 in update() only when needed","fromName":"Pavel Roskin","fromEmail":"proski@gnu.org","sentAt":"2007-12-08T08:48:21Z","receivedAt":"2007-12-08T08:48:21Z","isPatch":true,"sender":{"key":"proski@gnu.org","avatar":null},"body":"Quoting Junio C Hamano <gitster@pobox.com>:\n\n> Pavel Roskin <proski@gnu.org> writes:\n>\n>> diff --git a/git-cvsserver.perl b/git-cvsserver.perl\n>> index ecded3b..409b301 100755\n>> --- a/git-cvsserver.perl\n>> +++ b/git-cvsserver.perl\n>> @@ -2427,9 +2427,6 @@ sub update\n>>      # first lets get the commit list\n>>      $ENV{GIT_DIR} = $self->{git_path};\n>>\n>> -    my $commitsha1 = `git rev-parse $self->{module}`;\n>> -    chomp $commitsha1;\n>> -\n>>      my $commitinfo = `git cat-file commit $self->{module} 2>&1`;\n>>      unless ( $commitinfo =~ /tree\\s+[a-zA-Z0-9]{40}/ )\n>>      {\n>\n> Hmm.  The first rev-parse could be squelched with 2>/dev/null and then\n> you can check if it does not match [a-f0-9]{40} and die early before\n> running \"cat-file commit\", can't you?\n\nYes, my impression is that the code in question can be improved a lot.\n\nThis is specifically the error message I'd like to see fixed in some  \nway, as it's confusing to beginners trying to check out the module for  \nthe first time.\n\n$ CVS_SERVER=/home/proski/bin/git-cvsserver cvs -d \\\n  :fork:/home/proski/src/qgit/.git co foo\nfatal: ambiguous argument 'foo': unknown revision or path not in the  \nworking tree.\nUse '--' to separate paths from revisions\nInvalid module 'foo' at /home/proski/bin/git-cvsserver line 2437,  \n<STDIN> line 15.\ncvs [checkout aborted]: end of file from server (consult above  \nmessages if any)\n\nIt's possible that the message about \"--\" makes sense and it should  \nactually be added in some spaces.\n\n-- \nRegards,\nPavel Roskin\n"}]}