Re: BUG in fetching non-checked out submodule

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 03.12.20 00:06, Junio C Hamano wrote:
Philippe Blain <levraiphilippeblain@xxxxxxxxx> 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@xxxxxxxxx/

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;



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux