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

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

From
Junio C Hamano <gitster@pobox.com>
Date
Aug 14, 2020, 19:05 UTC
Message-ID
<xmqqo8ndvyje.fsf@gitster.c.googlers.com>
In-Reply-To
<4bbfd345d16da4604dd20decda8ecb12372e4223.1597428440.git.gitgitgadget@gmail.com>
"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 12 quoted lines
> 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.
Very good idea.
Show 6 quoted lines
> 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.

That is perfectly fine. I think any file and on-wire protocol that uses anything but SHA-1 without identifying what it uses is a bug.

Will queue.  Thanks.
Show 106 quoted lines
> +OID_VERSION=1
> +if [ "$GIT_DEFAULT_HASH" = "sha256" ]
> +then
> +	OID_VERSION=2
> +fi
> +
>  test_expect_success 'setup test - repo, commits, commit graph, log outputs' '
>  	git init &&
>  	mkdir A A/B A/B/C &&
> @@ -35,7 +41,7 @@ test_expect_success 'setup test - repo, commits, commit graph, log outputs' '
>  graph_read_expect () {
>  	NUM_CHUNKS=5
>  	cat >expect <<- EOF
> -	header: 43475048 1 1 $NUM_CHUNKS 0
> +	header: 43475048 1 $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..5b65017676 100755
> --- a/t/t5318-commit-graph.sh
> +++ b/t/t5318-commit-graph.sh
> @@ -5,6 +5,12 @@ test_description='commit graph'
>  
>  GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS=0
>  
> +OID_VERSION=1
> +if [ "$GIT_DEFAULT_HASH" = "sha256" ]
> +then
> +	OID_VERSION=2
> +fi
> +
>  test_expect_success 'setup full repo' '
>  	mkdir full &&
>  	cd "$TRASH_DIRECTORY/full" &&
> @@ -77,7 +83,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 $OID_VERSION $NUM_CHUNKS 0
>  	num_commits: $1
>  	chunks: oid_fanout oid_lookup commit_metadata$OPTIONAL
>  	EOF
> @@ -412,6 +418,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..6f1a324f4f 100755
> --- a/t/t5324-split-commit-graph.sh
> +++ b/t/t5324-split-commit-graph.sh
> @@ -6,6 +6,12 @@ test_description='split commit graph'
>  GIT_TEST_COMMIT_GRAPH=0
>  GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS=0
>  
> +OID_VERSION=1
> +if [ "$GIT_DEFAULT_HASH" = "sha256" ]
> +then
> +	OID_VERSION=2
> +fi
> +
>  test_expect_success 'setup repo' '
>  	git init &&
>  	git config core.commitGraph true &&
> @@ -28,7 +34,7 @@ graph_read_expect() {
>  		NUM_BASE=$2
>  	fi
>  	cat >expect <<- EOF
> -	header: 43475048 1 1 3 $NUM_BASE
> +	header: 43475048 1 $OID_VERSION 3 $NUM_BASE
>  	num_commits: $1
>  	chunks: oid_fanout oid_lookup commit_metadata
>  	EOF
Previous: Derrick Stolee via GitGitGadgetNext: Taylor Blau
Message 7 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.