Re: [PATCH v2 5/7] builtin/gc: add a `--detach` flag

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

Patrick Steinhardt <ps@pks.im> writes:

> +test_expect_success '--detach overrides gc.autoDetach=false' '
> +	test_when_finished "rm -rf repo" &&
> +	git init repo &&
> +	(
> +		cd repo &&
> +
> +		# Prepare the repository such that git-gc(1) ends up repacking.
> +		test_commit "$(test_oid blob17_1)" &&
> +		test_commit "$(test_oid blob17_2)" &&
> +		git config gc.autodetach false &&
> +		git config gc.auto 2 &&
> +
> +		cat >expect <<-EOF &&
> +		Auto packing the repository in background for optimum performance.
> +		See "git help gc" for manual housekeeping.
> +		EOF
> +		GIT_PROGRESS_DELAY=0 git gc --auto --detach 2>actual &&
> +		test_cmp expect actual
> +	)
> +'

If the gc/maintenance is going to background itself, it is possible
that it still is running, possibly with files under repo/.git/ open
and the process running in repo directory, when the test_when_finished
clean-up trap goes in effect?

I am wondering where this comes from:

  https://github.com/git/git/actions/runs/10408467351/job/28825980833#step:6:2000

where "rm -rf repo" dies with an unusual

  rm: can't remove 'repo/.git': Directory not empty

and my theory is that after "rm -rf" _thinks_ it removed everything
underneath, before it attempts to rmdir("repo/.git"), the repack
process in the background has created a new pack, and "rm -rf" does
not go back and try to create such a new cruft.

The most robust way to work around such a "race" is to wait for the
backgrounded process before cleaning up, or after seeing that the
message we use as a signal that the "gc" has backgrounded itself,
kill that backgrounded process before exiting the test and causing
the clean-up to trigger.