On Sat, Jun 01, 2013 at 10:01:49AM -0500, Felipe Contreras wrote:

> Anyway, if you care so much about the current behavior, why isn't
> there any tests that check for this?
> My patch passes *all* the tests.

The test suite has never been (and probably never will be) a complete
specification of git's behavior. Noticing that a desired behavior is not
in the test suite is an opportunity to improve its coverage, not argue
that a change which breaks the desired behavior must therefore be

We could do something like the patch below, with two caveats:

  1. The test immediately before it checks for exit codes other than
     "143" on various platforms. I do not know to what degree we need to
     deal with that here. Git is doing the interpretation here rather
     than the shell, so the ksh case should not matter. But I do not
     know what part of Windows converts the exit code to 3. Do we need
     to be looking for 3? Or 131 (128+3)?

  2. The test detects a total breakage of the exit code, like the one
     caused by your patch. But I do not know if it would reliably detect
     us failing to convert the code at all, as the shell running the
     test harness would presumably convert it to shell-convention
     itself. Getting around that would require a custom C program
     checking the status returned by waitpid().

     On the other hand, perhaps it is not the conversion we care about;
     as long as we end up with 143, we are fine. We just want to make
     sure that signal death is recorded in _one_ of the potential signal
     formats. So we do not care if git or the shell did the conversion,
     as long as we end up with 143. The real breakage is exiting with
     code 15, which is losing information.

-- >8 --
Subject: [PATCH] t0005: test git exit code from signal death

When a sub-process dies with a signal, we convert the exit
code to the shell convention of 128+sig. Callers of git may
be relying on this behavior, so let's make sure it does not

Signed-off-by: Jeff King <p...@peff.net>
 t/t0005-signals.sh | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/t/t0005-signals.sh b/t/t0005-signals.sh
index 93e58c0..63f9764 100755
--- a/t/t0005-signals.sh
+++ b/t/t0005-signals.sh
@@ -20,4 +20,11 @@ test_expect_success 'sigchain works' '
        test_cmp expect actual
+test_expect_success 'signals are propagated using shell convention' '
+       # we use exec here to avoid any sub-shell interpretation
+       # of the exit code
+       git config alias.sigterm "!exec test-sigchain" &&
+       test_expect_code 143 git sigterm

To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to