[PATCH] Git.pm: localise $? in command_close_bidi_pipe()

From: Abhijit Menon-Sen <ams@toroid.org>
Date: Aug 4, 2008, 11:38 UTC
Message-ID: <20080804113827.GA1239@toroid.org>
In-Reply-To: <7vhca12n2l.fsf@gitster.siamese.dyndns.org>

Git::DESTROY calls _close_cat_blob and _close_hash_and_insert_object, which in turn call command_close_bidi_pipe, which calls waitpid, which alters $?. If this happens during global destruction, it may alter the program's exit status unexpectedly. Making $? local to the function solves the problem.

(The problem was discovered due to a failure of test #8 in t9106-git-svn-commit-diff-clobber.sh.)

Signed-off-by: Abhijit Menon-Sen <ams@toroid.org>
---

At 2008-08-04 01:37:06 -0700, gitster@pobox.com wrote:

>
> After queueing it, I actually had to revert it, because it seems to
> break git-svn (t9106-git-svn-commit-diff-clobber.sh, test #8), and I
> am about to go to bed.

This patch in addition to my earlier one should solve the problem.

For test #8 to fail, the "git svn dcommit" must succeed, but in both cases (i.e. without my patch applied, or with), the rebase fails:

    rebase refs/remotes/git-svn: command returned error: 1

This results in a call to "fatal $@" on git-svn.perl:254, which calls "exit 1", and test_must_fail is happy.

With my patch, however, Git::DESTROY calls the two _close functions during global destruction, which in turn call command_close_bidi_pipe, which calls waitpid with sensible arguments this time, which alters $?, thus altering the exit status of the dcommit itself to 0. Oops.

All of "make test" passes for me after this change.

-- ams

 perl/Git.pm |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/perl/Git.pm b/perl/Git.pm
index 2ef437f..3b6707b 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -417,6 +417,7 @@ have more complicated structure.
 =cut
 
 sub command_close_bidi_pipe {
+	local $?;
 	my ($pid, $in, $out, $ctx) = @_;
 	foreach my $fh ($in, $out) {
 		unless (close $fh) {

-- 
1.6.0.rc0.43.g2aa74