From: Pavel Roskin <proski@gnu.org>
Date: Sat, 08 Dec 2007 08:48:21 GMT
Subject: Re: [PATCH] Calculate $commitsha1 in update() only when needed
Message-ID: <20071208034821.8icn2cflr4ksc0kw@webmail.spamcop.net>
In-Reply-To: <7vtzmtwqff.fsf@gitster.siamese.dyndns.org>

Quoting Junio C Hamano <gitster@pobox.com>:

> Pavel Roskin <proski@gnu.org> writes:
>
>> diff --git a/git-cvsserver.perl b/git-cvsserver.perl
>> index ecded3b..409b301 100755
>> --- a/git-cvsserver.perl
>> +++ b/git-cvsserver.perl
>> @@ -2427,9 +2427,6 @@ sub update
>>      # first lets get the commit list
>>      $ENV{GIT_DIR} = $self->{git_path};
>>
>> -    my $commitsha1 = `git rev-parse $self->{module}`;
>> -    chomp $commitsha1;
>> -
>>      my $commitinfo = `git cat-file commit $self->{module} 2>&1`;
>>      unless ( $commitinfo =~ /tree\s+[a-zA-Z0-9]{40}/ )
>>      {
>
> Hmm.  The first rev-parse could be squelched with 2>/dev/null and then
> you can check if it does not match [a-f0-9]{40} and die early before
> running "cat-file commit", can't you?

Yes, my impression is that the code in question can be improved a lot.

This is specifically the error message I'd like to see fixed in some  
way, as it's confusing to beginners trying to check out the module for  
the first time.

$ CVS_SERVER=/home/proski/bin/git-cvsserver cvs -d \
  :fork:/home/proski/src/qgit/.git co foo
fatal: ambiguous argument 'foo': unknown revision or path not in the  
working tree.
Use '--' to separate paths from revisions
Invalid module 'foo' at /home/proski/bin/git-cvsserver line 2437,  
<STDIN> line 15.
cvs [checkout aborted]: end of file from server (consult above  
messages if any)

It's possible that the message about "--" makes sense and it should  
actually be added in some spaces.

-- 
Regards,
Pavel Roskin