{"thread":{"id":"42279","subject":"[PATCH 2/7] submodule add: label submodules if asked to","startedAt":"2016-05-11T00:59:50Z","lastAt":"2016-05-12T16:53:38Z","messageCount":30,"participants":["Stefan Beller","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":7},"messages":[{"id":"286134","messageId":"1462928397-1708-1-git-send-email-sbeller@google.com","threadId":"42279","inReplyTo":null,"subject":"[PATCH 0/7] submodule groups","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-05-11T00:59:50Z","receivedAt":"2016-05-11T00:59:50Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"I started from scratch as I think there were some sharp edges in the design.\nMy thinking shifted from \"submodule groups\" towards \"actually it's just an\nenhanced pathspec, called submodulespec\".\n\nThe meat is found in the last 3 patches.\n\nWhat is this series about?\n==========================\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 labels to submodules 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        label = default\n        label = devel\n[submodule \"linux\"]\n        path = linux\n        url = git://...\n        label = default\n[submodule \"nethack\"]\n        path = nethack\n        url = git://...\n        label = optional\n        label = games\n\nand by this series you can work on an arbitrary group of these submodules\ncomposed by the labels, names or paths of the submodules.\n\n    git clone --init-submodule=\\*<label> --init-submodule=:<name> git://...\n    # will clone the superproject checkout\n    # any submodule being labeled <label> or named <name>\n\n    git submodule add --label <name> git://... ..\n    # record a label while adding a submodule\n\n    git submodule update [--init] \\*label2\n    # update only the submodules labeled \"label2\"\n    \n    git config submodule.updateGroups default\n    git config --add submodule.updateGroups devel\n    # configure which submodules you are interested in....\n    git submodule update [--init-default-group] \n    # ... and update them    \n\n    git status \n    git diff\n    git submodule summary\n    # unlike the last series, these are not touched\n    \n    git submodule status \\*label2\n    # only show information about \"label2\" submodules.\n    \n\nAny feedback welcome, specially on the design level!\n\nThanks,\nStefan\n\nPrior series found here: http://thread.gmane.org/gmane.comp.version-control.git/292666\n\nStefan Beller (7):\n  submodule--helper: add valid-label-name\n  submodule add: label submodules if asked to\n  submodule-config: keep labels around\n  submodule-config: check if a submodule is in a group\n  submodule--helper module_list_compute: allow label or name arguments\n  submodule update: learn partial initialization\n  clone: allow specification of submodules to be cloned\n\n Documentation/config.txt        |   5 ++\n Documentation/git-clone.txt     |  26 ++++--\n Documentation/git-submodule.txt |  30 +++++--\n builtin/clone.c                 |  40 ++++++++-\n builtin/submodule--helper.c     | 146 +++++++++++++++++++++++++++---\n git-submodule.sh                |  38 ++++++--\n submodule-config.c              |  66 ++++++++++++++\n submodule-config.h              |   5 ++\n t/t7400-submodule-basic.sh      | 176 ++++++++++++++++++++++++++++++++++++\n t/t7412-submodule--helper.sh    | 193 ++++++++++++++++++++++++++++++++++++++++\n 10 files changed, 692 insertions(+), 33 deletions(-)\n create mode 100755 t/t7412-submodule--helper.sh\n\n-- \n2.8.0.35.g58985d9.dirty\n"},{"id":"286136","messageId":"1462928397-1708-2-git-send-email-sbeller@google.com","threadId":"42279","inReplyTo":"1462928397-1708-1-git-send-email-sbeller@google.com","subject":"[PATCH 1/7] submodule--helper: add valid-label-name","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-05-11T00:59:51Z","receivedAt":"2016-05-11T00:59:51Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"We could allow more than just alphanumeric and dash characters\nfor submodule labels. As a precaution we'll first allow only this\nsubset and later on we can extend it once we have more experience\nwith them.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n builtin/submodule--helper.c  | 30 ++++++++++++++++++++++++++-\n t/t7412-submodule--helper.sh | 49 ++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 78 insertions(+), 1 deletion(-)\n create mode 100755 t/t7412-submodule--helper.sh\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 7f0941d..d3f4684 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -831,6 +831,33 @@ static int update_clone(int argc, const char **argv, const char *prefix)\n \treturn 0;\n }\n \n+static int submodule_valid_label_name(const char *label)\n+{\n+\tif (!label || !strlen(label))\n+\t\treturn 0;\n+\n+\tif (!isalnum(*label))\n+\t\treturn 0;\n+\n+\twhile (*label) {\n+\t\tif (!(isalnum(*label) ||\n+\t\t\t*label == '-'))\n+\t\t\treturn 0;\n+\t\tlabel++;\n+\t}\n+\n+\treturn 1;\n+}\n+\n+static int valid_label_name(int argc, const char **argv, const char *prefix)\n+{\n+\tif (argc == 2 && submodule_valid_label_name(argv[1]))\n+\t\treturn 0;\n+\n+\tdie(_(\"submodule label must start with an alphanumeric character\"\n+\t      \"and must contain alphanumeric characters or dashes only.\"));\n+}\n+\n struct cmd_struct {\n \tconst char *cmd;\n \tint (*fn)(int, const char **, const char *);\n@@ -843,7 +870,8 @@ static struct cmd_struct commands[] = {\n \t{\"update-clone\", update_clone},\n \t{\"resolve-relative-url\", resolve_relative_url},\n \t{\"resolve-relative-url-test\", resolve_relative_url_test},\n-\t{\"init\", module_init}\n+\t{\"init\", module_init},\n+\t{\"valid-label-name\", valid_label_name}\n };\n \n int cmd_submodule__helper(int argc, const char **argv, const char *prefix)\ndiff --git a/t/t7412-submodule--helper.sh b/t/t7412-submodule--helper.sh\nnew file mode 100755\nindex 0000000..3af315c\n--- /dev/null\n+++ b/t/t7412-submodule--helper.sh\n@@ -0,0 +1,49 @@\n+#!/bin/sh\n+\n+test_description='Basic plumbing support of submodule--helper\n+\n+This test verifies the submodule--helper plumbing command used to implement\n+git-submodule.\n+'\n+\n+. ./test-lib.sh\n+\n+\n+test_expect_success 'valid-label-name tests empty label' '\n+\ttest_must_fail git submodule--helper valid-label-name 2>actual &&\n+\ttest_i18ngrep alphanumeric actual &&\n+\ttest_must_fail git submodule--helper valid-label-name \"\" 2>actual &&\n+\ttest_i18ngrep alphanumeric actual\n+'\n+\n+test_expect_success 'valid-label-name tests correct label asdf' '\n+\tgit submodule--helper valid-label-name asdf 2>actual &&\n+\ttest_must_be_empty actual\n+'\n+\n+test_expect_success 'valid-label-name tests correct label a' '\n+\tgit submodule--helper valid-label-name a 2>actual &&\n+\ttest_must_be_empty actual\n+'\n+\n+test_expect_success 'valid-label-name tests correct label a-b' '\n+\tgit submodule--helper valid-label-name a-b 2>actual &&\n+\ttest_must_be_empty actual\n+'\n+\n+test_expect_success 'valid-label-name fails with multiple arguments' '\n+\ttest_must_fail git submodule--helper valid-label-name a b 2>actual &&\n+\ttest_i18ngrep alphanumeric actual\n+'\n+\n+test_expect_success 'valid-label-name fails with white spaced arguments' '\n+\ttest_must_fail git submodule--helper valid-label-name \"a b\" 2>actual &&\n+\ttest_i18ngrep alphanumeric actual\n+'\n+\n+test_expect_success 'valid-label-name fails with utf8 characters' '\n+\ttest_must_fail git submodule--helper valid-label-name ☺ 2>actual &&\n+\ttest_i18ngrep alphanumeric actual\n+'\n+\n+test_done\n-- \n2.8.0.35.g58985d9.dirty\n"},{"id":"286132","messageId":"1462928397-1708-3-git-send-email-sbeller@google.com","threadId":"42279","inReplyTo":"1462928397-1708-1-git-send-email-sbeller@google.com","subject":"[PATCH 2/7] submodule add: label submodules if asked to","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-05-11T00:59:52Z","receivedAt":"2016-05-11T00:59:52Z","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 labels the submodule\nbelongs to by giving one or more --label arguments. This will record\neach label in the .gitmodules file as a value of the key\n\"submodule.$NAME.label\".\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n Documentation/git-submodule.txt |  4 +++-\n git-submodule.sh                | 16 ++++++++++++++-\n t/t7400-submodule-basic.sh      | 44 +++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 62 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt\nindex 13adebf..9ba8895 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 <branch>] [-f|--force] [--name <name>]\n+'git submodule' [--quiet] add [-b <branch>] [-f|--force] [-l|--label <label>]\n \t      [--reference <repository>] [--depth <depth>] [--] <repository> [<path>]\n 'git submodule' [--quiet] status [--cached] [--recursive] [--] [<path>...]\n 'git submodule' [--quiet] init [--] [<path>...]\n@@ -101,6 +101,8 @@ 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+All labels are recorded in the .gitmodules file in the label fields.\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 82e95a9..c1213d8 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -5,7 +5,7 @@\n # Copyright (c) 2007 Lars Hjemli\n \n dashless=$(basename \"$0\" | sed -e 's/-/ /')\n-USAGE=\"[--quiet] add [-b <branch>] [-f|--force] [--name <name>] [--reference <repository>] [--] <repository> [<path>]\n+USAGE=\"[--quiet] add [-b <branch>] [-f|--force] [--name <name>] [--reference <repository>] [-l|--label <label>][--] <repository> [<path>]\n    or: $dashless [--quiet] status [--cached] [--recursive] [--] [<path>...]\n    or: $dashless [--quiet] init [--] [<path>...]\n    or: $dashless [--quiet] deinit [-f|--force] [--] <path>...\n@@ -130,6 +130,7 @@ cmd_add()\n {\n \t# parse $args after \"submodule ... add\".\n \treference_path=\n+\tlabels=\n \twhile test $# -ne 0\n \tdo\n \t\tcase \"$1\" in\n@@ -165,6 +166,15 @@ cmd_add()\n \t\t--depth=*)\n \t\t\tdepth=$1\n \t\t\t;;\n+\t\t-l|--label)\n+\t\t\tgit submodule--helper valid-label-name \"$2\" || exit\n+\t\t\tlabels=\"${labels} $2\"\n+\t\t\tshift\n+\t\t\t;;\n+\t\t--label=*)\n+\t\t\tgit submodule--helper valid-label-name \"${1#--label=}\" || exit\n+\t\t\tlabels=\"${labels} ${1#--label=}\"\n+\t\t\t;;\n \t\t--)\n \t\t\tshift\n \t\t\tbreak\n@@ -292,6 +302,10 @@ 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+\tfor label in $labels\n+\tdo\n+\t\tgit config --add -f .gitmodules submodule.\"$sm_name\".label \"$label\"\n+\tdone &&\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 814ee63..0adc4e4 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -1056,6 +1056,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@@ -1094,5 +1095,48 @@ test_expect_success 'submodule helper list is not confused by common prefixes' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'submodule add records a label' '\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 --label labelA file://\"$pwd\"/example2 submodule &&\n+\t\tgit config -f .gitmodules submodule.\"submodule\".label >../actual &&\n+\t\techo labelA >../expect\n+\t) &&\n+\ttest_cmp expect actual\n+'\n+\n+cat >expect <<-EOF\n+labelA\n+labelB\n+EOF\n+\n+test_expect_success 'submodule add records multiple labels' '\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 --label=labelA -l labelB file://\"$pwd\"/example2 submodule &&\n+\t\tgit config --get-all -f .gitmodules submodule.\"submodule\".label >../actual\n+\t) &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'submodule add recording wrong labels reports an error' '\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\ttest_must_fail git submodule add --label=\"labelA labelB\" file://\"$pwd\"/example2 submodule 2>../actual\n+\t) &&\n+\ttest_i18ngrep alphanumeric actual\n+'\n \n test_done\n-- \n2.8.0.35.g58985d9.dirty\n"},{"id":"286133","messageId":"1462928397-1708-4-git-send-email-sbeller@google.com","threadId":"42279","inReplyTo":"1462928397-1708-1-git-send-email-sbeller@google.com","subject":"[PATCH 3/7] submodule-config: keep labels around","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-05-11T00:59:53Z","receivedAt":"2016-05-11T00:59:53Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"We need the submodule labels in a later patch.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n submodule-config.c | 16 ++++++++++++++++\n submodule-config.h |  2 ++\n 2 files changed, 18 insertions(+)\n\ndiff --git a/submodule-config.c b/submodule-config.c\nindex b82d1fb..0cdb47e 100644\n--- a/submodule-config.c\n+++ b/submodule-config.c\n@@ -60,6 +60,10 @@ static void free_one_config(struct submodule_entry *entry)\n \tfree((void *) entry->config->path);\n \tfree((void *) entry->config->name);\n \tfree((void *) entry->config->update_strategy.command);\n+\tif (entry->config->labels) {\n+\t\tstring_list_clear(entry->config->labels, 0);\n+\t\tfree(entry->config->labels);\n+\t}\n \tfree(entry->config);\n }\n \n@@ -199,6 +203,7 @@ static struct submodule *lookup_or_create_by_name(struct submodule_cache *cache,\n \tsubmodule->update_strategy.command = NULL;\n \tsubmodule->fetch_recurse = RECURSE_SUBMODULES_NONE;\n \tsubmodule->ignore = NULL;\n+\tsubmodule->labels = NULL;\n \n \thashcpy(submodule->gitmodules_sha1, gitmodules_sha1);\n \n@@ -353,6 +358,17 @@ static int parse_config(const char *var, const char *value, void *data)\n \t\telse if (parse_submodule_update_strategy(value,\n \t\t\t &submodule->update_strategy) < 0)\n \t\t\t\tdie(_(\"invalid value for %s\"), var);\n+\t} else if (!strcmp(item.buf, \"label\")) {\n+\t\tif (!value)\n+\t\t\tret = config_error_nonbool(var);\n+\t\telse {\n+\t\t\tif (!submodule->labels) {\n+\t\t\t\tsubmodule->labels =\n+\t\t\t\t\txmalloc(sizeof(*submodule->labels));\n+\t\t\t\tstring_list_init(submodule->labels, 1);\n+\t\t\t}\n+\t\t\tstring_list_insert(submodule->labels, value);\n+\t\t}\n \t}\n \n \tstrbuf_release(&name);\ndiff --git a/submodule-config.h b/submodule-config.h\nindex e4857f5..d57da59 100644\n--- a/submodule-config.h\n+++ b/submodule-config.h\n@@ -18,6 +18,8 @@ struct submodule {\n \tstruct submodule_update_strategy update_strategy;\n \t/* the sha1 blob id of the responsible .gitmodules file */\n \tunsigned char gitmodules_sha1[20];\n+\t/* sorted, not as on disk */\n+\tstruct string_list *labels;\n };\n \n int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg);\n-- \n2.8.0.35.g58985d9.dirty\n"},{"id":"286139","messageId":"1462928397-1708-5-git-send-email-sbeller@google.com","threadId":"42279","inReplyTo":"1462928397-1708-1-git-send-email-sbeller@google.com","subject":"[PATCH 4/7] submodule-config: check if a submodule is in a group","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-05-11T00:59:54Z","receivedAt":"2016-05-11T00:59:54Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"In later patches we need to tell if a submodule is in a group,\nso expose a handy test function in both C and shell.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n builtin/submodule--helper.c  | 42 +++++++++++++++++++++++++++++++-\n submodule-config.c           | 50 ++++++++++++++++++++++++++++++++++++++\n submodule-config.h           |  3 +++\n t/t7412-submodule--helper.sh | 58 ++++++++++++++++++++++++++++++++++++++++++++\n 4 files changed, 152 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex d3f4684..6ffd1c1 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -858,6 +858,45 @@ static int valid_label_name(int argc, const char **argv, const char *prefix)\n \t      \"and must contain alphanumeric characters or dashes only.\"));\n }\n \n+static int in_group(int argc, const char **argv, const char *prefix)\n+{\n+\tconst struct string_list *list;\n+\tstruct string_list actual_list = STRING_LIST_INIT_DUP;\n+\tconst struct submodule *sub;\n+\tconst char *group = NULL;\n+\n+\tstruct option default_group_options[] = {\n+\t\tOPT_STRING('g', \"group\", &group, N_(\"group\"),\n+\t\t\t\tN_(\"comma separated group specifier for submodules\")),\n+\t\tOPT_END()\n+\t};\n+\n+\tconst char *const git_submodule_helper_usage[] = {\n+\t\tN_(\"git submodule--helper in-group <path>\"),\n+\t\tNULL\n+\t};\n+\n+\targc = parse_options(argc, argv, prefix, default_group_options,\n+\t\t\t     git_submodule_helper_usage, 0);\n+\n+\tgitmodules_config();\n+\tgit_config(submodule_config, NULL);\n+\n+\tif (argc != 1)\n+\t\tusage(git_submodule_helper_usage[0]);\n+\n+\tsub = submodule_from_path(null_sha1, argv[0]);\n+\n+\tif (!group)\n+\t\tlist = git_config_get_value_multi(\"submodule.updateGroup\");\n+\telse {\n+\t\tstring_list_split(&actual_list, group, ',', -1);\n+\t\tlist = &actual_list;\n+\t}\n+\n+\treturn !submodule_in_group(list, sub);\n+}\n+\n struct cmd_struct {\n \tconst char *cmd;\n \tint (*fn)(int, const char **, const char *);\n@@ -871,7 +910,8 @@ static struct cmd_struct commands[] = {\n \t{\"resolve-relative-url\", resolve_relative_url},\n \t{\"resolve-relative-url-test\", resolve_relative_url_test},\n \t{\"init\", module_init},\n-\t{\"valid-label-name\", valid_label_name}\n+\t{\"valid-label-name\", valid_label_name},\n+\t{\"in-group\", in_group}\n };\n \n int cmd_submodule__helper(int argc, const char **argv, const char *prefix)\ndiff --git a/submodule-config.c b/submodule-config.c\nindex 0cdb47e..7f38ebd 100644\n--- a/submodule-config.c\n+++ b/submodule-config.c\n@@ -522,3 +522,53 @@ void submodule_free(void)\n \tcache_free(&cache);\n \tis_cache_init = 0;\n }\n+\n+int submodule_in_group(const struct string_list *group,\n+\t\t       const struct submodule *sub)\n+{\n+\tint matched = 0;\n+\tstruct strbuf sb = STRBUF_INIT;\n+\n+\tif (!group)\n+\t\t/*\n+\t\t * If no group is specified at all, all submodules match to\n+\t\t * keep traditional behavior.\n+\t\t */\n+\t\treturn 1;\n+\n+\tif (sub->labels) {\n+\t\tstruct string_list_item *item;\n+\t\tfor_each_string_list_item(item, sub->labels) {\n+\t\t\tstrbuf_reset(&sb);\n+\t\t\tstrbuf_addf(&sb, \"*%s\", item->string);\n+\t\t\tif (string_list_has_string(group, sb.buf)) {\n+\t\t\t\tmatched = 1;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t}\n+\t}\n+\tif (sub->path) {\n+\t\t/*\n+\t\t * NEEDSWORK: This currently works only for\n+\t\t * exact paths, but we want to enable\n+\t\t * inexact matches such wildcards.\n+\t\t */\n+\t\tstrbuf_reset(&sb);\n+\t\tstrbuf_addf(&sb, \"./%s\", sub->path);\n+\t\tif (string_list_has_string(group, sb.buf))\n+\t\t\tmatched = 1;\n+\t}\n+\tif (sub->name) {\n+\t\t/*\n+\t\t * NEEDSWORK: Same as with path. Do we want to\n+\t\t * support wildcards or such?\n+\t\t */\n+\t\tstrbuf_reset(&sb);\n+\t\tstrbuf_addf(&sb, \":%s\", sub->name);\n+\t\tif (string_list_has_string(group, sb.buf))\n+\t\t\tmatched = 1;\n+\t}\n+\tstrbuf_release(&sb);\n+\n+\treturn matched;\n+}\ndiff --git a/submodule-config.h b/submodule-config.h\nindex d57da59..4c696cc 100644\n--- a/submodule-config.h\n+++ b/submodule-config.h\n@@ -31,4 +31,7 @@ const struct submodule *submodule_from_path(const unsigned char *commit_sha1,\n \t\tconst char *path);\n void submodule_free(void);\n \n+int submodule_in_group(const struct string_list *group,\n+\t\t       const struct submodule *sub);\n+\n #endif /* SUBMODULE_CONFIG_H */\ndiff --git a/t/t7412-submodule--helper.sh b/t/t7412-submodule--helper.sh\nindex 3af315c..042f3f5 100755\n--- a/t/t7412-submodule--helper.sh\n+++ b/t/t7412-submodule--helper.sh\n@@ -46,4 +46,62 @@ test_expect_success 'valid-label-name fails with utf8 characters' '\n \ttest_i18ngrep alphanumeric actual\n '\n \n+test_expect_success 'setup superproject with submodules' '\n+\n+\tmkdir sub &&\n+\t(\n+\t\tcd sub &&\n+\t\tgit init &&\n+\t\ttest_commit test\n+\t\ttest_commit test2\n+\t) &&\n+\tmkdir super &&\n+\t(\n+\t\tcd super &&\n+\t\tgit init &&\n+\t\tgit submodule add ../sub sub0 &&\n+\t\tgit submodule add -l bit1 ../sub sub1 &&\n+\t\tgit submodule add -l bit2 ../sub sub2 &&\n+\t\tgit submodule add -l bit2 -l bit1 ../sub sub3 &&\n+\t\tgit submodule add ../sub sub_name &&\n+\t\tgit mv sub_name sub_path &&\n+\t\tgit commit -m \"add labeled submodules\"\n+\t)\n+'\n+\n+test_expect_success 'in-group' '\n+\t(\n+\t\tcd super &&\n+\t\t# we do not specify a group nor have set a default group,\n+\t\t# any submodule should be in the default group:\n+\t\tgit submodule--helper in-group sub0 &&\n+\t\tgit submodule--helper in-group sub1 &&\n+\t\tgit submodule--helper in-group sub2 &&\n+\t\tgit submodule--helper in-group sub3 &&\n+\n+\t\t# test bit1:\n+\t\ttest_must_fail git submodule--helper in-group --group=\\*bit1 sub0 &&\n+\t\t\t       git submodule--helper in-group --group=\\*bit1 sub1 &&\n+\t\ttest_must_fail git submodule--helper in-group --group=\\*bit1 sub2 &&\n+\t\t\t       git submodule--helper in-group --group=\\*bit1 sub3 &&\n+\t\ttest_must_fail git submodule--helper in-group --group=\\*bit1 sub_path &&\n+\n+\t\t# test by path:\n+\t\t\t       git submodule--helper in-group --group=./sub0 sub0 &&\n+\t\ttest_must_fail git submodule--helper in-group --group=./sub0 sub1 &&\n+\t\ttest_must_fail git submodule--helper in-group --group=./sub0 sub_path &&\n+\n+\t\t# tests by name:\n+\t\t\t       git submodule--helper in-group --group=:sub0 sub0 &&\n+\t\ttest_must_fail git submodule--helper in-group --group=:sub0 sub1 &&\n+\t\t\t       git submodule--helper in-group --group=:sub_name sub_path &&\n+\n+\t\t# logical OR of path and labels\n+\t\t\t       git submodule--helper in-group --group=\\*bit1,./sub0 sub0 &&\n+\t\t\t       git submodule--helper in-group --group=\\*bit1,./sub0 sub1 &&\n+\t\ttest_must_fail git submodule--helper in-group --group=\\*bit1,./sub0 sub2 &&\n+\t\t\t       git submodule--helper in-group --group=\\*bit1,./sub0 sub3\n+\t)\n+'\n+\n test_done\n-- \n2.8.0.35.g58985d9.dirty\n"},{"id":"286137","messageId":"1462928397-1708-6-git-send-email-sbeller@google.com","threadId":"42279","inReplyTo":"1462928397-1708-1-git-send-email-sbeller@google.com","subject":"[PATCH 5/7] submodule--helper module_list_compute: allow label or name arguments","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-05-11T00:59:55Z","receivedAt":"2016-05-11T00:59:55Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Additionally to taking a pathspec, `module_list_compute` will also take\nlabels and submodule names, when these are prefixed by '*' and ':'\nrespectively.\n\n`module_list_compute` is used by other functions in the submodule helper:\n* module_list, used by `submodule {deinit, status, sync, foreach}`\n* module_init, used by `submodule init`\n* update_clone, used by `submodule update`\n\n{foreach, update} do not pass on command line arguments to the listing\nfunction such that these are not affected. The rest of them {deinit,\nstatus, sync, init} will be exercised in additional tests.\n\nAs the labeling requires lookup of .gitmodules files, we need to make sure\nthe submodule config cache is initialized in all the functions, that's why\nthere are additional calls to gitmodules_config() and\tgit_config(...);\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n Documentation/git-submodule.txt | 22 +++++++++--\n builtin/submodule--helper.c     | 76 +++++++++++++++++++++++++++++++-----\n git-submodule.sh                |  8 ++--\n t/t7412-submodule--helper.sh    | 86 +++++++++++++++++++++++++++++++++++++++++\n 4 files changed, 175 insertions(+), 17 deletions(-)\n\ndiff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt\nindex 9ba8895..35ca355 100644\n--- a/Documentation/git-submodule.txt\n+++ b/Documentation/git-submodule.txt\n@@ -11,16 +11,16 @@ SYNOPSIS\n [verse]\n 'git submodule' [--quiet] add [-b <branch>] [-f|--force] [-l|--label <label>]\n \t      [--reference <repository>] [--depth <depth>] [--] <repository> [<path>]\n-'git submodule' [--quiet] status [--cached] [--recursive] [--] [<path>...]\n-'git submodule' [--quiet] init [--] [<path>...]\n-'git submodule' [--quiet] deinit [-f|--force] [--] <path>...\n+'git submodule' [--quiet] status [--cached] [--recursive] [--] [<submodulespec>...]\n+'git submodule' [--quiet] init [--] [<submodulespec>...]\n+'git submodule' [--quiet] deinit [-f|--force] [--] <submodulespec>...\n 'git submodule' [--quiet] update [--init] [--remote] [-N|--no-fetch]\n \t      [-f|--force] [--rebase|--merge] [--reference <repository>]\n \t      [--depth <depth>] [--recursive] [--jobs <n>] [--] [<path>...]\n 'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) <n>]\n \t      [commit] [--] [<path>...]\n 'git submodule' [--quiet] foreach [--recursive] <command>\n-'git submodule' [--quiet] sync [--recursive] [--] [<path>...]\n+'git submodule' [--quiet] sync [--recursive] [--] [<submodulespec>...]\n \n \n DESCRIPTION\n@@ -37,6 +37,20 @@ these will not be checked out by default; the 'init' and 'update'\n subcommands will maintain submodules checked out and at\n appropriate revision in your working tree.\n \n+When working on submodules you can specify the desired submodule\n+group or give no specification at all to apply the command to the\n+default group of submodules, which includes all submodules.\n+To group submodules you can make use of\n+ . a pathspec,\n+ . their name,\n+ . labels.\n+It is undefined behavior to give a spec that is ambigious for a name,\n+pathspec or label. To make the specification unambigous, you can prefix\n+ . pathspecs with './',\n+ . names with ':',\n+ . labels with '*'.\n+Labels must consist of alphanumeric characters or the dash character.\n+\n Submodules are composed from a so-called `gitlink` tree entry\n in the main repository that refers to a particular commit object\n within the inner repository that is completely separate.\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 6ffd1c1..ba43c80 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -12,6 +12,7 @@\n #include \"remote.h\"\n #include \"refs.h\"\n #include \"connect.h\"\n+#include \"argv-array.h\"\n \n static char *get_default_remote(void)\n {\n@@ -215,6 +216,36 @@ static int resolve_relative_url_test(int argc, const char **argv, const char *pr\n \treturn 0;\n }\n \n+static void split_argv_pathspec_groups(int argc, const char **argv,\n+\t\t\t\t       const char ***pathspec_argv,\n+\t\t\t\t       struct string_list *group)\n+{\n+\tint i;\n+\tstruct argv_array ps = ARGV_ARRAY_INIT;\n+\tfor (i = 0; i < argc; i++) {\n+\t\tif (starts_with(argv[i], \"*\")\n+\t\t    || starts_with(argv[i], \":\")) {\n+\t\t\tstring_list_insert(group, argv[i]);\n+\t\t} else if (starts_with(argv[i], \"./\")) {\n+\t\t\targv_array_push(&ps, argv[i]);\n+\t\t} else {\n+\t\t\t/*\n+\t\t\t* NEEDSWORK:\n+\t\t\t* Improve autodetection of items with no prefixes,\n+\t\t\t* roughly like\n+\t\t\t* if (label_or_name_exists(argv[i]))\n+\t\t\t*\tstring_list_insert(group, argv[i]);\n+\t\t\t* else\n+\t\t\t*\targv_array_push(&ps, argv[i]);\n+\t\t\t*/\n+\t\t\targv_array_push(&ps, argv[i]);\n+\t\t}\n+\t}\n+\n+\t*pathspec_argv = argv_array_detach(&ps);\n+\targv_array_clear(&ps);\n+}\n+\n struct module_list {\n \tconst struct cache_entry **entries;\n \tint alloc, nr;\n@@ -228,10 +259,15 @@ static int module_list_compute(int argc, const char **argv,\n {\n \tint i, result = 0;\n \tchar *ps_matched = NULL;\n+\tconst char **pathspec_argv;\n+\tstruct string_list group = STRING_LIST_INIT_DUP;\n+\n+\tsplit_argv_pathspec_groups(argc, argv, &pathspec_argv, &group);\n+\n \tparse_pathspec(pathspec, 0,\n \t\t       PATHSPEC_PREFER_FULL |\n \t\t       PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,\n-\t\t       prefix, argv);\n+\t\t       prefix, pathspec_argv);\n \n \tif (pathspec->nr)\n \t\tps_matched = xcalloc(pathspec->nr, 1);\n@@ -242,10 +278,27 @@ static int module_list_compute(int argc, const char **argv,\n \tfor (i = 0; i < active_nr; i++) {\n \t\tconst struct cache_entry *ce = active_cache[i];\n \n-\t\tif (!match_pathspec(pathspec, ce->name, ce_namelen(ce),\n-\t\t\t\t    0, ps_matched, 1) ||\n-\t\t    !S_ISGITLINK(ce->ce_mode))\n-\t\t\tcontinue;\n+\t\tif (!group.nr) {\n+\t\t\tif (!match_pathspec(pathspec, ce->name, ce_namelen(ce),\n+\t\t\t\t\t    0, ps_matched, 1) ||\n+\t\t\t    !S_ISGITLINK(ce->ce_mode))\n+\t\t\t\tcontinue;\n+\t\t} else {\n+\t\t\tconst struct submodule *sub;\n+\t\t\tint ps = 0, gr = 0;\n+\t\t\tif (!S_ISGITLINK(ce->ce_mode))\n+\t\t\t\tcontinue;\n+\t\t\tsub = submodule_from_path(null_sha1, ce->name);\n+\n+\t\t\tps = match_pathspec(pathspec, ce->name, ce_namelen(ce),\n+\t\t\t\t\t    0, ps_matched, 1);\n+\t\t\tgr = submodule_in_group(&group, sub);\n+\n+\t\t\tif (!pathspec->nr && !gr)\n+\t\t\t\tcontinue;\n+\t\t\tif (!ps && !gr)\n+\t\t\t\tcontinue;\n+\t\t}\n \n \t\tALLOC_GROW(list->entries, list->nr + 1, list->alloc);\n \t\tlist->entries[list->nr++] = ce;\n@@ -287,6 +340,9 @@ static int module_list(int argc, const char **argv, const char *prefix)\n \targc = parse_options(argc, argv, prefix, module_list_options,\n \t\t\t     git_submodule_helper_usage, 0);\n \n+\tgitmodules_config();\n+\tgit_config(submodule_config, NULL);\n+\n \tif (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0) {\n \t\tprintf(\"#unmatched\\n\");\n \t\treturn 1;\n@@ -415,6 +471,9 @@ static int module_init(int argc, const char **argv, const char *prefix)\n \targc = parse_options(argc, argv, prefix, module_init_options,\n \t\t\t     git_submodule_helper_usage, 0);\n \n+\tgitmodules_config();\n+\tgit_config(submodule_config, NULL);\n+\n \tif (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)\n \t\treturn 1;\n \n@@ -795,16 +854,15 @@ static int update_clone(int argc, const char **argv, const char *prefix)\n \t\tif (parse_submodule_update_strategy(update, &suc.update) < 0)\n \t\t\tdie(_(\"bad value for update parameter\"));\n \n+\tgitmodules_config();\n+\tgit_config(submodule_config, NULL);\n+\n \tif (module_list_compute(argc, argv, prefix, &pathspec, &suc.list) < 0)\n \t\treturn 1;\n \n \tif (pathspec.nr)\n \t\tsuc.warn_if_uninitialized = 1;\n \n-\t/* Overlay the parsed .gitmodules file with .git/config */\n-\tgitmodules_config();\n-\tgit_config(submodule_config, NULL);\n-\n \tif (max_jobs < 0)\n \t\tmax_jobs = parallel_submodules();\n \ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex c1213d8..c8e36c5 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -6,13 +6,13 @@\n \n dashless=$(basename \"$0\" | sed -e 's/-/ /')\n USAGE=\"[--quiet] add [-b <branch>] [-f|--force] [--name <name>] [--reference <repository>] [-l|--label <label>][--] <repository> [<path>]\n-   or: $dashless [--quiet] status [--cached] [--recursive] [--] [<path>...]\n-   or: $dashless [--quiet] init [--] [<path>...]\n-   or: $dashless [--quiet] deinit [-f|--force] [--] <path>...\n+   or: $dashless [--quiet] status [--cached] [--recursive] [--] [<submodulespec>...]\n+   or: $dashless [--quiet] init [--] [<submodulespec>...]\n+   or: $dashless [--quiet] deinit [-f|--force] [--] <submodulespec>...\n    or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] [-f|--force] [--checkout|--merge|--rebase] [--reference <repository>] [--recursive] [--] [<path>...]\n    or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]\n    or: $dashless [--quiet] foreach [--recursive] <command>\n-   or: $dashless [--quiet] sync [--recursive] [--] [<path>...]\"\n+   or: $dashless [--quiet] sync [--recursive] [--] [<submodulespec>...]\"\n OPTIONS_SPEC=\n SUBDIRECTORY_OK=Yes\n . git-sh-setup\ndiff --git a/t/t7412-submodule--helper.sh b/t/t7412-submodule--helper.sh\nindex 042f3f5..0f2e5c6 100755\n--- a/t/t7412-submodule--helper.sh\n+++ b/t/t7412-submodule--helper.sh\n@@ -104,4 +104,90 @@ test_expect_success 'in-group' '\n \t)\n '\n \n+test_expect_success 'submodule init respects label' '\n+\ttest_when_finished \"rm -rf super_clone\" &&\n+\tsuburl=$(pwd)/sub &&\n+\tgit clone super super_clone &&\n+\t(\n+\t\tcd super_clone &&\n+\t\tgit submodule init \\*bit1 &&\n+\t\ttest_must_fail git config submodule.sub0.url &&\n+\t\ttest        \"$(git config submodule.sub1.url)\" = \"$suburl\" &&\n+\t\ttest_must_fail git config submodule.sub2.url &&\n+\t\ttest        \"$(git config submodule.sub3.url)\" = \"$suburl\"\n+\t)\n+'\n+\n+test_expect_success 'submodule init respects name' '\n+\ttest_when_finished \"rm -rf super_clone\" &&\n+\tsuburl=$(pwd)/sub &&\n+\tgit clone super super_clone &&\n+\t(\n+\t\tcd super_clone &&\n+\t\tgit submodule init :sub_name &&\n+\t\ttest_must_fail git config submodule.sub0.url &&\n+\t\ttest \"$(git config submodule.sub_name.url)\" = \"$suburl\"\n+\t)\n+'\n+\n+test_expect_success 'submodule deinit respects label' '\n+\tsuburl=$(pwd)/sub &&\n+\t(\n+\t\tcd super &&\n+\t\tgit submodule init &&\n+\t\tgit submodule deinit \\*bit2 &&\n+\t\ttest        \"$(git config submodule.sub0.url)\" = \"$suburl\" &&\n+\t\ttest        \"$(git config submodule.sub1.url)\" = \"$suburl\" &&\n+\t\ttest_must_fail git config submodule.sub2.url &&\n+\t\ttest_must_fail git config submodule.sub3.url\n+\t)\n+'\n+\n+test_expect_success 'submodule deinit respects label and path' '\n+\tsuburl=$(pwd)/sub &&\n+\t(\n+\t\tcd super &&\n+\t\tgit submodule init &&\n+\t\tgit submodule deinit \\*bit2 ./sub1 &&\n+\t\ttest        \"$(git config submodule.sub0.url)\" = \"$suburl\" &&\n+\t\ttest_must_fail git config submodule.sub1.url &&\n+\t\ttest_must_fail git config submodule.sub2.url &&\n+\t\ttest_must_fail git config submodule.sub3.url\n+\t)\n+'\n+\n+cat >expect <<-EOF\n+-sub1\n+-sub3\n+EOF\n+\n+test_expect_success 'submodule status respects label and path' '\n+\tsuburl=$(pwd)/sub &&\n+\t(\n+\t\tcd super &&\n+\t\tgit submodule deinit .\n+\t\tgit submodule init &&\n+\t\tgit submodule status \\*bit1 ./sub1 | cut -c 1,43- >../actual\n+\t) &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'submodule sync respects name and path' '\n+\tsuburl=$(pwd)/sub &&\n+\t(\n+\t\tcd super &&\n+\t\tgit submodule init &&\n+\t\tgit config submodule.\"sub0\".url \"wrong\" &&\n+\t\tgit config submodule.\"sub1\".url \"wrong\" &&\n+\t\tgit config submodule.\"sub2\".url \"wrong\" &&\n+\t\tgit config submodule.\"sub3\".url \"wrong\" &&\n+\t\tgit submodule sync :sub_name ./sub1 | cut -c 1,43- >../actual &&\n+\t\ttest \"$(git config submodule.sub0.url)\" = \"wrong\" &&\n+\t\ttest \"$(git config submodule.sub1.url)\" = \"$suburl\" &&\n+\t\ttest \"$(git config submodule.sub2.url)\" = \"wrong\" &&\n+\t\ttest \"$(git config submodule.sub3.url)\" = \"wrong\" &&\n+\t\ttest \"$(git config submodule.sub_name.url)\" = \"$suburl\"\n+\t)\n+'\n+\n test_done\n-- \n2.8.0.35.g58985d9.dirty\n"},{"id":"286138","messageId":"1462928397-1708-7-git-send-email-sbeller@google.com","threadId":"42279","inReplyTo":"1462928397-1708-1-git-send-email-sbeller@google.com","subject":"[PATCH 6/7] submodule update: learn partial initialization","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-05-11T00:59:56Z","receivedAt":"2016-05-11T00:59:56Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The new switch `--init-default-group` updates the submodules which are\nconfigured in `submodule.updateGroup`\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n Documentation/config.txt        |  5 ++++\n Documentation/git-submodule.txt |  4 ++--\n git-submodule.sh                | 14 +++++++++--\n t/t7400-submodule-basic.sh      | 53 +++++++++++++++++++++++++++++++++++++++++\n 4 files changed, 72 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 59d7046..0f20019 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2735,6 +2735,11 @@ submodule.fetchJobs::\n \tin parallel. A value of 0 will give some reasonable default.\n \tIf unset, it defaults to 1.\n \n+submodule.updateGroup::\n+\tSpecifies the group of submodules when `git submodule --init-group`\n+\tis called with no arguments. This setting is recorded in the initial\n+\tclone when `--init-submodule` was given.\n+\n tag.sort::\n \tThis variable controls the sort ordering of tags when displayed by\n \tlinkgit:git-tag[1]. Without the \"--sort=<value>\" option provided, the\ndiff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt\nindex 35ca355..e658d15 100644\n--- a/Documentation/git-submodule.txt\n+++ b/Documentation/git-submodule.txt\n@@ -14,9 +14,9 @@ SYNOPSIS\n 'git submodule' [--quiet] status [--cached] [--recursive] [--] [<submodulespec>...]\n 'git submodule' [--quiet] init [--] [<submodulespec>...]\n 'git submodule' [--quiet] deinit [-f|--force] [--] <submodulespec>...\n-'git submodule' [--quiet] update [--init] [--remote] [-N|--no-fetch]\n+'git submodule' [--quiet] update [--init[-default-group]] [--remote] [-N|--no-fetch]\n \t      [-f|--force] [--rebase|--merge] [--reference <repository>]\n-\t      [--depth <depth>] [--recursive] [--jobs <n>] [--] [<path>...]\n+\t      [--depth <depth>] [--recursive] [--jobs <n>] [--] [<submodulespec>...]\n 'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) <n>]\n \t      [commit] [--] [<path>...]\n 'git submodule' [--quiet] foreach [--recursive] <command>\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex c8e36c5..2b0b0cb 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -522,7 +522,12 @@ cmd_update()\n \t\t\tGIT_QUIET=1\n \t\t\t;;\n \t\t-i|--init)\n-\t\t\tinit=1\n+\t\t\ttest -z $init || test $init = by_args || die \"$(gettext \"Only one of --init or --init-default-group may be used.\")\"\n+\t\t\tinit=by_args\n+\t\t\t;;\n+\t\t--init-default-group)\n+\t\t\ttest -z $init || test $init = by_config || die \"$(gettext \"Only one of --init or --init-default-group may be used.\")\"\n+\t\t\tinit=by_config\n \t\t\t;;\n \t\t--remote)\n \t\t\tremote=1\n@@ -585,7 +590,12 @@ cmd_update()\n \n \tif test -n \"$init\"\n \tthen\n-\t\tcmd_init \"--\" \"$@\" || return\n+\t\tadditional_init=\n+\t\tif test \"$init\" = \"by_config\"\n+\t\tthen\n+\t\t\tadditional_init=$(git config --get-all submodule.updateGroup)\n+\t\tfi\n+\t\tcmd_init \"--\" \"$@\" ${additional_init:+$additional_init} || return\n \tfi\n \n \t{\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex 0adc4e4..41e65c2 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -1139,4 +1139,57 @@ test_expect_success 'submodule add recording wrong labels reports an error' '\n \ttest_i18ngrep alphanumeric actual\n '\n \n+test_expect_success 'setup superproject with labeled submodules' '\n+\tmkdir sub1 &&\n+\t(\n+\t\tcd sub1 &&\n+\t\tgit init &&\n+\t\ttest_commit test\n+\t\ttest_commit test2\n+\t) &&\n+\tmkdir labeledsuper &&\n+\t(\n+\t\tcd labeledsuper &&\n+\t\tgit init &&\n+\t\tgit submodule add ../sub1 sub0 &&\n+\t\tgit submodule add -l bit1 ../sub1 sub1 &&\n+\t\tgit submodule add -l bit2 ../sub1 sub2 &&\n+\t\tgit submodule add -l bit2 -l bit1 ../sub1 sub3 &&\n+\t\tgit commit -m \"add labeled submodules\"\n+\t)\n+'\n+\n+cat >expect <<-EOF\n+-sub0\n+ sub1 (test2)\n+ sub2 (test2)\n+ sub3 (test2)\n+EOF\n+\n+test_expect_success 'submodule update --init with a group' '\n+\ttest_when_finished \"rm -rf labeledsuper_clone\" &&\n+\tpwd=$(pwd) &&\n+\tgit clone file://\"$pwd\"/labeledsuper labeledsuper_clone &&\n+\t(\n+\t\tcd labeledsuper_clone &&\n+\t\tgit submodule update --init \\*bit1 ./sub2 &&\n+\t\tgit submodule status |cut -c 1,43- >../actual\n+\t) &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'submodule update --init-default-group' '\n+\ttest_when_finished \"rm -rf labeledsuper_clone\" &&\n+\tpwd=$(pwd) &&\n+\tgit clone file://\"$pwd\"/labeledsuper labeledsuper_clone &&\n+\t(\n+\t\tcd labeledsuper_clone &&\n+\t\tgit config submodule.updateGroup \\*bit1 &&\n+\t\tgit config --add submodule.updateGroup ./sub2 &&\n+\t\tgit submodule update --init-default-group &&\n+\t\tgit submodule status |cut -c 1,43- >../actual\n+\t) &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.8.0.35.g58985d9.dirty\n"},{"id":"286135","messageId":"1462928397-1708-8-git-send-email-sbeller@google.com","threadId":"42279","inReplyTo":"1462928397-1708-1-git-send-email-sbeller@google.com","subject":"[PATCH 7/7] clone: allow specification of submodules to be cloned","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-05-11T00:59:57Z","receivedAt":"2016-05-11T00:59:57Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This allows to specify a subset of all available submodules to be\ninitialized and cloned. It is unrelated to the `--recursive` option,\ni.e. the user may still want to give `--recursive` as an option.\n\nOriginally `--recursive` implied to initialize all submodules, this\nchanges as well with the new option, such that only the specified\nsubmodules are cloned, and their submodules (i.e. subsubmodules)\nare cloned in full as the submodule specification is not passed on.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n Documentation/git-clone.txt | 26 +++++++++++----\n builtin/clone.c             | 40 +++++++++++++++++++++--\n t/t7400-submodule-basic.sh  | 79 +++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 136 insertions(+), 9 deletions(-)\n\ndiff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\nindex 45d74be..4a9e8bb 100644\n--- a/Documentation/git-clone.txt\n+++ b/Documentation/git-clone.txt\n@@ -14,7 +14,8 @@ SYNOPSIS\n \t  [-o <name>] [-b <name>] [-u <upload-pack>] [--reference <repository>]\n \t  [--dissociate] [--separate-git-dir <git dir>]\n \t  [--depth <depth>] [--[no-]single-branch]\n-\t  [--recursive | --recurse-submodules] [--jobs <n>] [--] <repository>\n+\t  [--recursive | --recurse-submodules] [--jobs <n>]\n+\t  [--init-submodule <submodulespec>] [--] <repository>\n \t  [<directory>]\n \n DESCRIPTION\n@@ -205,12 +206,23 @@ objects from the source repository into a pack in the cloned repository.\n \n --recursive::\n --recurse-submodules::\n-\tAfter the clone is created, initialize all submodules within,\n-\tusing their default settings. This is equivalent to running\n-\t`git submodule update --init --recursive` immediately after\n-\tthe clone is finished. This option is ignored if the cloned\n-\trepository does not have a worktree/checkout (i.e. if any of\n-\t`--no-checkout`/`-n`, `--bare`, or `--mirror` is given)\n+\tAfter the clone is created, initialize and clone all submodules\n+\twithin, using their default settings. This is equivalent to\n+\trunning `git submodule update --recursive --init <submodulespec>`\n+\timmediately after the clone is finished. This option is ignored\n+\tif the cloned repository does not have a worktree/checkout (i.e.\n+\tif any of `--no-checkout`/`-n`, `--bare`, or `--mirror` is given)\n+\n+--init-submodule::\n+\tAfter the clone is cloned, initialize and clone specified\n+\tsubmodules within, using their default settings. It is possible\n+\tto give multiple specifications by giving this argument multiple\n+\ttimes or by giving a comma separated list. This is equivalent to\n+\trunning `git submodule update <submodulespec>` immediately\n+\tafter the clone is finished. To specify submodules you can use\n+\ttheir path, name or labels, see linkgit:git-submodules[1]. This\n+\toption will be recorded in the repository config as\n+\t`submodule.updateGroup`.\n \n --separate-git-dir=<git dir>::\n \tInstead of placing the cloned repository where it is supposed\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 6576ecf..fa2f989 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -52,6 +52,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 init_submodules;\n+\n+static int init_submodules_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@@ -100,6 +116,8 @@ static struct option builtin_clone_options[] = {\n \t\t\tTRANSPORT_FAMILY_IPV4),\n \tOPT_SET_INT('6', \"ipv6\", &family, N_(\"use IPv6 addresses only\"),\n \t\t\tTRANSPORT_FAMILY_IPV6),\n+\tOPT_CALLBACK(0, \"init-submodule\", &init_submodules, N_(\"string\"),\n+\t\t\tN_(\"clone specific submodules\"), init_submodules_cb),\n \tOPT_END()\n };\n \n@@ -731,13 +749,22 @@ 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 && (option_recursive || init_submodules.nr > 0)) {\n \t\tstruct argv_array args = ARGV_ARRAY_INIT;\n-\t\targv_array_pushl(&args, \"submodule\", \"update\", \"--init\", \"--recursive\", NULL);\n+\t\tstruct string_list_item *item;\n+\t\targv_array_pushl(&args, \"submodule\", \"update\", NULL);\n+\n+\t\targv_array_pushf(&args, \"--init\");\n+\n+\t\tif (option_recursive)\n+\t\t\targv_array_pushf(&args, \"--recursive\");\n \n \t\tif (max_jobs != -1)\n \t\t\targv_array_pushf(&args, \"--jobs=%d\", max_jobs);\n \n+\t\tfor_each_string_list_item(item, &init_submodules)\n+\t\t\targv_array_push(&args, item->string);\n+\n \t\terr = run_command_v_opt(args.argv, RUN_GIT_CMD);\n \t\targv_array_clear(&args);\n \t}\n@@ -876,6 +903,15 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\toption_no_checkout = 1;\n \t}\n \n+\tif (init_submodules.nr > 0) {\n+\t\tstruct string_list_item *item;\n+\t\tstruct strbuf sb = STRBUF_INIT;\n+\t\tfor_each_string_list_item(item, &init_submodules) {\n+\t\t\tstrbuf_addf(&sb, \"submodule.updateGroup=%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\";\n \ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex 41e65c2..e7b6c1f 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -1192,4 +1192,83 @@ test_expect_success 'submodule update --init-default-group' '\n \ttest_cmp expect actual\n '\n \n+cat <<EOF > expected\n+-sub0\n+ sub1 (test2)\n+-sub2\n+ sub3 (test2)\n+EOF\n+\n+test_expect_success 'clone --init-submodule works' '\n+\ttest_when_finished \"rm -rf super super_clone\" &&\n+\tgit clone --recurse-submodules --init-submodule \\*bit1 labeledsuper super_clone &&\n+\t(\n+\t\tcd super_clone &&\n+\t\tgit submodule status |cut -c 1,43- >../actual\n+\t) &&\n+\ttest_cmp actual expected\n+'\n+\n+cat <<EOF > expect\n+ sub0 (test2)\n+ sub1 (test2)\n+-sub2\n+ sub3 (test2)\n+EOF\n+test_expect_success 'clone with multiple --init-submodule options' '\n+\ttest_when_finished \"rm -rf super super_clone\" &&\n+\tgit clone --recurse-submodules --init-submodule=\\*bit1 --init-submodule ./sub0 labeledsuper super_clone &&\n+\t(\n+\t\tcd super_clone &&\n+\t\tgit submodule status |cut -c1,43- >../actual\n+\t) &&\n+\ttest_cmp expect actual\n+'\n+\n+cat <<EOF > expect\n+ submoduleA (test2)\n+-submoduleB\n+EOF\n+\n+cat <<EOF > expect2\n+ submoduleA (test2)\n+-submoduleB\n+ submoduleC (test2)\n+EOF\n+\n+test_expect_success 'clone and subsequent updates correctly auto-initialize submodules' '\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 --label LA file://\"$pwd\"/sub1 submoduleA &&\n+\t\tgit submodule add file://\"$pwd\"/sub1 submoduleB &&\n+\t\tgit commit -a -m \"create repository with submodules groups\"\n+\t) &&\n+\tgit clone --recurse-submodules --init-submodule=\\*LA super super_clone &&\n+\t(\n+\t\tcd super_clone &&\n+\t\tgit submodule status |cut -c1,43- >../actual\n+\t) &&\n+\ttest_cmp expect actual &&\n+\t(\n+\t\tcd super &&\n+\t\tgit init &&\n+\t\tgit submodule add --label LA file://\"$pwd\"/sub1 submoduleC &&\n+\t\tgit commit -a -m \"add another labled submodule\"\n+\t) &&\n+\t(\n+\t\tcd super_clone &&\n+\t\t# obtain the new superproject\n+\t\tgit pull &&\n+\t\t# submoduleC should just appear as it has the label LA\n+\t\t# which was configured in git clone\n+\t\tgit submodule update --init-default-group &&\n+\t\tgit submodule status |cut -c1,43- >../actual\n+\t) &&\n+\ttest_cmp expect2 actual\n+'\n+\n test_done\n-- \n2.8.0.35.g58985d9.dirty\n"},{"id":"286140","messageId":"xmqqr3d9l7pk.fsf@gitster.mtv.corp.google.com","threadId":"42279","inReplyTo":"1462928397-1708-2-git-send-email-sbeller@google.com","subject":"Re: [PATCH 1/7] submodule--helper: add valid-label-name","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-11T01:11:19Z","receivedAt":"2016-05-11T01:11:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> +static int submodule_valid_label_name(const char *label)\n> +{\n> +\tif (!label || !strlen(label))\n> +\t\treturn 0;\n> +\n> +\tif (!isalnum(*label))\n> +\t\treturn 0;\n\nI'd limit this one to isalpha() if I were doing this to make the\nrestriction similar to identifiers in traditional programming\nlanguage.\n\n> +\twhile (*label) {\n> +\t\tif (!(isalnum(*label) ||\n> +\t\t\t*label == '-'))\n\nAnd throw in '_' to the mix while at it.\n\n> +\t\t\treturn 0;\n> +\t\tlabel++;\n> +\t}\n> +\n> +\treturn 1;\n> +}\n\nIf the convention is \"0 is good\", then please signal \"bad\" with a\nnegative value, not just \"non-zero\".\n"},{"id":"286141","messageId":"xmqqlh3hl7mc.fsf@gitster.mtv.corp.google.com","threadId":"42279","inReplyTo":"1462928397-1708-3-git-send-email-sbeller@google.com","subject":"Re: [PATCH 2/7] submodule add: label submodules if asked to","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-11T01:13:15Z","receivedAt":"2016-05-11T01:13:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\n> index 814ee63..0adc4e4 100755\n> --- a/t/t7400-submodule-basic.sh\n> +++ b/t/t7400-submodule-basic.sh\n> @@ -1056,6 +1056,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> @@ -1094,5 +1095,48 @@ test_expect_success 'submodule helper list is not confused by common prefixes' '\n>  \ttest_cmp expect actual\n>  '\n>  \n> +test_expect_success 'submodule add records a label' '\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 --label labelA file://\"$pwd\"/example2 submodule &&\n> +\t\tgit config -f .gitmodules submodule.\"submodule\".label >../actual &&\n> +\t\techo labelA >../expect\n> +\t) &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +cat >expect <<-EOF\n> +labelA\n> +labelB\n> +EOF\n> +\n> +test_expect_success 'submodule add records multiple labels' '\n\nThe existing tests in this file may be littered with this bad\nconstruct, but please do not add more example of running things\noutside of test_expect_{success,failure} block unless there is a\ngood reason to do so.\n"},{"id":"286142","messageId":"xmqqh9e5l7id.fsf@gitster.mtv.corp.google.com","threadId":"42279","inReplyTo":"1462928397-1708-4-git-send-email-sbeller@google.com","subject":"Re: [PATCH 3/7] submodule-config: keep labels around","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-11T01:15:38Z","receivedAt":"2016-05-11T01:15:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> @@ -199,6 +203,7 @@ static struct submodule *lookup_or_create_by_name(struct submodule_cache *cache,\n>  \tsubmodule->update_strategy.command = NULL;\n>  \tsubmodule->fetch_recurse = RECURSE_SUBMODULES_NONE;\n>  \tsubmodule->ignore = NULL;\n> +\tsubmodule->labels = NULL;\n\nHmph, is there a reason to do this, instead of embedding an instance\nof \"struct string_list\" inside submodule structure?\n\nI am not yet claiming that embedding is better.  Just wondering if\nit makes it easier to handle initialization as seen in the hunk\nbelow, and also _clear() procedure.\n\n> @@ -353,6 +358,17 @@ static int parse_config(const char *var, const char *value, void *data)\n>  \t\telse if (parse_submodule_update_strategy(value,\n>  \t\t\t &submodule->update_strategy) < 0)\n>  \t\t\t\tdie(_(\"invalid value for %s\"), var);\n> +\t} else if (!strcmp(item.buf, \"label\")) {\n> +\t\tif (!value)\n> +\t\t\tret = config_error_nonbool(var);\n> +\t\telse {\n> +\t\t\tif (!submodule->labels) {\n> +\t\t\t\tsubmodule->labels =\n> +\t\t\t\t\txmalloc(sizeof(*submodule->labels));\n> +\t\t\t\tstring_list_init(submodule->labels, 1);\n> +\t\t\t}\n> +\t\t\tstring_list_insert(submodule->labels, value);\n> +\t\t}\n>  \t}\n"},{"id":"286143","messageId":"xmqqd1otl7ca.fsf@gitster.mtv.corp.google.com","threadId":"42279","inReplyTo":"1462928397-1708-5-git-send-email-sbeller@google.com","subject":"Re: [PATCH 4/7] submodule-config: check if a submodule is in a group","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-11T01:19:17Z","receivedAt":"2016-05-11T01:19:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> +static int in_group(int argc, const char **argv, const char *prefix)\n> + ...\n> +\tif (!group)\n> +\t\tlist = git_config_get_value_multi(\"submodule.updateGroup\");\n> +\telse {\n> +\t\tstring_list_split(&actual_list, group, ',', -1);\n\nIs this expected to be used only by test scripts, or will it have\nreal scripted Porcelain as its users?  I am wondering if\nconcatenation with ' ' would be more natural if it is the latter (if\nonly used for testing, I don't care that much).\n"},{"id":"286144","messageId":"xmqq8tzhl6v7.fsf@gitster.mtv.corp.google.com","threadId":"42279","inReplyTo":"1462928397-1708-6-git-send-email-sbeller@google.com","subject":"Re: [PATCH 5/7] submodule--helper module_list_compute: allow label or name arguments","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-11T01:29:32Z","receivedAt":"2016-05-11T01:29:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> +static void split_argv_pathspec_groups(int argc, const char **argv,\n> +\t\t\t\t       const char ***pathspec_argv,\n> +\t\t\t\t       struct string_list *group)\n> +{\n> +\tint i;\n> +\tstruct argv_array ps = ARGV_ARRAY_INIT;\n> +\tfor (i = 0; i < argc; i++) {\n> +\t\tif (starts_with(argv[i], \"*\")\n> +\t\t    || starts_with(argv[i], \":\")) {\n> +\t\t\tstring_list_insert(group, argv[i]);\n> +\t\t} else if (starts_with(argv[i], \"./\")) {\n> +\t\t\targv_array_push(&ps, argv[i]);\n\nI'd suggest stripping \"./\" when you do this.  That is, make the rule\nto be \"*label is a label, :name is a name, and everything else is a\npath.  You can prefix ./ to an unfortunate path that begins with an\nasterisk or a colon and we will strip ./ disambiguator\".\n\n> +\t\t} else {\n> +\t\t\t/*\n> +\t\t\t* NEEDSWORK:\n> +\t\t\t* Improve autodetection of items with no prefixes,\n> +\t\t\t* roughly like\n> +\t\t\t* if (label_or_name_exists(argv[i]))\n> +\t\t\t*\tstring_list_insert(group, argv[i]);\n> +\t\t\t* else\n> +\t\t\t*\targv_array_push(&ps, argv[i]);\n> +\t\t\t*/\n\nI do not think this is desirable.  \"label\" that changes its meaning\nbetween \"a path ./label\" to \"a label *label\" would force people to\ngive unnecessary prefix \"./\" when naming their path, from fear of\ncolliding with a label that may or may not exist.\n\n> +\t\t\targv_array_push(&ps, argv[i]);\n> +\t\t}\n> +\t}\n\n\n\n> +\t\tif (!group.nr) {\n> +\t\t\tif (!match_pathspec(pathspec, ce->name, ce_namelen(ce),\n> +\t\t\t\t\t    0, ps_matched, 1) ||\n> +\t\t\t    !S_ISGITLINK(ce->ce_mode))\n> +\t\t\t\tcontinue;\n> +\t\t} else {\n> +\t\t\tconst struct submodule *sub;\n> +\t\t\tint ps = 0, gr = 0;\n> +\t\t\tif (!S_ISGITLINK(ce->ce_mode))\n> +\t\t\t\tcontinue;\n> +\t\t\tsub = submodule_from_path(null_sha1, ce->name);\n> +\n> +\t\t\tps = match_pathspec(pathspec, ce->name, ce_namelen(ce),\n> +\t\t\t\t\t    0, ps_matched, 1);\n> +\t\t\tgr = submodule_in_group(&group, sub);\n> +\n> +\t\t\tif (!pathspec->nr && !gr)\n> +\t\t\t\tcontinue;\n> +\t\t\tif (!ps && !gr)\n> +\t\t\t\tcontinue;\n\nHmph, so the rule is \"a submodule that is in the specified group is\nchosen, even if it is outside the subset of paths narrowed by the\ngiven pathspec.\"  I think that is consistent with the way how the\nlist of arguments given to module_list (i.e. a pathspec element plus\na group specifier OR together just like two pathspec elements do not\nforce a path to match both, and instead they OR together).\n\nIs that rule (i.e. specifiers are ORed together) written down\nsomewhere for users?\n"},{"id":"286145","messageId":"xmqq4ma5l526.fsf@gitster.mtv.corp.google.com","threadId":"42279","inReplyTo":"1462928397-1708-1-git-send-email-sbeller@google.com","subject":"Re: [PATCH 0/7] submodule groups","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-11T02:08:33Z","receivedAt":"2016-05-11T02:08:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> I started from scratch as I think there were some sharp edges in the design.\n> My thinking shifted from \"submodule groups\" towards \"actually it's just an\n> enhanced pathspec, called submodulespec\".\n\nExcept for minor things I mentioned separately, overall, this seems\nquite cleanly done.\n\nLooks very promising.\n\nThanks.\n"},{"id":"286146","messageId":"xmqqzirxjpqw.fsf@gitster.mtv.corp.google.com","threadId":"42279","inReplyTo":"xmqq8tzhl6v7.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 5/7] submodule--helper module_list_compute: allow label or name arguments","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-11T02:24:39Z","receivedAt":"2016-05-11T02:24:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Stefan Beller <sbeller@google.com> writes:\n>\n>> +static void split_argv_pathspec_groups(int argc, const char **argv,\n>> +\t\t\t\t       const char ***pathspec_argv,\n>> +\t\t\t\t       struct string_list *group)\n>> +{\n>> +\tint i;\n>> +\tstruct argv_array ps = ARGV_ARRAY_INIT;\n>> +\tfor (i = 0; i < argc; i++) {\n>> +\t\tif (starts_with(argv[i], \"*\")\n>> +\t\t    || starts_with(argv[i], \":\")) {\n>> +\t\t\tstring_list_insert(group, argv[i]);\n>> +\t\t} else if (starts_with(argv[i], \"./\")) {\n>> +\t\t\targv_array_push(&ps, argv[i]);\n>\n> I'd suggest stripping \"./\" when you do this.  That is, make the rule\n> to be \"*label is a label, :name is a name, and everything else is a\n> path.  You can prefix ./ to an unfortunate path that begins with an\n> asterisk or a colon and we will strip ./ disambiguator\".\n\nTo clarify, \"./$path and $path are the same so why strip it?\" is an\nunderstandable, even though naive, question. The reason is because\nyou do not want to contaminate the code that parses pathspecs with\nthe knowledge of this submodule-group specific prefix.  \"./$path\"\nand \"$path\" may be equivalent for a literal pathspec, but I'd want\nto see user be able to say\n\n   git submodule foreach './:(icase)foo'\n\nand find Foo, foo, FOO, etc.\n\nI would also recommend to strip '*' and ':' from group names and\nmodule names, and maintain two separate lists.  You would eventually\nwant to do \"*default*\" to name all groups whose name matches with\nthe pattern \"default*\", as if it were a pathspec matched against the\navailable group names, and that will keep the door open for future\nextension like  \"*:(icase)default*\".\n"},{"id":"286276","messageId":"CAGZ79kYWg08Cz0oznLSOryvXvYof+Y0=qbszF5r5fXqxr2HDMw@mail.gmail.com","threadId":"42279","inReplyTo":"xmqqlh3hl7mc.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 2/7] submodule add: label submodules if asked to","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-05-11T17:26:15Z","receivedAt":"2016-05-11T17:26:15Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":">> +cat >expect <<-EOF\n>> +labelA\n>> +labelB\n>> +EOF\n>> +\n>> +test_expect_success 'submodule add records multiple labels' '\n>\n> The existing tests in this file may be littered with this bad\n> construct, but please do not add more example of running things\n> outside of test_expect_{success,failure} block unless there is a\n> good reason to do so.\n\nI thought that was the standard for tests as I have seen\nthem quite a few times. Looking for those \"cat >expect ...\" constructs\nmore closely they are often found inside tests as well. Makes sense\nif the expectation is used for only one test.\n"},{"id":"286280","messageId":"CAGZ79kYSxccNLvJ-=UghabQ_MG73jRSabVy5rgfNo-A6Fjz_yQ@mail.gmail.com","threadId":"42279","inReplyTo":"xmqqh9e5l7id.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 3/7] submodule-config: keep labels around","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-05-11T17:41:01Z","receivedAt":"2016-05-11T17:41:01Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, May 10, 2016 at 6:15 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Stefan Beller <sbeller@google.com> writes:\n>\n>> @@ -199,6 +203,7 @@ static struct submodule *lookup_or_create_by_name(struct submodule_cache *cache,\n>>       submodule->update_strategy.command = NULL;\n>>       submodule->fetch_recurse = RECURSE_SUBMODULES_NONE;\n>>       submodule->ignore = NULL;\n>> +     submodule->labels = NULL;\n>\n> Hmph, is there a reason to do this, instead of embedding an instance\n> of \"struct string_list\" inside submodule structure?\n>\n> I am not yet claiming that embedding is better.  Just wondering if\n> it makes it easier to handle initialization as seen in the hunk\n> below, and also _clear() procedure.\n\nThanks for pointing out that alternative.  That looks so much\nbetter in this patch. Let's see how the follow up patches develop.\nAs we'd not check != NULL first, but check against the count of the\nstring list. (I expect no problems down that road though).\n"},{"id":"286289","messageId":"xmqqfutojnd3.fsf@gitster.mtv.corp.google.com","threadId":"42279","inReplyTo":"CAGZ79kYSxccNLvJ-=UghabQ_MG73jRSabVy5rgfNo-A6Fjz_yQ@mail.gmail.com","subject":"Re: [PATCH 3/7] submodule-config: keep labels around","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-11T21:28:24Z","receivedAt":"2016-05-11T21:28:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> On Tue, May 10, 2016 at 6:15 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Stefan Beller <sbeller@google.com> writes:\n>>\n>>> @@ -199,6 +203,7 @@ static struct submodule *lookup_or_create_by_name(struct submodule_cache *cache,\n>>>       submodule->update_strategy.command = NULL;\n>>>       submodule->fetch_recurse = RECURSE_SUBMODULES_NONE;\n>>>       submodule->ignore = NULL;\n>>> +     submodule->labels = NULL;\n>>\n>> Hmph, is there a reason to do this, instead of embedding an instance\n>> of \"struct string_list\" inside submodule structure?\n>>\n>> I am not yet claiming that embedding is better.  Just wondering if\n>> it makes it easier to handle initialization as seen in the hunk\n>> below, and also _clear() procedure.\n>\n> Thanks for pointing out that alternative.  That looks so much\n> better in this patch. Let's see how the follow up patches develop.\n> As we'd not check != NULL first, but check against the count of the\n> string list. (I expect no problems down that road though).\n\nI also wonder if we can say the same for the .ignore field, by the\nway, if we agree that it is a better direction to go.\n"},{"id":"286293","messageId":"CAGZ79kYfNyyLvhZKbmKNZV=2nR00HDEQcD_knPLo3OHRJY+yfg@mail.gmail.com","threadId":"42279","inReplyTo":"xmqqfutojnd3.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 3/7] submodule-config: keep labels around","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-05-11T21:39:57Z","receivedAt":"2016-05-11T21:39:57Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"+Heiko\n\nOn Wed, May 11, 2016 at 2:28 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Stefan Beller <sbeller@google.com> writes:\n>\n>> On Tue, May 10, 2016 at 6:15 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>>> Stefan Beller <sbeller@google.com> writes:\n>>>\n>>>> @@ -199,6 +203,7 @@ static struct submodule *lookup_or_create_by_name(struct submodule_cache *cache,\n>>>>       submodule->update_strategy.command = NULL;\n>>>>       submodule->fetch_recurse = RECURSE_SUBMODULES_NONE;\n>>>>       submodule->ignore = NULL;\n>>>> +     submodule->labels = NULL;\n>>>\n>>> Hmph, is there a reason to do this, instead of embedding an instance\n>>> of \"struct string_list\" inside submodule structure?\n>>>\n>>> I am not yet claiming that embedding is better.  Just wondering if\n>>> it makes it easier to handle initialization as seen in the hunk\n>>> below, and also _clear() procedure.\n>>\n>> Thanks for pointing out that alternative.  That looks so much\n>> better in this patch. Let's see how the follow up patches develop.\n>> As we'd not check != NULL first, but check against the count of the\n>> string list. (I expect no problems down that road though).\n>\n> I also wonder if we can say the same for the .ignore field, by the\n> way, if we agree that it is a better direction to go.\n\nI don't think this is a good idea, as it's just a string, like .{url,\nname, path}\n\nInstead of storing the string we could store an enum {UNTRACKED,\nDIRTY, ALL, NONE, NOT_INIT} though. That would be better I guess.\n"},{"id":"286296","messageId":"CAGZ79kY3S6SmJOMQ7RAKw0yTvicY=Y1VRRGdm5uK9+Eb7W+Ykg@mail.gmail.com","threadId":"42279","inReplyTo":"xmqq4ma5l526.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 0/7] submodule groups","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-05-11T23:07:07Z","receivedAt":"2016-05-11T23:07:07Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, May 10, 2016 at 7:08 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Stefan Beller <sbeller@google.com> writes:\n>\n>> I started from scratch as I think there were some sharp edges in the design.\n>> My thinking shifted from \"submodule groups\" towards \"actually it's just an\n>> enhanced pathspec, called submodulespec\".\n>\n> Except for minor things I mentioned separately, overall, this seems\n> quite cleanly done.\n\nI disagree (now).\n\nI started documenting the <submodulespec> as an extension of\nthe pathspecs. While I thought the logical OR was the right way to go,\nI think it is wrong now. So there is stuff in tests like:\n\n    git submodule--helper matches-submodulespec sub0 ./.\n./:(exclude)*0 *label-sub0\n\nwhich should test if the first argument (sub0) matches the submodulespec\nwhich follows. And it matches sub0 by matching the label, although\nwe told it to ignore anything ending in 0\n\nSo I wonder if we rather want to extend the pathspec magic to\ninclude properties of blobs (i.e. submodules):\n\n    git <command> . :(sub-label:label-sub0) :(exclude)*0\n\nwould look much more powerful too me. Properties of blobs\nmay also be interesting for otherwise. Imagine looking for huge files\n(in a bare repo, so you have to use Git and not your shell tools):\n\n  git ls-files . :(file-size:>1024k)\n\n>\n> Looks very promising.\n>\n\nThanks for the encouragement!\n\nThanks,\nStefan\n"},{"id":"286300","messageId":"xmqqlh3gi2qr.fsf@gitster.mtv.corp.google.com","threadId":"42279","inReplyTo":"CAGZ79kY3S6SmJOMQ7RAKw0yTvicY=Y1VRRGdm5uK9+Eb7W+Ykg@mail.gmail.com","subject":"Re: [PATCH 0/7] submodule groups","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-11T23:39:08Z","receivedAt":"2016-05-11T23:39:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> So I wonder if we rather want to extend the pathspec magic to\n> include properties of blobs (i.e. submodules):\n>\n>     git <command> . :(sub-label:label-sub0) :(exclude)*0\n>\n> would look much more powerful too me. Properties of blobs\n> may also be interesting for otherwise. Imagine looking for huge files\n> (in a bare repo, so you have to use Git and not your shell tools):\n>\n>   git ls-files . :(file-size:>1024k)\n\nI somehow do not think this is a way normal people (read: end users)\nwould want to interact with Git.  Pathspec is about \"paths\" and\nvarious ways to match them.  It is not about contents that happens\nto be currently named by that path.  Don't tie types or sizes to it.\n"},{"id":"286301","messageId":"xmqqh9e4i2av.fsf@gitster.mtv.corp.google.com","threadId":"42279","inReplyTo":"xmqqlh3gi2qr.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 0/7] submodule groups","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-11T23:48:40Z","receivedAt":"2016-05-11T23:48:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n>>   git ls-files . :(file-size:>1024k)\n>\n> I somehow do not think this is a way normal people (read: end users)\n> would want to interact with Git.  Pathspec is about \"paths\" and\n> various ways to match them.  It is not about contents that happens\n> to be currently named by that path.  Don't tie types or sizes to it.\n\nTo clarify, think what that non-pathspec means when used like this:\n\n    $ git diff :(size>1M)\n    $ git log --follow :(size>1M)\n\nWhich side of comparison does the \"size\" thing apply?  Either, both,\nrandomly?  More importantly, what use case of users do these\ncommands serve?\n\nThat is why I said that pathspec should never consider anything but\nthe pathname string you see.\n"},{"id":"286302","messageId":"CAGZ79kYj2Vs-Bp_6Fya4kDzpo1_9TouSPq0PBrwJSV7q5RScjA@mail.gmail.com","threadId":"42279","inReplyTo":"xmqqh9e4i2av.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 0/7] submodule groups","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-05-11T23:57:40Z","receivedAt":"2016-05-11T23:57:40Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, May 11, 2016 at 4:48 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>>>   git ls-files . :(file-size:>1024k)\n>>\n>> I somehow do not think this is a way normal people (read: end users)\n>> would want to interact with Git.  Pathspec is about \"paths\" and\n>> various ways to match them.  It is not about contents that happens\n>> to be currently named by that path.  Don't tie types or sizes to it.\n>\n> To clarify, think what that non-pathspec means when used like this:\n>\n>     $ git diff :(size>1M)\n>     $ git log --follow :(size>1M)\n>\n> Which side of comparison does the \"size\" thing apply?  Either, both,\n> randomly?  More importantly, what use case of users do these\n> commands serve?\n>\n> That is why I said that pathspec should never consider anything but\n> the pathname string you see.\n\nI see. That is bad indeed. So I'll go back and finish the submodulespec\nto present.\n\nThanks,\nStefan\n"},{"id":"286303","messageId":"xmqqd1osi1r8.fsf@gitster.mtv.corp.google.com","threadId":"42279","inReplyTo":"CAGZ79kY3S6SmJOMQ7RAKw0yTvicY=Y1VRRGdm5uK9+Eb7W+Ykg@mail.gmail.com","subject":"Re: [PATCH 0/7] submodule groups","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-12T00:00:27Z","receivedAt":"2016-05-12T00:00:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n>     git submodule--helper matches-submodulespec sub0 ./.\n> ./:(exclude)*0 *label-sub0\n>\n> which should test if the first argument (sub0) matches the submodulespec\n> which follows.\n\nThis, according to that \"OR'ed together\" definition, asks to find a\nsubmodule\n\n    - whose path matches pathspec ./. ./:(exclude)*0; or\n    - is labeled with label-sub0.\n\nSo I'd say it is natural sub0 matches if its path is at sub0 and has\na label label-sub0.\n\nYou could instead choose to use \"AND'ed together\" semantics, but\nthat would break expectation by those who expect \"This OR that\"\nbehaviour.  Unless you are willing to support --and/--or/(/) like\n\"git grep\" does to express a way to combine hits from individual\nterms, that is an inherent limitation.\n\nI'd suggest not to over-engineer this.  Go back and imagine how\n\"/bin/ls\" would work is a good sanity check to gauge what complexity\nlevels ordinary users would feel comfortable to handle.\n\n\"ls a b\" would give union of what \"ls a\" and \"ls b\" would output,\nthere is no negation, and the users won't die from having to say \"ls\n[A-Za-qs-z]*\" to exclude files whose names begin with 'r'.\n"},{"id":"286305","messageId":"xmqq7fezj3ns.fsf@gitster.mtv.corp.google.com","threadId":"42279","inReplyTo":"xmqqd1osi1r8.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 0/7] submodule groups","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-12T04:33:59Z","receivedAt":"2016-05-12T04:33:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> I'd suggest not to over-engineer this.  Go back and imagine how\n> \"/bin/ls\" would work is a good sanity check to gauge what complexity\n> levels ordinary users would feel comfortable to handle.\n>\n> \"ls a b\" would give union of what \"ls a\" and \"ls b\" would output,\n> there is no negation, and the users won't die from having to say \"ls\n> [A-Za-qs-z]*\" to exclude files whose names begin with 'r'.\n\nHaving said all that, there is one thing we may want to consider.\n\nFor negative pathspecs, we deliberately defined its semantics to\n\"see if the path matches with any positive pathspec element (and it\nis a non-match if no positive one matches), and then exclude if it\nmatches with any negative pathspec element\".\n\nThat is, when you are matching 'x.o' against three-element pathspec \n\n\t'*.c' ':(exclude)*.o' 'x.?'\n\nyou do not say \"x.o does not match *.c, but it matches *.o so it is\nto be excluded, ah, wait, x.? matches to revive it so it is a\nmatch\".  Instead \"*.c does not and x.? does match, but *.o matches\nso it is excluded\".  IOW, the order of the pathspec elements given\non the command line does not matter.\n\nNow we are adding two different criteria to specify inclusion based\non labels and names.\n\nAs implemented in the patch series under discussion, we are saying\nthat a submodule is included if\n\n    - its path matches the pathspec (using the \"matches one or more\n      positive pathspec elements, and does not match any negaative\n      pathspec element\" as the definition of \"matches\"), OR\n\n    - it is included in the specified set of *labels, OR\n\n    - its name is specified by :name\n\nThere is no reason not to consider future enhancements to specify\nexclusion base on these two new criretia.  A naive and easier to\nimplement enhancement would be to rewrite the above to (the latter\ntwo item changes):\n\n\n    - its path matches the pathspec (using the \"matches one or more\n      positive pathspec elements, and does not match any negaative\n      pathspec element\" as the definition of \"matches\"), OR\n\n    - it is included in the specified set of *labels, and does not\n      have any excluded *!labels, OR\n\n    - its name is specified by :name, and is not among any excluded\n      :!name.\n\nbut it does not have to be that way.  I suspect that it may make it\neasier to understand and use if we OR'ed together only the positive\nones from three classes of criteria together and require at least\none of them to match, and then requiring none of the negative ones\nto match.  That is, a module-spec with three elements:\n\n     'sub*' '*label0' './:(exclude)*0'\n\nwith the implemented algorithm would choose submodules whose name\nbegins with 'sub' except the ones whose name ends with '0', OR those\nwith label0, but if we redefine the behaviour to \"positive ones\ntogether, and then exclude negative ones\", then we would choose ones\nwhose name begins with 'sub' or ones with label0, and among these,\nexclude those whose name ends with '0' (which is what your \"test\"\nwanted to express).\n\nThe implementation of match_pathspec() does the \"first positive ones\nonly and then negative ones\" two-step process already, so I think\nyou could update its \"int is_dir\" argument to \"unsigned flags\",\nintroduce a \"negative only\" flag, and then do something like:\n\n        for each path\n                if (!(match_pathspec(ps, name, ..., 0) ||\n                      has a \"positive\" label specified ||\n                      has its name specified as a \"postiive\")\n                        /* does not match any positive criteria */\n                        continue;\n                if (match_pathspec(ps, name, ..., NEGATIVE_ONLY) ||\n                    has a \"negative\" label specified ||\n                    has its name specified as a \"negative\")\n                        /* explicitly excluded */\n                        continue;\n                /* included! */\n\nThat would open door to something like\n\n     'sub*' '*label0' './:(exclude)*0' '*!label1'\n\nto allow you to say \"(those with label0 or whose path begins with\nsub) minus (those with label1 or whose path ends with 0)\".\n"},{"id":"286313","messageId":"xmqq37pnj04f.fsf@gitster.mtv.corp.google.com","threadId":"42279","inReplyTo":"1462928397-1708-1-git-send-email-sbeller@google.com","subject":"Re: [PATCH 0/7] submodule groups","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-12T05:50:24Z","receivedAt":"2016-05-12T05:50:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> If you have lots of submodules, you probably don't need all of them at once,\n> but you have functional units. Some submodules are absolutely required,\n> some are optional and only for very specific purposes.\n>\n> This patch series adds labels to submodules in the .gitmodules file.\n\nI hate to bring this up in this thread, primarily because I cannot\nsee how to make it mesh well with the \"submodule spec lets you\nspecify a group of submodule with labels\", but for completeness of\ndiscussion, I'll mention it anyway.\n\nInstead of specifying \"all files written in Perl\" to work on by\ngiving a pathspec with three elements, e.g.\n\n    git cmd -- \\*.perl \\*.pl \\*.pm\n\nI've often wondered if it would be a good idea to let attributes\nfile to specify \"these paths form the group Perl\" with something\nlike:\n\n    *.pm        group=perl\n    *.pl        group=perl\n    *.perl      group=perl\n    *.h         group=c\n    *.c         group=c\n\nand say\n\n    git cmd -- ':(group=perl)'\n\ninstead.\n\nThe reason why I suspect that this may not work well with submodule\nlabels is because submodule labels (or any attribute we give via\n.gitmodules to a submodule) are keyed by a submodule name, which is\nthe primary unchanging key (so that people can \"mv\" a submodule in\nthe context of the toplevel superproject without losing track of\nsubmodule identity), not by paths to submodules, while the \"group\"\nthing I want is merely a short-hand for pathspec elements and wants\nto be keyed by paths.\n\nBut there may be somebody more clever than I who can come up with a\nway to unify these two similar concepts without confusing end users.\n"},{"id":"286329","messageId":"CAGZ79ka-d9skygE-LeoFz4ENY48mnn9CzVDmLvYSXH3PNVy3ug@mail.gmail.com","threadId":"42279","inReplyTo":"xmqq37pnj04f.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 0/7] submodule groups","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-05-12T15:32:46Z","receivedAt":"2016-05-12T15:32:46Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, May 11, 2016 at 10:50 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>\n>     git cmd -- \\*.perl \\*.pl \\*.pm\n>\n> I've often wondered if it would be a good idea to let attributes\n> file to specify \"these paths form the group Perl\" with something\n> like:\n>\n>     *.pm        group=perl\n>     *.pl        group=perl\n>     *.perl      group=perl\n>     *.h         group=c\n>     *.c         group=c\n>\n> and say\n>\n>     git cmd -- ':(group=perl)'\n>\n> instead.\n\nHow is that different to the file size example I gave earlier?\nYou could have a change which includes:\n\n    -    *.pl    group=prolog\n    +    *.pl   group=perl\n\nWhat happens in the diff/log case then (just as a file can pass an\narbitrary file size within that change) ? These file attributes are as\narbitrary as sizes as well as submodule labels or names.\n"},{"id":"286332","messageId":"xmqqoa8bgten.fsf@gitster.mtv.corp.google.com","threadId":"42279","inReplyTo":"xmqq37pnj04f.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 0/7] submodule groups","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-12T15:58:24Z","receivedAt":"2016-05-12T15:58:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> The reason why I suspect that this may not work well with submodule\n> labels is because submodule labels (or any attribute we give via\n> .gitmodules to a submodule) are keyed by a submodule name, which is\n> the primary unchanging key (so that people can \"mv\" a submodule in\n> the context of the toplevel superproject without losing track of\n> submodule identity), not by paths to submodules, while the \"group\"\n> thing I want is merely a short-hand for pathspec elements and wants\n> to be keyed by paths.\n>\n> But there may be somebody more clever than I who can come up with a\n> way to unify these two similar concepts without confusing end users.\n\nThinking about this even more, if there is no requirement that\nlabels must be tied to submodule names, we just can scrap the idea\nof \"submodule labels\" to group things and instead just use \"path\nlabels\", i.e. write the full path to the submodule and assign it a\nlabel in .gitattributes and use it in place of where we used *label\nin the patch series.  After all, an easy way to choose some among\nmany submodules logically is a subset of an easy way to choose some\namong many paths.\n\nThe only reason why we added the submodule label to .gitmodules is\nbecause we viewed it as submodule-specific thing and the \"keyed by\nname, not path\" came as a consequence, not because any real \"we must\nkey it by name because...\" reason, I would think.\n\nI know this is a huge departure from the design presented both at\nthe conceptual level and also at the implementation level, and that\nis one of the reasons why I do not particularly want to like it, but\non the other hand, I am not bold enough to say that I will have a\ngood answer when later somebody asks \"Why can we group only\nsubmodules with labels, but not random group of paths (e.g. 'these\ndirectories are about documentation')?\"  And then, if we add path\nlabels to allow expressing groups of paths later, the follow-up\nquestion would be \"When should I use submodule labels and when\nshould I use path labels?  I can use path labels to group submodules\nand say 'git submodule update -- :(group=docs)' can't I?\".\n\nAnd that makes me pause and step back from the \"submodule labels\"\nidea.\n"},{"id":"286339","messageId":"CAGZ79kZHbAiP13rC+VLnFUtoxPZEcyx_wkrxfKb+cGBxQaojZQ@mail.gmail.com","threadId":"42279","inReplyTo":"xmqqoa8bgten.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 0/7] submodule groups","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-05-12T16:35:43Z","receivedAt":"2016-05-12T16:35:43Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, May 12, 2016 at 8:58 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> The reason why I suspect that this may not work well with submodule\n>> labels is because submodule labels (or any attribute we give via\n>> .gitmodules to a submodule) are keyed by a submodule name, which is\n>> the primary unchanging key (so that people can \"mv\" a submodule in\n>> the context of the toplevel superproject without losing track of\n>> submodule identity), not by paths to submodules, while the \"group\"\n>> thing I want is merely a short-hand for pathspec elements and wants\n>> to be keyed by paths.\n>>\n>> But there may be somebody more clever than I who can come up with a\n>> way to unify these two similar concepts without confusing end users.\n>\n> Thinking about this even more, if there is no requirement that\n> labels must be tied to submodule names, we just can scrap the idea\n> of \"submodule labels\" to group things and instead just use \"path\n> labels\", i.e. write the full path to the submodule and assign it a\n> label in .gitattributes and use it in place of where we used *label\n> in the patch series.  After all, an easy way to choose some among\n> many submodules logically is a subset of an easy way to choose some\n> among many paths.\n>\n> The only reason why we added the submodule label to .gitmodules is\n> because we viewed it as submodule-specific thing and the \"keyed by\n> name, not path\" came as a consequence, not because any real \"we must\n> key it by name because...\" reason, I would think.\n>\n> I know this is a huge departure from the design presented both at\n> the conceptual level and also at the implementation level, and that\n> is one of the reasons why I do not particularly want to like it, but\n> on the other hand, I am not bold enough to say that I will have a\n> good answer when later somebody asks \"Why can we group only\n> submodules with labels, but not random group of paths (e.g. 'these\n> directories are about documentation')?\"  And then, if we add path\n> labels to allow expressing groups of paths later, the follow-up\n> question would be \"When should I use submodule labels and when\n> should I use path labels?  I can use path labels to group submodules\n> and say 'git submodule update -- :(group=docs)' can't I?\".\n>\n> And that makes me pause and step back from the \"submodule labels\"\n> idea.\n>\n\nIt sounds better at first (and I haven't thought further).\nSo if we were to go with this idea:\n\nLabel paths (or even pathspecs?) in the .gitattributes file.\n\nI think it is important to keep the property of defining the labeling\nin the tree, so you can follow upstream easier.\n\nI tried coming up with an example for labels for generic paths,\nbut it looks like most of the time it is a substitution for a pathspec,\nI did not find a convincing example which makes it easier to use.\n\n`:(group=docs)` in the non submodule case could be expressed\nas `Documentation/**`. Well maybe we also want to include README\nand some other files which need to stay outside the Documentation\ndirectory, so I can see how it may be useful.\n\nWe do not need a special labeling command. We do not\nship with a command which writes the .gitattributes or .gitignore\nfor you, and labels don't require this. So I could drop the patch\nfor \"submodule add --label\".\n\nWe can still keep the submodule.defaultGroup. (In the WIP I renamed\nit to updateGroup as its only feature is to have it set during clone\nand remebered for `git submodule update`)\n\nWhen we allow labels to be generic path labels instead of submodule\nlabels, the user might be tempted to ask, why the submodules can\nbe specified but not the individual paths, i.e.\n\n    git clone --init-submodule=\"(:group=docs)\" ...\n\nmay strongly hint at:\n\n    git clone --narrow=\"(:group=docs)\" ...\n\nwould only get parts of the repository.\n\nFor the submodule case, this may add confusion as the user would\nneed to configure some properties in the .gitmodules file and some\nin the .gitattributes file.\n\nI think I'll try implementing a mock and see how much code it is for\na more fundamental pathspec extension.\n\nThanks,\nStefan\n"},{"id":"286341","messageId":"xmqqzirvfca5.fsf@gitster.mtv.corp.google.com","threadId":"42279","inReplyTo":"CAGZ79kZHbAiP13rC+VLnFUtoxPZEcyx_wkrxfKb+cGBxQaojZQ@mail.gmail.com","subject":"Re: [PATCH 0/7] submodule groups","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-12T16:53:38Z","receivedAt":"2016-05-12T16:53:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> We can still keep the submodule.defaultGroup. (In the WIP I renamed\n> it to updateGroup as its only feature is to have it set during clone\n> and remebered for `git submodule update`)\n\nYes, my idle speculation between \"[submodule\"x\"] label=A\" stored\nin .gitmodules and \"path/to/submodule  group=A\" in .gitattributes\nis completely orthogonal to the need for submodule.defaultGroup.\n\nThe only difference it brings in is how we evaluate what\nsubmodule.defaultGroup names (i.e. via \"submodule label\" vs\n\"path label\").\n\n> When we allow labels to be generic path labels instead of submodule\n> labels, the user might be tempted to ask, why the submodules can\n> be specified but not the individual paths, i.e.\n>\n>     git clone --init-submodule=\"(:group=docs)\" ...\n>\n> may strongly hint at:\n>\n>     git clone --narrow=\"(:group=docs)\" ...\n>\n> would only get parts of the repository.\n\nIf we are to have a (proper) narrow clone, I do not think there is\nany reason why the latter should not work as expected.\n\n> For the submodule case, this may add confusion as the user would\n> need to configure some properties in the .gitmodules file and some\n> in the .gitattributes file.\n\nI do not quite understand this comment.\n\nA new reader who knows nothing about \"grouping submodules\" would not\nhave any trouble, I suspect, if all and the only grouping she learns\nis \"grouping paths\" from the beginning.\n\nThanks.\n"}]}