Re: BUG in fetching non-checked out submodule

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 2, 2020, 23:06 UTC
Message-ID: <xmqqzh2vkdu8.fsf@gitster.c.googlers.com>
In-Reply-To: <CC0FA973-E37A-4BD3-B5A2-1436DD8DF16F@gmail.com>

Philippe Blain <levraiphilippeblain@gmail.com> writes:

Show 13 quoted lines

> Thanks for bisecting it. That commit wanted to fix a different bug
> related to nested submodules, and the route taken was simply
> reverting an earlier commit (a62387b (submodule.c: fetch in
> submodules git directory instead of in worktree, 2018-11-28).
>
> As you discovered, it breaks other scenarios.
>
>> 
>> $ git version
>> git version 2.29.2.435.g72ffeb997e
>> 
>> $ git config --get submodule.recurse
>> true

I think the current situation is probably worse.

As a short-term fix, we should revert 1b7ac4e6d4 until we can come up with a real fix, probably.

> Yeah, I think the test suite could make more efforts
> to run more tests with that setting turned 'on', but
> it would require significants efforts since it changes 
> the behaviour of several commands.

I am not sure if the question is about amount of efforts.

A configuration variable is there to change the behaviour of commands, so a test of a command that has been running happily and producing a set of expected outcome with a configuration unset should break the expectation when the configuration is set --- otherwise there is no point in having a configuration variable.

> Meta question: is there an easy way to run the whole test
> suite with specific config options turned on ?

Hence, I do not think it even makes sense to have such an "easy way". If the "fetch" command, for example, is expected to change behaviour depending on the value of submodule.recurse, a test written for the case where the variable is not set should produce different outcome when the variable is set.

What we need may be a better test coverage. submodule.recurse is a later addition, and all tests written earlier do test how the commands behave without the configuration being set. If one wants to change the behaviour of these commands when the configuration is set, new tests to specify what the expected behaviour need to be added.

> Thanks for the report,

Yup, thanks for helping out.