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

From: Thomas Rast <trast@inf.ethz.ch>
Date: Apr 9, 2013, 21:38 UTC
Message-ID: <87ip3v1j2a.fsf@hexa.v.cablecom.net>
In-Reply-To: <1365432004-20132-1-git-send-email-felipe.contreras@gmail.com>

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

Show 5 quoted lines

> 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

[...]

Show 10 quoted lines

> +# 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