Re: [PATCH 15/15] builtin/gc: pack refs when using `git maintenance run --auto`

[Patrick Steinhardt <ps@pks.im>]

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

On Wed, Mar 20, 2024 at 04:59:06PM -0700, Karthik Nayak wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > When running `git maintenance run --auto`, then the various subtasks
> > will only run as needed. Thus, we for example end up only packing loose
> > objects if we hit a certain threshold.
> >
> > Interestingly enough, the "pack-refs" task is actually _never_ executed
> > when the auto-flag is set because it does not have a condition at all.
> > As 41abfe15d9 (maintenance: add pack-refs task, 2021-02-09) mentions:
> >
> >     The 'auto_condition' function pointer is left NULL for now. We could
> >     extend this in the future to have a condition check if pack-refs
> >     should be run during 'git maintenance run --auto'.
> >
> 
> Ok, this answers my question in the previous email.
> 
> > It is not quite clear from that quote whether it is actually intended
> > that the task doesn't run at all in this mode. Also, no test was added
> > to verify this behaviour. Ultimately though, it feels quite surprising
> > that `git maintenance run --auto --task=pack-refs` would quietly never
> > do anything at all.
> >
> > In any case, now that we do have the logic in place to let ref backends
> > decide whether or not to repack refs, it does make sense to wire it up
> > accordingly. With the "reftable" backend we will thus now perform
> > auto-compaction, which optimizes the refdb as needed.
> >
> > But for the "files" backend we now unconditionally pack refs as it does
> > not yet know to handle the "auto" flag. Arguably, this can be seen as a
> > bug fix given that previously the task never did anything at all.
> > Eventually though we should amend the "files" backend to use some
> > heuristics for auto compaction, as well.
> >
> 
> Agreed, thanks for the clear explanation here.

Thanks for your thorough review!

Patrick

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