Re: [PATCH 00/15] refs: introduce `--auto` to pack refs as needed

From: Patrick Steinhardt <ps@pks.im>
Date: Mar 25, 2024, 09:10 UTC
Message-ID: <ZgE_fJpufp463Bp7@tanuki>
In-Reply-To: <CAOLa=ZTgJj=moQC2Ve40KzSinmNGFwTSx_RcKaiCEQArcg8NoQ@mail.gmail.com>

On Wed, Mar 20, 2024 at 12:30:58PM -0700, Karthik Nayak wrote:

Show 7 quoted lines

> Patrick Steinhardt <ps@pks.im> writes:
> > The `PACK_REFS_AUTO` flag is exposed via a new `git pack-refs --auto`
> > flag. It is wired up in both `git gc --auto` and `git maintenance run
> > --auto`.
> >
> 
> Wondering if this is also exposed as a config option.

It's not, no, and I don't quite think it would make sense. We don't expose the current `git gc --auto` flag as config option, either. While we have "gc.auto", it doesn't configure whether or not the `--auto` flag is set, but rather configures the boundary at which objects would be repacked.

In hindsight that config option is too narrowly focussed on objects while pretending to be quite general. It would've been great if this was called "gc.autoLooseObjectLimit" or something similar instead to not cause confusion and to be more easily extendable in the future.

I can certainly see that we might eventually want to have ref backend specific configuration here. For the "files" backend this could be be the number of loose refs that are allowed to exist before repacking, whereas for the "reftable" backend this could be the geometric factor. But I'll leave that for a future series.

Show 15 quoted lines

> > The series is structured as follows:
> >
> >     - Patches 1 - 5: Bugfixes and improvements for the reftable-specific
> >       compaction code. These are issues that I've found while working on
> >       this series.
> >
> >     - Patches 6 - 8: Refactorings to drop the `PACK_REFS_ALL` flag,
> >       which isn't actually used by the ref backends anymore and confused
> >       me multiple times.
> >
> >     - Patches 9 - 15: Handling of `PACK_REFS_ALL` in git-pack-refs(1),
> >       git-gc(1) and git-maintenance(1).
> >
> 
> I'm assuming this means `PACK_REFS_AUTO` in the last sentence.

Ah, yes.

Patrick