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

[Stefan Beller <sbeller@google.com>]

Any thoughts on my thoughts below?

On Fri, Apr 29, 2016 at 12:17 PM, Stefan Beller <sbeller@google.com> wrote:
> On Fri, Apr 29, 2016 at 11:37 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> 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