git cvsimport fails noisily if cvs has no server support

[Jean-Luc Herren]

Hello list!

cvs (1.12.12) can be compiled with --disable-server to omit
support for cvs servers. Although this is not ./configure's
default, it was the default on my distro (gentoo). git-cvsimport
fails loudly as pasted below (note that this command is part of
the test t9600-cvsimport.sh). Nicer behavior would of course be
to detect the situation and inform the user that server support is
missing (and to skip the test).

jlh

$ git-cvsimport -a -z 0 -C module-git module
Unknown command: `server'

CVS commands are:
add Add a new file/directory to the repository
admin Administration front end for rcs
[...26 lines omitted...]
watch Set watches
watchers See who is watching a file
(Specify the --help option for a list of other help options)
Use of uninitialized value in scalar chomp at /home/jlh/cvs/git/t/../git-cvsimport line 345.
Use of uninitialized value in substitution (s///) at /home/jlh/cvs/git/t/../git-cvsimport line 346.
Expected Valid-requests from server, but got: <unknown>
$

[Robin Rosenberg]

Post by Jean-Luc Herren
Hello list!
=20
cvs (1.12.12) can be compiled with --disable-server to omit
support for cvs servers. Although this is not ./configure's
default, it was the default on my distro (gentoo). git-cvsimport
fails loudly as pasted below (note that this command is part of
the test t9600-cvsimport.sh). Nicer behavior would of course be
to detect the situation and inform the user that server support is
missing (and to skip the test).
=20
jlh
=20
$ git-cvsimport -a -z 0 -C module-git module

I'm guessing now, but try -Z '--cvs-direct'.

-- robin

[Jean-Luc Herren]

git-cvsimport now exits less noisily and prints an appropriate
message when the installed cvs binary doesn't know the 'server'
subcommand; this happens when cvs is ./configure'ed with
--disable-server. The test t9600-cvsimport.sh now also tests for
this and skips instead of failing.

Signed-off-by: Jean-Luc Herren <***@gmx.ch>
---

Post by Robin Rosenberg

Post by Jean-Luc Herren
cvs (1.12.12) can be compiled with --disable-server to omit
support for cvs servers. Although this is not ./configure's
default, it was the default on my distro (gentoo). git-cvsimport
fails loudly as pasted below (note that this command is part of
the test t9600-cvsimport.sh). Nicer behavior would of course be
to detect the situation and inform the user that server support is
missing (and to skip the test).
$ git-cvsimport -a -z 0 -C module-git module

=20
I'm guessing now, but try -Z '--cvs-direct'.

git-cvsimport doesn't have a -Z option, maybe you meant "-p
--cvs-direct" to pass --cvs-direct to cvsps. However this is not
a problem with cvsps, it's about cvs not knowing the server
subcommand, which is required when specifying a cvsroot that is a
local path.

Note that if cvs misses the server subcommand, it will spit out
the list of available commands to stderr, which is not useful in
this situation. It seemed to me that redirecting stderr to
/dev/null is a bad idea, as cvs (when it works properly) might
potentially print out useful informations to stderr. Maybe
someone has an idea about how to eliminate the help message
properly.

jlh

git-cvsimport.perl | 11 +++++++++--
t/t9600-cvsimport.sh | 7 +++++++
2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index 5694978..e1bcf0e 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -207,6 +207,7 @@ sub new {
sub conn {
my $self =3D shift;
my $repo =3D $self->{'fullrep'};
+ my $ownserver;
if ($repo =3D~ s/^:pserver(?:([^:]*)):(?:(.*?)(?::(.*?))?@)?([^:\/]*)=
(?::(\d*))?//) {
my ($param,$user,$pass,$serv,$port) =3D ($1,$2,$3,$4,$5);
=20
@@ -285,6 +286,7 @@ sub conn {
$s->flush();
=20
$rep =3D <$s>;
+ die "Remote end hung up unexpectedly" unless defined $rep;
=20
if ($rep ne "I LOVE YOU\n") {
$rep=3D"<unknown>" unless $rep;
@@ -293,6 +295,7 @@ sub conn {
$self->{'socketo'} =3D $s;
$self->{'socketi'} =3D $s;
} else { # local or ext: Fork off our own cvs server.
+ $ownserver =3D 1;
my $pr =3D IO::Pipe->new();
my $pw =3D IO::Pipe->new();
my $pid =3D fork();
@@ -325,7 +328,7 @@ sub conn {
dup2($pr->fileno(),1);
$pr->close();
$pw->close();
- exec(@cvs);
+ exec(@cvs) or exit 1;
}
$pw->writer();
$pr->reader();
@@ -340,7 +343,11 @@ sub conn {
$self->{'socketo'}->write("valid-requests\n");
$self->{'socketo'}->flush();
=20
- chomp(my $rep=3D$self->readline());
+ my $rep=3D$self->readline();
+ if (!defined $rep) {
+ die $ownserver ? "'cvs server' failed; make sure you have a cvs with=
server support" : "Remote end hung up unexpectedly";
+ }
+ chomp $rep;
if ($rep !~ s/^Valid-requests\s*//) {
$rep=3D"<unknown>" unless $rep;
die "Expected Valid-requests from server, but got: $rep\n";
diff --git a/t/t9600-cvsimport.sh b/t/t9600-cvsimport.sh
index 7706430..d8cbfd0 100755
--- a/t/t9600-cvsimport.sh
+++ b/t/t9600-cvsimport.sh
@@ -10,6 +10,13 @@ then
exit
fi
=20
+if echo -n | cvs server 2>&1 | grep 'Unknown command' > /dev/null
+then
+ say 'skipping cvsimport tests, cvs has support for server mode'
+ test_done
+ exit
+fi
+
cvsps_version=3D`cvsps -h 2>&1 | sed -ne 's/cvsps version //p'`
case "$cvsps_version" in
2.1)
--=20
1.5.3.8

[Junio C Hamano]

Post by Jean-Luc Herren
Note that if cvs misses the server subcommand, it will spit out
the list of available commands to stderr, which is not useful in
this situation. It seemed to me that redirecting stderr to
/dev/null is a bad idea, as cvs (when it works properly) might
potentially print out useful informations to stderr. Maybe
someone has an idea about how to eliminate the help message
properly.
...
@@ -340,7 +343,11 @@ sub conn {
$self->{'socketo'}->write("valid-requests\n");
$self->{'socketo'}->flush();
- chomp(my $rep=$self->readline());
+ my $rep=$self->readline();
+ if (!defined $rep) {
+ die $ownserver ? "'cvs server' failed; make sure you have a cvs with server support" : "Remote end hung up unexpectedly";
+ }
+ chomp $rep;

I guess this is probably the best we can do without bending
backwards too much.

If we do not have cvs with server support, is there a fallback
method we can still use to run cvsps?

Post by Jean-Luc Herren
diff --git a/t/t9600-cvsimport.sh b/t/t9600-cvsimport.sh
index 7706430..d8cbfd0 100755
--- a/t/t9600-cvsimport.sh
+++ b/t/t9600-cvsimport.sh
@@ -10,6 +10,13 @@ then
exit
fi
+if echo -n | cvs server 2>&1 | grep 'Unknown command' > /dev/null
+then
+ say 'skipping cvsimport tests, cvs has support for server mode'
+ test_done
+ exit
+fi

Do you mean "has to support server" or "does not have support for"?

[Jean-Luc Herren]

Post by Junio C Hamano
If we do not have cvs with server support, is there a fallback
method we can still use to run cvsps?

I have no idea, I haven't looked at cvsps closely enough.

Post by Junio C Hamano

Post by Jean-Luc Herren
diff --git a/t/t9600-cvsimport.sh b/t/t9600-cvsimport.sh
index 7706430..d8cbfd0 100755
--- a/t/t9600-cvsimport.sh
+++ b/t/t9600-cvsimport.sh
@@ -10,6 +10,13 @@ then
exit
fi
+if echo -n | cvs server 2>&1 | grep 'Unknown command' > /dev/null
+then
+ say 'skipping cvsimport tests, cvs has support for server mode'
+ test_done
+ exit
+fi

Do you mean "has to support server" or "does not have support for"?

I meant to say "cvs has no support for server mode", but I think
"doesn't have support for" is better.

jlh