Re: [PATCH v2 0/5] Create commit-graph file format v2

From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, sandals@crustytoothpaste.net,
	Junio C Hamano <gitster@pobox.com>, Jeff King <peff@peff.net>
Subject: Re: [PATCH v2 0/5] Create commit-graph file format v2
Date: Fri, 26 Apr 2019 00:09:15 +0200	[thread overview]
Message-ID: <87a7gdspo4.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <pull.112.v2.git.gitgitgadget@gmail.com>

On Wed, Apr 24 2019, Derrick Stolee via GitGitGadget wrote:

> NOTE: this series was rebased onto ab/commit-graph-fixes, as the conflicts
> were significant and subtle.

Sorry, hopefully it helped more than it harmed :)

A few unrelated things:

1)

First, before that series of mine applying this and writing a v2 file
would make most things (e.g. "status") hard error on e.g. v2.21.0:

    $ git status
    error: graph version 2 does not match version 1
    $

Now as noted in my series we now on 'master' downgrade that to a warning
(along with the rest of the errors):

    $ ~/g/git/git --exec-path=$PWD status
    error: commit-graph version 2 does not match version 1
    On branch master
    [...]

...and this series sets the default version for all new graphs to v2.

I think this is *way* too aggressive of an upgrade path. If these
patches go into v2.22.0 then git clients on all older versions that grok
the commit graph (IIRC v2.18 and above) will have their git completely
broken if they're in a mixed-git-version environment.

Is it really so important to move to v2 right away that we need to risk
those breakages? I think even with my ab/commit-graph-fixes it's still
too annoying (I was mostly trying to fix other stuff...). If only we
could detect "we should make a new graph now" ....

2)

...speaking of which, digging up outstanding stuff I have on the
commit-graph I was reminded to finish up my "commit graph on clone"
patch in:
https://public-inbox.org/git/87in2hgzin.fsf@evledraar.gmail.com/

And re #1 above: I guess we could also do that "let's make a graph" and
call "gc --auto" if a) we have gc.writeCommitGraph b) we see it's not
the "right" version. As long as older versions always write a "old" one
if they can't grok the "new" one, and newer versions leave existing
graphs alone even if they're older versions, so we don't flip-flop.

One of the things that would make that "graph on clone/fetch/whatever"
easier is having the graph store the total number of objects while it
was at it, you indicated in
https://public-inbox.org/git/934fa00e-f6df-c333-4968-3e9acffab22d@gmail.com/
that you already have an internal MSFT implementation of it that does
it.

Any reason not to make it part of v2 while we're at it? We already find
out how many (packed) objects we have in "add_packed_commits", we just
don't do anything with that information now.

3)

Also (but mostly unrelated). I see that "Future Work" in
Documentation/technical/commit-graph.txt now appears to entirely
describe "Past Work" instead :)

4)

The third point in "Design Details" also says that the format doesn't
need a change for a future hash algo change, yet here we are at v2
making a (small) change for that purpose :)