On Fri, Oct 21, 2022 at 08:21:54PM -0400, Jeff King wrote:
> +test_expect_success 'clean up .tmp-* packs on error' '
> + test_must_fail git \
> + -c repack.cruftwindow=bogus \
> + repack -ad --cruft &&
> + find $objdir/pack -name '.tmp-*' >tmpfiles &&
> + test_must_be_empty tmpfiles
> +'
Ugh, I think this test is racy. I saw a failure via SIGPIPE on OS X in
CI, and running it locally with --stress confirmed. I think the problem
is in our method to trigger pack-objects to fail for the cruft pack. We
pass a bogus command line argument, so pack-objects exits more or less
immediately. But the parent git-repack process is trying to write to its
stdin (to name packs to keep/exclude). So that write may succeed or fail
based on how quickly the child dies.
Some options:
- find a different way to convince repack to die. The most likely is
probably corrupting the cruft objects in some way such that
pack-objects dies, but not until it starts doing real work.
- mark the test_must_fail with ok=sigpipe. In most case this is a
band-aid, but here we still test what we want. The .tmp cleanup
should kick in from a die() or from a signal death, so either is
sufficient for our purposes.
- teach git-repack to ignore sigpipe. We don't actually check the
writes to pack-objects (since they're done by fprintf), but we'd
notice its failing exit code. And arguably this is improving the
user experience. Saying "pack-objects died with an error" is more
useful than a sigpipe.
Thoughts?
-Peff
