patch, 9 partsrefs: add a '--required' flag to 'git refs optimize'

> The `struct ref_store` variable, exposes two ways to optimize a reftable
> backend:
> 
>   1. pack_refs
>   2. optimize
> 
> The former was specific to the 'files' + 'packed' refs backend. The
> latter is more generic and covers all backends. While the naming is
> different, both these tend to perform the same functionality.

> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index a4cda57981..0b81bd7f74 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -1445,8 +1445,8 @@ static int should_pack_refs(struct files_ref_store *refs,
>  	return 0;
>  }
>  
> -static int files_pack_refs(struct ref_store *ref_store,
> -			   struct pack_refs_opts *opts)
> +static int files_optimize(struct ref_store *ref_store,
> +			  struct pack_refs_opts *opts)

> On Fri, Oct 10, 2025 at 12:27:05PM +0200, Karthik Nayak wrote:
>> The `struct ref_store` variable, exposes two ways to optimize a reftable
>> backend:
>>
>>   1. pack_refs
>>   2. optimize
>>
>> The former was specific to the 'files' + 'packed' refs backend. The
>> latter is more generic and covers all backends. While the naming is
>> different, both these tend to perform the same functionality.
>
> "tend to perform" is a bit of a curious thing to say, as it raises the
> question when it doesn't.
>

>> In the following commit, we will consolidate this code to only maintain
>> the 'optimize' functions. In preparation, modify the backends to also do
>> the same, by moving to supporting the 'optimize' function.
>
> Maybe: "In preparation, modify the backends so that they exclusively
> implement the `optimize` callback, only."
>

>> diff --git a/refs/files-backend.c b/refs/files-backend.c
>> index a4cda57981..0b81bd7f74 100644
>> --- a/refs/files-backend.c
>> +++ b/refs/files-backend.c
>> @@ -1445,8 +1445,8 @@ static int should_pack_refs(struct files_ref_store *refs,
>>  	return 0;
>>  }
>>
>> -static int files_pack_refs(struct ref_store *ref_store,
>> -			   struct pack_refs_opts *opts)
>> +static int files_optimize(struct ref_store *ref_store,
>> +			  struct pack_refs_opts *opts)
>
> `struct pack_refs_opts` really should be renamed to
> `refs_optimize_opts`. Let's read on, maybe you do this in subsequent
> patches.
>

> The previous commit, moved all backends to only use/support the
> 'optimize' function within the `ref_store` structure. With this, cleanup
> all references to the 'pack_refs' field of the structure and code around
> it.
> 
> Modify existing documentation in this regard.

> diff --git a/refs.h b/refs.h
> index 2dd7ac1a16..c6c955d78d 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -514,15 +514,11 @@ struct pack_refs_opts {
>  	struct string_list *includes;
>  };
>  
> -/*
> - * Write a packed-refs file for the current repository.
> - * flags: Combination of the above PACK_REFS_* flags.
> - */
> -int refs_pack_refs(struct ref_store *refs, struct pack_refs_opts *opts);
> -
>  /*
>   * Optimize the ref store. The exact behavior is up to the backend.
>   * For the files backend, this is equivalent to packing refs.
> + *
> + * flags: Combination of the above PACK_REFS_* flags.
>   */
>  int refs_optimize(struct ref_store *refs, struct pack_refs_opts *opts);
>  

> On Fri, Oct 10, 2025 at 12:27:06PM +0200, Karthik Nayak wrote:
>> The previous commit, moved all backends to only use/support the
>> 'optimize' function within the `ref_store` structure. With this, cleanup
>> all references to the 'pack_refs' field of the structure and code around
>> it.
>>
>> Modify existing documentation in this regard.
>
> Makes sense.
>
>> diff --git a/refs.h b/refs.h
>> index 2dd7ac1a16..c6c955d78d 100644
>> --- a/refs.h
>> +++ b/refs.h
>> @@ -514,15 +514,11 @@ struct pack_refs_opts {
>>  	struct string_list *includes;
>>  };
>>
>> -/*
>> - * Write a packed-refs file for the current repository.
>> - * flags: Combination of the above PACK_REFS_* flags.
>> - */
>> -int refs_pack_refs(struct ref_store *refs, struct pack_refs_opts *opts);
>> -
>>  /*
>>   * Optimize the ref store. The exact behavior is up to the backend.
>>   * For the files backend, this is equivalent to packing refs.
>> + *
>> + * flags: Combination of the above PACK_REFS_* flags.
>>   */
>>  int refs_optimize(struct ref_store *refs, struct pack_refs_opts *opts);
>>
>
> I don't think it makes sense to carry over this documentation here. If
> not already the case, we should document the `struct opts::flags` field
> in the structure itself.
>
> Patrick

> The previous commit removed all references to 'pack_refs()' within
> the refs subsystem. Continue this cleanup by also renaming
> 'pack_refs_opts' to 'optimize_refs_opts' and the respective flags
> accordingly. Keeping the naming consistent will make the code easier to
> maintain.

> diff --git a/refs.c b/refs.c
> index 77dc1ab501..514fb85af2 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -2312,7 +2312,7 @@ void base_ref_store_init(struct ref_store *refs, struct repository *repo,
>  	refs->gitdir = xstrdup(path);
>  }
>  
> -int refs_optimize(struct ref_store *refs, struct pack_refs_opts *opts)
> +int refs_optimize(struct ref_store *refs, struct optimize_refs_opts *opts)

> diff --git a/refs.h b/refs.h
> index c6c955d78d..58b222ac02 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -500,15 +500,15 @@ void refs_warn_dangling_symrefs(struct ref_store *refs, FILE *fp,
>  
>  /*
>   * Flags for controlling behaviour of refs_optimize()
> - * PACK_REFS_PRUNE: Prune loose refs after packing
> - * PACK_REFS_AUTO: Pack refs on a best effort basis. The heuristics and end
> - *                 result are decided by the ref backend. Backends may ignore
> - *                 this flag and fall back to a normal repack.
> + * OPTIMIZE_REFS_PRUNE: Prune loose refs after packing
> + * OPTIMIZE_REFS_AUTO: Pack refs on a best effort basis. The heuristics and end
> + *                     result are decided by the ref backend. Backends may ignore
> + *                     this flag and fall back to a normal repack.
>   */
> -#define PACK_REFS_PRUNE (1 << 0)
> -#define PACK_REFS_AUTO  (1 << 1)
> +#define OPTIMIZE_REFS_PRUNE (1 << 0)
> +#define OPTIMIZE_REFS_AUTO  (1 << 1)

> On Fri, Oct 10, 2025 at 12:27:07PM +0200, Karthik Nayak wrote:
>> The previous commit removed all references to 'pack_refs()' within
>> the refs subsystem. Continue this cleanup by also renaming
>> 'pack_refs_opts' to 'optimize_refs_opts' and the respective flags
>> accordingly. Keeping the naming consistent will make the code easier to
>> maintain.
>
> Good, this is what I was hoping to see :)
>
>> diff --git a/refs.c b/refs.c
>> index 77dc1ab501..514fb85af2 100644
>> --- a/refs.c
>> +++ b/refs.c
>> @@ -2312,7 +2312,7 @@ void base_ref_store_init(struct ref_store *refs, struct repository *repo,
>>  	refs->gitdir = xstrdup(path);
>>  }
>>
>> -int refs_optimize(struct ref_store *refs, struct pack_refs_opts *opts)
>> +int refs_optimize(struct ref_store *refs, struct optimize_refs_opts *opts)
>
> An options struct for a function `do_something()` should typically be
> called `struct do_something_opts`. So we should rename the struct to
> `refs_optimize_opts`.
>

>> diff --git a/refs.h b/refs.h
>> index c6c955d78d..58b222ac02 100644
>> --- a/refs.h
>> +++ b/refs.h
>> @@ -500,15 +500,15 @@ void refs_warn_dangling_symrefs(struct ref_store *refs, FILE *fp,
>>
>>  /*
>>   * Flags for controlling behaviour of refs_optimize()
>> - * PACK_REFS_PRUNE: Prune loose refs after packing
>> - * PACK_REFS_AUTO: Pack refs on a best effort basis. The heuristics and end
>> - *                 result are decided by the ref backend. Backends may ignore
>> - *                 this flag and fall back to a normal repack.
>> + * OPTIMIZE_REFS_PRUNE: Prune loose refs after packing
>> + * OPTIMIZE_REFS_AUTO: Pack refs on a best effort basis. The heuristics and end
>> + *                     result are decided by the ref backend. Backends may ignore
>> + *                     this flag and fall back to a normal repack.
>>   */
>> -#define PACK_REFS_PRUNE (1 << 0)
>> -#define PACK_REFS_AUTO  (1 << 1)
>> +#define OPTIMIZE_REFS_PRUNE (1 << 0)
>> +#define OPTIMIZE_REFS_AUTO  (1 << 1)
>
> And these would then be called `REFS_OPTIMIZE_PRUNE` etc.
>
> Patrick

> diff --git a/t/pack-refs-tests.sh b/t/pack-refs-tests.sh
> index 095823d915..6a71838ffa 100644
> --- a/t/pack-refs-tests.sh
> +++ b/t/pack-refs-tests.sh
> @@ -460,4 +460,3 @@ test_expect_success 'pack-refs does not store invalid peeled tag value' '
>  	)
>  '
>  
> -test_done

> On Fri, Oct 10, 2025 at 12:27:08PM +0200, Karthik Nayak wrote:
>> diff --git a/t/pack-refs-tests.sh b/t/pack-refs-tests.sh
>> index 095823d915..6a71838ffa 100644
>> --- a/t/pack-refs-tests.sh
>> +++ b/t/pack-refs-tests.sh
>> @@ -460,4 +460,3 @@ test_expect_success 'pack-refs does not store invalid peeled tag value' '
>>  	)
>>  '
>>
>> -test_done
>
> There's an empty trailing line now.
>
> Patrick

> 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 f91ce50bcd..9d9326ce0e 100644
> --- a/reftable/stack.c
> +++ b/reftable/stack.c
> @@ -1639,29 +1640,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 = 0;

> On Fri, Oct 10, 2025 at 12:27:10PM +0200, 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 process the table sizes.
>
> Maybe: "to decide which tables need to be compacted, if any."
>

>> extra step of receiving the sizes only to pass it to
>> `suggest_compaction_segment()`.
>>
>> A future commit will also add functionality for checking if
>
> s/if/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 f91ce50bcd..9d9326ce0e 100644
>> --- a/reftable/stack.c
>> +++ b/reftable/stack.c
>> @@ -1639,29 +1640,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 = 0;
>
> The initialization is unnecessary.
>

> Patrick Steinhardt <ps@pks.im> writes:
> >> diff --git a/reftable/stack.c b/reftable/stack.c
> >> index f91ce50bcd..9d9326ce0e 100644
> >> --- a/reftable/stack.c
> >> +++ b/reftable/stack.c
> >> @@ -1639,29 +1640,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 = 0;
> >
> > The initialization is unnecessary.
> >
> 
> True, but I always think its nicer to define the base state like this.
> This makes it less error prone when/if the code is modified in the
> future. But doesn't really matter, so let me remove it.

> diff --git a/reftable/reftable-stack.h b/reftable/reftable-stack.h
> index d70fcb705d..754e955206 100644
> --- a/reftable/reftable-stack.h
> +++ b/reftable/reftable-stack.h
> @@ -123,6 +123,9 @@ struct reftable_log_expiry_config {
>  int reftable_stack_compact_all(struct reftable_stack *st,
>  			       struct reftable_log_expiry_config *config);
>  
> +/* check if heuristic based compaction is required  */
> +int reftable_stack_compaction_required(struct reftable_stack *st, bool *required);

> diff --git a/reftable/stack.c b/reftable/stack.c
> index 9d9326ce0e..732141d46c 100644
> --- a/reftable/stack.c
> +++ b/reftable/stack.c
> @@ -1652,6 +1652,24 @@ static int stack_segments_for_compaction(struct reftable_stack *st,
>  	return 0;
>  }
>  
> +int reftable_stack_compaction_required(struct reftable_stack *st, bool *required)
> +{
> +	struct segment seg;
> +	int err = 0;
> +
> +	if (st->merged->tables_len < 2) {
> +		*required = false;
> +		return 0;
> +	}
> +
> +	err = stack_segments_for_compaction(st, &seg);
> +	if (err)
> +		return err;
> +
> +	*required = segment_size(&seg) > 0;
> +	return 0;
> +}

> On Fri, Oct 10, 2025 at 12:27:11PM +0200, Karthik Nayak wrote:
>> The reftable backend for references, performs auto-compaction as part of
>
> s/for references, //
>
>> its regular flow. This allows it to stay optimized.
>
> Maybe: "flow, which is required to keep the number of tables part of a
> stack at bay."
>

>> diff --git a/reftable/reftable-stack.h b/reftable/reftable-stack.h
>> index d70fcb705d..754e955206 100644
>> --- a/reftable/reftable-stack.h
>> +++ b/reftable/reftable-stack.h
>> @@ -123,6 +123,9 @@ struct reftable_log_expiry_config {
>>  int reftable_stack_compact_all(struct reftable_stack *st,
>>  			       struct reftable_log_expiry_config *config);
>>
>> +/* check if heuristic based compaction is required  */
>> +int reftable_stack_compaction_required(struct reftable_stack *st, bool *required);
>
> Let's use a full sentence here, starting with an upper-case letter and
> ending with punctuation.
>

> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index 1c37899006..c262ae1a7b 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -1513,6 +1513,16 @@ static int files_optimize(struct ref_store *ref_store,
>  	return 0;
>  }
>  
> +static int files_optimize_required(struct ref_store *ref_store,
> +				   struct optimize_refs_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;
> +}

> diff --git a/refs/packed-backend.c b/refs/packed-backend.c
> index acaa5a6e57..c94948f618 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 optimize_refs_opts *opts UNUSED,
> +				    bool *required)
> +{
> +	/*
> +	 * Packed refs are already optimized.
> +	 */
> +	*required = false;
> +	return 0;
> +}

> diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
> index d77714366a..df39fe9b38 100644
> --- a/refs/reftable-backend.c
> +++ b/refs/reftable-backend.c
> @@ -1732,6 +1732,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 optimize_refs_opts *opts,
> +					 bool *required)
> +{
> +	struct reftable_ref_store *refs = reftable_be_downcast(ref_store, REF_STORE_READ,
> +							       "optimize_refs_required");
> +	struct reftable_stack *stack;
> +
> +	if (refs->err)
> +		return refs->err;
> +
> +	stack = refs->worktree_backend.stack;
> +	if (!stack)
> +		stack = refs->main_backend.stack;
> +
> +	if (opts->flags & OPTIMIZE_REFS_AUTO)
> +		return reftable_stack_compaction_required(stack, required);
> +	else
> +		*required = true;

> On Fri, Oct 10, 2025 at 12:27:12PM +0200, Karthik Nayak wrote:
>> diff --git a/refs/files-backend.c b/refs/files-backend.c
>> index 1c37899006..c262ae1a7b 100644
>> --- a/refs/files-backend.c
>> +++ b/refs/files-backend.c
>> @@ -1513,6 +1513,16 @@ static int files_optimize(struct ref_store *ref_store,
>>  	return 0;
>>  }
>>
>> +static int files_optimize_required(struct ref_store *ref_store,
>> +				   struct optimize_refs_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;
>> +}
>
> Okay, this is nice and straight-forward. One might argue that we could
> also have the following (uncompiled, so pseudo-code-y) implementation:
>
> 	static int files_optimize_required(struct ref_store *ref_store,
> 					   struct optimize_refs_opts *opts,
> 					   bool *required)
> 	{
> 		struct files_ref_store *refs = files_downcast(ref_store, REF_STORE_READ,
> 							      "optimize_required");
> 		*required = should_pack_refs(refs, opts) ||
> 			refs_optimize_required(refs->packed_refs, opts);
> 		return 0;
> 	}
>
> But on the other hand we know that they aren't ever repacked, so it's
> probably dumb.
>

>> diff --git a/refs/packed-backend.c b/refs/packed-backend.c
>> index acaa5a6e57..c94948f618 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 optimize_refs_opts *opts UNUSED,
>> +				    bool *required)
>> +{
>> +	/*
>> +	 * Packed refs are already optimized.
>> +	 */
>> +	*required = false;
>> +	return 0;
>> +}
>
> Yup.
>
>> diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
>> index d77714366a..df39fe9b38 100644
>> --- a/refs/reftable-backend.c
>> +++ b/refs/reftable-backend.c
>> @@ -1732,6 +1732,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 optimize_refs_opts *opts,
>> +					 bool *required)
>> +{
>> +	struct reftable_ref_store *refs = reftable_be_downcast(ref_store, REF_STORE_READ,
>> +							       "optimize_refs_required");
>> +	struct reftable_stack *stack;
>> +
>> +	if (refs->err)
>> +		return refs->err;
>> +
>> +	stack = refs->worktree_backend.stack;
>> +	if (!stack)
>> +		stack = refs->main_backend.stack;
>> +
>> +	if (opts->flags & OPTIMIZE_REFS_AUTO)
>> +		return reftable_stack_compaction_required(stack, required);
>> +	else
>> +		*required = true;
>
> This doesn't make much sense. We should only indicate that we require
> repacking when there are at least two tables in the stack.
>
> Patrick

> The 'git pack-refs' and 'git refs optimize' commands allow users to
> optimize their reference backends. However they provide no functionality
> to check if optimization is required without performing it.
> 
> Add a '--required' flag to these commands to do that. This is useful
> on the server side where this information can be utilized to perform
> more targetted maintenance runs of the repository.

> diff --git a/Documentation/git-pack-refs.adoc b/Documentation/git-pack-refs.adoc
> index fde9f2f294..62bc01b29b 100644
> --- a/Documentation/git-pack-refs.adoc
> +++ b/Documentation/git-pack-refs.adoc
> @@ -9,6 +9,7 @@ SYNOPSIS
>  --------
>  [verse]
>  'git pack-refs' [--all] [--no-prune] [--auto] [--include <pattern>] [--exclude <pattern>]
> +                [--required]

> diff --git a/Documentation/pack-refs-options.adoc b/Documentation/pack-refs-options.adoc
> index 0b11282941..66d69530b9 100644
> --- a/Documentation/pack-refs-options.adoc
> +++ b/Documentation/pack-refs-options.adoc
> @@ -50,3 +50,8 @@ the provided `--exclude` patterns.
>  +
>  When used with `--include`, refs provided to `--include`, minus refs that are
>  provided to `--exclude` will be packed.
> +
> +--required::
> +
> +Check if pack-refs is required to run, without actually performing the changes.

> diff --git a/pack-refs.c b/pack-refs.c
> index fee77fbf9f..5d4d4266de 100644
> --- a/pack-refs.c
> +++ b/pack-refs.c
> @@ -21,6 +21,7 @@ int pack_refs_core(int argc,
>  	};
>  	struct string_list option_excluded_refs = STRING_LIST_INIT_NODUP;
>  	struct string_list_item *item;
> +	bool check_required = false;
>  	int pack_all = 0;
>  	int ret;
>  
> @@ -28,6 +29,7 @@ int pack_refs_core(int argc,
>  		OPT_BOOL(0, "all",   &pack_all, N_("pack everything")),
>  		OPT_BIT(0, "prune", &pack_refs_opts.flags, N_("prune loose refs (default)"), OPTIMIZE_REFS_PRUNE),
>  		OPT_BIT(0, "auto", &pack_refs_opts.flags, N_("auto-pack refs as needed"), OPTIMIZE_REFS_AUTO),
> +		OPT_BOOL(0, "required", &check_required, N_("check if optimization is required")),
>  		OPT_STRING_LIST(0, "include", pack_refs_opts.includes, N_("pattern"),
>  			N_("references to include")),
>  		OPT_STRING_LIST(0, "exclude", &option_excluded_refs, N_("pattern"),
> @@ -47,7 +49,14 @@ int pack_refs_core(int argc,
>  	if (!pack_refs_opts.includes->nr)
>  		string_list_append(pack_refs_opts.includes, "refs/tags/*");
>  
> -	ret = refs_optimize(get_main_ref_store(repo), &pack_refs_opts);
> +	if (check_required) {
> +		bool required = false;
> +		ret = refs_optimize_required(get_main_ref_store(repo), &pack_refs_opts,
> +					     &required);
> +		ret |= !required;

> diff --git a/refs.c b/refs.c
> index 514fb85af2..59a48b36b7 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -2317,6 +2317,13 @@ int refs_optimize(struct ref_store *refs, struct optimize_refs_opts *opts)
>  	return refs->be->optimize(refs, opts);
>  }
>  
> +int refs_optimize_required(struct ref_store *refs,
> +			   struct optimize_refs_opts *opts,
> +			   bool *required)
> +{
> +	return refs->be->optimize_required(refs, opts, required);
> +}
> +

> On Fri, Oct 10, 2025 at 12:27:13PM +0200, Karthik Nayak wrote:
>> The 'git pack-refs' and 'git refs optimize' commands allow users to
>> optimize their reference backends. However they provide no functionality
>> to check if optimization is required without performing it.
>>
>> Add a '--required' flag to these commands to do that. This is useful
>> on the server side where this information can be utilized to perform
>> more targetted maintenance runs of the repository.
>
> s/targetted/targeted/
>

>> Add a corresponding test for the files backend. For the reftable
>> backend, this cannot be tested easily as it performs auto-compaction.
>> However, an earlier commit ensured the functionality was covered by
>> unit test.
>
> You can disable auto-compaction via the
> "GIT_TEST_REFTABLE_AUTOCOMPACTION" environment variable, so it should be
> rather easy to add a test.
>

>> diff --git a/Documentation/git-pack-refs.adoc b/Documentation/git-pack-refs.adoc
>> index fde9f2f294..62bc01b29b 100644
>> --- a/Documentation/git-pack-refs.adoc
>> +++ b/Documentation/git-pack-refs.adoc
>> @@ -9,6 +9,7 @@ SYNOPSIS
>>  --------
>>  [verse]
>>  'git pack-refs' [--all] [--no-prune] [--auto] [--include <pattern>] [--exclude <pattern>]
>> +                [--required]
>
> Hm, I'm not a huge fan of that name. "--dry-run" might be a better name,
> but on the other hand one might expect that such a command also yields
> information about what happens or that the compaction should be
> successful. So I cannot come up with a better name, either.
>

>> diff --git a/Documentation/pack-refs-options.adoc b/Documentation/pack-refs-options.adoc
>> index 0b11282941..66d69530b9 100644
>> --- a/Documentation/pack-refs-options.adoc
>> +++ b/Documentation/pack-refs-options.adoc
>> @@ -50,3 +50,8 @@ the provided `--exclude` patterns.
>>  +
>>  When used with `--include`, refs provided to `--include`, minus refs that are
>>  provided to `--exclude` will be packed.
>> +
>> +--required::
>> +
>> +Check if pack-refs is required to run, without actually performing the changes.
>
> Let's not say "pack-refs" here anymore, as we're migrating away from
> that name towards more generic terminology. Maybe:
>
>     Check whether the reference store needs to be optimized without
>     actually performing the changes.
>

>> diff --git a/pack-refs.c b/pack-refs.c
>> index fee77fbf9f..5d4d4266de 100644
>> --- a/pack-refs.c
>> +++ b/pack-refs.c
>> @@ -21,6 +21,7 @@ int pack_refs_core(int argc,
>>  	};
>>  	struct string_list option_excluded_refs = STRING_LIST_INIT_NODUP;
>>  	struct string_list_item *item;
>> +	bool check_required = false;
>>  	int pack_all = 0;
>>  	int ret;
>>
>> @@ -28,6 +29,7 @@ int pack_refs_core(int argc,
>>  		OPT_BOOL(0, "all",   &pack_all, N_("pack everything")),
>>  		OPT_BIT(0, "prune", &pack_refs_opts.flags, N_("prune loose refs (default)"), OPTIMIZE_REFS_PRUNE),
>>  		OPT_BIT(0, "auto", &pack_refs_opts.flags, N_("auto-pack refs as needed"), OPTIMIZE_REFS_AUTO),
>> +		OPT_BOOL(0, "required", &check_required, N_("check if optimization is required")),
>>  		OPT_STRING_LIST(0, "include", pack_refs_opts.includes, N_("pattern"),
>>  			N_("references to include")),
>>  		OPT_STRING_LIST(0, "exclude", &option_excluded_refs, N_("pattern"),
>> @@ -47,7 +49,14 @@ int pack_refs_core(int argc,
>>  	if (!pack_refs_opts.includes->nr)
>>  		string_list_append(pack_refs_opts.includes, "refs/tags/*");
>>
>> -	ret = refs_optimize(get_main_ref_store(repo), &pack_refs_opts);
>> +	if (check_required) {
>> +		bool required = false;
>> +		ret = refs_optimize_required(get_main_ref_store(repo), &pack_refs_opts,
>> +					     &required);
>> +		ret |= !required;
>
> I think we shouldn't use `|=` here, and furthermore I think we need to
> define semantics more specifically. E.g.:
>
>     - Return 0 in case optimization is required.
>
>     - Return 2 in case no optimization is required. I mostly shy away
>       from `1` here because it might already be used? We'll have to
>       double check though.
>
>     - Return any other non-zero error code in case an error occurs.
>
> This also needs to be documented.
>

>> diff --git a/refs.c b/refs.c
>> index 514fb85af2..59a48b36b7 100644
>> --- a/refs.c
>> +++ b/refs.c
>> @@ -2317,6 +2317,13 @@ int refs_optimize(struct ref_store *refs, struct optimize_refs_opts *opts)
>>  	return refs->be->optimize(refs, opts);
>>  }
>>
>> +int refs_optimize_required(struct ref_store *refs,
>> +			   struct optimize_refs_opts *opts,
>> +			   bool *required)
>> +{
>> +	return refs->be->optimize_required(refs, opts, required);
>> +}
>> +
>
> I feel like this should be introduced in a preceding commit.
>
> Patrick

>>  'git pack-refs' [--all] [--no-prune] [--auto] [--include <pattern>] [--exclude <pattern>]
>> +                [--required]
>
> Hm, I'm not a huge fan of that name. "--dry-run" might be a better name,
> but on the other hand one might expect that such a command also yields
> information about what happens or that the compaction should be
> successful.

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Perhaps "--check-" followed by a word specific to what we are trying
>> to achieve (e.g., if we are trying to see if auto-compaction is
>> necessary, "--check-for-auto" "check for auto compaction")?  I
>> dunno.
>
> After reading what you did in the previou step, I am reasonably sure
> "required" is a wrong word to use, with or without other words like
> "check".  Semantically it is similar to the should_pack_refs() check
> that we use for pack-refs even before "optimize" came.  We expect it
> to answer this question cheaply: are we better off if we repacked,
> or can we go on without repacking for now?  It is not about "are we
> performing so poorly that we MUST optimize now?"

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> Perhaps "--check-" followed by a word specific to what we are trying
>>> to achieve (e.g., if we are trying to see if auto-compaction is
>>> necessary, "--check-for-auto" "check for auto compaction")?  I
>>> dunno.
>>
>> After reading what you did in the previou step, I am reasonably sure
>> "required" is a wrong word to use, with or without other words like
>> "check".  Semantically it is similar to the should_pack_refs() check
>> that we use for pack-refs even before "optimize" came.  We expect it
>> to answer this question cheaply: are we better off if we repacked,
>> or can we go on without repacking for now?  It is not about "are we
>> performing so poorly that we MUST optimize now?"
>
> I agree '--required' isn't the best name, and like we discussed
> '--dry-run' wouldn't be either since that would imply that the work is
> being done but not persisted.
>
> I was leaning towards '--check', which is simple. But It might be nicer
> to be verbose here and simply add something like '--is-worthwhile'.
>
> Being verbose here is okay, since it will only be used sparingly and
> specifically by those who require such a use case.

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> Perhaps "--check-" followed by a word specific to what we are trying
>>> to achieve (e.g., if we are trying to see if auto-compaction is
>>> necessary, "--check-for-auto" "check for auto compaction")?  I
>>> dunno.
>>
>> After reading what you did in the previou step, I am reasonably sure
>> "required" is a wrong word to use, with or without other words like
>> "check".  Semantically it is similar to the should_pack_refs() check
>> that we use for pack-refs even before "optimize" came.  We expect it
>> to answer this question cheaply: are we better off if we repacked, or
>> can we go on without repacking for now?  It is not about "are we
>> performing so poorly that we MUST optimize now?"
>
> I agree '--required' isn't the best name, and like we discussed
> '--dry-run' wouldn't be either since that would imply that the work is
> being done but not persisted.
>
> I was leaning towards '--check', which is simple. But It might be
> nicer to be verbose here and simply add something like '--is-worthwhile'.
>
> Being verbose here is okay, since it will only be used sparingly and
> specifically by those who require such a use case.

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> Junio C Hamano <gitster@pobox.com> writes:
>>>
>>>> Perhaps "--check-" followed by a word specific to what we are trying
>>>> to achieve (e.g., if we are trying to see if auto-compaction is
>>>> necessary, "--check-for-auto" "check for auto compaction")?  I
>>>> dunno.
>>>
>>> After reading what you did in the previou step, I am reasonably sure
>>> "required" is a wrong word to use, with or without other words like
>>> "check".  Semantically it is similar to the should_pack_refs() check
>>> that we use for pack-refs even before "optimize" came.  We expect it
>>> to answer this question cheaply: are we better off if we repacked,
>>> or can we go on without repacking for now?  It is not about "are we
>>> performing so poorly that we MUST optimize now?"
>>
>> I agree '--required' isn't the best name, and like we discussed
>> '--dry-run' wouldn't be either since that would imply that the work is
>> being done but not persisted.
>>
>> I was leaning towards '--check', which is simple. But It might be nicer
>> to be verbose here and simply add something like '--is-worthwhile'.
>>
>> Being verbose here is okay, since it will only be used sparingly and
>> specifically by those who require such a use case.
>
> Nah, "worthwhile" is relative and it would be less meaningful
> without expressing for what goal we are judging how it is worthwhile
> to do.
>

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Karthik Nayak <karthik.188@gmail.com> writes:
>>
>>> Junio C Hamano <gitster@pobox.com> writes:
>>>
>>>> Junio C Hamano <gitster@pobox.com> writes:
>>>>
>>>>> Perhaps "--check-" followed by a word specific to what we are trying
>>>>> to achieve (e.g., if we are trying to see if auto-compaction is
>>>>> necessary, "--check-for-auto" "check for auto compaction")?  I
>>>>> dunno.
>>>>
>>>> After reading what you did in the previou step, I am reasonably sure
>>>> "required" is a wrong word to use, with or without other words like
>>>> "check".  Semantically it is similar to the should_pack_refs() check
>>>> that we use for pack-refs even before "optimize" came.  We expect it
>>>> to answer this question cheaply: are we better off if we repacked,
>>>> or can we go on without repacking for now?  It is not about "are we
>>>> performing so poorly that we MUST optimize now?"
>>>
>>> I agree '--required' isn't the best name, and like we discussed
>>> '--dry-run' wouldn't be either since that would imply that the work is
>>> being done but not persisted.
>>>
>>> I was leaning towards '--check', which is simple. But It might be nicer
>>> to be verbose here and simply add something like '--is-worthwhile'.
>>>
>>> Being verbose here is okay, since it will only be used sparingly and
>>> specifically by those who require such a use case.
>>
>> Nah, "worthwhile" is relative and it would be less meaningful
>> without expressing for what goal we are judging how it is worthwhile
>> to do.
>>
>
> I see what you mean.
>
>> Choosing a phrase around "check" is better, I would think.
>
> How about simply `--check`. Since the flag can be used with other
> existing flags, it would make more sense to do
>
>   git refs optimize --all --auto --check
>   git refs optimize --check
>   git refs optimize --all --check
>
> I will send in a new version around this and we can discuss further on
> top of that!

> Karthik Nayak <karthik.188@gmail.com> writes:
> 
> > Junio C Hamano <gitster@pobox.com> writes:
> >
> >> Karthik Nayak <karthik.188@gmail.com> writes:
> >>
> >>> Junio C Hamano <gitster@pobox.com> writes:
> >>>
> >>>> Junio C Hamano <gitster@pobox.com> writes:
> >>>>
> >>>>> Perhaps "--check-" followed by a word specific to what we are trying
> >>>>> to achieve (e.g., if we are trying to see if auto-compaction is
> >>>>> necessary, "--check-for-auto" "check for auto compaction")?  I
> >>>>> dunno.
> >>>>
> >>>> After reading what you did in the previou step, I am reasonably sure
> >>>> "required" is a wrong word to use, with or without other words like
> >>>> "check".  Semantically it is similar to the should_pack_refs() check
> >>>> that we use for pack-refs even before "optimize" came.  We expect it
> >>>> to answer this question cheaply: are we better off if we repacked,
> >>>> or can we go on without repacking for now?  It is not about "are we
> >>>> performing so poorly that we MUST optimize now?"
> >>>
> >>> I agree '--required' isn't the best name, and like we discussed
> >>> '--dry-run' wouldn't be either since that would imply that the work is
> >>> being done but not persisted.
> >>>
> >>> I was leaning towards '--check', which is simple. But It might be nicer
> >>> to be verbose here and simply add something like '--is-worthwhile'.
> >>>
> >>> Being verbose here is okay, since it will only be used sparingly and
> >>> specifically by those who require such a use case.
> >>
> >> Nah, "worthwhile" is relative and it would be less meaningful
> >> without expressing for what goal we are judging how it is worthwhile
> >> to do.
> >>
> >
> > I see what you mean.
> >
> >> Choosing a phrase around "check" is better, I would think.
> >
> > How about simply `--check`. Since the flag can be used with other
> > existing flags, it would make more sense to do
> >
> >   git refs optimize --all --auto --check
> >   git refs optimize --check
> >   git refs optimize --all --check
> >
> > I will send in a new version around this and we can discuss further on
> > top of that!
> 
> After speaking to Patrick today about this and his work on 'git
> maintenance', we realized we can actually broaden the scope. We could
> implement something like 'git maintenance needed'. This would check on a
> whole if repository maintenance is needed for refs and objects.

> So if we build on top of that mechanism we can easily broaden the scope
> of this patch series to:
>
>   - Check whether maintenance is needed for any of the enabled tasks.
>
>   - Check whether maintenance is needed for a single task.
>
>   - Check whether maintenance is needed given a specific maintenance
>     strategy.
>
> Another benefit is that it's a bit easier to put a verb into a proper
> subcommand of git-maintenance(1) instead of having to add a verb-ish
> flag to git-pack-refs(1). `git maintenance is-needed` feels way more
> natural to me at least compared to `git refs optimize --check`.