Re: [PATCH v2 4/5] repack: use tempfiles for signal cleanup

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> Here's a patch on top of of jk/repack-tempfile-cleanup that adjusts the
> test (and should make the immediate racy CI pain go away). It gives some
> explanation why the third option isn't as interesting as you'd think. If
> somebody later wants to add such a "pack-objects died" error, we can
> adjust sigpipe handling there.

An extremely simplified alternative would be just to say !  instead
of test_must_fail, which essentially is ok=anycrash ;-)

I did try the same exact patch before going to bed last night but
t7700 somehow failed some other steps in my local tests and I gave
up digging further X-<.  One step at a time...

Will queue.  Thanks.

> -- >8 --
> Subject: [PATCH] t7700: annotate cruft-pack failure with ok=sigpipe
>
> One of our tests intentionally causes the cruft-pack generation phase of
> repack to fail, in order to stimulate an exit from repack at the desired
> moment. It does so by feeding a bogus option argument to pack-objects.
> This is a simple and reliable way to get pack-objects to fail, but it
> has one downside: pack-objects will die before reading its stdin, which
> means the caller repack may racily get SIGPIPE writing to it.
>
> For the purposes of this test, that's OK. We are checking whether repack
> cleans up already-created .tmp files, and it will do so whether it exits
> or dies by signal (because the tempfile API hooks both).
>
> But we have to tell test_must_fail that either outcome is OK, or it
> complains about the signal. Arguably this is a workaround (compared to
> fixing repack), as repack dying to SIGPIPE means that it loses the
> opportunity to give a more detailed message. But we don't actually write
> such a message anyway; we rely on pack-objects to have written something
> useful to stderr, and it does. In either case (signal or exit), that is
> the main thing the user will see.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  t/t7700-repack.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh
> index edcda849b9..9164acbe02 100755
> --- a/t/t7700-repack.sh
> +++ b/t/t7700-repack.sh
> @@ -433,7 +433,7 @@ test_expect_success TTY '--quiet disables progress' '
>  '
>  
>  test_expect_success 'clean up .tmp-* packs on error' '
> -	test_must_fail git \
> +	test_must_fail ok=sigpipe git \
>  		-c repack.cruftwindow=bogus \
>  		repack -ad --cruft &&
>  	find $objdir/pack -name '.tmp-*' >tmpfiles &&