Re: [PATCH 5/5] maintenance: add 'is-needed' subcommand
- From
Karthik Nayak <karthik.188@gmail.com>
- Date
- Nov 4, 2025, 08:28 UTC
- Message-ID
- <CAOLa=ZRA33ro1-9jbh71QpAa3Sj-NZY5fOL_T4Shyn8jPYQi_A@mail.gmail.com>
- In-Reply-To
- <aQmU_hOPO55_ojw2@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
Show 29 quoted lines
> On Mon, Nov 03, 2025 at 09:18:35AM -0800, Karthik Nayak wrote: >> Patrick Steinhardt <ps@pks.im> writes: >> >> > On Fri, Oct 31, 2025 at 03:22:25PM +0100, Karthik Nayak wrote: >> >> diff --git a/Documentation/git-maintenance.adoc b/Documentation/git-maintenance.adoc >> >> index 540b5cf68b..edcc88f4d0 100644 >> >> --- a/Documentation/git-maintenance.adoc >> >> +++ b/Documentation/git-maintenance.adoc >> >> @@ -84,6 +85,11 @@ The `unregister` subcommand will report an error if the current repository >> >> is not already registered. Use the `--force` option to return success even >> >> when the current repository is not registered. >> >> >> >> +is-needed:: >> >> + Check whether maintenance needs to be run without actually running it. >> >> + Exits with a 0 status code if maintenance needs to be run, 1 otherwise. >> >> + Can be used along with `--task`. Ideally should be used with '--auto'. >> > >> > Okay. I assume when `--task` is not given we'll check all tasks >> > specified by the configured strategy? Might make sense to document if >> > so. >> > >> >> Actually no. It's similar to the 'run' command, if nothing is specified, >> we check `maintenance.<task>.enabled`. By default it is only enabled for >> 'gc'. This is important information, I will add it in. > > But we use `initialize_task_config()`, and that function knows to use > the configured strategy unless it's given an explicit list of tasks. So > we do use the maintenance strategy.
Yes, and the default strategy is to run 'gc'.
I miss-read your earlier comment, you were talking about the configured strategy. I thought you were asking if no '--task' is given, we'd run all available tasks.
Show 37 quoted lines
>
>> >> diff --git a/builtin/gc.c b/builtin/gc.c
>> >> index 72177305ff..4d20487ed6 100644
>> >> --- a/builtin/gc.c
>> >> +++ b/builtin/gc.c
> [snip]
>> >> + } else {
>> >> + /* When not using --auto, we should always require maintenance. */
>> >> + is_needed = true;
>> >> + }
>> >
>> > I guess for now this is good enough, but it's not quite true. Some tasks
>> > won't require maintenance even without `--auto`, like for example when
>> > the reftable stack only has a single table.
>> >
>> > Patrick
>>
>> Good point. Thought I'm not sure how we'd go about it. Initially I
>> wanted to not have an `--auto` flag and simply make it the default
>> behavior. But that would restrict us from introducing the `schedule`
>> flag in the future. Which I think might be a worthwhile addition.
>
> Yeah, agreed.
>
> I guess eventually we could extend `auto_condition()` to honor the
> "--auto" flag:
>
> - If it's set the task verifies that it needs to trigger housekeeping
> tasks with heuristics.
>
> - Otherwise it checks whether there even is anything that could be
> cleaned up.
>
> But that's certainly out of scope of this patch series, I think it's
> good enough to bail on that specific part for now.
>
> PatrickI think that's a fair conclusion. I'll leave this as is for now then.
Thanks for the review.