git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Junio C Hamano <gitster@pobox.com>
Date
Nov 1, 2023, 02:06 UTC
Message-ID
<xmqq34xqxm5u.fsf@gitster.g>
In-Reply-To
<xmqqh6m6z6pe.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
Show 7 quoted lines
>> Thanks, the range-diff here looks exactly as expected. Thanks for
>> working on this, this version LGTM.
>
> OK, I'd like a version as incremental to v2 (since it already is in
> 'next') that results in the same tree state as v3 then.
>
> Thanks for working on it, and reviewing it.

In the meantime, here is a mechanically produced incremental I'll tentatively queue. Hopefully I did not screw up while generating it.

Thanks.
--- >8 ---
From: Patrick Steinhardt <ps@pks.im>
Date: Tue, 31 Oct 2023 08:16:09 +0100
Subject: [PATCH] commit-graph: clarify GIT_COMMIT_GRAPH_PARANOIA documentation

In response to reviews of the previous round that has already hit 'next', clarify the help text for GIT_COMMIT_GRAPH_PARANOIA and rename object_paranoia variable to commit_graph_paranoia for consistency.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git.txt   | 15 ++++++++-------
 commit-graph.c          |  8 ++++----
 commit.c                |  8 ++++----
 t/t5318-commit-graph.sh |  2 +-
 4 files changed, 17 insertions(+), 16 deletions(-)
diff --git a/Documentation/git.txt b/Documentation/git.txt
index 22c2b537aa..3bac24cf8a 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -912,13 +912,14 @@ 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
diff --git a/commit-graph.c b/commit-graph.c
index 376f59af73..b37fdcb214 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -907,18 +907,18 @@ int repo_find_commit_pos_in_graph(struct repository *r, struct commit *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 (object_paranoia && !has_object(repo, id, 0))
+	if (commit_graph_paranoia && !has_object(repo, id, 0))
 		return NULL;
 
 	commit = lookup_commit(repo, id);
diff --git a/commit.c b/commit.c
index 7399e90212..8405d7c3fc 100644
--- a/commit.c
+++ b/commit.c
@@ -574,12 +574,12 @@ int repo_parse_commit_internal(struct repository *r,
 	if (item->object.parsed)
 		return 0;
 	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"),
diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
index 55e3c7ec78..2c62b91ef9 100755
--- a/t/t5318-commit-graph.sh
+++ b/t/t5318-commit-graph.sh
@@ -847,7 +847,7 @@ test_expect_success 'stale commit cannot be parsed when traversing graph' '
 		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.

-- 
2.42.0-530-g692be87cbb
Previous: Junio C HamanoNext: Patrick Steinhardt
Message 15 of 18 in “commit-graph: detect commits missing in ODB”
  1. 0/2 commit-graph: detect commits missing in ODBPatrick Steinhardt, Oct 23, 2023
  2. 1/2 commit-graph: introduce envvar to disable commit existence checksPatrick Steinhardt, Oct 23, 2023
  3. Taylor BlauOct 30, 2023
  4. Patrick SteinhardtOct 31, 2023
  5. 2/2 commit: detect commits that exist in commit-graph but not in the ODBPatrick Steinhardt, Oct 23, 2023
  6. Junio C HamanoOct 24, 2023
  7. Taylor BlauOct 30, 2023
  8. Junio C HamanoOct 31, 2023
  9. Taylor BlauOct 30, 2023
  10. 0/2 commit-graph: detect commits missing in ODBPatrick Steinhardt, Oct 31, 2023
  11. 1/2 commit-graph: introduce envvar to disable commit existence checksPatrick Steinhardt, Oct 31, 2023
  12. 2/2 commit: detect commits that exist in commit-graph but not in the ODBPatrick Steinhardt, Oct 31, 2023
  13. Taylor BlauOct 31, 2023
  14. Junio C HamanoOct 31, 2023
  15. Junio C HamanoNov 1, 2023
  16. Patrick SteinhardtNov 1, 2023
  17. Junio C HamanoNov 1, 2023
  18. Junio C HamanoOct 31, 2023

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.