Re: [PATCH v3 4/5] maintenance: add checking logic in `pack_refs_condition()`
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Nov 6, 2025, 11:58 UTC
- Message-ID
- <aQyNSOdPWAxm15U3@pks.im>
- In-Reply-To
- <20251106-562-add-sub-command-to-check-if-maintenance-is-needed-v3-4-d611a2a95cf5@gmail.com>
On Thu, Nov 06, 2025 at 09:22:33AM +0100, Karthik Nayak wrote:
Show 34 quoted lines
> 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