git-cvsserver test failures (still)

git-cvsserver test failures (still)

[Brian Gernhardt]

Periodically I am reminded that the git-cvsserver does not pass it's  
tests (t9400-git-cvsserver.sh) on my machine, so I once again ask for  
help.  The failing tests are: (they say skip because this is from the  
last "find other errors" run)

* skip 9: req_Root failure (strict-paths)
* skip 11: req_Root failure (w/o strict-paths)
* skip 13: req_Root failure (base-path)

No other tests fail, including other req_Root failure tests :

*   ok 6: req_Root failure (relative pathname)
*   ok 7: req_Root failure (conflicting roots)
*   ok 15: req_Root failure (export-all w/o whitelist)

All three tests fail with the exact same error log:

E /Users/brian/dev/git/t/trash/gitcvs.git/ does not seem to be a  
valid GIT repository
E
error 1 /Users/brian/dev/git/t/trash/gitcvs.git/ is not a valid  
repository
E Invalid root /Users/brian/dev/git/t/trash/gitcvs.git

This appears to be from git-cvsserver.perl:148-9:

     req_Root('root', $line) # reuse Root
        or die "E Invalid root $line \n";

This fails the test suite because die() exits with code 255 (checked  
with "perl -e 'die'; echo $?"), which is outside what  
test_expect_failure accepts (see t/test-lib.sh:179).

My questions become:
1) Why hasn't this hit anyone else?
2) Is this where these tests are supposed to fail?
3) If it is, should the code be using print and exit 1 instead of die?
4) If not, should the test be altered to end with "|| false" or  
similar so the test passes?

I'd happily submit a patch to fix this, but don't know what the  
correct fix is.

~~ Brian Gernhardt

Re: git-cvsserver test failures (still)

[Frank Lichtenheld]

Hi.

Some investigations on this:

On Wed, Oct 03, 2007 at 03:50:18PM -0400, Brian Gernhardt wrote:
> This appears to be from git-cvsserver.perl:148-9:
> 
>     req_Root('root', $line) # reuse Root
>        or die "E Invalid root $line \n";
> 
> This fails the test suite because die() exits with code 255 (checked  
> with "perl -e 'die'; echo $?"), which is outside what  
> test_expect_failure accepts (see t/test-lib.sh:179).
> 
> My questions become:
> 1) Why hasn't this hit anyone else?

die doesn't always quit with 255:

"exits with the current value of $! (errno).  If $! is 0, exits with the
value of "($? >> 8)" (backtick ‘command‘ status).  If "($? >> 8)" is 0,
exits with 255"

On my system $! is 9 for these cases and so it exits with exit code 9.

> 2) Is this where these tests are supposed to fail?

yes, IIRC

> 3) If it is, should the code be using print and exit 1 instead of die?

I think that would be the best solution, yes.

> 4) If not, should the test be altered to end with "|| false" or  
> similar so the test passes?

Gruesse,
-- 
Frank Lichtenheld <frank@lichtenheld.de>
www: http://www.djpig.de/

[PATCH] Use exit 1 instead of die when req_Root fails.

[Brian Gernhardt]

This was causing test failures because die was exiting 255.
---

 This finally takes care of my test failures.
 
 git-cvsserver.perl |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index 13dbd27..0d55fec 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl
@@ -145,8 +145,10 @@ if ($state->{method} eq 'pserver') {
     }
     my $request = $1;
     $line = <STDIN>; chomp $line;
-    req_Root('root', $line) # reuse Root
-       or die "E Invalid root $line \n";
+    unless (req_Root('root', $line)) { # reuse Root
+       print "E Invalid root $line \n";
+       exit 1;
+    }
     $line = <STDIN>; chomp $line;
     unless ($line eq 'anonymous') {
        print "E Only anonymous user allowed via pserver\n";
-- 
1.5.3.4.203.gcc61a