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

[Patrick Steinhardt <ps@pks.im>]

On Thu, Aug 15, 2024 at 10:04:10AM -0400, Derrick Stolee wrote:
> On 8/13/24 3:17 AM, Patrick Steinhardt wrote:
> 
> > 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`.
> 
> Thank you for noticing this behavior, which is essentially an unintended
> regression from when the maintenance command was first introduced. It
> worked for most users because of the accidental detachment of the GC
> task, but now users can correctly customize their automatic maintenance
> to run in the background.
> 
> This was my oversight, as I was focused on scheduled maintenance as
> being the primary way that users would customize their maintenance tasks.
> Thank you for unifying the concepts.
> 
> I sprinkled in commentary, and most of it was just things I noticed
> while reading the series in order but then later patches or a careful
> read made my comments non-actionable.
> 
> This v1 looks good to me.

Thanks for your thorough review!

Patrick