Re: [PATCH v2 2/6] tests: Use `git submodule add` instead of `git add`

From: Calvin Wan <calvinwan@google.com>
Date: Mar 3, 2023, 00:16 UTC
Message-ID: <CAFySSZDeZ-HHPsh9noX1xqGfdzkRALwH0DRzcG3XPUKp57P-rg@mail.gmail.com>
In-Reply-To: <xmqqttz5pduy.fsf@gitster.g>

On Tue, Feb 28, 2023 at 3:30 PM Junio C Hamano <gitster@pobox.com> wrote:

Show 18 quoted lines

>
> Calvin Wan <calvinwan@google.com> writes:
>
> > diff --git a/t/t2013-checkout-submodule.sh b/t/t2013-checkout-submodule.sh
> > index b2bdd1fcb4..dd2858648b 100755
> > --- a/t/t2013-checkout-submodule.sh
> > +++ b/t/t2013-checkout-submodule.sh
> > @@ -10,7 +10,7 @@ test_expect_success 'setup' '
> >       (cd submodule &&
> >        git init &&
> >        test_commit first) &&
> > -     git add submodule &&
> > +     git submodule add ./submodule &&
>
> The change from "submodule" to "./submodule" was not explained in
> the proposed log message.  I think this is necessary for "git
> submodule add" to function as expected, but if that is why we are
> making this change, perhaps we should mention it?

ack.

Show 8 quoted lines

>
> > @@ -51,6 +51,7 @@ test_expect_success '"checkout <submodule>" honors submodule.*.ignore from .gitm
> >       git config diff.ignoreSubmodules none &&
> >       git config -f .gitmodules submodule.submodule.path submodule &&
> >       git config -f .gitmodules submodule.submodule.ignore untracked &&
> > +     git commit -m "Update patterns in .gitmodules" .gitmodules &&
>
> What does "patterns" refer here (another one in the next hunk)?

"patterns" seems unnecessary here. "Update .gitmodules" is more than sufficient.

Show 18 quoted lines

>
> > diff --git a/t/t2103-update-index-ignore-missing.sh b/t/t2103-update-index-ignore-missing.sh
> > index e9451cd567..11bc136f6e 100755
> > --- a/t/t2103-update-index-ignore-missing.sh
> > +++ b/t/t2103-update-index-ignore-missing.sh
> > @@ -36,7 +36,7 @@ test_expect_success basics '
> >               git add file &&
> >               git commit -m "sub initial"
> >       ) &&
> > -     git add xyzzy &&
> > +     git add ./xyzzy &&
>
> Is this supposed to have become "git submodule add ./xyzzy"?  Or
> "git add xyzzy" will trigger "don't add gitlink" warning but you can
> write "git add ./xyzzy" as a way to work around the warning?
>
> Or is this an incomplete change that wasn't spotted during
> proofreading?

The latter. Missed this one when I was reverting the change from v1.

Show 14 quoted lines

>
> > diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh
> > index 302e4cbdba..f8ef70b5a2 100755
> > --- a/t/t5531-deep-submodule-push.sh
> > +++ b/t/t5531-deep-submodule-push.sh
> > @@ -28,7 +28,7 @@ test_expect_success setup '
> >                       git add junk &&
> >                       git commit -m "Initial junk"
> >               ) &&
> > -             git add gar/bage &&
> > +             git submodule add ./gar/bage ./gar/bage &&
>
> Why does this one (and only this one) look different?  Everybody
> else changed "git add A" to "git submodule add ./A", it seems?

The second ./gar/bage is to define the submodule path. Without it, submodule add attempts to add it to ./bage instead of ./gar/bage. This is unique only to this test since the submodule is 2 folders deep.

Show 9 quoted lines

>
> > diff --git a/t/t6437-submodule-merge.sh b/t/t6437-submodule-merge.sh
> > ...
>
> I think I saw a code section that was touched in the previous patch
> that hand-crafted .gitmodules file to make the gitlink it adds into
> a submodule.  It is unexpected and puzzling that there is no removal
> of that "cat >.gitmodules" from t4060.
>

I have gone ahead and removed the "cat >.gitmodules" from t4060, and fixed the subsequent tests that were affected by that