t7800-difftool.sh failure on pu

* t7800-difftool.sh failure on pu
@ 2012-03-29 18:12 Ramsay Jones
  2012-03-29 21:26 ` Junio C Hamano
  2012-04-02 17:09 ` Tim Henigan
  0 siblings, 2 replies; 7+ messages in thread
From: Ramsay Jones @ 2012-03-29 18:12 UTC (permalink / raw)
  To: tim.henigan; +Cc: Junio C Hamano, GIT Mailing-list

Hi Tim,

With the current pu branch, I have t7800.3 (difftool ignores bad --tool values)
failing on Linux (I haven't tried cygwin or mingw yet). The failure is caused
by the test for the value of the exit code; for me the exit code is 9 not 1.

I have investigated, briefly, and found *two* alternatives for a fix. ;-)

The first option is to (effectively) revert commit 0440ed72 ("difftool: replace
system call with Git::command_noisy", 22-03-2012), like so:

-- >8 --

diff --git a/git-difftool.perl b/git-difftool.perl
index e1754ff..49613b1 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -237,5 +237,7 @@ if (defined($dirdiff)) {
 
 	$ENV{GIT_PAGER} = '';
 	$ENV{GIT_EXTERNAL_DIFF} = 'git-difftool--helper';
-	git_cmd_try { Git::command_noisy(('diff', @ARGV)) } 'exit code %d';
+	my @command = ('git', 'diff', @ARGV);
+	my $rc = system(@command);
+	exit($rc | ($rc >> 8));
 }
-- 8< --

The second option is a bit of a mystery, since I don't see why it is necessary
or why it works! :-P

First take a look at the following:

$ perl -e 'print $!+0, " $!\n";'
0 
$ echo $?
0

$ perl -e 'use Carp qw(croak); print $!+0, " $!\n";'
9 Bad file descriptor
$ echo $?
0

$ perl -e 'use Carp qw(croak); print $!+0, " $!\n"; croak oops;'
9 Bad file descriptor
oops at -e line 1
$ echo $?
9

$ perl -e 'use Carp qw(croak); print $!+0, " $!\n"; $!=0; croak oops;'
9 Bad file descriptor
oops at -e line 1
$ echo $?
255

$ perl -e 'use Carp qw(croak); print $!+0, " $!\n"; $!=0;  $?=1<<8; croak oops;'
9 Bad file descriptor
oops at -e line 1
$ echo $?
1

$ 

So, it seems that a stray non-zero errno (Bad file descriptor) is finding it's
way to the perl exit value via the die call from within croak(). In particular,
the call to croak within git_cmd_try() from Git.pm.

However, I don't see why this is happening, since just before the call to croak
the perl vars $! (errno) and $? (exit status) are set to the *correct* values.
Namely, $! is zero and $? is 256 (1<<8). As you can see from the above, that
*should* result in the correct exit code. However, unlike above, git-difftool.perl
returns 9.

Since 'croak' exits via a 'die' call, in desperation, I tried the following
patch - and it works! Just don't ask me why. :-D

-- 8< --
diff --git a/perl/Git.pm b/perl/Git.pm
index 1c96a20..264a45f 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -1258,7 +1258,7 @@ sub git_cmd_try(&$) {
 		# We can't croak here since Error.pm would mangle
 		# that to Error::Simple.
 	};
-	$err and croak $err;
+	$err and die $err;
 	return $array ? @result : $result[0];
 }
 
-- >8 --

HTH

ATB,
Ramsay Jones

^ permalink raw reply related	[flat|nested] 7+ messages in thread