From eb319d6771f5829dc26499af93050350083adc7d Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Fri, 7 Jul 2023 20:31:36 -0400 Subject: commit-graph.c: extract `verify_one_commit_graph()` When the `verify_commit_graph()` function was extended to support commit-graph chains via 3da4b609bb1 (commit-graph: verify chains with --shallow mode, 2019-06-18), it did so by recursively calling itself on each layer of the commit-graph chain. In practice this poses no issues, since commit-graph chains do not loop, and there are few enough of them that adding additional frames to the stack is not a problem. A future commit will consolidate the progress output from `git commit-graph verify` when verifying chained commit-graphs to print a single line instead of one progress meter per commit-graph layer. Prepare for this by extracting a routine to verify a single layer of a commit-graph. Note that `verify_commit_graph()` is still recursive after this patch, but this will change in the subsequent patch. Signed-off-by: Taylor Blau Acked-by: Derrick Stolee Signed-off-by: Junio C Hamano --- commit-graph.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) (limited to 'commit-graph.c') diff --git a/commit-graph.c b/commit-graph.c index 843bdb458d..e846920935 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -2543,18 +2543,14 @@ static int commit_graph_checksum_valid(struct commit_graph *g) return hashfile_checksum_valid(g->data, g->data_len); } -int verify_commit_graph(struct repository *r, struct commit_graph *g, int flags) +static int verify_one_commit_graph(struct repository *r, + struct commit_graph *g, + int flags) { uint32_t i, cur_fanout_pos = 0; struct object_id prev_oid, cur_oid; int generation_zero = 0; struct progress *progress = NULL; - int local_error = 0; - - if (!g) { - graph_report("no commit-graph file loaded"); - return 1; - } verify_commit_graph_error = verify_commit_graph_lite(g); if (verify_commit_graph_error) @@ -2700,7 +2696,19 @@ int verify_commit_graph(struct repository *r, struct commit_graph *g, int flags) } stop_progress(&progress); - local_error = verify_commit_graph_error; + return verify_commit_graph_error; +} + +int verify_commit_graph(struct repository *r, struct commit_graph *g, int flags) +{ + int local_error = 0; + + if (!g) { + graph_report("no commit-graph file loaded"); + return 1; + } + + local_error = verify_one_commit_graph(r, g, flags); if (!(flags & COMMIT_GRAPH_VERIFY_SHALLOW) && g->base_graph) local_error |= verify_commit_graph(r, g->base_graph, flags); -- cgit v1.2.3 From f5facaa4653d3bcdb5ad5508e47d0e9a03c2aaa5 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Fri, 7 Jul 2023 20:31:39 -0400 Subject: commit-graph.c: iteratively verify commit-graph chains Now that we have a function which can verify a single layer of a commit-graph chain, implement `verify_commit_graph()` in terms of iterating over commit-graphs along their `->base_graph` pointers. This further prepares us to consolidate the progress output of `git commit-graph verify`. Signed-off-by: Taylor Blau Acked-by: Derrick Stolee Signed-off-by: Junio C Hamano --- commit-graph.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'commit-graph.c') diff --git a/commit-graph.c b/commit-graph.c index e846920935..bb56733d8c 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -2708,10 +2708,11 @@ int verify_commit_graph(struct repository *r, struct commit_graph *g, int flags) return 1; } - local_error = verify_one_commit_graph(r, g, flags); - - if (!(flags & COMMIT_GRAPH_VERIFY_SHALLOW) && g->base_graph) - local_error |= verify_commit_graph(r, g->base_graph, flags); + for (; g; g = g->base_graph) { + local_error |= verify_one_commit_graph(r, g, flags); + if (flags & COMMIT_GRAPH_VERIFY_SHALLOW) + break; + } return local_error; } -- cgit v1.2.3 From 7248857b6e74bb989dd067d2ac53605e77764700 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Fri, 7 Jul 2023 20:31:42 -0400 Subject: commit-graph.c: pass progress to `verify_one_commit_graph()` This is the final step to prepare for consolidating the output of `git commit-graph verify`. Instead of having each call to `verify_one_commit_graph()` initialize its own progress struct, have the caller pass one in instead. This patch does not alter the output of `git commit-graph verify`, but the next commit will consolidate the output. Signed-off-by: Taylor Blau Acked-by: Derrick Stolee Signed-off-by: Junio C Hamano --- commit-graph.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) (limited to 'commit-graph.c') diff --git a/commit-graph.c b/commit-graph.c index bb56733d8c..9e89952831 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -2545,12 +2545,11 @@ static int commit_graph_checksum_valid(struct commit_graph *g) static int verify_one_commit_graph(struct repository *r, struct commit_graph *g, - int flags) + struct progress *progress) { uint32_t i, cur_fanout_pos = 0; struct object_id prev_oid, cur_oid; int generation_zero = 0; - struct progress *progress = NULL; verify_commit_graph_error = verify_commit_graph_lite(g); if (verify_commit_graph_error) @@ -2601,10 +2600,6 @@ static int verify_one_commit_graph(struct repository *r, if (verify_commit_graph_error & ~VERIFY_COMMIT_GRAPH_ERROR_HASH) return verify_commit_graph_error; - if (flags & COMMIT_GRAPH_WRITE_PROGRESS) - progress = start_progress(_("Verifying commits in commit graph"), - g->num_commits); - for (i = 0; i < g->num_commits; i++) { struct commit *graph_commit, *odb_commit; struct commit_list *graph_parents, *odb_parents; @@ -2694,7 +2689,6 @@ static int verify_one_commit_graph(struct repository *r, graph_commit->date, odb_commit->date); } - stop_progress(&progress); return verify_commit_graph_error; } @@ -2709,9 +2703,16 @@ int verify_commit_graph(struct repository *r, struct commit_graph *g, int flags) } for (; g; g = g->base_graph) { - local_error |= verify_one_commit_graph(r, g, flags); + 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); if (flags & COMMIT_GRAPH_VERIFY_SHALLOW) break; + + stop_progress(&progress); } return local_error; -- cgit v1.2.3 From 9281cd07f014263b5385f13b47ff8399282c7cdc Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Fri, 7 Jul 2023 20:31:45 -0400 Subject: commit-graph.c: avoid duplicated progress output during `verify` 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 Acked-by: Derrick Stolee Signed-off-by: Junio C Hamano --- commit-graph.c | 27 +++++++++++++++++---------- t/t5324-split-commit-graph.sh | 3 ++- 2 files changed, 19 insertions(+), 11 deletions(-) (limited to 'commit-graph.c') diff --git a/commit-graph.c b/commit-graph.c index 9e89952831..fda4b6e14d 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -2545,7 +2545,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; @@ -2606,7 +2607,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); @@ -2695,26 +2696,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; } - 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); + 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); + } - local_error |= verify_one_commit_graph(r, g, progress); + for (; g; g = g->base_graph) { + 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 669ddc645f..36c4141e67 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 ' -- cgit v1.2.3