Subject: Re: sb/submodule-move-nested breaks t7411 under GIT_FSMONITOR_TEST

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Thu, 06 Sep 2018 14:31:44 +0200

On Fri, May 25 2018, Stefan Beller wrote:

> On Fri, May 25, 2018 at 5:28 AM, Ævar Arnfjörð Bjarmason
> <[email-masked]> wrote:
>>
>> On Thu, May 17 2018, Junio C Hamano wrote:
>>
>>> * sb/submodule-move-nested (2018-03-29) 6 commits
>>>   (merged to 'next' on 2018-04-25 at 86b177433a)
>>>  + submodule: fixup nested submodules after moving the submodule
>>>  + submodule-config: remove submodule_from_cache
>>>  + submodule-config: add repository argument to submodule_from_{name, path}
>>>  + submodule-config: allow submodule_free to handle arbitrary repositories
>>>  + grep: remove "repo" arg from non-supporting funcs
>>>  + submodule.h: drop declaration of connect_work_tree_and_git_dir
>>>
>>>  Moving a submodule that itself has submodule in it with "git mv"
>>>  forgot to make necessary adjustment to the nested sub-submodules;
>>>  now the codepath learned to recurse into the submodules.
>>
>> I didn't spot this earlier because I don't test this a lot, but I've
>> bisected the following breakage down to da62f786d2 ("submodule: fixup
>> nested submodules after moving the submodule", 2018-03-28) (and manually
>> confirmed by reverting). On Linux both Debian & CentOS I get tests 3 and
>> 4 failing with:
>>
>>      GIT_FSMONITOR_TEST=$PWD/t7519/fsmonitor-all ./t7411-submodule-config.sh
>>
>> -v -x output follows:
>>
>> expecting success:
>>         mkdir submodule &&
>>         (cd submodule &&
>>                 git init &&
>>                 echo a >a &&
>>                 git add . &&
>>                 git commit -ma
>>         ) &&
>>         mkdir super &&
>>         (cd super &&
>>                 git init &&
>>                 git submodule add ../submodule &&
>>                 git submodule add ../submodule a &&
>>                 git commit -m "add as submodule and as a" &&
>>                 git mv a b &&
>>                 git commit -m "move a to b"
>>         )
>
> when you add a test_pause here and dump the
> state of the setup, then it can be observed that when the fsmonitor is active
> the last commit is different; without fsmonitor the moved gitlink and the change
> to the .gitmodules file is part of the commit, i.e.
>
> $ git -C super show
>         commit d3d90b70a01bd17d026f75a803c8b65f5903a7c0 (HEAD -> master)
>         Author: A U Thor <[email-masked]>
>         Date:   Fri May 25 19:21:58 2018 +0000
>
>             move a to b
>
>         diff --git a/.gitmodules b/.gitmodules
>         index 3f4d474..6149210 100644
>         --- a/.gitmodules
>         +++ b/.gitmodules
>         @@ -2,5 +2,5 @@
>           path = submodule
>           url = ../submodule
>          [submodule "a"]
>         - path = a
>         + path = b
>           url = ../submodule
>         diff --git a/a b/b
>         similarity index 100%
>         rename from a
>         rename to b
> When running with the fsmonitor:
>
> $ git -C super show
>         commit 57022a92acf46f303498c045440ec099cbc35a2d (HEAD -> master)
>         Author: A U Thor <[email-masked]>
>         Date:   Fri May 25 19:22:52 2018 +0000
>
>             move a to b
>
>         diff --git a/a b/b
>         similarity index 100%
>         rename from a
>         rename to b
> $ git -C super diff
>         diff --git a/.gitmodules b/.gitmodules
>         index 3f4d474..6149210 100644
>         --- a/.gitmodules
>         +++ b/.gitmodules
>         @@ -2,5 +2,5 @@
>           path = submodule
>           url = ../submodule
>          [submodule "a"]
>         - path = a
>         + path = b
>           url = ../submodule
>
> This hints at a problem with git commit;
>
> I tried adding test_tick, to unconfuse the fsmonitor, but that doesn't help,
> digging further, the problem is in the git mv command, which fails to
> add the change in
> .gitmodules to the index.
>
> Adding the verbose flag to stage_updated_gitmodules() that is called by
> git-mv very late in the game, such that
>
> void stage_updated_gitmodules(struct index_state *istate)
> {
>     trace_printf("staging .gitmodules files");
>     if (add_file_to_index(istate, GITMODULES_FILE, ADD_CACHE_VERBOSE))
>         die(_("staging updated .gitmodules failed"));
> }
>
> We would get a message if the .gitmodules file is staged correctly, as
> add_file_to_index() that calls add_to_index that would print
>
>     if (verbose && !was_same)
>         printf("add '%s'\n", path);
>
> I could not see that message, so I suspect, that there is something
> racy.
>
> Will debug further.

I spotted this again after testing the split index (see
https://public-inbox.org/git/[email-masked]/) and
was testing the fsmonitor test mode as well.

So gentle *poke*: Did you get anywhere with debugging this? It's still
failing on "master" now.