Merge branch 'ab/pager-exit-log'
When a pager spawned by us exited, the trace log did not record its
exit status correctly, which has been corrected.
* ab/pager-exit-log:
pager: properly log pager exit code when signalled
run-command: add braces for "if" block in wait_or_whine()
pager: test for exit code with and without SIGPIPE
pager: refactor wait_for_pager() function
diff --git a/pager.c b/pager.c
index ee435de..3d37dd7 100644
--- a/pager.c
+++ b/pager.c
@@ -11,29 +11,25 @@
static struct child_process pager_process = CHILD_PROCESS_INIT;
static const char *pager_program;
-static void wait_for_pager(int in_signal)
+static void close_pager_fds(void)
{
- if (!in_signal) {
- fflush(stdout);
- fflush(stderr);
- }
/* signal EOF to pager */
close(1);
close(2);
- if (in_signal)
- finish_command_in_signal(&pager_process);
- else
- finish_command(&pager_process);
}
static void wait_for_pager_atexit(void)
{
- wait_for_pager(0);
+ fflush(stdout);
+ fflush(stderr);
+ close_pager_fds();
+ finish_command(&pager_process);
}
static void wait_for_pager_signal(int signo)
{
- wait_for_pager(1);
+ close_pager_fds();
+ finish_command_in_signal(&pager_process);
sigchain_pop(signo);
raise(signo);
}
diff --git a/run-command.c b/run-command.c
index ea4d0fb..509841b 100644
--- a/run-command.c
+++ b/run-command.c
@@ -551,8 +551,11 @@
while ((waiting = waitpid(pid, &status, 0)) < 0 && errno == EINTR)
; /* nothing */
- if (in_signal)
- return 0;
+ if (in_signal) {
+ if (WIFEXITED(status))
+ code = WEXITSTATUS(status);
+ return code;
+ }
if (waiting < 0) {
failed_errno = errno;
diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
index fdb450e..0e7cf75 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -656,4 +656,134 @@
test_cmp expect actual
'
+test_expect_success 'setup trace2' '
+ GIT_TRACE2_BRIEF=1 &&
+ export GIT_TRACE2_BRIEF
+'
+
+test_expect_success TTY 'git returns SIGPIPE on early pager exit' '
+ test_when_finished "rm pager-used trace.normal" &&
+ test_config core.pager ">pager-used; head -n 1; exit 0" &&
+ GIT_TRACE2="$(pwd)/trace.normal" &&
+ export GIT_TRACE2 &&
+ test_when_finished "unset GIT_TRACE2" &&
+
+ if test_have_prereq !MINGW
+ then
+ OUT=$( ((test_terminal git log; echo $? 1>&3) | :) 3>&1 ) &&
+ test_match_signal 13 "$OUT"
+ else
+ test_terminal git log
+ fi &&
+
+ grep child_exit trace.normal >child-exits &&
+ test_line_count = 1 child-exits &&
+ grep " code:0 " child-exits &&
+ test_path_is_file pager-used
+'
+
+test_expect_success TTY 'git returns SIGPIPE on early pager non-zero exit' '
+ test_when_finished "rm pager-used trace.normal" &&
+ test_config core.pager ">pager-used; head -n 1; exit 1" &&
+ GIT_TRACE2="$(pwd)/trace.normal" &&
+ export GIT_TRACE2 &&
+ test_when_finished "unset GIT_TRACE2" &&
+
+ if test_have_prereq !MINGW
+ then
+ OUT=$( ((test_terminal git log; echo $? 1>&3) | :) 3>&1 ) &&
+ test_match_signal 13 "$OUT"
+ else
+ test_terminal git log
+ fi &&
+
+ grep child_exit trace.normal >child-exits &&
+ test_line_count = 1 child-exits &&
+ grep " code:1 " child-exits &&
+ test_path_is_file pager-used
+'
+
+test_expect_success TTY 'git discards pager non-zero exit without SIGPIPE' '
+ test_when_finished "rm pager-used trace.normal" &&
+ test_config core.pager "wc >pager-used; exit 1" &&
+ GIT_TRACE2="$(pwd)/trace.normal" &&
+ export GIT_TRACE2 &&
+ test_when_finished "unset GIT_TRACE2" &&
+
+ if test_have_prereq !MINGW
+ then
+ OUT=$( ((test_terminal git log; echo $? 1>&3) | :) 3>&1 ) &&
+ test "$OUT" -eq 0
+ else
+ test_terminal git log
+ fi &&
+
+ grep child_exit trace.normal >child-exits &&
+ test_line_count = 1 child-exits &&
+ grep " code:1 " child-exits &&
+ test_path_is_file pager-used
+'
+
+test_expect_success TTY 'git discards nonexisting pager without SIGPIPE' '
+ test_when_finished "rm pager-used trace.normal" &&
+ test_config core.pager "wc >pager-used; does-not-exist" &&
+ GIT_TRACE2="$(pwd)/trace.normal" &&
+ export GIT_TRACE2 &&
+ test_when_finished "unset GIT_TRACE2" &&
+
+ if test_have_prereq !MINGW
+ then
+ OUT=$( ((test_terminal git log; echo $? 1>&3) | :) 3>&1 ) &&
+ test "$OUT" -eq 0
+ else
+ test_terminal git log
+ fi &&
+
+ grep child_exit trace.normal >child-exits &&
+ test_line_count = 1 child-exits &&
+ grep " code:127 " child-exits &&
+ test_path_is_file pager-used
+'
+
+test_expect_success TTY 'git attempts to page to nonexisting pager command, gets SIGPIPE' '
+ test_when_finished "rm trace.normal" &&
+ test_config core.pager "does-not-exist" &&
+ GIT_TRACE2="$(pwd)/trace.normal" &&
+ export GIT_TRACE2 &&
+ test_when_finished "unset GIT_TRACE2" &&
+
+ if test_have_prereq !MINGW
+ then
+ OUT=$( ((test_terminal git log; echo $? 1>&3) | :) 3>&1 ) &&
+ test_match_signal 13 "$OUT"
+ else
+ test_terminal git log
+ fi &&
+
+ grep child_exit trace.normal >child-exits &&
+ test_line_count = 1 child-exits &&
+ grep " code:-1 " child-exits
+'
+
+test_expect_success TTY 'git returns SIGPIPE on propagated signals from pager' '
+ test_when_finished "rm pager-used trace.normal" &&
+ test_config core.pager ">pager-used; test-tool sigchain" &&
+ GIT_TRACE2="$(pwd)/trace.normal" &&
+ export GIT_TRACE2 &&
+ test_when_finished "unset GIT_TRACE2" &&
+
+ if test_have_prereq !MINGW
+ then
+ OUT=$( ((test_terminal git log; echo $? 1>&3) | :) 3>&1 ) &&
+ test_match_signal 13 "$OUT"
+ else
+ test_terminal git log
+ fi &&
+
+ grep child_exit trace.normal >child-exits &&
+ test_line_count = 1 child-exits &&
+ grep " code:143 " child-exits &&
+ test_path_is_file pager-used
+'
+
test_done
