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
