Re: [PATCH v2] builtin/pack-objects.c: introduce `pack.extraCruftTips`

[Taylor Blau <me@ttaylorr.com>]

On Wed, May 03, 2023 at 03:59:06PM -0400, Jeff King wrote:
> On Tue, May 02, 2023 at 08:09:47PM -0400, Taylor Blau wrote:
>
> > However, there is no option to be able to keep around certain objects
> > that have otherwise aged out of the grace period. The only way to retain
> > those objects is:
> >
> >   - to point a reference at them, which may be undesirable or
> >     infeasible,
> >   - to track them via the reflog, which may be undesirable since the
> >     reflog's lifetime is limited to that of the reference it's tracking
> >     (and callers may want to keep those unreachable objects around for
> >     longer)
> >   - to extend the grace period, which may keep around other objects that
> >     the caller *does* want to discard,
> >   - or, to force the caller to construct the pack of objects they want
> >     to keep themselves, and then mark the pack as kept by adding a
> >     ".keep" file.
>
> OK, I understand the use case you're trying to support, and your
> approach mostly makes sense. But there are two things I was surprised by
> in the implementation:
>
>   1. Does this need to be tied to cruft packs? The same logic would
>      apply to "repack -A" which turns objects loose (of course you
>      probably don't want to do that in the long term for performance
>      reasons, but it's conceptually the same thing; see below).

I agree that you wouldn't want to do it for performance reasons, but I'm
comfortable with the asymmetry here, since this is `pack.extraCruftTips`
(emphasis on "cruft"), so it's not clear that it has to be related to
"repack -A".

Happy to change things up if you feel strongly, though.

Thanks,
Taylor