git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 2/3] commit-graph: use the "hash version" byte

From
Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>
Date
Aug 17, 2020, 14:04 UTC
Message-ID
<8d481f3b223960f2193678929b0a1705222386b7.1597673089.git.gitgitgadget@gmail.com>
In-Reply-To
<pull.703.v2.git.1597673089.gitgitgadget@gmail.com>
From: Derrick Stolee <dstolee@microsoft.com>

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>
---
 .../technical/commit-graph-format.txt         |  9 ++++-
 commit-graph.c                                |  9 ++++-
 t/t4216-log-bloom.sh                          |  9 ++++-
 t/t5318-commit-graph.sh                       | 38 ++++++++++++++++++-
 t/t5324-split-commit-graph.sh                 |  5 ++-
 5 files changed, 62 insertions(+), 8 deletions(-)
diff --git a/Documentation/technical/commit-graph-format.txt b/Documentation/technical/commit-graph-format.txt
index 440541045d..6ddbceba15 100644
--- 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 e51c91dd5b..0ed003e218 100644
--- 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 c21cc160f3..4bb9e9dbe2 100755
--- 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 044cf8a3de..2ed0c1544d 100755
--- 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 ea28d522b8..18216463c7 100755
--- 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

-- 
gitgitgadget
Previous: Derrick Stolee via GitGitGadgetNext: Derrick Stolee via GitGitGadget
Message 20 of 22 in “SHA-256: Update commit-graph and multi-pack-index formats”
  1. 0/3 SHA-256: Update commit-graph and multi-pack-index formatsDerrick Stolee via GitGitGadget, Aug 14, 2020
  2. 1/3 t/README: document GIT_TEST_DEFAULT_HASHDerrick Stolee via GitGitGadget, Aug 14, 2020
  3. Junio C HamanoAug 14, 2020
  4. Eric SunshineAug 14, 2020
  5. Derrick StoleeAug 14, 2020
  6. 2/3 commit-graph: use the hash version byteDerrick Stolee via GitGitGadget, Aug 14, 2020
  7. Junio C HamanoAug 14, 2020
  8. Taylor BlauAug 14, 2020
  9. brian m. carlsonAug 14, 2020
  10. Junio C HamanoAug 14, 2020
  11. Derrick StoleeAug 14, 2020
  12. Martin ÅgrenAug 15, 2020
  13. 3/3 multi-pack-index: use hash version byteDerrick Stolee via GitGitGadget, Aug 14, 2020
  14. brian m. carlsonAug 14, 2020
  15. Junio C HamanoAug 14, 2020
  16. Derrick StoleeAug 14, 2020
  17. Junio C HamanoAug 14, 2020
  18. 0/3 SHA-256: Update commit-graph and multi-pack-index formatsDerrick Stolee via GitGitGadget, Aug 17, 2020
  19. 1/3 t/README: document GIT_TEST_DEFAULT_HASHDerrick Stolee via GitGitGadget, Aug 17, 2020
  20. 2/3 commit-graph: use the "hash version" byteDerrick Stolee via GitGitGadget, Aug 17, 2020
  21. 3/3 multi-pack-index: use hash version byteDerrick Stolee via GitGitGadget, Aug 17, 2020
  22. brian m. carlsonAug 17, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.