Skip to content

Commit 97991df

Browse files
gitstergitster
committed
Merge branch 'jk/t7006-sigpipe-tests-fix'
The function to cull a child process and determine the exit status had two separate code paths for normal callers and callers in a signal handler, and the latter did not yield correct value when the child has caught a signal. The handling of the exit status has been unified for these two code paths. An existing test with flakiness has also been corrected. * jk/t7006-sigpipe-tests-fix: t7006: simplify exit-code checks for sigpipe tests t7006: clean up SIGPIPE handling in trace2 tests run-command: unify signal and regular logic for wait_or_whine()
2 parents 8bb6fe8 + 5263e22 commit 97991df

File tree

2 files changed

+23
-50
lines changed

2 files changed

+23
-50
lines changed

‎run-command.c‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -552,20 +552,17 @@ static int wait_or_whine(pid_t pid, const char *argv0, int in_signal)
552552

553553
while ((waiting = waitpid(pid, &status, 0)) < 0 && errno == EINTR)
554554
; /* nothing */
555-
if (in_signal) {
556-
if (WIFEXITED(status))
557-
code = WEXITSTATUS(status);
558-
return code;
559-
}
560555

561556
if (waiting < 0) {
562557
failed_errno = errno;
563-
error_errno("waitpid for %s failed", argv0);
558+
if (!in_signal)
559+
error_errno("waitpid for %s failed", argv0);
564560
} else if (waiting != pid) {
565-
error("waitpid is confused (%s)", argv0);
561+
if (!in_signal)
562+
error("waitpid is confused (%s)", argv0);
566563
} else if (WIFSIGNALED(status)) {
567564
code = WTERMSIG(status);
568-
if (code != SIGINT && code != SIGQUIT && code != SIGPIPE)
565+
if (!in_signal && code != SIGINT && code != SIGQUIT && code != SIGPIPE)
569566
error("%s died of signal %d", argv0, code);
570567
/*
571568
* This return value is chosen so that code & 0xff
@@ -576,10 +573,12 @@ static int wait_or_whine(pid_t pid, const char *argv0, int in_signal)
576573
} else if (WIFEXITED(status)) {
577574
code = WEXITSTATUS(status);
578575
} else {
579-
error("waitpid is confused (%s)", argv0);
576+
if (!in_signal)
577+
error("waitpid is confused (%s)", argv0);
580578
}
581579

582-
clear_child_for_cleanup(pid);
580+
if (!in_signal)
581+
clear_child_for_cleanup(pid);
583582

584583
errno = failed_errno;
585584
return code;

‎t/t7006-pager.sh‎

Lines changed: 14 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -661,6 +661,13 @@ test_expect_success 'setup trace2' '
661661
export GIT_TRACE2_BRIEF
662662
'
663663

664+
test_expect_success 'setup large log output' '
665+
perl -e "
666+
print \"this is a long commit message\" x 50000
667+
" >commit-msg &&
668+
git commit --allow-empty -F commit-msg
669+
'
670+
664671
test_expect_success TTY 'git returns SIGPIPE on early pager exit' '
665672
test_when_finished "rm pager-used trace.normal" &&
666673
test_config core.pager ">pager-used; head -n 1; exit 0" &&
@@ -670,7 +677,7 @@ test_expect_success TTY 'git returns SIGPIPE on early pager exit' '
670677
671678
if test_have_prereq !MINGW
672679
then
673-
OUT=$( ((test_terminal git log; echo $? 1>&3) | :) 3>&1 ) &&
680+
{ test_terminal git log >/dev/null; OUT=$?; } &&
674681
test_match_signal 13 "$OUT"
675682
else
676683
test_terminal git log
@@ -691,7 +698,7 @@ test_expect_success TTY 'git returns SIGPIPE on early pager non-zero exit' '
691698
692699
if test_have_prereq !MINGW
693700
then
694-
OUT=$( ((test_terminal git log; echo $? 1>&3) | :) 3>&1 ) &&
701+
{ test_terminal git log >/dev/null; OUT=$?; } &&
695702
test_match_signal 13 "$OUT"
696703
else
697704
test_terminal git log
@@ -710,55 +717,22 @@ test_expect_success TTY 'git discards pager non-zero exit without SIGPIPE' '
710717
export GIT_TRACE2 &&
711718
test_when_finished "unset GIT_TRACE2" &&
712719
713-
if test_have_prereq !MINGW
714-
then
715-
OUT=$( ((test_terminal git log; echo $? 1>&3) | :) 3>&1 ) &&
716-
test "$OUT" -eq 0
717-
else
718-
test_terminal git log
719-
fi &&
720+
test_terminal git log &&
720721
721722
grep child_exit trace.normal >child-exits &&
722723
test_line_count = 1 child-exits &&
723724
grep " code:1 " child-exits &&
724725
test_path_is_file pager-used
725726
'
726727

727-
test_expect_success TTY 'git discards nonexisting pager without SIGPIPE' '
728-
test_when_finished "rm pager-used trace.normal" &&
729-
test_config core.pager "wc >pager-used; does-not-exist" &&
730-
GIT_TRACE2="$(pwd)/trace.normal" &&
731-
export GIT_TRACE2 &&
732-
test_when_finished "unset GIT_TRACE2" &&
733-
734-
if test_have_prereq !MINGW
735-
then
736-
OUT=$( ((test_terminal git log; echo $? 1>&3) | :) 3>&1 ) &&
737-
test "$OUT" -eq 0
738-
else
739-
test_terminal git log
740-
fi &&
741-
742-
grep child_exit trace.normal >child-exits &&
743-
test_line_count = 1 child-exits &&
744-
grep " code:127 " child-exits &&
745-
test_path_is_file pager-used
746-
'
747-
748-
test_expect_success TTY 'git attempts to page to nonexisting pager command, gets SIGPIPE' '
728+
test_expect_success TTY 'git skips paging nonexisting command' '
749729
test_when_finished "rm trace.normal" &&
750730
test_config core.pager "does-not-exist" &&
751731
GIT_TRACE2="$(pwd)/trace.normal" &&
752732
export GIT_TRACE2 &&
753733
test_when_finished "unset GIT_TRACE2" &&
754734
755-
if test_have_prereq !MINGW
756-
then
757-
OUT=$( ((test_terminal git log; echo $? 1>&3) | :) 3>&1 ) &&
758-
test_match_signal 13 "$OUT"
759-
else
760-
test_terminal git log
761-
fi &&
735+
test_terminal git log &&
762736
763737
grep child_exit trace.normal >child-exits &&
764738
test_line_count = 1 child-exits &&
@@ -767,14 +741,14 @@ test_expect_success TTY 'git attempts to page to nonexisting pager command, gets
767741

768742
test_expect_success TTY 'git returns SIGPIPE on propagated signals from pager' '
769743
test_when_finished "rm pager-used trace.normal" &&
770-
test_config core.pager ">pager-used; test-tool sigchain" &&
744+
test_config core.pager ">pager-used; exec test-tool sigchain" &&
771745
GIT_TRACE2="$(pwd)/trace.normal" &&
772746
export GIT_TRACE2 &&
773747
test_when_finished "unset GIT_TRACE2" &&
774748
775749
if test_have_prereq !MINGW
776750
then
777-
OUT=$( ((test_terminal git log; echo $? 1>&3) | :) 3>&1 ) &&
751+
{ test_terminal git log >/dev/null; OUT=$?; } &&
778752
test_match_signal 13 "$OUT"
779753
else
780754
test_terminal git log

0 commit comments

Comments
 (0)