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

On 13/08/2024 12:59, Patrick Steinhardt wrote:

On Tue, Aug 13, 2024 at 12:29:47PM +0100, Phillip Wood wrote:

quoted

Hi Patrick

On 13/08/2024 08:18, Patrick Steinhardt wrote:

quoted

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

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