Re: [RFC PATCH 4/6] tests: use `git submodule add` and fix expected diffs

From: Junio C Hamano <gitster@pobox.com>
To: Calvin Wan <calvinwan@google.com>
Cc: git@vger.kernel.org, Josh Steadmon <steadmon@google.com>
Subject: Re: [RFC PATCH 4/6] tests: use `git submodule add` and fix expected diffs
Date: Mon, 13 Feb 2023 15:19:14 -0800	[thread overview]
Message-ID: <xmqqttzp887h.fsf@gitster.g> (raw)
In-Reply-To: <xmqqy1p188rj.fsf@gitster.g> (Junio C. Hamano's message of "Mon, 13 Feb 2023 15:07:12 -0800")

Junio C Hamano <gitster@pobox.com> writes:

> Calvin Wan <calvinwan@google.com> writes:
>
>> From: Josh Steadmon <steadmon@google.com>
>>
>> This commit continues the previous work of updating the test suite to
>> use `git submodule add` to create submodules instead of using `git add`
>> to include embedded repositories. Specifically, in this commit we update
>> test cases where expected diffs must change due to the presence of a
>> .gitmodules file.
>
> Good that these "same" tasks are split into two steps, ones that
> don't notice the additional presence of .gitmodules (i.e. the
> previous step) and the result that do.
>
> Strictly speaking, gitlink support in the index and trees does not
> have to be used to record submodules, and changing all "git add" to
> "git submodule add" is somewhat a sad change that closes the door to
> those folks who want to use the mechanism for implementing "better
> submodules" that do not depend on a separate .gitmodules files, for
> example.  To leave the door open, "git add" may need to learn an
> overriding mechanism allow adding a gitlink to the index without
> anything else.  And then this step could use "git add -f" (or
> whategver the overriding mechanism) instead of "git submodule add",
> without changing the expected diff output.
>
> Would such a tweak make this step better?  I dunno.

The other side of the coin is that the tests that were touched by
the previous step were not testing half of what "submodule add" is
doing.  Not all the "let's add submodule here" test may not have to
verify the resulting .gitmodules file (e.g. ones that want to see
the low-level diff machinery works correctly by taking output from
"git diff --cached --raw" would be primarily interested in what
happens to the index and not to .gitmodules at all), but some of
them should.  A rough rule of thumb might be that the tests that are
about "submodules" should care what ".gitmodules" has, while the
tests tare are about "gitlink" should not.  It might even make sense
to use "add -f" (or whatever override to allow adding a gitlink only
to the index without touching .gitmodules and other things), not
"submodule add", for the latter class of tests.

Thanks.