[PATCH 0/5] maintenance: add an 'is-needed' subcommand

[PATCH 0/5] maintenance: add an 'is-needed' subcommand

[Karthik Nayak]

Hello,

I recently raised a patch series [1] to add 'git refs optimize --required'
which checks if the reference backend can be optimized, without actually
performing the optimization.

Back then, we had decided [2] that it would be a better to broaden the
approach and add a 'is-needed' subcommand to 'git-maintenance(1)'. This
would allow users to check if maintenance was required for the
repository and users could also provide a task via the '--task' to check
if maintenance was needed for a particular task.

Ideally the subcommand will be used with the '--auto' flag which can
check the same heuristics as that used with 'git maintenance run
--auto'. Future patches can also add support for the '--schedule' flag
which can be used to check required schedule it met. However that flag
isn't added as part of this series.

This series implements that.

Commits 1-3 add the required functionality in the refs subsystem to
expose an 'optimize_required' field which can be used to check if
backends need to be optimized.
Commit 4 utilizes this within the 'git-maintenance(1)' code.
Commit 5 adds the 'is-needed' subcommand to 'git-maintenance(1)'.

This is based on top of master a99f379adf (The 27th batch, 2025-10-30)
and is dependent on the following series:

    - kn/refs-optim-cleanup
    - ps/ref-peeled-tags

Merges cleanly with `next`. I think those two topics are close to being
merged to `next` so hopefully this dependency tree doesn't get too
complicated. I'll rebase as needed to resolve conflicts.

[1]: https://lore.kernel.org/git/20251010-562-add-option-to-check-if-reference-backend-needs-repacking-v1-0-c7962be584fa@gmail.com/
[2]: https://lore.kernel.org/git/CAOLa=ZRdxm787nE4FSr2VUHDB+hW06Ggc6yUcKmeTKAb6B7YOA@mail.gmail.com/

---
 Documentation/git-maintenance.adoc |  8 ++++
 builtin/gc.c                       | 86 +++++++++++++++++++++++++++++++++-----
 object.h                           |  1 -
 refs.c                             |  7 ++++
 refs.h                             |  7 ++++
 refs/debug.c                       | 13 ++++++
 refs/files-backend.c               | 11 +++++
 refs/packed-backend.c              | 13 ++++++
 refs/refs-internal.h               |  6 +++
 refs/reftable-backend.c            | 25 +++++++++++
 reftable/reftable-stack.h          |  5 +++
 reftable/stack.c                   | 48 ++++++++++++++++-----
 t/t7900-maintenance.sh             | 54 +++++++++++++++++-------
 t/unit-tests/u-reftable-stack.c    | 12 +++++-
 14 files changed, 256 insertions(+), 40 deletions(-)

Karthik Nayak (5):
      reftable/stack: return stack segments directly
      reftable/stack: add function to check if optimization is required
      refs: add a `optimize_required` field to `struct ref_storage_be`
      maintenance: add checking logic in `pack_refs_condition()`
      maintenance: add 'is-needed' subcommand



base-commit: edd2018f5db39d68d55a7a4af42375b1a06b9406
change-id: 20251021-562-add-sub-command-to-check-if-maintenance-is-needed-01cae01b4606

Thanks
- Karthik

[PATCH 1/5] reftable/stack: return stack segments directly

[Karthik Nayak]

The `stack_table_sizes_for_compaction()` function returns individual
sizes of each reftable table. This function is only called by
`reftable_stack_auto_compact()` to decide which tables need to be
compacted, if any.

Modify the function to directly return the segments, which avoids the
extra step of receiving the sizes only to pass it to
`suggest_compaction_segment()`.

A future commit will also add functionality for checking whether
auto-compaction is necessary without performing it. This change allows
code re-usability in that context.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 reftable/stack.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/reftable/stack.c b/reftable/stack.c
index 65d89820bd..49387f9344 100644
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -1626,7 +1626,8 @@ struct segment suggest_compaction_segment(uint64_t *sizes, size_t n,
 	return seg;
 }
 
-static uint64_t *stack_table_sizes_for_compaction(struct reftable_stack *st)
+static int stack_segments_for_compaction(struct reftable_stack *st,
+					 struct segment *seg)
 {
 	int version = (st->opts.hash_id == REFTABLE_HASH_SHA1) ? 1 : 2;
 	int overhead = header_size(version) - 1;
@@ -1634,29 +1635,29 @@ static uint64_t *stack_table_sizes_for_compaction(struct reftable_stack *st)
 
 	REFTABLE_CALLOC_ARRAY(sizes, st->merged->tables_len);
 	if (!sizes)
-		return NULL;
+		return REFTABLE_OUT_OF_MEMORY_ERROR;
 
 	for (size_t i = 0; i < st->merged->tables_len; i++)
 		sizes[i] = st->tables[i]->size - overhead;
 
-	return sizes;
+	*seg = suggest_compaction_segment(sizes, st->merged->tables_len,
+					  st->opts.auto_compaction_factor);
+	reftable_free(sizes);
+
+	return 0;
 }
 
 int reftable_stack_auto_compact(struct reftable_stack *st)
 {
 	struct segment seg;
-	uint64_t *sizes;
+	int err;
 
 	if (st->merged->tables_len < 2)
 		return 0;
 
-	sizes = stack_table_sizes_for_compaction(st);
-	if (!sizes)
-		return REFTABLE_OUT_OF_MEMORY_ERROR;
-
-	seg = suggest_compaction_segment(sizes, st->merged->tables_len,
-					 st->opts.auto_compaction_factor);
-	reftable_free(sizes);
+	err = stack_segments_for_compaction(st, &seg);
+	if (err)
+		return err;
 
 	if (segment_size(&seg) > 0)
 		return stack_compact_range(st, seg.start, seg.end - 1,

-- 
2.51.0

Re: [PATCH 1/5] reftable/stack: return stack segments directly

[Justin Tobler]

On 25/10/31 03:22PM, Karthik Nayak wrote:
> The `stack_table_sizes_for_compaction()` function returns individual
> sizes of each reftable table. This function is only called by
> `reftable_stack_auto_compact()` to decide which tables need to be
> compacted, if any.

`stack_table_sizes_for_compaction()` provides the sizes of tables which
gets used by `suggest_compaction_segment()` to figure out the range of
tables that need to be compacted in order to restore the geometric
sequence. `reftable_stack_auto_compact()` coordinates invoking these two
functions and actually performs the compaction via
`stack_compact_range()`.

> Modify the function to directly return the segments, which avoids the
> extra step of receiving the sizes only to pass it to
> `suggest_compaction_segment()`.

Ok, so we want `suggest_compaction_segment()` to be invoked by
`stack_table_sizes_for_compaction()` instead of
`reftable_stack_auto_compact()`. So we are not really avoiding this
step, but just changing where it occurs.

> A future commit will also add functionality for checking whether
> auto-compaction is necessary without performing it. This change allows
> code re-usability in that context.

Makes sense.

> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
>  reftable/stack.c | 23 ++++++++++++-----------
>  1 file changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/reftable/stack.c b/reftable/stack.c
> index 65d89820bd..49387f9344 100644
> --- a/reftable/stack.c
> +++ b/reftable/stack.c
> @@ -1626,7 +1626,8 @@ struct segment suggest_compaction_segment(uint64_t *sizes, size_t n,
>  	return seg;
>  }
>  
> -static uint64_t *stack_table_sizes_for_compaction(struct reftable_stack *st)
> +static int stack_segments_for_compaction(struct reftable_stack *st,
> +					 struct segment *seg)

`stack_segements_for_compaction()` now handles both getting the table
sizes and getting the segment range for compaction.

>  {
>  	int version = (st->opts.hash_id == REFTABLE_HASH_SHA1) ? 1 : 2;
>  	int overhead = header_size(version) - 1;
> @@ -1634,29 +1635,29 @@ static uint64_t *stack_table_sizes_for_compaction(struct reftable_stack *st)
>  
>  	REFTABLE_CALLOC_ARRAY(sizes, st->merged->tables_len);
>  	if (!sizes)
> -		return NULL;
> +		return REFTABLE_OUT_OF_MEMORY_ERROR;
>  
>  	for (size_t i = 0; i < st->merged->tables_len; i++)
>  		sizes[i] = st->tables[i]->size - overhead;
>  
> -	return sizes;
> +	*seg = suggest_compaction_segment(sizes, st->merged->tables_len,
> +					  st->opts.auto_compaction_factor);
> +	reftable_free(sizes);
> +
> +	return 0;
>  }
>  
>  int reftable_stack_auto_compact(struct reftable_stack *st)
>  {
>  	struct segment seg;
> -	uint64_t *sizes;
> +	int err;
>  
>  	if (st->merged->tables_len < 2)
>  		return 0;
>  
> -	sizes = stack_table_sizes_for_compaction(st);
> -	if (!sizes)
> -		return REFTABLE_OUT_OF_MEMORY_ERROR;
> -
> -	seg = suggest_compaction_segment(sizes, st->merged->tables_len,
> -					 st->opts.auto_compaction_factor);
> -	reftable_free(sizes);
> +	err = stack_segments_for_compaction(st, &seg);
> +	if (err)
> +		return err;

Looks good.

>  
>  	if (segment_size(&seg) > 0)
>  		return stack_compact_range(st, seg.start, seg.end - 1,

Do we expect the errors returned by `stack_segments_for_compaction()` to
always be negative? If so, I wonder if we should also have it return the
number of tables in the segment. That way it could also handle the
followup `segment_size()`.

-Justin

Re: [PATCH 1/5] reftable/stack: return stack segments directly

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 1278 bytes --]

Justin Tobler <jltobler@gmail.com> writes:

[snip]

>>
>>  	if (segment_size(&seg) > 0)
>>  		return stack_compact_range(st, seg.start, seg.end - 1,
>
> Do we expect the errors returned by `stack_segments_for_compaction()` to
> always be negative? If so, I wonder if we should also have it return the
> number of tables in the segment. That way it could also handle the
> followup `segment_size()`.
>

Currently yes, since all 'REFTABLE_<error>' errors return negative
value. But I must say I'm not a fan of combining errors and values
together in a single return. This only creates confusion.

I'm not sure removing `segment_size()` is also a good idea, because it
describes what the check is. Otherwise we're looking at something like:

@@ -1655,11 +1646,10 @@ int reftable_stack_auto_compact(struct
reftable_stack *st)
 	if (st->merged->tables_len < 2)
 		return 0;

-	err = stack_segments_for_compaction(st, &seg);
-	if (err)
+	err_or_stack_size = stack_segments_for_compaction(st, &seg);
+	if (err_or_stack_size < 0)
 		return err;
-
-	if (segment_size(&seg) > 0)
+	else if (err_or_stack_size > 0)
 		return stack_compact_range(st, seg.start, seg.end - 1,
 					   NULL, STACK_COMPACT_RANGE_BEST_EFFORT);

I'm not sure that this would be better? Or am I missing something?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH 1/5] reftable/stack: return stack segments directly

[Justin Tobler]

On 25/11/03 07:05AM, Karthik Nayak wrote:
> Justin Tobler <jltobler@gmail.com> writes:
> 
> [snip]
> 
> >>
> >>  	if (segment_size(&seg) > 0)
> >>  		return stack_compact_range(st, seg.start, seg.end - 1,
> >
> > Do we expect the errors returned by `stack_segments_for_compaction()` to
> > always be negative? If so, I wonder if we should also have it return the
> > number of tables in the segment. That way it could also handle the
> > followup `segment_size()`.
> >
> 
> Currently yes, since all 'REFTABLE_<error>' errors return negative
> value. But I must say I'm not a fan of combining errors and values
> together in a single return. This only creates confusion.
> 
> I'm not sure removing `segment_size()` is also a good idea, because it
> describes what the check is. Otherwise we're looking at something like:
> 
> @@ -1655,11 +1646,10 @@ int reftable_stack_auto_compact(struct
> reftable_stack *st)
>  	if (st->merged->tables_len < 2)
>  		return 0;
> 
> -	err = stack_segments_for_compaction(st, &seg);
> -	if (err)
> +	err_or_stack_size = stack_segments_for_compaction(st, &seg);
> +	if (err_or_stack_size < 0)
>  		return err;
> -
> -	if (segment_size(&seg) > 0)
> +	else if (err_or_stack_size > 0)
>  		return stack_compact_range(st, seg.start, seg.end - 1,
>  					   NULL, STACK_COMPACT_RANGE_BEST_EFFORT);
> 
> I'm not sure that this would be better? Or am I missing something?

That's fair. I was thinking that we could just make
`stack_segments_for_compaction()` responsible for the boolean check of
whether compaction is required or not. It's probably not worth
overloading the return value though.

-Justin

[PATCH 2/5] reftable/stack: add function to check if optimization is required

[Karthik Nayak]

The reftable backend, performs auto-compaction as part of its regular
flow, which is required to keep the number of tables part of a stack at
bay. This allows it to stay optimized.

Compaction can also be triggered voluntarily by the user via the 'git
pack-refs' or the 'git refs optimize' command. However, currently there
is no way for the user to check if optimization is required without
actually performing it.

Add and expose `reftable_stack_compaction_required()` which will allow
users to check if the reftable backend can be optimized.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 reftable/reftable-stack.h       |  5 +++++
 reftable/stack.c                | 25 +++++++++++++++++++++++++
 t/unit-tests/u-reftable-stack.c | 12 ++++++++++--
 3 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/reftable/reftable-stack.h b/reftable/reftable-stack.h
index d70fcb705d..a875149439 100644
--- a/reftable/reftable-stack.h
+++ b/reftable/reftable-stack.h
@@ -123,6 +123,11 @@ struct reftable_log_expiry_config {
 int reftable_stack_compact_all(struct reftable_stack *st,
 			       struct reftable_log_expiry_config *config);
 
+/* Check if compaction is required. */
+int reftable_stack_compaction_required(struct reftable_stack *st,
+				       bool use_heuristics,
+				       bool *required);
+
 /* heuristically compact unbalanced table stack. */
 int reftable_stack_auto_compact(struct reftable_stack *st);
 
diff --git a/reftable/stack.c b/reftable/stack.c
index 49387f9344..18fa41cd5c 100644
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -1647,6 +1647,31 @@ static int stack_segments_for_compaction(struct reftable_stack *st,
 	return 0;
 }
 
+int reftable_stack_compaction_required(struct reftable_stack *st,
+				       bool use_heuristics,
+				       bool *required)
+{
+	struct segment seg;
+	int err = 0;
+
+	if (st->merged->tables_len < 2) {
+		*required = false;
+		return 0;
+	}
+
+	if (!use_heuristics) {
+		*required = true;
+		return 0;
+	}
+
+	err = stack_segments_for_compaction(st, &seg);
+	if (err)
+		return err;
+
+	*required = segment_size(&seg) > 0;
+	return 0;
+}
+
 int reftable_stack_auto_compact(struct reftable_stack *st)
 {
 	struct segment seg;
diff --git a/t/unit-tests/u-reftable-stack.c b/t/unit-tests/u-reftable-stack.c
index a8b91812e8..b8110cdeee 100644
--- a/t/unit-tests/u-reftable-stack.c
+++ b/t/unit-tests/u-reftable-stack.c
@@ -1067,6 +1067,7 @@ void test_reftable_stack__add_performs_auto_compaction(void)
 			.value_type = REFTABLE_REF_SYMREF,
 			.value.symref = (char *) "master",
 		};
+		bool required = false;
 		char buf[128];
 
 		/*
@@ -1087,10 +1088,17 @@ void test_reftable_stack__add_performs_auto_compaction(void)
 		 * auto compaction is disabled. When enabled, we should merge
 		 * all tables in the stack.
 		 */
-		if (i != n)
+		cl_assert_equal_i(reftable_stack_compaction_required(st, true, &required), 0);
+		if (i != n) {
 			cl_assert_equal_i(st->merged->tables_len, i + 1);
-		else
+			if (i < 1)
+				cl_assert_equal_b(required, false);
+			else
+				cl_assert_equal_b(required, true);
+		} else {
 			cl_assert_equal_i(st->merged->tables_len, 1);
+			cl_assert_equal_b(required, false);
+		}
 	}
 
 	reftable_stack_destroy(st);

-- 
2.51.0

Re: [PATCH 2/5] reftable/stack: add function to check if optimization is required

[Justin Tobler]

On 25/10/31 03:22PM, Karthik Nayak wrote:
> The reftable backend, performs auto-compaction as part of its regular
> flow, which is required to keep the number of tables part of a stack at
> bay. This allows it to stay optimized.
> 
> Compaction can also be triggered voluntarily by the user via the 'git
> pack-refs' or the 'git refs optimize' command. However, currently there
> is no way for the user to check if optimization is required without
> actually performing it.
> 
> Add and expose `reftable_stack_compaction_required()` which will allow
> users to check if the reftable backend can be optimized.
> 
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
>  reftable/reftable-stack.h       |  5 +++++
>  reftable/stack.c                | 25 +++++++++++++++++++++++++
>  t/unit-tests/u-reftable-stack.c | 12 ++++++++++--
>  3 files changed, 40 insertions(+), 2 deletions(-)
> 
> diff --git a/reftable/reftable-stack.h b/reftable/reftable-stack.h
> index d70fcb705d..a875149439 100644
> --- a/reftable/reftable-stack.h
> +++ b/reftable/reftable-stack.h
> @@ -123,6 +123,11 @@ struct reftable_log_expiry_config {
>  int reftable_stack_compact_all(struct reftable_stack *st,
>  			       struct reftable_log_expiry_config *config);
>  
> +/* Check if compaction is required. */
> +int reftable_stack_compaction_required(struct reftable_stack *st,
> +				       bool use_heuristics,
> +				       bool *required);
> +
>  /* heuristically compact unbalanced table stack. */
>  int reftable_stack_auto_compact(struct reftable_stack *st);
>  
> diff --git a/reftable/stack.c b/reftable/stack.c
> index 49387f9344..18fa41cd5c 100644
> --- a/reftable/stack.c
> +++ b/reftable/stack.c
> @@ -1647,6 +1647,31 @@ static int stack_segments_for_compaction(struct reftable_stack *st,
>  	return 0;
>  }
>  
> +int reftable_stack_compaction_required(struct reftable_stack *st,
> +				       bool use_heuristics,
> +				       bool *required)
> +{
> +	struct segment seg;
> +	int err = 0;
> +
> +	if (st->merged->tables_len < 2) {
> +		*required = false;
> +		return 0;
> +	}

Both `reftable_stack_auto_compact()` and `suggest_compaction_segement()`
already check if the stack has less than two tables. I wonder if we can
avoid having multiple of these checks by instead having a single one at
the start of `stack_segements_for_compaction()`?

> +	if (!use_heuristics) {
> +		*required = true;
> +		return 0;
> +	}

Is there a reason we would want to skip validating the geometric
sequence and just assume it compaction is required?

> +
> +	err = stack_segments_for_compaction(st, &seg);
> +	if (err)
> +		return err;
> +
> +	*required = segment_size(&seg) > 0;

As mentioned on the previous patch, I wonder if we could just return the
number of tables in the compaction segment as part of
`stack_segments_for_compaction()`. A negative value could indicate an
error. All other values would reflect the number of tables to be
compacted.

This way callers interested in whether compaction should be performed
could just do: stack_segments_for_compaction > 0. We could maybe avoid
having a separate function like we do here and just expose
`stack_segments_for_compaction()`.

-Justin

Re: [PATCH 2/5] reftable/stack: add function to check if optimization is required

[Junio C Hamano]

Justin Tobler <jltobler@gmail.com> writes:

>> +	err = stack_segments_for_compaction(st, &seg);
>> +	if (err)
>> +		return err;
>> +
>> +	*required = segment_size(&seg) > 0;
>
> As mentioned on the previous patch, I wonder if we could just return the
> number of tables in the compaction segment as part of
> `stack_segments_for_compaction()`. A negative value could indicate an
> error. All other values would reflect the number of tables to be
> compacted.
>
> This way callers interested in whether compaction should be performed
> could just do: stack_segments_for_compaction > 0. We could maybe avoid
> having a separate function like we do here and just expose
> `stack_segments_for_compaction()`.

Is the cost of compacting a single table expected to be roughly the
same across tables?  The number of tables to be compacted would not
be a useful information to help making a better decision otherwise,
so I am guessing that it is the underlying assumption the above
suggestion comes from.

Re: [PATCH 2/5] reftable/stack: add function to check if optimization is required

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 1195 bytes --]

Junio C Hamano <gitster@pobox.com> writes:

> Justin Tobler <jltobler@gmail.com> writes:
>
>>> +	err = stack_segments_for_compaction(st, &seg);
>>> +	if (err)
>>> +		return err;
>>> +
>>> +	*required = segment_size(&seg) > 0;
>>
>> As mentioned on the previous patch, I wonder if we could just return the
>> number of tables in the compaction segment as part of
>> `stack_segments_for_compaction()`. A negative value could indicate an
>> error. All other values would reflect the number of tables to be
>> compacted.
>>
>> This way callers interested in whether compaction should be performed
>> could just do: stack_segments_for_compaction > 0. We could maybe avoid
>> having a separate function like we do here and just expose
>> `stack_segments_for_compaction()`.
>
> Is the cost of compacting a single table expected to be roughly the
> same across tables?  The number of tables to be compacted would not
> be a useful information to help making a better decision otherwise,
> so I am guessing that it is the underlying assumption the above
> suggestion comes from.

It would be sufficient information to know whether or not we can
compact. But only when in 'git refs optimize --auto' mode.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH 2/5] reftable/stack: add function to check if optimization is required

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 2286 bytes --]

Justin Tobler <jltobler@gmail.com> writes:

>> +int reftable_stack_compaction_required(struct reftable_stack *st,
>> +				       bool use_heuristics,
>> +				       bool *required)
>> +{
>> +	struct segment seg;
>> +	int err = 0;
>> +
>> +	if (st->merged->tables_len < 2) {
>> +		*required = false;
>> +		return 0;
>> +	}
>
> Both `reftable_stack_auto_compact()` and `suggest_compaction_segement()`
> already check if the stack has less than two tables. I wonder if we can
> avoid having multiple of these checks by instead having a single one at
> the start of `stack_segements_for_compaction()`?
>

Well we can't for two reasons:
1. We want to perform this check independent of whether `use_heuristics`
   is set or not.
2. Currently `stack_segements_for_compaction()` does one thing only,
   which is stack the segments. I wouldn't want to introduce another
   responsibility to it.

>> +	if (!use_heuristics) {
>> +		*required = true;
>> +		return 0;
>> +	}
>
> Is there a reason we would want to skip validating the geometric
> sequence and just assume it compaction is required?
>

This is the difference between running 'git refs optimize' with and
without '--auto'. With '--auto' we will use heuristics to do a geometric
progression. Without, we simply compact all tables into one.

So we need to support both modes here.
>> +
>> +	err = stack_segments_for_compaction(st, &seg);
>> +	if (err)
>> +		return err;
>> +
>> +	*required = segment_size(&seg) > 0;
>
> As mentioned on the previous patch, I wonder if we could just return the
> number of tables in the compaction segment as part of
> `stack_segments_for_compaction()`. A negative value could indicate an
> error. All other values would reflect the number of tables to be
> compacted.
>
> This way callers interested in whether compaction should be performed
> could just do: stack_segments_for_compaction > 0. We could maybe avoid
> having a separate function like we do here and just expose
> `stack_segments_for_compaction()`.
>

We'd still need to expose a new function as
`stack_segments_for_compaction()` is still internal details to the
reftable backend, which we wouldn't want to expose externally. Users of
this function, should only need to know a boolean value wether the
backend needs to be optimized or not.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH 2/5] reftable/stack: add function to check if optimization is required

[Justin Tobler]

On 25/11/03 07:51AM, Karthik Nayak wrote:
> Justin Tobler <jltobler@gmail.com> writes:
> 
> >> +int reftable_stack_compaction_required(struct reftable_stack *st,
> >> +				       bool use_heuristics,
> >> +				       bool *required)
> >> +{
> >> +	struct segment seg;
> >> +	int err = 0;
> >> +
> >> +	if (st->merged->tables_len < 2) {
> >> +		*required = false;
> >> +		return 0;
> >> +	}
> >
> > Both `reftable_stack_auto_compact()` and `suggest_compaction_segement()`
> > already check if the stack has less than two tables. I wonder if we can
> > avoid having multiple of these checks by instead having a single one at
> > the start of `stack_segements_for_compaction()`?
> >
> 
> Well we can't for two reasons:
> 1. We want to perform this check independent of whether `use_heuristics`
>    is set or not.
> 2. Currently `stack_segements_for_compaction()` does one thing only,
>    which is stack the segments. I wouldn't want to introduce another
>    responsibility to it.

That's fair. From my understanding, `stack_segements_for_compaction()`
populates a segment which defines the range of tables that should be
compacted to restore the geometric sequence. Since we want to ultimately
know whether compaction needs to occur, my thought process was we could
maybe have a single function ("check_compaction_needed()") that
effectively returns a boolean and maybe be able to reuse that. I don't
think it matters much though and as you mention we also want to consider
`use_heuristics`.

> >> +	if (!use_heuristics) {
> >> +		*required = true;
> >> +		return 0;
> >> +	}
> >
> > Is there a reason we would want to skip validating the geometric
> > sequence and just assume it compaction is required?
> >
> 
> This is the difference between running 'git refs optimize' with and
> without '--auto'. With '--auto' we will use heuristics to do a geometric
> progression. Without, we simply compact all tables into one.

That's for the clarification. So without --auto, instead of following a
geometric sequence, a different maintenance strategy is used and we
compact all the tables into one. Makes sense.

-Justin

Re: [PATCH 2/5] reftable/stack: add function to check if optimization is required

[Patrick Steinhardt]

On Fri, Oct 31, 2025 at 03:22:22PM +0100, Karthik Nayak wrote:
> The reftable backend, performs auto-compaction as part of its regular

s/,//

> diff --git a/reftable/reftable-stack.h b/reftable/reftable-stack.h
> index d70fcb705d..a875149439 100644
> --- a/reftable/reftable-stack.h
> +++ b/reftable/reftable-stack.h
> @@ -123,6 +123,11 @@ struct reftable_log_expiry_config {
>  int reftable_stack_compact_all(struct reftable_stack *st,
>  			       struct reftable_log_expiry_config *config);
>  
> +/* Check if compaction is required. */
> +int reftable_stack_compaction_required(struct reftable_stack *st,
> +				       bool use_heuristics,
> +				       bool *required);
> +

I think the documentation here could be improved a bit. Somebody not
deeply familiar with reftables wouldn't know what `use_heuristics`
really is supposed to mean.

> diff --git a/reftable/stack.c b/reftable/stack.c
> index 49387f9344..18fa41cd5c 100644
> --- a/reftable/stack.c
> +++ b/reftable/stack.c
> @@ -1647,6 +1647,31 @@ static int stack_segments_for_compaction(struct reftable_stack *st,
>  	return 0;
>  }
>  
> +int reftable_stack_compaction_required(struct reftable_stack *st,
> +				       bool use_heuristics,
> +				       bool *required)
> +{
> +	struct segment seg;
> +	int err = 0;
> +
> +	if (st->merged->tables_len < 2) {
> +		*required = false;
> +		return 0;
> +	}
> +
> +	if (!use_heuristics) {
> +		*required = true;
> +		return 0;
> +	}
> +
> +	err = stack_segments_for_compaction(st, &seg);
> +	if (err)
> +		return err;
> +
> +	*required = segment_size(&seg) > 0;
> +	return 0;
> +}
> +

All of these conditions make sense.

Patrick

Re: [PATCH 2/5] reftable/stack: add function to check if optimization is required

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 1787 bytes --]

Patrick Steinhardt <ps@pks.im> writes:

> On Fri, Oct 31, 2025 at 03:22:22PM +0100, Karthik Nayak wrote:
>> The reftable backend, performs auto-compaction as part of its regular
>
> s/,//
>
>> diff --git a/reftable/reftable-stack.h b/reftable/reftable-stack.h
>> index d70fcb705d..a875149439 100644
>> --- a/reftable/reftable-stack.h
>> +++ b/reftable/reftable-stack.h
>> @@ -123,6 +123,11 @@ struct reftable_log_expiry_config {
>>  int reftable_stack_compact_all(struct reftable_stack *st,
>>  			       struct reftable_log_expiry_config *config);
>>
>> +/* Check if compaction is required. */
>> +int reftable_stack_compaction_required(struct reftable_stack *st,
>> +				       bool use_heuristics,
>> +				       bool *required);
>> +
>
> I think the documentation here could be improved a bit. Somebody not
> deeply familiar with reftables wouldn't know what `use_heuristics`
> really is supposed to mean.
>

I agree. I'll add in something.

>> diff --git a/reftable/stack.c b/reftable/stack.c
>> index 49387f9344..18fa41cd5c 100644
>> --- a/reftable/stack.c
>> +++ b/reftable/stack.c
>> @@ -1647,6 +1647,31 @@ static int stack_segments_for_compaction(struct reftable_stack *st,
>>  	return 0;
>>  }
>>
>> +int reftable_stack_compaction_required(struct reftable_stack *st,
>> +				       bool use_heuristics,
>> +				       bool *required)
>> +{
>> +	struct segment seg;
>> +	int err = 0;
>> +
>> +	if (st->merged->tables_len < 2) {
>> +		*required = false;
>> +		return 0;
>> +	}
>> +
>> +	if (!use_heuristics) {
>> +		*required = true;
>> +		return 0;
>> +	}
>> +
>> +	err = stack_segments_for_compaction(st, &seg);
>> +	if (err)
>> +		return err;
>> +
>> +	*required = segment_size(&seg) > 0;
>> +	return 0;
>> +}
>> +
>
> All of these conditions make sense.
>
> Patrick

Thanks!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

[PATCH 3/5] refs: add a `optimize_required` field to `struct ref_storage_be`

[Karthik Nayak]

To allow users of the refs namespace to check if the reference backend
requires optimization, add a new field `optimize_required` field to
`struct ref_storage_be`. This field is of type `optimize_required_fn`
which is also introduced in this commit.

Modify the debug, files, packed and reftable backend to implement this
field. A following commit will expose this via 'git pack-refs' and 'git
refs optimize'.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 refs.c                  |  7 +++++++
 refs.h                  |  7 +++++++
 refs/debug.c            | 13 +++++++++++++
 refs/files-backend.c    | 11 +++++++++++
 refs/packed-backend.c   | 13 +++++++++++++
 refs/refs-internal.h    |  6 ++++++
 refs/reftable-backend.c | 25 +++++++++++++++++++++++++
 7 files changed, 82 insertions(+)

diff --git a/refs.c b/refs.c
index 0d0831f29b..5583f6e09d 100644
--- a/refs.c
+++ b/refs.c
@@ -2318,6 +2318,13 @@ int refs_optimize(struct ref_store *refs, struct refs_optimize_opts *opts)
 	return refs->be->optimize(refs, opts);
 }
 
+int refs_optimize_required(struct ref_store *refs,
+			   struct refs_optimize_opts *opts,
+			   bool *required)
+{
+	return refs->be->optimize_required(refs, opts, required);
+}
+
 int reference_get_peeled_oid(struct repository *repo,
 			     const struct reference *ref,
 			     struct object_id *peeled_oid)
diff --git a/refs.h b/refs.h
index 6b05bba527..d9051bbb04 100644
--- a/refs.h
+++ b/refs.h
@@ -520,6 +520,13 @@ struct refs_optimize_opts {
  */
 int refs_optimize(struct ref_store *refs, struct refs_optimize_opts *opts);
 
+/*
+ * Check if refs backend can be optimized by calling 'refs_optimize'.
+ */
+int refs_optimize_required(struct ref_store *ref_store,
+			   struct refs_optimize_opts *opts,
+			   bool *required);
+
 /*
  * Setup reflog before using. Fill in err and return -1 on failure.
  */
diff --git a/refs/debug.c b/refs/debug.c
index 2defd2d465..36f8c58b6c 100644
--- a/refs/debug.c
+++ b/refs/debug.c
@@ -124,6 +124,17 @@ static int debug_optimize(struct ref_store *ref_store, struct refs_optimize_opts
 	return res;
 }
 
+static int debug_optimize_required(struct ref_store *ref_store,
+				   struct refs_optimize_opts *opts,
+				   bool *required)
+{
+	struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;
+	int res = drefs->refs->be->optimize_required(drefs->refs, opts, required);
+	trace_printf_key(&trace_refs, "optimize_required: %s, res: %d\n",
+			 required ? "yes" : "no", res);
+	return res;
+}
+
 static int debug_rename_ref(struct ref_store *ref_store, const char *oldref,
 			    const char *newref, const char *logmsg)
 {
@@ -431,6 +442,8 @@ struct ref_storage_be refs_be_debug = {
 	.transaction_abort = debug_transaction_abort,
 
 	.optimize = debug_optimize,
+	.optimize_required = debug_optimize_required,
+
 	.rename_ref = debug_rename_ref,
 	.copy_ref = debug_copy_ref,
 
diff --git a/refs/files-backend.c b/refs/files-backend.c
index a1e70b1c10..6e0c9b340a 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1512,6 +1512,16 @@ static int files_optimize(struct ref_store *ref_store,
 	return 0;
 }
 
+static int files_optimize_required(struct ref_store *ref_store,
+				   struct refs_optimize_opts *opts,
+				   bool *required)
+{
+	struct files_ref_store *refs = files_downcast(ref_store, REF_STORE_READ,
+						      "optimize_required");
+	*required = should_pack_refs(refs, opts);
+	return 0;
+}
+
 /*
  * People using contrib's git-new-workdir have .git/logs/refs ->
  * /some/other/path/.git/logs/refs, and that may live on another device.
@@ -3982,6 +3992,7 @@ struct ref_storage_be refs_be_files = {
 	.transaction_abort = files_transaction_abort,
 
 	.optimize = files_optimize,
+	.optimize_required = files_optimize_required,
 	.rename_ref = files_rename_ref,
 	.copy_ref = files_copy_ref,
 
diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index 10062fd8b6..19ce4d5872 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -1784,6 +1784,17 @@ static int packed_optimize(struct ref_store *ref_store UNUSED,
 	return 0;
 }
 
+static int packed_optimize_required(struct ref_store *ref_store UNUSED,
+				    struct refs_optimize_opts *opts UNUSED,
+				    bool *required)
+{
+	/*
+	 * Packed refs are already optimized.
+	 */
+	*required = false;
+	return 0;
+}
+
 static struct ref_iterator *packed_reflog_iterator_begin(struct ref_store *ref_store UNUSED)
 {
 	return empty_ref_iterator_begin();
@@ -2130,6 +2141,8 @@ struct ref_storage_be refs_be_packed = {
 	.transaction_abort = packed_transaction_abort,
 
 	.optimize = packed_optimize,
+	.optimize_required = packed_optimize_required,
+
 	.rename_ref = NULL,
 	.copy_ref = NULL,
 
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index dee42f231d..c7d2a6e50b 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -424,6 +424,11 @@ typedef int ref_transaction_commit_fn(struct ref_store *refs,
 
 typedef int optimize_fn(struct ref_store *ref_store,
 			struct refs_optimize_opts *opts);
+
+typedef int optimize_required_fn(struct ref_store *ref_store,
+				 struct refs_optimize_opts *opts,
+				 bool *required);
+
 typedef int rename_ref_fn(struct ref_store *ref_store,
 			  const char *oldref, const char *newref,
 			  const char *logmsg);
@@ -549,6 +554,7 @@ struct ref_storage_be {
 	ref_transaction_abort_fn *transaction_abort;
 
 	optimize_fn *optimize;
+	optimize_required_fn *optimize_required;
 	rename_ref_fn *rename_ref;
 	copy_ref_fn *copy_ref;
 
diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index c23c45f3bf..a3ae0cf74a 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -1733,6 +1733,29 @@ static int reftable_be_optimize(struct ref_store *ref_store,
 	return ret;
 }
 
+static int reftable_be_optimize_required(struct ref_store *ref_store,
+					 struct refs_optimize_opts *opts,
+					 bool *required)
+{
+	struct reftable_ref_store *refs = reftable_be_downcast(ref_store, REF_STORE_READ,
+							       "optimize_refs_required");
+	struct reftable_stack *stack;
+	bool use_heuristics = false;
+
+	if (refs->err)
+		return refs->err;
+
+	stack = refs->worktree_backend.stack;
+	if (!stack)
+		stack = refs->main_backend.stack;
+
+	if (opts->flags & REFS_OPTIMIZE_AUTO)
+		use_heuristics = true;
+
+	return reftable_stack_compaction_required(stack, use_heuristics,
+						  required);
+}
+
 struct write_create_symref_arg {
 	struct reftable_ref_store *refs;
 	struct reftable_stack *stack;
@@ -2756,6 +2779,8 @@ struct ref_storage_be refs_be_reftable = {
 	.transaction_abort = reftable_be_transaction_abort,
 
 	.optimize = reftable_be_optimize,
+	.optimize_required = reftable_be_optimize_required,
+
 	.rename_ref = reftable_be_rename_ref,
 	.copy_ref = reftable_be_copy_ref,
 

-- 
2.51.0

[PATCH 4/5] maintenance: add checking logic in `pack_refs_condition()`

[Karthik Nayak]

The 'git-maintenance(1)' command support an '--auto' flag. Usage of the
flag ensures to run maintenance tasks only if certain thresholds are
met. The heuristic is defined on a task level, wherein each task defines
a 'auto_condition', which states if the task should be run.

The 'pack-refs' task is hard-coded to return 1 as:
1. There was never a way to check if the reference backend needs to be
optimized without actually performing the optimization.
2. We can pass in the '--auto' flag to 'git-pack-refs(1)' which would
optimize based on heuristics.

The previous commit added a `refs_optimize_required()` function, which
can be used to check if a reference backend required optimization. Use
this within `pack_refs_condition()`.

This allows us to add a 'git maintenance is-needed' subcommand which can
notify the user if maintenance is needed without actually performing the
optimization, without this change, the reference backend would always
state that optimization is needed.

Since we import 'revision.h', we need to remove the definition for
'SEEN' which is duplicated in the included header.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 builtin/gc.c | 30 +++++++++++++++++++++---------
 object.h     |  1 -
 2 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index c6d62c74a7..72177305ff 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -35,6 +35,7 @@
 #include "path.h"
 #include "reflog.h"
 #include "rerere.h"
+#include "revision.h"
 #include "blob.h"
 #include "tree.h"
 #include "promisor-remote.h"
@@ -285,12 +286,26 @@ static void maintenance_run_opts_release(struct maintenance_run_opts *opts)
 
 static int pack_refs_condition(UNUSED struct gc_config *cfg)
 {
-	/*
-	 * The auto-repacking logic for refs is handled by the ref backends and
-	 * exposed via `git pack-refs --auto`. We thus always return truish
-	 * here and let the backend decide for us.
-	 */
-	return 1;
+	struct string_list included_refs = STRING_LIST_INIT_NODUP;
+	struct ref_exclusions excludes = REF_EXCLUSIONS_INIT;
+	struct refs_optimize_opts optimize_opts = {
+		.exclusions = &excludes,
+		.includes = &included_refs,
+		.flags = REFS_OPTIMIZE_PRUNE | REFS_OPTIMIZE_AUTO,
+	};
+	bool required;
+
+	// Check for all refs, similar to 'git refs optimize --all'.
+	string_list_append(optimize_opts.includes, "*");
+
+	if (refs_optimize_required(get_main_ref_store(the_repository),
+				   &optimize_opts, &required))
+		return 0;
+
+	clear_ref_exclusions(&excludes);
+	string_list_clear(&included_refs, 0);
+
+	return required;
 }
 
 static int maintenance_task_pack_refs(struct maintenance_run_opts *opts,
@@ -1090,9 +1105,6 @@ static int maintenance_opt_schedule(const struct option *opt, const char *arg,
 	return 0;
 }
 
-/* Remember to update object flag allocation in object.h */
-#define SEEN		(1u<<0)
-
 struct cg_auto_data {
 	int num_not_in_graph;
 	int limit;
diff --git a/object.h b/object.h
index 1499f63d50..832299e763 100644
--- a/object.h
+++ b/object.h
@@ -79,7 +79,6 @@ void object_array_init(struct object_array *array);
  * list-objects-filter.c:                                      21
  * bloom.c:                                                    2122
  * builtin/fsck.c:           0--3
- * builtin/gc.c:             0
  * builtin/index-pack.c:                                     2021
  * reflog.c:                           10--12
  * builtin/show-branch.c:    0-------------------------------------------26

-- 
2.51.0

Re: [PATCH 4/5] maintenance: add checking logic in `pack_refs_condition()`

[Patrick Steinhardt]

On Fri, Oct 31, 2025 at 03:22:24PM +0100, Karthik Nayak wrote:
> The 'git-maintenance(1)' command support an '--auto' flag. Usage of the

s/support/&s/

> flag ensures to run maintenance tasks only if certain thresholds are
> met. The heuristic is defined on a task level, wherein each task defines
> a 'auto_condition', which states if the task should be run.

s/a/an/

> The 'pack-refs' task is hard-coded to return 1 as:
> 1. There was never a way to check if the reference backend needs to be
> optimized without actually performing the optimization.
> 2. We can pass in the '--auto' flag to 'git-pack-refs(1)' which would
> optimize based on heuristics.
> 
> The previous commit added a `refs_optimize_required()` function, which
> can be used to check if a reference backend required optimization. Use
> this within `pack_refs_condition()`.
> 
> This allows us to add a 'git maintenance is-needed' subcommand which can
> notify the user if maintenance is needed without actually performing the
> optimization, without this change, the reference backend would always

s/optimize, without/optimize. Without/

> state that optimization is needed.
> 
> Since we import 'revision.h', we need to remove the definition for
> 'SEEN' which is duplicated in the included header.

Quite weird that it was redefined in the first place. Feels like a nice
side effect.

> diff --git a/builtin/gc.c b/builtin/gc.c
> index c6d62c74a7..72177305ff 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -285,12 +286,26 @@ static void maintenance_run_opts_release(struct maintenance_run_opts *opts)
>  
>  static int pack_refs_condition(UNUSED struct gc_config *cfg)
>  {
> -	/*
> -	 * The auto-repacking logic for refs is handled by the ref backends and
> -	 * exposed via `git pack-refs --auto`. We thus always return truish
> -	 * here and let the backend decide for us.
> -	 */
> -	return 1;
> +	struct string_list included_refs = STRING_LIST_INIT_NODUP;
> +	struct ref_exclusions excludes = REF_EXCLUSIONS_INIT;
> +	struct refs_optimize_opts optimize_opts = {
> +		.exclusions = &excludes,
> +		.includes = &included_refs,

A bit weird that we have to declare these two fields even though we
don't really care for either of them. But I don't mind that too much.

> +		.flags = REFS_OPTIMIZE_PRUNE | REFS_OPTIMIZE_AUTO,
> +	};
> +	bool required;
> +
> +	// Check for all refs, similar to 'git refs optimize --all'.

Style: this should use `/* */` comments.

> +	string_list_append(optimize_opts.includes, "*");
> +
> +	if (refs_optimize_required(get_main_ref_store(the_repository),
> +				   &optimize_opts, &required))
> +		return 0;
> +
> +	clear_ref_exclusions(&excludes);
> +	string_list_clear(&included_refs, 0);
> +
> +	return required;

You return a boolean, but the function is declared to return an integer.
This works, but it feels wrong.

Patrick

Re: [PATCH 4/5] maintenance: add checking logic in `pack_refs_condition()`

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 3426 bytes --]

Patrick Steinhardt <ps@pks.im> writes:

> On Fri, Oct 31, 2025 at 03:22:24PM +0100, Karthik Nayak wrote:
>> The 'git-maintenance(1)' command support an '--auto' flag. Usage of the
>
> s/support/&s/
>

Ah, will change.

>> flag ensures to run maintenance tasks only if certain thresholds are
>> met. The heuristic is defined on a task level, wherein each task defines
>> a 'auto_condition', which states if the task should be run.
>
> s/a/an/

Yup, thanks!

>
>> The 'pack-refs' task is hard-coded to return 1 as:
>> 1. There was never a way to check if the reference backend needs to be
>> optimized without actually performing the optimization.
>> 2. We can pass in the '--auto' flag to 'git-pack-refs(1)' which would
>> optimize based on heuristics.
>>
>> The previous commit added a `refs_optimize_required()` function, which
>> can be used to check if a reference backend required optimization. Use
>> this within `pack_refs_condition()`.
>>
>> This allows us to add a 'git maintenance is-needed' subcommand which can
>> notify the user if maintenance is needed without actually performing the
>> optimization, without this change, the reference backend would always
>
> s/optimize, without/optimize. Without/
>

Thanks, this is better.

>> state that optimization is needed.
>>
>> Since we import 'revision.h', we need to remove the definition for
>> 'SEEN' which is duplicated in the included header.
>
> Quite weird that it was redefined in the first place. Feels like a nice
> side effect.
>

Indeed.

>> diff --git a/builtin/gc.c b/builtin/gc.c
>> index c6d62c74a7..72177305ff 100644
>> --- a/builtin/gc.c
>> +++ b/builtin/gc.c
>> @@ -285,12 +286,26 @@ static void maintenance_run_opts_release(struct maintenance_run_opts *opts)
>>
>>  static int pack_refs_condition(UNUSED struct gc_config *cfg)
>>  {
>> -	/*
>> -	 * The auto-repacking logic for refs is handled by the ref backends and
>> -	 * exposed via `git pack-refs --auto`. We thus always return truish
>> -	 * here and let the backend decide for us.
>> -	 */
>> -	return 1;
>> +	struct string_list included_refs = STRING_LIST_INIT_NODUP;
>> +	struct ref_exclusions excludes = REF_EXCLUSIONS_INIT;
>> +	struct refs_optimize_opts optimize_opts = {
>> +		.exclusions = &excludes,
>> +		.includes = &included_refs,
>
> A bit weird that we have to declare these two fields even though we
> don't really care for either of them. But I don't mind that too much.
>

Yeah, I think there is some cleanup to be done in the files backend. But
I don't think it should be part of this series. If we don't add these,
we crash with a SIGSEGV.

>> +		.flags = REFS_OPTIMIZE_PRUNE | REFS_OPTIMIZE_AUTO,
>> +	};
>> +	bool required;
>> +
>> +	// Check for all refs, similar to 'git refs optimize --all'.
>
> Style: this should use `/* */` comments.
>

Thanks, will fix.

>> +	string_list_append(optimize_opts.includes, "*");
>> +
>> +	if (refs_optimize_required(get_main_ref_store(the_repository),
>> +				   &optimize_opts, &required))
>> +		return 0;
>> +
>> +	clear_ref_exclusions(&excludes);
>> +	string_list_clear(&included_refs, 0);
>> +
>> +	return required;
>
> You return a boolean, but the function is declared to return an integer.
> This works, but it feels wrong.
>
> Patrick

I get what you're saying but returning `required == true` also feel like
a bool return to me (even though it is an int in C).

Anyways, I'll make the change. I don't care much for either.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

[PATCH 5/5] maintenance: add 'is-needed' subcommand

[Karthik Nayak]

The 'git-maintenance(1)' command provides tooling to run maintenance
tasks over Git repositories. The 'run' subcommand, as the name suggests,
runs the maintenance tasks. When used with the '--auto' flag, it uses
heuristics to determine if the required thresholds are met for running
said maintenance tasks.

There is however a lack of insight into these heuristics. Meaning, the
checks are linked to the execution.

Add a new 'is-needed' subcommand to 'git-maintenance(1)' which allows
users to simply check if it is needed to run maintenance without
performing it.

This subcommand can check if it is needed to run maintenance without
actually running it. Ideally it should be used with the '--auto' flag,
which would allow users to check if the thresholds required are met. The
subcommand also supports the '--task' flag which can be used to check
specific maintenance tasks.

While adding the respective tests in 't/t7900-maintenance.sh', remove a
duplicate of the test: 'worktree-prune task with --auto honors
maintenance.worktree-prune.auto'.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 Documentation/git-maintenance.adoc |  8 ++++++
 builtin/gc.c                       | 56 +++++++++++++++++++++++++++++++++++++-
 t/t7900-maintenance.sh             | 54 +++++++++++++++++++++++++-----------
 3 files changed, 101 insertions(+), 17 deletions(-)

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
@@ -12,6 +12,7 @@ SYNOPSIS
 'git maintenance' run [<options>]
 'git maintenance' start [--scheduler=<scheduler>]
 'git maintenance' (stop|register|unregister) [<options>]
+'git maintenance' is-needed [<options>]
 
 
 DESCRIPTION
@@ -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'.
+
 TASKS
 -----
 
@@ -183,6 +189,8 @@ OPTIONS
 	in the `gc.auto` config setting, or when the number of pack-files
 	exceeds the `gc.autoPackLimit` config setting. Not compatible with
 	the `--schedule` option.
+	When combined with the `is-needed` subcommand, check if the required
+	thresholds are met without actually running maintenance.
 
 --schedule::
 	When combined with the `run` subcommand, run maintenance tasks
diff --git a/builtin/gc.c b/builtin/gc.c
index 72177305ff..4d20487ed6 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -3253,7 +3253,60 @@ static int maintenance_stop(int argc, const char **argv, const char *prefix,
 	return update_background_schedule(NULL, 0);
 }
 
-static const char * const builtin_maintenance_usage[] = {
+static const char *const builtin_maintenance_is_needed_usage[] = {
+	"git maintenance is-needed [--task=<task>] [--schedule]",
+	NULL
+};
+
+static int maintenance_is_needed(int argc, const char **argv, const char *prefix,
+				 struct repository *repo UNUSED)
+{
+	struct maintenance_run_opts opts = MAINTENANCE_RUN_OPTS_INIT;
+	struct string_list selected_tasks = STRING_LIST_INIT_DUP;
+	struct gc_config cfg = GC_CONFIG_INIT;
+	struct option options[] = {
+		OPT_BOOL(0, "auto", &opts.auto_flag,
+			 N_("run tasks based on the state of the repository")),
+		OPT_CALLBACK_F(0, "task", &selected_tasks, N_("task"),
+			       N_("check a specific task"),
+			       PARSE_OPT_NONEG, task_option_parse),
+		OPT_END()
+	};
+	bool is_needed = false;
+
+	argc = parse_options(argc, argv, prefix, options,
+			     builtin_maintenance_is_needed_usage,
+			     PARSE_OPT_STOP_AT_NON_OPTION);
+
+	gc_config(&cfg);
+	initialize_task_config(&opts, &selected_tasks);
+
+	if (argc)
+		usage_with_options(builtin_maintenance_is_needed_usage, options);
+
+	if (opts.auto_flag) {
+		for (size_t i = 0; i < opts.tasks_nr; i++) {
+			if (tasks[opts.tasks[i]].auto_condition &&
+			    tasks[opts.tasks[i]].auto_condition(&cfg)) {
+				is_needed = true;
+				break;
+			}
+		}
+	} else {
+		/* When not using --auto, we should always require maintenance. */
+		is_needed = true;
+	}
+
+	string_list_clear(&selected_tasks, 0);
+	maintenance_run_opts_release(&opts);
+	gc_config_release(&cfg);
+
+	if (is_needed)
+		return 0;
+	return 1;
+}
+
+static const char *const builtin_maintenance_usage[] = {
 	N_("git maintenance <subcommand> [<options>]"),
 	NULL,
 };
@@ -3270,6 +3323,7 @@ int cmd_maintenance(int argc,
 		OPT_SUBCOMMAND("stop", &fn, maintenance_stop),
 		OPT_SUBCOMMAND("register", &fn, maintenance_register),
 		OPT_SUBCOMMAND("unregister", &fn, maintenance_unregister),
+		OPT_SUBCOMMAND("is-needed", &fn, maintenance_is_needed),
 		OPT_END(),
 	};
 
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index ddd273d8dc..a17e2091c2 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -49,7 +49,9 @@ test_expect_success 'run [--auto|--quiet]' '
 		git maintenance run --auto 2>/dev/null &&
 	GIT_TRACE2_EVENT="$(pwd)/run-no-quiet.txt" \
 		git maintenance run --no-quiet 2>/dev/null &&
+	git maintenance is-needed &&
 	test_subcommand git gc --quiet --no-detach --skip-foreground-tasks <run-no-auto.txt &&
+	! git maintenance is-needed --auto &&
 	test_subcommand ! git gc --auto --quiet --no-detach --skip-foreground-tasks <run-auto.txt &&
 	test_subcommand git gc --no-quiet --no-detach --skip-foreground-tasks <run-no-quiet.txt
 '
@@ -180,6 +182,11 @@ test_expect_success 'commit-graph auto condition' '
 
 	test_commit first &&
 
+	! git -c maintenance.commit-graph.auto=0 \
+		maintenance is-needed --auto --task=commit-graph &&
+	git -c maintenance.commit-graph.auto=1 \
+		maintenance is-needed --auto --task=commit-graph &&
+
 	GIT_TRACE2_EVENT="$(pwd)/cg-zero-means-no.txt" \
 		git -c maintenance.commit-graph.auto=0 $COMMAND &&
 	GIT_TRACE2_EVENT="$(pwd)/cg-one-satisfied.txt" \
@@ -290,16 +297,23 @@ test_expect_success 'maintenance.loose-objects.auto' '
 		git -c maintenance.loose-objects.auto=1 maintenance \
 		run --auto --task=loose-objects 2>/dev/null &&
 	test_subcommand ! git prune-packed --quiet <trace-lo1.txt &&
+
 	printf data-A | git hash-object -t blob --stdin -w &&
+	! git -c maintenance.loose-objects.auto=2 \
+		maintenance is-needed --auto --task=loose-objects &&
 	GIT_TRACE2_EVENT="$(pwd)/trace-loA" \
 		git -c maintenance.loose-objects.auto=2 \
 		maintenance run --auto --task=loose-objects 2>/dev/null &&
 	test_subcommand ! git prune-packed --quiet <trace-loA &&
+
 	printf data-B | git hash-object -t blob --stdin -w &&
+	git -c maintenance.loose-objects.auto=2 \
+		maintenance is-needed --auto --task=loose-objects &&
 	GIT_TRACE2_EVENT="$(pwd)/trace-loB" \
 		git -c maintenance.loose-objects.auto=2 \
 		maintenance run --auto --task=loose-objects 2>/dev/null &&
 	test_subcommand git prune-packed --quiet <trace-loB &&
+
 	GIT_TRACE2_EVENT="$(pwd)/trace-loC" \
 		git -c maintenance.loose-objects.auto=2 \
 		maintenance run --auto --task=loose-objects 2>/dev/null &&
@@ -421,10 +435,13 @@ run_incremental_repack_and_verify () {
 	test_commit A &&
 	git repack -adk &&
 	git multi-pack-index write &&
+	! git -c maintenance.incremental-repack.auto=1 \
+		maintenance is-needed --auto --task=incremental-repack &&
 	GIT_TRACE2_EVENT="$(pwd)/midx-init.txt" git \
 		-c maintenance.incremental-repack.auto=1 \
 		maintenance run --auto --task=incremental-repack 2>/dev/null &&
 	test_subcommand ! git multi-pack-index write --no-progress <midx-init.txt &&
+
 	test_commit B &&
 	git pack-objects --revs .git/objects/pack/pack <<-\EOF &&
 	HEAD
@@ -434,11 +451,14 @@ run_incremental_repack_and_verify () {
 		-c maintenance.incremental-repack.auto=2 \
 		maintenance run --auto --task=incremental-repack 2>/dev/null &&
 	test_subcommand ! git multi-pack-index write --no-progress <trace-A &&
+
 	test_commit C &&
 	git pack-objects --revs .git/objects/pack/pack <<-\EOF &&
 	HEAD
 	^HEAD~1
 	EOF
+	git -c maintenance.incremental-repack.auto=2 \
+		maintenance is-needed --auto --task=incremental-repack &&
 	GIT_TRACE2_EVENT=$(pwd)/trace-B git \
 		-c maintenance.incremental-repack.auto=2 \
 		maintenance run --auto --task=incremental-repack 2>/dev/null &&
@@ -485,9 +505,15 @@ test_expect_success 'reflog-expire task --auto only packs when exceeding limits'
 	git reflog expire --all --expire=now &&
 	test_commit reflog-one &&
 	test_commit reflog-two &&
+
+	! git -c maintenance.reflog-expire.auto=3 \
+		maintenance is-needed --auto --task=reflog-expire &&
 	GIT_TRACE2_EVENT="$(pwd)/reflog-expire-auto.txt" \
 		git -c maintenance.reflog-expire.auto=3 maintenance run --auto --task=reflog-expire &&
 	test_subcommand ! git reflog expire --all <reflog-expire-auto.txt &&
+
+	git -c maintenance.reflog-expire.auto=2 \
+		maintenance is-needed --auto --task=reflog-expire &&
 	GIT_TRACE2_EVENT="$(pwd)/reflog-expire-auto.txt" \
 		git -c maintenance.reflog-expire.auto=2 maintenance run --auto --task=reflog-expire &&
 	test_subcommand git reflog expire --all <reflog-expire-auto.txt
@@ -514,6 +540,7 @@ test_expect_success 'worktree-prune task --auto only prunes with prunable worktr
 	test_expect_worktree_prune ! git maintenance run --auto --task=worktree-prune &&
 	mkdir .git/worktrees &&
 	: >.git/worktrees/abc &&
+	git maintenance is-needed --auto --task=worktree-prune &&
 	test_expect_worktree_prune git maintenance run --auto --task=worktree-prune
 '
 
@@ -530,22 +557,7 @@ test_expect_success 'worktree-prune task with --auto honors maintenance.worktree
 	test_expect_worktree_prune ! git -c maintenance.worktree-prune.auto=0 maintenance run --auto --task=worktree-prune &&
 	# A positive value should require at least this many prunable worktrees.
 	test_expect_worktree_prune ! git -c maintenance.worktree-prune.auto=4 maintenance run --auto --task=worktree-prune &&
-	test_expect_worktree_prune git -c maintenance.worktree-prune.auto=3 maintenance run --auto --task=worktree-prune
-'
-
-test_expect_success 'worktree-prune task with --auto honors maintenance.worktree-prune.auto' '
-	# A negative value should always prune.
-	test_expect_worktree_prune git -c maintenance.worktree-prune.auto=-1 maintenance run --auto --task=worktree-prune &&
-
-	mkdir .git/worktrees &&
-	: >.git/worktrees/first &&
-	: >.git/worktrees/second &&
-	: >.git/worktrees/third &&
-
-	# Zero should never prune.
-	test_expect_worktree_prune ! git -c maintenance.worktree-prune.auto=0 maintenance run --auto --task=worktree-prune &&
-	# A positive value should require at least this many prunable worktrees.
-	test_expect_worktree_prune ! git -c maintenance.worktree-prune.auto=4 maintenance run --auto --task=worktree-prune &&
+	git -c maintenance.worktree-prune.auto=3 maintenance is-needed --auto --task=worktree-prune &&
 	test_expect_worktree_prune git -c maintenance.worktree-prune.auto=3 maintenance run --auto --task=worktree-prune
 '
 
@@ -554,11 +566,13 @@ test_expect_success 'worktree-prune task honors gc.worktreePruneExpire' '
 	rm -rf worktree &&
 
 	rm -f worktree-prune.txt &&
+	! git -c gc.worktreePruneExpire=1.week.ago maintenance is-needed --auto --task=worktree-prune &&
 	GIT_TRACE2_EVENT="$(pwd)/worktree-prune.txt" git -c gc.worktreePruneExpire=1.week.ago maintenance run --auto --task=worktree-prune &&
 	test_subcommand ! git worktree prune --expire 1.week.ago <worktree-prune.txt &&
 	test_path_is_dir .git/worktrees/worktree &&
 
 	rm -f worktree-prune.txt &&
+	git -c gc.worktreePruneExpire=now maintenance is-needed --auto --task=worktree-prune &&
 	GIT_TRACE2_EVENT="$(pwd)/worktree-prune.txt" git -c gc.worktreePruneExpire=now maintenance run --auto --task=worktree-prune &&
 	test_subcommand git worktree prune --expire now <worktree-prune.txt &&
 	test_path_is_missing .git/worktrees/worktree
@@ -583,10 +597,13 @@ test_expect_success 'rerere-gc task without --auto always collects garbage' '
 
 test_expect_success 'rerere-gc task with --auto only prunes with prunable entries' '
 	test_when_finished "rm -rf .git/rr-cache" &&
+	! git maintenance is-needed --auto --task=rerere-gc &&
 	test_expect_rerere_gc ! git maintenance run --auto --task=rerere-gc &&
 	mkdir .git/rr-cache &&
+	! git maintenance is-needed --auto --task=rerere-gc &&
 	test_expect_rerere_gc ! git maintenance run --auto --task=rerere-gc &&
 	: >.git/rr-cache/entry &&
+	git maintenance is-needed --auto --task=rerere-gc &&
 	test_expect_rerere_gc git maintenance run --auto --task=rerere-gc
 '
 
@@ -594,17 +611,22 @@ test_expect_success 'rerere-gc task with --auto honors maintenance.rerere-gc.aut
 	test_when_finished "rm -rf .git/rr-cache" &&
 
 	# A negative value should always prune.
+	git -c maintenance.rerere-gc.auto=-1 maintenance is-needed --auto --task=rerere-gc &&
 	test_expect_rerere_gc git -c maintenance.rerere-gc.auto=-1 maintenance run --auto --task=rerere-gc &&
 
 	# A positive value prunes when there is at least one entry.
+	! git -c maintenance.rerere-gc.auto=9000 maintenance is-needed --auto --task=rerere-gc &&
 	test_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=9000 maintenance run --auto --task=rerere-gc &&
 	mkdir .git/rr-cache &&
+	! git -c maintenance.rerere-gc.auto=9000 maintenance is-needed --auto --task=rerere-gc &&
 	test_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=9000 maintenance run --auto --task=rerere-gc &&
 	: >.git/rr-cache/entry-1 &&
+	git -c maintenance.rerere-gc.auto=9000 maintenance is-needed --auto --task=rerere-gc &&
 	test_expect_rerere_gc git -c maintenance.rerere-gc.auto=9000 maintenance run --auto --task=rerere-gc &&
 
 	# Zero should never prune.
 	: >.git/rr-cache/entry-1 &&
+	! git -c maintenance.rerere-gc.auto=0 maintenance is-needed --auto --task=rerere-gc &&
 	test_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=0 maintenance run --auto --task=rerere-gc
 '
 

-- 
2.51.0

Re: [PATCH 5/5] maintenance: add 'is-needed' subcommand

[Patrick Steinhardt]

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.

> diff --git a/builtin/gc.c b/builtin/gc.c
> index 72177305ff..4d20487ed6 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -3253,7 +3253,60 @@ static int maintenance_stop(int argc, const char **argv, const char *prefix,
>  	return update_background_schedule(NULL, 0);
>  }
>  
> -static const char * const builtin_maintenance_usage[] = {
> +static const char *const builtin_maintenance_is_needed_usage[] = {
> +	"git maintenance is-needed [--task=<task>] [--schedule]",
> +	NULL
> +};
> +
> +static int maintenance_is_needed(int argc, const char **argv, const char *prefix,
> +				 struct repository *repo UNUSED)
> +{
> +	struct maintenance_run_opts opts = MAINTENANCE_RUN_OPTS_INIT;
> +	struct string_list selected_tasks = STRING_LIST_INIT_DUP;
> +	struct gc_config cfg = GC_CONFIG_INIT;
> +	struct option options[] = {
> +		OPT_BOOL(0, "auto", &opts.auto_flag,
> +			 N_("run tasks based on the state of the repository")),
> +		OPT_CALLBACK_F(0, "task", &selected_tasks, N_("task"),
> +			       N_("check a specific task"),
> +			       PARSE_OPT_NONEG, task_option_parse),
> +		OPT_END()
> +	};
> +	bool is_needed = false;
> +
> +	argc = parse_options(argc, argv, prefix, options,
> +			     builtin_maintenance_is_needed_usage,
> +			     PARSE_OPT_STOP_AT_NON_OPTION);
> +
> +	gc_config(&cfg);
> +	initialize_task_config(&opts, &selected_tasks);
> +
> +	if (argc)
> +		usage_with_options(builtin_maintenance_is_needed_usage, options);

Shouldn't this check be directly after the call to `parse_options()`?

> +	if (opts.auto_flag) {
> +		for (size_t i = 0; i < opts.tasks_nr; i++) {
> +			if (tasks[opts.tasks[i]].auto_condition &&
> +			    tasks[opts.tasks[i]].auto_condition(&cfg)) {
> +				is_needed = true;
> +				break;
> +			}
> +		}

Okay, we need to guard against the auto-condition not existing indeed.
This is only due to the "prefetch" task though, all the others do have
the callback.

> +	} 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

Re: [PATCH 5/5] maintenance: add 'is-needed' subcommand

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 3781 bytes --]

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.

>> diff --git a/builtin/gc.c b/builtin/gc.c
>> index 72177305ff..4d20487ed6 100644
>> --- a/builtin/gc.c
>> +++ b/builtin/gc.c
>> @@ -3253,7 +3253,60 @@ static int maintenance_stop(int argc, const char **argv, const char *prefix,
>>  	return update_background_schedule(NULL, 0);
>>  }
>>
>> -static const char * const builtin_maintenance_usage[] = {
>> +static const char *const builtin_maintenance_is_needed_usage[] = {
>> +	"git maintenance is-needed [--task=<task>] [--schedule]",
>> +	NULL
>> +};
>> +
>> +static int maintenance_is_needed(int argc, const char **argv, const char *prefix,
>> +				 struct repository *repo UNUSED)
>> +{
>> +	struct maintenance_run_opts opts = MAINTENANCE_RUN_OPTS_INIT;
>> +	struct string_list selected_tasks = STRING_LIST_INIT_DUP;
>> +	struct gc_config cfg = GC_CONFIG_INIT;
>> +	struct option options[] = {
>> +		OPT_BOOL(0, "auto", &opts.auto_flag,
>> +			 N_("run tasks based on the state of the repository")),
>> +		OPT_CALLBACK_F(0, "task", &selected_tasks, N_("task"),
>> +			       N_("check a specific task"),
>> +			       PARSE_OPT_NONEG, task_option_parse),
>> +		OPT_END()
>> +	};
>> +	bool is_needed = false;
>> +
>> +	argc = parse_options(argc, argv, prefix, options,
>> +			     builtin_maintenance_is_needed_usage,
>> +			     PARSE_OPT_STOP_AT_NON_OPTION);
>> +
>> +	gc_config(&cfg);
>> +	initialize_task_config(&opts, &selected_tasks);
>> +
>> +	if (argc)
>> +		usage_with_options(builtin_maintenance_is_needed_usage, options);
>
> Shouldn't this check be directly after the call to `parse_options()`?
>

Yes, I moved it around, will fix it.

>> +	if (opts.auto_flag) {
>> +		for (size_t i = 0; i < opts.tasks_nr; i++) {
>> +			if (tasks[opts.tasks[i]].auto_condition &&
>> +			    tasks[opts.tasks[i]].auto_condition(&cfg)) {
>> +				is_needed = true;
>> +				break;
>> +			}
>> +		}
>
> Okay, we need to guard against the auto-condition not existing indeed.
> This is only due to the "prefetch" task though, all the others do have
> the callback.
>

Yup, that's correct.

>> +	} 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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH 5/5] maintenance: add 'is-needed' subcommand

[Patrick Steinhardt]

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.

> >> 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

Re: [PATCH 5/5] maintenance: add 'is-needed' subcommand

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 3052 bytes --]

Patrick Steinhardt <ps@pks.im> writes:

> 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.

>
>> >> 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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

[PATCH v2 0/5] maintenance: add an 'is-needed' subcommand

[Karthik Nayak]

Hello,

I recently raised a patch series [1] to add 'git refs optimize --required'
which checks if the reference backend can be optimized, without actually
performing the optimization.

Back then, we had decided [2] that it would be a better to broaden the
approach and add a 'is-needed' subcommand to 'git-maintenance(1)'. This
would allow users to check if maintenance was required for the
repository and users could also provide a task via the '--task' to check
if maintenance was needed for a particular task.

Ideally the subcommand will be used with the '--auto' flag which can
check the same heuristics as that used with 'git maintenance run
--auto'. Future patches can also add support for the '--schedule' flag
which can be used to check required schedule it met. However that flag
isn't added as part of this series.

This series implements that.

Commits 1-3 add the required functionality in the refs subsystem to
expose an 'optimize_required' field which can be used to check if
backends need to be optimized.
Commit 4 utilizes this within the 'git-maintenance(1)' code.
Commit 5 adds the 'is-needed' subcommand to 'git-maintenance(1)'.

This is based on top of master a99f379adf (The 27th batch, 2025-10-30)
and is dependent on the following series:

    - kn/refs-optim-cleanup
    - ps/ref-peeled-tags

Merges cleanly with `next`. I think those two topics are close to being
merged to `next` so hopefully this dependency tree doesn't get too
complicated. I'll rebase as needed to resolve conflicts.

[1]: https://lore.kernel.org/git/20251010-562-add-option-to-check-if-reference-backend-needs-repacking-v1-0-c7962be584fa@gmail.com/
[2]: https://lore.kernel.org/git/CAOLa=ZRdxm787nE4FSr2VUHDB+hW06Ggc6yUcKmeTKAb6B7YOA@mail.gmail.com/

---
Changes in v2:
- Added more documentation for `reftable_stack_compaction_required()`.
- Fixed some typos and grammar mistakes in commit messages.
- Clarify which tasks will be run when '--task' is not used.
- Move the call to 'usage_with_options()' to be with 'parse_options()'.
- Link to v1: https://patch.msgid.link/20251031-562-add-sub-command-to-check-if-maintenance-is-needed-v1-0-a03d53e28d0e@gmail.com

---
 Documentation/git-maintenance.adoc | 13 ++++++
 builtin/gc.c                       | 85 +++++++++++++++++++++++++++++++++-----
 object.h                           |  1 -
 refs.c                             |  7 ++++
 refs.h                             |  7 ++++
 refs/debug.c                       | 13 ++++++
 refs/files-backend.c               | 11 +++++
 refs/packed-backend.c              | 13 ++++++
 refs/refs-internal.h               |  6 +++
 refs/reftable-backend.c            | 25 +++++++++++
 reftable/reftable-stack.h          | 11 +++++
 reftable/stack.c                   | 48 ++++++++++++++++-----
 t/t7900-maintenance.sh             | 54 +++++++++++++++++-------
 t/unit-tests/u-reftable-stack.c    | 12 +++++-
 14 files changed, 266 insertions(+), 40 deletions(-)

Karthik Nayak (5):
      reftable/stack: return stack segments directly
      reftable/stack: add function to check if optimization is required
      refs: add a `optimize_required` field to `struct ref_storage_be`
      maintenance: add checking logic in `pack_refs_condition()`
      maintenance: add 'is-needed' subcommand

Range-diff versus v1:

1:  e5e6eedfbe = 1:  5431fe40ef reftable/stack: return stack segments directly
2:  6fac9e5eb5 ! 2:  248882aad6 reftable/stack: add function to check if optimization is required
    @@ Metadata
      ## Commit message ##
         reftable/stack: add function to check if optimization is required
     
    -    The reftable backend, performs auto-compaction as part of its regular
    +    The reftable backend performs auto-compaction as part of its regular
         flow, which is required to keep the number of tables part of a stack at
         bay. This allows it to stay optimized.
     
    @@ reftable/reftable-stack.h: struct reftable_log_expiry_config {
      int reftable_stack_compact_all(struct reftable_stack *st,
      			       struct reftable_log_expiry_config *config);
      
    -+/* Check if compaction is required. */
    ++/*
    ++ * Check if compaction is required.
    ++ *
    ++ * When `use_heuristics` is false, check if all tables can be compacted to a
    ++ * single table. If true, use heuristics to determine if the tables need to be
    ++ * compacted to maintain geometric progression.
    ++ */
     +int reftable_stack_compaction_required(struct reftable_stack *st,
     +				       bool use_heuristics,
     +				       bool *required);
3:  2ec39102a5 = 3:  e98c2c2e3e refs: add a `optimize_required` field to `struct ref_storage_be`
4:  c246efdc4a ! 4:  9b7fa79c8c maintenance: add checking logic in `pack_refs_condition()`
    @@ Metadata
      ## Commit message ##
         maintenance: add checking logic in `pack_refs_condition()`
     
    -    The 'git-maintenance(1)' command support an '--auto' flag. Usage of the
    +    The 'git-maintenance(1)' command supports an '--auto' flag. Usage of the
         flag ensures to run maintenance tasks only if certain thresholds are
         met. The heuristic is defined on a task level, wherein each task defines
    -    a 'auto_condition', which states if the task should be run.
    +    an 'auto_condition', which states if the task should be run.
     
         The 'pack-refs' task is hard-coded to return 1 as:
         1. There was never a way to check if the reference backend needs to be
    @@ Commit message
     
         This allows us to add a 'git maintenance is-needed' subcommand which can
         notify the user if maintenance is needed without actually performing the
    -    optimization, without this change, the reference backend would always
    +    optimization. Without this change, the reference backend would always
         state that optimization is needed.
     
         Since we import 'revision.h', we need to remove the definition for
    @@ builtin/gc.c: static void maintenance_run_opts_release(struct maintenance_run_op
     +	};
     +	bool required;
     +
    -+	// Check for all refs, similar to 'git refs optimize --all'.
    ++	/* Check for all refs, similar to 'git refs optimize --all'. */
     +	string_list_append(optimize_opts.includes, "*");
     +
     +	if (refs_optimize_required(get_main_ref_store(the_repository),
    @@ builtin/gc.c: static void maintenance_run_opts_release(struct maintenance_run_op
     +	clear_ref_exclusions(&excludes);
     +	string_list_clear(&included_refs, 0);
     +
    -+	return required;
    ++	return required == true;
      }
      
      static int maintenance_task_pack_refs(struct maintenance_run_opts *opts,
5:  64c15c1319 ! 5:  ed3658528b maintenance: add 'is-needed' subcommand
    @@ Documentation/git-maintenance.adoc: The `unregister` subcommand will report an e
     +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'.
    ++    Ideally used with the '--auto' flag.
    +++
    ++If one or more `--task` options	are specified, then those tasks are checked
    ++in that order. Otherwise, the tasks are determined by which
    ++`maintenance.<task>.enabled` config options are true. By default, only
    ++`maintenance.gc.enabled` is true.
     +
      TASKS
      -----
    @@ builtin/gc.c: static int maintenance_stop(int argc, const char **argv, const cha
     +	argc = parse_options(argc, argv, prefix, options,
     +			     builtin_maintenance_is_needed_usage,
     +			     PARSE_OPT_STOP_AT_NON_OPTION);
    ++	if (argc)
    ++		usage_with_options(builtin_maintenance_is_needed_usage, options);
     +
     +	gc_config(&cfg);
     +	initialize_task_config(&opts, &selected_tasks);
     +
    -+	if (argc)
    -+		usage_with_options(builtin_maintenance_is_needed_usage, options);
    -+
     +	if (opts.auto_flag) {
     +		for (size_t i = 0; i < opts.tasks_nr; i++) {
     +			if (tasks[opts.tasks[i]].auto_condition &&


base-commit: edd2018f5db39d68d55a7a4af42375b1a06b9406
change-id: 20251021-562-add-sub-command-to-check-if-maintenance-is-needed-01cae01b4606

Thanks
- Karthik

[PATCH v2 1/5] reftable/stack: return stack segments directly

[Karthik Nayak]

The `stack_table_sizes_for_compaction()` function returns individual
sizes of each reftable table. This function is only called by
`reftable_stack_auto_compact()` to decide which tables need to be
compacted, if any.

Modify the function to directly return the segments, which avoids the
extra step of receiving the sizes only to pass it to
`suggest_compaction_segment()`.

A future commit will also add functionality for checking whether
auto-compaction is necessary without performing it. This change allows
code re-usability in that context.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 reftable/stack.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/reftable/stack.c b/reftable/stack.c
index 65d89820bd..49387f9344 100644
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -1626,7 +1626,8 @@ struct segment suggest_compaction_segment(uint64_t *sizes, size_t n,
 	return seg;
 }
 
-static uint64_t *stack_table_sizes_for_compaction(struct reftable_stack *st)
+static int stack_segments_for_compaction(struct reftable_stack *st,
+					 struct segment *seg)
 {
 	int version = (st->opts.hash_id == REFTABLE_HASH_SHA1) ? 1 : 2;
 	int overhead = header_size(version) - 1;
@@ -1634,29 +1635,29 @@ static uint64_t *stack_table_sizes_for_compaction(struct reftable_stack *st)
 
 	REFTABLE_CALLOC_ARRAY(sizes, st->merged->tables_len);
 	if (!sizes)
-		return NULL;
+		return REFTABLE_OUT_OF_MEMORY_ERROR;
 
 	for (size_t i = 0; i < st->merged->tables_len; i++)
 		sizes[i] = st->tables[i]->size - overhead;
 
-	return sizes;
+	*seg = suggest_compaction_segment(sizes, st->merged->tables_len,
+					  st->opts.auto_compaction_factor);
+	reftable_free(sizes);
+
+	return 0;
 }
 
 int reftable_stack_auto_compact(struct reftable_stack *st)
 {
 	struct segment seg;
-	uint64_t *sizes;
+	int err;
 
 	if (st->merged->tables_len < 2)
 		return 0;
 
-	sizes = stack_table_sizes_for_compaction(st);
-	if (!sizes)
-		return REFTABLE_OUT_OF_MEMORY_ERROR;
-
-	seg = suggest_compaction_segment(sizes, st->merged->tables_len,
-					 st->opts.auto_compaction_factor);
-	reftable_free(sizes);
+	err = stack_segments_for_compaction(st, &seg);
+	if (err)
+		return err;
 
 	if (segment_size(&seg) > 0)
 		return stack_compact_range(st, seg.start, seg.end - 1,

-- 
2.51.0

[PATCH v2 2/5] reftable/stack: add function to check if optimization is required

[Karthik Nayak]

The reftable backend performs auto-compaction as part of its regular
flow, which is required to keep the number of tables part of a stack at
bay. This allows it to stay optimized.

Compaction can also be triggered voluntarily by the user via the 'git
pack-refs' or the 'git refs optimize' command. However, currently there
is no way for the user to check if optimization is required without
actually performing it.

Add and expose `reftable_stack_compaction_required()` which will allow
users to check if the reftable backend can be optimized.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 reftable/reftable-stack.h       | 11 +++++++++++
 reftable/stack.c                | 25 +++++++++++++++++++++++++
 t/unit-tests/u-reftable-stack.c | 12 ++++++++++--
 3 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/reftable/reftable-stack.h b/reftable/reftable-stack.h
index d70fcb705d..c2415cbc6e 100644
--- a/reftable/reftable-stack.h
+++ b/reftable/reftable-stack.h
@@ -123,6 +123,17 @@ struct reftable_log_expiry_config {
 int reftable_stack_compact_all(struct reftable_stack *st,
 			       struct reftable_log_expiry_config *config);
 
+/*
+ * Check if compaction is required.
+ *
+ * When `use_heuristics` is false, check if all tables can be compacted to a
+ * single table. If true, use heuristics to determine if the tables need to be
+ * compacted to maintain geometric progression.
+ */
+int reftable_stack_compaction_required(struct reftable_stack *st,
+				       bool use_heuristics,
+				       bool *required);
+
 /* heuristically compact unbalanced table stack. */
 int reftable_stack_auto_compact(struct reftable_stack *st);
 
diff --git a/reftable/stack.c b/reftable/stack.c
index 49387f9344..18fa41cd5c 100644
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -1647,6 +1647,31 @@ static int stack_segments_for_compaction(struct reftable_stack *st,
 	return 0;
 }
 
+int reftable_stack_compaction_required(struct reftable_stack *st,
+				       bool use_heuristics,
+				       bool *required)
+{
+	struct segment seg;
+	int err = 0;
+
+	if (st->merged->tables_len < 2) {
+		*required = false;
+		return 0;
+	}
+
+	if (!use_heuristics) {
+		*required = true;
+		return 0;
+	}
+
+	err = stack_segments_for_compaction(st, &seg);
+	if (err)
+		return err;
+
+	*required = segment_size(&seg) > 0;
+	return 0;
+}
+
 int reftable_stack_auto_compact(struct reftable_stack *st)
 {
 	struct segment seg;
diff --git a/t/unit-tests/u-reftable-stack.c b/t/unit-tests/u-reftable-stack.c
index a8b91812e8..b8110cdeee 100644
--- a/t/unit-tests/u-reftable-stack.c
+++ b/t/unit-tests/u-reftable-stack.c
@@ -1067,6 +1067,7 @@ void test_reftable_stack__add_performs_auto_compaction(void)
 			.value_type = REFTABLE_REF_SYMREF,
 			.value.symref = (char *) "master",
 		};
+		bool required = false;
 		char buf[128];
 
 		/*
@@ -1087,10 +1088,17 @@ void test_reftable_stack__add_performs_auto_compaction(void)
 		 * auto compaction is disabled. When enabled, we should merge
 		 * all tables in the stack.
 		 */
-		if (i != n)
+		cl_assert_equal_i(reftable_stack_compaction_required(st, true, &required), 0);
+		if (i != n) {
 			cl_assert_equal_i(st->merged->tables_len, i + 1);
-		else
+			if (i < 1)
+				cl_assert_equal_b(required, false);
+			else
+				cl_assert_equal_b(required, true);
+		} else {
 			cl_assert_equal_i(st->merged->tables_len, 1);
+			cl_assert_equal_b(required, false);
+		}
 	}
 
 	reftable_stack_destroy(st);

-- 
2.51.0

Re: [PATCH v2 2/5] reftable/stack: add function to check if optimization is required

[Junio C Hamano]

Karthik Nayak <karthik.188@gmail.com> writes:

> The reftable backend performs auto-compaction as part of its regular
> flow, which is required to keep the number of tables part of a stack at
> bay. This allows it to stay optimized.

Sounds very sensible.

> Compaction can also be triggered voluntarily by the user via the 'git
> pack-refs' or the 'git refs optimize' command. However, currently there
> is no way for the user to check if optimization is required without
> actually performing it.

Sounds very sensible goal.

But where is the existing logic to decide when it needs to
auto-compact, performed as part of its regular flow?

After reading "the reftable machinery already decides when it needs
to compact and does so" plus "but the logic to decide is not made
available to users", I would have expected for this patch to extract
such an existing logic or otherwise make it available to new callers
so that things like "gc --auto" can call it, but the diffstat shows
mostly additions, which does not give readers any confidence in the
new function that answers "do we need compaction?".  It would give
_an_ answer, but there is no clue if the answer it gives is the same
answer as the existing logic that decides when to compact as part of
the regular operation.

I am puzzled.

> +int reftable_stack_compaction_required(struct reftable_stack *st,
> +				       bool use_heuristics,
> +				       bool *required)
> +{
> +	struct segment seg;
> +	int err = 0;
> +
> +	if (st->merged->tables_len < 2) {
> +		*required = false;
> +		return 0;
> +	}
> +
> +	if (!use_heuristics) {
> +		*required = true;
> +		return 0;
> +	}
> +
> +	err = stack_segments_for_compaction(st, &seg);
> +	if (err)
> +		return err;
> +
> +	*required = segment_size(&seg) > 0;
> +	return 0;
> +}

Specifically, where is the above logic come from?  Is it duplicating
an existing logic but that code is hard to separate out into this
helper?

>  int reftable_stack_auto_compact(struct reftable_stack *st)
>  {
>  	struct segment seg;
> diff --git a/t/unit-tests/u-reftable-stack.c b/t/unit-tests/u-reftable-stack.c
> index a8b91812e8..b8110cdeee 100644
> --- a/t/unit-tests/u-reftable-stack.c
> +++ b/t/unit-tests/u-reftable-stack.c
> @@ -1067,6 +1067,7 @@ void test_reftable_stack__add_performs_auto_compaction(void)
>  			.value_type = REFTABLE_REF_SYMREF,
>  			.value.symref = (char *) "master",
>  		};
> +		bool required = false;
>  		char buf[128];
>  
>  		/*
> @@ -1087,10 +1088,17 @@ void test_reftable_stack__add_performs_auto_compaction(void)
>  		 * auto compaction is disabled. When enabled, we should merge
>  		 * all tables in the stack.
>  		 */
> -		if (i != n)
> +		cl_assert_equal_i(reftable_stack_compaction_required(st, true, &required), 0);
> +		if (i != n) {
>  			cl_assert_equal_i(st->merged->tables_len, i + 1);
> -		else
> +			if (i < 1)
> +				cl_assert_equal_b(required, false);
> +			else
> +				cl_assert_equal_b(required, true);
> +		} else {
>  			cl_assert_equal_i(st->merged->tables_len, 1);
> +			cl_assert_equal_b(required, false);
> +		}
>  	}
>  
>  	reftable_stack_destroy(st);

Re: [PATCH v2 2/5] reftable/stack: add function to check if optimization is required

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 2761 bytes --]

Junio C Hamano <gitster@pobox.com> writes:

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> The reftable backend performs auto-compaction as part of its regular
>> flow, which is required to keep the number of tables part of a stack at
>> bay. This allows it to stay optimized.
>
> Sounds very sensible.
>
>> Compaction can also be triggered voluntarily by the user via the 'git
>> pack-refs' or the 'git refs optimize' command. However, currently there
>> is no way for the user to check if optimization is required without
>> actually performing it.
>
> Sounds very sensible goal.
>
> But where is the existing logic to decide when it needs to
> auto-compact, performed as part of its regular flow?
>
> After reading "the reftable machinery already decides when it needs
> to compact and does so" plus "but the logic to decide is not made
> available to users", I would have expected for this patch to extract
> such an existing logic or otherwise make it available to new callers
> so that things like "gc --auto" can call it, but the diffstat shows
> mostly additions, which does not give readers any confidence in the
> new function that answers "do we need compaction?".  It would give
> _an_ answer, but there is no clue if the answer it gives is the same
> answer as the existing logic that decides when to compact as part of
> the regular operation.
>
> I am puzzled.
>
>> +int reftable_stack_compaction_required(struct reftable_stack *st,
>> +				       bool use_heuristics,
>> +				       bool *required)
>> +{
>> +	struct segment seg;
>> +	int err = 0;
>> +
>> +	if (st->merged->tables_len < 2) {
>> +		*required = false;
>> +		return 0;
>> +	}
>> +
>> +	if (!use_heuristics) {
>> +		*required = true;
>> +		return 0;
>> +	}
>> +
>> +	err = stack_segments_for_compaction(st, &seg);
>> +	if (err)
>> +		return err;
>> +
>> +	*required = segment_size(&seg) > 0;
>> +	return 0;
>> +}
>
> Specifically, where is the above logic come from?  Is it duplicating
> an existing logic but that code is hard to separate out into this
> helper?
>

Good question.

Most of this logic is already part of 'reftable_stack_auto_compact()'.
We also have another similar function 'reftable_stack_compact_all()'.
The former is used for compaction based on heuristics and the latter is
for compacting all tables into one. In the refs subsystem usage of
heuristics is denoted by the usage of the 'REFS_OPTIMIZE_AUTO' flag.

The function we're introducing allows users to explicitly mention if
they want to use heuristics or not. This allows us to differentiate
between the two modes. The result of which is that this uses intertwined
logic of the two existing functions. Hence we can't extract any code out.

I'll add this information in the commit message.

Karthik

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH v2 2/5] reftable/stack: add function to check if optimization is required

[Junio C Hamano]

Karthik Nayak <karthik.188@gmail.com> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Karthik Nayak <karthik.188@gmail.com> writes:
>>
>>> The reftable backend performs auto-compaction as part of its regular
>>> flow, which is required to keep the number of tables part of a stack at
>>> bay. This allows it to stay optimized.
>>
>> Sounds very sensible.
>>
>>> Compaction can also be triggered voluntarily by the user via the 'git
>>> pack-refs' or the 'git refs optimize' command. However, currently there
>>> is no way for the user to check if optimization is required without
>>> actually performing it.
>>
>> Sounds very sensible goal.
>>
>> But where is the existing logic to decide when it needs to
>> auto-compact, performed as part of its regular flow?
>>
>> After reading "the reftable machinery already decides when it needs
>> to compact and does so" plus "but the logic to decide is not made
>> available to users", I would have expected for this patch to extract
>> such an existing logic or otherwise make it available to new callers
>> so that things like "gc --auto" can call it, but the diffstat shows
>> mostly additions, which does not give readers any confidence in the
>> new function that answers "do we need compaction?".  It would give
>> _an_ answer, but there is no clue if the answer it gives is the same
>> answer as the existing logic that decides when to compact as part of
>> the regular operation.
>>
>> I am puzzled.
>>
>>> +int reftable_stack_compaction_required(struct reftable_stack *st,
>>> +				       bool use_heuristics,
>>> +				       bool *required)
>>> +{
>>> +	struct segment seg;
>>> +	int err = 0;
>>> +
>>> +	if (st->merged->tables_len < 2) {
>>> +		*required = false;
>>> +		return 0;
>>> +	}
>>> +
>>> +	if (!use_heuristics) {
>>> +		*required = true;
>>> +		return 0;
>>> +	}
>>> +
>>> +	err = stack_segments_for_compaction(st, &seg);
>>> +	if (err)
>>> +		return err;
>>> +
>>> +	*required = segment_size(&seg) > 0;
>>> +	return 0;
>>> +}
>>
>> Specifically, where is the above logic come from?  Is it duplicating
>> an existing logic but that code is hard to separate out into this
>> helper?
>>
>
> Good question.
>
> Most of this logic is already part of 'reftable_stack_auto_compact()'.
> We also have another similar function 'reftable_stack_compact_all()'.
> The former is used for compaction based on heuristics and the latter is
> for compacting all tables into one. In the refs subsystem usage of
> heuristics is denoted by the usage of the 'REFS_OPTIMIZE_AUTO' flag.
>
> The function we're introducing allows users to explicitly mention if
> they want to use heuristics or not. This allows us to differentiate
> between the two modes. The result of which is that this uses intertwined
> logic of the two existing functions. Hence we can't extract any code out.
>
> I'll add this information in the commit message.

You mean you already have two duplicate implementations whose
definition of "when should we compact?" can drift apart over time
(worse, they may already be subtly different), and you are adding
yet another one?

Instead of describing such an insanity in the commit message, can we
refactor to have a single central logic that is used from three
places?

Thanks.

Re: [PATCH v2 2/5] reftable/stack: add function to check if optimization is required

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 3660 bytes --]

Junio C Hamano <gitster@pobox.com> writes:

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> Karthik Nayak <karthik.188@gmail.com> writes:
>>>
>>>> The reftable backend performs auto-compaction as part of its regular
>>>> flow, which is required to keep the number of tables part of a stack at
>>>> bay. This allows it to stay optimized.
>>>
>>> Sounds very sensible.
>>>
>>>> Compaction can also be triggered voluntarily by the user via the 'git
>>>> pack-refs' or the 'git refs optimize' command. However, currently there
>>>> is no way for the user to check if optimization is required without
>>>> actually performing it.
>>>
>>> Sounds very sensible goal.
>>>
>>> But where is the existing logic to decide when it needs to
>>> auto-compact, performed as part of its regular flow?
>>>
>>> After reading "the reftable machinery already decides when it needs
>>> to compact and does so" plus "but the logic to decide is not made
>>> available to users", I would have expected for this patch to extract
>>> such an existing logic or otherwise make it available to new callers
>>> so that things like "gc --auto" can call it, but the diffstat shows
>>> mostly additions, which does not give readers any confidence in the
>>> new function that answers "do we need compaction?".  It would give
>>> _an_ answer, but there is no clue if the answer it gives is the same
>>> answer as the existing logic that decides when to compact as part of
>>> the regular operation.
>>>
>>> I am puzzled.
>>>
>>>> +int reftable_stack_compaction_required(struct reftable_stack *st,
>>>> +				       bool use_heuristics,
>>>> +				       bool *required)
>>>> +{
>>>> +	struct segment seg;
>>>> +	int err = 0;
>>>> +
>>>> +	if (st->merged->tables_len < 2) {
>>>> +		*required = false;
>>>> +		return 0;
>>>> +	}
>>>> +
>>>> +	if (!use_heuristics) {
>>>> +		*required = true;
>>>> +		return 0;
>>>> +	}
>>>> +
>>>> +	err = stack_segments_for_compaction(st, &seg);
>>>> +	if (err)
>>>> +		return err;
>>>> +
>>>> +	*required = segment_size(&seg) > 0;
>>>> +	return 0;
>>>> +}
>>>
>>> Specifically, where is the above logic come from?  Is it duplicating
>>> an existing logic but that code is hard to separate out into this
>>> helper?
>>>
>>
>> Good question.
>>
>> Most of this logic is already part of 'reftable_stack_auto_compact()'.
>> We also have another similar function 'reftable_stack_compact_all()'.
>> The former is used for compaction based on heuristics and the latter is
>> for compacting all tables into one. In the refs subsystem usage of
>> heuristics is denoted by the usage of the 'REFS_OPTIMIZE_AUTO' flag.
>>
>> The function we're introducing allows users to explicitly mention if
>> they want to use heuristics or not. This allows us to differentiate
>> between the two modes. The result of which is that this uses intertwined
>> logic of the two existing functions. Hence we can't extract any code out.
>>
>> I'll add this information in the commit message.
>
> You mean you already have two duplicate implementations whose
> definition of "when should we compact?" can drift apart over time
> (worse, they may already be subtly different), and you are adding
> yet another one?

More like we have two functions:
1. compact all tables into one
2. compact based on heuristics

This function oversees logic from both.

> Instead of describing such an insanity in the commit message, can we
> refactor to have a single central logic that is used from three
> places?
>
> Thanks.

You're right though, I did manage to extract out the common code and
will send in a new version. Thanks for the push.

- Karthik

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

[PATCH v2 3/5] refs: add a `optimize_required` field to `struct ref_storage_be`

[Karthik Nayak]

To allow users of the refs namespace to check if the reference backend
requires optimization, add a new field `optimize_required` field to
`struct ref_storage_be`. This field is of type `optimize_required_fn`
which is also introduced in this commit.

Modify the debug, files, packed and reftable backend to implement this
field. A following commit will expose this via 'git pack-refs' and 'git
refs optimize'.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 refs.c                  |  7 +++++++
 refs.h                  |  7 +++++++
 refs/debug.c            | 13 +++++++++++++
 refs/files-backend.c    | 11 +++++++++++
 refs/packed-backend.c   | 13 +++++++++++++
 refs/refs-internal.h    |  6 ++++++
 refs/reftable-backend.c | 25 +++++++++++++++++++++++++
 7 files changed, 82 insertions(+)

diff --git a/refs.c b/refs.c
index 0d0831f29b..5583f6e09d 100644
--- a/refs.c
+++ b/refs.c
@@ -2318,6 +2318,13 @@ int refs_optimize(struct ref_store *refs, struct refs_optimize_opts *opts)
 	return refs->be->optimize(refs, opts);
 }
 
+int refs_optimize_required(struct ref_store *refs,
+			   struct refs_optimize_opts *opts,
+			   bool *required)
+{
+	return refs->be->optimize_required(refs, opts, required);
+}
+
 int reference_get_peeled_oid(struct repository *repo,
 			     const struct reference *ref,
 			     struct object_id *peeled_oid)
diff --git a/refs.h b/refs.h
index 6b05bba527..d9051bbb04 100644
--- a/refs.h
+++ b/refs.h
@@ -520,6 +520,13 @@ struct refs_optimize_opts {
  */
 int refs_optimize(struct ref_store *refs, struct refs_optimize_opts *opts);
 
+/*
+ * Check if refs backend can be optimized by calling 'refs_optimize'.
+ */
+int refs_optimize_required(struct ref_store *ref_store,
+			   struct refs_optimize_opts *opts,
+			   bool *required);
+
 /*
  * Setup reflog before using. Fill in err and return -1 on failure.
  */
diff --git a/refs/debug.c b/refs/debug.c
index 2defd2d465..36f8c58b6c 100644
--- a/refs/debug.c
+++ b/refs/debug.c
@@ -124,6 +124,17 @@ static int debug_optimize(struct ref_store *ref_store, struct refs_optimize_opts
 	return res;
 }
 
+static int debug_optimize_required(struct ref_store *ref_store,
+				   struct refs_optimize_opts *opts,
+				   bool *required)
+{
+	struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;
+	int res = drefs->refs->be->optimize_required(drefs->refs, opts, required);
+	trace_printf_key(&trace_refs, "optimize_required: %s, res: %d\n",
+			 required ? "yes" : "no", res);
+	return res;
+}
+
 static int debug_rename_ref(struct ref_store *ref_store, const char *oldref,
 			    const char *newref, const char *logmsg)
 {
@@ -431,6 +442,8 @@ struct ref_storage_be refs_be_debug = {
 	.transaction_abort = debug_transaction_abort,
 
 	.optimize = debug_optimize,
+	.optimize_required = debug_optimize_required,
+
 	.rename_ref = debug_rename_ref,
 	.copy_ref = debug_copy_ref,
 
diff --git a/refs/files-backend.c b/refs/files-backend.c
index a1e70b1c10..6e0c9b340a 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1512,6 +1512,16 @@ static int files_optimize(struct ref_store *ref_store,
 	return 0;
 }
 
+static int files_optimize_required(struct ref_store *ref_store,
+				   struct refs_optimize_opts *opts,
+				   bool *required)
+{
+	struct files_ref_store *refs = files_downcast(ref_store, REF_STORE_READ,
+						      "optimize_required");
+	*required = should_pack_refs(refs, opts);
+	return 0;
+}
+
 /*
  * People using contrib's git-new-workdir have .git/logs/refs ->
  * /some/other/path/.git/logs/refs, and that may live on another device.
@@ -3982,6 +3992,7 @@ struct ref_storage_be refs_be_files = {
 	.transaction_abort = files_transaction_abort,
 
 	.optimize = files_optimize,
+	.optimize_required = files_optimize_required,
 	.rename_ref = files_rename_ref,
 	.copy_ref = files_copy_ref,
 
diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index 10062fd8b6..19ce4d5872 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -1784,6 +1784,17 @@ static int packed_optimize(struct ref_store *ref_store UNUSED,
 	return 0;
 }
 
+static int packed_optimize_required(struct ref_store *ref_store UNUSED,
+				    struct refs_optimize_opts *opts UNUSED,
+				    bool *required)
+{
+	/*
+	 * Packed refs are already optimized.
+	 */
+	*required = false;
+	return 0;
+}
+
 static struct ref_iterator *packed_reflog_iterator_begin(struct ref_store *ref_store UNUSED)
 {
 	return empty_ref_iterator_begin();
@@ -2130,6 +2141,8 @@ struct ref_storage_be refs_be_packed = {
 	.transaction_abort = packed_transaction_abort,
 
 	.optimize = packed_optimize,
+	.optimize_required = packed_optimize_required,
+
 	.rename_ref = NULL,
 	.copy_ref = NULL,
 
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index dee42f231d..c7d2a6e50b 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -424,6 +424,11 @@ typedef int ref_transaction_commit_fn(struct ref_store *refs,
 
 typedef int optimize_fn(struct ref_store *ref_store,
 			struct refs_optimize_opts *opts);
+
+typedef int optimize_required_fn(struct ref_store *ref_store,
+				 struct refs_optimize_opts *opts,
+				 bool *required);
+
 typedef int rename_ref_fn(struct ref_store *ref_store,
 			  const char *oldref, const char *newref,
 			  const char *logmsg);
@@ -549,6 +554,7 @@ struct ref_storage_be {
 	ref_transaction_abort_fn *transaction_abort;
 
 	optimize_fn *optimize;
+	optimize_required_fn *optimize_required;
 	rename_ref_fn *rename_ref;
 	copy_ref_fn *copy_ref;
 
diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index c23c45f3bf..a3ae0cf74a 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -1733,6 +1733,29 @@ static int reftable_be_optimize(struct ref_store *ref_store,
 	return ret;
 }
 
+static int reftable_be_optimize_required(struct ref_store *ref_store,
+					 struct refs_optimize_opts *opts,
+					 bool *required)
+{
+	struct reftable_ref_store *refs = reftable_be_downcast(ref_store, REF_STORE_READ,
+							       "optimize_refs_required");
+	struct reftable_stack *stack;
+	bool use_heuristics = false;
+
+	if (refs->err)
+		return refs->err;
+
+	stack = refs->worktree_backend.stack;
+	if (!stack)
+		stack = refs->main_backend.stack;
+
+	if (opts->flags & REFS_OPTIMIZE_AUTO)
+		use_heuristics = true;
+
+	return reftable_stack_compaction_required(stack, use_heuristics,
+						  required);
+}
+
 struct write_create_symref_arg {
 	struct reftable_ref_store *refs;
 	struct reftable_stack *stack;
@@ -2756,6 +2779,8 @@ struct ref_storage_be refs_be_reftable = {
 	.transaction_abort = reftable_be_transaction_abort,
 
 	.optimize = reftable_be_optimize,
+	.optimize_required = reftable_be_optimize_required,
+
 	.rename_ref = reftable_be_rename_ref,
 	.copy_ref = reftable_be_copy_ref,
 

-- 
2.51.0

[PATCH v2 4/5] maintenance: add checking logic in `pack_refs_condition()`

[Karthik Nayak]

The 'git-maintenance(1)' command supports an '--auto' flag. Usage of the
flag ensures to run maintenance tasks only if certain thresholds are
met. The heuristic is defined on a task level, wherein each task defines
an 'auto_condition', which states if the task should be run.

The 'pack-refs' task is hard-coded to return 1 as:
1. There was never a way to check if the reference backend needs to be
optimized without actually performing the optimization.
2. We can pass in the '--auto' flag to 'git-pack-refs(1)' which would
optimize based on heuristics.

The previous commit added a `refs_optimize_required()` function, which
can be used to check if a reference backend required optimization. Use
this within `pack_refs_condition()`.

This allows us to add a 'git maintenance is-needed' subcommand which can
notify the user if maintenance is needed without actually performing the
optimization. Without this change, the reference backend would always
state that optimization is needed.

Since we import 'revision.h', we need to remove the definition for
'SEEN' which is duplicated in the included header.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 builtin/gc.c | 30 +++++++++++++++++++++---------
 object.h     |  1 -
 2 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index c6d62c74a7..c3e7a84ec2 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -35,6 +35,7 @@
 #include "path.h"
 #include "reflog.h"
 #include "rerere.h"
+#include "revision.h"
 #include "blob.h"
 #include "tree.h"
 #include "promisor-remote.h"
@@ -285,12 +286,26 @@ static void maintenance_run_opts_release(struct maintenance_run_opts *opts)
 
 static int pack_refs_condition(UNUSED struct gc_config *cfg)
 {
-	/*
-	 * The auto-repacking logic for refs is handled by the ref backends and
-	 * exposed via `git pack-refs --auto`. We thus always return truish
-	 * here and let the backend decide for us.
-	 */
-	return 1;
+	struct string_list included_refs = STRING_LIST_INIT_NODUP;
+	struct ref_exclusions excludes = REF_EXCLUSIONS_INIT;
+	struct refs_optimize_opts optimize_opts = {
+		.exclusions = &excludes,
+		.includes = &included_refs,
+		.flags = REFS_OPTIMIZE_PRUNE | REFS_OPTIMIZE_AUTO,
+	};
+	bool required;
+
+	/* Check for all refs, similar to 'git refs optimize --all'. */
+	string_list_append(optimize_opts.includes, "*");
+
+	if (refs_optimize_required(get_main_ref_store(the_repository),
+				   &optimize_opts, &required))
+		return 0;
+
+	clear_ref_exclusions(&excludes);
+	string_list_clear(&included_refs, 0);
+
+	return required == true;
 }
 
 static int maintenance_task_pack_refs(struct maintenance_run_opts *opts,
@@ -1090,9 +1105,6 @@ static int maintenance_opt_schedule(const struct option *opt, const char *arg,
 	return 0;
 }
 
-/* Remember to update object flag allocation in object.h */
-#define SEEN		(1u<<0)
-
 struct cg_auto_data {
 	int num_not_in_graph;
 	int limit;
diff --git a/object.h b/object.h
index 1499f63d50..832299e763 100644
--- a/object.h
+++ b/object.h
@@ -79,7 +79,6 @@ void object_array_init(struct object_array *array);
  * list-objects-filter.c:                                      21
  * bloom.c:                                                    2122
  * builtin/fsck.c:           0--3
- * builtin/gc.c:             0
  * builtin/index-pack.c:                                     2021
  * reflog.c:                           10--12
  * builtin/show-branch.c:    0-------------------------------------------26

-- 
2.51.0

[PATCH v2 5/5] maintenance: add 'is-needed' subcommand

[Karthik Nayak]

The 'git-maintenance(1)' command provides tooling to run maintenance
tasks over Git repositories. The 'run' subcommand, as the name suggests,
runs the maintenance tasks. When used with the '--auto' flag, it uses
heuristics to determine if the required thresholds are met for running
said maintenance tasks.

There is however a lack of insight into these heuristics. Meaning, the
checks are linked to the execution.

Add a new 'is-needed' subcommand to 'git-maintenance(1)' which allows
users to simply check if it is needed to run maintenance without
performing it.

This subcommand can check if it is needed to run maintenance without
actually running it. Ideally it should be used with the '--auto' flag,
which would allow users to check if the thresholds required are met. The
subcommand also supports the '--task' flag which can be used to check
specific maintenance tasks.

While adding the respective tests in 't/t7900-maintenance.sh', remove a
duplicate of the test: 'worktree-prune task with --auto honors
maintenance.worktree-prune.auto'.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 Documentation/git-maintenance.adoc | 13 +++++++++
 builtin/gc.c                       | 55 +++++++++++++++++++++++++++++++++++++-
 t/t7900-maintenance.sh             | 54 ++++++++++++++++++++++++++-----------
 3 files changed, 105 insertions(+), 17 deletions(-)

diff --git a/Documentation/git-maintenance.adoc b/Documentation/git-maintenance.adoc
index 540b5cf68b..37939510d4 100644
--- a/Documentation/git-maintenance.adoc
+++ b/Documentation/git-maintenance.adoc
@@ -12,6 +12,7 @@ SYNOPSIS
 'git maintenance' run [<options>]
 'git maintenance' start [--scheduler=<scheduler>]
 'git maintenance' (stop|register|unregister) [<options>]
+'git maintenance' is-needed [<options>]
 
 
 DESCRIPTION
@@ -84,6 +85,16 @@ 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.
+    Ideally used with the '--auto' flag.
++
+If one or more `--task` options	are specified, then those tasks are checked
+in that order. Otherwise, the tasks are determined by which
+`maintenance.<task>.enabled` config options are true. By default, only
+`maintenance.gc.enabled` is true.
+
 TASKS
 -----
 
@@ -183,6 +194,8 @@ OPTIONS
 	in the `gc.auto` config setting, or when the number of pack-files
 	exceeds the `gc.autoPackLimit` config setting. Not compatible with
 	the `--schedule` option.
+	When combined with the `is-needed` subcommand, check if the required
+	thresholds are met without actually running maintenance.
 
 --schedule::
 	When combined with the `run` subcommand, run maintenance tasks
diff --git a/builtin/gc.c b/builtin/gc.c
index c3e7a84ec2..e5ba2a2e72 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -3253,7 +3253,59 @@ static int maintenance_stop(int argc, const char **argv, const char *prefix,
 	return update_background_schedule(NULL, 0);
 }
 
-static const char * const builtin_maintenance_usage[] = {
+static const char *const builtin_maintenance_is_needed_usage[] = {
+	"git maintenance is-needed [--task=<task>] [--schedule]",
+	NULL
+};
+
+static int maintenance_is_needed(int argc, const char **argv, const char *prefix,
+				 struct repository *repo UNUSED)
+{
+	struct maintenance_run_opts opts = MAINTENANCE_RUN_OPTS_INIT;
+	struct string_list selected_tasks = STRING_LIST_INIT_DUP;
+	struct gc_config cfg = GC_CONFIG_INIT;
+	struct option options[] = {
+		OPT_BOOL(0, "auto", &opts.auto_flag,
+			 N_("run tasks based on the state of the repository")),
+		OPT_CALLBACK_F(0, "task", &selected_tasks, N_("task"),
+			       N_("check a specific task"),
+			       PARSE_OPT_NONEG, task_option_parse),
+		OPT_END()
+	};
+	bool is_needed = false;
+
+	argc = parse_options(argc, argv, prefix, options,
+			     builtin_maintenance_is_needed_usage,
+			     PARSE_OPT_STOP_AT_NON_OPTION);
+	if (argc)
+		usage_with_options(builtin_maintenance_is_needed_usage, options);
+
+	gc_config(&cfg);
+	initialize_task_config(&opts, &selected_tasks);
+
+	if (opts.auto_flag) {
+		for (size_t i = 0; i < opts.tasks_nr; i++) {
+			if (tasks[opts.tasks[i]].auto_condition &&
+			    tasks[opts.tasks[i]].auto_condition(&cfg)) {
+				is_needed = true;
+				break;
+			}
+		}
+	} else {
+		/* When not using --auto, we should always require maintenance. */
+		is_needed = true;
+	}
+
+	string_list_clear(&selected_tasks, 0);
+	maintenance_run_opts_release(&opts);
+	gc_config_release(&cfg);
+
+	if (is_needed)
+		return 0;
+	return 1;
+}
+
+static const char *const builtin_maintenance_usage[] = {
 	N_("git maintenance <subcommand> [<options>]"),
 	NULL,
 };
@@ -3270,6 +3322,7 @@ int cmd_maintenance(int argc,
 		OPT_SUBCOMMAND("stop", &fn, maintenance_stop),
 		OPT_SUBCOMMAND("register", &fn, maintenance_register),
 		OPT_SUBCOMMAND("unregister", &fn, maintenance_unregister),
+		OPT_SUBCOMMAND("is-needed", &fn, maintenance_is_needed),
 		OPT_END(),
 	};
 
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index ddd273d8dc..a17e2091c2 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -49,7 +49,9 @@ test_expect_success 'run [--auto|--quiet]' '
 		git maintenance run --auto 2>/dev/null &&
 	GIT_TRACE2_EVENT="$(pwd)/run-no-quiet.txt" \
 		git maintenance run --no-quiet 2>/dev/null &&
+	git maintenance is-needed &&
 	test_subcommand git gc --quiet --no-detach --skip-foreground-tasks <run-no-auto.txt &&
+	! git maintenance is-needed --auto &&
 	test_subcommand ! git gc --auto --quiet --no-detach --skip-foreground-tasks <run-auto.txt &&
 	test_subcommand git gc --no-quiet --no-detach --skip-foreground-tasks <run-no-quiet.txt
 '
@@ -180,6 +182,11 @@ test_expect_success 'commit-graph auto condition' '
 
 	test_commit first &&
 
+	! git -c maintenance.commit-graph.auto=0 \
+		maintenance is-needed --auto --task=commit-graph &&
+	git -c maintenance.commit-graph.auto=1 \
+		maintenance is-needed --auto --task=commit-graph &&
+
 	GIT_TRACE2_EVENT="$(pwd)/cg-zero-means-no.txt" \
 		git -c maintenance.commit-graph.auto=0 $COMMAND &&
 	GIT_TRACE2_EVENT="$(pwd)/cg-one-satisfied.txt" \
@@ -290,16 +297,23 @@ test_expect_success 'maintenance.loose-objects.auto' '
 		git -c maintenance.loose-objects.auto=1 maintenance \
 		run --auto --task=loose-objects 2>/dev/null &&
 	test_subcommand ! git prune-packed --quiet <trace-lo1.txt &&
+
 	printf data-A | git hash-object -t blob --stdin -w &&
+	! git -c maintenance.loose-objects.auto=2 \
+		maintenance is-needed --auto --task=loose-objects &&
 	GIT_TRACE2_EVENT="$(pwd)/trace-loA" \
 		git -c maintenance.loose-objects.auto=2 \
 		maintenance run --auto --task=loose-objects 2>/dev/null &&
 	test_subcommand ! git prune-packed --quiet <trace-loA &&
+
 	printf data-B | git hash-object -t blob --stdin -w &&
+	git -c maintenance.loose-objects.auto=2 \
+		maintenance is-needed --auto --task=loose-objects &&
 	GIT_TRACE2_EVENT="$(pwd)/trace-loB" \
 		git -c maintenance.loose-objects.auto=2 \
 		maintenance run --auto --task=loose-objects 2>/dev/null &&
 	test_subcommand git prune-packed --quiet <trace-loB &&
+
 	GIT_TRACE2_EVENT="$(pwd)/trace-loC" \
 		git -c maintenance.loose-objects.auto=2 \
 		maintenance run --auto --task=loose-objects 2>/dev/null &&
@@ -421,10 +435,13 @@ run_incremental_repack_and_verify () {
 	test_commit A &&
 	git repack -adk &&
 	git multi-pack-index write &&
+	! git -c maintenance.incremental-repack.auto=1 \
+		maintenance is-needed --auto --task=incremental-repack &&
 	GIT_TRACE2_EVENT="$(pwd)/midx-init.txt" git \
 		-c maintenance.incremental-repack.auto=1 \
 		maintenance run --auto --task=incremental-repack 2>/dev/null &&
 	test_subcommand ! git multi-pack-index write --no-progress <midx-init.txt &&
+
 	test_commit B &&
 	git pack-objects --revs .git/objects/pack/pack <<-\EOF &&
 	HEAD
@@ -434,11 +451,14 @@ run_incremental_repack_and_verify () {
 		-c maintenance.incremental-repack.auto=2 \
 		maintenance run --auto --task=incremental-repack 2>/dev/null &&
 	test_subcommand ! git multi-pack-index write --no-progress <trace-A &&
+
 	test_commit C &&
 	git pack-objects --revs .git/objects/pack/pack <<-\EOF &&
 	HEAD
 	^HEAD~1
 	EOF
+	git -c maintenance.incremental-repack.auto=2 \
+		maintenance is-needed --auto --task=incremental-repack &&
 	GIT_TRACE2_EVENT=$(pwd)/trace-B git \
 		-c maintenance.incremental-repack.auto=2 \
 		maintenance run --auto --task=incremental-repack 2>/dev/null &&
@@ -485,9 +505,15 @@ test_expect_success 'reflog-expire task --auto only packs when exceeding limits'
 	git reflog expire --all --expire=now &&
 	test_commit reflog-one &&
 	test_commit reflog-two &&
+
+	! git -c maintenance.reflog-expire.auto=3 \
+		maintenance is-needed --auto --task=reflog-expire &&
 	GIT_TRACE2_EVENT="$(pwd)/reflog-expire-auto.txt" \
 		git -c maintenance.reflog-expire.auto=3 maintenance run --auto --task=reflog-expire &&
 	test_subcommand ! git reflog expire --all <reflog-expire-auto.txt &&
+
+	git -c maintenance.reflog-expire.auto=2 \
+		maintenance is-needed --auto --task=reflog-expire &&
 	GIT_TRACE2_EVENT="$(pwd)/reflog-expire-auto.txt" \
 		git -c maintenance.reflog-expire.auto=2 maintenance run --auto --task=reflog-expire &&
 	test_subcommand git reflog expire --all <reflog-expire-auto.txt
@@ -514,6 +540,7 @@ test_expect_success 'worktree-prune task --auto only prunes with prunable worktr
 	test_expect_worktree_prune ! git maintenance run --auto --task=worktree-prune &&
 	mkdir .git/worktrees &&
 	: >.git/worktrees/abc &&
+	git maintenance is-needed --auto --task=worktree-prune &&
 	test_expect_worktree_prune git maintenance run --auto --task=worktree-prune
 '
 
@@ -530,22 +557,7 @@ test_expect_success 'worktree-prune task with --auto honors maintenance.worktree
 	test_expect_worktree_prune ! git -c maintenance.worktree-prune.auto=0 maintenance run --auto --task=worktree-prune &&
 	# A positive value should require at least this many prunable worktrees.
 	test_expect_worktree_prune ! git -c maintenance.worktree-prune.auto=4 maintenance run --auto --task=worktree-prune &&
-	test_expect_worktree_prune git -c maintenance.worktree-prune.auto=3 maintenance run --auto --task=worktree-prune
-'
-
-test_expect_success 'worktree-prune task with --auto honors maintenance.worktree-prune.auto' '
-	# A negative value should always prune.
-	test_expect_worktree_prune git -c maintenance.worktree-prune.auto=-1 maintenance run --auto --task=worktree-prune &&
-
-	mkdir .git/worktrees &&
-	: >.git/worktrees/first &&
-	: >.git/worktrees/second &&
-	: >.git/worktrees/third &&
-
-	# Zero should never prune.
-	test_expect_worktree_prune ! git -c maintenance.worktree-prune.auto=0 maintenance run --auto --task=worktree-prune &&
-	# A positive value should require at least this many prunable worktrees.
-	test_expect_worktree_prune ! git -c maintenance.worktree-prune.auto=4 maintenance run --auto --task=worktree-prune &&
+	git -c maintenance.worktree-prune.auto=3 maintenance is-needed --auto --task=worktree-prune &&
 	test_expect_worktree_prune git -c maintenance.worktree-prune.auto=3 maintenance run --auto --task=worktree-prune
 '
 
@@ -554,11 +566,13 @@ test_expect_success 'worktree-prune task honors gc.worktreePruneExpire' '
 	rm -rf worktree &&
 
 	rm -f worktree-prune.txt &&
+	! git -c gc.worktreePruneExpire=1.week.ago maintenance is-needed --auto --task=worktree-prune &&
 	GIT_TRACE2_EVENT="$(pwd)/worktree-prune.txt" git -c gc.worktreePruneExpire=1.week.ago maintenance run --auto --task=worktree-prune &&
 	test_subcommand ! git worktree prune --expire 1.week.ago <worktree-prune.txt &&
 	test_path_is_dir .git/worktrees/worktree &&
 
 	rm -f worktree-prune.txt &&
+	git -c gc.worktreePruneExpire=now maintenance is-needed --auto --task=worktree-prune &&
 	GIT_TRACE2_EVENT="$(pwd)/worktree-prune.txt" git -c gc.worktreePruneExpire=now maintenance run --auto --task=worktree-prune &&
 	test_subcommand git worktree prune --expire now <worktree-prune.txt &&
 	test_path_is_missing .git/worktrees/worktree
@@ -583,10 +597,13 @@ test_expect_success 'rerere-gc task without --auto always collects garbage' '
 
 test_expect_success 'rerere-gc task with --auto only prunes with prunable entries' '
 	test_when_finished "rm -rf .git/rr-cache" &&
+	! git maintenance is-needed --auto --task=rerere-gc &&
 	test_expect_rerere_gc ! git maintenance run --auto --task=rerere-gc &&
 	mkdir .git/rr-cache &&
+	! git maintenance is-needed --auto --task=rerere-gc &&
 	test_expect_rerere_gc ! git maintenance run --auto --task=rerere-gc &&
 	: >.git/rr-cache/entry &&
+	git maintenance is-needed --auto --task=rerere-gc &&
 	test_expect_rerere_gc git maintenance run --auto --task=rerere-gc
 '
 
@@ -594,17 +611,22 @@ test_expect_success 'rerere-gc task with --auto honors maintenance.rerere-gc.aut
 	test_when_finished "rm -rf .git/rr-cache" &&
 
 	# A negative value should always prune.
+	git -c maintenance.rerere-gc.auto=-1 maintenance is-needed --auto --task=rerere-gc &&
 	test_expect_rerere_gc git -c maintenance.rerere-gc.auto=-1 maintenance run --auto --task=rerere-gc &&
 
 	# A positive value prunes when there is at least one entry.
+	! git -c maintenance.rerere-gc.auto=9000 maintenance is-needed --auto --task=rerere-gc &&
 	test_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=9000 maintenance run --auto --task=rerere-gc &&
 	mkdir .git/rr-cache &&
+	! git -c maintenance.rerere-gc.auto=9000 maintenance is-needed --auto --task=rerere-gc &&
 	test_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=9000 maintenance run --auto --task=rerere-gc &&
 	: >.git/rr-cache/entry-1 &&
+	git -c maintenance.rerere-gc.auto=9000 maintenance is-needed --auto --task=rerere-gc &&
 	test_expect_rerere_gc git -c maintenance.rerere-gc.auto=9000 maintenance run --auto --task=rerere-gc &&
 
 	# Zero should never prune.
 	: >.git/rr-cache/entry-1 &&
+	! git -c maintenance.rerere-gc.auto=0 maintenance is-needed --auto --task=rerere-gc &&
 	test_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=0 maintenance run --auto --task=rerere-gc
 '
 

-- 
2.51.0

Re: [PATCH v2 0/5] maintenance: add an 'is-needed' subcommand

[Junio C Hamano]

Karthik Nayak <karthik.188@gmail.com> writes:

> This is based on top of master a99f379adf (The 27th batch, 2025-10-30)
> and is dependent on the following series:
>
>     - kn/refs-optim-cleanup
>     - ps/ref-peeled-tags

Yuck.  ps/ref-peeled-tags needed an update so kn/refs-optim-cleanup
that depends on it needs rebuilding on top (no action needed from
your side, but somebody is doing the necessary rebasing somewhere),
and then these five patches need to be queued on top, which will
require further shuffling when any of these two series need to be
updated again.

I expect that during the pre-release freeze things will be slowing
down, so I'll manage and survive ;-)

Will queue.  Thanks.

Re: [PATCH v2 0/5] maintenance: add an 'is-needed' subcommand

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 902 bytes --]

Junio C Hamano <gitster@pobox.com> writes:

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> This is based on top of master a99f379adf (The 27th batch, 2025-10-30)
>> and is dependent on the following series:
>>
>>     - kn/refs-optim-cleanup
>>     - ps/ref-peeled-tags
>
> Yuck.  ps/ref-peeled-tags needed an update so kn/refs-optim-cleanup
> that depends on it needs rebuilding on top (no action needed from
> your side, but somebody is doing the necessary rebasing somewhere),
> and then these five patches need to be queued on top, which will
> require further shuffling when any of these two series need to be
> updated again.
>

I know, and I must thank you in this regard for putting up with this.
This dependency chain is certainly not pleasant.

> I expect that during the pre-release freeze things will be slowing
> down, so I'll manage and survive ;-)
>
> Will queue.  Thanks.

Thanks!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

[PATCH v3 0/5] maintenance: add an 'is-needed' subcommand

[Karthik Nayak]

Hello,

I recently raised a patch series [1] to add 'git refs optimize --required'
which checks if the reference backend can be optimized, without actually
performing the optimization.

Back then, we had decided [2] that it would be a better to broaden the
approach and add a 'is-needed' subcommand to 'git-maintenance(1)'. This
would allow users to check if maintenance was required for the
repository and users could also provide a task via the '--task' to check
if maintenance was needed for a particular task.

Ideally the subcommand will be used with the '--auto' flag which can
check the same heuristics as that used with 'git maintenance run
--auto'. Future patches can also add support for the '--schedule' flag
which can be used to check required schedule it met. However that flag
isn't added as part of this series.

This series implements that.

Commits 1-3 add the required functionality in the refs subsystem to
expose an 'optimize_required' field which can be used to check if
backends need to be optimized.
Commit 4 utilizes this within the 'git-maintenance(1)' code.
Commit 5 adds the 'is-needed' subcommand to 'git-maintenance(1)'.

This is based on top of master a99f379adf (The 27th batch, 2025-10-30)
and is dependent on the following series:

    - kn/refs-optim-cleanup
    - ps/ref-peeled-tags

Merges cleanly with `next`. I think those two topics are close to being
merged to `next` so hopefully this dependency tree doesn't get too
complicated. I'll rebase as needed to resolve conflicts.

[1]: https://lore.kernel.org/git/20251010-562-add-option-to-check-if-reference-backend-needs-repacking-v1-0-c7962be584fa@gmail.com/
[2]: https://lore.kernel.org/git/CAOLa=ZRdxm787nE4FSr2VUHDB+hW06Ggc6yUcKmeTKAb6B7YOA@mail.gmail.com/

---
Changes in v3:
- In patch 2/5 extract out code for deciding if compaction is required
  into a static function. This removes duplication of logic for deciding
  if compaction is needed.
- Link to v2: https://patch.msgid.link/20251104-562-add-sub-command-to-check-if-maintenance-is-needed-v2-0-303462a9e4ed@gmail.com

Changes in v2:
- Added more documentation for `reftable_stack_compaction_required()`.
- Fixed some typos and grammar mistakes in commit messages.
- Clarify which tasks will be run when '--task' is not used.
- Move the call to 'usage_with_options()' to be with 'parse_options()'.
- Link to v1: https://patch.msgid.link/20251031-562-add-sub-command-to-check-if-maintenance-is-needed-v1-0-a03d53e28d0e@gmail.com

---
 Documentation/git-maintenance.adoc | 13 ++++++
 builtin/gc.c                       | 85 +++++++++++++++++++++++++++++++++-----
 object.h                           |  1 -
 refs.c                             |  7 ++++
 refs.h                             |  7 ++++
 refs/debug.c                       | 13 ++++++
 refs/files-backend.c               | 11 +++++
 refs/packed-backend.c              | 13 ++++++
 refs/refs-internal.h               |  6 +++
 refs/reftable-backend.c            | 25 +++++++++++
 reftable/reftable-stack.h          | 11 +++++
 reftable/stack.c                   | 61 ++++++++++++++++++++-------
 t/t7900-maintenance.sh             | 54 +++++++++++++++++-------
 t/unit-tests/u-reftable-stack.c    | 12 +++++-
 14 files changed, 276 insertions(+), 43 deletions(-)

Karthik Nayak (5):
      reftable/stack: return stack segments directly
      reftable/stack: add function to check if optimization is required
      refs: add a `optimize_required` field to `struct ref_storage_be`
      maintenance: add checking logic in `pack_refs_condition()`
      maintenance: add 'is-needed' subcommand

Range-diff versus v2:

1:  16b447b66c = 1:  fe8977aefc reftable/stack: return stack segments directly
2:  b463d5c69d ! 2:  e73f672566 reftable/stack: add function to check if optimization is required
    @@ Commit message
         is no way for the user to check if optimization is required without
         actually performing it.
     
    -    Add and expose `reftable_stack_compaction_required()` which will allow
    -    users to check if the reftable backend can be optimized.
    +    Extract out the heuristics logic from 'reftable_stack_auto_compact()'
    +    into an internal function 'update_segment_if_compaction_required()'.
    +    Then use this to add and expose `reftable_stack_compaction_required()`
    +    which will allow users to check if the reftable backend can be
    +    optimized.
     
         Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
     
    @@ reftable/stack.c: static int stack_segments_for_compaction(struct reftable_stack
      	return 0;
      }
      
    -+int reftable_stack_compaction_required(struct reftable_stack *st,
    -+				       bool use_heuristics,
    -+				       bool *required)
    -+{
    -+	struct segment seg;
    -+	int err = 0;
    -+
    +-int reftable_stack_auto_compact(struct reftable_stack *st)
    ++static int update_segment_if_compaction_required(struct reftable_stack *st,
    ++						 struct segment *seg,
    ++						 bool use_heuristics,
    ++						 bool *required)
    + {
    +-	struct segment seg;
    + 	int err;
    + 
    +-	if (st->merged->tables_len < 2)
     +	if (st->merged->tables_len < 2) {
     +		*required = false;
     +		return 0;
    @@ reftable/stack.c: static int stack_segments_for_compaction(struct reftable_stack
     +
     +	if (!use_heuristics) {
     +		*required = true;
    -+		return 0;
    + 		return 0;
     +	}
     +
    -+	err = stack_segments_for_compaction(st, &seg);
    ++	err = stack_segments_for_compaction(st, seg);
     +	if (err)
     +		return err;
     +
    -+	*required = segment_size(&seg) > 0;
    ++	*required = segment_size(seg) > 0;
     +	return 0;
     +}
     +
    - int reftable_stack_auto_compact(struct reftable_stack *st)
    - {
    - 	struct segment seg;
    ++int reftable_stack_compaction_required(struct reftable_stack *st,
    ++				       bool use_heuristics,
    ++				       bool *required)
    ++{
    ++	struct segment seg;
    ++	return update_segment_if_compaction_required(st, &seg, use_heuristics,
    ++						     required);
    ++}
    ++
    ++int reftable_stack_auto_compact(struct reftable_stack *st)
    ++{
    ++	struct segment seg;
    ++	bool required;
    ++	int err;
    + 
    +-	err = stack_segments_for_compaction(st, &seg);
    ++	err = update_segment_if_compaction_required(st, &seg, true, &required);
    + 	if (err)
    + 		return err;
    + 
    +-	if (segment_size(&seg) > 0)
    ++	if (required)
    + 		return stack_compact_range(st, seg.start, seg.end - 1,
    + 					   NULL, STACK_COMPACT_RANGE_BEST_EFFORT);
    + 
     
      ## t/unit-tests/u-reftable-stack.c ##
     @@ t/unit-tests/u-reftable-stack.c: void test_reftable_stack__add_performs_auto_compaction(void)
3:  b9f44f61f2 = 3:  7d2428b2f9 refs: add a `optimize_required` field to `struct ref_storage_be`
4:  f94ddfdba3 = 4:  3c5f969d62 maintenance: add checking logic in `pack_refs_condition()`
5:  bc56849266 = 5:  bc2d3c26e2 maintenance: add 'is-needed' subcommand


base-commit: edd2018f5db39d68d55a7a4af42375b1a06b9406
change-id: 20251021-562-add-sub-command-to-check-if-maintenance-is-needed-01cae01b4606

Thanks
- Karthik

[PATCH v3 1/5] reftable/stack: return stack segments directly

[Karthik Nayak]

The `stack_table_sizes_for_compaction()` function returns individual
sizes of each reftable table. This function is only called by
`reftable_stack_auto_compact()` to decide which tables need to be
compacted, if any.

Modify the function to directly return the segments, which avoids the
extra step of receiving the sizes only to pass it to
`suggest_compaction_segment()`.

A future commit will also add functionality for checking whether
auto-compaction is necessary without performing it. This change allows
code re-usability in that context.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 reftable/stack.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/reftable/stack.c b/reftable/stack.c
index 65d89820bd..49387f9344 100644
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -1626,7 +1626,8 @@ struct segment suggest_compaction_segment(uint64_t *sizes, size_t n,
 	return seg;
 }
 
-static uint64_t *stack_table_sizes_for_compaction(struct reftable_stack *st)
+static int stack_segments_for_compaction(struct reftable_stack *st,
+					 struct segment *seg)
 {
 	int version = (st->opts.hash_id == REFTABLE_HASH_SHA1) ? 1 : 2;
 	int overhead = header_size(version) - 1;
@@ -1634,29 +1635,29 @@ static uint64_t *stack_table_sizes_for_compaction(struct reftable_stack *st)
 
 	REFTABLE_CALLOC_ARRAY(sizes, st->merged->tables_len);
 	if (!sizes)
-		return NULL;
+		return REFTABLE_OUT_OF_MEMORY_ERROR;
 
 	for (size_t i = 0; i < st->merged->tables_len; i++)
 		sizes[i] = st->tables[i]->size - overhead;
 
-	return sizes;
+	*seg = suggest_compaction_segment(sizes, st->merged->tables_len,
+					  st->opts.auto_compaction_factor);
+	reftable_free(sizes);
+
+	return 0;
 }
 
 int reftable_stack_auto_compact(struct reftable_stack *st)
 {
 	struct segment seg;
-	uint64_t *sizes;
+	int err;
 
 	if (st->merged->tables_len < 2)
 		return 0;
 
-	sizes = stack_table_sizes_for_compaction(st);
-	if (!sizes)
-		return REFTABLE_OUT_OF_MEMORY_ERROR;
-
-	seg = suggest_compaction_segment(sizes, st->merged->tables_len,
-					 st->opts.auto_compaction_factor);
-	reftable_free(sizes);
+	err = stack_segments_for_compaction(st, &seg);
+	if (err)
+		return err;
 
 	if (segment_size(&seg) > 0)
 		return stack_compact_range(st, seg.start, seg.end - 1,

-- 
2.51.0

[PATCH v3 2/5] reftable/stack: add function to check if optimization is required

[Karthik Nayak]

The reftable backend performs auto-compaction as part of its regular
flow, which is required to keep the number of tables part of a stack at
bay. This allows it to stay optimized.

Compaction can also be triggered voluntarily by the user via the 'git
pack-refs' or the 'git refs optimize' command. However, currently there
is no way for the user to check if optimization is required without
actually performing it.

Extract out the heuristics logic from 'reftable_stack_auto_compact()'
into an internal function 'update_segment_if_compaction_required()'.
Then use this to add and expose `reftable_stack_compaction_required()`
which will allow users to check if the reftable backend can be
optimized.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 reftable/reftable-stack.h       | 11 +++++++++++
 reftable/stack.c                | 42 ++++++++++++++++++++++++++++++++++++-----
 t/unit-tests/u-reftable-stack.c | 12 ++++++++++--
 3 files changed, 58 insertions(+), 7 deletions(-)

diff --git a/reftable/reftable-stack.h b/reftable/reftable-stack.h
index d70fcb705d..c2415cbc6e 100644
--- a/reftable/reftable-stack.h
+++ b/reftable/reftable-stack.h
@@ -123,6 +123,17 @@ struct reftable_log_expiry_config {
 int reftable_stack_compact_all(struct reftable_stack *st,
 			       struct reftable_log_expiry_config *config);
 
+/*
+ * Check if compaction is required.
+ *
+ * When `use_heuristics` is false, check if all tables can be compacted to a
+ * single table. If true, use heuristics to determine if the tables need to be
+ * compacted to maintain geometric progression.
+ */
+int reftable_stack_compaction_required(struct reftable_stack *st,
+				       bool use_heuristics,
+				       bool *required);
+
 /* heuristically compact unbalanced table stack. */
 int reftable_stack_auto_compact(struct reftable_stack *st);
 
diff --git a/reftable/stack.c b/reftable/stack.c
index 49387f9344..826500abed 100644
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -1647,19 +1647,51 @@ static int stack_segments_for_compaction(struct reftable_stack *st,
 	return 0;
 }
 
-int reftable_stack_auto_compact(struct reftable_stack *st)
+static int update_segment_if_compaction_required(struct reftable_stack *st,
+						 struct segment *seg,
+						 bool use_heuristics,
+						 bool *required)
 {
-	struct segment seg;
 	int err;
 
-	if (st->merged->tables_len < 2)
+	if (st->merged->tables_len < 2) {
+		*required = false;
+		return 0;
+	}
+
+	if (!use_heuristics) {
+		*required = true;
 		return 0;
+	}
+
+	err = stack_segments_for_compaction(st, seg);
+	if (err)
+		return err;
+
+	*required = segment_size(seg) > 0;
+	return 0;
+}
+
+int reftable_stack_compaction_required(struct reftable_stack *st,
+				       bool use_heuristics,
+				       bool *required)
+{
+	struct segment seg;
+	return update_segment_if_compaction_required(st, &seg, use_heuristics,
+						     required);
+}
+
+int reftable_stack_auto_compact(struct reftable_stack *st)
+{
+	struct segment seg;
+	bool required;
+	int err;
 
-	err = stack_segments_for_compaction(st, &seg);
+	err = update_segment_if_compaction_required(st, &seg, true, &required);
 	if (err)
 		return err;
 
-	if (segment_size(&seg) > 0)
+	if (required)
 		return stack_compact_range(st, seg.start, seg.end - 1,
 					   NULL, STACK_COMPACT_RANGE_BEST_EFFORT);
 
diff --git a/t/unit-tests/u-reftable-stack.c b/t/unit-tests/u-reftable-stack.c
index a8b91812e8..b8110cdeee 100644
--- a/t/unit-tests/u-reftable-stack.c
+++ b/t/unit-tests/u-reftable-stack.c
@@ -1067,6 +1067,7 @@ void test_reftable_stack__add_performs_auto_compaction(void)
 			.value_type = REFTABLE_REF_SYMREF,
 			.value.symref = (char *) "master",
 		};
+		bool required = false;
 		char buf[128];
 
 		/*
@@ -1087,10 +1088,17 @@ void test_reftable_stack__add_performs_auto_compaction(void)
 		 * auto compaction is disabled. When enabled, we should merge
 		 * all tables in the stack.
 		 */
-		if (i != n)
+		cl_assert_equal_i(reftable_stack_compaction_required(st, true, &required), 0);
+		if (i != n) {
 			cl_assert_equal_i(st->merged->tables_len, i + 1);
-		else
+			if (i < 1)
+				cl_assert_equal_b(required, false);
+			else
+				cl_assert_equal_b(required, true);
+		} else {
 			cl_assert_equal_i(st->merged->tables_len, 1);
+			cl_assert_equal_b(required, false);
+		}
 	}
 
 	reftable_stack_destroy(st);

-- 
2.51.0

Re: [PATCH v3 2/5] reftable/stack: add function to check if optimization is required

[Junio C Hamano]

Karthik Nayak <karthik.188@gmail.com> writes:

> The reftable backend performs auto-compaction as part of its regular
> flow, which is required to keep the number of tables part of a stack at
> bay. This allows it to stay optimized.
>
> Compaction can also be triggered voluntarily by the user via the 'git
> pack-refs' or the 'git refs optimize' command. However, currently there
> is no way for the user to check if optimization is required without
> actually performing it.
>
> Extract out the heuristics logic from 'reftable_stack_auto_compact()'
> into an internal function 'update_segment_if_compaction_required()'.
> Then use this to add and expose `reftable_stack_compaction_required()`
> which will allow users to check if the reftable backend can be
> optimized.
>
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
>  reftable/reftable-stack.h       | 11 +++++++++++
>  reftable/stack.c                | 42 ++++++++++++++++++++++++++++++++++++-----
>  t/unit-tests/u-reftable-stack.c | 12 ++++++++++--
>  3 files changed, 58 insertions(+), 7 deletions(-)

The required change is surprisingly small, which is a good sign.

> diff --git a/reftable/stack.c b/reftable/stack.c
> index 49387f9344..826500abed 100644
> --- a/reftable/stack.c
> +++ b/reftable/stack.c
> @@ -1647,19 +1647,51 @@ static int stack_segments_for_compaction(struct reftable_stack *st,
>  	return 0;
>  }
>  
> -int reftable_stack_auto_compact(struct reftable_stack *st)
> +static int update_segment_if_compaction_required(struct reftable_stack *st,
> +						 struct segment *seg,
> +						 bool use_heuristics,
> +						 bool *required)
>  {

Am I correct to understand that "use_heuristics" is almost a synonym
to "maintain geometric progression" in the context of this patch?
Are we expecting other heuristics in the future, in which case, this
may not be a single "bool" but a set of flag bits, and until then
s/heuristics/geometric/ might make it a better name for the
parameter?

Thanks.

Re: [PATCH v3 2/5] reftable/stack: add function to check if optimization is required

[Patrick Steinhardt]

On Thu, Nov 06, 2025 at 10:18:37AM -0800, Junio C Hamano wrote:
> Karthik Nayak <karthik.188@gmail.com> writes:
> > diff --git a/reftable/stack.c b/reftable/stack.c
> > index 49387f9344..826500abed 100644
> > --- a/reftable/stack.c
> > +++ b/reftable/stack.c
> > @@ -1647,19 +1647,51 @@ static int stack_segments_for_compaction(struct reftable_stack *st,
> >  	return 0;
> >  }
> >  
> > -int reftable_stack_auto_compact(struct reftable_stack *st)
> > +static int update_segment_if_compaction_required(struct reftable_stack *st,
> > +						 struct segment *seg,
> > +						 bool use_heuristics,
> > +						 bool *required)
> >  {
> 
> Am I correct to understand that "use_heuristics" is almost a synonym
> to "maintain geometric progression" in the context of this patch?
> Are we expecting other heuristics in the future, in which case, this
> may not be a single "bool" but a set of flag bits, and until then
> s/heuristics/geometric/ might make it a better name for the
> parameter?

I don't expect that this will change anytime soon. So renaming it
accordingly feels like the right direction to me indeed.

Patrick

[PATCH v3 3/5] refs: add a `optimize_required` field to `struct ref_storage_be`

[Karthik Nayak]

To allow users of the refs namespace to check if the reference backend
requires optimization, add a new field `optimize_required` field to
`struct ref_storage_be`. This field is of type `optimize_required_fn`
which is also introduced in this commit.

Modify the debug, files, packed and reftable backend to implement this
field. A following commit will expose this via 'git pack-refs' and 'git
refs optimize'.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 refs.c                  |  7 +++++++
 refs.h                  |  7 +++++++
 refs/debug.c            | 13 +++++++++++++
 refs/files-backend.c    | 11 +++++++++++
 refs/packed-backend.c   | 13 +++++++++++++
 refs/refs-internal.h    |  6 ++++++
 refs/reftable-backend.c | 25 +++++++++++++++++++++++++
 7 files changed, 82 insertions(+)

diff --git a/refs.c b/refs.c
index 0d0831f29b..5583f6e09d 100644
--- a/refs.c
+++ b/refs.c
@@ -2318,6 +2318,13 @@ int refs_optimize(struct ref_store *refs, struct refs_optimize_opts *opts)
 	return refs->be->optimize(refs, opts);
 }
 
+int refs_optimize_required(struct ref_store *refs,
+			   struct refs_optimize_opts *opts,
+			   bool *required)
+{
+	return refs->be->optimize_required(refs, opts, required);
+}
+
 int reference_get_peeled_oid(struct repository *repo,
 			     const struct reference *ref,
 			     struct object_id *peeled_oid)
diff --git a/refs.h b/refs.h
index 6b05bba527..d9051bbb04 100644
--- a/refs.h
+++ b/refs.h
@@ -520,6 +520,13 @@ struct refs_optimize_opts {
  */
 int refs_optimize(struct ref_store *refs, struct refs_optimize_opts *opts);
 
+/*
+ * Check if refs backend can be optimized by calling 'refs_optimize'.
+ */
+int refs_optimize_required(struct ref_store *ref_store,
+			   struct refs_optimize_opts *opts,
+			   bool *required);
+
 /*
  * Setup reflog before using. Fill in err and return -1 on failure.
  */
diff --git a/refs/debug.c b/refs/debug.c
index 2defd2d465..36f8c58b6c 100644
--- a/refs/debug.c
+++ b/refs/debug.c
@@ -124,6 +124,17 @@ static int debug_optimize(struct ref_store *ref_store, struct refs_optimize_opts
 	return res;
 }
 
+static int debug_optimize_required(struct ref_store *ref_store,
+				   struct refs_optimize_opts *opts,
+				   bool *required)
+{
+	struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;
+	int res = drefs->refs->be->optimize_required(drefs->refs, opts, required);
+	trace_printf_key(&trace_refs, "optimize_required: %s, res: %d\n",
+			 required ? "yes" : "no", res);
+	return res;
+}
+
 static int debug_rename_ref(struct ref_store *ref_store, const char *oldref,
 			    const char *newref, const char *logmsg)
 {
@@ -431,6 +442,8 @@ struct ref_storage_be refs_be_debug = {
 	.transaction_abort = debug_transaction_abort,
 
 	.optimize = debug_optimize,
+	.optimize_required = debug_optimize_required,
+
 	.rename_ref = debug_rename_ref,
 	.copy_ref = debug_copy_ref,
 
diff --git a/refs/files-backend.c b/refs/files-backend.c
index a1e70b1c10..6e0c9b340a 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1512,6 +1512,16 @@ static int files_optimize(struct ref_store *ref_store,
 	return 0;
 }
 
+static int files_optimize_required(struct ref_store *ref_store,
+				   struct refs_optimize_opts *opts,
+				   bool *required)
+{
+	struct files_ref_store *refs = files_downcast(ref_store, REF_STORE_READ,
+						      "optimize_required");
+	*required = should_pack_refs(refs, opts);
+	return 0;
+}
+
 /*
  * People using contrib's git-new-workdir have .git/logs/refs ->
  * /some/other/path/.git/logs/refs, and that may live on another device.
@@ -3982,6 +3992,7 @@ struct ref_storage_be refs_be_files = {
 	.transaction_abort = files_transaction_abort,
 
 	.optimize = files_optimize,
+	.optimize_required = files_optimize_required,
 	.rename_ref = files_rename_ref,
 	.copy_ref = files_copy_ref,
 
diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index 10062fd8b6..19ce4d5872 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -1784,6 +1784,17 @@ static int packed_optimize(struct ref_store *ref_store UNUSED,
 	return 0;
 }
 
+static int packed_optimize_required(struct ref_store *ref_store UNUSED,
+				    struct refs_optimize_opts *opts UNUSED,
+				    bool *required)
+{
+	/*
+	 * Packed refs are already optimized.
+	 */
+	*required = false;
+	return 0;
+}
+
 static struct ref_iterator *packed_reflog_iterator_begin(struct ref_store *ref_store UNUSED)
 {
 	return empty_ref_iterator_begin();
@@ -2130,6 +2141,8 @@ struct ref_storage_be refs_be_packed = {
 	.transaction_abort = packed_transaction_abort,
 
 	.optimize = packed_optimize,
+	.optimize_required = packed_optimize_required,
+
 	.rename_ref = NULL,
 	.copy_ref = NULL,
 
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index dee42f231d..c7d2a6e50b 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -424,6 +424,11 @@ typedef int ref_transaction_commit_fn(struct ref_store *refs,
 
 typedef int optimize_fn(struct ref_store *ref_store,
 			struct refs_optimize_opts *opts);
+
+typedef int optimize_required_fn(struct ref_store *ref_store,
+				 struct refs_optimize_opts *opts,
+				 bool *required);
+
 typedef int rename_ref_fn(struct ref_store *ref_store,
 			  const char *oldref, const char *newref,
 			  const char *logmsg);
@@ -549,6 +554,7 @@ struct ref_storage_be {
 	ref_transaction_abort_fn *transaction_abort;
 
 	optimize_fn *optimize;
+	optimize_required_fn *optimize_required;
 	rename_ref_fn *rename_ref;
 	copy_ref_fn *copy_ref;
 
diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index c23c45f3bf..a3ae0cf74a 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -1733,6 +1733,29 @@ static int reftable_be_optimize(struct ref_store *ref_store,
 	return ret;
 }
 
+static int reftable_be_optimize_required(struct ref_store *ref_store,
+					 struct refs_optimize_opts *opts,
+					 bool *required)
+{
+	struct reftable_ref_store *refs = reftable_be_downcast(ref_store, REF_STORE_READ,
+							       "optimize_refs_required");
+	struct reftable_stack *stack;
+	bool use_heuristics = false;
+
+	if (refs->err)
+		return refs->err;
+
+	stack = refs->worktree_backend.stack;
+	if (!stack)
+		stack = refs->main_backend.stack;
+
+	if (opts->flags & REFS_OPTIMIZE_AUTO)
+		use_heuristics = true;
+
+	return reftable_stack_compaction_required(stack, use_heuristics,
+						  required);
+}
+
 struct write_create_symref_arg {
 	struct reftable_ref_store *refs;
 	struct reftable_stack *stack;
@@ -2756,6 +2779,8 @@ struct ref_storage_be refs_be_reftable = {
 	.transaction_abort = reftable_be_transaction_abort,
 
 	.optimize = reftable_be_optimize,
+	.optimize_required = reftable_be_optimize_required,
+
 	.rename_ref = reftable_be_rename_ref,
 	.copy_ref = reftable_be_copy_ref,
 

-- 
2.51.0

[PATCH v3 4/5] maintenance: add checking logic in `pack_refs_condition()`

[Karthik Nayak]

The 'git-maintenance(1)' command supports an '--auto' flag. Usage of the
flag ensures to run maintenance tasks only if certain thresholds are
met. The heuristic is defined on a task level, wherein each task defines
an 'auto_condition', which states if the task should be run.

The 'pack-refs' task is hard-coded to return 1 as:
1. There was never a way to check if the reference backend needs to be
optimized without actually performing the optimization.
2. We can pass in the '--auto' flag to 'git-pack-refs(1)' which would
optimize based on heuristics.

The previous commit added a `refs_optimize_required()` function, which
can be used to check if a reference backend required optimization. Use
this within `pack_refs_condition()`.

This allows us to add a 'git maintenance is-needed' subcommand which can
notify the user if maintenance is needed without actually performing the
optimization. Without this change, the reference backend would always
state that optimization is needed.

Since we import 'revision.h', we need to remove the definition for
'SEEN' which is duplicated in the included header.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 builtin/gc.c | 30 +++++++++++++++++++++---------
 object.h     |  1 -
 2 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index c6d62c74a7..c3e7a84ec2 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -35,6 +35,7 @@
 #include "path.h"
 #include "reflog.h"
 #include "rerere.h"
+#include "revision.h"
 #include "blob.h"
 #include "tree.h"
 #include "promisor-remote.h"
@@ -285,12 +286,26 @@ static void maintenance_run_opts_release(struct maintenance_run_opts *opts)
 
 static int pack_refs_condition(UNUSED struct gc_config *cfg)
 {
-	/*
-	 * The auto-repacking logic for refs is handled by the ref backends and
-	 * exposed via `git pack-refs --auto`. We thus always return truish
-	 * here and let the backend decide for us.
-	 */
-	return 1;
+	struct string_list included_refs = STRING_LIST_INIT_NODUP;
+	struct ref_exclusions excludes = REF_EXCLUSIONS_INIT;
+	struct refs_optimize_opts optimize_opts = {
+		.exclusions = &excludes,
+		.includes = &included_refs,
+		.flags = REFS_OPTIMIZE_PRUNE | REFS_OPTIMIZE_AUTO,
+	};
+	bool required;
+
+	/* Check for all refs, similar to 'git refs optimize --all'. */
+	string_list_append(optimize_opts.includes, "*");
+
+	if (refs_optimize_required(get_main_ref_store(the_repository),
+				   &optimize_opts, &required))
+		return 0;
+
+	clear_ref_exclusions(&excludes);
+	string_list_clear(&included_refs, 0);
+
+	return required == true;
 }
 
 static int maintenance_task_pack_refs(struct maintenance_run_opts *opts,
@@ -1090,9 +1105,6 @@ static int maintenance_opt_schedule(const struct option *opt, const char *arg,
 	return 0;
 }
 
-/* Remember to update object flag allocation in object.h */
-#define SEEN		(1u<<0)
-
 struct cg_auto_data {
 	int num_not_in_graph;
 	int limit;
diff --git a/object.h b/object.h
index 1499f63d50..832299e763 100644
--- a/object.h
+++ b/object.h
@@ -79,7 +79,6 @@ void object_array_init(struct object_array *array);
  * list-objects-filter.c:                                      21
  * bloom.c:                                                    2122
  * builtin/fsck.c:           0--3
- * builtin/gc.c:             0
  * builtin/index-pack.c:                                     2021
  * reflog.c:                           10--12
  * builtin/show-branch.c:    0-------------------------------------------26

-- 
2.51.0

Re: [PATCH v3 4/5] maintenance: add checking logic in `pack_refs_condition()`

[Patrick Steinhardt]

On Thu, Nov 06, 2025 at 09:22:33AM +0100, Karthik Nayak wrote:
> diff --git a/builtin/gc.c b/builtin/gc.c
> index c6d62c74a7..c3e7a84ec2 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -285,12 +286,26 @@ static void maintenance_run_opts_release(struct maintenance_run_opts *opts)
>  
>  static int pack_refs_condition(UNUSED struct gc_config *cfg)
>  {
> -	/*
> -	 * The auto-repacking logic for refs is handled by the ref backends and
> -	 * exposed via `git pack-refs --auto`. We thus always return truish
> -	 * here and let the backend decide for us.
> -	 */
> -	return 1;
> +	struct string_list included_refs = STRING_LIST_INIT_NODUP;
> +	struct ref_exclusions excludes = REF_EXCLUSIONS_INIT;
> +	struct refs_optimize_opts optimize_opts = {
> +		.exclusions = &excludes,
> +		.includes = &included_refs,
> +		.flags = REFS_OPTIMIZE_PRUNE | REFS_OPTIMIZE_AUTO,
> +	};
> +	bool required;
> +
> +	/* Check for all refs, similar to 'git refs optimize --all'. */
> +	string_list_append(optimize_opts.includes, "*");
> +
> +	if (refs_optimize_required(get_main_ref_store(the_repository),
> +				   &optimize_opts, &required))
> +		return 0;
> +
> +	clear_ref_exclusions(&excludes);
> +	string_list_clear(&included_refs, 0);
> +
> +	return required == true;

Tiny nit: I think in our codebase this can be written in a more
idiomatic way by saying `!!required`.

Other than that I don't have anything more to add to this series.
Thanks!

Patrick

Re: [PATCH v3 4/5] maintenance: add checking logic in `pack_refs_condition()`

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 1582 bytes --]

Patrick Steinhardt <ps@pks.im> writes:

> On Thu, Nov 06, 2025 at 09:22:33AM +0100, Karthik Nayak wrote:
>> diff --git a/builtin/gc.c b/builtin/gc.c
>> index c6d62c74a7..c3e7a84ec2 100644
>> --- a/builtin/gc.c
>> +++ b/builtin/gc.c
>> @@ -285,12 +286,26 @@ static void maintenance_run_opts_release(struct maintenance_run_opts *opts)
>>
>>  static int pack_refs_condition(UNUSED struct gc_config *cfg)
>>  {
>> -	/*
>> -	 * The auto-repacking logic for refs is handled by the ref backends and
>> -	 * exposed via `git pack-refs --auto`. We thus always return truish
>> -	 * here and let the backend decide for us.
>> -	 */
>> -	return 1;
>> +	struct string_list included_refs = STRING_LIST_INIT_NODUP;
>> +	struct ref_exclusions excludes = REF_EXCLUSIONS_INIT;
>> +	struct refs_optimize_opts optimize_opts = {
>> +		.exclusions = &excludes,
>> +		.includes = &included_refs,
>> +		.flags = REFS_OPTIMIZE_PRUNE | REFS_OPTIMIZE_AUTO,
>> +	};
>> +	bool required;
>> +
>> +	/* Check for all refs, similar to 'git refs optimize --all'. */
>> +	string_list_append(optimize_opts.includes, "*");
>> +
>> +	if (refs_optimize_required(get_main_ref_store(the_repository),
>> +				   &optimize_opts, &required))
>> +		return 0;
>> +
>> +	clear_ref_exclusions(&excludes);
>> +	string_list_clear(&included_refs, 0);
>> +
>> +	return required == true;
>
> Tiny nit: I think in our codebase this can be written in a more
> idiomatic way by saying `!!required`.
>

Fair. Will change.

> Other than that I don't have anything more to add to this series.
> Thanks!
>
> Patrick

Thanks for your review!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH v3 4/5] maintenance: add checking logic in `pack_refs_condition()`

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

>> +	/* Check for all refs, similar to 'git refs optimize --all'. */
>> +	string_list_append(optimize_opts.includes, "*");
>> +
>> +	if (refs_optimize_required(get_main_ref_store(the_repository),
>> +				   &optimize_opts, &required))
>> +		return 0;
>> +
>> +	clear_ref_exclusions(&excludes);
>> +	string_list_clear(&included_refs, 0);
>> +
>> +	return required == true;
>
> Tiny nit: I think in our codebase this can be written in a more
> idiomatic way by saying `!!required`.

Comparing for equality with Boolean in general is stupid, as
Booleans are designed to be usable as-is.  If it is "true", it is
true, and you do not have to compare it with "true" to ascertain
that it is true.

I do 100% prefer "!!required" over "required == true" or "required
!= false" all the time, since it is more idiomatic, but I vaguely
recall we had something that contradicts it in the CodingGuidelines
document.  Perhaps we'd want to fix that.

Thanks.


[Footnote]

But doesn't your suggested rewrite potentially change the meaning?

The original allows required to be "true" and nothing else, while
"!!required" allows it to be any form of true (and in C, things that
are not zero, even a pointer that is not NULL, are all true).

Re: [PATCH v3 4/5] maintenance: add checking logic in `pack_refs_condition()`

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 1876 bytes --]

Junio C Hamano <gitster@pobox.com> writes:

> Patrick Steinhardt <ps@pks.im> writes:
>
>>> +	/* Check for all refs, similar to 'git refs optimize --all'. */
>>> +	string_list_append(optimize_opts.includes, "*");
>>> +
>>> +	if (refs_optimize_required(get_main_ref_store(the_repository),
>>> +				   &optimize_opts, &required))
>>> +		return 0;
>>> +
>>> +	clear_ref_exclusions(&excludes);
>>> +	string_list_clear(&included_refs, 0);
>>> +
>>> +	return required == true;
>>
>> Tiny nit: I think in our codebase this can be written in a more
>> idiomatic way by saying `!!required`.
>
> Comparing for equality with Boolean in general is stupid, as
> Booleans are designed to be usable as-is.  If it is "true", it is
> true, and you do not have to compare it with "true" to ascertain
> that it is true.
>
> I do 100% prefer "!!required" over "required == true" or "required
> != false" all the time, since it is more idiomatic, but I vaguely
> recall we had something that contradicts it in the CodingGuidelines
> document.  Perhaps we'd want to fix that.
>

I could only find

  - Some clever tricks, like using the !! operator with arithmetic
     constructs, can be extremely confusing to others.  Avoid them,
     unless there is a compelling reason to use them.

I think its okay? This is more of a suggestion than a rule.

> Thanks.
>
>
> [Footnote]
>
> But doesn't your suggested rewrite potentially change the meaning?
>
> The original allows required to be "true" and nothing else, while
> "!!required" allows it to be any form of true (and in C, things that
> are not zero, even a pointer that is not NULL, are all true).

I get what you mean, but with the context that required is of type
'bool', this would mean that we simply convert it to '0'/'1' here.

With all this, perhaps `return required` as used in the v1 was the best
approach. I'm happy to go either ways.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH v3 4/5] maintenance: add checking logic in `pack_refs_condition()`

[Junio C Hamano]

Karthik Nayak <karthik.188@gmail.com> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Patrick Steinhardt <ps@pks.im> writes:
>>
>>>> +	/* Check for all refs, similar to 'git refs optimize --all'. */
>>>> +	string_list_append(optimize_opts.includes, "*");
>>>> +
>>>> +	if (refs_optimize_required(get_main_ref_store(the_repository),
>>>> +				   &optimize_opts, &required))
>>>> +		return 0;
>>>> +
>>>> +	clear_ref_exclusions(&excludes);
>>>> +	string_list_clear(&included_refs, 0);
>>>> +
>>>> +	return required == true;
>>>
>>> Tiny nit: I think in our codebase this can be written in a more
>>> idiomatic way by saying `!!required`.
>>
>> Comparing for equality with Boolean in general is stupid, as
>> Booleans are designed to be usable as-is.  If it is "true", it is
>> true, and you do not have to compare it with "true" to ascertain
>> that it is true.
>>
>> I do 100% prefer "!!required" over "required == true" or "required
>> != false" all the time, since it is more idiomatic, but I vaguely
>> recall we had something that contradicts it in the CodingGuidelines
>> document.  Perhaps we'd want to fix that.
>>
>
> I could only find
>
>   - Some clever tricks, like using the !! operator with arithmetic
>      constructs, can be extremely confusing to others.  Avoid them,
>      unless there is a compelling reason to use them.
>
> I think its okay? This is more of a suggestion than a rule.

"Unless there is a reason to use" sounds like an outright
prohibition to me, though.

By the way, in the on-topic part of the discussion, "required" is a
bool, the helper function that takes &required takes a pointer to a
bool, and the function in question returns a bool.  So I should
update my preference above.  "return required" is the most natural
way to write, and it uses "bool" as it was designed to be used.
When the reader knows that required is a bool already, "return
!!required" is just as pointless as "return required == true".

If required and the helper that takes a pointer to it were "int",
and this function returns a bool, then my original preference would
apply; even if an "int required" has 3 in it, we probably can still
say "return required" and the function would coerce that 3 into
"true", but manually coercing it to 0/1 with !!required is more
explicit and less confusing.

Thanks.

Re: [PATCH v3 4/5] maintenance: add checking logic in `pack_refs_condition()`

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 1876 bytes --]

Junio C Hamano <gitster@pobox.com> writes:

> Patrick Steinhardt <ps@pks.im> writes:
>
>>> +	/* Check for all refs, similar to 'git refs optimize --all'. */
>>> +	string_list_append(optimize_opts.includes, "*");
>>> +
>>> +	if (refs_optimize_required(get_main_ref_store(the_repository),
>>> +				   &optimize_opts, &required))
>>> +		return 0;
>>> +
>>> +	clear_ref_exclusions(&excludes);
>>> +	string_list_clear(&included_refs, 0);
>>> +
>>> +	return required == true;
>>
>> Tiny nit: I think in our codebase this can be written in a more
>> idiomatic way by saying `!!required`.
>
> Comparing for equality with Boolean in general is stupid, as
> Booleans are designed to be usable as-is.  If it is "true", it is
> true, and you do not have to compare it with "true" to ascertain
> that it is true.
>
> I do 100% prefer "!!required" over "required == true" or "required
> != false" all the time, since it is more idiomatic, but I vaguely
> recall we had something that contradicts it in the CodingGuidelines
> document.  Perhaps we'd want to fix that.
>

I could only find

  - Some clever tricks, like using the !! operator with arithmetic
     constructs, can be extremely confusing to others.  Avoid them,
     unless there is a compelling reason to use them.

I think its okay? This is more of a suggestion than a rule.

> Thanks.
>
>
> [Footnote]
>
> But doesn't your suggested rewrite potentially change the meaning?
>
> The original allows required to be "true" and nothing else, while
> "!!required" allows it to be any form of true (and in C, things that
> are not zero, even a pointer that is not NULL, are all true).

I get what you mean, but with the context that required is of type
'bool', this would mean that we simply convert it to '0'/'1' here.

With all this, perhaps `return required` as used in the v1 was the best
approach. I'm happy to go either ways.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

[PATCH v3 5/5] maintenance: add 'is-needed' subcommand

[Karthik Nayak]

The 'git-maintenance(1)' command provides tooling to run maintenance
tasks over Git repositories. The 'run' subcommand, as the name suggests,
runs the maintenance tasks. When used with the '--auto' flag, it uses
heuristics to determine if the required thresholds are met for running
said maintenance tasks.

There is however a lack of insight into these heuristics. Meaning, the
checks are linked to the execution.

Add a new 'is-needed' subcommand to 'git-maintenance(1)' which allows
users to simply check if it is needed to run maintenance without
performing it.

This subcommand can check if it is needed to run maintenance without
actually running it. Ideally it should be used with the '--auto' flag,
which would allow users to check if the thresholds required are met. The
subcommand also supports the '--task' flag which can be used to check
specific maintenance tasks.

While adding the respective tests in 't/t7900-maintenance.sh', remove a
duplicate of the test: 'worktree-prune task with --auto honors
maintenance.worktree-prune.auto'.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 Documentation/git-maintenance.adoc | 13 +++++++++
 builtin/gc.c                       | 55 +++++++++++++++++++++++++++++++++++++-
 t/t7900-maintenance.sh             | 54 ++++++++++++++++++++++++++-----------
 3 files changed, 105 insertions(+), 17 deletions(-)

diff --git a/Documentation/git-maintenance.adoc b/Documentation/git-maintenance.adoc
index 540b5cf68b..37939510d4 100644
--- a/Documentation/git-maintenance.adoc
+++ b/Documentation/git-maintenance.adoc
@@ -12,6 +12,7 @@ SYNOPSIS
 'git maintenance' run [<options>]
 'git maintenance' start [--scheduler=<scheduler>]
 'git maintenance' (stop|register|unregister) [<options>]
+'git maintenance' is-needed [<options>]
 
 
 DESCRIPTION
@@ -84,6 +85,16 @@ 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.
+    Ideally used with the '--auto' flag.
++
+If one or more `--task` options	are specified, then those tasks are checked
+in that order. Otherwise, the tasks are determined by which
+`maintenance.<task>.enabled` config options are true. By default, only
+`maintenance.gc.enabled` is true.
+
 TASKS
 -----
 
@@ -183,6 +194,8 @@ OPTIONS
 	in the `gc.auto` config setting, or when the number of pack-files
 	exceeds the `gc.autoPackLimit` config setting. Not compatible with
 	the `--schedule` option.
+	When combined with the `is-needed` subcommand, check if the required
+	thresholds are met without actually running maintenance.
 
 --schedule::
 	When combined with the `run` subcommand, run maintenance tasks
diff --git a/builtin/gc.c b/builtin/gc.c
index c3e7a84ec2..e5ba2a2e72 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -3253,7 +3253,59 @@ static int maintenance_stop(int argc, const char **argv, const char *prefix,
 	return update_background_schedule(NULL, 0);
 }
 
-static const char * const builtin_maintenance_usage[] = {
+static const char *const builtin_maintenance_is_needed_usage[] = {
+	"git maintenance is-needed [--task=<task>] [--schedule]",
+	NULL
+};
+
+static int maintenance_is_needed(int argc, const char **argv, const char *prefix,
+				 struct repository *repo UNUSED)
+{
+	struct maintenance_run_opts opts = MAINTENANCE_RUN_OPTS_INIT;
+	struct string_list selected_tasks = STRING_LIST_INIT_DUP;
+	struct gc_config cfg = GC_CONFIG_INIT;
+	struct option options[] = {
+		OPT_BOOL(0, "auto", &opts.auto_flag,
+			 N_("run tasks based on the state of the repository")),
+		OPT_CALLBACK_F(0, "task", &selected_tasks, N_("task"),
+			       N_("check a specific task"),
+			       PARSE_OPT_NONEG, task_option_parse),
+		OPT_END()
+	};
+	bool is_needed = false;
+
+	argc = parse_options(argc, argv, prefix, options,
+			     builtin_maintenance_is_needed_usage,
+			     PARSE_OPT_STOP_AT_NON_OPTION);
+	if (argc)
+		usage_with_options(builtin_maintenance_is_needed_usage, options);
+
+	gc_config(&cfg);
+	initialize_task_config(&opts, &selected_tasks);
+
+	if (opts.auto_flag) {
+		for (size_t i = 0; i < opts.tasks_nr; i++) {
+			if (tasks[opts.tasks[i]].auto_condition &&
+			    tasks[opts.tasks[i]].auto_condition(&cfg)) {
+				is_needed = true;
+				break;
+			}
+		}
+	} else {
+		/* When not using --auto, we should always require maintenance. */
+		is_needed = true;
+	}
+
+	string_list_clear(&selected_tasks, 0);
+	maintenance_run_opts_release(&opts);
+	gc_config_release(&cfg);
+
+	if (is_needed)
+		return 0;
+	return 1;
+}
+
+static const char *const builtin_maintenance_usage[] = {
 	N_("git maintenance <subcommand> [<options>]"),
 	NULL,
 };
@@ -3270,6 +3322,7 @@ int cmd_maintenance(int argc,
 		OPT_SUBCOMMAND("stop", &fn, maintenance_stop),
 		OPT_SUBCOMMAND("register", &fn, maintenance_register),
 		OPT_SUBCOMMAND("unregister", &fn, maintenance_unregister),
+		OPT_SUBCOMMAND("is-needed", &fn, maintenance_is_needed),
 		OPT_END(),
 	};
 
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index ddd273d8dc..a17e2091c2 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -49,7 +49,9 @@ test_expect_success 'run [--auto|--quiet]' '
 		git maintenance run --auto 2>/dev/null &&
 	GIT_TRACE2_EVENT="$(pwd)/run-no-quiet.txt" \
 		git maintenance run --no-quiet 2>/dev/null &&
+	git maintenance is-needed &&
 	test_subcommand git gc --quiet --no-detach --skip-foreground-tasks <run-no-auto.txt &&
+	! git maintenance is-needed --auto &&
 	test_subcommand ! git gc --auto --quiet --no-detach --skip-foreground-tasks <run-auto.txt &&
 	test_subcommand git gc --no-quiet --no-detach --skip-foreground-tasks <run-no-quiet.txt
 '
@@ -180,6 +182,11 @@ test_expect_success 'commit-graph auto condition' '
 
 	test_commit first &&
 
+	! git -c maintenance.commit-graph.auto=0 \
+		maintenance is-needed --auto --task=commit-graph &&
+	git -c maintenance.commit-graph.auto=1 \
+		maintenance is-needed --auto --task=commit-graph &&
+
 	GIT_TRACE2_EVENT="$(pwd)/cg-zero-means-no.txt" \
 		git -c maintenance.commit-graph.auto=0 $COMMAND &&
 	GIT_TRACE2_EVENT="$(pwd)/cg-one-satisfied.txt" \
@@ -290,16 +297,23 @@ test_expect_success 'maintenance.loose-objects.auto' '
 		git -c maintenance.loose-objects.auto=1 maintenance \
 		run --auto --task=loose-objects 2>/dev/null &&
 	test_subcommand ! git prune-packed --quiet <trace-lo1.txt &&
+
 	printf data-A | git hash-object -t blob --stdin -w &&
+	! git -c maintenance.loose-objects.auto=2 \
+		maintenance is-needed --auto --task=loose-objects &&
 	GIT_TRACE2_EVENT="$(pwd)/trace-loA" \
 		git -c maintenance.loose-objects.auto=2 \
 		maintenance run --auto --task=loose-objects 2>/dev/null &&
 	test_subcommand ! git prune-packed --quiet <trace-loA &&
+
 	printf data-B | git hash-object -t blob --stdin -w &&
+	git -c maintenance.loose-objects.auto=2 \
+		maintenance is-needed --auto --task=loose-objects &&
 	GIT_TRACE2_EVENT="$(pwd)/trace-loB" \
 		git -c maintenance.loose-objects.auto=2 \
 		maintenance run --auto --task=loose-objects 2>/dev/null &&
 	test_subcommand git prune-packed --quiet <trace-loB &&
+
 	GIT_TRACE2_EVENT="$(pwd)/trace-loC" \
 		git -c maintenance.loose-objects.auto=2 \
 		maintenance run --auto --task=loose-objects 2>/dev/null &&
@@ -421,10 +435,13 @@ run_incremental_repack_and_verify () {
 	test_commit A &&
 	git repack -adk &&
 	git multi-pack-index write &&
+	! git -c maintenance.incremental-repack.auto=1 \
+		maintenance is-needed --auto --task=incremental-repack &&
 	GIT_TRACE2_EVENT="$(pwd)/midx-init.txt" git \
 		-c maintenance.incremental-repack.auto=1 \
 		maintenance run --auto --task=incremental-repack 2>/dev/null &&
 	test_subcommand ! git multi-pack-index write --no-progress <midx-init.txt &&
+
 	test_commit B &&
 	git pack-objects --revs .git/objects/pack/pack <<-\EOF &&
 	HEAD
@@ -434,11 +451,14 @@ run_incremental_repack_and_verify () {
 		-c maintenance.incremental-repack.auto=2 \
 		maintenance run --auto --task=incremental-repack 2>/dev/null &&
 	test_subcommand ! git multi-pack-index write --no-progress <trace-A &&
+
 	test_commit C &&
 	git pack-objects --revs .git/objects/pack/pack <<-\EOF &&
 	HEAD
 	^HEAD~1
 	EOF
+	git -c maintenance.incremental-repack.auto=2 \
+		maintenance is-needed --auto --task=incremental-repack &&
 	GIT_TRACE2_EVENT=$(pwd)/trace-B git \
 		-c maintenance.incremental-repack.auto=2 \
 		maintenance run --auto --task=incremental-repack 2>/dev/null &&
@@ -485,9 +505,15 @@ test_expect_success 'reflog-expire task --auto only packs when exceeding limits'
 	git reflog expire --all --expire=now &&
 	test_commit reflog-one &&
 	test_commit reflog-two &&
+
+	! git -c maintenance.reflog-expire.auto=3 \
+		maintenance is-needed --auto --task=reflog-expire &&
 	GIT_TRACE2_EVENT="$(pwd)/reflog-expire-auto.txt" \
 		git -c maintenance.reflog-expire.auto=3 maintenance run --auto --task=reflog-expire &&
 	test_subcommand ! git reflog expire --all <reflog-expire-auto.txt &&
+
+	git -c maintenance.reflog-expire.auto=2 \
+		maintenance is-needed --auto --task=reflog-expire &&
 	GIT_TRACE2_EVENT="$(pwd)/reflog-expire-auto.txt" \
 		git -c maintenance.reflog-expire.auto=2 maintenance run --auto --task=reflog-expire &&
 	test_subcommand git reflog expire --all <reflog-expire-auto.txt
@@ -514,6 +540,7 @@ test_expect_success 'worktree-prune task --auto only prunes with prunable worktr
 	test_expect_worktree_prune ! git maintenance run --auto --task=worktree-prune &&
 	mkdir .git/worktrees &&
 	: >.git/worktrees/abc &&
+	git maintenance is-needed --auto --task=worktree-prune &&
 	test_expect_worktree_prune git maintenance run --auto --task=worktree-prune
 '
 
@@ -530,22 +557,7 @@ test_expect_success 'worktree-prune task with --auto honors maintenance.worktree
 	test_expect_worktree_prune ! git -c maintenance.worktree-prune.auto=0 maintenance run --auto --task=worktree-prune &&
 	# A positive value should require at least this many prunable worktrees.
 	test_expect_worktree_prune ! git -c maintenance.worktree-prune.auto=4 maintenance run --auto --task=worktree-prune &&
-	test_expect_worktree_prune git -c maintenance.worktree-prune.auto=3 maintenance run --auto --task=worktree-prune
-'
-
-test_expect_success 'worktree-prune task with --auto honors maintenance.worktree-prune.auto' '
-	# A negative value should always prune.
-	test_expect_worktree_prune git -c maintenance.worktree-prune.auto=-1 maintenance run --auto --task=worktree-prune &&
-
-	mkdir .git/worktrees &&
-	: >.git/worktrees/first &&
-	: >.git/worktrees/second &&
-	: >.git/worktrees/third &&
-
-	# Zero should never prune.
-	test_expect_worktree_prune ! git -c maintenance.worktree-prune.auto=0 maintenance run --auto --task=worktree-prune &&
-	# A positive value should require at least this many prunable worktrees.
-	test_expect_worktree_prune ! git -c maintenance.worktree-prune.auto=4 maintenance run --auto --task=worktree-prune &&
+	git -c maintenance.worktree-prune.auto=3 maintenance is-needed --auto --task=worktree-prune &&
 	test_expect_worktree_prune git -c maintenance.worktree-prune.auto=3 maintenance run --auto --task=worktree-prune
 '
 
@@ -554,11 +566,13 @@ test_expect_success 'worktree-prune task honors gc.worktreePruneExpire' '
 	rm -rf worktree &&
 
 	rm -f worktree-prune.txt &&
+	! git -c gc.worktreePruneExpire=1.week.ago maintenance is-needed --auto --task=worktree-prune &&
 	GIT_TRACE2_EVENT="$(pwd)/worktree-prune.txt" git -c gc.worktreePruneExpire=1.week.ago maintenance run --auto --task=worktree-prune &&
 	test_subcommand ! git worktree prune --expire 1.week.ago <worktree-prune.txt &&
 	test_path_is_dir .git/worktrees/worktree &&
 
 	rm -f worktree-prune.txt &&
+	git -c gc.worktreePruneExpire=now maintenance is-needed --auto --task=worktree-prune &&
 	GIT_TRACE2_EVENT="$(pwd)/worktree-prune.txt" git -c gc.worktreePruneExpire=now maintenance run --auto --task=worktree-prune &&
 	test_subcommand git worktree prune --expire now <worktree-prune.txt &&
 	test_path_is_missing .git/worktrees/worktree
@@ -583,10 +597,13 @@ test_expect_success 'rerere-gc task without --auto always collects garbage' '
 
 test_expect_success 'rerere-gc task with --auto only prunes with prunable entries' '
 	test_when_finished "rm -rf .git/rr-cache" &&
+	! git maintenance is-needed --auto --task=rerere-gc &&
 	test_expect_rerere_gc ! git maintenance run --auto --task=rerere-gc &&
 	mkdir .git/rr-cache &&
+	! git maintenance is-needed --auto --task=rerere-gc &&
 	test_expect_rerere_gc ! git maintenance run --auto --task=rerere-gc &&
 	: >.git/rr-cache/entry &&
+	git maintenance is-needed --auto --task=rerere-gc &&
 	test_expect_rerere_gc git maintenance run --auto --task=rerere-gc
 '
 
@@ -594,17 +611,22 @@ test_expect_success 'rerere-gc task with --auto honors maintenance.rerere-gc.aut
 	test_when_finished "rm -rf .git/rr-cache" &&
 
 	# A negative value should always prune.
+	git -c maintenance.rerere-gc.auto=-1 maintenance is-needed --auto --task=rerere-gc &&
 	test_expect_rerere_gc git -c maintenance.rerere-gc.auto=-1 maintenance run --auto --task=rerere-gc &&
 
 	# A positive value prunes when there is at least one entry.
+	! git -c maintenance.rerere-gc.auto=9000 maintenance is-needed --auto --task=rerere-gc &&
 	test_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=9000 maintenance run --auto --task=rerere-gc &&
 	mkdir .git/rr-cache &&
+	! git -c maintenance.rerere-gc.auto=9000 maintenance is-needed --auto --task=rerere-gc &&
 	test_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=9000 maintenance run --auto --task=rerere-gc &&
 	: >.git/rr-cache/entry-1 &&
+	git -c maintenance.rerere-gc.auto=9000 maintenance is-needed --auto --task=rerere-gc &&
 	test_expect_rerere_gc git -c maintenance.rerere-gc.auto=9000 maintenance run --auto --task=rerere-gc &&
 
 	# Zero should never prune.
 	: >.git/rr-cache/entry-1 &&
+	! git -c maintenance.rerere-gc.auto=0 maintenance is-needed --auto --task=rerere-gc &&
 	test_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=0 maintenance run --auto --task=rerere-gc
 '
 

-- 
2.51.0

Re: [PATCH v3 5/5] maintenance: add 'is-needed' subcommand

[Patrick Steinhardt]

On Thu, Nov 06, 2025 at 09:22:34AM +0100, Karthik Nayak wrote:
> diff --git a/Documentation/git-maintenance.adoc b/Documentation/git-maintenance.adoc
> index 540b5cf68b..37939510d4 100644
> --- a/Documentation/git-maintenance.adoc
> +++ b/Documentation/git-maintenance.adoc
> @@ -84,6 +85,16 @@ 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.
> +    Ideally used with the '--auto' flag.
> ++
> +If one or more `--task` options	are specified, then those tasks are checked

I spoke too soon, forgot that there's one more patch :) s/\t/ /

> +in that order. Otherwise, the tasks are determined by which
> +`maintenance.<task>.enabled` config options are true. By default, only
> +`maintenance.gc.enabled` is true.

This could use a pointer to "maintenance.strategy", but I see that you
took this explanation from the "run" subcommand. I think this is good
enough for now.

> diff --git a/builtin/gc.c b/builtin/gc.c
> index c3e7a84ec2..e5ba2a2e72 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -3253,7 +3253,59 @@ static int maintenance_stop(int argc, const char **argv, const char *prefix,
>  	return update_background_schedule(NULL, 0);
>  }
>  
> -static const char * const builtin_maintenance_usage[] = {
> +static const char *const builtin_maintenance_is_needed_usage[] = {
> +	"git maintenance is-needed [--task=<task>] [--schedule]",
> +	NULL
> +};
> +
> +static int maintenance_is_needed(int argc, const char **argv, const char *prefix,
> +				 struct repository *repo UNUSED)
> +{
> +	struct maintenance_run_opts opts = MAINTENANCE_RUN_OPTS_INIT;
> +	struct string_list selected_tasks = STRING_LIST_INIT_DUP;
> +	struct gc_config cfg = GC_CONFIG_INIT;
> +	struct option options[] = {
> +		OPT_BOOL(0, "auto", &opts.auto_flag,
> +			 N_("run tasks based on the state of the repository")),
> +		OPT_CALLBACK_F(0, "task", &selected_tasks, N_("task"),
> +			       N_("check a specific task"),
> +			       PARSE_OPT_NONEG, task_option_parse),
> +		OPT_END()
> +	};
> +	bool is_needed = false;
> +
> +	argc = parse_options(argc, argv, prefix, options,
> +			     builtin_maintenance_is_needed_usage,
> +			     PARSE_OPT_STOP_AT_NON_OPTION);
> +	if (argc)
> +		usage_with_options(builtin_maintenance_is_needed_usage, options);
> +
> +	gc_config(&cfg);
> +	initialize_task_config(&opts, &selected_tasks);
> +
> +	if (opts.auto_flag) {
> +		for (size_t i = 0; i < opts.tasks_nr; i++) {
> +			if (tasks[opts.tasks[i]].auto_condition &&
> +			    tasks[opts.tasks[i]].auto_condition(&cfg)) {
> +				is_needed = true;
> +				break;
> +			}
> +		}
> +	} else {
> +		/* When not using --auto, we should always require maintenance. */

Nit: we might add a TODO comment here.

    /*
     * When not using --auto we always require maintenance right now.
     *
     * TODO: this certainly is too eager, as some maintenance tasks may
     * decide to not do anything because the data structures are already
     * fully optimized. We may eventually want to extend the auto
     * condition to also cover non-auto runs so that we can detect such
     * cases.
     /

Patrick

Re: [PATCH v3 5/5] maintenance: add 'is-needed' subcommand

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 2232 bytes --]

Patrick Steinhardt <ps@pks.im> writes:

> On Thu, Nov 06, 2025 at 09:22:34AM +0100, Karthik Nayak wrote:
>> diff --git a/Documentation/git-maintenance.adoc b/Documentation/git-maintenance.adoc
>> index 540b5cf68b..37939510d4 100644
>> --- a/Documentation/git-maintenance.adoc
>> +++ b/Documentation/git-maintenance.adoc
>> @@ -84,6 +85,16 @@ 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.
>> +    Ideally used with the '--auto' flag.
>> ++
>> +If one or more `--task` options	are specified, then those tasks are checked
>
> I spoke too soon, forgot that there's one more patch :) s/\t/ /

Weird, not sure how that happened, good catch.

>
>> +in that order. Otherwise, the tasks are determined by which
>> +`maintenance.<task>.enabled` config options are true. By default, only
>> +`maintenance.gc.enabled` is true.
>
> This could use a pointer to "maintenance.strategy", but I see that you
> took this explanation from the "run" subcommand. I think this is good
> enough for now.

Yeah, that's what I went with, so I'll leave it as is :)

[snip]

>> +	if (opts.auto_flag) {
>> +		for (size_t i = 0; i < opts.tasks_nr; i++) {
>> +			if (tasks[opts.tasks[i]].auto_condition &&
>> +			    tasks[opts.tasks[i]].auto_condition(&cfg)) {
>> +				is_needed = true;
>> +				break;
>> +			}
>> +		}
>> +	} else {
>> +		/* When not using --auto, we should always require maintenance. */
>
> Nit: we might add a TODO comment here.
>
>     /*
>      * When not using --auto we always require maintenance right now.
>      *
>      * TODO: this certainly is too eager, as some maintenance tasks may
>      * decide to not do anything because the data structures are already
>      * fully optimized. We may eventually want to extend the auto
>      * condition to also cover non-auto runs so that we can detect such
>      * cases.
>      /
>
> Patrick

Sure this makes sense, will add it in.

Thanks
Karthik

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

[PATCH v4 0/5] maintenance: add an 'is-needed' subcommand

[Karthik Nayak]

Hello,

I recently raised a patch series [1] to add 'git refs optimize --required'
which checks if the reference backend can be optimized, without actually
performing the optimization.

Back then, we had decided [2] that it would be a better to broaden the
approach and add a 'is-needed' subcommand to 'git-maintenance(1)'. This
would allow users to check if maintenance was required for the
repository and users could also provide a task via the '--task' to check
if maintenance was needed for a particular task.

Ideally the subcommand will be used with the '--auto' flag which can
check the same heuristics as that used with 'git maintenance run
--auto'. Future patches can also add support for the '--schedule' flag
which can be used to check required schedule it met. However that flag
isn't added as part of this series.

This series implements that.

Commits 1-3 add the required functionality in the refs subsystem to
expose an 'optimize_required' field which can be used to check if
backends need to be optimized.
Commit 4 utilizes this within the 'git-maintenance(1)' code.
Commit 5 adds the 'is-needed' subcommand to 'git-maintenance(1)'.

This is based on top of master a99f379adf (The 27th batch, 2025-10-30)
and is dependent on the following series:

    - kn/refs-optim-cleanup
    - ps/ref-peeled-tags

Merges cleanly with `next`. I think those two topics are close to being
merged to `next` so hopefully this dependency tree doesn't get too
complicated. I'll rebase as needed to resolve conflicts.

[1]: https://lore.kernel.org/git/20251010-562-add-option-to-check-if-reference-backend-needs-repacking-v1-0-c7962be584fa@gmail.com/
[2]: https://lore.kernel.org/git/CAOLa=ZRdxm787nE4FSr2VUHDB+hW06Ggc6yUcKmeTKAb6B7YOA@mail.gmail.com/

---
Changes in v4:
- In `update_segment_if_compaction_required()` change the argument name
  from `use_heuristics` to `use_geometric` since we only have one
  heuristic currently and this is much clearer to understand.
- There were a lot of discussion on how to return a bool variable when
  the function has a return type of int. We discussed both '!!required',
  and 'required != true'. I'm going to punt this discussion keeping it
  simple as 'return required' as in my first version, since even Junio
  expressed his thoughts in favor of it.
- Add a TODO for improvements to the flow when running `git maintenance
  is-needed` without the `--auto` flag.
- Link to v3: https://patch.msgid.link/20251106-562-add-sub-command-to-check-if-maintenance-is-needed-v3-0-d611a2a95cf5@gmail.com

Changes in v3:
- In patch 2/5 extract out code for deciding if compaction is required
  into a static function. This removes duplication of logic for deciding
  if compaction is needed.
- Link to v2: https://patch.msgid.link/20251104-562-add-sub-command-to-check-if-maintenance-is-needed-v2-0-303462a9e4ed@gmail.com

Changes in v2:
- Added more documentation for `reftable_stack_compaction_required()`.
- Fixed some typos and grammar mistakes in commit messages.
- Clarify which tasks will be run when '--task' is not used.
- Move the call to 'usage_with_options()' to be with 'parse_options()'.
- Link to v1: https://patch.msgid.link/20251031-562-add-sub-command-to-check-if-maintenance-is-needed-v1-0-a03d53e28d0e@gmail.com

---
 Documentation/git-maintenance.adoc | 13 ++++++
 builtin/gc.c                       | 93 ++++++++++++++++++++++++++++++++++----
 object.h                           |  1 -
 refs.c                             |  7 +++
 refs.h                             |  7 +++
 refs/debug.c                       | 13 ++++++
 refs/files-backend.c               | 11 +++++
 refs/packed-backend.c              | 13 ++++++
 refs/refs-internal.h               |  6 +++
 refs/reftable-backend.c            | 25 ++++++++++
 reftable/reftable-stack.h          | 11 +++++
 reftable/stack.c                   | 61 +++++++++++++++++++------
 t/t7900-maintenance.sh             | 54 +++++++++++++++-------
 t/unit-tests/u-reftable-stack.c    | 12 ++++-
 14 files changed, 284 insertions(+), 43 deletions(-)

Karthik Nayak (5):
      reftable/stack: return stack segments directly
      reftable/stack: add function to check if optimization is required
      refs: add a `optimize_required` field to `struct ref_storage_be`
      maintenance: add checking logic in `pack_refs_condition()`
      maintenance: add 'is-needed' subcommand

Range-diff versus v3:

1:  48c877bbc4 = 1:  4026aad0e2 reftable/stack: return stack segments directly
2:  b2c0da304b ! 2:  ed2b307572 reftable/stack: add function to check if optimization is required
    @@ reftable/stack.c: static int stack_segments_for_compaction(struct reftable_stack
     -int reftable_stack_auto_compact(struct reftable_stack *st)
     +static int update_segment_if_compaction_required(struct reftable_stack *st,
     +						 struct segment *seg,
    -+						 bool use_heuristics,
    ++						 bool use_geometric,
     +						 bool *required)
      {
     -	struct segment seg;
    @@ reftable/stack.c: static int stack_segments_for_compaction(struct reftable_stack
     +		return 0;
     +	}
     +
    -+	if (!use_heuristics) {
    ++	if (!use_geometric) {
     +		*required = true;
      		return 0;
     +	}
3:  aec4861598 = 3:  4ac6fe6346 refs: add a `optimize_required` field to `struct ref_storage_be`
4:  36d9bfbfe0 ! 4:  e239f9d1d1 maintenance: add checking logic in `pack_refs_condition()`
    @@ builtin/gc.c: static void maintenance_run_opts_release(struct maintenance_run_op
     +	clear_ref_exclusions(&excludes);
     +	string_list_clear(&included_refs, 0);
     +
    -+	return required == true;
    ++	return required;
      }
      
      static int maintenance_task_pack_refs(struct maintenance_run_opts *opts,
5:  2fc7dcb38c ! 5:  2457e5ce0e maintenance: add 'is-needed' subcommand
    @@ Documentation/git-maintenance.adoc: The `unregister` subcommand will report an e
     +    Exits with a 0 status code if maintenance needs to be run, 1 otherwise.
     +    Ideally used with the '--auto' flag.
     ++
    -+If one or more `--task` options	are specified, then those tasks are checked
    ++If one or more `--task` options are specified, then those tasks are checked
     +in that order. Otherwise, the tasks are determined by which
     +`maintenance.<task>.enabled` config options are true. By default, only
     +`maintenance.gc.enabled` is true.
    @@ builtin/gc.c: static int maintenance_stop(int argc, const char **argv, const cha
     +			}
     +		}
     +	} else {
    -+		/* When not using --auto, we should always require maintenance. */
    ++		/*
    ++		 * When not using --auto we always require maintenance right now.
    ++		 *
    ++		 * TODO: this certainly is too eager, as some maintenance tasks may
    ++		 * decide to not do anything because the data structures are already
    ++		 * fully optimized. We may eventually want to extend the auto
    ++		 * condition to also cover non-auto runs so that we can detect such
    ++		 * cases.
    ++		 */
     +		is_needed = true;
     +	}
     +


base-commit: edd2018f5db39d68d55a7a4af42375b1a06b9406
change-id: 20251021-562-add-sub-command-to-check-if-maintenance-is-needed-01cae01b4606

Thanks
- Karthik

[PATCH v4 1/5] reftable/stack: return stack segments directly

[Karthik Nayak]

The `stack_table_sizes_for_compaction()` function returns individual
sizes of each reftable table. This function is only called by
`reftable_stack_auto_compact()` to decide which tables need to be
compacted, if any.

Modify the function to directly return the segments, which avoids the
extra step of receiving the sizes only to pass it to
`suggest_compaction_segment()`.

A future commit will also add functionality for checking whether
auto-compaction is necessary without performing it. This change allows
code re-usability in that context.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 reftable/stack.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/reftable/stack.c b/reftable/stack.c
index 65d89820bd..49387f9344 100644
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -1626,7 +1626,8 @@ struct segment suggest_compaction_segment(uint64_t *sizes, size_t n,
 	return seg;
 }
 
-static uint64_t *stack_table_sizes_for_compaction(struct reftable_stack *st)
+static int stack_segments_for_compaction(struct reftable_stack *st,
+					 struct segment *seg)
 {
 	int version = (st->opts.hash_id == REFTABLE_HASH_SHA1) ? 1 : 2;
 	int overhead = header_size(version) - 1;
@@ -1634,29 +1635,29 @@ static uint64_t *stack_table_sizes_for_compaction(struct reftable_stack *st)
 
 	REFTABLE_CALLOC_ARRAY(sizes, st->merged->tables_len);
 	if (!sizes)
-		return NULL;
+		return REFTABLE_OUT_OF_MEMORY_ERROR;
 
 	for (size_t i = 0; i < st->merged->tables_len; i++)
 		sizes[i] = st->tables[i]->size - overhead;
 
-	return sizes;
+	*seg = suggest_compaction_segment(sizes, st->merged->tables_len,
+					  st->opts.auto_compaction_factor);
+	reftable_free(sizes);
+
+	return 0;
 }
 
 int reftable_stack_auto_compact(struct reftable_stack *st)
 {
 	struct segment seg;
-	uint64_t *sizes;
+	int err;
 
 	if (st->merged->tables_len < 2)
 		return 0;
 
-	sizes = stack_table_sizes_for_compaction(st);
-	if (!sizes)
-		return REFTABLE_OUT_OF_MEMORY_ERROR;
-
-	seg = suggest_compaction_segment(sizes, st->merged->tables_len,
-					 st->opts.auto_compaction_factor);
-	reftable_free(sizes);
+	err = stack_segments_for_compaction(st, &seg);
+	if (err)
+		return err;
 
 	if (segment_size(&seg) > 0)
 		return stack_compact_range(st, seg.start, seg.end - 1,

-- 
2.51.0

[PATCH v4 2/5] reftable/stack: add function to check if optimization is required

[Karthik Nayak]

The reftable backend performs auto-compaction as part of its regular
flow, which is required to keep the number of tables part of a stack at
bay. This allows it to stay optimized.

Compaction can also be triggered voluntarily by the user via the 'git
pack-refs' or the 'git refs optimize' command. However, currently there
is no way for the user to check if optimization is required without
actually performing it.

Extract out the heuristics logic from 'reftable_stack_auto_compact()'
into an internal function 'update_segment_if_compaction_required()'.
Then use this to add and expose `reftable_stack_compaction_required()`
which will allow users to check if the reftable backend can be
optimized.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 reftable/reftable-stack.h       | 11 +++++++++++
 reftable/stack.c                | 42 ++++++++++++++++++++++++++++++++++++-----
 t/unit-tests/u-reftable-stack.c | 12 ++++++++++--
 3 files changed, 58 insertions(+), 7 deletions(-)

diff --git a/reftable/reftable-stack.h b/reftable/reftable-stack.h
index d70fcb705d..c2415cbc6e 100644
--- a/reftable/reftable-stack.h
+++ b/reftable/reftable-stack.h
@@ -123,6 +123,17 @@ struct reftable_log_expiry_config {
 int reftable_stack_compact_all(struct reftable_stack *st,
 			       struct reftable_log_expiry_config *config);
 
+/*
+ * Check if compaction is required.
+ *
+ * When `use_heuristics` is false, check if all tables can be compacted to a
+ * single table. If true, use heuristics to determine if the tables need to be
+ * compacted to maintain geometric progression.
+ */
+int reftable_stack_compaction_required(struct reftable_stack *st,
+				       bool use_heuristics,
+				       bool *required);
+
 /* heuristically compact unbalanced table stack. */
 int reftable_stack_auto_compact(struct reftable_stack *st);
 
diff --git a/reftable/stack.c b/reftable/stack.c
index 49387f9344..1c9f21dfe1 100644
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -1647,19 +1647,51 @@ static int stack_segments_for_compaction(struct reftable_stack *st,
 	return 0;
 }
 
-int reftable_stack_auto_compact(struct reftable_stack *st)
+static int update_segment_if_compaction_required(struct reftable_stack *st,
+						 struct segment *seg,
+						 bool use_geometric,
+						 bool *required)
 {
-	struct segment seg;
 	int err;
 
-	if (st->merged->tables_len < 2)
+	if (st->merged->tables_len < 2) {
+		*required = false;
+		return 0;
+	}
+
+	if (!use_geometric) {
+		*required = true;
 		return 0;
+	}
+
+	err = stack_segments_for_compaction(st, seg);
+	if (err)
+		return err;
+
+	*required = segment_size(seg) > 0;
+	return 0;
+}
+
+int reftable_stack_compaction_required(struct reftable_stack *st,
+				       bool use_heuristics,
+				       bool *required)
+{
+	struct segment seg;
+	return update_segment_if_compaction_required(st, &seg, use_heuristics,
+						     required);
+}
+
+int reftable_stack_auto_compact(struct reftable_stack *st)
+{
+	struct segment seg;
+	bool required;
+	int err;
 
-	err = stack_segments_for_compaction(st, &seg);
+	err = update_segment_if_compaction_required(st, &seg, true, &required);
 	if (err)
 		return err;
 
-	if (segment_size(&seg) > 0)
+	if (required)
 		return stack_compact_range(st, seg.start, seg.end - 1,
 					   NULL, STACK_COMPACT_RANGE_BEST_EFFORT);
 
diff --git a/t/unit-tests/u-reftable-stack.c b/t/unit-tests/u-reftable-stack.c
index a8b91812e8..b8110cdeee 100644
--- a/t/unit-tests/u-reftable-stack.c
+++ b/t/unit-tests/u-reftable-stack.c
@@ -1067,6 +1067,7 @@ void test_reftable_stack__add_performs_auto_compaction(void)
 			.value_type = REFTABLE_REF_SYMREF,
 			.value.symref = (char *) "master",
 		};
+		bool required = false;
 		char buf[128];
 
 		/*
@@ -1087,10 +1088,17 @@ void test_reftable_stack__add_performs_auto_compaction(void)
 		 * auto compaction is disabled. When enabled, we should merge
 		 * all tables in the stack.
 		 */
-		if (i != n)
+		cl_assert_equal_i(reftable_stack_compaction_required(st, true, &required), 0);
+		if (i != n) {
 			cl_assert_equal_i(st->merged->tables_len, i + 1);
-		else
+			if (i < 1)
+				cl_assert_equal_b(required, false);
+			else
+				cl_assert_equal_b(required, true);
+		} else {
 			cl_assert_equal_i(st->merged->tables_len, 1);
+			cl_assert_equal_b(required, false);
+		}
 	}
 
 	reftable_stack_destroy(st);

-- 
2.51.0

[PATCH v4 3/5] refs: add a `optimize_required` field to `struct ref_storage_be`

[Karthik Nayak]

To allow users of the refs namespace to check if the reference backend
requires optimization, add a new field `optimize_required` field to
`struct ref_storage_be`. This field is of type `optimize_required_fn`
which is also introduced in this commit.

Modify the debug, files, packed and reftable backend to implement this
field. A following commit will expose this via 'git pack-refs' and 'git
refs optimize'.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 refs.c                  |  7 +++++++
 refs.h                  |  7 +++++++
 refs/debug.c            | 13 +++++++++++++
 refs/files-backend.c    | 11 +++++++++++
 refs/packed-backend.c   | 13 +++++++++++++
 refs/refs-internal.h    |  6 ++++++
 refs/reftable-backend.c | 25 +++++++++++++++++++++++++
 7 files changed, 82 insertions(+)

diff --git a/refs.c b/refs.c
index 0d0831f29b..5583f6e09d 100644
--- a/refs.c
+++ b/refs.c
@@ -2318,6 +2318,13 @@ int refs_optimize(struct ref_store *refs, struct refs_optimize_opts *opts)
 	return refs->be->optimize(refs, opts);
 }
 
+int refs_optimize_required(struct ref_store *refs,
+			   struct refs_optimize_opts *opts,
+			   bool *required)
+{
+	return refs->be->optimize_required(refs, opts, required);
+}
+
 int reference_get_peeled_oid(struct repository *repo,
 			     const struct reference *ref,
 			     struct object_id *peeled_oid)
diff --git a/refs.h b/refs.h
index 6b05bba527..d9051bbb04 100644
--- a/refs.h
+++ b/refs.h
@@ -520,6 +520,13 @@ struct refs_optimize_opts {
  */
 int refs_optimize(struct ref_store *refs, struct refs_optimize_opts *opts);
 
+/*
+ * Check if refs backend can be optimized by calling 'refs_optimize'.
+ */
+int refs_optimize_required(struct ref_store *ref_store,
+			   struct refs_optimize_opts *opts,
+			   bool *required);
+
 /*
  * Setup reflog before using. Fill in err and return -1 on failure.
  */
diff --git a/refs/debug.c b/refs/debug.c
index 2defd2d465..36f8c58b6c 100644
--- a/refs/debug.c
+++ b/refs/debug.c
@@ -124,6 +124,17 @@ static int debug_optimize(struct ref_store *ref_store, struct refs_optimize_opts
 	return res;
 }
 
+static int debug_optimize_required(struct ref_store *ref_store,
+				   struct refs_optimize_opts *opts,
+				   bool *required)
+{
+	struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;
+	int res = drefs->refs->be->optimize_required(drefs->refs, opts, required);
+	trace_printf_key(&trace_refs, "optimize_required: %s, res: %d\n",
+			 required ? "yes" : "no", res);
+	return res;
+}
+
 static int debug_rename_ref(struct ref_store *ref_store, const char *oldref,
 			    const char *newref, const char *logmsg)
 {
@@ -431,6 +442,8 @@ struct ref_storage_be refs_be_debug = {
 	.transaction_abort = debug_transaction_abort,
 
 	.optimize = debug_optimize,
+	.optimize_required = debug_optimize_required,
+
 	.rename_ref = debug_rename_ref,
 	.copy_ref = debug_copy_ref,
 
diff --git a/refs/files-backend.c b/refs/files-backend.c
index a1e70b1c10..6e0c9b340a 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1512,6 +1512,16 @@ static int files_optimize(struct ref_store *ref_store,
 	return 0;
 }
 
+static int files_optimize_required(struct ref_store *ref_store,
+				   struct refs_optimize_opts *opts,
+				   bool *required)
+{
+	struct files_ref_store *refs = files_downcast(ref_store, REF_STORE_READ,
+						      "optimize_required");
+	*required = should_pack_refs(refs, opts);
+	return 0;
+}
+
 /*
  * People using contrib's git-new-workdir have .git/logs/refs ->
  * /some/other/path/.git/logs/refs, and that may live on another device.
@@ -3982,6 +3992,7 @@ struct ref_storage_be refs_be_files = {
 	.transaction_abort = files_transaction_abort,
 
 	.optimize = files_optimize,
+	.optimize_required = files_optimize_required,
 	.rename_ref = files_rename_ref,
 	.copy_ref = files_copy_ref,
 
diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index 10062fd8b6..19ce4d5872 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -1784,6 +1784,17 @@ static int packed_optimize(struct ref_store *ref_store UNUSED,
 	return 0;
 }
 
+static int packed_optimize_required(struct ref_store *ref_store UNUSED,
+				    struct refs_optimize_opts *opts UNUSED,
+				    bool *required)
+{
+	/*
+	 * Packed refs are already optimized.
+	 */
+	*required = false;
+	return 0;
+}
+
 static struct ref_iterator *packed_reflog_iterator_begin(struct ref_store *ref_store UNUSED)
 {
 	return empty_ref_iterator_begin();
@@ -2130,6 +2141,8 @@ struct ref_storage_be refs_be_packed = {
 	.transaction_abort = packed_transaction_abort,
 
 	.optimize = packed_optimize,
+	.optimize_required = packed_optimize_required,
+
 	.rename_ref = NULL,
 	.copy_ref = NULL,
 
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index dee42f231d..c7d2a6e50b 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -424,6 +424,11 @@ typedef int ref_transaction_commit_fn(struct ref_store *refs,
 
 typedef int optimize_fn(struct ref_store *ref_store,
 			struct refs_optimize_opts *opts);
+
+typedef int optimize_required_fn(struct ref_store *ref_store,
+				 struct refs_optimize_opts *opts,
+				 bool *required);
+
 typedef int rename_ref_fn(struct ref_store *ref_store,
 			  const char *oldref, const char *newref,
 			  const char *logmsg);
@@ -549,6 +554,7 @@ struct ref_storage_be {
 	ref_transaction_abort_fn *transaction_abort;
 
 	optimize_fn *optimize;
+	optimize_required_fn *optimize_required;
 	rename_ref_fn *rename_ref;
 	copy_ref_fn *copy_ref;
 
diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index c23c45f3bf..a3ae0cf74a 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -1733,6 +1733,29 @@ static int reftable_be_optimize(struct ref_store *ref_store,
 	return ret;
 }
 
+static int reftable_be_optimize_required(struct ref_store *ref_store,
+					 struct refs_optimize_opts *opts,
+					 bool *required)
+{
+	struct reftable_ref_store *refs = reftable_be_downcast(ref_store, REF_STORE_READ,
+							       "optimize_refs_required");
+	struct reftable_stack *stack;
+	bool use_heuristics = false;
+
+	if (refs->err)
+		return refs->err;
+
+	stack = refs->worktree_backend.stack;
+	if (!stack)
+		stack = refs->main_backend.stack;
+
+	if (opts->flags & REFS_OPTIMIZE_AUTO)
+		use_heuristics = true;
+
+	return reftable_stack_compaction_required(stack, use_heuristics,
+						  required);
+}
+
 struct write_create_symref_arg {
 	struct reftable_ref_store *refs;
 	struct reftable_stack *stack;
@@ -2756,6 +2779,8 @@ struct ref_storage_be refs_be_reftable = {
 	.transaction_abort = reftable_be_transaction_abort,
 
 	.optimize = reftable_be_optimize,
+	.optimize_required = reftable_be_optimize_required,
+
 	.rename_ref = reftable_be_rename_ref,
 	.copy_ref = reftable_be_copy_ref,
 

-- 
2.51.0

[PATCH v4 4/5] maintenance: add checking logic in `pack_refs_condition()`

[Karthik Nayak]

The 'git-maintenance(1)' command supports an '--auto' flag. Usage of the
flag ensures to run maintenance tasks only if certain thresholds are
met. The heuristic is defined on a task level, wherein each task defines
an 'auto_condition', which states if the task should be run.

The 'pack-refs' task is hard-coded to return 1 as:
1. There was never a way to check if the reference backend needs to be
optimized without actually performing the optimization.
2. We can pass in the '--auto' flag to 'git-pack-refs(1)' which would
optimize based on heuristics.

The previous commit added a `refs_optimize_required()` function, which
can be used to check if a reference backend required optimization. Use
this within `pack_refs_condition()`.

This allows us to add a 'git maintenance is-needed' subcommand which can
notify the user if maintenance is needed without actually performing the
optimization. Without this change, the reference backend would always
state that optimization is needed.

Since we import 'revision.h', we need to remove the definition for
'SEEN' which is duplicated in the included header.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 builtin/gc.c | 30 +++++++++++++++++++++---------
 object.h     |  1 -
 2 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index c6d62c74a7..85e9a38d10 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -35,6 +35,7 @@
 #include "path.h"
 #include "reflog.h"
 #include "rerere.h"
+#include "revision.h"
 #include "blob.h"
 #include "tree.h"
 #include "promisor-remote.h"
@@ -285,12 +286,26 @@ static void maintenance_run_opts_release(struct maintenance_run_opts *opts)
 
 static int pack_refs_condition(UNUSED struct gc_config *cfg)
 {
-	/*
-	 * The auto-repacking logic for refs is handled by the ref backends and
-	 * exposed via `git pack-refs --auto`. We thus always return truish
-	 * here and let the backend decide for us.
-	 */
-	return 1;
+	struct string_list included_refs = STRING_LIST_INIT_NODUP;
+	struct ref_exclusions excludes = REF_EXCLUSIONS_INIT;
+	struct refs_optimize_opts optimize_opts = {
+		.exclusions = &excludes,
+		.includes = &included_refs,
+		.flags = REFS_OPTIMIZE_PRUNE | REFS_OPTIMIZE_AUTO,
+	};
+	bool required;
+
+	/* Check for all refs, similar to 'git refs optimize --all'. */
+	string_list_append(optimize_opts.includes, "*");
+
+	if (refs_optimize_required(get_main_ref_store(the_repository),
+				   &optimize_opts, &required))
+		return 0;
+
+	clear_ref_exclusions(&excludes);
+	string_list_clear(&included_refs, 0);
+
+	return required;
 }
 
 static int maintenance_task_pack_refs(struct maintenance_run_opts *opts,
@@ -1090,9 +1105,6 @@ static int maintenance_opt_schedule(const struct option *opt, const char *arg,
 	return 0;
 }
 
-/* Remember to update object flag allocation in object.h */
-#define SEEN		(1u<<0)
-
 struct cg_auto_data {
 	int num_not_in_graph;
 	int limit;
diff --git a/object.h b/object.h
index 1499f63d50..832299e763 100644
--- a/object.h
+++ b/object.h
@@ -79,7 +79,6 @@ void object_array_init(struct object_array *array);
  * list-objects-filter.c:                                      21
  * bloom.c:                                                    2122
  * builtin/fsck.c:           0--3
- * builtin/gc.c:             0
  * builtin/index-pack.c:                                     2021
  * reflog.c:                           10--12
  * builtin/show-branch.c:    0-------------------------------------------26

-- 
2.51.0

[PATCH v4 5/5] maintenance: add 'is-needed' subcommand

[Karthik Nayak]

The 'git-maintenance(1)' command provides tooling to run maintenance
tasks over Git repositories. The 'run' subcommand, as the name suggests,
runs the maintenance tasks. When used with the '--auto' flag, it uses
heuristics to determine if the required thresholds are met for running
said maintenance tasks.

There is however a lack of insight into these heuristics. Meaning, the
checks are linked to the execution.

Add a new 'is-needed' subcommand to 'git-maintenance(1)' which allows
users to simply check if it is needed to run maintenance without
performing it.

This subcommand can check if it is needed to run maintenance without
actually running it. Ideally it should be used with the '--auto' flag,
which would allow users to check if the thresholds required are met. The
subcommand also supports the '--task' flag which can be used to check
specific maintenance tasks.

While adding the respective tests in 't/t7900-maintenance.sh', remove a
duplicate of the test: 'worktree-prune task with --auto honors
maintenance.worktree-prune.auto'.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 Documentation/git-maintenance.adoc | 13 ++++++++
 builtin/gc.c                       | 63 +++++++++++++++++++++++++++++++++++++-
 t/t7900-maintenance.sh             | 54 ++++++++++++++++++++++----------
 3 files changed, 113 insertions(+), 17 deletions(-)

diff --git a/Documentation/git-maintenance.adoc b/Documentation/git-maintenance.adoc
index 540b5cf68b..bda616f14c 100644
--- a/Documentation/git-maintenance.adoc
+++ b/Documentation/git-maintenance.adoc
@@ -12,6 +12,7 @@ SYNOPSIS
 'git maintenance' run [<options>]
 'git maintenance' start [--scheduler=<scheduler>]
 'git maintenance' (stop|register|unregister) [<options>]
+'git maintenance' is-needed [<options>]
 
 
 DESCRIPTION
@@ -84,6 +85,16 @@ 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.
+    Ideally used with the '--auto' flag.
++
+If one or more `--task` options are specified, then those tasks are checked
+in that order. Otherwise, the tasks are determined by which
+`maintenance.<task>.enabled` config options are true. By default, only
+`maintenance.gc.enabled` is true.
+
 TASKS
 -----
 
@@ -183,6 +194,8 @@ OPTIONS
 	in the `gc.auto` config setting, or when the number of pack-files
 	exceeds the `gc.autoPackLimit` config setting. Not compatible with
 	the `--schedule` option.
+	When combined with the `is-needed` subcommand, check if the required
+	thresholds are met without actually running maintenance.
 
 --schedule::
 	When combined with the `run` subcommand, run maintenance tasks
diff --git a/builtin/gc.c b/builtin/gc.c
index 85e9a38d10..928c805f02 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -3253,7 +3253,67 @@ static int maintenance_stop(int argc, const char **argv, const char *prefix,
 	return update_background_schedule(NULL, 0);
 }
 
-static const char * const builtin_maintenance_usage[] = {
+static const char *const builtin_maintenance_is_needed_usage[] = {
+	"git maintenance is-needed [--task=<task>] [--schedule]",
+	NULL
+};
+
+static int maintenance_is_needed(int argc, const char **argv, const char *prefix,
+				 struct repository *repo UNUSED)
+{
+	struct maintenance_run_opts opts = MAINTENANCE_RUN_OPTS_INIT;
+	struct string_list selected_tasks = STRING_LIST_INIT_DUP;
+	struct gc_config cfg = GC_CONFIG_INIT;
+	struct option options[] = {
+		OPT_BOOL(0, "auto", &opts.auto_flag,
+			 N_("run tasks based on the state of the repository")),
+		OPT_CALLBACK_F(0, "task", &selected_tasks, N_("task"),
+			       N_("check a specific task"),
+			       PARSE_OPT_NONEG, task_option_parse),
+		OPT_END()
+	};
+	bool is_needed = false;
+
+	argc = parse_options(argc, argv, prefix, options,
+			     builtin_maintenance_is_needed_usage,
+			     PARSE_OPT_STOP_AT_NON_OPTION);
+	if (argc)
+		usage_with_options(builtin_maintenance_is_needed_usage, options);
+
+	gc_config(&cfg);
+	initialize_task_config(&opts, &selected_tasks);
+
+	if (opts.auto_flag) {
+		for (size_t i = 0; i < opts.tasks_nr; i++) {
+			if (tasks[opts.tasks[i]].auto_condition &&
+			    tasks[opts.tasks[i]].auto_condition(&cfg)) {
+				is_needed = true;
+				break;
+			}
+		}
+	} else {
+		/*
+		 * When not using --auto we always require maintenance right now.
+		 *
+		 * TODO: this certainly is too eager, as some maintenance tasks may
+		 * decide to not do anything because the data structures are already
+		 * fully optimized. We may eventually want to extend the auto
+		 * condition to also cover non-auto runs so that we can detect such
+		 * cases.
+		 */
+		is_needed = true;
+	}
+
+	string_list_clear(&selected_tasks, 0);
+	maintenance_run_opts_release(&opts);
+	gc_config_release(&cfg);
+
+	if (is_needed)
+		return 0;
+	return 1;
+}
+
+static const char *const builtin_maintenance_usage[] = {
 	N_("git maintenance <subcommand> [<options>]"),
 	NULL,
 };
@@ -3270,6 +3330,7 @@ int cmd_maintenance(int argc,
 		OPT_SUBCOMMAND("stop", &fn, maintenance_stop),
 		OPT_SUBCOMMAND("register", &fn, maintenance_register),
 		OPT_SUBCOMMAND("unregister", &fn, maintenance_unregister),
+		OPT_SUBCOMMAND("is-needed", &fn, maintenance_is_needed),
 		OPT_END(),
 	};
 
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index ddd273d8dc..a17e2091c2 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -49,7 +49,9 @@ test_expect_success 'run [--auto|--quiet]' '
 		git maintenance run --auto 2>/dev/null &&
 	GIT_TRACE2_EVENT="$(pwd)/run-no-quiet.txt" \
 		git maintenance run --no-quiet 2>/dev/null &&
+	git maintenance is-needed &&
 	test_subcommand git gc --quiet --no-detach --skip-foreground-tasks <run-no-auto.txt &&
+	! git maintenance is-needed --auto &&
 	test_subcommand ! git gc --auto --quiet --no-detach --skip-foreground-tasks <run-auto.txt &&
 	test_subcommand git gc --no-quiet --no-detach --skip-foreground-tasks <run-no-quiet.txt
 '
@@ -180,6 +182,11 @@ test_expect_success 'commit-graph auto condition' '
 
 	test_commit first &&
 
+	! git -c maintenance.commit-graph.auto=0 \
+		maintenance is-needed --auto --task=commit-graph &&
+	git -c maintenance.commit-graph.auto=1 \
+		maintenance is-needed --auto --task=commit-graph &&
+
 	GIT_TRACE2_EVENT="$(pwd)/cg-zero-means-no.txt" \
 		git -c maintenance.commit-graph.auto=0 $COMMAND &&
 	GIT_TRACE2_EVENT="$(pwd)/cg-one-satisfied.txt" \
@@ -290,16 +297,23 @@ test_expect_success 'maintenance.loose-objects.auto' '
 		git -c maintenance.loose-objects.auto=1 maintenance \
 		run --auto --task=loose-objects 2>/dev/null &&
 	test_subcommand ! git prune-packed --quiet <trace-lo1.txt &&
+
 	printf data-A | git hash-object -t blob --stdin -w &&
+	! git -c maintenance.loose-objects.auto=2 \
+		maintenance is-needed --auto --task=loose-objects &&
 	GIT_TRACE2_EVENT="$(pwd)/trace-loA" \
 		git -c maintenance.loose-objects.auto=2 \
 		maintenance run --auto --task=loose-objects 2>/dev/null &&
 	test_subcommand ! git prune-packed --quiet <trace-loA &&
+
 	printf data-B | git hash-object -t blob --stdin -w &&
+	git -c maintenance.loose-objects.auto=2 \
+		maintenance is-needed --auto --task=loose-objects &&
 	GIT_TRACE2_EVENT="$(pwd)/trace-loB" \
 		git -c maintenance.loose-objects.auto=2 \
 		maintenance run --auto --task=loose-objects 2>/dev/null &&
 	test_subcommand git prune-packed --quiet <trace-loB &&
+
 	GIT_TRACE2_EVENT="$(pwd)/trace-loC" \
 		git -c maintenance.loose-objects.auto=2 \
 		maintenance run --auto --task=loose-objects 2>/dev/null &&
@@ -421,10 +435,13 @@ run_incremental_repack_and_verify () {
 	test_commit A &&
 	git repack -adk &&
 	git multi-pack-index write &&
+	! git -c maintenance.incremental-repack.auto=1 \
+		maintenance is-needed --auto --task=incremental-repack &&
 	GIT_TRACE2_EVENT="$(pwd)/midx-init.txt" git \
 		-c maintenance.incremental-repack.auto=1 \
 		maintenance run --auto --task=incremental-repack 2>/dev/null &&
 	test_subcommand ! git multi-pack-index write --no-progress <midx-init.txt &&
+
 	test_commit B &&
 	git pack-objects --revs .git/objects/pack/pack <<-\EOF &&
 	HEAD
@@ -434,11 +451,14 @@ run_incremental_repack_and_verify () {
 		-c maintenance.incremental-repack.auto=2 \
 		maintenance run --auto --task=incremental-repack 2>/dev/null &&
 	test_subcommand ! git multi-pack-index write --no-progress <trace-A &&
+
 	test_commit C &&
 	git pack-objects --revs .git/objects/pack/pack <<-\EOF &&
 	HEAD
 	^HEAD~1
 	EOF
+	git -c maintenance.incremental-repack.auto=2 \
+		maintenance is-needed --auto --task=incremental-repack &&
 	GIT_TRACE2_EVENT=$(pwd)/trace-B git \
 		-c maintenance.incremental-repack.auto=2 \
 		maintenance run --auto --task=incremental-repack 2>/dev/null &&
@@ -485,9 +505,15 @@ test_expect_success 'reflog-expire task --auto only packs when exceeding limits'
 	git reflog expire --all --expire=now &&
 	test_commit reflog-one &&
 	test_commit reflog-two &&
+
+	! git -c maintenance.reflog-expire.auto=3 \
+		maintenance is-needed --auto --task=reflog-expire &&
 	GIT_TRACE2_EVENT="$(pwd)/reflog-expire-auto.txt" \
 		git -c maintenance.reflog-expire.auto=3 maintenance run --auto --task=reflog-expire &&
 	test_subcommand ! git reflog expire --all <reflog-expire-auto.txt &&
+
+	git -c maintenance.reflog-expire.auto=2 \
+		maintenance is-needed --auto --task=reflog-expire &&
 	GIT_TRACE2_EVENT="$(pwd)/reflog-expire-auto.txt" \
 		git -c maintenance.reflog-expire.auto=2 maintenance run --auto --task=reflog-expire &&
 	test_subcommand git reflog expire --all <reflog-expire-auto.txt
@@ -514,6 +540,7 @@ test_expect_success 'worktree-prune task --auto only prunes with prunable worktr
 	test_expect_worktree_prune ! git maintenance run --auto --task=worktree-prune &&
 	mkdir .git/worktrees &&
 	: >.git/worktrees/abc &&
+	git maintenance is-needed --auto --task=worktree-prune &&
 	test_expect_worktree_prune git maintenance run --auto --task=worktree-prune
 '
 
@@ -530,22 +557,7 @@ test_expect_success 'worktree-prune task with --auto honors maintenance.worktree
 	test_expect_worktree_prune ! git -c maintenance.worktree-prune.auto=0 maintenance run --auto --task=worktree-prune &&
 	# A positive value should require at least this many prunable worktrees.
 	test_expect_worktree_prune ! git -c maintenance.worktree-prune.auto=4 maintenance run --auto --task=worktree-prune &&
-	test_expect_worktree_prune git -c maintenance.worktree-prune.auto=3 maintenance run --auto --task=worktree-prune
-'
-
-test_expect_success 'worktree-prune task with --auto honors maintenance.worktree-prune.auto' '
-	# A negative value should always prune.
-	test_expect_worktree_prune git -c maintenance.worktree-prune.auto=-1 maintenance run --auto --task=worktree-prune &&
-
-	mkdir .git/worktrees &&
-	: >.git/worktrees/first &&
-	: >.git/worktrees/second &&
-	: >.git/worktrees/third &&
-
-	# Zero should never prune.
-	test_expect_worktree_prune ! git -c maintenance.worktree-prune.auto=0 maintenance run --auto --task=worktree-prune &&
-	# A positive value should require at least this many prunable worktrees.
-	test_expect_worktree_prune ! git -c maintenance.worktree-prune.auto=4 maintenance run --auto --task=worktree-prune &&
+	git -c maintenance.worktree-prune.auto=3 maintenance is-needed --auto --task=worktree-prune &&
 	test_expect_worktree_prune git -c maintenance.worktree-prune.auto=3 maintenance run --auto --task=worktree-prune
 '
 
@@ -554,11 +566,13 @@ test_expect_success 'worktree-prune task honors gc.worktreePruneExpire' '
 	rm -rf worktree &&
 
 	rm -f worktree-prune.txt &&
+	! git -c gc.worktreePruneExpire=1.week.ago maintenance is-needed --auto --task=worktree-prune &&
 	GIT_TRACE2_EVENT="$(pwd)/worktree-prune.txt" git -c gc.worktreePruneExpire=1.week.ago maintenance run --auto --task=worktree-prune &&
 	test_subcommand ! git worktree prune --expire 1.week.ago <worktree-prune.txt &&
 	test_path_is_dir .git/worktrees/worktree &&
 
 	rm -f worktree-prune.txt &&
+	git -c gc.worktreePruneExpire=now maintenance is-needed --auto --task=worktree-prune &&
 	GIT_TRACE2_EVENT="$(pwd)/worktree-prune.txt" git -c gc.worktreePruneExpire=now maintenance run --auto --task=worktree-prune &&
 	test_subcommand git worktree prune --expire now <worktree-prune.txt &&
 	test_path_is_missing .git/worktrees/worktree
@@ -583,10 +597,13 @@ test_expect_success 'rerere-gc task without --auto always collects garbage' '
 
 test_expect_success 'rerere-gc task with --auto only prunes with prunable entries' '
 	test_when_finished "rm -rf .git/rr-cache" &&
+	! git maintenance is-needed --auto --task=rerere-gc &&
 	test_expect_rerere_gc ! git maintenance run --auto --task=rerere-gc &&
 	mkdir .git/rr-cache &&
+	! git maintenance is-needed --auto --task=rerere-gc &&
 	test_expect_rerere_gc ! git maintenance run --auto --task=rerere-gc &&
 	: >.git/rr-cache/entry &&
+	git maintenance is-needed --auto --task=rerere-gc &&
 	test_expect_rerere_gc git maintenance run --auto --task=rerere-gc
 '
 
@@ -594,17 +611,22 @@ test_expect_success 'rerere-gc task with --auto honors maintenance.rerere-gc.aut
 	test_when_finished "rm -rf .git/rr-cache" &&
 
 	# A negative value should always prune.
+	git -c maintenance.rerere-gc.auto=-1 maintenance is-needed --auto --task=rerere-gc &&
 	test_expect_rerere_gc git -c maintenance.rerere-gc.auto=-1 maintenance run --auto --task=rerere-gc &&
 
 	# A positive value prunes when there is at least one entry.
+	! git -c maintenance.rerere-gc.auto=9000 maintenance is-needed --auto --task=rerere-gc &&
 	test_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=9000 maintenance run --auto --task=rerere-gc &&
 	mkdir .git/rr-cache &&
+	! git -c maintenance.rerere-gc.auto=9000 maintenance is-needed --auto --task=rerere-gc &&
 	test_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=9000 maintenance run --auto --task=rerere-gc &&
 	: >.git/rr-cache/entry-1 &&
+	git -c maintenance.rerere-gc.auto=9000 maintenance is-needed --auto --task=rerere-gc &&
 	test_expect_rerere_gc git -c maintenance.rerere-gc.auto=9000 maintenance run --auto --task=rerere-gc &&
 
 	# Zero should never prune.
 	: >.git/rr-cache/entry-1 &&
+	! git -c maintenance.rerere-gc.auto=0 maintenance is-needed --auto --task=rerere-gc &&
 	test_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=0 maintenance run --auto --task=rerere-gc
 '
 

-- 
2.51.0

Re: [PATCH v4 0/5] maintenance: add an 'is-needed' subcommand

[Patrick Steinhardt]

On Sat, Nov 08, 2025 at 10:51:52PM +0100, Karthik Nayak wrote:
> Changes in v4:
> - In `update_segment_if_compaction_required()` change the argument name
>   from `use_heuristics` to `use_geometric` since we only have one
>   heuristic currently and this is much clearer to understand.
> - There were a lot of discussion on how to return a bool variable when
>   the function has a return type of int. We discussed both '!!required',
>   and 'required != true'. I'm going to punt this discussion keeping it
>   simple as 'return required' as in my first version, since even Junio
>   expressed his thoughts in favor of it.
> - Add a TODO for improvements to the flow when running `git maintenance
>   is-needed` without the `--auto` flag.
> - Link to v3: https://patch.msgid.link/20251106-562-add-sub-command-to-check-if-maintenance-is-needed-v3-0-d611a2a95cf5@gmail.com

I'm happy with this version. Thanks!

Patrick