Re: [RFC PATCH 6/6] hex: allow only lowercase object IDs in breaking changes mode
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Aug 25, 2026, 16:36 UTC
- Message-ID
- <xmqq8q5unomp.fsf@gitster.g>
- In-Reply-To
- <20260729233215.398654-7-sandals@crustytoothpaste.net>
"brian m. carlson" <sandals@crustytoothpaste.net> writes:
> IDs. Note that in t5324, we change the hex character from "A" to "b" > because in SHA-256 mode, "a" is the correct value, so our test_must_fail > assertion will unexpectedly succeed in that case.
This was a bit hard to read and puzzled me, as you have two "A" and change only one of them to "b".
Is the idea that we wanted to make sure we use lowercase letters, because we do not want to see the tested "verify" command fail for now-forbidden uppercase hex but we want the command to read the data as valid hex and fail because it notices the corruption? So the first hunk is a no-op change (i.e., the first hash identifier on the first line is corrupt with the 30-th char in the file replaced with either 'a' or 'A'), while the second hunk is not (i.e., the second hash identifier on the second line in the file is corrupt with the 70-th char in the file replaced with 'A' but it is OK with 'a' because in the SHA-256 mode, the correct character for the place happens to be 'a')? It is puzzling if that is the case, because what this series wanted to tighten was that we used to treat hex chars case insensitively. So, if 'a' happened to be the right uncorrupted value for position 70, how did the original that replaced it to 'A' tested a "corrupted" state?
Show 22 quoted lines
> diff --git a/t/t5324-split-commit-graph.sh b/t/t5324-split-commit-graph.sh > index bf7ba0e558..29db815c77 100755 > --- a/t/t5324-split-commit-graph.sh > +++ b/t/t5324-split-commit-graph.sh > @@ -349,7 +349,7 @@ test_expect_success 'verify after commit-graph-chain corruption (base)' ' > test_must_fail git commit-graph verify 2>test_err && > grep -v "^+" test_err >err && > test_grep "invalid commit-graph chain" err && > - corrupt_file "$graphdir/commit-graph-chain" 30 "A" && > + corrupt_file "$graphdir/commit-graph-chain" 30 "a" && > test_must_fail git commit-graph verify 2>test_err && > grep -v "^+" test_err >err && > test_grep "unable to find all commit-graph files" err > @@ -364,7 +364,7 @@ test_expect_success 'verify after commit-graph-chain corruption (tip)' ' > test_must_fail git commit-graph verify 2>test_err && > grep -v "^+" test_err >err && > test_grep "invalid commit-graph chain" err && > - corrupt_file "$graphdir/commit-graph-chain" 70 "A" && > + corrupt_file "$graphdir/commit-graph-chain" 70 "b" && > test_must_fail git commit-graph verify 2>test_err && > grep -v "^+" test_err >err && > test_grep "unable to find all commit-graph files" err