Re: [PATCH v2] builtin/pack-objects.c: introduce `pack.extraCruftTips`
- From
Taylor Blau <me@ttaylorr.com>
- Date
- May 3, 2023, 21:28 UTC
- Message-ID
- <ZFLSDS89y8P2+g/2@nand.local>
- In-Reply-To
- <20230503195906.GA1796816@coredump.intra.peff.net>
On Wed, May 03, 2023 at 03:59:06PM -0400, Jeff King wrote:
Show 26 quoted lines
> 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