{"thread":{"id":"41220","subject":"[PATCH 0/4] Submodule Groups","startedAt":"2016-01-20T03:34:36Z","lastAt":"2016-01-22T08:55:07Z","messageCount":27,"participants":["Stefan Beller","Junio C Hamano","Sebastian Schuberth"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"276406","messageId":"1453260880-628-1-git-send-email-sbeller@google.com","threadId":"41220","inReplyTo":null,"subject":"[PATCH 0/4] Submodule Groups","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-01-20T03:34:36Z","receivedAt":"2016-01-20T03:34:36Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This applies on top of the updated sb/submodule-init branch.\n\nWhy?\n====\nConsider having a real large software project in Git with each component\nin a submodule (such as an operating system, Android, Debian, Fedora,\nno toy OS such as https://github.com/gittup/gittup as that doesn't quite\ndemonstrate the scale of the problem).\n\nIf you have lots of submodules, you probably don't need all of them at once,\nbut you have functional units. Some submodules are absolutely required,\nsome are optional and only for very specific purposes.\n\nThis patch series adds meaning to a \"groups\" field in the .gitmodules file.\n\nSo you could have a .gitmodules file such as:\n\n[submodule \"gcc\"]\n path = gcc\n url = git://...\n groups = default\n groups = devel\n[submodule \"linux\"]\n path = linux\n url = git://...\n groups = default\n[submodule \"nethack\"]\n path = nethack\n url = git://...\n groups = optional\n groups = games\n\nand by this series you can work on an arbitrary subgroup of these submodules such\nusing these commands:\n \n git submodule add --group default --group devel git://... ..\n # will add a submodule, adding 2 submodule\n # groups to its entry in .gitmodule (default and devel)\n \n \n git submodule update --groups\n # All submodules as selected by groups (submodule.groups in .git/config)\n # will be initialized if they are not, before updating.\n \n git clone --group default,default2 --group devel git://...\n # will clone the superproject and recursively\n # checkout any submodule being in at least one of the groups (default, default2, devel)\n\n\nChanges to v1\n=============\n\n* All entries are stored as separate lines (both in .git/config as well as in\n the .gitmodules file)\n\n* No harm is done to `init` as it is implied by `update` now. :)\n\n* I tried to keep it as simple as possible (update and clone being the minimal set\n of supported commands required, `add --group` being syntactic sugar for editing\n the .gitmodules file.\n\nThis is also available at https://github.com/stefanbeller/git/tree/submodule-groups-v2\n\nThanks,\nStefan\n\n\nStefan Beller (4):\n git submodule: Teach add to accept --group\n submodule-config: keep groups around\n submodule update: Initialize all group-selected submodules by default\n builtin/clone: support submodule groups\n\n Documentation/git-clone.txt | 13 +++++\n Documentation/git-submodule.txt | 8 ++-\n builtin/clone.c | 46 +++++++++++++++--\n builtin/submodule--helper.c | 30 ++++++++++-\n git-submodule.sh | 15 ++++++\n submodule-config.c | 13 +++++\n submodule-config.h | 1 +\n t/t7400-submodule-basic.sh | 112 ++++++++++++++++++++++++++++++++++++++++\n 8 files changed, 233 insertions(+), 5 deletions(-)\n\n-- \n2.7.0.rc0.41.g89994f2.dirty\n"},{"id":"276408","messageId":"1453260880-628-2-git-send-email-sbeller@google.com","threadId":"41220","inReplyTo":"1453260880-628-1-git-send-email-sbeller@google.com","subject":"[PATCH 1/4] git submodule: Teach add to accept --group","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-01-20T03:34:37Z","receivedAt":"2016-01-20T03:34:37Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"When adding new submodules, you can specify the\ngroup(s) the submodule belongs to by giving one or more\n--group arguments. This will record each group in the\n.gitmodules file as a value of the key\n\"submodule.$NAME.group\".\n\nSigned-off-by: Stefan Beller \n---\n Documentation/git-submodule.txt | 8 +++++++-\n git-submodule.sh | 15 +++++++++++++++\n t/t7400-submodule-basic.sh | 32 ++++++++++++++++++++++++++++++++\n 3 files changed, 54 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt\nindex 13adebf..135c739 100644\n--- a/Documentation/git-submodule.txt\n+++ b/Documentation/git-submodule.txt\n@@ -9,7 +9,7 @@ git-submodule - Initialize, update or inspect submodules\n SYNOPSIS\n --------\n [verse]\n-'git submodule' [--quiet] add [-b ] [-f|--force] [--name ]\n+'git submodule' [--quiet] add [-b ] [-f|--force] [-g ][--name ]\n \t [--reference ] [--depth ] [--] []\n 'git submodule' [--quiet] status [--cached] [--recursive] [--] [...]\n 'git submodule' [--quiet] init [--] [...]\n@@ -59,6 +59,9 @@ instead of treating the other project as a submodule. Directories\n that come from both projects can be cloned and checked out as a whole\n if you choose to go that route.\n \n+If you manage a large set of submodules, but do not require all of them\n+to be checked out, you should look into the submodule groups feature.\n+\n COMMANDS\n --------\n add::\n@@ -101,6 +104,9 @@ is the superproject and submodule repositories will be kept\n together in the same relative location, and only the\n superproject's URL needs to be provided: git-submodule will correctly\n locate the submodule using the relative URL in .gitmodules.\n++\n+If at least one group argument was given, all groups are recorded in the\n+.gitmodules file in the groups field.\n \n status::\n \tShow the status of the submodules. This will print the SHA-1 of the\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 6fce0dc..ab0f209 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -130,6 +130,7 @@ cmd_add()\n {\n \t# parse $args after \"submodule ... add\".\n \treference_path=\n+\tsubmodule_groups=\n \twhile test $# -ne 0\n \tdo\n \t\tcase \"$1\" in\n@@ -165,6 +166,10 @@ cmd_add()\n \t\t--depth=*)\n \t\t\tdepth=$1\n \t\t\t;;\n+\t\t-g|--group)\n+\t\t\tsubmodule_groups=${submodule_groups:+${submodule_groups};}\"$2\"\n+\t\t\tshift\n+\t\t\t;;\n \t\t--)\n \t\t\tshift\n \t\t\tbreak\n@@ -292,6 +297,16 @@ Use -f if you really want to add it.\" >&2\n \n \tgit config -f .gitmodules submodule.\"$sm_name\".path \"$sm_path\" &&\n \tgit config -f .gitmodules submodule.\"$sm_name\".url \"$repo\" &&\n+\tif test -n \"$submodule_groups\"\n+\tthen\n+\t\tOIFS=$IFS\n+\t\tIFS=';'\n+\t\tfor group in $submodule_groups\n+\t\tdo\n+\t\t\tgit config --add -f .gitmodules submodule.\"$sm_name\".group \"${group}\"\n+\t\tdone\n+\t\tIFS=$OIFS\n+\tfi &&\n \tif test -n \"$branch\"\n \tthen\n \t\tgit config -f .gitmodules submodule.\"$sm_name\".branch \"$branch\"\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex 5991e3c..b468278 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -986,6 +986,7 @@ test_expect_success 'submodule with UTF-8 name' '\n '\n \n test_expect_success 'submodule add clone shallow submodule' '\n+\ttest_when_finished \"rm -rf super\" &&\n \tmkdir super &&\n \tpwd=$(pwd) &&\n \t(\n@@ -999,5 +1000,36 @@ test_expect_success 'submodule add clone shallow submodule' '\n \t)\n '\n \n+test_expect_success 'submodule add records a group' '\n+\ttest_when_finished \"rm -rf super\" &&\n+\tmkdir super &&\n+\tpwd=$(pwd) &&\n+\t(\n+\t\tcd super &&\n+\t\tgit init &&\n+\t\tgit submodule add --group groupA file://\"$pwd\"/example2 submodule &&\n+\t\tgit config -f .gitmodules submodule.\"submodule\".group >actual &&\n+\t\techo groupA >expected &&\n+\t\ttest_cmp expected actual\n+\t)\n+'\n+\n+cat >expected <<-EOF\n+groupA\n+groupB\n+EOF\n+\n+test_expect_success 'submodule add records groups' '\n+\ttest_when_finished \"rm -rf super\" &&\n+\tmkdir super &&\n+\tpwd=$(pwd) &&\n+\t(\n+\t\tcd super &&\n+\t\tgit init &&\n+\t\tgit submodule add --group groupA -g groupB file://\"$pwd\"/example2 submodule &&\n+\t\tgit config --get-all -f .gitmodules submodule.\"submodule\".group >../actual\n+\t) &&\n+\ttest_cmp expected actual\n+'\n \n test_done\n-- \n2.7.0.rc0.41.g89994f2.dirty\n"},{"id":"276407","messageId":"1453260880-628-3-git-send-email-sbeller@google.com","threadId":"41220","inReplyTo":"1453260880-628-1-git-send-email-sbeller@google.com","subject":"[PATCH 2/4] submodule-config: keep groups around","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-01-20T03:34:38Z","receivedAt":"2016-01-20T03:34:38Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"We need the submodule groups in a later patch.\n\nSigned-off-by: Stefan Beller \n---\n submodule-config.c | 13 +++++++++++++\n submodule-config.h | 1 +\n 2 files changed, 14 insertions(+)\n\ndiff --git a/submodule-config.c b/submodule-config.c\nindex a32259e..b5453d0 100644\n--- a/submodule-config.c\n+++ b/submodule-config.c\n@@ -60,6 +60,8 @@ static void free_one_config(struct submodule_entry *entry)\n {\n \tfree((void *) entry->config->path);\n \tfree((void *) entry->config->name);\n+\tif (entry->config->groups)\n+\t\tstring_list_clear(entry->config->groups, 0);\n \tfree(entry->config);\n }\n \n@@ -184,6 +186,7 @@ static struct submodule *lookup_or_create_by_name(struct submodule_cache *cache,\n \tsubmodule->update = NULL;\n \tsubmodule->fetch_recurse = RECURSE_SUBMODULES_NONE;\n \tsubmodule->ignore = NULL;\n+\tsubmodule->groups = NULL;\n \n \thashcpy(submodule->gitmodules_sha1, gitmodules_sha1);\n \n@@ -324,6 +327,16 @@ static int parse_specific_submodule_config(const char *subsection, int subsectio\n \t\t\tfree((void *) submodule->update);\n \t\t\tsubmodule->update = xstrdup(value);\n \t\t}\n+\t} else if (!strcmp(key, \"group\")) {\n+\t\tif (!value)\n+\t\t\tret = config_error_nonbool(var);\n+\t\telse {\n+\t\t\tif (!submodule->groups) {\n+\t\t\t\tsubmodule->groups = xmalloc(sizeof(*submodule->groups));\n+\t\t\t\tstring_list_init(submodule->groups, 1);\n+\t\t\t}\n+\t\t\tstring_list_insert(submodule->groups, value);\n+\t\t}\n \t}\n \n \treturn ret;\ndiff --git a/submodule-config.h b/submodule-config.h\nindex d9bbf9a..332f5be 100644\n--- a/submodule-config.h\n+++ b/submodule-config.h\n@@ -17,6 +17,7 @@ struct submodule {\n \tconst char *update;\n \t/* the sha1 blob id of the responsible .gitmodules file */\n \tunsigned char gitmodules_sha1[20];\n+\tstruct string_list *groups;\n };\n \n int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg);\n-- \n2.7.0.rc0.41.g89994f2.dirty\n"},{"id":"276409","messageId":"1453260880-628-4-git-send-email-sbeller@google.com","threadId":"41220","inReplyTo":"1453260880-628-1-git-send-email-sbeller@google.com","subject":"[PATCH 3/4] submodule update: Initialize all group-selected submodules by default","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-01-20T03:34:39Z","receivedAt":"2016-01-20T03:34:39Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"All submodules which are selected via submodule groups\n(\"submodule.groups\") are initialized if they were not initialized\nbefore updating the submodules.\n\nSigned-off-by: Stefan Beller \n---\n builtin/submodule--helper.c | 30 +++++++++++++++++++++++++++++-\n t/t7400-submodule-basic.sh | 26 ++++++++++++++++++++++++++\n 2 files changed, 55 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 4684f16..def382e 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -577,6 +577,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \n struct submodule_update_clone {\n \t/* states */\n+\tstruct string_list *submodule_groups;\n \tint count;\n \tint print_unmatched;\n \t/* configuration */\n@@ -590,7 +591,7 @@ struct submodule_update_clone {\n \tstruct string_list projectlines;\n \tstruct pathspec pathspec;\n };\n-#define SUBMODULE_UPDATE_CLONE_INIT {0, 0, 0, NULL, NULL, NULL, NULL, NULL, MODULE_LIST_INIT, STRING_LIST_INIT_DUP}\n+#define SUBMODULE_UPDATE_CLONE_INIT {NULL, 0, 0, 0, NULL, NULL, NULL, NULL, NULL, MODULE_LIST_INIT, STRING_LIST_INIT_DUP}\n \n static void fill_clone_command(struct child_process *cp, int quiet,\n \t\t\t const char *prefix, const char *path,\n@@ -633,6 +634,7 @@ static int update_clone_get_next_task(struct child_process *cp,\n \t\tconst char *update_module = NULL;\n \t\tchar *url = NULL;\n \t\tint needs_cloning = 0;\n+\t\tint in_submodule_groups = 0;\n \n \t\tif (ce_stage(ce)) {\n \t\t\tif (pp->recursive_prefix)\n@@ -676,6 +678,22 @@ static int update_clone_get_next_task(struct child_process *cp,\n \t\tstrbuf_reset(&sb);\n \t\tstrbuf_addf(&sb, \"submodule.%s.url\", sub->name);\n \t\tgit_config_get_string(sb.buf, &url);\n+\t\tif (pp->submodule_groups && sub->groups){\n+\t\t\tstruct string_list_item *item;\n+\t\t\tfor_each_string_list_item(item, sub->groups) {\n+\t\t\t\tif (string_list_has_string(\n+\t\t\t\t pp->submodule_groups, item->string)) {\n+\t\t\t\t\tin_submodule_groups = 1;\n+\t\t\t\t\tbreak;\n+\t\t\t\t}\n+\t\t\t}\n+\t\t\tif (in_submodule_groups) {\n+\t\t\t\tif (!url) {\n+\t\t\t\t\tinit_submodule(sub->name, pp->prefix, pp->quiet);\n+\t\t\t\t\turl = xstrdup(sub->url);\n+\t\t\t\t}\n+\t\t\t}\n+\t\t}\n \t\tif (!url) {\n \t\t\t/*\n \t\t\t * Only mention uninitialized submodules when its\n@@ -687,6 +705,7 @@ static int update_clone_get_next_task(struct child_process *cp,\n \t\t\tcontinue;\n \t\t}\n \n+\n \t\tstrbuf_reset(&sb);\n \t\tstrbuf_addf(&sb, \"%s/.git\", ce->name);\n \t\tneeds_cloning = !file_exists(sb.buf);\n@@ -742,6 +761,7 @@ static int update_clone_task_finished(int result,\n static int update_clone(int argc, const char **argv, const char *prefix)\n {\n \tint max_jobs = -1;\n+\tconst struct string_list *list;\n \tstruct string_list_item *item;\n \tstruct submodule_update_clone pp = SUBMODULE_UPDATE_CLONE_INIT;\n \n@@ -786,6 +806,14 @@ static int update_clone(int argc, const char **argv, const char *prefix)\n \t/* Overlay the parsed .gitmodules file with .git/config */\n \tgit_config(git_submodule_config, NULL);\n \n+\tlist = git_config_get_value_multi(\"submodule.groups\");\n+\tif (list) {\n+\t\tpp.submodule_groups = xmalloc(sizeof(*pp.submodule_groups));\n+\t\tstring_list_init(pp.submodule_groups, 1);\n+\t\tfor_each_string_list_item(item, list)\n+\t\t\tstring_list_insert(pp.submodule_groups, item->string);\n+\t}\n+\n \tif (max_jobs < 0)\n \t\tmax_jobs = config_parallel_submodules();\n \tif (max_jobs < 0)\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex b468278..e6a008c 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -1032,4 +1032,30 @@ test_expect_success 'submodule add records groups' '\n \ttest_cmp expected actual\n '\n \n+cat < expected\n+submodule\n+-submodule1\n+EOF\n+\n+test_expect_success 'submodule update works with groups implied' '\n+\ttest_when_finished \"rm -rf super super_clone\" &&\n+\tmkdir super &&\n+\tpwd=$(pwd) &&\n+\t(\n+\t\tcd super &&\n+\t\tgit init &&\n+\t\tgit submodule add --group groupA file://\"$pwd\"/example2 submodule &&\n+\t\tgit submodule add file://\"$pwd\"/example2 submodule1 &&\n+\t\tgit commit -a -m \"create repository with 2 submodules, one is in a group\"\n+\t) &&\n+\tgit clone super super_clone &&\n+\t(\n+\t\tcd super_clone &&\n+\t\tgit config submodule.groups groupA &&\n+\t\tgit submodule update &&\n+\t\tgit submodule status |cut -c1,42-52 | tr -d \" \" >../actual\n+\t) &&\n+\ttest_cmp actual expected\n+'\n+\n test_done\n-- \n2.7.0.rc0.41.g89994f2.dirty\n"},{"id":"276410","messageId":"1453260880-628-5-git-send-email-sbeller@google.com","threadId":"41220","inReplyTo":"1453260880-628-1-git-send-email-sbeller@google.com","subject":"[PATCH 4/4] builtin/clone: support submodule groups","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-01-20T03:34:40Z","receivedAt":"2016-01-20T03:34:40Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"When invocing clone with the groups option,\nthe repository will be configured to the specified groups.\nThis has implications for the submodule commands, such as\nupdate.\n\nHaving groups configured will imply init for all uninitialized\nsubmodules belonging to at least one of the configured groups,\nsuch that new submodules in the group are initialized\nautomatically.\n\nSigned-off-by: Stefan Beller \n---\n Documentation/git-clone.txt | 13 +++++++++++\n builtin/clone.c | 46 +++++++++++++++++++++++++++++++++++---\n t/t7400-submodule-basic.sh | 54 +++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 110 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\nindex 6db7b6d..685fb7c 100644\n--- a/Documentation/git-clone.txt\n+++ b/Documentation/git-clone.txt\n@@ -214,6 +214,19 @@ objects from the source repository into a pack in the cloned repository.\n \trepository does not have a worktree/checkout (i.e. if any of\n \t`--no-checkout`/`-n`, `--bare`, or `--mirror` is given)\n \n+--group::\n+\tAfter the clone is created, all submodules which are part of the\n+\tgiven groups are cloned. To specify multiple groups, you can either\n+\tgive the group argument multiple times or comma separate the groups.\n+\tThis option will be recorded in the `submodule.groups` config,\n+\twhich will affect the behavior of other submodule related commands,\n+\tsuch as `git submodule update`.\n+\tThis option implies recursive submodule checkout. If you don't\n+\twant to recurse into nested submodules, you need to specify\n+\t`--no-recursive`. The group selection will be passed on recursively,\n+\ti.e. if a submodule is cloned because of group membership, its\n+\tsubmodules will be cloned according to group membership, too.\n+\n --separate-git-dir=::\n \tInstead of placing the cloned repository where it is supposed\n \tto be, place the cloned repository at the specified directory,\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex b004fb4..72aea3a 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -51,6 +51,22 @@ static struct string_list option_config;\n static struct string_list option_reference;\n static int option_dissociate;\n static int max_jobs = -1;\n+static struct string_list submodule_groups;\n+\n+static int groups_cb(const struct option *opt, const char *arg, int unset)\n+{\n+\tstruct string_list_item *item;\n+\tstruct string_list sl = STRING_LIST_INIT_DUP;\n+\n+\tif (unset)\n+\t\treturn -1;\n+\n+\tstring_list_split(&sl, arg, ',', -1);\n+\tfor_each_string_list_item(item, &sl)\n+\t\tstring_list_append((struct string_list *)opt->value, item->string);\n+\n+\treturn 0;\n+}\n \n static struct option builtin_clone_options[] = {\n \tOPT__VERBOSITY(&option_verbosity),\n@@ -95,6 +111,8 @@ static struct option builtin_clone_options[] = {\n \t\t N_(\"separate git dir from working tree\")),\n \tOPT_STRING_LIST('c', \"config\", &option_config, N_(\"key=value\"),\n \t\t\tN_(\"set config inside the new repository\")),\n+\tOPT_CALLBACK('g', \"group\", &submodule_groups, N_(\"string\"),\n+\t\t\tN_(\"clone specific submodule groups\"), groups_cb),\n \tOPT_END()\n };\n \n@@ -723,9 +741,17 @@ static int checkout(void)\n \terr |= run_hook_le(NULL, \"post-checkout\", sha1_to_hex(null_sha1),\n \t\t\t sha1_to_hex(sha1), \"1\", NULL);\n \n-\tif (!err && option_recursive) {\n+\tif (err)\n+\t\tgoto out;\n+\n+\tif (option_recursive || submodule_groups.nr > 0) {\n \t\tstruct argv_array args = ARGV_ARRAY_INIT;\n-\t\targv_array_pushl(&args, \"submodule\", \"update\", \"--init\", \"--recursive\", NULL);\n+\t\targv_array_pushl(&args, \"submodule\", \"update\", NULL);\n+\n+\t\tif (option_recursive) {\n+\t\t\targv_array_pushf(&args, \"--init\");\n+\t\t\targv_array_pushf(&args, \"--recursive\");\n+\t\t}\n \n \t\tif (max_jobs != -1)\n \t\t\targv_array_pushf(&args, \"--jobs=%d\", max_jobs);\n@@ -733,7 +759,7 @@ static int checkout(void)\n \t\terr = run_command_v_opt(args.argv, RUN_GIT_CMD);\n \t\targv_array_clear(&args);\n \t}\n-\n+out:\n \treturn err;\n }\n \n@@ -867,6 +893,20 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\t\tdie(_(\"--bare and --separate-git-dir are incompatible.\"));\n \t\toption_no_checkout = 1;\n \t}\n+\tif (option_recursive == -1) {\n+\t\tif (submodule_groups.nr > 0)\n+\t\t\toption_recursive = 1; /* submodule groups implies recursive */\n+\t\telse\n+\t\t\toption_recursive = 0; /* preserve historical default */\n+\t}\n+\tif (submodule_groups.nr > 0) {\n+\t\tstruct string_list_item *item;\n+\t\tstruct strbuf sb = STRBUF_INIT;\n+\t\tfor_each_string_list_item(item, &submodule_groups) {\n+\t\t\tstrbuf_addf(&sb, \"submodule.groups=%s\", item->string);\n+\t\t\tstring_list_append(&option_config, strbuf_detach(&sb, 0));\n+\t\t}\n+\t}\n \n \tif (!option_origin)\n \t\toption_origin = \"origin\";\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex e6a008c..5ed5250 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -1058,4 +1058,58 @@ test_expect_success 'submodule update works with groups implied' '\n \ttest_cmp actual expected\n '\n \n+cat < expected\n+submodule\n+-submodule1\n+EOF\n+\n+test_expect_success 'clone --group works' '\n+\ttest_when_finished \"rm -rf super super_clone\" &&\n+\tmkdir super &&\n+\tpwd=$(pwd) &&\n+\t(\n+\t\tcd super &&\n+\t\tgit init &&\n+\t\tgit submodule add --group groupA file://\"$pwd\"/example2 submodule &&\n+\t\tgit submodule add file://\"$pwd\"/example2 submodule1 &&\n+\t\tgit commit -a -m \"create repository with 2 submodules, one is in a group\"\n+\t) &&\n+\tgit clone --group groupA super super_clone &&\n+\t(\n+\t\tcd super_clone &&\n+\t\tgit submodule status |cut -c1,42-52 | tr -d \" \" >../actual\n+\t) &&\n+\ttest_cmp actual expected\n+'\n+\n+cat < expected\n+-submodule1\n+submoduleA\n+submoduleB\n+submoduleC\n+-submoduleD\n+EOF\n+\n+test_expect_success 'clone with multiple groups works' '\n+\ttest_when_finished \"rm -rf super super_clone\" &&\n+\tmkdir super &&\n+\tpwd=$(pwd) &&\n+\t(\n+\t\tcd super &&\n+\t\tgit init &&\n+\t\tgit submodule add --group groupA file://\"$pwd\"/example2 submoduleA &&\n+\t\tgit submodule add --group groupB file://\"$pwd\"/example2 submoduleB &&\n+\t\tgit submodule add --group groupC file://\"$pwd\"/example2 submoduleC &&\n+\t\tgit submodule add --group groupD file://\"$pwd\"/example2 submoduleD &&\n+\t\tgit submodule add file://\"$pwd\"/example2 submodule1 &&\n+\t\tgit commit -a -m \"create repository with submodules groups\"\n+\t) &&\n+\tgit clone --group groupA,groupB --group groupC super super_clone &&\n+\t(\n+\t\tcd super_clone &&\n+\t\tgit submodule status |cut -c1,42-52 | tr -d \" \" >../actual\n+\t) &&\n+\ttest_cmp actual expected\n+'\n+\n test_done\n-- \n2.7.0.rc0.41.g89994f2.dirty\n"},{"id":"276453","messageId":"xmqq1t9c54qz.fsf@gitster.mtv.corp.google.com","threadId":"41220","inReplyTo":"1453260880-628-2-git-send-email-sbeller@google.com","subject":"Re: [PATCH 1/4] git submodule: Teach add to accept --group","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-20T21:18:44Z","receivedAt":"2016-01-20T21:18:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> diff --git a/git-submodule.sh b/git-submodule.sh\n> index 6fce0dc..ab0f209 100755\n> --- a/git-submodule.sh\n> +++ b/git-submodule.sh\n> @@ -130,6 +130,7 @@ cmd_add()\n> {\n> \t# parse $args after \"submodule ... add\".\n> \treference_path=\n> +\tsubmodule_groups=\n\nThis can just be called $groups in the context of this script. I do\nnot foresee we would be planning to deal with other kinds of groups\nhere.\n\n> \twhile test $# -ne 0\n> \tdo\n> \t\tcase \"$1\" in\n> @@ -165,6 +166,10 @@ cmd_add()\n> \t\t--depth=*)\n> \t\t\tdepth=$1\n> \t\t\t;;\n> +\t\t-g|--group)\n> +\t\t\tsubmodule_groups=${submodule_groups:+${submodule_groups};}\"$2\"\n> +\t\t\tshift\n> +\t\t\t;;\n\nYou would want to accept \"--group=\" as well, just like\nexisting --reference and --depth do. It won't be much more code,\nand when you move to C (hence parse_options) you'd get it for free\nanyway.\n\n> @@ -292,6 +297,16 @@ Use -f if you really want to add it.\" >&2\n> \n> \tgit config -f .gitmodules submodule.\"$sm_name\".path \"$sm_path\" &&\n> \tgit config -f .gitmodules submodule.\"$sm_name\".url \"$repo\" &&\n> +\tif test -n \"$submodule_groups\"\n> +\tthen\n> +\t\tOIFS=$IFS\n> +\t\tIFS=';'\n\nI do not quite understand the choice of ';' here. If and only if\nyou _must_ accept multi-word name that has spaces in between as a\ngroup name, the above construct may make sense, but I do not think\nwe have such requirement. Why not separate with $IFS letters just\nlike any other normal list managed in shell scripts do? Is there\nanything special about names of submodule groups?\n"},{"id":"276454","messageId":"xmqqwpr43pyz.fsf@gitster.mtv.corp.google.com","threadId":"41220","inReplyTo":"1453260880-628-3-git-send-email-sbeller@google.com","subject":"Re: [PATCH 2/4] submodule-config: keep groups around","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-20T21:23:16Z","receivedAt":"2016-01-20T21:23:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> We need the submodule groups in a later patch.\n>\n> Signed-off-by: Stefan Beller \n> ---\n> submodule-config.c | 13 +++++++++++++\n> submodule-config.h | 1 +\n> 2 files changed, 14 insertions(+)\n>\n> diff --git a/submodule-config.c b/submodule-config.c\n> index a32259e..b5453d0 100644\n> --- a/submodule-config.c\n> +++ b/submodule-config.c\n> @@ -60,6 +60,8 @@ static void free_one_config(struct submodule_entry *entry)\n> {\n> \tfree((void *) entry->config->path);\n> \tfree((void *) entry->config->name);\n> +\tif (entry->config->groups)\n> +\t\tstring_list_clear(entry->config->groups, 0);\n\nYou are allocating entry->config->groups itself in the in the hunk\naround ll.327, so that also need to be freed, I would think.\n\n> \tfree(entry->config);\n> }\n> \n> @@ -184,6 +186,7 @@ static struct submodule *lookup_or_create_by_name(struct submodule_cache *cache,\n> \tsubmodule->update = NULL;\n> \tsubmodule->fetch_recurse = RECURSE_SUBMODULES_NONE;\n> \tsubmodule->ignore = NULL;\n> +\tsubmodule->groups = NULL;\n> \n> \thashcpy(submodule->gitmodules_sha1, gitmodules_sha1);\n> \n> @@ -324,6 +327,16 @@ static int parse_specific_submodule_config(const char *subsection, int subsectio\n> \t\t\tfree((void *) submodule->update);\n> \t\t\tsubmodule->update = xstrdup(value);\n> \t\t}\n> +\t} else if (!strcmp(key, \"group\")) {\n> +\t\tif (!value)\n> +\t\t\tret = config_error_nonbool(var);\n> +\t\telse {\n> +\t\t\tif (!submodule->groups) {\n> +\t\t\t\tsubmodule->groups = xmalloc(sizeof(*submodule->groups));\n> +\t\t\t\tstring_list_init(submodule->groups, 1);\n> +\t\t\t}\n> +\t\t\tstring_list_insert(submodule->groups, value);\n> +\t\t}\n> \t}\n> \n> \treturn ret;\n> diff --git a/submodule-config.h b/submodule-config.h\n> index d9bbf9a..332f5be 100644\n> --- a/submodule-config.h\n> +++ b/submodule-config.h\n> @@ -17,6 +17,7 @@ struct submodule {\n> \tconst char *update;\n> \t/* the sha1 blob id of the responsible .gitmodules file */\n> \tunsigned char gitmodules_sha1[20];\n> +\tstruct string_list *groups;\n> };\n\nIs there a case where you need to enumerate and show the groups a\nsubmodule belongs to to the end users? Using string_list_insert()\nto manage this list would mean you will lose the original ordering\nyou saw the list of groups in their .gitmodules files, which may or\nmay not matter. I'd assume that the ordering should not matter, but\nthat is something the user may want to see documented.\n\n> \n> int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg);\n"},{"id":"276455","messageId":"xmqqr3hc3pmz.fsf@gitster.mtv.corp.google.com","threadId":"41220","inReplyTo":"1453260880-628-4-git-send-email-sbeller@google.com","subject":"Re: [PATCH 3/4] submodule update: Initialize all group-selected submodules by default","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-20T21:30:28Z","receivedAt":"2016-01-20T21:30:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> All submodules which are selected via submodule groups\n> (\"submodule.groups\") are initialized if they were not initialized\n> before updating the submodules.\n>\n> Signed-off-by: Stefan Beller \n> ---\n> builtin/submodule--helper.c | 30 +++++++++++++++++++++++++++++-\n> t/t7400-submodule-basic.sh | 26 ++++++++++++++++++++++++++\n> 2 files changed, 55 insertions(+), 1 deletion(-)\n>\n> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\n> index 4684f16..def382e 100644\n> --- a/builtin/submodule--helper.c\n> +++ b/builtin/submodule--helper.c\n> @@ -577,6 +577,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n> \n> struct submodule_update_clone {\n> \t/* states */\n> +\tstruct string_list *submodule_groups;\n\nAgain, do we need a field named submodule_groups in a struct whose\nname already makes it clear this is about submodule?\n\nMore importantly.\n\nIf this were used to implement \"there are various groups defined,\nand the user tells us that submodules in this and that groups are to\nbe automatically initialized\", then naming the field with a name\nthat is more specific than just \"groups\" makes tons of sense, but\neven in such a case, the best adjective to clarify what kind of\n\"group\" this field is about is not \"this is a submodule group\".\n\nThe answer I would give to a question \"what kind of group this field\nis about?\" would be \"this is an auto-init group\", so I'd have that\n'auto-init' ness somewhere in its name.\n\n> @@ -633,6 +634,7 @@ static int update_clone_get_next_task(struct child_process *cp,\n> \t\tconst char *update_module = NULL;\n> \t\tchar *url = NULL;\n> \t\tint needs_cloning = 0;\n> +\t\tint in_submodule_groups = 0;\n\nLikewise. The significance of the membership in this group is\ntotally unclear from this variable name. I read this boolean to be\nkeeping track of \"is this submodule to be auto initialized\"?\n"},{"id":"276457","messageId":"xmqqlh7k3p1b.fsf@gitster.mtv.corp.google.com","threadId":"41220","inReplyTo":"1453260880-628-5-git-send-email-sbeller@google.com","subject":"Re: [PATCH 4/4] builtin/clone: support submodule groups","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-20T21:43:28Z","receivedAt":"2016-01-20T21:43:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> When invocing clone with the groups option,\n> the repository will be configured to the specified groups.\n> This has implications for the submodule commands, such as\n> update.\n>\n> Having groups configured will imply init for all uninitialized\n> submodules belonging to at least one of the configured groups,\n> such that new submodules in the group are initialized\n> automatically.\n>\n> Signed-off-by: Stefan Beller \n> ---\n> Documentation/git-clone.txt | 13 +++++++++++\n> builtin/clone.c | 46 +++++++++++++++++++++++++++++++++++---\n> t/t7400-submodule-basic.sh | 54 +++++++++++++++++++++++++++++++++++++++++++++\n> 3 files changed, 110 insertions(+), 3 deletions(-)\n>\n> diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\n> index 6db7b6d..685fb7c 100644\n> --- a/Documentation/git-clone.txt\n> +++ b/Documentation/git-clone.txt\n> @@ -214,6 +214,19 @@ objects from the source repository into a pack in the cloned repository.\n> \trepository does not have a worktree/checkout (i.e. if any of\n> \t`--no-checkout`/`-n`, `--bare`, or `--mirror` is given)\n> \n> +--group::\n\nThis is misnamed but in an opposite way as the variables and fields\nin previous patches. \"Clone\" is not primarily about submodules, so\na plain \"group\" in the context of \"clone\" does not hint that this\noption is about submodules. It also does not tell the user anything\nabout what the parameter is for (i.e. there are submodules that are\nand are not in the group that is specified with this option. What\nspecial things happens to the ones in the group?)\n\nI am guessing that it is about the auto-vivifying behaviour like 3/4\ndid, and if that is the case, perhaps \"--init-submodule=\" or\nsomething like that?\n\nOh by the way, I think you meant this option to take an argument by\nreading the description (you say \"part of the given groups\"). Follow\nthe example of existing option you see below in the context and make\nsure that the reader knows they have to give an argument to it.\n\n A tangent. I think it would be handy to have a way to name a\n single submodule and have the code treat as if there were a\n group that contains that single submodule already defined and\n the user specified that group. Then this option (and any other\n option that takes a submodule group name) can instead take a\n submodule name and the user can expect a natural thing to happen.\n\n> +\tAfter the clone is created, all submodules which are part of the\n> +\tgiven groups are cloned. To specify multiple groups, you can either\n> +\tgive the group argument multiple times or comma separate the groups.\n> +\tThis option will be recorded in the `submodule.groups` config,\n> +\twhich will affect the behavior of other submodule related commands,\n> +\tsuch as `git submodule update`.\n> +\tThis option implies recursive submodule checkout. If you don't\n> +\twant to recurse into nested submodules, you need to specify\n> +\t`--no-recursive`. The group selection will be passed on recursively,\n> +\ti.e. if a submodule is cloned because of group membership, its\n> +\tsubmodules will be cloned according to group membership, too.\n> +\n\n> --separate-git-dir=::\n> \tInstead of placing the cloned repository where it is supposed\n> \tto be, place the cloned repository at the specified directory,\n> diff --git a/builtin/clone.c b/builtin/clone.c\n> index b004fb4..72aea3a 100644\n> --- a/builtin/clone.c\n> +++ b/builtin/clone.c\n> @@ -51,6 +51,22 @@ static struct string_list option_config;\n> static struct string_list option_reference;\n> static int option_dissociate;\n> static int max_jobs = -1;\n> +static struct string_list submodule_groups;\n> +\n> +static int groups_cb(const struct option *opt, const char *arg, int unset)\n> +{\n> +\tstruct string_list_item *item;\n> +\tstruct string_list sl = STRING_LIST_INIT_DUP;\n> +\n> +\tif (unset)\n> +\t\treturn -1;\n> +\n> +\tstring_list_split(&sl, arg, ',', -1);\n\nWhy not just do \"--group A --group B\" without \"--group A,B\"? I do\nnot think you accept \"git submodule add --group A,B\" anyway [*1*],\nso doing \"nice\" things like this is just being inconsistent and does\nnot help the users. And I do personally do not think we should make\nthings consistent by accepting \"--group A,B\" everywhere.\n\n[Footnote]\n\n*1* Technically, your 'add --group' accepts A;B, I think, but then\nthis ',' is still inconsistent with it ;-)\n"},{"id":"276471","messageId":"CAGZ79kaHLkQpaHOY2aqPgdNvWjmpbuMZSogqzPkkVHRUXyibYw@mail.gmail.com","threadId":"41220","inReplyTo":"xmqq1t9c54qz.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 1/4] git submodule: Teach add to accept --group","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-01-20T23:57:46Z","receivedAt":"2016-01-20T23:57:46Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Jan 20, 2016 at 1:18 PM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>\n>> diff --git a/git-submodule.sh b/git-submodule.sh\n>> index 6fce0dc..ab0f209 100755\n>> --- a/git-submodule.sh\n>> +++ b/git-submodule.sh\n>> @@ -130,6 +130,7 @@ cmd_add()\n>> {\n>> # parse $args after \"submodule ... add\".\n>> reference_path=\n>> + submodule_groups=\n>\n> This can just be called $groups in the context of this script. I do\n> not foresee we would be planning to deal with other kinds of groups\n> here.\n\nok, done.\n\n>\n>> while test $# -ne 0\n>> do\n>> case \"$1\" in\n>> @@ -165,6 +166,10 @@ cmd_add()\n>> --depth=*)\n>> depth=$1\n>> ;;\n>> + -g|--group)\n>> + submodule_groups=${submodule_groups:+${submodule_groups};}\"$2\"\n>> + shift\n>> + ;;\n>\n> You would want to accept \"--group=\" as well, just like\n> existing --reference and --depth do. It won't be much more code,\n> and when you move to C (hence parse_options) you'd get it for free\n> anyway.\n\nI am not sure, if I will to move `add` to C any time soon. Sure I desire\nless shell and more C[1], but I'd think my time could be spent better than\njust converting scripts to C. Sometimes I have to though, such as in the\ncase of `init` as the the call out from C to shell is too ugly and the effort to\ndo that is not that much less.\n\n>\n>> @@ -292,6 +297,16 @@ Use -f if you really want to add it.\" >&2\n>>\n>> git config -f .gitmodules submodule.\"$sm_name\".path \"$sm_path\" &&\n>> git config -f .gitmodules submodule.\"$sm_name\".url \"$repo\" &&\n>> + if test -n \"$submodule_groups\"\n>> + then\n>> + OIFS=$IFS\n>> + IFS=';'\n>\n> I do not quite understand the choice of ';' here. If and only if\n> you _must_ accept multi-word name that has spaces in between as a\n> group name, the above construct may make sense, but I do not think\n> we have such requirement. Why not separate with $IFS letters just\n> like any other normal list managed in shell scripts do? Is there\n> anything special about names of submodule groups?\n\nJust prior to writing this patch, I spent a good amount of time understanding\nthe error message handling in the parts of the shell code, where the $IFS is\nused, so my mind was deceived to imitate that.\n\nI agree, we can just use a space as separator here.\n\nThanks,\nStefan\n\n[1] Reason why I have such a disdain to shell is that I do not\nunderstand nearly as\nmuch as C. I am self-taught in both C and shell, so I do not claim to\nbe perfect.\nHowever C is a small but powerful language. After reading \"The C Programming\nLanguage\" by Kernighan and Ritchie I do not think I found a thing that\nwas really\nunknown to me. However even plain posix-shell is complex enough, that there\nis no such thing as a compact book to read to understand all its concepts IMHO.\n"},{"id":"276474","messageId":"xmqq37tr3ibb.fsf@gitster.mtv.corp.google.com","threadId":"41220","inReplyTo":"CAGZ79kaHLkQpaHOY2aqPgdNvWjmpbuMZSogqzPkkVHRUXyibYw@mail.gmail.com","subject":"Re: [PATCH 1/4] git submodule: Teach add to accept --group","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-21T00:08:40Z","receivedAt":"2016-01-21T00:08:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n>>> @@ -165,6 +166,10 @@ cmd_add()\n>>> --depth=*)\n>>> depth=$1\n>>> ;;\n>>> + -g|--group)\n>>> + submodule_groups=${submodule_groups:+${submodule_groups};}\"$2\"\n>>> + shift\n>>> + ;;\n>>\n>> You would want to accept \"--group=\" as well, just like\n>> existing --reference and --depth do. It won't be much more code,\n>> and when you move to C (hence parse_options) you'd get it for free\n>> anyway.\n>\n> I am not sure, if I will to move `add` to C any time soon. Sure I desire\n> less shell and more C[1], but I'd think my time could be spent better than\n> just converting scripts to C. Sometimes I have to though, such as in the\n> case of `init` as the the call out from C to shell is too ugly and the effort to\n> do that is not that much less.\n\nYou can do so in less time than you spent making the above 5-line\nexcuse. It won't be much more code, and it is not ugly at all.\n\n\t--group=*)\n \tgroup=$group ${1#--group=} ;;\n\nor something, right?\n"},{"id":"276475","messageId":"CAGZ79kYY950Ob252ob=sqCkEh_48geBi-a9t-p0jPCn6naCWxg@mail.gmail.com","threadId":"41220","inReplyTo":"xmqq37tr3ibb.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 1/4] git submodule: Teach add to accept --group","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-01-21T00:16:05Z","receivedAt":"2016-01-21T00:16:05Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Jan 20, 2016 at 4:08 PM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>\n>>>> @@ -165,6 +166,10 @@ cmd_add()\n>>>> --depth=*)\n>>>> depth=$1\n>>>> ;;\n>>>> + -g|--group)\n>>>> + submodule_groups=${submodule_groups:+${submodule_groups};}\"$2\"\n>>>> + shift\n>>>> + ;;\n>>>\n>>> You would want to accept \"--group=\" as well, just like\n>>> existing --reference and --depth do. It won't be much more code,\n>>> and when you move to C (hence parse_options) you'd get it for free\n>>> anyway.\n>>\n>> I am not sure, if I will to move `add` to C any time soon. Sure I desire\n>> less shell and more C[1], but I'd think my time could be spent better than\n>> just converting scripts to C. Sometimes I have to though, such as in the\n>> case of `init` as the the call out from C to shell is too ugly and the effort to\n>> do that is not that much less.\n>\n> You can do so in less time than you spent making the above 5-line\n> excuse. It won't be much more code, and it is not ugly at all.\n>\n> --group=*)\n> group=$group ${1#--group=} ;;\n>\n> or something, right?\n\nRight, that's what I have here now. I should have said that.\n\nI was not trying to excuse anything, but more explaining the situation\nwhen reading the second sentence. (You seem to assume I'd want to rewrite\nall the shell scripts. Which I am not)\n"},{"id":"276477","messageId":"CAGZ79kZXSUkMx=+P0+L4yastYzqeCAUf83L2t8H9=QAC1FwuiA@mail.gmail.com","threadId":"41220","inReplyTo":"xmqqwpr43pyz.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 2/4] submodule-config: keep groups around","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-01-21T00:20:57Z","receivedAt":"2016-01-21T00:20:57Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Jan 20, 2016 at 1:23 PM, Junio C Hamano wrote:\n>> free((void *) entry->config->name);\n>> + if (entry->config->groups)\n>> + string_list_clear(entry->config->groups, 0);\n>\n> You are allocating entry->config->groups itself in the in the hunk\n> around ll.327, so that also need to be freed, I would think.\n\ndone\n\n\n>> unsigned char gitmodules_sha1[20];\n>> + struct string_list *groups;\n>> };\n>\n> Is there a case where you need to enumerate and show the groups a\n> submodule belongs to to the end users? Using string_list_insert()\n> to manage this list would mean you will lose the original ordering\n> you saw the list of groups in their .gitmodules files, which may or\n> may not matter. I'd assume that the ordering should not matter, but\n> that is something the user may want to see documented.\n\nIn this patch series we only need to check the items of that list one by one,\nno need for a sorted version, so I'll change it to be unsorted and\ndocument that.\n\n>\n>>\n>> int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg);\n"},{"id":"276480","messageId":"CAGZ79kYJJ+JxgLBqD1Y_UWACUd7yJGnU+rnwEnU6X041deftjQ@mail.gmail.com","threadId":"41220","inReplyTo":"xmqqr3hc3pmz.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 3/4] submodule update: Initialize all group-selected submodules by default","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-01-21T01:44:25Z","receivedAt":"2016-01-21T01:44:25Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Jan 20, 2016 at 1:30 PM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>\n>> All submodules which are selected via submodule groups\n>> (\"submodule.groups\") are initialized if they were not initialized\n>> before updating the submodules.\n>>\n>> Signed-off-by: Stefan Beller \n>> ---\n>> builtin/submodule--helper.c | 30 +++++++++++++++++++++++++++++-\n>> t/t7400-submodule-basic.sh | 26 ++++++++++++++++++++++++++\n>> 2 files changed, 55 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\n>> index 4684f16..def382e 100644\n>> --- a/builtin/submodule--helper.c\n>> +++ b/builtin/submodule--helper.c\n>> @@ -577,6 +577,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n>>\n>> struct submodule_update_clone {\n>> /* states */\n>> + struct string_list *submodule_groups;\n>\n> Again, do we need a field named submodule_groups in a struct whose\n> name already makes it clear this is about submodule?\n>\n> More importantly.\n>\n> If this were used to implement \"there are various groups defined,\n> and the user tells us that submodules in this and that groups are to\n> be automatically initialized\", then naming the field with a name\n> that is more specific than just \"groups\" makes tons of sense, but\n> even in such a case, the best adjective to clarify what kind of\n> \"group\" this field is about is not \"this is a submodule group\".\n>\n> The answer I would give to a question \"what kind of group this field\n> is about?\" would be \"this is an auto-init group\", so I'd have that\n> 'auto-init' ness somewhere in its name.\n\nSo you mean something like `autoinit` would maybe already suffice.\nWhich leads to another question if we want to extend the concept of\nthese submodule groups a little bit, such that we also allow\ndirect names in there, in .git/config we may have the configuration\n\n [submodule]\n groups = \n groups = \n\nsuch that `git clone --group=default --group=mySingleSubmodule ....` would work.\n\nOf course then the --group option would need to be named differently\nin git clone\nand probably the submodule.groups should also be named differently.\n\nHowever:\nAt this point in time we only care about auto-initing submodules\nto get submodules somewhat easier to handle when having lots of them.\nMaybe we also want to add other features to these \"groups\" concept, e.g.\nall submodules of one groups should have the \"(force-)checkout\" update strategy.\nIf the submodule consists of binaries only, this would make lots of sense to me.\n\nSo it is not yet clear to me if we want to extend the grouping feature\nlater on for\nother things, which is why I named it by its concept. A group can be used for\ndifferent purposes, where as \"all submodules having the same auto-init-tag can\nbe treated the same using one update strategy\" just adds to user confusion,\nhence I'd think telling the user about groups is the right thing to do?\n\n>\n>> @@ -633,6 +634,7 @@ static int update_clone_get_next_task(struct child_process *cp,\n>> const char *update_module = NULL;\n>> char *url = NULL;\n>> int needs_cloning = 0;\n>> + int in_submodule_groups = 0;\n>\n> Likewise. The significance of the membership in this group is\n> totally unclear from this variable name. I read this boolean to be\n> keeping track of \"is this submodule to be auto initialized\"?\n\nIn that part of the code it makes sense to rename it to auto_init, I'd guess.\n"},{"id":"276481","messageId":"xmqqio2n1wvn.fsf@gitster.mtv.corp.google.com","threadId":"41220","inReplyTo":"CAGZ79kZXSUkMx=+P0+L4yastYzqeCAUf83L2t8H9=QAC1FwuiA@mail.gmail.com","subject":"Re: [PATCH 2/4] submodule-config: keep groups around","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-21T02:37:00Z","receivedAt":"2016-01-21T02:37:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> In this patch series we only need to check the items of that list one by one,\n> no need for a sorted version, so I'll change it to be unsorted and\n> document that.\n\nI actually was going to suggest keeping the sorted one. The order\nmay be changed from the order specified in the file, but the sorted\nvariant has one very desirable property, namely de-duping, that the\ncode that uses this data structure may find useful.\n"},{"id":"276484","messageId":"xmqqlh7jzgsx.fsf@gitster.mtv.corp.google.com","threadId":"41220","inReplyTo":"CAGZ79kYJJ+JxgLBqD1Y_UWACUd7yJGnU+rnwEnU6X041deftjQ@mail.gmail.com","subject":"Re: [PATCH 3/4] submodule update: Initialize all group-selected submodules by default","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-21T04:40:14Z","receivedAt":"2016-01-21T04:40:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> On Wed, Jan 20, 2016 at 1:30 PM, Junio C Hamano wrote:\n>\n>> If this were used to implement \"there are various groups defined,\n>> and the user tells us that submodules in this and that groups are to\n>> be automatically initialized\", then naming the field with a name\n>> that is more specific than just \"groups\" makes tons of sense, but\n>> even in such a case, the best adjective to clarify what kind of\n>> \"group\" this field is about is not \"this is a submodule group\".\n>>\n>> The answer I would give to a question \"what kind of group this field\n>> is about?\" would be \"this is an auto-init group\", so I'd have that\n>> 'auto-init' ness somewhere in its name.\n>\n> So you mean something like `autoinit` would maybe already suffice.\n\nIf you have (and I wish we have, but I do not seem to be able to\ncome up with myself offhand) a shorter and more intuitive word than\nauto-init, that may be preferred, but basically yes. I'd call that\nautoinit_group or something, as you might want to come up with ways\nother than using groups to specify which ones are to be automatically\ninitialized.\n\nCome to think of it, I think we should drop \"auto\" from the name.\n\nThe only reason we think of this as \"auto\" is because we know that\nhistorically \"clone\" was purely about cloning and anything submodule\nrelated had to be done as a separate step, and this series makes the\nsecond step a part of the \"clone\" proper. But from the point of the\nend user, who specifies that \"init\" to be done for the specified set\nof submodules, there is no longer anything \"auto\" about it. We are\ndoing what the user asked us to do; \"auto\" is what we do without\nexplicitly being told, but the user is telling as explicitly which\nmodules to work on with the option.\n\n> Which leads to another question if we want to extend the concept of\n> these submodule groups a little bit, such that we also allow\n> direct names in there, in .git/config we may have the configuration\n>\n> [submodule]\n> groups = \n> groups = \n>\n> such that `git clone --group=default --group=mySingleSubmodule ....` would work.\n\nI am not sure what you mean by the above snippet. With\n\n\t[submodule \"foo\"]\n \tgroup = default\n\t[submodule \"bar\"]\n \tgroup = optional\n\t[submodule \"baz\"]\n \tgroup = optional\n path = sub/baz\n\nit would be nice if you can say \"modules in the default group\",\n\"modules in the default group and the module 'bar'\", \"modules at\npath sub/baz\", etc.\n\nAm I repeating more or less the same thing as you wanted to say with\nthe above? If so, yes, I do think there should be a uniform syntax\nthat lets users specify set of modules via different mechanisms.\n\n> Of course then the --group option would need to be named\n> differently in git clone and probably the submodule.groups should\n> also be named differently.\n\nI do think --group option is a mistake, as you are only saying\n\"please give a name of a group to this option\" without hinting how\nthe modules in the specified group are to be treated differently, or\nmore importantly, the option is about submodules.\n\n Side note: this is a common delusion developers fall into\n thinking that the feature they currently working on is the most\n important thing in the world. In the context of \"clone\", there\n is no reason to expect that \"groups of submodules\" is any\n special than groups of other things. For the same reason, I\n think --init= is a mistake, as it is not clear from the\n option name that we are initializing submodules in the context\n of \"clone\".\n\nSo perhaps --init-module.\n\nOnce we establish a uniform convention for specifying a group of\nsubmodules is by giving the names of groups, the \"group\" ness of the\noption argument becomes less and less important, as that would be\nimplicitly known by users. \"clone --init=A\" would be more\nunderstandable than \"clone --group=A\", as everybody would know A is\nnaming a set of submodules either way, but the latter does not say\nwhat will happen to the chosen modules.\n\nThere could be other ways to specify the modules, and as long as we\ncan come up with the \"uniform convention for specifying a group of\nsubmodules\", \"clone --init-module=$X --init-module=$Y\", would be\nunderstandable by the users when $X specifies the modules by their\ngroup name and $Y specifies another set of modules by something else,\ne.g. their names or paths to them.\n\n A strawman. You can pass (1) the path to a submodule, (2) you\n can pass a colon followed by the name of a submodule, or (3) you\n can pass an asterisk followed by the name of a group. (1) and\n (2) specifies a single submodule, (3) specifies the submodules\n that belong to the group. I.e.\n\n $ git clone --init-module='*default' --init-module=sub/module\n\n would be a way to say \"clone and then initialize the submodule\n at path sub/module and also those in the default group.\n\n This strawman makes \"path\" the default way, merely because many\n subcommands of \"git submodule\" already specify which submodule\n to operate on by taking paths arguments, and '*' prefix as the\n sign to specify by a group, as an asterisk looks like specifying\n multiple things. ':' is just another prefix that is unlikely to\n be in a pathname.\n\n This is merely an illustration of the kind of syntax that can be\n used to name a set of modules using different ways. I am sure\n people can come up with a different and better syntax, but the\n point of this illustration is not the exact syntax but showing\n that a way to uniformly specify a set of modules to operate on\n would allow us not having to worry about making 'groups' any\n special.\n\nI however do not see why you think \"submodule.group\" needs to be\nspelled differently (it should be \"submodule.$name.group\", though).\nThe 'group' is merely a convenient way to name and choose a set of\nmodules. The name of the operation, i.e. what is to be done to the\nchosen modules, should be orthogonal, so I do not think you should\nhave \"submodule.autoinitgroup\" or somesuch.\n\n> However:\n> At this point in time we only care about auto-initing submodules\n> to get submodules somewhat easier to handle when having lots of them.\n> Maybe we also want to add other features to these \"groups\" concept, e.g.\n> all submodules of one groups should have the \"(force-)checkout\" update strategy.\n> If the submodule consists of binaries only, this would make lots of sense to me.\n>\n> So it is not yet clear to me if we want to extend the grouping feature\n> later on for\n> other things, which is why I named it by its concept. A group can be used for\n> different purposes, where as \"all submodules having the same auto-init-tag can\n> be treated the same using one update strategy\" just adds to user confusion,\n> hence I'd think telling the user about groups is the right thing to do?\n\n\"clone\" and other commands may want to gain use of some mechanism to\nspecify a set of modules, and 'group' is one of the mechanisms to do\nso. Also \"clone\" and other commands may want to gain features to do\ndifferent things to such sets of modules. So there are two orthogonal\naxes.\n\nWhich one is more pleasant to use from the end-user's point of view?\n\n (1) options are named after _how_ you specify the submodules:\n\n $ clone --init-group=A --init-name=B --init-path=C\n\n and what is done to them is implicit.\n\n (2) options are named after _what_ is done to them:\n\n $ clone --init-module=A --init-module=\\*B --init-module=/C\n\n and how the set is specified is implicit to the syntax.\n\nAnd more importantly, which one is more extensible in the future\nwhen you want to add more features that work on multiple but not all\nsubmodules?\n\n\tclone --init-module='*A' --distim-module=':B'\n\nwould be one clear way to say that modules in group A would be\ninited, while the submodule B would be distimmed. What would an\nequivalent of it look like in your \"clone --group=A --name=B\" world?\nWouldn't the design along the lines of (2) above be much better?\n"},{"id":"276485","messageId":"xmqqegdbzgjg.fsf@gitster.mtv.corp.google.com","threadId":"41220","inReplyTo":"CAGZ79kYY950Ob252ob=sqCkEh_48geBi-a9t-p0jPCn6naCWxg@mail.gmail.com","subject":"Re: [PATCH 1/4] git submodule: Teach add to accept --group","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-21T04:45:55Z","receivedAt":"2016-01-21T04:45:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> Right, that's what I have here now. I should have said that.\n>\n> I was not trying to excuse anything, but more explaining the situation\n> when reading the second sentence. (You seem to assume I'd want to rewrite\n> all the shell scripts. Which I am not)\n\nI think it is a reasonable way to spend your time by not rewriting\neverything in C but the parts the matter the most. But it makes it\neven more important to do the part that will be left in shell for a\nlonger time in such a way that will be helpful to the end users (by\naccepting both \"--group name\" and \"--group=name\", in the codepath\nunder discussion).\n"},{"id":"276503","messageId":"CAGZ79kbHvPRa9vn7_BjiNJ+PGT3LWXZXMdTZRJmHP9bJ-_rHZA@mail.gmail.com","threadId":"41220","inReplyTo":"xmqqlh7jzgsx.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 3/4] submodule update: Initialize all group-selected submodules by default","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-01-21T19:39:38Z","receivedAt":"2016-01-21T19:39:38Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Jan 20, 2016 at 8:40 PM, Junio C Hamano wrote:\n> I am not sure what you mean by the above snippet. With\n>\n> [submodule \"foo\"]\n> group = default\n> [submodule \"bar\"]\n> group = optional\n> [submodule \"baz\"]\n> group = optional\n> path = sub/baz\n>\n> it would be nice if you can say \"modules in the default group\",\n> \"modules in the default group and the module 'bar'\", \"modules at\n> path sub/baz\", etc.\n>\n> Am I repeating more or less the same thing as you wanted to say with\n> the above? If so, yes, I do think there should be a uniform syntax\n> that lets users specify set of modules via different mechanisms.\n\nYes, we have the same vision here. However I wonder if we really need\nsyntax to differentiate between groups, names and paths. At least\nfor groups we can make it a requirement to have the same identifier as\nanother name or path.\n\nTechnically it is possible for names and paths to mix up such as\n [submodule \"bar\"]\n path = baz\n [submodule \"baz\"]\n path = bar\n\nsuch that we would need to have a way to tell apart if the user means\nthe name or path. This could be solved implicit by some sort of precedence\n(If there is a name then take it by name, otherwise take the path.)\n\n\n>> Of course then the --group option would need to be named\n>> differently in git clone and probably the submodule.groups should\n>> also be named differently.\n>\n> I do think --group option is a mistake, as you are only saying\n> \"please give a name of a group to this option\" without hinting how\n> the modules in the specified group are to be treated differently, or\n> more importantly, the option is about submodules.\n>\n> Side note: this is a common delusion developers fall into\n> thinking that the feature they currently working on is the most\n> important thing in the world.\n\nI fell into that trap, sure.\n\n> In the context of \"clone\", there\n> is no reason to expect that \"groups of submodules\" is any\n> special than groups of other things. For the same reason, I\n> think --init= is a mistake, as it is not clear from the\n> option name that we are initializing submodules in the context\n> of \"clone\".\n>\n> So perhaps --init-module.\n\nWhy not spell it out completely (--init-submodule)?\nThen we don't have to add to the glossary that a module is the same as\na submodule. (Could there be other modules such as co-modules,\nwhich happen to be not in the tree, but in \"../sibling\" relative to the\nroot of the repository?)\n\n\n\n>\n> Once we establish a uniform convention for specifying a group of\n> submodules is by giving the names of groups, the \"group\" ness of the\n> option argument becomes less and less important, as that would be\n> implicitly known by users. \"clone --init=A\" would be more\n> understandable than \"clone --group=A\", as everybody would know A is\n> naming a set of submodules either way, but the latter does not say\n> what will happen to the chosen modules.\n\nSo keep the \"groups\" in .gitmodules, but ban the name from clone options.\nIn the .gitmodules file we could also name it \"set\" just as you used it in that\nparagraph.\n\n>\n> There could be other ways to specify the modules, and as long as we\n> can come up with the \"uniform convention for specifying a group of\n> submodules\", \"clone --init-module=$X --init-module=$Y\", would be\n> understandable by the users when $X specifies the modules by their\n> group name and $Y specifies another set of modules by something else,\n> e.g. their names or paths to them.\n\nok.\n\n>\n> A strawman. You can pass (1) the path to a submodule, (2) you\n> can pass a colon followed by the name of a submodule, or (3) you\n> can pass an asterisk followed by the name of a group. (1) and\n> (2) specifies a single submodule, (3) specifies the submodules\n> that belong to the group. I.e.\n>\n> $ git clone --init-module='*default' --init-module=sub/module\n>\n> would be a way to say \"clone and then initialize the submodule\n> at path sub/module and also those in the default group.\n>\n> This strawman makes \"path\" the default way, merely because many\n> subcommands of \"git submodule\" already specify which submodule\n> to operate on by taking paths arguments, and '*' prefix as the\n> sign to specify by a group, as an asterisk looks like specifying\n> multiple things. ':' is just another prefix that is unlikely to\n> be in a pathname.\n\nBut it could be in a pathname. Another strawman:\n\n You can pass either (1) the path (2) or the name (3) or the group\n of submodules to --init-submodule if there are no collisions in the\n three name spaces. When there are namespaces, you need to be\n explicit, such as --init-submodule=path:foo:baz.\n Note that \"path:\" is the selector which of the three namespaces\n we mean and \"foo:baz\" is a path asking for pain.\n\n\n\n>\n> This is merely an illustration of the kind of syntax that can be\n> used to name a set of modules using different ways. I am sure\n> people can come up with a different and better syntax, but the\n> point of this illustration is not the exact syntax but showing\n> that a way to uniformly specify a set of modules to operate on\n> would allow us not having to worry about making 'groups' any\n> special.\n\nok\n\n> The 'group' is merely a convenient way to name and choose a set of\n> modules.\n\nBut not just at the time of cloning. If later upstream decides that\ngroup=default includes another submodule, this submodule should\nbe initialized and fetched in the next call to \"submodule update\",\nsuch that downstream has a working repository.\n\nThis idea makes groups more than just a temporary collection for\ninit-purposes, but we need to store the selection.\n\n>\n> I however do not see why you think \"submodule.group\" needs to be\n> spelled differently (it should be \"submodule.$name.group\", though).\n\n\"submodule.$name.group\" to be found in .gitmodules, maybe overwritten in\n.git/config tells for each submodule its memberships of groups.\n\n\"submodule.group\" should be found in .git/config only, to tell some time\nafter cloning which group selection was made, such that we can check\nif new submodules need to be initialized (or even automatically removed).\n\n \"git submodule update\" may initialze and fetch new modules if the\n .gitmodules file changed their view of what \"default\" is.\n\n> The name of the operation, i.e. what is to be done to the\n> chosen modules, should be orthogonal, so I do not think you should\n> have \"submodule.autoinitgroup\" or somesuch.\n\nI agree there.\n\n>\n>> However:\n>> At this point in time we only care about auto-initing submodules\n>> to get submodules somewhat easier to handle when having lots of them.\n>> Maybe we also want to add other features to these \"groups\" concept, e.g.\n>> all submodules of one groups should have the \"(force-)checkout\" update strategy.\n>> If the submodule consists of binaries only, this would make lots of sense to me.\n>>\n>> So it is not yet clear to me if we want to extend the grouping feature\n>> later on for\n>> other things, which is why I named it by its concept. A group can be used for\n>> different purposes, where as \"all submodules having the same auto-init-tag can\n>> be treated the same using one update strategy\" just adds to user confusion,\n>> hence I'd think telling the user about groups is the right thing to do?\n>\n> \"clone\" and other commands may want to gain use of some mechanism to\n> specify a set of modules, and 'group' is one of the mechanisms to do\n> so. Also \"clone\" and other commands may want to gain features to do\n> different things to such sets of modules. So there are two orthogonal\n> axes.\n>\n> Which one is more pleasant to use from the end-user's point of view?\n>\n> (1) options are named after _how_ you specify the submodules:\n>\n> $ clone --init-group=A --init-name=B --init-path=C\n>\n> and what is done to them is implicit.\n>\n> (2) options are named after _what_ is done to them:\n>\n> $ clone --init-module=A --init-module=\\*B --init-module=/C\n>\n> and how the set is specified is implicit to the syntax.\n\nI do think (2) is easier, unless the syntax is crazy or counterintuitive.\n\n>\n> And more importantly, which one is more extensible in the future\n> when you want to add more features that work on multiple but not all\n> submodules?\n>\n> clone --init-module='*A' --distim-module=':B'\n>\n> would be one clear way to say that modules in group A would be\n> inited, while the submodule B would be distimmed. What would an\n> equivalent of it look like in your \"clone --group=A --name=B\" world?\n> Wouldn't the design along the lines of (2) above be much better?\n\nI agree on (2) being better.\n\nHowever as pointed out, we want more than to just init submodules at time\nof clone, so it would be a\n\n clone --init-module=A --init-module=\\*B --init-module=/C \\\n --and-remember-to-keep-track-of=A \\\n --and-remember-to-keep-track-of=\\*B \\\n --and-remember-to-keep-track-of=/C\n\nI am hoping we can put that in shorter options, such as\n\n clone --init-module=A --init-module=\\*B --init-module=/C \\\n --remember-init-for-tracking\n\nwhereas:\n\n --remember-init-for-tracking: Submodule groups which are given to clone\n will be remembered, such that each invocation of \"update\" will make sure\n that group is fully there, i.e. new submodules in the group\nwill be initialized\n before updating.\n\nThanks,\nStefan\n"},{"id":"276506","messageId":"xmqqk2n2y815.fsf@gitster.mtv.corp.google.com","threadId":"41220","inReplyTo":"CAGZ79kbHvPRa9vn7_BjiNJ+PGT3LWXZXMdTZRJmHP9bJ-_rHZA@mail.gmail.com","subject":"Re: [PATCH 3/4] submodule update: Initialize all group-selected submodules by default","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-21T20:47:18Z","receivedAt":"2016-01-21T20:47:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> \"submodule.$name.group\" to be found in .gitmodules, maybe overwritten in\n> .git/config tells for each submodule its memberships of groups.\n>\n> \"submodule.group\" should be found in .git/config only, to tell some time\n> after cloning which group selection was made, such that we can check\n> if new submodules need to be initialized (or even automatically removed).\n\nAhh, it wasn't clear that was what you were trying to do. If that\nis the case, then ...\n\n> \"git submodule update\" may initialze and fetch new modules if the\n> .gitmodules file changed their view of what \"default\" is.\n>\n>> The name of the operation, i.e. what is to be done to the\n>> chosen modules, should be orthogonal, so I do not think you should\n>> have \"submodule.autoinitgroup\" or somesuch.\n>\n> I agree there.\n\n... I do not agree that the name of \"submodule.group\" variable\nshould be neutral to the operation that is done to the groups of\nsubmodules named by the variable at all.\n\nYou are recording the criteria to choose a set of submodules there\nin the configuration, with a plan to keep doing something to them\n(e.g. \"they will never be left uninitialized, i.e. initialied if a\nnew submodule is added to the set\"). The name of the configuration\nmust have something that tells what that \"something\" is, in order to\n(1) hint the purpose of specifying the group selection criteria\nthere, and (2) allow different selection criteria to keep doning\nanother kind of something to them and distinguish these two group\nselection criteria.\n\nE.g. \"submodule.autoInitialize = *default !:foo\" may mean all the\nmodules in the default group (now or added in the future) except the\nmodule with name 'foo' will be initialized as needed, while\n\"submodule.autoDistim = *optional\" may want to be defined to allow\nthe system to automatically distim (whatever that operation is that\nwill be added to Git in later versions) the modules in the optional\ngroup.\n\n> I am hoping we can put that in shorter options, such as\n>\n> clone --init-module=A --init-module=\\*B --init-module=/C \\\n> --remember-init-for-tracking\n>\n> whereas:\n>\n> --remember-init-for-tracking: Submodule groups which are given\n> to clone will be remembered, such that each invocation of \"update\"\n> will make sure that group is fully there, i.e. new submodules in\n> the group will be initialized before updating.\n\nIs there a need for --no-remember-init-for-tracking? I do not think\nit would be useful.\n\nWhen the upstream adds a new module and defines it to be part of the\ndefault group _after_ you cloned with --init set to 'default', and\nyou do not need that new module, at that point you can tweak your\nsubmodule.autoInitialize definition to exclude that new module.\n\nTweaking submodule.autoInitialize definition to contain nothing\nafter cloning with --init because you do not want the autoinit\ncriteria kept in the resulting repository is merely a special case\nof that.\n\nSo I do not think --remember-init-for-tracking is necessary. Just\nmake it _always_ on and be done with it, I would say.\n"},{"id":"276508","messageId":"xmqqfuxqy7k6.fsf@gitster.mtv.corp.google.com","threadId":"41220","inReplyTo":"xmqqk2n2y815.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 3/4] submodule update: Initialize all group-selected submodules by default","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-21T20:57:29Z","receivedAt":"2016-01-21T20:57:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Ahh, it wasn't clear that was what you were trying to do. If that\n> is the case, then ...\n> ...\n> So I do not think --remember-init-for-tracking is necessary. Just\n> make it _always_ on and be done with it, I would say.\n\nBy the way, I forgot to say that we're basically in agreement on\neverything in your message that I didn't touch in my response. Also\nI forgot to say that I am happy to see that we are designing a new\nmechanism to name a set of modules in a convenient way so that the\nusers can do things to them without hassle.\n\n;-)\n"},{"id":"276512","messageId":"56A14AD4.6040608@gmail.com","threadId":"41220","inReplyTo":"1453260880-628-1-git-send-email-sbeller@google.com","subject":"Re: [PATCH 0/4] Submodule Groups","fromName":"Sebastian Schuberth","fromEmail":"sschuberth@gmail.com","sentAt":"2016-01-21T21:17:08Z","receivedAt":"2016-01-21T21:17:08Z","isPatch":true,"sender":{"key":"sschuberth@gmail.com","avatar":"https://avatars.githubusercontent.com/u/349154?v=4"},"body":"On 20.01.2016 04:34, Stefan Beller wrote:\n\n> So you could have a .gitmodules file such as:\n> \n> [submodule \"gcc\"]\n> path = gcc\n> url = git://...\n> groups = default\n> groups = devel\n\nOn 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.\n\nAnyway, 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\".\n\nHowever, 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\".\n\nLong story short, my personal favorite still would be \n\n[submodule \"gcc\"]\n groups = default,devel\n\nfollowed by\n\n[submodule \"gcc\"]\n group = default\n group = devel\n\n-- \nSebastian Schuberth\n"},{"id":"276515","messageId":"CAGZ79ka+LZHRjVB8bZ4u+-n-U06mUWsbx9nRVCvd4aPwrb7yHQ@mail.gmail.com","threadId":"41220","inReplyTo":"56A14AD4.6040608@gmail.com","subject":"Re: [PATCH 0/4] Submodule Groups","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-01-21T21:56:16Z","receivedAt":"2016-01-21T21:56:16Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Jan 21, 2016 at 1:17 PM, Sebastian Schuberth\n wrote:\n> On 20.01.2016 04:34, Stefan Beller wrote:\n>\n>> So you could have a .gitmodules file such as:\n>>\n>> [submodule \"gcc\"]\n>> path = gcc\n>> url = git://...\n>> groups = default\n>> groups = devel\n>\n> 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.\n\nIIUC the line oriented way is preferred as it is our standard. Do we\nhave any other options stored as a comma separated list?\n\n>\n> 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\".\n\nMakes sense to use singular then. However per discussion with Junio in\n[PATCH 3/4] submodule update: Initialize all group-selected submodules\nby default, we want to not name it \"group\", as it's unclear what a group is\nsupposed to mean. What does a group do? which operations are supported?\n\nSo for git clone, we'd rather use \"--init-submodule\" which can be passed a\nname, path or group.\n\nFor storing that selection we'd go with \"submodule.autoInitialize\" in\n.git /config.\n\nThe third user visible place submodule.$NAME.group however can stay in that\nvariable name as to point out the the concept of a submodule set/collection.\nThe groups concept may be used in the future for more than initialzing\nsubmodules.\n\nInstead of having a submodule -> set assignment, we could do it the\nother way round:\n\n [submodule \"gcc\"]\n ...\n\n [submodule-set \"default\"]\n submodule = gcc\n submodule = foo\n submodule = by/path/*\n\nThis may be more handy from our perspective (while designing it and\nwriting the code),\nbut I'd assume this is less useful for the user. How often does a user ask:\n\"How many/Which submodules are in $GROUP\" as opposed to \"What about\nsubmodule foo,\nis that part of group $GROUP?\"\n\n>\n> 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\".\n>\n> Long story short, my personal favorite still would be\n>\n> [submodule \"gcc\"]\n> groups = default,devel\n>\n> followed by\n>\n> [submodule \"gcc\"]\n> group = default\n> group = devel\n\nAs asked above, how many comma separated things do we have in git configs?\nI'd really not want to add more mental complexity to Git. As far as I\nremember we have\nrather double configs than one long line separated somehow.\n(The only thing that comes to mind is multiple remote urls for pushing)\n\nThanks,\nStefan\n\n>\n> --\n> Sebastian Schuberth\n"},{"id":"276518","messageId":"xmqq37tqy3t4.fsf@gitster.mtv.corp.google.com","threadId":"41220","inReplyTo":"CAGZ79ka+LZHRjVB8bZ4u+-n-U06mUWsbx9nRVCvd4aPwrb7yHQ@mail.gmail.com","subject":"Re: [PATCH 0/4] Submodule Groups","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-21T22:18:31Z","receivedAt":"2016-01-21T22:18:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> Instead of having a submodule -> set assignment, we could do it the\n> other way round:\n>\n> [submodule \"gcc\"]\n> ...\n>\n> [submodule-set \"default\"]\n> submodule = gcc\n> submodule = foo\n> submodule = by/path/*\n>\n> This may be more handy from our perspective (while designing it and\n> writing the code),\n> but I'd assume this is less useful for the user. How often does a user ask:\n> \"How many/Which submodules are in $GROUP\" as opposed to \"What about\n> submodule foo,\n> is that part of group $GROUP?\"\n\nI suspect that we will end up needing to support both styles. The\nlatter style is easier when you want to express a larger set as a\ncollection of groups, e.g.\n\n [submodule \"gcc\"]\n \tgroup = development-tools\n\n [submodule \"emacs\"]\n \tgroup = editors\n\n [submodule \"frotz\"]\n group = games\n\n [submoduleGroup \"default\"]\n member = *development-tools\n member = *editors\n member = :frotz\n\tmember = !*xyzzy\n\nmight be a way to say \"the default group includes everything in the\n'development-tools' or 'editors' group, plus 'frotz' module, but do\nnot include modules in the xyzzy group\" or something like that.\n"},{"id":"276519","messageId":"xmqqy4biwowb.fsf@gitster.mtv.corp.google.com","threadId":"41220","inReplyTo":"xmqq37tqy3t4.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 0/4] Submodule Groups","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-21T22:25:56Z","receivedAt":"2016-01-21T22:25:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> I suspect that we will end up needing to support both styles. The\n> latter style is easier when you want to express a larger set as a\n> collection of groups, e.g.\n> ...\n> might be a way to say \"the default group includes everything in the\n> 'development-tools' or 'editors' group, plus 'frotz' module, but do\n> not include modules in the xyzzy group\" or something like that.\n\nI forgot to say that I personally do not think we need to support\nsuch synthetic groups from day one.\n"},{"id":"276520","messageId":"CAGZ79kYVE7a=kXi__+Y3C7cuQUhY78o0unuw7vU4hrO71q1dwA@mail.gmail.com","threadId":"41220","inReplyTo":"xmqq37tqy3t4.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 0/4] Submodule Groups","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-01-21T22:30:32Z","receivedAt":"2016-01-21T22:30:32Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Jan 21, 2016 at 2:18 PM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>\n>> Instead of having a submodule -> set assignment, we could do it the\n>> other way round:\n>>\n>> [submodule \"gcc\"]\n>> ...\n>>\n>> [submodule-set \"default\"]\n>> submodule = gcc\n>> submodule = foo\n>> submodule = by/path/*\n>>\n>> This may be more handy from our perspective (while designing it and\n>> writing the code),\n>> but I'd assume this is less useful for the user. How often does a user ask:\n>> \"How many/Which submodules are in $GROUP\" as opposed to \"What about\n>> submodule foo,\n>> is that part of group $GROUP?\"\n>\n> I suspect that we will end up needing to support both styles.\n\nI think having both is bad as it may contradict each other?\nWhat is supposed to happen here:\n\n [submodule \"frotz\"]\n group = default\n\n [submoduleGroup \"default\"]\n member = !:frotz\n\nInitially I disliked your proposal of : and * to indicate name and groups,\nbut the example you gave is very clear and understandable on the syntax level.\n\n> The\n> latter style is easier when you want to express a larger set as a\n> collection of groups, e.g.\n>\n> [submodule \"gcc\"]\n> group = development-tools\n>\n> [submodule \"emacs\"]\n> group = editors\n>\n> [submodule \"frotz\"]\n> group = games\n>\n> [submoduleGroup \"default\"]\n> member = *development-tools\n> member = *editors\n> member = :frotz\n> member = !*xyzzy\n>\n> might be a way to say \"the default group includes everything in the\n> 'development-tools' or 'editors' group, plus 'frotz' module, but do\n> not include modules in the xyzzy group\" or something like that.\n\n> I forgot to say that I personally do not think we need to support\n> such synthetic groups from day one.\n\nSo groups of groups, we discovered recursion today. :)\nHaving this discussion makes me realize, the groups handling logic\nwill be more complex than I anticipated for the first RFC patch series.\n\nThe two basic questions the logic has to answer is:\n * Give me all the submodules that fit these specifiers (i.e. the\n --init-submodules collection from clone)\n * Given submodule X, does it fit the specifier ( a new uninitialized\n submodule during a later update)\n\nThe second question can be answered by answering the first question\nand then checking if X is in the set. However that may be not the most\nefficient solution.\n"},{"id":"276522","messageId":"xmqqpowuwocb.fsf@gitster.mtv.corp.google.com","threadId":"41220","inReplyTo":"CAGZ79kYVE7a=kXi__+Y3C7cuQUhY78o0unuw7vU4hrO71q1dwA@mail.gmail.com","subject":"Re: [PATCH 0/4] Submodule Groups","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-21T22:37:56Z","receivedAt":"2016-01-21T22:37:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> I think having both is bad as it may contradict each other?\n> What is supposed to happen here:\n>\n> [submodule \"frotz\"]\n> group = default\n>\n> [submoduleGroup \"default\"]\n> member = !:frotz\n\nWhat is supposed to happen is that you will write code to diagnose\nthis as an error, and give helpful error message to the user.\n\n> So groups of groups, we discovered recursion today. :)\n> Having this discussion makes me realize, the groups handling logic\n> will be more complex than I anticipated for the first RFC patch series.\n\nThat is why I think we do not have to have a very complex group\nlogic from day one.\n\n> The two basic questions the logic has to answer is:\n> * Give me all the submodules that fit these specifiers (i.e. the\n> --init-submodules collection from clone)\n> * Given submodule X, does it fit the specifier ( a new uninitialized\n> submodule during a later update)\n\nYes.\n"},{"id":"276535","messageId":"CAHGBnuMRd3bg4KYito08U8LJNC3JAJFBVz-APZxVJyd6WKO6TQ@mail.gmail.com","threadId":"41220","inReplyTo":"CAGZ79ka+LZHRjVB8bZ4u+-n-U06mUWsbx9nRVCvd4aPwrb7yHQ@mail.gmail.com","subject":"Re: [PATCH 0/4] Submodule Groups","fromName":"Sebastian Schuberth","fromEmail":"sschuberth@gmail.com","sentAt":"2016-01-22T08:55:07Z","receivedAt":"2016-01-22T08:55:07Z","isPatch":true,"sender":{"key":"sschuberth@gmail.com","avatar":"https://avatars.githubusercontent.com/u/349154?v=4"},"body":"On Thu, Jan 21, 2016 at 10:56 PM, Stefan Beller wrote:\n\n>>> [submodule \"gcc\"]\n>>> path = gcc\n>>> url = git://...\n>>> groups = default\n>>> groups = devel\n>>\n>> 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.\n>\n> IIUC the line oriented way is preferred as it is our standard. Do we\n> have any other options stored as a comma separated list?\n\nOut of my head I cannot think of any. But that shouldn't mean we\ncannot introduce such comma separated list if it makes sense.\n\n> Makes sense to use singular then. However per discussion with Junio in\n> [PATCH 3/4] submodule update: Initialize all group-selected submodules\n> by default, we want to not name it \"group\", as it's unclear what a group is\n> supposed to mean. What does a group do? which operations are supported?\n\nHow about calling it \"label\" instead of \"group\"? IMO with the word\n\"label\" it's more clean that a single submodule can have multiple\nlabels, as the concept of labels is familiar to the user already from\napplications like Firefox (bookmarks), Google Mail, Mac OS X Finder\n(files) etc.\n\n> Instead of having a submodule -> set assignment, we could do it the\n> other way round:\n>\n> [submodule \"gcc\"]\n> ...\n>\n> [submodule-set \"default\"]\n> submodule = gcc\n> submodule = foo\n> submodule = by/path/*\n\nIn your example you're now introducing \"set\" as a new term. Shouldn't\nthis better be \"submodule-group\" then? I actually like this idea quite\na bit as it completely solves the problem about being clear that a\nsubmodule can belong to mutiple groups.\n\n> but I'd assume this is less useful for the user. How often does a user ask:\n> \"How many/Which submodules are in $GROUP\" as opposed to \"What about\n> submodule foo,\n> is that part of group $GROUP?\"\n\nTrue, but for answering that question a user would not look at\n.gitmodules, but run a command, and the implementation of that command\nwould completely hide that complexity from the user.\n\n> As asked above, how many comma separated things do we have in git configs?\n> I'd really not want to add more mental complexity to Git. As far as I\n\nI don't think it can get much worse anyway ;-)\n\n> remember we have\n> rather double configs than one long line separated somehow.\n> (The only thing that comes to mind is multiple remote urls for pushing)\n\nI believe so, too. But I'd see the introduction of comma-separated\nvalues as an exit-strategy to that. More settings could make use of\nthat in the future, then.\n\n-- \nSebastian Schuberth\n"}]}