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

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 3, 2021, 18:41 UTC
Message-ID: <xmqqk0rpc7uj.fsf@gitster.c.googlers.com>
In-Reply-To: <YBrCli7AR/XrB3Pr@nand.local>

Taylor Blau <me@ttaylorr.com> writes:

Show 8 quoted lines

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