Guy Rouillier wrote:
> Note that I've left this test in, although I still think it is a bad idea:
>
> elsif (!$pass) {
> $pass = "A";
> }
[...]
> But that doesn't explain why it was put in there in the first
> place. I still say a better idea, if we don't want to allow an empty
> password, is to error out rather than silently set a bogus password.
It might be a good idea after all to do something else in that case
(as a separate patch :)), but it would require a little investigation.
Isn't the convention in CVS for anonymous pserver access to accept an
arbitrary password?
> The CVS password file separates tokens with a space character, while
> the CVSNT password file separates tokens with an equal (=) character.
> Add a sub find_password_entry that accepts the password file name
> and a delimiter to eliminate code duplication.
> ---
Sounds sensible to my untrained ears. Sign-off?
[...]
> +++ b/git-cvsimport.perl
> @@ -227,6 +227,30 @@ 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@xxxxxxxxxxxxxxx:/cvsroot/zmailer Ah<Z
> + while (<$fh>) {
> + chomp;
> + s/^\/\d+\s+//;
> + my ($w, $p) = split($delim,$_,2);
> + for my $cvsroot (@cvsroot) {
> + if ($w eq $cvsroot) {
> + $pass = $p;
> + last;
In the old code, this "last" applied to the while loop, while in the
new code it applies to the for loop. Intentional?
[...]
> + if (1 < @loc) {
> + die("More than one cvs password files have ".
> + "entries for CVSROOT $opt_d: @loc");
Grammar nit: "More than one" is singular (weird, eh?). It might
be clearer to say:
"Multiple cvs password files have " .
"entries for CVSROOT $opt_d: @loc"
(or "Both cvs password files").
Thanks again, and hope that helps.
Regards,
Jonathan
--
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