Re: cvsimport still not working with cvsnt

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 20, 2011, 07:21 UTC
Message-ID: <7vy65bkw72.fsf@alter.siamese.dyndns.org>
In-Reply-To: <4D5F6E97.4000402@burntmail.com>

Guy Rouillier <guyr@burntmail.com> writes:

Show 6 quoted lines

> The two clients use a different delimiter to separate the CVS
> repository name from the user password.  The original CVS
> client separates the two entries with a space character, while
> CVSNT separates them with an equal (=) character.  Hence,
> the regular expression used to split these two tokens is
> altered to accept either delimiter.

That sounds like a wrong approach. If there are two clients, one reads from one location with one syntax, and the other one reads from another different location with a different syntax, shouldn't the code using the original syntax when reading the original file, and the other syntax when reading the file for the other client?

I personally don't even like the sloppiness of the original code before your patch that discards the version information ("/<digits>") and hopes the file format stays the same for some time to come, but "one uses space and the other uses equal, so lets mix them up and split at space-or-equal when we know we are reading from the file that uses space (iow the one we know we shouldn't be splitting at equal)" is making it even worse.

In practice, I would imagine that the cvsroot part wouldn't contain an equal sign, so this looser regexp would not hurt in the real life, but it does feel yucky.

Here is a totally untested patch. I think the original code used $pass="A" as a fall-back when it didn't find any password entry, and I tried to retain that instead of dying. Also this does not error out if you merely have two cvspass files, as long as you do not have the wanted entry for both of them.

 git-cvsimport.perl |   52 ++++++++++++++++++++++++++++++++++++++++------------
 1 files changed, 40 insertions(+), 12 deletions(-)

diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index 8e683e5..0a25926 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -227,6 +227,31 @@ sub new {
 	return $self;
 }
 
+sub find_password_entry {
+	my ($cvspass, @cvsroot) = @_;
+	my ($file, $delim) = @$cvspass;
+	my $pass;
+	local ($_);
+
+	if (open(my $fh, $file)) {
+		# :pserver:cvs@mea.tmt.tele.fi:/cvsroot/zmailer Ah<Z
+	      CVSPASSFILE:
+		while (<$fh>) {
+			chomp;
+			s/^\/\d+\s+//;
+			my ($w, $p) = split($delim,$_,2);
+			for my $cvsroot (@cvsroot) {
+				if ($w eq $cvsroot) {
+					$pass = $p;
+					last CVSPASSFILE;
+				}
+			}
+		}
+		close($fh);
+	}
+	return $pass;
+}
+
 sub conn {
 	my $self = shift;
 	my $repo = $self->{'fullrep'};
@@ -259,19 +284,22 @@ sub conn {
 		if ($pass) {
 			$pass = $self->_scramble($pass);
 		} else {
-			open(H,$ENV{'HOME'}."/.cvspass") and do {
-				# :pserver:cvs@mea.tmt.tele.fi:/cvsroot/zmailer Ah<Z
-				while (<H>) {
-					chomp;
-					s/^\/\d+\s+//;
-					my ($w,$p) = split(/\s/,$_,2);
-					if ($w eq $rr or $w eq $rr2) {
-						$pass = $p;
-						last;
-					}
+			my @cvspass = ([$ENV{'HOME'}."/.cvspass", qr/\s/],
+				       [$ENV{'HOME'}."/.cvs/cvspass", qr/=/]);
+			my @loc = ();
+			foreach my $cvspass (@cvspass) {
+				my $p = find_password_entry($cvspass, $rr, $rr2);
+				if ($p) {
+					push @loc, $cvspass->[0];
+					$pass = $p;
 				}
-			};
-			$pass = "A" unless $pass;
+			}
+			if (1 < @loc) {
+				die("More than one cvs password files have ".
+				    "entries for CVSROOT $opt_d: @loc");
+			} elsif (!$pass) {
+				$pass = "A";
+			}
 		}
 
 		my ($s, $rep);