From: Karthik Nayak Date: Tue, 04 Nov 2025 08:28:08 GMT Subject: Re: [PATCH 5/5] maintenance: add 'is-needed' subcommand Message-ID: In-Reply-To: Patrick Steinhardt writes: > On Mon, Nov 03, 2025 at 09:18:35AM -0800, Karthik Nayak wrote: >> Patrick Steinhardt 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..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. > >> >> 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. > > Patrick I think that's a fair conclusion. I'll leave this as is for now then. Thanks for the review.