Re: [PATCH v4] transport-helper: report errors properly

[Thomas Rast <trast@inf.ethz.ch>]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> If a push fails because the remote-helper died (with fast-export), the
> user won't see any error message. So let's add one.
>
> At the same time lets add tests to ensure this error is reported, and
> while we are at it, check the error from fast-import
[...]
> +# We sleep to give fast-export a chance to catch the SIGPIPE
> +test_expect_success 'proper failure checks for pushing' '
> +	(GIT_REMOTE_TESTGIT_FAILURE=1 &&
> +	export GIT_REMOTE_TESTGIT_FAILURE &&
> +	cd local &&
> +	test_must_fail git push --all 2> error &&
> +	cat error &&
> +	grep -q "Reading from remote helper failed" error
> +	)
> +'

There appears to be a race in the version that is in today's pu
(5eb25f737b).  I reproduced with this:

  cd git/t
  i=1
  while ./t5801-remote-helpers.sh --root=/dev/shm --valgrind
  do
    i=$(($i+1))
  done

Two out of six of these loops quit within 1 and 2 iterations,
respectively, both with an error along the lines of:

  expecting success: 
          (GIT_REMOTE_TESTGIT_FAILURE=1 &&
          export GIT_REMOTE_TESTGIT_FAILURE &&
          cd local &&
          test_must_fail git push --all 2> error &&
          cat error &&
          grep -q "Reading from remote helper failed" error
          )

  error: fast-export died of signal 13
  fatal: Error while running fast-export
  not ok 21 - proper failure checks for pushing

I haven't been able to reproduce outside of valgrind tests.  Is this an
expected issue, caused by overrunning the sleep somehow?  If so, can you
increase the sleep delay under valgrind so as to not cause intermittent
failures in the test suite?

-- 
Thomas Rast
trast@{inf,student}.ethz.ch