When validating that a commit-graph has either all zero, or all non-zero generation numbers, we emit a warning on both the rising and falling edge of transitioning between the two. So if we are unfortunate enough to see a commit-graph which has a repeating sequence of zero, then non-zero generation numbers, we'll generate many warnings that contain more or less the same information. Avoid this by treating `generation_zero` as a bit-field, and warn under the same conditions, but only do so once. Suggested-by: Junio C Hamano <gitster@xxxxxxxxx> Signed-off-by: Taylor Blau <me@xxxxxxxxxxxx> --- commit-graph.c | 17 ++++++++++------- t/t5318-commit-graph.sh | 14 ++++++++++++++ 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/commit-graph.c b/commit-graph.c index f7d9b31546..8d924b4509 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -2562,6 +2562,8 @@ static void graph_report(const char *fmt, ...) #define GENERATION_ZERO_EXISTS 1 #define GENERATION_NUMBER_EXISTS 2 +#define GENERATION_NUMBER_BOTH_EXIST \ + (GENERATION_ZERO_EXISTS | GENERATION_NUMBER_EXISTS) static int commit_graph_checksum_valid(struct commit_graph *g) { @@ -2573,19 +2575,19 @@ static void verify_mixed_generation_numbers(struct commit_graph *g, int *generation_zero) { if (commit_graph_generation_from_graph(graph_commit)) { - if (*generation_zero == GENERATION_ZERO_EXISTS) + if (*generation_zero & GENERATION_ZERO_EXISTS) graph_report(_("commit-graph has non-zero generation " "number for commit %s, but zero " "elsewhere"), oid_to_hex(&graph_commit->object.oid)); - *generation_zero = GENERATION_NUMBER_EXISTS; + *generation_zero |= GENERATION_NUMBER_EXISTS; } else { - if (*generation_zero == GENERATION_NUMBER_EXISTS) + if (*generation_zero & GENERATION_NUMBER_EXISTS) graph_report(_("commit-graph has generation number " "zero for commit %s, but non-zero " "elsewhere"), oid_to_hex(&graph_commit->object.oid)); - *generation_zero = GENERATION_ZERO_EXISTS; + *generation_zero |= GENERATION_ZERO_EXISTS; } } @@ -2702,10 +2704,11 @@ static int verify_one_commit_graph(struct repository *r, graph_report(_("commit-graph parent list for commit %s terminates early"), oid_to_hex(&cur_oid)); - verify_mixed_generation_numbers(g, graph_commit, - &generation_zero); + if (generation_zero != GENERATION_NUMBER_BOTH_EXIST) + verify_mixed_generation_numbers(g, graph_commit, + &generation_zero); - if (generation_zero == GENERATION_ZERO_EXISTS) + if (generation_zero & GENERATION_ZERO_EXISTS) continue; /* diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh index 8e96471b34..2626d41c94 100755 --- a/t/t5318-commit-graph.sh +++ b/t/t5318-commit-graph.sh @@ -628,6 +628,20 @@ test_expect_success 'detect mixed generation numbers (zero to non-zero)' ' "but zero elsewhere" ' +test_expect_success 'detect mixed generation numbers (flip-flop)' ' + corrupt_graph_setup && + for pos in \ + $GRAPH_BYTE_COMMIT_GENERATION \ + $GRAPH_BYTE_COMMIT_GENERATION_LAST + do + printf "\0\0\0\0" | dd of="full/$objdir/info/commit-graph" bs=1 \ + seek="$pos" conv=notrunc || return 1 + done && + + test_must_fail git -C full commit-graph verify 2>err && + test 1 -eq "$(grep -c "generation number" err)" +' + test_expect_success 'git fsck (checks commit-graph when config set to true)' ' git -C full fsck && corrupt_graph_and_verify $GRAPH_BYTE_FOOTER "\00" \ -- 2.42.0.rc0.30.gb82b15ebc8