Re: [PATCH 5/6] commit-graph: implement file format version 2

[Jonathan Tan <jonathantanmy@google.com>]

> +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]

> 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.