Web lists-archives.com

Re: [PATCH] t6500: don't run detached auto gc at the end of the test script




On Mon, Apr 10, 2017 at 3:58 PM, Jeff King <peff@xxxxxxxx> wrote:
> On Mon, Apr 10, 2017 at 02:59:11PM +0200, SZEDER Gábor wrote:
>
>> While this change doesn't completely eliminate the possibility of
>> this race, it significantly and seemingly sufficiently reduces its
>> probability.  Running t6500 in a loop while my box was under heavy CPU
>> and I/O load usually triggered the error under 15 seconds, but with
>> this patch it run overnight without incident.
>>
>> (Alternatively, we could check the presence of the gc.pid file after
>> starting the detached auto gc, and wait while it's there.  However,
>> this would create a different race: the auto gc process creates the
>> pid file after it goes to the background, so our check in the
>> foreground might racily look for the pid file before it's even
>> created, and thus would not wait for the background gc to finish.
>> Furthermore, this would open up the possibility of the test hanging if
>> something were to go wrong with the gc process and the pid file were
>> not removed.)
>
> Could we just leave open a pipe descriptor that the child inherits, and
> wait for it to close?
>
> Something like:
>
>   git gc --auto 9>&1 | read
>
> should wait until the background gc process finishes. It depends on our
> daemonize() not closing descriptors beyond 0/1/2, but that is certainly
> the case now.
>
> It also loses the exit status of the main "git gc", but that can be
> fixed with shell hackery:
>
>   code=$(sh -c 'git gc --auto; echo $?' 9>&1)
>   test "$code" = 0

Indeed this seems to work, and luckily we don't need that much
hackery.  When there is a single variable assignment and the expansion
of a command substitution is assigned to the variable, then the exit
status is that of the command inside the command substitution, i.e.

  $ v=$(false) ; echo $?
  1

This means we can write this simply as:

  doesnt_matter=$(git gc --auto 9>&1)

It's still hackery :)

OTOH, this makes it possible to continue the test reliably after the
gc finished in the background, so we could also check that there is
only a single pack file left, i.e. that the detached gc did what it
was supposed to do.