While iterating through the commit parents, perform the generation
number calculation and compare against the value stored in the
commit-graph.

The tests demonstrate that having a different set of parents affects
the generation number calculation, and this value propagates to
descendants. Hence, we drop the single-line condition on the output.

Since Git will ship with the commit-graph feature without generation
numbers, we need to accept commit-graphs with all generation numbers
equal to zero. In this case, ignore the generation number calculation.

However, verify that we should never have a mix of zero and non-zero
generation numbers. Create a test that sets one commit to generation
zero and all following commits report a failure as they have non-zero
generation in a file that contains generation number zero.

Signed-off-by: Derrick Stolee <dsto...@microsoft.com>
---
 commit-graph.c          | 34 ++++++++++++++++++++++++++++++++++
 t/t5318-commit-graph.sh | 11 +++++++++++
 2 files changed, 45 insertions(+)

diff --git a/commit-graph.c b/commit-graph.c
index d2a86e329e..5faecae2a7 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -842,10 +842,14 @@ static void graph_report(const char *fmt, ...)
        va_end(ap);
 }
 
+#define GENERATION_ZERO_EXISTS 1
+#define GENERATION_NUMBER_EXISTS 2
+
 int verify_commit_graph(struct commit_graph *g)
 {
        uint32_t i, cur_fanout_pos = 0;
        struct object_id prev_oid, cur_oid;
+       int generation_zero = 0;
 
        if (!g) {
                graph_report("no commit-graph file loaded");
@@ -907,6 +911,7 @@ int verify_commit_graph(struct commit_graph *g)
        for (i = 0; i < g->num_commits; i++) {
                struct commit *graph_commit, *odb_commit;
                struct commit_list *graph_parents, *odb_parents;
+               uint32_t max_generation = 0;
 
                hashcpy(cur_oid.hash, g->chunk_oid_lookup + g->hash_len * i);
 
@@ -941,6 +946,9 @@ int verify_commit_graph(struct commit_graph *g)
                                             
oid_to_hex(&graph_parents->item->object.oid),
                                             
oid_to_hex(&odb_parents->item->object.oid));
 
+                       if (graph_parents->item->generation > max_generation)
+                               max_generation = 
graph_parents->item->generation;
+
                        graph_parents = graph_parents->next;
                        odb_parents = odb_parents->next;
                }
@@ -948,6 +956,32 @@ int verify_commit_graph(struct commit_graph *g)
                if (odb_parents != NULL)
                        graph_report("commit-graph parent list for commit %s 
terminates early",
                                     oid_to_hex(&cur_oid));
+
+               if (!graph_commit->generation) {
+                       if (generation_zero == GENERATION_NUMBER_EXISTS)
+                               graph_report("commit-graph has generation 
number zero for commit %s, but non-zero elsewhere",
+                                            oid_to_hex(&cur_oid));
+                       generation_zero = GENERATION_ZERO_EXISTS;
+               } else if (generation_zero == GENERATION_ZERO_EXISTS)
+                       graph_report("commit-graph has non-zero generation 
number for commit %s, but zero elsewhere",
+                                    oid_to_hex(&cur_oid));
+
+               if (generation_zero == GENERATION_ZERO_EXISTS)
+                       continue;
+
+               /*
+                * If one of our parents has generation GENERATION_NUMBER_MAX, 
then
+                * our generation is also GENERATION_NUMBER_MAX. Decrement to 
avoid
+                * extra logic in the following condition.
+                */
+               if (max_generation == GENERATION_NUMBER_MAX)
+                       max_generation--;
+
+               if (graph_commit->generation != max_generation + 1)
+                       graph_report("commit-graph generation for commit %s is 
%u != %u",
+                                    oid_to_hex(&cur_oid),
+                                    graph_commit->generation,
+                                    max_generation + 1);
        }
 
        return verify_commit_graph_error;
diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
index ec0964112a..a6ea1341dc 100755
--- a/t/t5318-commit-graph.sh
+++ b/t/t5318-commit-graph.sh
@@ -272,6 +272,7 @@ GRAPH_BYTE_COMMIT_TREE=$GRAPH_COMMIT_DATA_OFFSET
 GRAPH_BYTE_COMMIT_PARENT=$(($GRAPH_COMMIT_DATA_OFFSET + $HASH_LEN))
 GRAPH_BYTE_COMMIT_EXTRA_PARENT=$(($GRAPH_COMMIT_DATA_OFFSET + $HASH_LEN + 4))
 GRAPH_BYTE_COMMIT_WRONG_PARENT=$(($GRAPH_COMMIT_DATA_OFFSET + $HASH_LEN + 3))
+GRAPH_BYTE_COMMIT_GENERATION=$(($GRAPH_COMMIT_DATA_OFFSET + $HASH_LEN + 11))
 
 # usage: corrupt_graph_and_verify <position> <data> <string>
 # Manipulates the commit-graph file at the position
@@ -366,4 +367,14 @@ test_expect_success 'detect wrong parent' '
                "commit-graph parent for"
 '
 
+test_expect_success 'detect incorrect generation number' '
+       corrupt_graph_and_verify $GRAPH_BYTE_COMMIT_GENERATION "\070" \
+               "generation for commit"
+'
+
+test_expect_success 'detect incorrect generation number' '
+       corrupt_graph_and_verify $GRAPH_BYTE_COMMIT_GENERATION "\01" \
+               "non-zero generation number"
+'
+
 test_done
-- 
2.18.0.rc1

Reply via email to