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

[PATCH 2/6] commit-graph: check mixed generation validation when loading chain file

From
Jeff King <peff@peff.net>
Date
Sep 26, 2023, 05:57 UTC
Message-ID
<20230926055751.GB1341418@coredump.intra.peff.net>
In-Reply-To
<20230926055452.GA1341109@coredump.intra.peff.net>

In read_commit_graph_one(), we call validate_mixed_generation_chain() after loading the graph. Even though we don't check the return value, this has the side effect of clearing the read_generation_data flag, which is important when working with mixed generation numbers.

But doing this in load_commit_graph_chain_fd_st() makes more sense:
  1. We are calling it even when we did not load a chain at all, which
     is pointless (you cannot have mixed generations in a single file).
  2. For now, all callers load the graph via read_commit_graph_one().
     But the point of factoring out the open/load in the previous commit
     was to let "commit-graph verify" call them separately. So it needs
     to trigger this function as part of the load.
     Without this patch, the mixed-generation tests in t5324 would start
     failing on "git commit-graph verify" calls, once we switch to using
     a separate open/load call there.
Signed-off-by: Jeff King <peff@peff.net>
---
 commit-graph.c | 54 +++++++++++++++++++++++++-------------------------
 1 file changed, 27 insertions(+), 27 deletions(-)
diff --git a/commit-graph.c b/commit-graph.c
index 12cdd9af8e..8b29c6de24 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -473,6 +473,31 @@ static struct commit_graph *load_commit_graph_v1(struct repository *r,
 	return g;
 }
 
+/*
+ * returns 1 if and only if all graphs in the chain have
+ * corrected commit dates stored in the generation_data chunk.
+ */
+static int validate_mixed_generation_chain(struct commit_graph *g)
+{
+	int read_generation_data = 1;
+	struct commit_graph *p = g;
+
+	while (read_generation_data && p) {
+		read_generation_data = p->read_generation_data;
+		p = p->base_graph;
+	}
+
+	if (read_generation_data)
+		return 1;
+
+	while (g) {
+		g->read_generation_data = 0;
+		g = g->base_graph;
+	}
+
+	return 0;
+}
+
 static int add_graph_to_chain(struct commit_graph *g,
 			      struct commit_graph *chain,
 			      struct object_id *oids,
@@ -581,6 +606,8 @@ struct commit_graph *load_commit_graph_chain_fd_st(struct repository *r,
 		}
 	}
 
+	validate_mixed_generation_chain(graph_chain);
+
 	free(oids);
 	fclose(fp);
 	strbuf_release(&line);
@@ -605,31 +632,6 @@ static struct commit_graph *load_commit_graph_chain(struct repository *r,
 	return g;
 }
 
-/*
- * returns 1 if and only if all graphs in the chain have
- * corrected commit dates stored in the generation_data chunk.
- */
-static int validate_mixed_generation_chain(struct commit_graph *g)
-{
-	int read_generation_data = 1;
-	struct commit_graph *p = g;
-
-	while (read_generation_data && p) {
-		read_generation_data = p->read_generation_data;
-		p = p->base_graph;
-	}
-
-	if (read_generation_data)
-		return 1;
-
-	while (g) {
-		g->read_generation_data = 0;
-		g = g->base_graph;
-	}
-
-	return 0;
-}
-
 struct commit_graph *read_commit_graph_one(struct repository *r,
 					   struct object_directory *odb)
 {
@@ -638,8 +640,6 @@ struct commit_graph *read_commit_graph_one(struct repository *r,
 	if (!g)
 		g = load_commit_graph_chain(r, odb);
 
-	validate_mixed_generation_chain(g);
-
 	return g;
 }
 
-- 
2.42.0.758.gd56856b565
Previous: Jeff KingNext: Jeff King
Message 3 of 16 in “some "commit-graph verify" fixes for chains”
  1. 0/6 some "commit-graph verify" fixes for chainsJeff King, Sep 26, 2023
  2. 1/6 commit-graph: factor out chain opening functionJeff King, Sep 26, 2023
  3. 2/6 commit-graph: check mixed generation validation when loading chain fileJeff King, Sep 26, 2023
  4. 3/6 t5324: harmonize sha1/sha256 graph chain corruptionJeff King, Sep 26, 2023
  5. 4/6 commit-graph: detect read errors when verifying graph chainJeff King, Sep 26, 2023
  6. 5/6 commit-graph: tighten chain size checkJeff King, Sep 26, 2023
  7. 6/6 commit-graph: report incomplete chains during verificationJeff King, Sep 26, 2023
  8. Jeff KingSep 28, 2023
  9. 0/6 some "commit-graph verify" fixes for chainsJeff King, Sep 28, 2023
  10. 1/6 commit-graph: factor out chain opening functionJeff King, Sep 28, 2023
  11. 2/6 commit-graph: check mixed generation validation when loading chain fileJeff King, Sep 28, 2023
  12. 3/6 t5324: harmonize sha1/sha256 graph chain corruptionJeff King, Sep 28, 2023
  13. 4/6 commit-graph: detect read errors when verifying graph chainJeff King, Sep 28, 2023
  14. 5/6 commit-graph: tighten chain size checkJeff King, Sep 28, 2023
  15. 6/6 commit-graph: report incomplete chains during verificationJeff King, Sep 28, 2023
  16. Taylor BlauOct 5, 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.