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

[Stefan Beller <sbeller@google.com>]

On Fri, May 25, 2018 at 5:28 AM, Ævar Arnfjörð Bjarmason
<avarab@gmail.com> 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 <author@example.com>
        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 <author@example.com>
        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.