Re: [PATCH 7/7] builtin/maintenance: fix auto-detach with non-standard tasks
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Aug 14, 2024, 04:15 UTC
- Message-ID
- <ZrwvWIhjZuFkRNgl@tanuki>
- In-Reply-To
- <a02e21f1-b2a4-499a-b767-3426876d31be@gmail.com>
On Tue, Aug 13, 2024 at 02:19:20PM +0100, Phillip Wood wrote:
Show 32 quoted lines
> On 13/08/2024 12:59, Patrick Steinhardt wrote: > > On Tue, Aug 13, 2024 at 12:29:47PM +0100, Phillip Wood wrote: > > > Hi Patrick > > > > > > On 13/08/2024 08:18, Patrick Steinhardt wrote: > > > > > > > > Fix this bug by asking git-gc(1) to not detach when it is being invoked > > > > via git-maintenance(1). Instead, the latter command now respects a new > > > > config "maintenance.autoDetach", the equivalent of "gc.autoDetach", and > > > > detaches itself into the background if not told otherwise. This should > > > > continue to behave the same for all users which use the git-gc(1) task, > > > > only. For others though, it means that we now properly perform all tasks > > > > in the background. > > > > > > I fear that users who are running "git maintenance" from a scheduler such as > > > cron are likely to be surprised by this change in behavior. At the very > > > least "git maintenance" will no-longer return a meaningful exit code. > > > Perhaps we could switch the logic to be opt in and pass "--detach" (or "-c > > > maintenance.autoDetach=true") when running "git maintenance" automatically > > > from "git rebase" etc. > > > > It's actually the reverse: the old behaviour when run via a scheduler > > was to detach by default, because git-gc(1) did. > > Oh, I misunderstood what this patch is changing. So despite being tagged > builtin/maintenance and talking about "git maintenance" it does not actually > touch builtin/maintenance.c or change its behavior. What it is actually > doing is changing how other git commands run "git maintenance --auto" so > that it is always run in the background unless the user configures > maintenance.autoDetach=false. That sounds like a good change. > > Thanks for clarifying
Yes. I should've probably prefixed this with "run-command:", not with "builtin/maintenance".
Patrick