Thanks for fixing this!
> -----Original Message-----
> From: SZEDER Gábor [mailto:[email protected]]
> Sent: Thursday, April 13, 2017 6:32 AM
> To: Junio C Hamano <[email protected]>
> Cc: Jeff King <[email protected]>; Johannes Sixt <[email protected]>; David Turner
> <[email protected]>; [email protected]; SZEDER Gábor
> <[email protected]>
> Subject: [PATCHv2.1] t6500: wait for detached auto gc at the end of the test
> script
>
> The last test in 't6500-gc', 'background auto gc does not run if gc.log is
> present
> and recent but does if it is old', added in
> a831c06a2 (gc: ignore old gc.log files, 2017-02-10), may sporadically trigger
> an
> error message from the test harness:
>
> rm: cannot remove 'trash directory.t6500-gc/.git/objects': Directory not
> empty
>
> The test in question ends with executing an auto gc in the backround, which
> occasionally takes so long that it's still running when 'test_done' is about
> to
> remove the trash directory. This 'rm -rf $trash' in the foreground might race
> with the detached auto gc to create and delete files and directories, and gc
> might (re-)create a path that 'rm' already visited and removed, triggering the
> above error message when 'rm' attempts to remove its parent directory.
>
> Commit bb05510e5 (t5510: run auto-gc in the foreground, 2016-05-01) fixed the
> same problem in a different test script by simply disallowing background gc.
> Unfortunately, what worked there is not applicable here, because the purpose
> of this test is to check the behavior of a detached auto gc.
>
> Make sure that the test doesn't continue before the gc is finished in the
> background with a clever bit of shell trickery:
>
> - Open fd 9 in the shell, to be inherited by the background gc
> process, because our daemonize() only closes the standard fds 0,
> 1 and 2.
> - Duplicate this fd 9 to stdout.
> - Read 'git gc's stdout, and thus fd 9, through a command
> substitution. We don't actually care about gc's output, but this
> construct has two useful properties:
> - This read blocks until stdout or fd 9 are open. While stdout is
> closed after the main gc process creates the background process
> and exits, fd 9 remains open until the backround process exits.
> - The variable assignment from the command substitution gets its
> exit status from the command executed within the command
> substitution, i.e. a failing main gc process will cause the test
> to fail.
>
> Note, that this fd trickery doesn't work on Windows, because due to MSYS
> limitations the git process only inherits the standard fds 0, 1 and 2 from
> the shell.
> Luckily, it doesn't matter in this case, because on Windows daemonize() is
> basically a noop, thus 'git gc --auto' always runs in the foreground.
>
> And since we can now continue the test reliably after the detached gc
> finished,
> check that there is only a single packfile left at the end, i.e. that the
> detached gc
> actually did what it was supposed to do.
> Also add a comment at the end of the test script to warn developers of future
> tests about this issue of long running detached gc processes.
>
> Helped-by: Jeff King <[email protected]>
> Helped-by: Johannes Sixt <[email protected]>
> Signed-off-by: SZEDER Gábor <[email protected]>
> ---
>
> Updated subject line, but otherwise the same as v2.
>
> t/t6500-gc.sh | 18 +++++++++++++++++-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh index 08de2e8ab..cc7acd101 100755
> --- a/t/t6500-gc.sh
> +++ b/t/t6500-gc.sh
> @@ -67,6 +67,16 @@ test_expect_success 'auto gc with too many loose
> objects does not attempt to cre
> test_line_count = 2 new # There is one new pack and its .idx '
>
> +run_and_wait_for_auto_gc () {
> + # We read stdout from gc for the side effect of waiting until the
> + # background gc process exits, closing its fd 9. Furthermore, the
> + # variable assignment from a command substitution preserves the
> + # exit status of the main gc process.
> + # Note: this fd trickery doesn't work on Windows, but there is no
> + # need to, because on Win the auto gc always runs in the foreground.
> + doesnt_matter=$(git gc --auto 9>&1)
> +}
> +
> test_expect_success 'background auto gc does not run if gc.log is present and
> recent but does if it is old' '
> test_commit foo &&
> test_commit bar &&
> @@ -80,7 +90,13 @@ test_expect_success 'background auto gc does not run if
> gc.log is present and re
> test-chmtime =-345600 .git/gc.log &&
> test_must_fail git gc --auto &&
> test_config gc.logexpiry 2.days &&
> - git gc --auto
> + run_and_wait_for_auto_gc &&
> + ls .git/objects/pack/pack-*.pack >packs &&
> + test_line_count = 1 packs
> '
>
> +# DO NOT leave a detached auto gc process running near the end of the #
> +test script: it can run long enough in the background to racily #
> +interfere with the cleanup in 'test_done'.
> +
> test_done
> --
> 2.12.2.613.g9c5b79913
