Re: [PATCH v2 2/6] commit-graph: always parse before commit_graph_data_at()

[Junio C Hamano <gitster@pobox.com>]

Taylor Blau <me@ttaylorr.com> writes:

> Thinking aloud, I'm not totally sure that we should be exposing "git
> commit-graph clear" to users. The only time that you'd want to run this
> is if you were trying to remove a corrupted commit-graph, so I'd rather
> see guidance on how to do that safely show up in
> Documentation/git-commit-graph.txt.
>
> On the other hand, now I'm encouraging running "rm -fr
> $GIT_DIR/objects/info/commit-graph*", which feels dangerous.

True.

As this is, like pack .idx file, supposed to be "precomputed cached
data that can be fully recreated using primary information" [*], I
am perfectly fine to say "commit-graph may have unexplored corners,
and when you hit a BUG(), you can safely use 'commit-graph clear'
and recreate it from scratch, or operate without it if you feel you
do not yet want to trust your data to it for now."  Giving safer and
easier way to opt out for those who need to get today's release
done, with enough performance incentive to re-enable it when the
crunch is over, would be an honest thing to do, I would think.

	Side note: the index file also used to be considered to hold
	such cached data, that can be recreated from the working
	tree data and the tip commit.  We no longer treat it that
	way, though.

> Somewhere in the middle would be something like:
>
>   git -c core.commitGraph=false commit-graph write --reachable

I am a bit worried about the thinking along this line, because it
gives the users an impression that there is no escaping from
trusting commit-graph---the one that was created from scratch is
bug-free and they only need to be cautious about incrementals.

But (1) we do not know that, and (2) it is an unconvincing message
to somebody who just got hit by a BUG().

> which would disable reading existing commit-graph files. Since
> 85102ac71b (commit-graph: don't write commit-graph when disabled,
> 2020-10-09), that causes us to exit immediately.

Meaning the three command sequence

	git commit-graph clear
	git commit-graph write --reachable
        git config core.commitGraph false

to force a clean build of a graph and forbid further updates until
the bug is squashed???  But should't core.commitGraph forbid reading
and using the data in the existing files, too?  In which case, shouldn't
it be equivalent to "git commit-graph clear"?

> I think that reverting that patch and advertising setting
> 'core.commitGraph=false' in the documentation makes the most sense.