Re: [PATCH 7/7] builtin/maintenance: fix auto-detach with non-standard tasks
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Aug 13, 2024, 13:19 UTC
- Message-ID
- <a02e21f1-b2a4-499a-b767-3426876d31be@gmail.com>
- In-Reply-To
- <ZrtKoVFci6cdvMS_@tanuki>
On 13/08/2024 12:59, Patrick Steinhardt wrote:
Show 22 quoted lines
> 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
Phillip
Show 14 quoted lines
> We now ask it to not > detach anymore, which fixes this. Furthermore, the default behaviour of > `git maintenance run` did not change either: it stays in the foreground > unless the `--detach` flag is given. So the thing you worry about is > actually getting fixed by this series :) > > What _does_ change though is when we run `git maintenance` via our > auto-maintenance framework. Here we now do detach the whole maintenance > process, instead of only git-gc(1). This logic is only being executed by > random commands (git-rebase, git-pull, git-commit etc), and I'd argue it > is the expected behaviour. > > Patrick >