[PATCH v3 0/2] commit-graph: detect commits missing in ODB

Hi,

this is version 3 of my patch series to more readily detect commits parsed from the commit graph which are missing in the object database.

Changes compared to v2:

    - Rewrote the help text for `GIT_COMMIT_GRAPH_PARANOIA` to be more
      accessible.

    - Renamed the `object_paranoia` variable to `commit_graph_paranoia`.

    - Fixed a typo.

Thanks!

Patrick

Patrick Steinhardt (2):
  commit-graph: introduce envvar to disable commit existence checks
  commit: detect commits that exist in commit-graph but not in the ODB

 Documentation/git.txt   | 10 +++++++++
 commit-graph.c          |  6 +++++-
 commit-graph.h          |  6 ++++++
 commit.c                | 16 +++++++++++++-
 t/t5318-commit-graph.sh | 48 +++++++++++++++++++++++++++++++++++++++++
 5 files changed, 84 insertions(+), 2 deletions(-)

Range-diff against v2:
1:  a89c435528 ! 1:  c433ec1254 commit-graph: introduce envvar to disable commit existence checks
    @@ Documentation/git.txt: for full details.
      	useful when trying to salvage data from a corrupted repository.
      
     +`GIT_COMMIT_GRAPH_PARANOIA`::
    -+	If this Boolean environment variable is set to false, ignore the
    -+	case where commits exist in the commit graph but not in the
    -+	object database. Normally, Git will check whether commits loaded
    -+	from the commit graph exist in the object database to avoid
    -+	issues with stale commit graphs, but this check comes with a
    -+	performance penalty. The default is `1` (i.e., be paranoid about
    -+	stale commits in the commit graph).
    ++	When loading a commit object from the commit-graph, Git performs an
    ++	existence check on the object in the object database. This is done to
    ++	avoid issues with stale commit-graphs that contain references to
    ++	already-deleted commits, but comes with a performance penalty.
    +++
    ++The default is "true", which enables the aforementioned behavior.
    ++Setting this to "false" disables the existence check. This can lead to
    ++a performance improvement at the cost of consistency.
     +
      `GIT_ALLOW_PROTOCOL`::
      	If set to a colon-separated list of protocols, behave as if
    @@ commit-graph.c: int repo_find_commit_pos_in_graph(struct repository *r, struct c
      
      struct commit *lookup_commit_in_graph(struct repository *repo, const struct object_id *id)
      {
    -+	static int object_paranoia = -1;
    ++	static int commit_graph_paranoia = -1;
      	struct commit *commit;
      	uint32_t pos;
      
    -+	if (object_paranoia == -1)
    -+		object_paranoia = git_env_bool(GIT_COMMIT_GRAPH_PARANOIA, 1);
    ++	if (commit_graph_paranoia == -1)
    ++		commit_graph_paranoia = git_env_bool(GIT_COMMIT_GRAPH_PARANOIA, 1);
     +
      	if (!prepare_commit_graph(repo))
      		return NULL;
      	if (!search_commit_pos_in_graph(id, repo->objects->commit_graph, &pos))
      		return NULL;
     -	if (!has_object(repo, id, 0))
    -+	if (object_paranoia && !has_object(repo, id, 0))
    ++	if (commit_graph_paranoia && !has_object(repo, id, 0))
      		return NULL;
      
      	commit = lookup_commit(repo, id);
2:  0476d48555 ! 2:  8629fd0892 commit: detect commits that exist in commit-graph but not in the ODB
    @@ commit.c: int repo_parse_commit_internal(struct repository *r,
      		return 0;
     -	if (use_commit_graph && parse_commit_in_graph(r, item))
     +	if (use_commit_graph && parse_commit_in_graph(r, item)) {
    -+		static int object_paranoia = -1;
    ++		static int commit_graph_paranoia = -1;
     +
    -+		if (object_paranoia == -1)
    -+			object_paranoia = git_env_bool(GIT_COMMIT_GRAPH_PARANOIA, 1);
    ++		if (commit_graph_paranoia == -1)
    ++			commit_graph_paranoia = git_env_bool(GIT_COMMIT_GRAPH_PARANOIA, 1);
     +
    -+		if (object_paranoia && !has_object(r, &item->object.oid, 0)) {
    ++		if (commit_graph_paranoia && !has_object(r, &item->object.oid, 0)) {
     +			unparse_commit(r, &item->object.oid);
     +			return quiet_on_missing ? -1 :
     +				error(_("commit %s exists in commit-graph but not in the object database"),
    @@ t/t5318-commit-graph.sh: test_expect_success 'stale commit cannot be parsed when
     +		test_commit C &&
     +		git commit-graph write --reachable &&
     +
    -+		# Corrupt the repository by deleting the intermittent commit
    ++		# Corrupt the repository by deleting the intermediate commit
     +		# object. Commands should notice that this object is absent and
     +		# thus that the repository is corrupt even if the commit graph
     +		# exists.