Re: [PATCH 0/7] builtin/maintenance: fix auto-detach with non-standard tasks

James Liu
2024-08-15T06:43:00Z

On Tue Aug 13, 2024 at 5:17 PM AEST, Patrick Steinhardt wrote:
> Hi,
>
> I recently configured git-maintenance(1) to not use git-gc(1) anymore,
> but instead to use git-multi-pack-index(1). I quickly noticed that the
> behaviour here is somewhat broken because instead of auto-detaching when
> `git maintenance run --auto` executes, we wait for the process to run to
> completion.
>
> The root cause is that git-maintenance(1), probably by accident,
> continues to rely on the auto-detaching mechanism in git-gc(1). So
> instead of having git-maintenance(1) detach, it is git-gc(1) that
> detaches and thus causes git-maintenance(1) to exit early. That of
> course falls flat once any maintenance task other than git-gc(1)
> executes, because these won't detach.
>
> Despite being a usability issue, this may also cause git-gc(1) to run
> concurrently with any other enabled maintenance tasks. This shouldn't
> lead to data loss, but it can certainly lead to processes stomping on
> each others feet.
>
> This patch series fixes this by wiring up new `--detach` flags for both
> git-gc(1) and git-maintenance(1). Like this, git-maintenance(1) now
> knows to execute `git gc --auto --no-detach`, while our auto-maintenance
> will execute `git mainteance run --auto --detach`.
>
> Patrick
>
> Patrick Steinhardt (7):
> config: fix constness of out parameter for `git_config_get_expiry()`
> builtin/gc: refactor to read config into structure
> builtin/gc: fix leaking config values
> builtin/gc: stop processing log file on signal
> builtin/gc: add a `--detach` flag
> builtin/maintenance: add a `--detach` flag
> builtin/maintenance: fix auto-detach with non-standard tasks
>
> Documentation/git-gc.txt | 5 +-
> builtin/gc.c | 384 ++++++++++++++++++++++++---------------
> config.c | 4 +-
> config.h | 2 +-
> read-cache.c | 12 +-
> run-command.c | 12 +-
> t/t5304-prune.sh | 1 +
> t/t5616-partial-clone.sh | 6 +-
> t/t6500-gc.sh | 39 ++++
> t/t7900-maintenance.sh | 82 ++++++++-
> 10 files changed, 381 insertions(+), 166 deletions(-)

Thanks Patrick! I've left a few replies, mostly non-blocking questions.

Cheers,
James