Re: [PATCH 5/6] commit-graph: implement file format version 2
- From
- Jonathan Tan <jonathantanmy@google.com>
- Date
- Jan 23, 2019, 23:56 UTC
- Message-ID
- <20190123235630.183779-1-jonathantanmy@google.com>
- In-Reply-To
- <c55e0a738c79c30affcf2cb68564c01c36c66dff.1548280753.git.gitgitgadget@gmail.com>
Show 14 quoted lines
> +Version 2: > + > + 1-byte number (C) of "chunks" > + > + 1-byte reachability index version number: > + Currently, the only valid number is 1. > + > + 1-byte (reserved for later use) > + Current clients expect this value to be zero, and will not > + try to read the commit-graph file if it is non-zero. > + > + 4-byte format identifier for the hash algorithm: > + If this identifier does not agree with the repository's current > + hash algorithm, then the client will not read the commit graph.
[snip]
Show 30 quoted lines
> diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
> index b79d6263e9..3ff5e3b48d 100755
> --- a/t/t5318-commit-graph.sh
> +++ b/t/t5318-commit-graph.sh
> @@ -65,7 +65,8 @@ graph_read_expect() {
> NUM_CHUNKS=$((3 + $(echo "$2" | wc -w)))
> fi
> cat >expect <<- EOF
> - header: 43475048 1 1 $NUM_CHUNKS 0
> + header: 43475048 2 $NUM_CHUNKS 1 0
> + hash algorithm: 73686131
> num_commits: $1
> chunks: oid_fanout oid_lookup commit_metadata$OPTIONAL
> EOF
> @@ -390,10 +391,14 @@ test_expect_success 'detect bad signature' '
> '
>
> test_expect_success 'detect bad version' '
> - corrupt_graph_and_verify $GRAPH_BYTE_VERSION "\02" \
> + corrupt_graph_and_verify $GRAPH_BYTE_VERSION "\03" \
> "graph version"
> '
>
> +test_expect_success 'detect version 2 with version 1 data' '
> + corrupt_graph_and_verify $GRAPH_BYTE_VERSION "\02" \
> + "reachability index version"
> +'
> test_expect_success 'detect bad hash version' '
> corrupt_graph_and_verify $GRAPH_BYTE_HASH "\02" \
> "hash version"Should there also be a test that the "reserved" section be 0 and the 4-byte identifier agrees with the repo's hash algorithm? I assume that this can be done by "corrupting" the version to 2 and then truly corrupting the subsequent bytes.
Other than that, this and the previous patches look good.