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

[PATCH 6/6] commit-graph.c: avoid duplicated progress output during `verify`

From
Taylor Blau <me@ttaylorr.com>
Date
Jul 8, 2023, 00:31 UTC
Message-ID
<6eec849fa5aa734670bfcba359ad470efbaf383c.1688776280.git.me@ttaylorr.com>
In-Reply-To
<cover.1688776280.git.me@ttaylorr.com>

When `git commit-graph verify` was taught how to verify commit-graph chains in 3da4b609bb1 (commit-graph: verify chains with --shallow mode, 2019-06-18), it produced one line of progress per layer of the commit-graph chain.

    $ git.compile commit-graph verify
    Verifying commits in commit graph: 100% (4356/4356), done.
    Verifying commits in commit graph: 100% (131912/131912), done.

This could be somewhat confusing to users, who may wonder why there are multiple occurrences of "Verifying commits in commit graph".

There are likely good arguments on whether or not there should be one line of progress output per commit-graph layer. On the one hand, the existing output shows us verifying each individual layer of the chain. But on the other hand, the fact that a commit-graph may be stored among multiple layers is an implementation detail that the caller need not be aware of.

Clarify this by showing a single progress meter regardless of the number of layers in the commit-graph chain. After this patch, the output reflects the logical contents of a commit-graph chain, instead of showing one line of output per commit-graph layer:

    $ git.compile commit-graph verify
    Verifying commits in commit graph: 100% (136268/136268), done.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 commit-graph.c                | 27 +++++++++++++++++----------
 t/t5324-split-commit-graph.sh |  3 ++-
 2 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/commit-graph.c b/commit-graph.c
index 65dd4edf0e1..ffcf548223f 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -2544,7 +2544,8 @@ static int commit_graph_checksum_valid(struct commit_graph *g)
 
 static int verify_one_commit_graph(struct repository *r,
 				   struct commit_graph *g,
-				   struct progress *progress)
+				   struct progress *progress,
+				   uint64_t *seen)
 {
 	uint32_t i, cur_fanout_pos = 0;
 	struct object_id prev_oid, cur_oid;
@@ -2605,7 +2606,7 @@ static int verify_one_commit_graph(struct repository *r,
 		timestamp_t max_generation = 0;
 		timestamp_t generation;
 
-		display_progress(progress, i + 1);
+		display_progress(progress, ++(*seen));
 		oidread(&cur_oid, g->chunk_oid_lookup + g->hash_len * i);
 
 		graph_commit = lookup_commit(r, &cur_oid);
@@ -2694,26 +2695,32 @@ static int verify_one_commit_graph(struct repository *r,
 
 int verify_commit_graph(struct repository *r, struct commit_graph *g, int flags)
 {
+	struct progress *progress = NULL;
 	int local_error = 0;
+	uint64_t seen = 0;
 
 	if (!g) {
 		graph_report("no commit-graph file loaded");
 		return 1;
 	}
 
+	if (flags & COMMIT_GRAPH_WRITE_PROGRESS) {
+		uint64_t total = g->num_commits;
+		if (!(flags & COMMIT_GRAPH_VERIFY_SHALLOW))
+			total += g->num_commits_in_base;
+
+		progress = start_progress(_("Verifying commits in commit graph"),
+					  total);
+	}
+
 	for (; g; g = g->base_graph) {
-		struct progress *progress = NULL;
-		if (flags & COMMIT_GRAPH_WRITE_PROGRESS)
-			progress = start_progress(_("Verifying commits in commit graph"),
-						g->num_commits);
-
-		local_error |= verify_one_commit_graph(r, g, progress);
+		local_error |= verify_one_commit_graph(r, g, progress, &seen);
 		if (flags & COMMIT_GRAPH_VERIFY_SHALLOW)
 			break;
-
-		stop_progress(&progress);
 	}
 
+	stop_progress(&progress);
+
 	return local_error;
 }
 
diff --git a/t/t5324-split-commit-graph.sh b/t/t5324-split-commit-graph.sh
index 669ddc645fa..36c4141e67b 100755
--- a/t/t5324-split-commit-graph.sh
+++ b/t/t5324-split-commit-graph.sh
@@ -351,7 +351,8 @@ test_expect_success 'add octopus merge' '
 	git branch merge/octopus &&
 	git commit-graph write --reachable --split &&
 	git commit-graph verify --progress 2>err &&
-	test_line_count = 3 err &&
+	test_line_count = 1 err &&
+	grep "Verifying commits in commit graph: 100% (18/18)" err &&
 	test_i18ngrep ! warning err &&
 	test_line_count = 3 $graphdir/commit-graph-chain
 '
-- 
2.41.0.242.g6eec849fa5a
Previous: Taylor BlauNext: Derrick Stolee
Message 7 of 8 in “fsck: squelch progress output with `--no-progress`”
  1. 0/6 fsck: squelch progress output with `--no-progress`Taylor Blau, Jul 8, 2023
  2. 1/6 fsck: suppress commit-graph output with `--no-progress`Taylor Blau, Jul 8, 2023
  3. 2/6 fsck: suppress MIDX output with `--no-progress`Taylor Blau, Jul 8, 2023
  4. 3/6 commit-graph.c: extract `verify_one_commit_graph()`Taylor Blau, Jul 8, 2023
  5. 4/6 commit-graph.c: iteratively verify commit-graph chainsTaylor Blau, Jul 8, 2023
  6. 5/6 commit-graph.c: pass progress to `verify_one_commit_graph()`Taylor Blau, Jul 8, 2023
  7. 6/6 commit-graph.c: avoid duplicated progress output during `verify`Taylor Blau, Jul 8, 2023
  8. Derrick StoleeJul 10, 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.