Re: [PATCH] Calculate $commitsha1 in update() only when needed

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Quoting Junio C Hamano <gitster@xxxxxxxxx>:

Pavel Roskin <proski@xxxxxxx> 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
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux