Re: [PATCH v3 4/5] maintenance: add checking logic in `pack_refs_condition()`
Patrick Steinhardt <ps@pks.im> writes:
Show 39 quoted lines
> 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