Re: [PATCH v2 2/6] commit-graph: always parse before commit_graph_data_at()
From: Junio C Hamano <hidden>
Date: 2021-02-03 18:42:06
Taylor Blau [off-list ref] 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.