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

[Jeff King <peff@peff.net>]

On Wed, May 03, 2023 at 05:28:45PM -0400, Taylor Blau wrote:

> > 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.

I don't feel strongly. I certainly have no intent to run "git repack -A"
with extra tips specified. Mostly I just thought that it would be
simpler to just apply it everywhere, both conceptually to the user and
within the implementation.

But since the cruft-pack implementation diverges quite a bit from the
regular "-A" handling, I guess it makes the code more complex rather
than less. The asymmetry feels like a wart to me, but I guess in the
long run we'd hope that the "turn unreachable loose" strategy is
deprecated anyway.

-Peff