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
acceptable.
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
break.
Signed-off-by: Jeff King <peff@xxxxxxxx>
---
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
+'
+
test_done
--
1.8.3.rc1.2.g12db477
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html