[PATCH 0/7] submodule groups

classic Classic list List threaded Threaded
30 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[PATCH 0/7] submodule groups

Stefan Beller-4
I started from scratch as I think there were some sharp edges in the design.
My thinking shifted from "submodule groups" towards "actually it's just an
enhanced pathspec, called submodulespec".

The meat is found in the last 3 patches.

What is this series about?
==========================

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

This patch series adds labels to submodules in the .gitmodules file.

So you could have a .gitmodules file such as:

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

and by this series you can work on an arbitrary group of these submodules
composed by the labels, names or paths of the submodules.

    git clone --init-submodule=\*<label> --init-submodule=:<name> git://...
    # will clone the superproject checkout
    # any submodule being labeled <label> or named <name>

    git submodule add --label <name> git://... ..
    # record a label while adding a submodule

    git submodule update [--init] \*label2
    # update only the submodules labeled "label2"
   
    git config submodule.updateGroups default
    git config --add submodule.updateGroups devel
    # configure which submodules you are interested in....
    git submodule update [--init-default-group]
    # ... and update them    

    git status
    git diff
    git submodule summary
    # unlike the last series, these are not touched
   
    git submodule status \*label2
    # only show information about "label2" submodules.
   

Any feedback welcome, specially on the design level!

Thanks,
Stefan

Prior series found here: http://thread.gmane.org/gmane.comp.version-control.git/292666

Stefan Beller (7):
  submodule--helper: add valid-label-name
  submodule add: label submodules if asked to
  submodule-config: keep labels around
  submodule-config: check if a submodule is in a group
  submodule--helper module_list_compute: allow label or name arguments
  submodule update: learn partial initialization
  clone: allow specification of submodules to be cloned

 Documentation/config.txt        |   5 ++
 Documentation/git-clone.txt     |  26 ++++--
 Documentation/git-submodule.txt |  30 +++++--
 builtin/clone.c                 |  40 ++++++++-
 builtin/submodule--helper.c     | 146 +++++++++++++++++++++++++++---
 git-submodule.sh                |  38 ++++++--
 submodule-config.c              |  66 ++++++++++++++
 submodule-config.h              |   5 ++
 t/t7400-submodule-basic.sh      | 176 ++++++++++++++++++++++++++++++++++++
 t/t7412-submodule--helper.sh    | 193 ++++++++++++++++++++++++++++++++++++++++
 10 files changed, 692 insertions(+), 33 deletions(-)
 create mode 100755 t/t7412-submodule--helper.sh

--
2.8.0.35.g58985d9.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/7] submodule--helper: add valid-label-name

Stefan Beller-4
We could allow more than just alphanumeric and dash characters
for submodule labels. As a precaution we'll first allow only this
subset and later on we can extend it once we have more experience
with them.

Signed-off-by: Stefan Beller <[hidden email]>
---
 builtin/submodule--helper.c  | 30 ++++++++++++++++++++++++++-
 t/t7412-submodule--helper.sh | 49 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 78 insertions(+), 1 deletion(-)
 create mode 100755 t/t7412-submodule--helper.sh

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 7f0941d..d3f4684 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -831,6 +831,33 @@ static int update_clone(int argc, const char **argv, const char *prefix)
  return 0;
 }
 
+static int submodule_valid_label_name(const char *label)
+{
+ if (!label || !strlen(label))
+ return 0;
+
+ if (!isalnum(*label))
+ return 0;
+
+ while (*label) {
+ if (!(isalnum(*label) ||
+ *label == '-'))
+ return 0;
+ label++;
+ }
+
+ return 1;
+}
+
+static int valid_label_name(int argc, const char **argv, const char *prefix)
+{
+ if (argc == 2 && submodule_valid_label_name(argv[1]))
+ return 0;
+
+ die(_("submodule label must start with an alphanumeric character"
+      "and must contain alphanumeric characters or dashes only."));
+}
+
 struct cmd_struct {
  const char *cmd;
  int (*fn)(int, const char **, const char *);
@@ -843,7 +870,8 @@ static struct cmd_struct commands[] = {
  {"update-clone", update_clone},
  {"resolve-relative-url", resolve_relative_url},
  {"resolve-relative-url-test", resolve_relative_url_test},
- {"init", module_init}
+ {"init", module_init},
+ {"valid-label-name", valid_label_name}
 };
 
 int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
diff --git a/t/t7412-submodule--helper.sh b/t/t7412-submodule--helper.sh
new file mode 100755
index 0000000..3af315c
--- /dev/null
+++ b/t/t7412-submodule--helper.sh
@@ -0,0 +1,49 @@
+#!/bin/sh
+
+test_description='Basic plumbing support of submodule--helper
+
+This test verifies the submodule--helper plumbing command used to implement
+git-submodule.
+'
+
+. ./test-lib.sh
+
+
+test_expect_success 'valid-label-name tests empty label' '
+ test_must_fail git submodule--helper valid-label-name 2>actual &&
+ test_i18ngrep alphanumeric actual &&
+ test_must_fail git submodule--helper valid-label-name "" 2>actual &&
+ test_i18ngrep alphanumeric actual
+'
+
+test_expect_success 'valid-label-name tests correct label asdf' '
+ git submodule--helper valid-label-name asdf 2>actual &&
+ test_must_be_empty actual
+'
+
+test_expect_success 'valid-label-name tests correct label a' '
+ git submodule--helper valid-label-name a 2>actual &&
+ test_must_be_empty actual
+'
+
+test_expect_success 'valid-label-name tests correct label a-b' '
+ git submodule--helper valid-label-name a-b 2>actual &&
+ test_must_be_empty actual
+'
+
+test_expect_success 'valid-label-name fails with multiple arguments' '
+ test_must_fail git submodule--helper valid-label-name a b 2>actual &&
+ test_i18ngrep alphanumeric actual
+'
+
+test_expect_success 'valid-label-name fails with white spaced arguments' '
+ test_must_fail git submodule--helper valid-label-name "a b" 2>actual &&
+ test_i18ngrep alphanumeric actual
+'
+
+test_expect_success 'valid-label-name fails with utf8 characters' '
+ test_must_fail git submodule--helper valid-label-name ☺ 2>actual &&
+ test_i18ngrep alphanumeric actual
+'
+
+test_done
--
2.8.0.35.g58985d9.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

[PATCH 2/7] submodule add: label submodules if asked to

Stefan Beller-4
In reply to this post by Stefan Beller-4
When adding new submodules, you can specify the labels the submodule
belongs to by giving one or more --label arguments. This will record
each label in the .gitmodules file as a value of the key
"submodule.$NAME.label".

Signed-off-by: Stefan Beller <[hidden email]>
---
 Documentation/git-submodule.txt |  4 +++-
 git-submodule.sh                | 16 ++++++++++++++-
 t/t7400-submodule-basic.sh      | 44 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 13adebf..9ba8895 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -9,7 +9,7 @@ git-submodule - Initialize, update or inspect submodules
 SYNOPSIS
 --------
 [verse]
-'git submodule' [--quiet] add [-b <branch>] [-f|--force] [--name <name>]
+'git submodule' [--quiet] add [-b <branch>] [-f|--force] [-l|--label <label>]
       [--reference <repository>] [--depth <depth>] [--] <repository> [<path>]
 'git submodule' [--quiet] status [--cached] [--recursive] [--] [<path>...]
 'git submodule' [--quiet] init [--] [<path>...]
@@ -101,6 +101,8 @@ is the superproject and submodule repositories will be kept
 together in the same relative location, and only the
 superproject's URL needs to be provided: git-submodule will correctly
 locate the submodule using the relative URL in .gitmodules.
++
+All labels are recorded in the .gitmodules file in the label fields.
 
 status::
  Show the status of the submodules. This will print the SHA-1 of the
diff --git a/git-submodule.sh b/git-submodule.sh
index 82e95a9..c1213d8 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -5,7 +5,7 @@
 # Copyright (c) 2007 Lars Hjemli
 
 dashless=$(basename "$0" | sed -e 's/-/ /')
-USAGE="[--quiet] add [-b <branch>] [-f|--force] [--name <name>] [--reference <repository>] [--] <repository> [<path>]
+USAGE="[--quiet] add [-b <branch>] [-f|--force] [--name <name>] [--reference <repository>] [-l|--label <label>][--] <repository> [<path>]
    or: $dashless [--quiet] status [--cached] [--recursive] [--] [<path>...]
    or: $dashless [--quiet] init [--] [<path>...]
    or: $dashless [--quiet] deinit [-f|--force] [--] <path>...
@@ -130,6 +130,7 @@ cmd_add()
 {
  # parse $args after "submodule ... add".
  reference_path=
+ labels=
  while test $# -ne 0
  do
  case "$1" in
@@ -165,6 +166,15 @@ cmd_add()
  --depth=*)
  depth=$1
  ;;
+ -l|--label)
+ git submodule--helper valid-label-name "$2" || exit
+ labels="${labels} $2"
+ shift
+ ;;
+ --label=*)
+ git submodule--helper valid-label-name "${1#--label=}" || exit
+ labels="${labels} ${1#--label=}"
+ ;;
  --)
  shift
  break
@@ -292,6 +302,10 @@ Use -f if you really want to add it." >&2
 
  git config -f .gitmodules submodule."$sm_name".path "$sm_path" &&
  git config -f .gitmodules submodule."$sm_name".url "$repo" &&
+ for label in $labels
+ do
+ git config --add -f .gitmodules submodule."$sm_name".label "$label"
+ done &&
  if test -n "$branch"
  then
  git config -f .gitmodules submodule."$sm_name".branch "$branch"
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 814ee63..0adc4e4 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -1056,6 +1056,7 @@ test_expect_success 'submodule with UTF-8 name' '
 '
 
 test_expect_success 'submodule add clone shallow submodule' '
+ test_when_finished "rm -rf super" &&
  mkdir super &&
  pwd=$(pwd) &&
  (
@@ -1094,5 +1095,48 @@ test_expect_success 'submodule helper list is not confused by common prefixes' '
  test_cmp expect actual
 '
 
+test_expect_success 'submodule add records a label' '
+ test_when_finished "rm -rf super" &&
+ mkdir super &&
+ pwd=$(pwd) &&
+ (
+ cd super &&
+ git init &&
+ git submodule add --label labelA file://"$pwd"/example2 submodule &&
+ git config -f .gitmodules submodule."submodule".label >../actual &&
+ echo labelA >../expect
+ ) &&
+ test_cmp expect actual
+'
+
+cat >expect <<-EOF
+labelA
+labelB
+EOF
+
+test_expect_success 'submodule add records multiple labels' '
+ test_when_finished "rm -rf super" &&
+ mkdir super &&
+ pwd=$(pwd) &&
+ (
+ cd super &&
+ git init &&
+ git submodule add --label=labelA -l labelB file://"$pwd"/example2 submodule &&
+ git config --get-all -f .gitmodules submodule."submodule".label >../actual
+ ) &&
+ test_cmp expect actual
+'
+
+test_expect_success 'submodule add recording wrong labels reports an error' '
+ test_when_finished "rm -rf super" &&
+ mkdir super &&
+ pwd=$(pwd) &&
+ (
+ cd super &&
+ git init &&
+ test_must_fail git submodule add --label="labelA labelB" file://"$pwd"/example2 submodule 2>../actual
+ ) &&
+ test_i18ngrep alphanumeric actual
+'
 
 test_done
--
2.8.0.35.g58985d9.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

[PATCH 3/7] submodule-config: keep labels around

Stefan Beller-4
In reply to this post by Stefan Beller-4
We need the submodule labels in a later patch.

Signed-off-by: Stefan Beller <[hidden email]>
---
 submodule-config.c | 16 ++++++++++++++++
 submodule-config.h |  2 ++
 2 files changed, 18 insertions(+)

diff --git a/submodule-config.c b/submodule-config.c
index b82d1fb..0cdb47e 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -60,6 +60,10 @@ static void free_one_config(struct submodule_entry *entry)
  free((void *) entry->config->path);
  free((void *) entry->config->name);
  free((void *) entry->config->update_strategy.command);
+ if (entry->config->labels) {
+ string_list_clear(entry->config->labels, 0);
+ free(entry->config->labels);
+ }
  free(entry->config);
 }
 
@@ -199,6 +203,7 @@ static struct submodule *lookup_or_create_by_name(struct submodule_cache *cache,
  submodule->update_strategy.command = NULL;
  submodule->fetch_recurse = RECURSE_SUBMODULES_NONE;
  submodule->ignore = NULL;
+ submodule->labels = NULL;
 
  hashcpy(submodule->gitmodules_sha1, gitmodules_sha1);
 
@@ -353,6 +358,17 @@ static int parse_config(const char *var, const char *value, void *data)
  else if (parse_submodule_update_strategy(value,
  &submodule->update_strategy) < 0)
  die(_("invalid value for %s"), var);
+ } else if (!strcmp(item.buf, "label")) {
+ if (!value)
+ ret = config_error_nonbool(var);
+ else {
+ if (!submodule->labels) {
+ submodule->labels =
+ xmalloc(sizeof(*submodule->labels));
+ string_list_init(submodule->labels, 1);
+ }
+ string_list_insert(submodule->labels, value);
+ }
  }
 
  strbuf_release(&name);
diff --git a/submodule-config.h b/submodule-config.h
index e4857f5..d57da59 100644
--- a/submodule-config.h
+++ b/submodule-config.h
@@ -18,6 +18,8 @@ struct submodule {
  struct submodule_update_strategy update_strategy;
  /* the sha1 blob id of the responsible .gitmodules file */
  unsigned char gitmodules_sha1[20];
+ /* sorted, not as on disk */
+ struct string_list *labels;
 };
 
 int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg);
--
2.8.0.35.g58985d9.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

[PATCH 4/7] submodule-config: check if a submodule is in a group

Stefan Beller-4
In reply to this post by Stefan Beller-4
In later patches we need to tell if a submodule is in a group,
so expose a handy test function in both C and shell.

Signed-off-by: Stefan Beller <[hidden email]>
---
 builtin/submodule--helper.c  | 42 +++++++++++++++++++++++++++++++-
 submodule-config.c           | 50 ++++++++++++++++++++++++++++++++++++++
 submodule-config.h           |  3 +++
 t/t7412-submodule--helper.sh | 58 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 152 insertions(+), 1 deletion(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index d3f4684..6ffd1c1 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -858,6 +858,45 @@ static int valid_label_name(int argc, const char **argv, const char *prefix)
       "and must contain alphanumeric characters or dashes only."));
 }
 
+static int in_group(int argc, const char **argv, const char *prefix)
+{
+ const struct string_list *list;
+ struct string_list actual_list = STRING_LIST_INIT_DUP;
+ const struct submodule *sub;
+ const char *group = NULL;
+
+ struct option default_group_options[] = {
+ OPT_STRING('g', "group", &group, N_("group"),
+ N_("comma separated group specifier for submodules")),
+ OPT_END()
+ };
+
+ const char *const git_submodule_helper_usage[] = {
+ N_("git submodule--helper in-group <path>"),
+ NULL
+ };
+
+ argc = parse_options(argc, argv, prefix, default_group_options,
+     git_submodule_helper_usage, 0);
+
+ gitmodules_config();
+ git_config(submodule_config, NULL);
+
+ if (argc != 1)
+ usage(git_submodule_helper_usage[0]);
+
+ sub = submodule_from_path(null_sha1, argv[0]);
+
+ if (!group)
+ list = git_config_get_value_multi("submodule.updateGroup");
+ else {
+ string_list_split(&actual_list, group, ',', -1);
+ list = &actual_list;
+ }
+
+ return !submodule_in_group(list, sub);
+}
+
 struct cmd_struct {
  const char *cmd;
  int (*fn)(int, const char **, const char *);
@@ -871,7 +910,8 @@ static struct cmd_struct commands[] = {
  {"resolve-relative-url", resolve_relative_url},
  {"resolve-relative-url-test", resolve_relative_url_test},
  {"init", module_init},
- {"valid-label-name", valid_label_name}
+ {"valid-label-name", valid_label_name},
+ {"in-group", in_group}
 };
 
 int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
diff --git a/submodule-config.c b/submodule-config.c
index 0cdb47e..7f38ebd 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -522,3 +522,53 @@ void submodule_free(void)
  cache_free(&cache);
  is_cache_init = 0;
 }
+
+int submodule_in_group(const struct string_list *group,
+       const struct submodule *sub)
+{
+ int matched = 0;
+ struct strbuf sb = STRBUF_INIT;
+
+ if (!group)
+ /*
+ * If no group is specified at all, all submodules match to
+ * keep traditional behavior.
+ */
+ return 1;
+
+ if (sub->labels) {
+ struct string_list_item *item;
+ for_each_string_list_item(item, sub->labels) {
+ strbuf_reset(&sb);
+ strbuf_addf(&sb, "*%s", item->string);
+ if (string_list_has_string(group, sb.buf)) {
+ matched = 1;
+ break;
+ }
+ }
+ }
+ if (sub->path) {
+ /*
+ * NEEDSWORK: This currently works only for
+ * exact paths, but we want to enable
+ * inexact matches such wildcards.
+ */
+ strbuf_reset(&sb);
+ strbuf_addf(&sb, "./%s", sub->path);
+ if (string_list_has_string(group, sb.buf))
+ matched = 1;
+ }
+ if (sub->name) {
+ /*
+ * NEEDSWORK: Same as with path. Do we want to
+ * support wildcards or such?
+ */
+ strbuf_reset(&sb);
+ strbuf_addf(&sb, ":%s", sub->name);
+ if (string_list_has_string(group, sb.buf))
+ matched = 1;
+ }
+ strbuf_release(&sb);
+
+ return matched;
+}
diff --git a/submodule-config.h b/submodule-config.h
index d57da59..4c696cc 100644
--- a/submodule-config.h
+++ b/submodule-config.h
@@ -31,4 +31,7 @@ const struct submodule *submodule_from_path(const unsigned char *commit_sha1,
  const char *path);
 void submodule_free(void);
 
+int submodule_in_group(const struct string_list *group,
+       const struct submodule *sub);
+
 #endif /* SUBMODULE_CONFIG_H */
diff --git a/t/t7412-submodule--helper.sh b/t/t7412-submodule--helper.sh
index 3af315c..042f3f5 100755
--- a/t/t7412-submodule--helper.sh
+++ b/t/t7412-submodule--helper.sh
@@ -46,4 +46,62 @@ test_expect_success 'valid-label-name fails with utf8 characters' '
  test_i18ngrep alphanumeric actual
 '
 
+test_expect_success 'setup superproject with submodules' '
+
+ mkdir sub &&
+ (
+ cd sub &&
+ git init &&
+ test_commit test
+ test_commit test2
+ ) &&
+ mkdir super &&
+ (
+ cd super &&
+ git init &&
+ git submodule add ../sub sub0 &&
+ git submodule add -l bit1 ../sub sub1 &&
+ git submodule add -l bit2 ../sub sub2 &&
+ git submodule add -l bit2 -l bit1 ../sub sub3 &&
+ git submodule add ../sub sub_name &&
+ git mv sub_name sub_path &&
+ git commit -m "add labeled submodules"
+ )
+'
+
+test_expect_success 'in-group' '
+ (
+ cd super &&
+ # we do not specify a group nor have set a default group,
+ # any submodule should be in the default group:
+ git submodule--helper in-group sub0 &&
+ git submodule--helper in-group sub1 &&
+ git submodule--helper in-group sub2 &&
+ git submodule--helper in-group sub3 &&
+
+ # test bit1:
+ test_must_fail git submodule--helper in-group --group=\*bit1 sub0 &&
+       git submodule--helper in-group --group=\*bit1 sub1 &&
+ test_must_fail git submodule--helper in-group --group=\*bit1 sub2 &&
+       git submodule--helper in-group --group=\*bit1 sub3 &&
+ test_must_fail git submodule--helper in-group --group=\*bit1 sub_path &&
+
+ # test by path:
+       git submodule--helper in-group --group=./sub0 sub0 &&
+ test_must_fail git submodule--helper in-group --group=./sub0 sub1 &&
+ test_must_fail git submodule--helper in-group --group=./sub0 sub_path &&
+
+ # tests by name:
+       git submodule--helper in-group --group=:sub0 sub0 &&
+ test_must_fail git submodule--helper in-group --group=:sub0 sub1 &&
+       git submodule--helper in-group --group=:sub_name sub_path &&
+
+ # logical OR of path and labels
+       git submodule--helper in-group --group=\*bit1,./sub0 sub0 &&
+       git submodule--helper in-group --group=\*bit1,./sub0 sub1 &&
+ test_must_fail git submodule--helper in-group --group=\*bit1,./sub0 sub2 &&
+       git submodule--helper in-group --group=\*bit1,./sub0 sub3
+ )
+'
+
 test_done
--
2.8.0.35.g58985d9.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

[PATCH 5/7] submodule--helper module_list_compute: allow label or name arguments

Stefan Beller-4
In reply to this post by Stefan Beller-4
Additionally to taking a pathspec, `module_list_compute` will also take
labels and submodule names, when these are prefixed by '*' and ':'
respectively.

`module_list_compute` is used by other functions in the submodule helper:
* module_list, used by `submodule {deinit, status, sync, foreach}`
* module_init, used by `submodule init`
* update_clone, used by `submodule update`

{foreach, update} do not pass on command line arguments to the listing
function such that these are not affected. The rest of them {deinit,
status, sync, init} will be exercised in additional tests.

As the labeling requires lookup of .gitmodules files, we need to make sure
the submodule config cache is initialized in all the functions, that's why
there are additional calls to gitmodules_config() and git_config(...);

Signed-off-by: Stefan Beller <[hidden email]>
---
 Documentation/git-submodule.txt | 22 +++++++++--
 builtin/submodule--helper.c     | 76 +++++++++++++++++++++++++++++++-----
 git-submodule.sh                |  8 ++--
 t/t7412-submodule--helper.sh    | 86 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 175 insertions(+), 17 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 9ba8895..35ca355 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -11,16 +11,16 @@ SYNOPSIS
 [verse]
 'git submodule' [--quiet] add [-b <branch>] [-f|--force] [-l|--label <label>]
       [--reference <repository>] [--depth <depth>] [--] <repository> [<path>]
-'git submodule' [--quiet] status [--cached] [--recursive] [--] [<path>...]
-'git submodule' [--quiet] init [--] [<path>...]
-'git submodule' [--quiet] deinit [-f|--force] [--] <path>...
+'git submodule' [--quiet] status [--cached] [--recursive] [--] [<submodulespec>...]
+'git submodule' [--quiet] init [--] [<submodulespec>...]
+'git submodule' [--quiet] deinit [-f|--force] [--] <submodulespec>...
 'git submodule' [--quiet] update [--init] [--remote] [-N|--no-fetch]
       [-f|--force] [--rebase|--merge] [--reference <repository>]
       [--depth <depth>] [--recursive] [--jobs <n>] [--] [<path>...]
 'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) <n>]
       [commit] [--] [<path>...]
 'git submodule' [--quiet] foreach [--recursive] <command>
-'git submodule' [--quiet] sync [--recursive] [--] [<path>...]
+'git submodule' [--quiet] sync [--recursive] [--] [<submodulespec>...]
 
 
 DESCRIPTION
@@ -37,6 +37,20 @@ these will not be checked out by default; the 'init' and 'update'
 subcommands will maintain submodules checked out and at
 appropriate revision in your working tree.
 
+When working on submodules you can specify the desired submodule
+group or give no specification at all to apply the command to the
+default group of submodules, which includes all submodules.
+To group submodules you can make use of
+ . a pathspec,
+ . their name,
+ . labels.
+It is undefined behavior to give a spec that is ambigious for a name,
+pathspec or label. To make the specification unambigous, you can prefix
+ . pathspecs with './',
+ . names with ':',
+ . labels with '*'.
+Labels must consist of alphanumeric characters or the dash character.
+
 Submodules are composed from a so-called `gitlink` tree entry
 in the main repository that refers to a particular commit object
 within the inner repository that is completely separate.
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 6ffd1c1..ba43c80 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -12,6 +12,7 @@
 #include "remote.h"
 #include "refs.h"
 #include "connect.h"
+#include "argv-array.h"
 
 static char *get_default_remote(void)
 {
@@ -215,6 +216,36 @@ static int resolve_relative_url_test(int argc, const char **argv, const char *pr
  return 0;
 }
 
+static void split_argv_pathspec_groups(int argc, const char **argv,
+       const char ***pathspec_argv,
+       struct string_list *group)
+{
+ int i;
+ struct argv_array ps = ARGV_ARRAY_INIT;
+ for (i = 0; i < argc; i++) {
+ if (starts_with(argv[i], "*")
+    || starts_with(argv[i], ":")) {
+ string_list_insert(group, argv[i]);
+ } else if (starts_with(argv[i], "./")) {
+ argv_array_push(&ps, argv[i]);
+ } else {
+ /*
+ * NEEDSWORK:
+ * Improve autodetection of items with no prefixes,
+ * roughly like
+ * if (label_or_name_exists(argv[i]))
+ * string_list_insert(group, argv[i]);
+ * else
+ * argv_array_push(&ps, argv[i]);
+ */
+ argv_array_push(&ps, argv[i]);
+ }
+ }
+
+ *pathspec_argv = argv_array_detach(&ps);
+ argv_array_clear(&ps);
+}
+
 struct module_list {
  const struct cache_entry **entries;
  int alloc, nr;
@@ -228,10 +259,15 @@ static int module_list_compute(int argc, const char **argv,
 {
  int i, result = 0;
  char *ps_matched = NULL;
+ const char **pathspec_argv;
+ struct string_list group = STRING_LIST_INIT_DUP;
+
+ split_argv_pathspec_groups(argc, argv, &pathspec_argv, &group);
+
  parse_pathspec(pathspec, 0,
        PATHSPEC_PREFER_FULL |
        PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
-       prefix, argv);
+       prefix, pathspec_argv);
 
  if (pathspec->nr)
  ps_matched = xcalloc(pathspec->nr, 1);
@@ -242,10 +278,27 @@ static int module_list_compute(int argc, const char **argv,
  for (i = 0; i < active_nr; i++) {
  const struct cache_entry *ce = active_cache[i];
 
- if (!match_pathspec(pathspec, ce->name, ce_namelen(ce),
-    0, ps_matched, 1) ||
-    !S_ISGITLINK(ce->ce_mode))
- continue;
+ if (!group.nr) {
+ if (!match_pathspec(pathspec, ce->name, ce_namelen(ce),
+    0, ps_matched, 1) ||
+    !S_ISGITLINK(ce->ce_mode))
+ continue;
+ } else {
+ const struct submodule *sub;
+ int ps = 0, gr = 0;
+ if (!S_ISGITLINK(ce->ce_mode))
+ continue;
+ sub = submodule_from_path(null_sha1, ce->name);
+
+ ps = match_pathspec(pathspec, ce->name, ce_namelen(ce),
+    0, ps_matched, 1);
+ gr = submodule_in_group(&group, sub);
+
+ if (!pathspec->nr && !gr)
+ continue;
+ if (!ps && !gr)
+ continue;
+ }
 
  ALLOC_GROW(list->entries, list->nr + 1, list->alloc);
  list->entries[list->nr++] = ce;
@@ -287,6 +340,9 @@ static int module_list(int argc, const char **argv, const char *prefix)
  argc = parse_options(argc, argv, prefix, module_list_options,
      git_submodule_helper_usage, 0);
 
+ gitmodules_config();
+ git_config(submodule_config, NULL);
+
  if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0) {
  printf("#unmatched\n");
  return 1;
@@ -415,6 +471,9 @@ static int module_init(int argc, const char **argv, const char *prefix)
  argc = parse_options(argc, argv, prefix, module_init_options,
      git_submodule_helper_usage, 0);
 
+ gitmodules_config();
+ git_config(submodule_config, NULL);
+
  if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)
  return 1;
 
@@ -795,16 +854,15 @@ static int update_clone(int argc, const char **argv, const char *prefix)
  if (parse_submodule_update_strategy(update, &suc.update) < 0)
  die(_("bad value for update parameter"));
 
+ gitmodules_config();
+ git_config(submodule_config, NULL);
+
  if (module_list_compute(argc, argv, prefix, &pathspec, &suc.list) < 0)
  return 1;
 
  if (pathspec.nr)
  suc.warn_if_uninitialized = 1;
 
- /* Overlay the parsed .gitmodules file with .git/config */
- gitmodules_config();
- git_config(submodule_config, NULL);
-
  if (max_jobs < 0)
  max_jobs = parallel_submodules();
 
diff --git a/git-submodule.sh b/git-submodule.sh
index c1213d8..c8e36c5 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -6,13 +6,13 @@
 
 dashless=$(basename "$0" | sed -e 's/-/ /')
 USAGE="[--quiet] add [-b <branch>] [-f|--force] [--name <name>] [--reference <repository>] [-l|--label <label>][--] <repository> [<path>]
-   or: $dashless [--quiet] status [--cached] [--recursive] [--] [<path>...]
-   or: $dashless [--quiet] init [--] [<path>...]
-   or: $dashless [--quiet] deinit [-f|--force] [--] <path>...
+   or: $dashless [--quiet] status [--cached] [--recursive] [--] [<submodulespec>...]
+   or: $dashless [--quiet] init [--] [<submodulespec>...]
+   or: $dashless [--quiet] deinit [-f|--force] [--] <submodulespec>...
    or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] [-f|--force] [--checkout|--merge|--rebase] [--reference <repository>] [--recursive] [--] [<path>...]
    or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]
    or: $dashless [--quiet] foreach [--recursive] <command>
-   or: $dashless [--quiet] sync [--recursive] [--] [<path>...]"
+   or: $dashless [--quiet] sync [--recursive] [--] [<submodulespec>...]"
 OPTIONS_SPEC=
 SUBDIRECTORY_OK=Yes
 . git-sh-setup
diff --git a/t/t7412-submodule--helper.sh b/t/t7412-submodule--helper.sh
index 042f3f5..0f2e5c6 100755
--- a/t/t7412-submodule--helper.sh
+++ b/t/t7412-submodule--helper.sh
@@ -104,4 +104,90 @@ test_expect_success 'in-group' '
  )
 '
 
+test_expect_success 'submodule init respects label' '
+ test_when_finished "rm -rf super_clone" &&
+ suburl=$(pwd)/sub &&
+ git clone super super_clone &&
+ (
+ cd super_clone &&
+ git submodule init \*bit1 &&
+ test_must_fail git config submodule.sub0.url &&
+ test        "$(git config submodule.sub1.url)" = "$suburl" &&
+ test_must_fail git config submodule.sub2.url &&
+ test        "$(git config submodule.sub3.url)" = "$suburl"
+ )
+'
+
+test_expect_success 'submodule init respects name' '
+ test_when_finished "rm -rf super_clone" &&
+ suburl=$(pwd)/sub &&
+ git clone super super_clone &&
+ (
+ cd super_clone &&
+ git submodule init :sub_name &&
+ test_must_fail git config submodule.sub0.url &&
+ test "$(git config submodule.sub_name.url)" = "$suburl"
+ )
+'
+
+test_expect_success 'submodule deinit respects label' '
+ suburl=$(pwd)/sub &&
+ (
+ cd super &&
+ git submodule init &&
+ git submodule deinit \*bit2 &&
+ test        "$(git config submodule.sub0.url)" = "$suburl" &&
+ test        "$(git config submodule.sub1.url)" = "$suburl" &&
+ test_must_fail git config submodule.sub2.url &&
+ test_must_fail git config submodule.sub3.url
+ )
+'
+
+test_expect_success 'submodule deinit respects label and path' '
+ suburl=$(pwd)/sub &&
+ (
+ cd super &&
+ git submodule init &&
+ git submodule deinit \*bit2 ./sub1 &&
+ test        "$(git config submodule.sub0.url)" = "$suburl" &&
+ test_must_fail git config submodule.sub1.url &&
+ test_must_fail git config submodule.sub2.url &&
+ test_must_fail git config submodule.sub3.url
+ )
+'
+
+cat >expect <<-EOF
+-sub1
+-sub3
+EOF
+
+test_expect_success 'submodule status respects label and path' '
+ suburl=$(pwd)/sub &&
+ (
+ cd super &&
+ git submodule deinit .
+ git submodule init &&
+ git submodule status \*bit1 ./sub1 | cut -c 1,43- >../actual
+ ) &&
+ test_cmp expect actual
+'
+
+test_expect_success 'submodule sync respects name and path' '
+ suburl=$(pwd)/sub &&
+ (
+ cd super &&
+ git submodule init &&
+ git config submodule."sub0".url "wrong" &&
+ git config submodule."sub1".url "wrong" &&
+ git config submodule."sub2".url "wrong" &&
+ git config submodule."sub3".url "wrong" &&
+ git submodule sync :sub_name ./sub1 | cut -c 1,43- >../actual &&
+ test "$(git config submodule.sub0.url)" = "wrong" &&
+ test "$(git config submodule.sub1.url)" = "$suburl" &&
+ test "$(git config submodule.sub2.url)" = "wrong" &&
+ test "$(git config submodule.sub3.url)" = "wrong" &&
+ test "$(git config submodule.sub_name.url)" = "$suburl"
+ )
+'
+
 test_done
--
2.8.0.35.g58985d9.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

[PATCH 6/7] submodule update: learn partial initialization

Stefan Beller-4
In reply to this post by Stefan Beller-4
The new switch `--init-default-group` updates the submodules which are
configured in `submodule.updateGroup`

Signed-off-by: Stefan Beller <[hidden email]>
---
 Documentation/config.txt        |  5 ++++
 Documentation/git-submodule.txt |  4 ++--
 git-submodule.sh                | 14 +++++++++--
 t/t7400-submodule-basic.sh      | 53 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 72 insertions(+), 4 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 59d7046..0f20019 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2735,6 +2735,11 @@ submodule.fetchJobs::
  in parallel. A value of 0 will give some reasonable default.
  If unset, it defaults to 1.
 
+submodule.updateGroup::
+ Specifies the group of submodules when `git submodule --init-group`
+ is called with no arguments. This setting is recorded in the initial
+ clone when `--init-submodule` was given.
+
 tag.sort::
  This variable controls the sort ordering of tags when displayed by
  linkgit:git-tag[1]. Without the "--sort=<value>" option provided, the
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 35ca355..e658d15 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -14,9 +14,9 @@ SYNOPSIS
 'git submodule' [--quiet] status [--cached] [--recursive] [--] [<submodulespec>...]
 'git submodule' [--quiet] init [--] [<submodulespec>...]
 'git submodule' [--quiet] deinit [-f|--force] [--] <submodulespec>...
-'git submodule' [--quiet] update [--init] [--remote] [-N|--no-fetch]
+'git submodule' [--quiet] update [--init[-default-group]] [--remote] [-N|--no-fetch]
       [-f|--force] [--rebase|--merge] [--reference <repository>]
-      [--depth <depth>] [--recursive] [--jobs <n>] [--] [<path>...]
+      [--depth <depth>] [--recursive] [--jobs <n>] [--] [<submodulespec>...]
 'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) <n>]
       [commit] [--] [<path>...]
 'git submodule' [--quiet] foreach [--recursive] <command>
diff --git a/git-submodule.sh b/git-submodule.sh
index c8e36c5..2b0b0cb 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -522,7 +522,12 @@ cmd_update()
  GIT_QUIET=1
  ;;
  -i|--init)
- init=1
+ test -z $init || test $init = by_args || die "$(gettext "Only one of --init or --init-default-group may be used.")"
+ init=by_args
+ ;;
+ --init-default-group)
+ test -z $init || test $init = by_config || die "$(gettext "Only one of --init or --init-default-group may be used.")"
+ init=by_config
  ;;
  --remote)
  remote=1
@@ -585,7 +590,12 @@ cmd_update()
 
  if test -n "$init"
  then
- cmd_init "--" "$@" || return
+ additional_init=
+ if test "$init" = "by_config"
+ then
+ additional_init=$(git config --get-all submodule.updateGroup)
+ fi
+ cmd_init "--" "$@" ${additional_init:+$additional_init} || return
  fi
 
  {
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 0adc4e4..41e65c2 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -1139,4 +1139,57 @@ test_expect_success 'submodule add recording wrong labels reports an error' '
  test_i18ngrep alphanumeric actual
 '
 
+test_expect_success 'setup superproject with labeled submodules' '
+ mkdir sub1 &&
+ (
+ cd sub1 &&
+ git init &&
+ test_commit test
+ test_commit test2
+ ) &&
+ mkdir labeledsuper &&
+ (
+ cd labeledsuper &&
+ git init &&
+ git submodule add ../sub1 sub0 &&
+ git submodule add -l bit1 ../sub1 sub1 &&
+ git submodule add -l bit2 ../sub1 sub2 &&
+ git submodule add -l bit2 -l bit1 ../sub1 sub3 &&
+ git commit -m "add labeled submodules"
+ )
+'
+
+cat >expect <<-EOF
+-sub0
+ sub1 (test2)
+ sub2 (test2)
+ sub3 (test2)
+EOF
+
+test_expect_success 'submodule update --init with a group' '
+ test_when_finished "rm -rf labeledsuper_clone" &&
+ pwd=$(pwd) &&
+ git clone file://"$pwd"/labeledsuper labeledsuper_clone &&
+ (
+ cd labeledsuper_clone &&
+ git submodule update --init \*bit1 ./sub2 &&
+ git submodule status |cut -c 1,43- >../actual
+ ) &&
+ test_cmp expect actual
+'
+
+test_expect_success 'submodule update --init-default-group' '
+ test_when_finished "rm -rf labeledsuper_clone" &&
+ pwd=$(pwd) &&
+ git clone file://"$pwd"/labeledsuper labeledsuper_clone &&
+ (
+ cd labeledsuper_clone &&
+ git config submodule.updateGroup \*bit1 &&
+ git config --add submodule.updateGroup ./sub2 &&
+ git submodule update --init-default-group &&
+ git submodule status |cut -c 1,43- >../actual
+ ) &&
+ test_cmp expect actual
+'
+
 test_done
--
2.8.0.35.g58985d9.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

[PATCH 7/7] clone: allow specification of submodules to be cloned

Stefan Beller-4
In reply to this post by Stefan Beller-4
This allows to specify a subset of all available submodules to be
initialized and cloned. It is unrelated to the `--recursive` option,
i.e. the user may still want to give `--recursive` as an option.

Originally `--recursive` implied to initialize all submodules, this
changes as well with the new option, such that only the specified
submodules are cloned, and their submodules (i.e. subsubmodules)
are cloned in full as the submodule specification is not passed on.

Signed-off-by: Stefan Beller <[hidden email]>
---
 Documentation/git-clone.txt | 26 +++++++++++----
 builtin/clone.c             | 40 +++++++++++++++++++++--
 t/t7400-submodule-basic.sh  | 79 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 136 insertions(+), 9 deletions(-)

diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index 45d74be..4a9e8bb 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -14,7 +14,8 @@ SYNOPSIS
   [-o <name>] [-b <name>] [-u <upload-pack>] [--reference <repository>]
   [--dissociate] [--separate-git-dir <git dir>]
   [--depth <depth>] [--[no-]single-branch]
-  [--recursive | --recurse-submodules] [--jobs <n>] [--] <repository>
+  [--recursive | --recurse-submodules] [--jobs <n>]
+  [--init-submodule <submodulespec>] [--] <repository>
   [<directory>]
 
 DESCRIPTION
@@ -205,12 +206,23 @@ objects from the source repository into a pack in the cloned repository.
 
 --recursive::
 --recurse-submodules::
- After the clone is created, initialize all submodules within,
- using their default settings. This is equivalent to running
- `git submodule update --init --recursive` immediately after
- the clone is finished. This option is ignored if the cloned
- repository does not have a worktree/checkout (i.e. if any of
- `--no-checkout`/`-n`, `--bare`, or `--mirror` is given)
+ After the clone is created, initialize and clone all submodules
+ within, using their default settings. This is equivalent to
+ running `git submodule update --recursive --init <submodulespec>`
+ immediately after the clone is finished. This option is ignored
+ if the cloned repository does not have a worktree/checkout (i.e.
+ if any of `--no-checkout`/`-n`, `--bare`, or `--mirror` is given)
+
+--init-submodule::
+ After the clone is cloned, initialize and clone specified
+ submodules within, using their default settings. It is possible
+ to give multiple specifications by giving this argument multiple
+ times or by giving a comma separated list. This is equivalent to
+ running `git submodule update <submodulespec>` immediately
+ after the clone is finished. To specify submodules you can use
+ their path, name or labels, see linkgit:git-submodules[1]. This
+ option will be recorded in the repository config as
+ `submodule.updateGroup`.
 
 --separate-git-dir=<git dir>::
  Instead of placing the cloned repository where it is supposed
diff --git a/builtin/clone.c b/builtin/clone.c
index 6576ecf..fa2f989 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -52,6 +52,22 @@ static struct string_list option_config;
 static struct string_list option_reference;
 static int option_dissociate;
 static int max_jobs = -1;
+static struct string_list init_submodules;
+
+static int init_submodules_cb(const struct option *opt, const char *arg, int unset)
+{
+ struct string_list_item *item;
+ struct string_list sl = STRING_LIST_INIT_DUP;
+
+ if (unset)
+ return -1;
+
+ string_list_split(&sl, arg, ',', -1);
+ for_each_string_list_item(item, &sl)
+ string_list_append((struct string_list *)opt->value, item->string);
+
+ return 0;
+}
 
 static struct option builtin_clone_options[] = {
  OPT__VERBOSITY(&option_verbosity),
@@ -100,6 +116,8 @@ static struct option builtin_clone_options[] = {
  TRANSPORT_FAMILY_IPV4),
  OPT_SET_INT('6', "ipv6", &family, N_("use IPv6 addresses only"),
  TRANSPORT_FAMILY_IPV6),
+ OPT_CALLBACK(0, "init-submodule", &init_submodules, N_("string"),
+ N_("clone specific submodules"), init_submodules_cb),
  OPT_END()
 };
 
@@ -731,13 +749,22 @@ static int checkout(void)
  err |= run_hook_le(NULL, "post-checkout", sha1_to_hex(null_sha1),
    sha1_to_hex(sha1), "1", NULL);
 
- if (!err && option_recursive) {
+ if (!err && (option_recursive || init_submodules.nr > 0)) {
  struct argv_array args = ARGV_ARRAY_INIT;
- argv_array_pushl(&args, "submodule", "update", "--init", "--recursive", NULL);
+ struct string_list_item *item;
+ argv_array_pushl(&args, "submodule", "update", NULL);
+
+ argv_array_pushf(&args, "--init");
+
+ if (option_recursive)
+ argv_array_pushf(&args, "--recursive");
 
  if (max_jobs != -1)
  argv_array_pushf(&args, "--jobs=%d", max_jobs);
 
+ for_each_string_list_item(item, &init_submodules)
+ argv_array_push(&args, item->string);
+
  err = run_command_v_opt(args.argv, RUN_GIT_CMD);
  argv_array_clear(&args);
  }
@@ -876,6 +903,15 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
  option_no_checkout = 1;
  }
 
+ if (init_submodules.nr > 0) {
+ struct string_list_item *item;
+ struct strbuf sb = STRBUF_INIT;
+ for_each_string_list_item(item, &init_submodules) {
+ strbuf_addf(&sb, "submodule.updateGroup=%s", item->string);
+ string_list_append(&option_config, strbuf_detach(&sb, 0));
+ }
+ }
+
  if (!option_origin)
  option_origin = "origin";
 
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 41e65c2..e7b6c1f 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -1192,4 +1192,83 @@ test_expect_success 'submodule update --init-default-group' '
  test_cmp expect actual
 '
 
+cat <<EOF > expected
+-sub0
+ sub1 (test2)
+-sub2
+ sub3 (test2)
+EOF
+
+test_expect_success 'clone --init-submodule works' '
+ test_when_finished "rm -rf super super_clone" &&
+ git clone --recurse-submodules --init-submodule \*bit1 labeledsuper super_clone &&
+ (
+ cd super_clone &&
+ git submodule status |cut -c 1,43- >../actual
+ ) &&
+ test_cmp actual expected
+'
+
+cat <<EOF > expect
+ sub0 (test2)
+ sub1 (test2)
+-sub2
+ sub3 (test2)
+EOF
+test_expect_success 'clone with multiple --init-submodule options' '
+ test_when_finished "rm -rf super super_clone" &&
+ git clone --recurse-submodules --init-submodule=\*bit1 --init-submodule ./sub0 labeledsuper super_clone &&
+ (
+ cd super_clone &&
+ git submodule status |cut -c1,43- >../actual
+ ) &&
+ test_cmp expect actual
+'
+
+cat <<EOF > expect
+ submoduleA (test2)
+-submoduleB
+EOF
+
+cat <<EOF > expect2
+ submoduleA (test2)
+-submoduleB
+ submoduleC (test2)
+EOF
+
+test_expect_success 'clone and subsequent updates correctly auto-initialize submodules' '
+ test_when_finished "rm -rf super super_clone" &&
+ mkdir super &&
+ pwd=$(pwd) &&
+ (
+ cd super &&
+ git init &&
+ git submodule add --label LA file://"$pwd"/sub1 submoduleA &&
+ git submodule add file://"$pwd"/sub1 submoduleB &&
+ git commit -a -m "create repository with submodules groups"
+ ) &&
+ git clone --recurse-submodules --init-submodule=\*LA super super_clone &&
+ (
+ cd super_clone &&
+ git submodule status |cut -c1,43- >../actual
+ ) &&
+ test_cmp expect actual &&
+ (
+ cd super &&
+ git init &&
+ git submodule add --label LA file://"$pwd"/sub1 submoduleC &&
+ git commit -a -m "add another labled submodule"
+ ) &&
+ (
+ cd super_clone &&
+ # obtain the new superproject
+ git pull &&
+ # submoduleC should just appear as it has the label LA
+ # which was configured in git clone
+ git submodule update --init-default-group &&
+ git submodule status |cut -c1,43- >../actual
+ ) &&
+ test_cmp expect2 actual
+'
+
 test_done
--
2.8.0.35.g58985d9.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/7] submodule--helper: add valid-label-name

Junio C Hamano
In reply to this post by Stefan Beller-4
Stefan Beller <[hidden email]> writes:

> +static int submodule_valid_label_name(const char *label)
> +{
> + if (!label || !strlen(label))
> + return 0;
> +
> + if (!isalnum(*label))
> + return 0;

I'd limit this one to isalpha() if I were doing this to make the
restriction similar to identifiers in traditional programming
language.

> + while (*label) {
> + if (!(isalnum(*label) ||
> + *label == '-'))

And throw in '_' to the mix while at it.

> + return 0;
> + label++;
> + }
> +
> + return 1;
> +}

If the convention is "0 is good", then please signal "bad" with a
negative value, not just "non-zero".
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/7] submodule add: label submodules if asked to

Junio C Hamano
In reply to this post by Stefan Beller-4
Stefan Beller <[hidden email]> writes:

> diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
> index 814ee63..0adc4e4 100755
> --- a/t/t7400-submodule-basic.sh
> +++ b/t/t7400-submodule-basic.sh
> @@ -1056,6 +1056,7 @@ test_expect_success 'submodule with UTF-8 name' '
>  '
>  
>  test_expect_success 'submodule add clone shallow submodule' '
> + test_when_finished "rm -rf super" &&
>   mkdir super &&
>   pwd=$(pwd) &&
>   (
> @@ -1094,5 +1095,48 @@ test_expect_success 'submodule helper list is not confused by common prefixes' '
>   test_cmp expect actual
>  '
>  
> +test_expect_success 'submodule add records a label' '
> + test_when_finished "rm -rf super" &&
> + mkdir super &&
> + pwd=$(pwd) &&
> + (
> + cd super &&
> + git init &&
> + git submodule add --label labelA file://"$pwd"/example2 submodule &&
> + git config -f .gitmodules submodule."submodule".label >../actual &&
> + echo labelA >../expect
> + ) &&
> + test_cmp expect actual
> +'
> +
> +cat >expect <<-EOF
> +labelA
> +labelB
> +EOF
> +
> +test_expect_success 'submodule add records multiple labels' '

The existing tests in this file may be littered with this bad
construct, but please do not add more example of running things
outside of test_expect_{success,failure} block unless there is a
good reason to do so.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 3/7] submodule-config: keep labels around

Junio C Hamano
In reply to this post by Stefan Beller-4
Stefan Beller <[hidden email]> writes:

> @@ -199,6 +203,7 @@ static struct submodule *lookup_or_create_by_name(struct submodule_cache *cache,
>   submodule->update_strategy.command = NULL;
>   submodule->fetch_recurse = RECURSE_SUBMODULES_NONE;
>   submodule->ignore = NULL;
> + submodule->labels = NULL;

Hmph, is there a reason to do this, instead of embedding an instance
of "struct string_list" inside submodule structure?

I am not yet claiming that embedding is better.  Just wondering if
it makes it easier to handle initialization as seen in the hunk
below, and also _clear() procedure.

> @@ -353,6 +358,17 @@ static int parse_config(const char *var, const char *value, void *data)
>   else if (parse_submodule_update_strategy(value,
>   &submodule->update_strategy) < 0)
>   die(_("invalid value for %s"), var);
> + } else if (!strcmp(item.buf, "label")) {
> + if (!value)
> + ret = config_error_nonbool(var);
> + else {
> + if (!submodule->labels) {
> + submodule->labels =
> + xmalloc(sizeof(*submodule->labels));
> + string_list_init(submodule->labels, 1);
> + }
> + string_list_insert(submodule->labels, value);
> + }
>   }
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 4/7] submodule-config: check if a submodule is in a group

Junio C Hamano
In reply to this post by Stefan Beller-4
Stefan Beller <[hidden email]> writes:

> +static int in_group(int argc, const char **argv, const char *prefix)
> + ...
> + if (!group)
> + list = git_config_get_value_multi("submodule.updateGroup");
> + else {
> + string_list_split(&actual_list, group, ',', -1);

Is this expected to be used only by test scripts, or will it have
real scripted Porcelain as its users?  I am wondering if
concatenation with ' ' would be more natural if it is the latter (if
only used for testing, I don't care that much).

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 5/7] submodule--helper module_list_compute: allow label or name arguments

Junio C Hamano
In reply to this post by Stefan Beller-4
Stefan Beller <[hidden email]> writes:

> +static void split_argv_pathspec_groups(int argc, const char **argv,
> +       const char ***pathspec_argv,
> +       struct string_list *group)
> +{
> + int i;
> + struct argv_array ps = ARGV_ARRAY_INIT;
> + for (i = 0; i < argc; i++) {
> + if (starts_with(argv[i], "*")
> +    || starts_with(argv[i], ":")) {
> + string_list_insert(group, argv[i]);
> + } else if (starts_with(argv[i], "./")) {
> + argv_array_push(&ps, argv[i]);

I'd suggest stripping "./" when you do this.  That is, make the rule
to be "*label is a label, :name is a name, and everything else is a
path.  You can prefix ./ to an unfortunate path that begins with an
asterisk or a colon and we will strip ./ disambiguator".

> + } else {
> + /*
> + * NEEDSWORK:
> + * Improve autodetection of items with no prefixes,
> + * roughly like
> + * if (label_or_name_exists(argv[i]))
> + * string_list_insert(group, argv[i]);
> + * else
> + * argv_array_push(&ps, argv[i]);
> + */

I do not think this is desirable.  "label" that changes its meaning
between "a path ./label" to "a label *label" would force people to
give unnecessary prefix "./" when naming their path, from fear of
colliding with a label that may or may not exist.

> + argv_array_push(&ps, argv[i]);
> + }
> + }



> + if (!group.nr) {
> + if (!match_pathspec(pathspec, ce->name, ce_namelen(ce),
> +    0, ps_matched, 1) ||
> +    !S_ISGITLINK(ce->ce_mode))
> + continue;
> + } else {
> + const struct submodule *sub;
> + int ps = 0, gr = 0;
> + if (!S_ISGITLINK(ce->ce_mode))
> + continue;
> + sub = submodule_from_path(null_sha1, ce->name);
> +
> + ps = match_pathspec(pathspec, ce->name, ce_namelen(ce),
> +    0, ps_matched, 1);
> + gr = submodule_in_group(&group, sub);
> +
> + if (!pathspec->nr && !gr)
> + continue;
> + if (!ps && !gr)
> + continue;

Hmph, so the rule is "a submodule that is in the specified group is
chosen, even if it is outside the subset of paths narrowed by the
given pathspec."  I think that is consistent with the way how the
list of arguments given to module_list (i.e. a pathspec element plus
a group specifier OR together just like two pathspec elements do not
force a path to match both, and instead they OR together).

Is that rule (i.e. specifiers are ORed together) written down
somewhere for users?

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/7] submodule groups

Junio C Hamano
In reply to this post by Stefan Beller-4
Stefan Beller <[hidden email]> writes:

> I started from scratch as I think there were some sharp edges in the design.
> My thinking shifted from "submodule groups" towards "actually it's just an
> enhanced pathspec, called submodulespec".

Except for minor things I mentioned separately, overall, this seems
quite cleanly done.

Looks very promising.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 5/7] submodule--helper module_list_compute: allow label or name arguments

Junio C Hamano
In reply to this post by Junio C Hamano
Junio C Hamano <[hidden email]> writes:

> Stefan Beller <[hidden email]> writes:
>
>> +static void split_argv_pathspec_groups(int argc, const char **argv,
>> +       const char ***pathspec_argv,
>> +       struct string_list *group)
>> +{
>> + int i;
>> + struct argv_array ps = ARGV_ARRAY_INIT;
>> + for (i = 0; i < argc; i++) {
>> + if (starts_with(argv[i], "*")
>> +    || starts_with(argv[i], ":")) {
>> + string_list_insert(group, argv[i]);
>> + } else if (starts_with(argv[i], "./")) {
>> + argv_array_push(&ps, argv[i]);
>
> I'd suggest stripping "./" when you do this.  That is, make the rule
> to be "*label is a label, :name is a name, and everything else is a
> path.  You can prefix ./ to an unfortunate path that begins with an
> asterisk or a colon and we will strip ./ disambiguator".

To clarify, "./$path and $path are the same so why strip it?" is an
understandable, even though naive, question. The reason is because
you do not want to contaminate the code that parses pathspecs with
the knowledge of this submodule-group specific prefix.  "./$path"
and "$path" may be equivalent for a literal pathspec, but I'd want
to see user be able to say

   git submodule foreach './:(icase)foo'

and find Foo, foo, FOO, etc.

I would also recommend to strip '*' and ':' from group names and
module names, and maintain two separate lists.  You would eventually
want to do "*default*" to name all groups whose name matches with
the pattern "default*", as if it were a pathspec matched against the
available group names, and that will keep the door open for future
extension like  "*:(icase)default*".

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/7] submodule add: label submodules if asked to

Stefan Beller-4
In reply to this post by Junio C Hamano
>> +cat >expect <<-EOF
>> +labelA
>> +labelB
>> +EOF
>> +
>> +test_expect_success 'submodule add records multiple labels' '
>
> The existing tests in this file may be littered with this bad
> construct, but please do not add more example of running things
> outside of test_expect_{success,failure} block unless there is a
> good reason to do so.

I thought that was the standard for tests as I have seen
them quite a few times. Looking for those "cat >expect ..." constructs
more closely they are often found inside tests as well. Makes sense
if the expectation is used for only one test.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 3/7] submodule-config: keep labels around

Stefan Beller-4
In reply to this post by Junio C Hamano
On Tue, May 10, 2016 at 6:15 PM, Junio C Hamano <[hidden email]> wrote:

> Stefan Beller <[hidden email]> writes:
>
>> @@ -199,6 +203,7 @@ static struct submodule *lookup_or_create_by_name(struct submodule_cache *cache,
>>       submodule->update_strategy.command = NULL;
>>       submodule->fetch_recurse = RECURSE_SUBMODULES_NONE;
>>       submodule->ignore = NULL;
>> +     submodule->labels = NULL;
>
> Hmph, is there a reason to do this, instead of embedding an instance
> of "struct string_list" inside submodule structure?
>
> I am not yet claiming that embedding is better.  Just wondering if
> it makes it easier to handle initialization as seen in the hunk
> below, and also _clear() procedure.

Thanks for pointing out that alternative.  That looks so much
better in this patch. Let's see how the follow up patches develop.
As we'd not check != NULL first, but check against the count of the
string list. (I expect no problems down that road though).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 3/7] submodule-config: keep labels around

Junio C Hamano
Stefan Beller <[hidden email]> writes:

> On Tue, May 10, 2016 at 6:15 PM, Junio C Hamano <[hidden email]> wrote:
>> Stefan Beller <[hidden email]> writes:
>>
>>> @@ -199,6 +203,7 @@ static struct submodule *lookup_or_create_by_name(struct submodule_cache *cache,
>>>       submodule->update_strategy.command = NULL;
>>>       submodule->fetch_recurse = RECURSE_SUBMODULES_NONE;
>>>       submodule->ignore = NULL;
>>> +     submodule->labels = NULL;
>>
>> Hmph, is there a reason to do this, instead of embedding an instance
>> of "struct string_list" inside submodule structure?
>>
>> I am not yet claiming that embedding is better.  Just wondering if
>> it makes it easier to handle initialization as seen in the hunk
>> below, and also _clear() procedure.
>
> Thanks for pointing out that alternative.  That looks so much
> better in this patch. Let's see how the follow up patches develop.
> As we'd not check != NULL first, but check against the count of the
> string list. (I expect no problems down that road though).

I also wonder if we can say the same for the .ignore field, by the
way, if we agree that it is a better direction to go.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 3/7] submodule-config: keep labels around

Stefan Beller-4
+Heiko

On Wed, May 11, 2016 at 2:28 PM, Junio C Hamano <[hidden email]> wrote:

> Stefan Beller <[hidden email]> writes:
>
>> On Tue, May 10, 2016 at 6:15 PM, Junio C Hamano <[hidden email]> wrote:
>>> Stefan Beller <[hidden email]> writes:
>>>
>>>> @@ -199,6 +203,7 @@ static struct submodule *lookup_or_create_by_name(struct submodule_cache *cache,
>>>>       submodule->update_strategy.command = NULL;
>>>>       submodule->fetch_recurse = RECURSE_SUBMODULES_NONE;
>>>>       submodule->ignore = NULL;
>>>> +     submodule->labels = NULL;
>>>
>>> Hmph, is there a reason to do this, instead of embedding an instance
>>> of "struct string_list" inside submodule structure?
>>>
>>> I am not yet claiming that embedding is better.  Just wondering if
>>> it makes it easier to handle initialization as seen in the hunk
>>> below, and also _clear() procedure.
>>
>> Thanks for pointing out that alternative.  That looks so much
>> better in this patch. Let's see how the follow up patches develop.
>> As we'd not check != NULL first, but check against the count of the
>> string list. (I expect no problems down that road though).
>
> I also wonder if we can say the same for the .ignore field, by the
> way, if we agree that it is a better direction to go.

I don't think this is a good idea, as it's just a string, like .{url,
name, path}

Instead of storing the string we could store an enum {UNTRACKED,
DIRTY, ALL, NONE, NOT_INIT} though. That would be better I guess.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/7] submodule groups

Stefan Beller-4
In reply to this post by Junio C Hamano
On Tue, May 10, 2016 at 7:08 PM, Junio C Hamano <[hidden email]> wrote:
> Stefan Beller <[hidden email]> writes:
>
>> I started from scratch as I think there were some sharp edges in the design.
>> My thinking shifted from "submodule groups" towards "actually it's just an
>> enhanced pathspec, called submodulespec".
>
> Except for minor things I mentioned separately, overall, this seems
> quite cleanly done.

I disagree (now).

I started documenting the <submodulespec> as an extension of
the pathspecs. While I thought the logical OR was the right way to go,
I think it is wrong now. So there is stuff in tests like:

    git submodule--helper matches-submodulespec sub0 ./.
./:(exclude)*0 *label-sub0

which should test if the first argument (sub0) matches the submodulespec
which follows. And it matches sub0 by matching the label, although
we told it to ignore anything ending in 0

So I wonder if we rather want to extend the pathspec magic to
include properties of blobs (i.e. submodules):

    git <command> . :(sub-label:label-sub0) :(exclude)*0

would look much more powerful too me. Properties of blobs
may also be interesting for otherwise. Imagine looking for huge files
(in a bare repo, so you have to use Git and not your shell tools):

  git ls-files . :(file-size:>1024k)

>
> Looks very promising.
>

Thanks for the encouragement!

Thanks,
Stefan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
12