commit-graph: use the "hash version" byte

The commit-graph format reserved a byte among the header of the file to
store a "hash version". During the SHA-256 work, this was not modified
because file formats are not necessarily intended to work across hash
versions. If a repository has SHA-256 as its hash algorithm, it
automatically up-shifts the lengths of object names in all necessary
formats.

However, since we have this byte available for adjusting the version, we
can make the file formats more obviously incompatible instead of relying
on other context from the repository.

Update the oid_version() method in commit-graph.c to add a new value, 2,
for sha-256. This automatically writes the new value in a SHA-256
repository _and_ verifies the value is correct. This is a breaking
change relative to the current 'master' branch since 092b677 (Merge
branch 'bc/sha-256-cvs-svn-updates', 2020-08-13) but it is not breaking
relative to any released version of Git.

The test impact is relatively minor: the output of 'test-tool
read-graph' lists the header information, so those instances of '1' need
to be replaced with a variable determined by GIT_TEST_DEFAULT_HASH. A
more careful test is added that specifically creates a repository of
each type then swaps the commit-graph files. The important value here is
that the "git log" command succeeds while writing a message to stderr.

Helped-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Reviewed-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/technical/commit-graph-format.txt b/Documentation/technical/commit-graph-format.txt
index 440541045d45c2ef162ca747f9f2ad4dc7d88c92..6ddbceba15c9708385ba51f339bf232c15753648 100644 (file)
--- a/Documentation/technical/commit-graph-format.txt
+++ b/Documentation/technical/commit-graph-format.txt
@@ -42,8 +42,13 @@ HEADER:
   1-byte version number:
       Currently, the only valid version is 1.
 
-  1-byte Hash Version (1 = SHA-1)
-      We infer the hash length (H) from this value.
+  1-byte Hash Version
+      We infer the hash length (H) from this value:
+       1 => SHA-1
+       2 => SHA-256
+      If the hash type does not match the repository's hash algorithm, the
+      commit-graph file should be ignored with a warning presented to the
+      user.
 
   1-byte number (C) of "chunks"
 

diff --git a/commit-graph.c b/commit-graph.c
index e51c91dd5b0af4e4946091e5ca7f06707b3a0c5b..0ed003e218fb9bb2c024fb913a2af4fca9441039 100644 (file)
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -179,7 +179,14 @@ static char *get_chain_filename(struct object_directory *odb)
 
 static uint8_t oid_version(void)
 {
-       return 1;
+       switch (hash_algo_by_ptr(the_hash_algo)) {
+       case GIT_HASH_SHA1:
+               return 1;
+       case GIT_HASH_SHA256:
+               return 2;
+       default:
+               die(_("invalid hash version"));
+       }
 }
 
 static struct commit_graph *alloc_commit_graph(void)

diff --git a/t/t4216-log-bloom.sh b/t/t4216-log-bloom.sh
index c21cc160f3bb9dac91ce222e4fda5216927c1859..4bb9e9dbe28bf22d86d7e4ef8629d4f73328d815 100755 (executable)
--- a/t/t4216-log-bloom.sh
+++ b/t/t4216-log-bloom.sh
@@ -30,12 +30,17 @@ test_expect_success 'setup test - repo, commits, commit graph, log outputs' '
        rm file_to_be_deleted &&
        git add . &&
        git commit -m "file removed" &&
-       git commit-graph write --reachable --changed-paths
+       git commit-graph write --reachable --changed-paths &&
+
+       test_oid_cache <<-EOF
+       oid_version sha1:1
+       oid_version sha256:2
+       EOF
 '
 graph_read_expect () {
        NUM_CHUNKS=5
        cat >expect <<- EOF
-       header: 43475048 1 1 $NUM_CHUNKS 0
+       header: 43475048 1 $(test_oid oid_version) $NUM_CHUNKS 0
        num_commits: $1
        chunks: oid_fanout oid_lookup commit_metadata bloom_indexes bloom_data
        EOF

diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
index 044cf8a3def493e4a0e7395613eaadda1c236f02..2ed0c1544da120c10db3afedb656429f12a45613 100755 (executable)
--- a/t/t5318-commit-graph.sh
+++ b/t/t5318-commit-graph.sh
@@ -10,7 +10,12 @@ test_expect_success 'setup full repo' '
        cd "$TRASH_DIRECTORY/full" &&
        git init &&
        git config core.commitGraph true &&
-       objdir=".git/objects"
+       objdir=".git/objects" &&
+
+       test_oid_cache <<-EOF
+       oid_version sha1:1
+       oid_version sha256:2
+       EOF
 '
 
 test_expect_success POSIXPERM 'tweak umask for modebit tests' '
@@ -77,7 +82,7 @@ graph_read_expect() {
                NUM_CHUNKS=$((3 + $(echo "$2" | wc -w)))
        fi
        cat >expect <<- EOF
-       header: 43475048 1 1 $NUM_CHUNKS 0
+       header: 43475048 1 $(test_oid oid_version) $NUM_CHUNKS 0
        num_commits: $1
        chunks: oid_fanout oid_lookup commit_metadata$OPTIONAL
        EOF
@@ -412,6 +417,35 @@ test_expect_success 'replace-objects invalidates commit-graph' '
        )
 '
 
+test_expect_success 'warn on improper hash version' '
+       git init --object-format=sha1 sha1 &&
+       (
+               cd sha1 &&
+               test_commit 1 &&
+               git commit-graph write --reachable &&
+               mv .git/objects/info/commit-graph ../cg-sha1
+       ) &&
+       git init --object-format=sha256 sha256 &&
+       (
+               cd sha256 &&
+               test_commit 1 &&
+               git commit-graph write --reachable &&
+               mv .git/objects/info/commit-graph ../cg-sha256
+       ) &&
+       (
+               cd sha1 &&
+               mv ../cg-sha256 .git/objects/info/commit-graph &&
+               git log -1 2>err &&
+               test_i18ngrep "commit-graph hash version 2 does not match version 1" err
+       ) &&
+       (
+               cd sha256 &&
+               mv ../cg-sha1 .git/objects/info/commit-graph &&
+               git log -1 2>err &&
+               test_i18ngrep "commit-graph hash version 1 does not match version 2" err
+       )
+'
+
 # the verify tests below expect the commit-graph to contain
 # exactly the commits reachable from the commits/8 branch.
 # If the file changes the set of commits in the list, then the

diff --git a/t/t5324-split-commit-graph.sh b/t/t5324-split-commit-graph.sh
index ea28d522b8375678c9a54d6c4b6717ef8f11ab6f..18216463c7db99e965833788ccd990f1e8515397 100755 (executable)
--- a/t/t5324-split-commit-graph.sh
+++ b/t/t5324-split-commit-graph.sh
@@ -18,6 +18,9 @@ test_expect_success 'setup repo' '
 
        base sha1:1376
        base sha256:1496
+
+       oid_version sha1:1
+       oid_version sha256:2
        EOM
 '
 
@@ -28,7 +31,7 @@ graph_read_expect() {
                NUM_BASE=$2
        fi
        cat >expect <<- EOF
-       header: 43475048 1 1 3 $NUM_BASE
+       header: 43475048 1 $(test_oid oid_version) 3 $NUM_BASE
        num_commits: $1
        chunks: oid_fanout oid_lookup commit_metadata
        EOF