patch, 4 partsSubmodule Groups

This applies on top of the updated sb/submodule-init branch.

Why? ==== Consider having a real large software project in Git with each component in a submodule (such as an operating system, Android, Debian, Fedora, no toy OS such as https://github.com/gittup/gittup as that doesn't quite demonstrate the scale of the problem).

If you have lots of submodules, you probably don't need all of them at once, but you have functional units. Some submodules are absolutely required, some are optional and only for very specific purposes.

This patch series adds meaning to a "groups" field in the .gitmodules file.

So you could have a .gitmodules file such as:

[submodule "gcc"]
        path = gcc
        url = git://...
        groups = default
        groups = devel
[submodule "linux"]
        path = linux
        url = git://...
        groups = default
[submodule "nethack"]
        path = nethack
        url = git://...
        groups = optional
        groups = games

and by this series you can work on an arbitrary subgroup of these submodules such
using these commands:
 
    git submodule add --group default --group devel git://... ..
    # will add a submodule, adding 2 submodule
    # groups to its entry in .gitmodule (default and devel)
    
    
    git submodule update --groups
    # All submodules as selected by groups (submodule.groups in .git/config)
    # will be initialized if they are not, before updating.
    
    git clone --group default,default2 --group devel git://...
    # will clone the superproject and recursively
    # checkout any submodule being in at least one of the groups (default, default2, devel)

Changes to v1 =============

* All entries are stored as separate lines (both in .git/config as well as in
  the .gitmodules file)

* No harm is done to `init` as it is implied by `update` now. :)

* I tried to keep it as simple as possible (update and clone being the minimal set
  of supported commands required, `add --group` being syntactic sugar for editing
  the .gitmodules file.

This is also available at https://github.com/stefanbeller/git/tree/submodule-groups-v2

Thanks, Stefan

Stefan Beller (4):
  git submodule: Teach add to accept --group
  submodule-config: keep groups around
  submodule update: Initialize all group-selected submodules by default
  builtin/clone: support submodule groups

 Documentation/git-clone.txt     |  13 +++++
 Documentation/git-submodule.txt |   8 ++-
 builtin/clone.c                 |  46 +++++++++++++++--
 builtin/submodule--helper.c     |  30 ++++++++++-
 git-submodule.sh                |  15 ++++++
 submodule-config.c              |  13 +++++
 submodule-config.h              |   1 +
 t/t7400-submodule-basic.sh      | 112 ++++++++++++++++++++++++++++++++++++++++
 8 files changed, 233 insertions(+), 5 deletions(-)

When adding new submodules, you can specify the group(s) the submodule belongs to by giving one or more --group arguments. This will record each group in the .gitmodules file as a value of the key "submodule.$NAME.group".

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 Documentation/git-submodule.txt |  8 +++++++-
 git-submodule.sh                | 15 +++++++++++++++
 t/t7400-submodule-basic.sh      | 32 ++++++++++++++++++++++++++++++++
 3 files changed, 54 insertions(+), 1 deletion(-)

Stefan Beller <sbeller@google.com> writes:

This can just be called $groups in the context of this script. I do not foresee we would be planning to deal with other kinds of groups here.

You would want to accept "--group=<name>" as well, just like existing --reference and --depth do. It won't be much more code, and when you move to C (hence parse_options) you'd get it for free anyway.

I do not quite understand the choice of ';' here. If and only if you _must_ accept multi-word name that has spaces in between as a group name, the above construct may make sense, but I do not think we have such requirement. Why not separate with $IFS letters just like any other normal list managed in shell scripts do? Is there anything special about names of submodule groups?

On Wed, Jan 20, 2016 at 1:18 PM, Junio C Hamano <gitster@pobox.com> wrote:

ok, done.

I am not sure, if I will to move `add` to C any time soon. Sure I desire less shell and more C[1], but I'd think my time could be spent better than just converting scripts to C. Sometimes I have to though, such as in the case of `init` as the the call out from C to shell is too ugly and the effort to do that is not that much less.

Just prior to writing this patch, I spent a good amount of time understanding the error message handling in the parts of the shell code, where the $IFS is used, so my mind was deceived to imitate that.

I agree, we can just use a space as separator here.

Thanks, Stefan

[1] Reason why I have such a disdain to shell is that I do not understand nearly as much as C. I am self-taught in both C and shell, so I do not claim to be perfect. However C is a small but powerful language. After reading "The C Programming Language" by Kernighan and Ritchie I do not think I found a thing that was really unknown to me. However even plain posix-shell is complex enough, that there is no such thing as a compact book to read to understand all its concepts IMHO.

Stefan Beller <sbeller@google.com> writes:

You can do so in less time than you spent making the above 5-line excuse. It won't be much more code, and it is not ugly at all.

	--group=*)
        	group=$group ${1#--group=} ;;

or something, right?

On Wed, Jan 20, 2016 at 4:08 PM, Junio C Hamano <gitster@pobox.com> wrote:

Right, that's what I have here now. I should have said that.

I was not trying to excuse anything, but more explaining the situation when reading the second sentence. (You seem to assume I'd want to rewrite all the shell scripts. Which I am not)

Stefan Beller <sbeller@google.com> writes:

I think it is a reasonable way to spend your time by not rewriting everything in C but the parts the matter the most. But it makes it even more important to do the part that will be left in shell for a longer time in such a way that will be helpful to the end users (by accepting both "--group name" and "--group=name", in the codepath under discussion).

We need the submodule groups in a later patch.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 submodule-config.c | 13 +++++++++++++
 submodule-config.h |  1 +
 2 files changed, 14 insertions(+)

Stefan Beller <sbeller@google.com> writes:

You are allocating entry->config->groups itself in the in the hunk around ll.327, so that also need to be freed, I would think.

Is there a case where you need to enumerate and show the groups a submodule belongs to to the end users? Using string_list_insert() to manage this list would mean you will lose the original ordering you saw the list of groups in their .gitmodules files, which may or may not matter. I'd assume that the ordering should not matter, but that is something the user may want to see documented.

On Wed, Jan 20, 2016 at 1:23 PM, Junio C Hamano <gitster@pobox.com> wrote:

done

In this patch series we only need to check the items of that list one by one, no need for a sorted version, so I'll change it to be unsorted and document that.

Stefan Beller <sbeller@google.com> writes:

I actually was going to suggest keeping the sorted one. The order may be changed from the order specified in the file, but the sorted variant has one very desirable property, namely de-duping, that the code that uses this data structure may find useful.

All submodules which are selected via submodule groups ("submodule.groups") are initialized if they were not initialized before updating the submodules.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 builtin/submodule--helper.c | 30 +++++++++++++++++++++++++++++-
 t/t7400-submodule-basic.sh  | 26 ++++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 1 deletion(-)

Stefan Beller <sbeller@google.com> writes:

Again, do we need a field named submodule_groups in a struct whose name already makes it clear this is about submodule?

More importantly.

If this were used to implement "there are various groups defined, and the user tells us that submodules in this and that groups are to be automatically initialized", then naming the field with a name that is more specific than just "groups" makes tons of sense, but even in such a case, the best adjective to clarify what kind of "group" this field is about is not "this is a submodule group".

The answer I would give to a question "what kind of group this field is about?" would be "this is an auto-init group", so I'd have that 'auto-init' ness somewhere in its name.

Likewise. The significance of the membership in this group is totally unclear from this variable name. I read this boolean to be keeping track of "is this submodule to be auto initialized"?

On Wed, Jan 20, 2016 at 1:30 PM, Junio C Hamano <gitster@pobox.com> wrote:

So you mean something like `autoinit` would maybe already suffice. Which leads to another question if we want to extend the concept of these submodule groups a little bit, such that we also allow direct names in there, in .git/config we may have the configuration

    [submodule]
        groups = <groupname-as-annotated-in-.gitmodules>
        groups = <explicit-submodule>

such that `git clone --group=default --group=mySingleSubmodule ....` would work.

Of course then the --group option would need to be named differently in git clone and probably the submodule.groups should also be named differently.

However: At this point in time we only care about auto-initing submodules to get submodules somewhat easier to handle when having lots of them. Maybe we also want to add other features to these "groups" concept, e.g. all submodules of one groups should have the "(force-)checkout" update strategy. If the submodule consists of binaries only, this would make lots of sense to me.

So it is not yet clear to me if we want to extend the grouping feature later on for other things, which is why I named it by its concept. A group can be used for different purposes, where as "all submodules having the same auto-init-tag can be treated the same using one update strategy" just adds to user confusion, hence I'd think telling the user about groups is the right thing to do?

In that part of the code it makes sense to rename it to auto_init, I'd guess.

Stefan Beller <sbeller@google.com> writes:

If you have (and I wish we have, but I do not seem to be able to come up with myself offhand) a shorter and more intuitive word than auto-init, that may be preferred, but basically yes. I'd call that autoinit_group or something, as you might want to come up with ways other than using groups to specify which ones are to be automatically initialized.

Come to think of it, I think we should drop "auto" from the name.

The only reason we think of this as "auto" is because we know that historically "clone" was purely about cloning and anything submodule related had to be done as a separate step, and this series makes the second step a part of the "clone" proper. But from the point of the end user, who specifies that "init" to be done for the specified set of submodules, there is no longer anything "auto" about it. We are doing what the user asked us to do; "auto" is what we do without explicitly being told, but the user is telling as explicitly which modules to work on with the option.

I am not sure what you mean by the above snippet.  With

	[submodule "foo"]
        	group = default
	[submodule "bar"]
        	group = optional
	[submodule "baz"]
        	group = optional
                path = sub/baz

it would be nice if you can say "modules in the default group", "modules in the default group and the module 'bar'", "modules at path sub/baz", etc.

Am I repeating more or less the same thing as you wanted to say with the above? If so, yes, I do think there should be a uniform syntax that lets users specify set of modules via different mechanisms.

I do think --group option is a mistake, as you are only saying "please give a name of a group to this option" without hinting how the modules in the specified group are to be treated differently, or more importantly, the option is about submodules.

    Side note: this is a common delusion developers fall into
    thinking that the feature they currently working on is the most
    important thing in the world.  In the context of "clone", there
    is no reason to expect that "groups of submodules" is any
    special than groups of other things.  For the same reason, I
    think --init=<group> is a mistake, as it is not clear from the
    option name that we are initializing submodules in the context
    of "clone".

So perhaps --init-module.

Once we establish a uniform convention for specifying a group of submodules is by giving the names of groups, the "group" ness of the option argument becomes less and less important, as that would be implicitly known by users. "clone --init=A" would be more understandable than "clone --group=A", as everybody would know A is naming a set of submodules either way, but the latter does not say what will happen to the chosen modules.

There could be other ways to specify the modules, and as long as we can come up with the "uniform convention for specifying a group of submodules", "clone --init-module=$X --init-module=$Y", would be understandable by the users when $X specifies the modules by their group name and $Y specifies another set of modules by something else, e.g. their names or paths to them.

    A strawman.  You can pass (1) the path to a submodule, (2) you
    can pass a colon followed by the name of a submodule, or (3) you
    can pass an asterisk followed by the name of a group.  (1) and
    (2) specifies a single submodule, (3) specifies the submodules
    that belong to the group.  I.e.

    $ git clone --init-module='*default' --init-module=sub/module

    would be a way to say "clone and then initialize the submodule
    at path sub/module and also those in the default group.

    This strawman makes "path" the default way, merely because many
    subcommands of "git submodule" already specify which submodule
    to operate on by taking paths arguments, and '*' prefix as the
    sign to specify by a group, as an asterisk looks like specifying
    multiple things.  ':' is just another prefix that is unlikely to
    be in a pathname.

    This is merely an illustration of the kind of syntax that can be
    used to name a set of modules using different ways.  I am sure
    people can come up with a different and better syntax, but the
    point of this illustration is not the exact syntax but showing
    that a way to uniformly specify a set of modules to operate on
    would allow us not having to worry about making 'groups' any
    special.

I however do not see why you think "submodule.group" needs to be spelled differently (it should be "submodule.$name.group", though). The 'group' is merely a convenient way to name and choose a set of modules. The name of the operation, i.e. what is to be done to the chosen modules, should be orthogonal, so I do not think you should have "submodule.autoinitgroup" or somesuch.

"clone" and other commands may want to gain use of some mechanism to specify a set of modules, and 'group' is one of the mechanisms to do so. Also "clone" and other commands may want to gain features to do different things to such sets of modules. So there are two orthogonal axes.

Which one is more pleasant to use from the end-user's point of view?

 (1) options are named after _how_ you specify the submodules:

     $ clone --init-group=A --init-name=B --init-path=C

     and what is done to them is implicit.

 (2) options are named after _what_ is done to them:

     $ clone --init-module=A --init-module=\*B --init-module=/C

     and how the set is specified is implicit to the syntax.

And more importantly, which one is more extensible in the future when you want to add more features that work on multiple but not all submodules?

	clone --init-module='*A' --distim-module=':B'

would be one clear way to say that modules in group A would be inited, while the submodule B would be distimmed. What would an equivalent of it look like in your "clone --group=A --name=B" world? Wouldn't the design along the lines of (2) above be much better?

On Wed, Jan 20, 2016 at 8:40 PM, Junio C Hamano <gitster@pobox.com> wrote:

Yes, we have the same vision here. However I wonder if we really need syntax to differentiate between groups, names and paths. At least for groups we can make it a requirement to have the same identifier as another name or path.

Technically it is possible for names and paths to mix up such as
         [submodule "bar"]
                 path = baz
         [submodule "baz"]
                 path = bar

such that we would need to have a way to tell apart if the user means the name or path. This could be solved implicit by some sort of precedence (If there is a name then take it by name, otherwise take the path.)

I fell into that trap, sure.

Why not spell it out completely (--init-submodule)? Then we don't have to add to the glossary that a module is the same as a submodule. (Could there be other modules such as co-modules, which happen to be not in the tree, but in "../sibling" relative to the root of the repository?)

So keep the "groups" in .gitmodules, but ban the name from clone options. In the .gitmodules file we could also name it "set" just as you used it in that paragraph.

ok.

But it could be in a pathname. Another strawman:

    You can pass either (1) the path (2) or the name (3) or the group
    of submodules to --init-submodule if there are no collisions in the
    three name spaces. When there are namespaces, you need to be
    explicit, such as --init-submodule=path:foo:baz.
    Note that "path:" is the selector which of the three namespaces
    we mean and "foo:baz" is a path asking for pain.

ok

But not just at the time of cloning. If later upstream decides that group=default includes another submodule, this submodule should be initialized and fetched in the next call to "submodule update", such that downstream has a working repository.

This idea makes groups more than just a temporary collection for init-purposes, but we need to store the selection.

"submodule.$name.group" to be found in .gitmodules, maybe overwritten in .git/config tells for each submodule its memberships of groups.

"submodule.group" should be found in .git/config only, to tell some time after cloning which group selection was made, such that we can check if new submodules need to be initialized (or even automatically removed).

    "git submodule update" may initialze and fetch new modules if the
    .gitmodules file changed their view of what "default" is.

I agree there.

I do think (2) is easier, unless the syntax is crazy or counterintuitive.

I agree on (2) being better.

However as pointed out, we want more than to just init submodules at time of clone, so it would be a

    clone --init-module=A --init-module=\*B --init-module=/C \
        --and-remember-to-keep-track-of=A \
        --and-remember-to-keep-track-of=\*B \
        --and-remember-to-keep-track-of=/C

I am hoping we can put that in shorter options, such as

    clone --init-module=A --init-module=\*B --init-module=/C \
        --remember-init-for-tracking

whereas:

    --remember-init-for-tracking: Submodule groups which are given to clone
        will be remembered, such that each invocation of "update" will make sure
        that group is fully there, i.e. new submodules in the group
will be initialized
        before updating.

Thanks, Stefan

Stefan Beller <sbeller@google.com> writes:

Ahh, it wasn't clear that was what you were trying to do. If that is the case, then ...

... I do not agree that the name of "submodule.group" variable should be neutral to the operation that is done to the groups of submodules named by the variable at all.

You are recording the criteria to choose a set of submodules there in the configuration, with a plan to keep doing something to them (e.g. "they will never be left uninitialized, i.e. initialied if a new submodule is added to the set"). The name of the configuration must have something that tells what that "something" is, in order to (1) hint the purpose of specifying the group selection criteria there, and (2) allow different selection criteria to keep doning another kind of something to them and distinguish these two group selection criteria.

E.g. "submodule.autoInitialize = *default !:foo" may mean all the modules in the default group (now or added in the future) except the module with name 'foo' will be initialized as needed, while "submodule.autoDistim = *optional" may want to be defined to allow the system to automatically distim (whatever that operation is that will be added to Git in later versions) the modules in the optional group.

Is there a need for --no-remember-init-for-tracking? I do not think it would be useful.

When the upstream adds a new module and defines it to be part of the default group _after_ you cloned with --init set to 'default', and you do not need that new module, at that point you can tweak your submodule.autoInitialize definition to exclude that new module.

Tweaking submodule.autoInitialize definition to contain nothing after cloning with --init because you do not want the autoinit criteria kept in the resulting repository is merely a special case of that.

So I do not think --remember-init-for-tracking is necessary. Just make it _always_ on and be done with it, I would say.

Junio C Hamano <gitster@pobox.com> writes:

By the way, I forgot to say that we're basically in agreement on everything in your message that I didn't touch in my response. Also I forgot to say that I am happy to see that we are designing a new mechanism to name a set of modules in a convenient way so that the users can do things to them without hassle.

;-)

When invocing clone with the groups option, the repository will be configured to the specified groups. This has implications for the submodule commands, such as update.

Having groups configured will imply init for all uninitialized submodules belonging to at least one of the configured groups, such that new submodules in the group are initialized automatically.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 Documentation/git-clone.txt | 13 +++++++++++
 builtin/clone.c             | 46 +++++++++++++++++++++++++++++++++++---
 t/t7400-submodule-basic.sh  | 54 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 110 insertions(+), 3 deletions(-)

Stefan Beller <sbeller@google.com> writes:

This is misnamed but in an opposite way as the variables and fields in previous patches. "Clone" is not primarily about submodules, so a plain "group" in the context of "clone" does not hint that this option is about submodules. It also does not tell the user anything about what the parameter is for (i.e. there are submodules that are and are not in the group that is specified with this option. What special things happens to the ones in the group?)

I am guessing that it is about the auto-vivifying behaviour like 3/4 did, and if that is the case, perhaps "--init-submodule=<group>" or something like that?

Oh by the way, I think you meant this option to take an argument by reading the description (you say "part of the given groups"). Follow the example of existing option you see below in the context and make sure that the reader knows they have to give an argument to it.

    A tangent. I think it would be handy to have a way to name a
    single submodule and have the code treat as if there were a
    group that contains that single submodule already defined and
    the user specified that group.  Then this option (and any other
    option that takes a submodule group name) can instead take a
    submodule name and the user can expect a natural thing to happen.

Why not just do "--group A --group B" without "--group A,B"? I do not think you accept "git submodule add --group A,B" anyway [*1*], so doing "nice" things like this is just being inconsistent and does not help the users. And I do personally do not think we should make things consistent by accepting "--group A,B" everywhere.

[Footnote]

*1* Technically, your 'add --group' accepts A;B, I think, but then this ',' is still inconsistent with it ;-)

On 20.01.2016 04:34, Stefan Beller wrote:

On the quick I was unable to find the rationale why entries are now stored as separated lines compared to v1. I liked the comma-separated approach better as it's more compact.

Anyway, if it's only one group per line, I'd find it more fitting to call the entry "group" instead of "groups" as it will always refer to a single group only. Also that would better match the "--group" command line option naming for "submodule add".

However, if I'd read the single line "group = default" in a .gitmodules file, it wouldn't be immediately clear to me that "group" can appear multiple times per submodule. "groups = default" would me more hinting is this regard because the plural is used, but without reading the docs I'd assume multiple groups would be specified like "groups = default,devel".

Long story short, my personal favorite still would be 

[submodule "gcc"]
         groups = default,devel

followed by

[submodule "gcc"]
         group = default
         group = devel

On Thu, Jan 21, 2016 at 1:17 PM, Sebastian Schuberth <sschuberth@gmail.com> wrote:

IIUC the line oriented way is preferred as it is our standard. Do we have any other options stored as a comma separated list?

Makes sense to use singular then. However per discussion with Junio in [PATCH 3/4] submodule update: Initialize all group-selected submodules by default, we want to not name it "group", as it's unclear what a group is supposed to mean. What does a group do? which operations are supported?

So for git clone, we'd rather use "--init-submodule" which can be passed a name, path or group.

For storing that selection we'd go with "submodule.autoInitialize" in .git /config.

The third user visible place submodule.$NAME.group however can stay in that variable name as to point out the the concept of a submodule set/collection. The groups concept may be used in the future for more than initialzing submodules.

Instead of having a submodule -> set assignment, we could do it the other way round:

     [submodule "gcc"]
         ...

     [submodule-set "default"]
        submodule = gcc
        submodule = foo
        submodule = by/path/*

This may be more handy from our perspective (while designing it and writing the code), but I'd assume this is less useful for the user. How often does a user ask: "How many/Which submodules are in $GROUP" as opposed to "What about submodule foo, is that part of group $GROUP?"

As asked above, how many comma separated things do we have in git configs? I'd really not want to add more mental complexity to Git. As far as I remember we have rather double configs than one long line separated somehow. (The only thing that comes to mind is multiple remote urls for pushing)

Thanks, Stefan

Stefan Beller <sbeller@google.com> writes:

I suspect that we will end up needing to support both styles. The latter style is easier when you want to express a larger set as a collection of groups, e.g.

    [submodule "gcc"]
    	group = development-tools

    [submodule "emacs"]
    	group = editors

    [submodule "frotz"]
        group = games

    [submoduleGroup "default"]
        member = *development-tools
        member = *editors
        member = :frotz
	member = !*xyzzy

might be a way to say "the default group includes everything in the 'development-tools' or 'editors' group, plus 'frotz' module, but do not include modules in the xyzzy group" or something like that.

Junio C Hamano <gitster@pobox.com> writes:

I forgot to say that I personally do not think we need to support such synthetic groups from day one.

On Thu, Jan 21, 2016 at 2:18 PM, Junio C Hamano <gitster@pobox.com> wrote:

I think having both is bad as it may contradict each other? What is supposed to happen here:

     [submodule "frotz"]
         group = default

     [submoduleGroup "default"]
         member = !:frotz

Initially I disliked your proposal of : and * to indicate name and groups, but the example you gave is very clear and understandable on the syntax level.

So groups of groups, we discovered recursion today. :) Having this discussion makes me realize, the groups handling logic will be more complex than I anticipated for the first RFC patch series.

The two basic questions the logic has to answer is:
 * Give me all the submodules that fit these specifiers (i.e. the
    --init-submodules collection from clone)
 * Given submodule X, does it fit the specifier ( a new uninitialized
    submodule during a later update)

The second question can be answered by answering the first question and then checking if X is in the set. However that may be not the most efficient solution.

Stefan Beller <sbeller@google.com> writes:

What is supposed to happen is that you will write code to diagnose this as an error, and give helpful error message to the user.

That is why I think we do not have to have a very complex group logic from day one.

Yes.

On Thu, Jan 21, 2016 at 10:56 PM, Stefan Beller <sbeller@google.com> wrote:

Out of my head I cannot think of any. But that shouldn't mean we cannot introduce such comma separated list if it makes sense.

How about calling it "label" instead of "group"? IMO with the word "label" it's more clean that a single submodule can have multiple labels, as the concept of labels is familiar to the user already from applications like Firefox (bookmarks), Google Mail, Mac OS X Finder (files) etc.

In your example you're now introducing "set" as a new term. Shouldn't this better be "submodule-group" then? I actually like this idea quite a bit as it completely solves the problem about being clear that a submodule can belong to mutiple groups.

True, but for answering that question a user would not look at .gitmodules, but run a command, and the implementation of that command would completely hide that complexity from the user.

I don't think it can get much worse anyway ;-)

I believe so, too. But I'd see the introduction of comma-separated values as an exit-strategy to that. More settings could make use of that in the future, then.