Re: BUG in fetching non-checked out submodule

["Peter Kästle" <peter.kaestle@nokia.com>]

Hi,

On 03.12.20 00:06, Junio C Hamano wrote:
> Philippe Blain <levraiphilippeblain@gmail.com> writes:
> 
>> 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.

Junio: This is why I originally intended to commit the test case for the 
testsuite separated from the revert and wanted to start a discussion 
about the actual real fix for the issue:
https://public-inbox.org/git/1604413399-63090-1-git-send-email-peter.kaestle@nokia.com/

My proposal would be to revert 1b7ac4e6d4 and isolate the test case 
"test_expect_success 'setup nested submodule fetch test' '" make it 
"test_expect_failure" and apply it instead, until we come up with a real 
solution.

>> 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.

Can we start a new test suite for tests on which this variable is set? 
The case of Ralf can be used as first test case.

Ralf: could you please send a command sequence, which recreates the 
required repository setup from scratch and is able to trigger your 
observation?  Then we can convert this into a test case.

Thanks.

-- 
best regards
--peter;