Re: [PATCH 5/5] builtin/clone: support submodule groups

On Wed, Nov 25, 2015 at 9:52 AM, Jens Lehmann [off-list ref] wrote:

quoted

+--group::
+       After the clone is created, all submodules which are part of the
+       group are cloned. This option can be given multiple times to
specify
+       different groups.

Ah, that answers my question in my response to the cover letter ;-)

quoted

This option will imply automatic submodule
+       updates for the groups by setting `submodule.update=groups`.

Please don't. The per-submodule update setting configures how a
submodule has to be updated, adding a global one with a completely
different meaning (what submodules should be updated?) is confusing.
Why not "submodule.groups=<groups>"?

The documentation is out of date :/ as I was churning through lots of ideas,
so we do have a config submodule.groups=<groups> by now, but the
documentation is wrong.

quoted

+       The group selection will be passed on recursively, i.e. if a
submodule
+       is cloned because of group membership, its submodules will
+       be cloned according to group membership, too. If a submodule is
+       not cloned however, its submodules are not evaluated for group
+       membership.

What do you mean by the last sentence? Did the clone fail? Then you
cannot update the submodule anyway ...

Consider nested submodules:

    A: superproject containing
        B: which contains
            C.

If you clone A with group <C-but-not-B> you won't get C as we do not traverse
the submodules of B, as we don't clone B. Maybe it's obvious?

quoted

@@ -864,6 +876,21 @@ int cmd_clone(int argc, const char **argv, const char

*prefix)
                option_no_checkout = 1;
        }

+       if (option_recursive && submodule_groups.nr > 0)
+               die(_("submodule groups and recursive flag are
incompatible"));

Me thinks this contradicts your description of the --group option
in the man page. I don't see why such a restriction would make
sense, what incompatibility are you trying to avoid here? Maybe
we need another submodule-specific setting to tell update what
groups to use inside that submodule?

So you want something like
    "In the top level respect the groups, but recursively get all of them"?

My thinking is that groups are implying recursive, whereas recursive implies
"all groups", so a git clone --group <half-the-submodules> --recursive
makes not much sense to me as it begs the question, what does --recursive
mean? Probably recurse into all submodules which are implied by the group
<half-the-submodules>. And then get all the nested submodules. But in case
you use the grouping feature, you could just mark the nested submodules with
groups, too?