On Sat, Jan 16, 2021 at 04:35:54PM +0100, Ævar Arnfjörð Bjarmason wrote:

Add a "set -o pipefail" test mode to the test suite to detect failures
in "git" its output is fed directly to a pipe. Doing so is a pattern
we discourage[1] in the test suite, but we've got plenty of tests like
that. Now we can reliably detect those failures.

There was a previous attempt in [2] to add such a test mode, but as
noted by Jeff King in [3] adding it is a matter of peeing against the
wind with current bash semantics of failing on SIGPIPE.

This series relies on a patch of mine to bash, which I'm submitting
upstream, while not breaking anything for vanilla bash users. They
won't have GIT_TEST_PIPEFAIL turned on for them, and will only get
breakages if they turn it on explicitly with "GIT_TEST_PIPEFAIL=true".

I'm not sure about adding code to our test framework that only works
with a patched shell.

Vanilla bash ignores SIGPIPE under "set -e" since version 3.1. It's
only under "set -o pipefail" (added in 3.2) that it doesn't take
account of SIGPIPE, in a seeming omission nobody bothered to fix yet.

Patching bash[4] with:

    diff --git a/jobs.c b/jobs.c
    index a581f305..fa5de82a 100644
    --- a/jobs.c
    +++ b/jobs.c
    @@ -2851,8 +2851,14 @@ raw_job_exit_status (job)
           p = jobs[job]->pipe;
           do
     	{
    -	  if (WSTATUS (p->status) != EXECUTION_SUCCESS)
    -	    fail = WSTATUS(p->status);
    +	  if (WSTATUS (p->status) != EXECUTION_SUCCESS
    +#if defined (DONT_REPORT_SIGPIPE)
    +              && WTERMSIG (p->status) != SIGPIPE
    +#endif
    +              )
    +            {
    +              fail = WSTATUS(p->status);
    +            }
     	  p = p->next;
     	}
           while (p != jobs[job]->pipe);

Makes it useful for something like the git test suite.

Under this test mode we only tests we need to skip those tests which
are explicitly testing that a piped command returned SIGPIPE. Those
tests will now return 0 instead of an exit code indicating SIGPIPE.

Forcing the mode to run under vanilla bash with
"GIT_TEST_PIPEFAIL=true" doesn't fail any tests for me, except the
test in t0000-basic.sh which explicitly checks for the desired
pipefail semantics. However, as Jeff noted in [3] that absence of
failure isn't reliable. I might not see some of the failures due to
the racy nature of how vanilla "set -o pipefail" interacts with *nix
pipe semantics.

1. a378fee5b0 (Documentation: add shell guidelines, 2018-10-05)
2. https://lore.kernel.org/git/cover.1573779465.git.liu.denton@gmail.com/ (local)
3. https://lore.kernel.org/git/20191115040909.GA21654@sigill.intra.peff.net/ (local)
4. https://github.com/bminor/bash/compare/master...avar:avar/ignore-sigterm-and-sigpipe-on-pipe-fail

Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>

We're too early to invoke 'git' like this, period.

At this point PATH has not yet been set up to include the 'git' binary
we are testing, and we can't rely on a recent enough 'git' supporting
'env--helper' to be present in the regular PATH, or that any 'git' is
present in PATH at all.  And indeed we have CI jobs whose output with
this patch now looks like this:

  *** prove ***
  t5401-update-hooks.sh: ./test-lib.sh: line 49: git: not found
  t9001-send-email.sh: ./test-lib.sh: line 49: git: not found
  t5608-clone-2gb.sh: ./test-lib.sh: line 49: git: not found
  [22:05:24] t9001-send-email.sh ................................ ok    58390 ms ( 0.06 usr  0.00 sys + 27.37 cusr  7.49 csys = 34.92 CPU)
  t2013-checkout-submodule.sh: ./test-lib.sh: line 49: git: not found
  [22:05:34] t2013-checkout-submodule.sh ........................ ok     9412 ms ( 0.03 usr  0.01 sys +  5.68 cusr  3.00 csys =  8.72 CPU)
  t0027-auto-crlf.sh: ./test-lib.sh: line 49: git: not found