On 6/2/2018 8:39 AM, Jakub Narebski wrote:
Derrick Stolee <dsto...@microsoft.com> writes:
The commit-graph file has an extra chunk to store the parent int-ids for
parents beyond the first parent for octopus merges. Our test repo has a
single octopus merge that we can manipulate to demonstrate the 'verify'
subcommand detects incorrect values in that chunk.
If I understand it correctly the above means that our _reading_ code
checks for validity (which then 'git commit-graph verify' uses), just
there were not any tests for that.
Signed-off-by: Derrick Stolee <dsto...@microsoft.com>
---
t/t5318-commit-graph.sh | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
index 58adb8246d..240aef6add 100755
--- a/t/t5318-commit-graph.sh
+++ b/t/t5318-commit-graph.sh
@@ -248,6 +248,7 @@ test_expect_success 'git commit-graph verify' '
'
NUM_COMMITS=9
+NUM_OCTOPUS_EDGES=2
HASH_LEN=20
GRAPH_BYTE_VERSION=4
GRAPH_BYTE_HASH=5
@@ -274,6 +275,10 @@ GRAPH_BYTE_COMMIT_EXTRA_PARENT=`expr
$GRAPH_COMMIT_DATA_OFFSET + $HASH_LEN + 4`
GRAPH_BYTE_COMMIT_WRONG_PARENT=`expr $GRAPH_COMMIT_DATA_OFFSET + $HASH_LEN +
3`
GRAPH_BYTE_COMMIT_GENERATION=`expr $GRAPH_COMMIT_DATA_OFFSET + $HASH_LEN + 8`
GRAPH_BYTE_COMMIT_DATE=`expr $GRAPH_COMMIT_DATA_OFFSET + $HASH_LEN + 12`
+GRAPH_COMMIT_DATA_WIDTH=`expr $HASH_LEN + 16`
+GRAPH_OCTOPUS_DATA_OFFSET=`expr $GRAPH_COMMIT_DATA_OFFSET + \
+ $GRAPH_COMMIT_DATA_WIDTH \* $NUM_COMMITS`
+GRAPH_BYTE_OCTOPUS=`expr $GRAPH_OCTOPUS_DATA_OFFSET + 4`
# usage: corrupt_graph_and_verify <position> <data> <string>
# Manipulates the commit-graph file at the position
@@ -378,4 +383,9 @@ test_expect_success 'detect incorrect commit date' '
"commit date"
'
+test_expect_success 'detect incorrect parent for octopus merge' '
+ corrupt_graph_and_verify $GRAPH_BYTE_OCTOPUS "\01" \
+ "invalid parent"
+'
So we change the int-id to non-existing commit, and check that
commit-graph code checks for that.
What about the case when there are octopus merges, but no EDGE chunk
(which I think we can emulate by changing / corrupting number of
chunks)?
What about the case where int-id of edge in EDGE chunk is correct, that
is points to a valid commit, but does not agree with what is in the
object database (what parents octopus merge has in reality)?
Do we detect the situation where the second parent value in the commit
data stores an array position within a Large Edge chunk, but we do not
reach a value with the most-significant bit on when reaching the end of
Large Edge chunk?
There are a few holes like this, but I think they are better suited to a
follow-up series, as this series is already quite large.
-Stolee
