Re: [PATCH v3 01/17] Documentation/technical: add cruft-packs.txt

[Taylor Blau <me@ttaylorr.com>]

On Tue, Mar 22, 2022 at 09:01:43PM -0400, Taylor Blau wrote:
> > >> Can you tell me a little more about why we would want _not_ to have a
> > >> repository format extension?  To me, it seems like a fairly simple
> > >> addition that would drastically reduce the cognitive overload for
> > >> people considering making use of this feature.
> > >
> > >There is no reason to prevent a pre-cruft packs version of Git from
> > > reading/writing a repository that uses cruft packs, since the two
> > > versions will still function as normal. Since there's no need to prevent
> > > the old version from interacting with a repository that has cruft packs,
> > > we wouldn't want to enforce an unnecessary boundary with an extension.
> >
> > Does "function as normal" include in repository maintenance operations
> > like "git maintenance", "git gc", and "git prune"?  If so, this seems
> > like something very useful to describe in the cruft-packs.txt
> > document, since what happens when we bounce back and forth between old
> > and new versions of Git operating on the same NFS mounted repository
> > would not be obvious without such a discussion.
>
> Yes, all of those commands will simply ignore the .mtimes file and treat
> the unreachable objects as normal (where "normal" means in the exact
> same way as they currently do without cruft packs). I think adding a
> section that summarizes our discussion would be useful.
>
> > I'm still interested in the _downsides_ of using a repository format
> > extension.  "There is no reason" is not a downside, unless you mean
> > that it requires adding a line of code. :)  The main downside I can
> > imagine is that it prevents accessing the repository _that has enabled
> > this feature_ with an older version of Git, but I (perhaps due to a
> > failure of imagination) haven't put two and two together yet about
> > when I would want to do so.
>
> Sorry for not being clear; I meant: "There is no reason [to prohibit
> two versions of Git from interacting with each other when they are
> compatible to do so]".

Jonathan, myself, and others discussed this extensively in today's
standup.

To summarize Jonathan's point (as I think I severely misunderstood it
before), if two writers are repacking a repository with unreachable
objects. The following can happen:

  - $NEWGIT packs the repository and writes a cruft pack and .mtimes
    file.

  - $OLDGIT packs the repository, exploding unreachable objects from the
    cruft pack as loose, setting their mtimes to "now".

This causes the repository to lose information about the unreachable
mtimes, which would cause the repository to never prune objects (except
for when`--unpack-unreachable=now` is passed).

One approach (that Jonathan suggested) is to prevent the above situation
by introducing a format extension, so $OLDGIT could not touch the
repository. But this comes at a (in my view, significant) cost which is
that $OLDGIT can't touch the repository _at all_. An extension would be
desirable if cross-version interaction resulted in repository
corruption, but this scenario does not lead to corruption at all.

Another approach (courtesy Stolee, in an off-list discussion) is that we
could introduce an optional extension available as an opt-in to prevent
older versions of Git from interacting in a repository that contains
cruft packs, but is not required to write them.

A third approach (and probably my preferred direction) is to indicate
clearly via a combination of updates to Documentation/cruft-packs.txt
and the release notes that say something along the lines of:

    If you use are repacking a repository using both a pre- and
    post-cruft packs version of Git, please be aware that you will lose
    information about the mtimes of unreachable objects.

I imagine that would probably be sufficient, but we could also introduce
the opt-in extension as an easy alternative to avoid forcing an upgrade
of Git.

Thanks,
Taylor