From: Srujith Poojary <poojarysrujith@gmail.com>
Date: Wed, 04 Nov 2026 03:58:40 GMT
Subject: Re: [PATCH v4 2/2] t4041: test SHA-256 submodule abbreviations
Message-ID: <CABh_KWg9bLLc64_UORHfLY3vhVSTfzN585fcQ9d6w8cZocGwvA@mail.gmail.com>
In-Reply-To: <xmqq7bitakkm.fsf@gitster.g>

On Tue, Nov 3, 2026 at 11:01 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Srujith A Poojary via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > From: Srujith A Poojary <poojarysrujith@gmail.com>
> >
> > The expected abbreviation for a submodule commit depends on the
> > repository's hash algorithm. Use test_oid_cache to provide the
> > expected value for each algorithm and remove the SHA1 prerequisite
> > so this test runs with both SHA-1 and SHA-256.
> >
> > Signed-off-by: Srujith A Poojary <poojarysrujith@gmail.com>
> > ---
> >  t/t4041-diff-submodule-option.sh | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
>
> It is strange to test SHA-1 in 1/2 and add test for SHA-256 as if an
> afterthought in 2/2.  Shouldn't all of these come in a single patch?
>
> The change to add the second hash algorithm support is surprisingly
> clean, primarily thanks to how test_oid* support was done long ago.
>
> > diff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh
> > index e80472af32..4539d6bba5 100755
> > --- a/t/t4041-diff-submodule-option.sh
> > +++ b/t/t4041-diff-submodule-option.sh
> > @@ -14,7 +14,11 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
> >
> >  . ./test-lib.sh
> >
> > -test_expect_success SHA1 'submodule commit abbreviation uses submodule object database' '
> > +test_expect_success 'submodule commit abbreviation uses submodule object database' '
> > +     test_oid_cache <<-EOF &&
> > +     submodule_commit1_abbrev sha1:0000000000e
> > +     submodule_commit1_abbrev sha256:24c81a5
> > +     EOF
> >       test_when_finished "rm -rf collision-super" &&
> >       git init -q collision-super &&
> >       git init -q collision-super/sm1 &&
> > @@ -27,7 +31,8 @@ test_expect_success SHA1 'submodule commit abbreviation uses submodule object da
> >       git -C collision-super update-index --add --cacheinfo 160000,"$commit1",sm1 &&
> >       git -C collision-super commit -q -m "Add submodule" &&
> >       git -C collision-super diff --submodule=log >actual &&
> > -     printf "Submodule sm1 0000000000e..%s:\n  > child\n" \
> > +     printf "Submodule sm1 %s..%s:\n  > child\n" \
> > +             "$(test_oid submodule_commit1_abbrev)" \
> >               "$(git -C collision-super/sm1 rev-parse --short "$commit2")" >expected &&
> >       test_cmp expected actual
> >  '

Hi Usman and Harald,

I wanted to share a progress update on my Git contribution.

I have submitted v5 of my submodule commit abbreviation patch through
GitGitGadget.

The latest version incorporates feedback from Junio C. Hamano and
Okhuomon Ajayi. I combined the implementation and SHA-256 test changes
into one commit.

The targeted test suite, `t4041-diff-submodule-option.sh`, passes all 48 tests.

PR: https://github.com/gitgitgadget/git/pull/2288

I will continue monitoring the review discussion and work on any
further feedback.

Regards,
Srujith