Re: cvsimport still not working with cvsnt

Guy Rouillier [off-list ref] writes:

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);