Re: [PATCH 0/3] SHA-256: Update commit-graph and multi-pack-index formats

From: Derrick Stolee <stolee@gmail.com>
Date: Aug 14, 2020, 20:34 UTC
Message-ID: <04ebec72-07de-4494-688f-a9f48442906e@gmail.com>
In-Reply-To: <xmqqeeo9vxl0.fsf@gitster.c.googlers.com>

On 8/14/2020 3:25 PM, Junio C Hamano wrote:

Show 14 quoted lines

> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> As discussed [1], there is some concern around binary file formats requiring
>> the context of the repository config in order to infer hash lengths. Two
>> formats that were designed with the hash transition in mind (commit-graph
>> and multi-pack-index) have bytes available to indicate the hash algorithm
>> used. Let's actually update these formats to be more self-contained with the
>> two hash algorithms being available.
>> ...
>> If this is the way we want to go with the formats, then I'll assist
>> coordinating these textual and semantic merge conflicts.
> 
> I agree that the files should be self-identifying, but have these
> changes tested without sha256 hash?

All of the test scripts pass with and without GIT_TEST_DEFAULT_HASH=sha256, and this test in t5318 (and a similar one in t5319) are explicit about testing both options:

+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 + ) +' +

Since this tests exactly that the "hash version" byte is the same in a SHA-1 repo, this checks that the new version of Git writes backwards- compatible data in SHA-1 repos.

Or are you hinting at a more subtle test scenario that I missed?

Thanks, -Stolee