Re: tb/cruft-packs (was Re: What's cooking in git.git (Mar 2022, #01; Thu, 3))

[Taylor Blau <me@ttaylorr.com>]

On Mon, Mar 07, 2022 at 10:06:00AM -0800, Jonathan Nieder wrote:
> I'm excited about this work!  I just sent a quick review to the
> design doc.

Thanks! I haven't had a chance to look at the design doc, but let me
respond quickly to this message:

> Before merging to 'next', I'd be interested in two things:
>
>  1. Marking the feature as experimental so we can learn from experience.
>     Clarifying what aspects we consider to be stable / set in stone and
>     what are subject to modification.

I'm not sure there is much practical benefit to marking this feature as
experimental. The only new file format here is the .mtimes one, which
should make it easy for us to modify the format in a
backwards-compatible way.

If there are other benefits you had in mind, I'm curious to hear them.
But I think we should be fine to "lock in" the first version of the
.mtimes format since we have an easy-ish mechanism to change it in the
future.

>  2. Marking this as a repository format extension so it doesn't interact
>     poorly with Git implementations (including older versions of Git
>     itself) that are not aware of the new feature

The design of cruft packs was done intentionally to avoid needing a
format extension. The cruft pack is "just a pack" to any older version
of Git. The only thing an older version of Git wouldn't understand is
how to interpret the .mtimes file. But that's no different than the
current behavior without cruft packs, where any unreachable object
inherits the mtime of its containing pack.

So an older version of Git might prune a different set of objects than a
version that understands cruft packs depending on the contents of the
.mtimes file, the mtime of the cruft pack, and the width of the grace
period. But I think by downgrading you are more or less buying into the
existing behavior. So I don't think there is a compelling reason to
introduce a format extension here.

Thanks,
Taylor