{"thread":{"id":"11854","subject":"git cvsimport fails noisily if cvs has no server support","startedAt":"2008-02-03T15:28:24Z","lastAt":"2008-02-05T13:03:51Z","messageCount":5,"participants":["Jean-Luc Herren","Robin Rosenberg","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"67218","messageId":"47A5DD98.6000606@gmx.ch","threadId":"11854","inReplyTo":null,"subject":"git cvsimport fails noisily if cvs has no server support","fromName":"Jean-Luc Herren","fromEmail":"jlh@gmx.ch","sentAt":"2008-02-03T15:28:24Z","receivedAt":"2008-02-03T15:28:24Z","isPatch":false,"sender":{"key":"jlh@gmx.ch","avatar":null},"body":"Hello list!\n\ncvs (1.12.12) can be compiled with --disable-server to omit\nsupport for cvs servers.  Although this is not ./configure's\ndefault, it was the default on my distro (gentoo).  git-cvsimport\nfails loudly as pasted below (note that this command is part of\nthe test t9600-cvsimport.sh).  Nicer behavior would of course be\nto detect the situation and inform the user that server support is\nmissing (and to skip the test).\n\njlh\n\n$ git-cvsimport -a -z 0 -C module-git module\nUnknown command: `server'\n\nCVS commands are:\n        add          Add a new file/directory to the repository\n        admin        Administration front end for rcs\n[...26 lines omitted...]\n        watch        Set watches\n        watchers     See who is watching a file\n(Specify the --help option for a list of other help options)\nUse of uninitialized value in scalar chomp at /home/jlh/cvs/git/t/../git-cvsimport line 345.\nUse of uninitialized value in substitution (s///) at /home/jlh/cvs/git/t/../git-cvsimport line 346.\nExpected Valid-requests from server, but got: <unknown>\n$ \n"},{"id":"67244","messageId":"200802031908.28115.robin.rosenberg.lists@dewire.com","threadId":"11854","inReplyTo":"47A5DD98.6000606@gmx.ch","subject":"Re: git cvsimport fails noisily if cvs has no server support","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg.lists@dewire.com","sentAt":"2008-02-03T18:08:27Z","receivedAt":"2008-02-03T18:08:27Z","isPatch":false,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"söndagen den 3 februari 2008 skrev Jean-Luc Herren:\n> Hello list!\n> \n> cvs (1.12.12) can be compiled with --disable-server to omit\n> support for cvs servers.  Although this is not ./configure's\n> default, it was the default on my distro (gentoo).  git-cvsimport\n> fails loudly as pasted below (note that this command is part of\n> the test t9600-cvsimport.sh).  Nicer behavior would of course be\n> to detect the situation and inform the user that server support is\n> missing (and to skip the test).\n> \n> jlh\n> \n> $ git-cvsimport -a -z 0 -C module-git module\n\nI'm guessing now, but try -Z '--cvs-direct'.\n\n-- robin\n"},{"id":"67374","messageId":"47A72EE5.2080904@gmx.ch","threadId":"11854","inReplyTo":"200802031908.28115.robin.rosenberg.lists@dewire.com","subject":"[PATCH] git-cvsimport: Detect cvs without support for server mode","fromName":"Jean-Luc Herren","fromEmail":"jlh@gmx.ch","sentAt":"2008-02-04T15:27:33Z","receivedAt":"2008-02-04T15:27:33Z","isPatch":true,"sender":{"key":"jlh@gmx.ch","avatar":null},"body":"git-cvsimport now exits less noisily and prints an appropriate\nmessage when the installed cvs binary doesn't know the 'server'\nsubcommand; this happens when cvs is ./configure'ed with\n--disable-server.  The test t9600-cvsimport.sh now also tests for\nthis and skips instead of failing.\n\nSigned-off-by: Jean-Luc Herren <jlh@gmx.ch>\n---\n\nRobin Rosenberg wrote:\n> söndagen den 3 februari 2008 skrev Jean-Luc Herren:\n>> cvs (1.12.12) can be compiled with --disable-server to omit\n>> support for cvs servers.  Although this is not ./configure's\n>> default, it was the default on my distro (gentoo).  git-cvsimport\n>> fails loudly as pasted below (note that this command is part of\n>> the test t9600-cvsimport.sh).  Nicer behavior would of course be\n>> to detect the situation and inform the user that server support is\n>> missing (and to skip the test).\n>>\n>> $ git-cvsimport -a -z 0 -C module-git module\n> \n> I'm guessing now, but try -Z '--cvs-direct'.\n\ngit-cvsimport doesn't have a -Z option, maybe you meant \"-p\n--cvs-direct\" to pass --cvs-direct to cvsps.  However this is not\na problem with cvsps, it's about cvs not knowing the server\nsubcommand, which is required when specifying a cvsroot that is a\nlocal path.\n\nNote that if cvs misses the server subcommand, it will spit out\nthe list of available commands to stderr, which is not useful in\nthis situation.  It seemed to me that redirecting stderr to\n/dev/null is a bad idea, as cvs (when it works properly) might\npotentially print out useful informations to stderr.  Maybe\nsomeone has an idea about how to eliminate the help message\nproperly.\n\njlh\n\n git-cvsimport.perl   |   11 +++++++++--\n t/t9600-cvsimport.sh |    7 +++++++\n 2 files changed, 16 insertions(+), 2 deletions(-)\n\ndiff --git a/git-cvsimport.perl b/git-cvsimport.perl\nindex 5694978..e1bcf0e 100755\n--- a/git-cvsimport.perl\n+++ b/git-cvsimport.perl\n@@ -207,6 +207,7 @@ sub new {\n sub conn {\n \tmy $self = shift;\n \tmy $repo = $self->{'fullrep'};\n+\tmy $ownserver;\n \tif ($repo =~ s/^:pserver(?:([^:]*)):(?:(.*?)(?::(.*?))?@)?([^:\\/]*)(?::(\\d*))?//) {\n \t\tmy ($param,$user,$pass,$serv,$port) = ($1,$2,$3,$4,$5);\n \n@@ -285,6 +286,7 @@ sub conn {\n \t\t$s->flush();\n \n \t\t$rep = <$s>;\n+\t\tdie \"Remote end hung up unexpectedly\" unless defined $rep;\n \n \t\tif ($rep ne \"I LOVE YOU\\n\") {\n \t\t\t$rep=\"<unknown>\" unless $rep;\n@@ -293,6 +295,7 @@ sub conn {\n \t\t$self->{'socketo'} = $s;\n \t\t$self->{'socketi'} = $s;\n \t} else { # local or ext: Fork off our own cvs server.\n+\t\t$ownserver = 1;\n \t\tmy $pr = IO::Pipe->new();\n \t\tmy $pw = IO::Pipe->new();\n \t\tmy $pid = fork();\n@@ -325,7 +328,7 @@ sub conn {\n \t\t\tdup2($pr->fileno(),1);\n \t\t\t$pr->close();\n \t\t\t$pw->close();\n-\t\t\texec(@cvs);\n+\t\t\texec(@cvs) or exit 1;\n \t\t}\n \t\t$pw->writer();\n \t\t$pr->reader();\n@@ -340,7 +343,11 @@ sub conn {\n \t$self->{'socketo'}->write(\"valid-requests\\n\");\n \t$self->{'socketo'}->flush();\n \n-\tchomp(my $rep=$self->readline());\n+\tmy $rep=$self->readline();\n+\tif (!defined $rep) {\n+\t\tdie $ownserver ? \"'cvs server' failed; make sure you have a cvs with server support\" : \"Remote end hung up unexpectedly\";\n+\t}\n+\tchomp $rep;\n \tif ($rep !~ s/^Valid-requests\\s*//) {\n \t\t$rep=\"<unknown>\" unless $rep;\n \t\tdie \"Expected Valid-requests from server, but got: $rep\\n\";\ndiff --git a/t/t9600-cvsimport.sh b/t/t9600-cvsimport.sh\nindex 7706430..d8cbfd0 100755\n--- a/t/t9600-cvsimport.sh\n+++ b/t/t9600-cvsimport.sh\n@@ -10,6 +10,13 @@ then\n \texit\n fi\n \n+if echo -n | cvs server 2>&1 | grep 'Unknown command' > /dev/null\n+then\n+\tsay 'skipping cvsimport tests, cvs has support for server mode'\n+\ttest_done\n+\texit\n+fi\n+\n cvsps_version=`cvsps -h 2>&1 | sed -ne 's/cvsps version //p'`\n case \"$cvsps_version\" in\n 2.1)\n-- \n1.5.3.8\n"},{"id":"67491","messageId":"7vsl07epo5.fsf@gitster.siamese.dyndns.org","threadId":"11854","inReplyTo":"47A72EE5.2080904@gmx.ch","subject":"Re: [PATCH] git-cvsimport: Detect cvs without support for server mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-05T09:08:26Z","receivedAt":"2008-02-05T09:08:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jean-Luc Herren <jlh@gmx.ch> writes:\n\n> Note that if cvs misses the server subcommand, it will spit out\n> the list of available commands to stderr, which is not useful in\n> this situation.  It seemed to me that redirecting stderr to\n> /dev/null is a bad idea, as cvs (when it works properly) might\n> potentially print out useful informations to stderr.  Maybe\n> someone has an idea about how to eliminate the help message\n> properly.\n> ...\n> @@ -340,7 +343,11 @@ sub conn {\n>  \t$self->{'socketo'}->write(\"valid-requests\\n\");\n>  \t$self->{'socketo'}->flush();\n>  \n> -\tchomp(my $rep=$self->readline());\n> +\tmy $rep=$self->readline();\n> +\tif (!defined $rep) {\n> +\t\tdie $ownserver ? \"'cvs server' failed; make sure you have a cvs with server support\" : \"Remote end hung up unexpectedly\";\n> +\t}\n> +\tchomp $rep;\n\nI guess this is probably the best we can do without bending\nbackwards too much.\n\nIf we do not have cvs with server support, is there a fallback\nmethod we can still use to run cvsps?\n\n> diff --git a/t/t9600-cvsimport.sh b/t/t9600-cvsimport.sh\n> index 7706430..d8cbfd0 100755\n> --- a/t/t9600-cvsimport.sh\n> +++ b/t/t9600-cvsimport.sh\n> @@ -10,6 +10,13 @@ then\n>  \texit\n>  fi\n>  \n> +if echo -n | cvs server 2>&1 | grep 'Unknown command' > /dev/null\n> +then\n> +\tsay 'skipping cvsimport tests, cvs has support for server mode'\n> +\ttest_done\n> +\texit\n> +fi\n\nDo you mean \"has to support server\" or \"does not have support for\"?\n"},{"id":"67509","messageId":"47A85EB7.2070105@gmx.ch","threadId":"11854","inReplyTo":"7vsl07epo5.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] git-cvsimport: Detect cvs without support for server mode","fromName":"Jean-Luc Herren","fromEmail":"jlh@gmx.ch","sentAt":"2008-02-05T13:03:51Z","receivedAt":"2008-02-05T13:03:51Z","isPatch":true,"sender":{"key":"jlh@gmx.ch","avatar":null},"body":"Junio C Hamano wrote:\n> If we do not have cvs with server support, is there a fallback\n> method we can still use to run cvsps?\n\nI have no idea, I haven't looked at cvsps closely enough.\n\n> Jean-Luc Herren <jlh@gmx.ch> writes:\n>> diff --git a/t/t9600-cvsimport.sh b/t/t9600-cvsimport.sh\n>> index 7706430..d8cbfd0 100755\n>> --- a/t/t9600-cvsimport.sh\n>> +++ b/t/t9600-cvsimport.sh\n>> @@ -10,6 +10,13 @@ then\n>>  \texit\n>>  fi\n>>  \n>> +if echo -n | cvs server 2>&1 | grep 'Unknown command' > /dev/null\n>> +then\n>> +\tsay 'skipping cvsimport tests, cvs has support for server mode'\n>> +\ttest_done\n>> +\texit\n>> +fi\n> \n> Do you mean \"has to support server\" or \"does not have support for\"?\n\nI meant to say \"cvs has no support for server mode\", but I think\n\"doesn't have support for\" is better.\n\njlh\n"}]}