Re: [PATCH 11/15] diff: ignore submodules excluded by groups

From: Stefan Beller <sbeller@google.com>
Date: Apr 29, 2016, 19:17 UTC
Message-ID: <CAGZ79kaOXxqDEqVnf5K3QjXg5bfmKW2XkmPT-mqJ93+RF5N40g@mail.gmail.com>
In-Reply-To: <xmqqlh3wxnuq.fsf@gitster.mtv.corp.google.com>

On Fri, Apr 29, 2016 at 11:37 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 18 quoted lines

> Stefan Beller <sbeller@google.com> writes:
>
>> We do not need to do anything special to initialize the `submodule_groups`
>> pointer as the diff options setup will fill in 0 by default.
>>
>> Signed-off-by: Stefan Beller <sbeller@google.com>
>> ---
>>  diff.c | 3 +++
>>  diff.h | 1 +
>>  2 files changed, 4 insertions(+)
>
> Isn't this going in the opposite way from what you described in 0/15
> with analogy to how "ignore" mechanism works?  Just like a path is
> tracked once it is tracked, whether it matches an ignore pattern,
> shouldn't we be getting a summary for a submodule for any submodule
> once submodule/.git/HEAD is there (i.e. we can give a comparison),
> whether it is specified by a separate mechanism that acts from
> sideways (e.g. the "default group").

That is why I started in 0/15 with

> One pain point I am still aware of:

as I did not do the right thing in these patches? These patches do however:

>    git status
>    git diff
>    git submodule summary
>    # show only changes to submodules which are in the default group.

which seems to be the wrong thing then.

So here is a thought experiment:

    # get all submodules into the work tree
    git submodule update --recursive --init

    # The selected default group will not include all submodules
    git config submodule.defaultGroup "*SomeLabel"

    git status
    # What do we expect now?
    # either a "nothing to commit, working directory clean"
    # or rather what was described in 0/15:

        More than 2 submodules (123 actually) including
            'path/foo'
            'lib/baz'
        are checked out, but not part of the default group.
        You can add these submodules via
            git config --add submodule.defaultGroup ./path/foo
            git config --add submodule.defaultGroup ./lib/baz

If we want to go with the second option, the design described in 0/15 is broken. Going one step further:

    # in all submodules including the excluded via groups,
    # by resetting the groups for this command
    git -c submodule.defaultGroup= submodule foreach git reset --hard HEAD^

    git status
    # Reporting the changes from submodules in the default Group is
uncontroversial:
    On branch master
    Your branch is up-to-date with 'origin/master'.
    Changes not staged for commit:
      (use "git add <file>..." to update what will be committed)
      (use "git checkout -- <file>..." to discard changes in working directory)

        modified:   subA (new commits)
        modified:   subB (new commits)

    no changes added to commit (use "git add" and/or "git commit -a")

    # But what about subC which is not in the default group? It was
changed as well.
    # one thing I could imagine, is similar to above:

        More than 2 submodules (123 actually) including
            modified:    'subC'  (new commits)
            modified:   'lib/baz' (new commits)
        are checked out, but not part of the default group.
        You can add these submodules via
            git config --add submodule.defaultGroup ./path/foo
            git config --add submodule.defaultGroup ./lib/baz

    # and the remaining 121 submodules are not reported in git status

    git diff
    # report them all below:
    diff --git a/subA b/subA
    index e4e79a2..6689f08 160000
    --- a/subA
    +++ b/subA
    @@ -1 +1 @@
    -Subproject commit e4e79a217576d24ef4d73b620766f62b155bcd98
    +Subproject commit 6689f08735d08a057f8d6f91af98b04960afa517
    ...
     # goes on including 123 submodule not in the default group.

In case we report these submodules which are checked out but not in the default group, we probably want to adapt "git submodule update" to un-checkout the work tree of the submodules in case they are clean.

Thanks, Stefan