Re: [PATCH v3 6/7] builtin/maintenance: add a `--detach` flag

Jeff King
2024-08-17T07:14:37Z

On Sat, Aug 17, 2024 at 03:09:25AM -0400, Jeff King wrote:

> While investigating that, I ran it with --stress locally (on Linux) and
> got some odd (and definitely racy) results. The test itself passes, but
> the "rm -rf repo" in the test_when_finished sometimes fails with:
>
> rm: cannot remove 'repo/.git/objects': Directory not empty
>
> or similar (sometimes it's another directory like 'repo/.git'). My guess
> is that the background process is still running and creating files in
> the repository, racing with rm's call to rmdir().
>
> Even if we remove the test_when_finished, it would mean that the final
> cleanup after test_done might similarly fail, leaving a crufty trash
> directory. I think to make this robust, we'd need some way of detecting
> when the background process has finished. I don't think we report the
> pid anywhere, and the daemonize() call means it won't even be in the
> same process group. Maybe we could spin looking for the incremental pack
> it will create (and timeout after N seconds)? That feels pretty hacky,
> but I can't think of anything better.

Ah, I just noticed that a similar problem happened in t6500, discussed
during v2 of the series (I only looked at the latest version). I think
t7900 needs a similar run_and_wait_for_auto_gc() solution.

I suspect the Windows "out is not empty" issue is separate, though.

-Peff