{"thread":{"id":"41333","subject":"[PATCHv8 1/9] submodule-config: keep update strategy around","startedAt":"2016-02-04T22:09:27Z","lastAt":"2016-02-06T01:36:02Z","messageCount":23,"participants":["Stefan Beller","Jonathan Nieder","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"277475","messageId":"1454623776-3347-1-git-send-email-sbeller@google.com","threadId":"41333","inReplyTo":null,"subject":"[PATCHv8 0/9] Expose submodule parallelism to the user","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-04T22:09:27Z","receivedAt":"2016-02-04T22:09:27Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This replaces sb/submodule-parallel-update.\n(which is based on sb/submodule-parallel-fetch,\nbut this series also applies cleanly on master)\n\n* using a \"more\" correct version of parsing the section title\n* fixed the memleaks, free-after-use in\n \"git submodule update: have a dedicated helper for cloning\"\n* reworded documentation.\n* another additional patch snuck in, which makes code a bit more readable\n (0004-submodule-config-slightly-simplify-lookup_or_create_.patch)\n\nThanks,\nStefan\n\nInterdiff to v7:\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex eda3276..3b02732 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2647,11 +2647,10 @@ submodule..ignore::\n \taffected by this setting.\n \n submodule.fetchJobs::\n-\tThis is used to determine how many submodules will be\n-\tfetched/cloned at the same time. Specifying a positive integer\n-\tallows up to that number of submodules being fetched in parallel.\n-\tThis is used in fetch and clone operations only. A value of 0 will\n-\tgive some reasonable configuration. It defaults to 1.\n+\tSpecifies how many submodules are fetched/cloned at the same time.\n+\tA positive integer allows up to that number of submodules fetched\n+\tin parallel. A value of 0 will give some reasonable default.\n+\tIf unset, it defaults to 1.\n \n tag.sort::\n \tThis variable controls the sort ordering of tags when displayed by\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 8002187..7e01b85 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -312,33 +312,31 @@ static int update_clone_get_next_task(struct child_process *cp,\n \n \tfor (; pp->count < pp->list.nr; pp->count++) {\n \t\tconst struct submodule *sub = NULL;\n-\t\tconst char *displaypath = NULL;\n \t\tconst struct cache_entry *ce = pp->list.entries[pp->count];\n+\t\tstruct strbuf displaypath_sb = STRBUF_INIT;\n \t\tstruct strbuf sb = STRBUF_INIT;\n+\t\tconst char *displaypath = NULL;\n \t\tconst char *update_module = NULL;\n \t\tchar *url = NULL;\n \t\tint needs_cloning = 0;\n \n \t\tif (ce_stage(ce)) {\n \t\t\tif (pp->recursive_prefix)\n-\t\t\t\tstrbuf_addf(err, \"Skipping unmerged submodule %s/%s\\n\",\n+\t\t\t\tstrbuf_addf(err,\n+\t\t\t\t\t\"Skipping unmerged submodule %s/%s\\n\",\n \t\t\t\t\tpp->recursive_prefix, ce->name);\n \t\t\telse\n-\t\t\t\tstrbuf_addf(err, \"Skipping unmerged submodule %s\\n\",\n+\t\t\t\tstrbuf_addf(err,\n+\t\t\t\t\t\"Skipping unmerged submodule %s\\n\",\n \t\t\t\t\tce->name);\n-\t\t\tcontinue;\n+\t\t\tgoto cleanup_and_continue;\n \t\t}\n \n \t\tsub = submodule_from_path(null_sha1, ce->name);\n-\t\tif (!sub) {\n-\t\t\tstrbuf_addf(err, \"BUG: internal error managing submodules. \"\n-\t\t\t\t \"The cache could not locate '%s'\", ce->name);\n-\t\t\tpp->print_unmatched = 1;\n-\t\t\tcontinue;\n-\t\t}\n \n \t\tif (pp->recursive_prefix)\n-\t\t\tdisplaypath = relative_path(pp->recursive_prefix, ce->name, &sb);\n+\t\t\tdisplaypath = relative_path(pp->recursive_prefix,\n+\t\t\t\t\t\t ce->name, &displaypath_sb);\n \t\telse\n \t\t\tdisplaypath = ce->name;\n \n@@ -349,14 +347,15 @@ static int update_clone_get_next_task(struct child_process *cp,\n \t\tif (!update_module)\n \t\t\tupdate_module = \"checkout\";\n \t\tif (!strcmp(update_module, \"none\")) {\n-\t\t\tstrbuf_addf(err, \"Skipping submodule '%s'\\n\", displaypath);\n-\t\t\tcontinue;\n+\t\t\tstrbuf_addf(err, \"Skipping submodule '%s'\\n\",\n+\t\t\t\t displaypath);\n+\t\t\tgoto cleanup_and_continue;\n \t\t}\n \n \t\t/*\n \t\t * Looking up the url in .git/config.\n-\t\t * We must not fall back to .gitmodules as we only want to process\n-\t\t * configured submodules.\n+\t\t * We must not fall back to .gitmodules as we only want\n+\t\t * to process configured submodules.\n \t\t */\n \t\tstrbuf_reset(&sb);\n \t\tstrbuf_addf(&sb, \"submodule.%s.url\", sub->name);\n@@ -367,9 +366,11 @@ static int update_clone_get_next_task(struct child_process *cp,\n \t\t\t * path have been specified\n \t\t\t */\n \t\t\tif (pp->pathspec.nr)\n-\t\t\t\tstrbuf_addf(err, _(\"Submodule path '%s' not initialized\\n\"\n-\t\t\t\t\t\"Maybe you want to use 'update --init'?\"), displaypath);\n-\t\t\tcontinue;\n+\t\t\t\tstrbuf_addf(err,\n+\t\t\t\t\t _(\"Submodule path '%s' not initialized\\n\"\n+\t\t\t\t\t \"Maybe you want to use 'update --init'?\"),\n+\t\t\t\t\t displaypath);\n+\t\t\tgoto cleanup_and_continue;\n \t\t}\n \n \t\tstrbuf_reset(&sb);\n@@ -383,13 +384,19 @@ static int update_clone_get_next_task(struct child_process *cp,\n \t\tstring_list_append(&pp->projectlines, sb.buf);\n \n \t\tif (needs_cloning) {\n-\t\t\tfill_clone_command(cp, pp->quiet, pp->prefix, ce->name,\n-\t\t\t\t\t sub->name, url, pp->reference, pp->depth);\n+\t\t\tfill_clone_command(cp, pp->quiet, pp->prefix,\n+\t\t\t\t\t ce->name, sub->name, strdup(url),\n+\t\t\t\t\t pp->reference, pp->depth);\n \t\t\tpp->count++;\n-\t\t\tfree(url);\n+\t\t}\n+\n+cleanup_and_continue:\n+\t\tfree(url);\n+\t\tstrbuf_reset(&displaypath_sb);\n+\t\tstrbuf_reset(&sb);\n+\n+\t\tif (needs_cloning)\n \t\t\treturn 1;\n-\t\t} else\n-\t\t\tfree(url);\n \t}\n \treturn 0;\n }\ndiff --git a/submodule-config.c b/submodule-config.c\nindex a32259e..339b59d 100644\n--- a/submodule-config.c\n+++ b/submodule-config.c\n@@ -32,7 +32,7 @@ enum lookup_type {\n \n static struct submodule_cache cache;\n static int is_cache_init;\n-static int parallel_jobs = -1;\n+static unsigned long parallel_jobs = -1;\n \n static int config_path_cmp(const struct submodule_entry *a,\n \t\t\t const struct submodule_entry *b,\n@@ -163,22 +163,17 @@ static struct submodule *cache_lookup_name(struct submodule_cache *cache,\n }\n \n static struct submodule *lookup_or_create_by_name(struct submodule_cache *cache,\n-\t\t\t\t\t\t const unsigned char *gitmodules_sha1,\n-\t\t\t\t\t\t const char *name_ptr, int name_len)\n+\t\tconst unsigned char *gitmodules_sha1, const char *name)\n {\n \tstruct submodule *submodule;\n-\tstruct strbuf name_buf = STRBUF_INIT;\n-\tchar *name = xmemdupz(name_ptr, name_len);\n \n \tsubmodule = cache_lookup_name(cache, gitmodules_sha1, name);\n \tif (submodule)\n-\t\tgoto out;\n+\t\treturn submodule;\n \n \tsubmodule = xmalloc(sizeof(*submodule));\n \n-\tstrbuf_addstr(&name_buf, name);\n-\tsubmodule->name = strbuf_detach(&name_buf, NULL);\n-\n+\tsubmodule->name = xstrdup(name);\n \tsubmodule->path = NULL;\n \tsubmodule->url = NULL;\n \tsubmodule->update = NULL;\n@@ -188,8 +183,7 @@ static struct submodule *lookup_or_create_by_name(struct submodule_cache *cache,\n \thashcpy(submodule->gitmodules_sha1, gitmodules_sha1);\n \n \tcache_add(cache, submodule);\n-out:\n-\tfree(name);\n+\n \treturn submodule;\n }\n \n@@ -241,18 +235,16 @@ static int parse_generic_submodule_config(const char *key,\n \t\t\t\t\t struct parse_config_parameter *me)\n {\n \tif (!strcmp(key, \"fetchjobs\")) {\n-\t\tparallel_jobs = strtol(value, NULL, 10);\n-\t\tif (parallel_jobs < 0) {\n-\t\t\twarning(\"submodule.fetchJobs not allowed to be negative.\");\n+\t\tif (!git_parse_ulong(value, ¶llel_jobs)) {\n+\t\t\twarning(_(\"Error parsing submodule.fetchJobs; Defaulting to 1.\"));\n \t\t\tparallel_jobs = 1;\n-\t\t\treturn 1;\n \t\t}\n \t}\n \n \treturn 0;\n }\n \n-static int parse_specific_submodule_config(const char *subsection, int subsection_len,\n+static int parse_specific_submodule_config(const char *subsection,\n \t\t\t\t\t const char *key,\n \t\t\t\t\t const char *var,\n \t\t\t\t\t const char *value,\n@@ -262,7 +254,7 @@ static int parse_specific_submodule_config(const char *subsection, int subsectio\n \tstruct submodule *submodule;\n \tsubmodule = lookup_or_create_by_name(me->cache,\n \t\t\t\t\t me->gitmodules_sha1,\n-\t\t\t\t\t subsection, subsection_len);\n+\t\t\t\t\t subsection);\n \n \tif (!strcmp(key, \"path\")) {\n \t\tif (!value)\n@@ -332,19 +324,22 @@ static int parse_specific_submodule_config(const char *subsection, int subsectio\n static int parse_config(const char *var, const char *value, void *data)\n {\n \tstruct parse_config_parameter *me = data;\n-\tint subsection_len;\n+\tint subsection_len, ret;\n \tconst char *subsection, *key;\n+\tchar *sub;\n \n \tif (parse_config_key(var, \"submodule\", &subsection,\n \t\t\t &subsection_len, &key) < 0)\n \t\treturn 0;\n \n-\tif (!subsection_len)\n+\tif (!subsection)\n \t\treturn parse_generic_submodule_config(key, var, value, me);\n-\telse\n-\t\treturn parse_specific_submodule_config(subsection,\n-\t\t\t\t\t\t subsection_len, key,\n-\t\t\t\t\t\t var, value, me);\n+\n+\tsub = xmemdupz(subsection, subsection_len);\n+\tret = parse_specific_submodule_config(sub, key,\n+\t\t\t\t\t var, value, me);\n+\tfree(sub);\n+\treturn ret;\n }\n \n static int gitmodule_sha1_from_commit(const unsigned char *commit_sha1,\n@@ -493,7 +488,7 @@ void submodule_free(void)\n \tis_cache_init = 0;\n }\n \n-int config_parallel_submodules(void)\n+unsigned long config_parallel_submodules(void)\n {\n \treturn parallel_jobs;\n }\ndiff --git a/submodule-config.h b/submodule-config.h\nindex d9bbf9a..bab1e8d 100644\n--- a/submodule-config.h\n+++ b/submodule-config.h\n@@ -27,6 +27,6 @@ const struct submodule *submodule_from_path(const unsigned char *commit_sha1,\n \t\tconst char *path);\n void submodule_free(void);\n \n-int config_parallel_submodules(void);\n+unsigned long config_parallel_submodules(void);\n \n #endif /* SUBMODULE_CONFIG_H */\n\n\nStefan Beller (9):\n submodule-config: keep update strategy around\n submodule-config: drop check against NULL\n submodule-config: remove name_and_item_from_var\n submodule-config: slightly simplify lookup_or_create_by_name\n submodule-config: introduce parse_generic_submodule_config\n fetching submodules: respect `submodule.fetchJobs` config option\n git submodule update: have a dedicated helper for cloning\n submodule update: expose parallelism to the user\n clone: allow an explicit argument for parallel submodule clones\n\n Documentation/config.txt | 6 +\n Documentation/git-clone.txt | 6 +-\n Documentation/git-submodule.txt | 7 +-\n builtin/clone.c | 19 +++-\n builtin/fetch.c | 2 +-\n builtin/submodule--helper.c | 246 ++++++++++++++++++++++++++++++++++++++++\n git-submodule.sh | 54 ++++-----\n submodule-config.c | 104 ++++++++++-------\n submodule-config.h | 3 +\n submodule.c | 5 +\n t/t5526-fetch-submodules.sh | 14 +++\n t/t7400-submodule-basic.sh | 4 +-\n t/t7406-submodule-update.sh | 27 +++++\n 13 files changed, 414 insertions(+), 83 deletions(-)\n\n-- \n2.7.0.rc0.41.gd102984.dirty\n"},{"id":"277467","messageId":"1454623776-3347-2-git-send-email-sbeller@google.com","threadId":"41333","inReplyTo":"1454623776-3347-1-git-send-email-sbeller@google.com","subject":"[PATCHv8 1/9] submodule-config: keep update strategy around","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-04T22:09:28Z","receivedAt":"2016-02-04T22:09:28Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"We need the submodule update strategies in a later patch.\n\nSigned-off-by: Stefan Beller \nSigned-off-by: Junio C Hamano \n---\n submodule-config.c | 11 +++++++++++\n submodule-config.h | 1 +\n 2 files changed, 12 insertions(+)\n\ndiff --git a/submodule-config.c b/submodule-config.c\nindex afe0ea8..4239b0e 100644\n--- a/submodule-config.c\n+++ b/submodule-config.c\n@@ -194,6 +194,7 @@ static struct submodule *lookup_or_create_by_name(struct submodule_cache *cache,\n \n \tsubmodule->path = NULL;\n \tsubmodule->url = NULL;\n+\tsubmodule->update = NULL;\n \tsubmodule->fetch_recurse = RECURSE_SUBMODULES_NONE;\n \tsubmodule->ignore = NULL;\n \n@@ -311,6 +312,16 @@ static int parse_config(const char *var, const char *value, void *data)\n \t\t\tfree((void *) submodule->url);\n \t\t\tsubmodule->url = xstrdup(value);\n \t\t}\n+\t} else if (!strcmp(item.buf, \"update\")) {\n+\t\tif (!value)\n+\t\t\tret = config_error_nonbool(var);\n+\t\telse if (!me->overwrite && submodule->update != NULL)\n+\t\t\twarn_multiple_config(me->commit_sha1, submodule->name,\n+\t\t\t\t\t \"update\");\n+\t\telse {\n+\t\t\tfree((void *) submodule->update);\n+\t\t\tsubmodule->update = xstrdup(value);\n+\t\t}\n \t}\n \n \tstrbuf_release(&name);\ndiff --git a/submodule-config.h b/submodule-config.h\nindex 9061e4e..f9e2a29 100644\n--- a/submodule-config.h\n+++ b/submodule-config.h\n@@ -14,6 +14,7 @@ struct submodule {\n \tconst char *url;\n \tint fetch_recurse;\n \tconst char *ignore;\n+\tconst char *update;\n \t/* the sha1 blob id of the responsible .gitmodules file */\n \tunsigned char gitmodules_sha1[20];\n };\n-- \n2.7.0.rc0.41.gd102984.dirty\n"},{"id":"277474","messageId":"1454623776-3347-3-git-send-email-sbeller@google.com","threadId":"41333","inReplyTo":"1454623776-3347-1-git-send-email-sbeller@google.com","subject":"[PATCHv8 2/9] submodule-config: drop check against NULL","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-04T22:09:29Z","receivedAt":"2016-02-04T22:09:29Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Adhere to the common coding style of Git and not check explicitly\nfor NULL throughout the file. There are still other occurrences in the\ncode base but that is usually inside of conditions with side effects.\n\nSigned-off-by: Stefan Beller \n---\n submodule-config.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/submodule-config.c b/submodule-config.c\nindex 4239b0e..6d01941 100644\n--- a/submodule-config.c\n+++ b/submodule-config.c\n@@ -265,7 +265,7 @@ static int parse_config(const char *var, const char *value, void *data)\n \tif (!strcmp(item.buf, \"path\")) {\n \t\tif (!value)\n \t\t\tret = config_error_nonbool(var);\n-\t\telse if (!me->overwrite && submodule->path != NULL)\n+\t\telse if (!me->overwrite && submodule->path)\n \t\t\twarn_multiple_config(me->commit_sha1, submodule->name,\n \t\t\t\t\t\"path\");\n \t\telse {\n@@ -289,7 +289,7 @@ static int parse_config(const char *var, const char *value, void *data)\n \t} else if (!strcmp(item.buf, \"ignore\")) {\n \t\tif (!value)\n \t\t\tret = config_error_nonbool(var);\n-\t\telse if (!me->overwrite && submodule->ignore != NULL)\n+\t\telse if (!me->overwrite && submodule->ignore)\n \t\t\twarn_multiple_config(me->commit_sha1, submodule->name,\n \t\t\t\t\t\"ignore\");\n \t\telse if (strcmp(value, \"untracked\") &&\n@@ -305,7 +305,7 @@ static int parse_config(const char *var, const char *value, void *data)\n \t} else if (!strcmp(item.buf, \"url\")) {\n \t\tif (!value) {\n \t\t\tret = config_error_nonbool(var);\n-\t\t} else if (!me->overwrite && submodule->url != NULL) {\n+\t\t} else if (!me->overwrite && submodule->url) {\n \t\t\twarn_multiple_config(me->commit_sha1, submodule->name,\n \t\t\t\t\t\"url\");\n \t\t} else {\n@@ -315,7 +315,7 @@ static int parse_config(const char *var, const char *value, void *data)\n \t} else if (!strcmp(item.buf, \"update\")) {\n \t\tif (!value)\n \t\t\tret = config_error_nonbool(var);\n-\t\telse if (!me->overwrite && submodule->update != NULL)\n+\t\telse if (!me->overwrite && submodule->update)\n \t\t\twarn_multiple_config(me->commit_sha1, submodule->name,\n \t\t\t\t\t \"update\");\n \t\telse {\n-- \n2.7.0.rc0.41.gd102984.dirty\n"},{"id":"277468","messageId":"1454623776-3347-4-git-send-email-sbeller@google.com","threadId":"41333","inReplyTo":"1454623776-3347-1-git-send-email-sbeller@google.com","subject":"[PATCHv8 3/9] submodule-config: remove name_and_item_from_var","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-04T22:09:30Z","receivedAt":"2016-02-04T22:09:30Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"`name_and_item_from_var` does not provide the proper abstraction\nwe need here in a later patch. As we have only a single call site\nfor `name_and_item_from_var`, just inline it. Instead of using\nstrbufs, use direct char *.\n\nSigned-off-by: Stefan Beller \n---\n submodule-config.c | 40 +++++++++++-----------------------------\n 1 file changed, 11 insertions(+), 29 deletions(-)\n\ndiff --git a/submodule-config.c b/submodule-config.c\nindex 6d01941..c08ee7f 100644\n--- a/submodule-config.c\n+++ b/submodule-config.c\n@@ -161,22 +161,6 @@ static struct submodule *cache_lookup_name(struct submodule_cache *cache,\n \treturn NULL;\n }\n \n-static int name_and_item_from_var(const char *var, struct strbuf *name,\n-\t\t\t\t struct strbuf *item)\n-{\n-\tconst char *subsection, *key;\n-\tint subsection_len, parse;\n-\tparse = parse_config_key(var, \"submodule\", &subsection,\n-\t\t\t&subsection_len, &key);\n-\tif (parse < 0 || !subsection)\n-\t\treturn 0;\n-\n-\tstrbuf_add(name, subsection, subsection_len);\n-\tstrbuf_addstr(item, key);\n-\n-\treturn 1;\n-}\n-\n static struct submodule *lookup_or_create_by_name(struct submodule_cache *cache,\n \t\tconst unsigned char *gitmodules_sha1, const char *name)\n {\n@@ -251,18 +235,19 @@ static int parse_config(const char *var, const char *value, void *data)\n {\n \tstruct parse_config_parameter *me = data;\n \tstruct submodule *submodule;\n-\tstruct strbuf name = STRBUF_INIT, item = STRBUF_INIT;\n-\tint ret = 0;\n+\tint subsection_len, ret = 0;\n+\tconst char *subsection, *key;\n \n-\t/* this also ensures that we only parse submodule entries */\n-\tif (!name_and_item_from_var(var, &name, &item))\n+\tif (parse_config_key(var, \"submodule\", &subsection,\n+\t\t\t &subsection_len, &key) < 0 || !subsection)\n \t\treturn 0;\n \n+\tsubsection = xmemdupz(subsection, subsection_len);\n \tsubmodule = lookup_or_create_by_name(me->cache,\n \t\t\t\t\t me->gitmodules_sha1,\n-\t\t\t\t\t name.buf);\n+\t\t\t\t\t subsection);\n \n-\tif (!strcmp(item.buf, \"path\")) {\n+\tif (!strcmp(key, \"path\")) {\n \t\tif (!value)\n \t\t\tret = config_error_nonbool(var);\n \t\telse if (!me->overwrite && submodule->path)\n@@ -275,7 +260,7 @@ static int parse_config(const char *var, const char *value, void *data)\n \t\t\tsubmodule->path = xstrdup(value);\n \t\t\tcache_put_path(me->cache, submodule);\n \t\t}\n-\t} else if (!strcmp(item.buf, \"fetchrecursesubmodules\")) {\n+\t} else if (!strcmp(key, \"fetchrecursesubmodules\")) {\n \t\t/* when parsing worktree configurations we can die early */\n \t\tint die_on_error = is_null_sha1(me->gitmodules_sha1);\n \t\tif (!me->overwrite &&\n@@ -286,7 +271,7 @@ static int parse_config(const char *var, const char *value, void *data)\n \t\t\tsubmodule->fetch_recurse = parse_fetch_recurse(\n \t\t\t\t\t\t\t\tvar, value,\n \t\t\t\t\t\t\t\tdie_on_error);\n-\t} else if (!strcmp(item.buf, \"ignore\")) {\n+\t} else if (!strcmp(key, \"ignore\")) {\n \t\tif (!value)\n \t\t\tret = config_error_nonbool(var);\n \t\telse if (!me->overwrite && submodule->ignore)\n@@ -302,7 +287,7 @@ static int parse_config(const char *var, const char *value, void *data)\n \t\t\tfree((void *) submodule->ignore);\n \t\t\tsubmodule->ignore = xstrdup(value);\n \t\t}\n-\t} else if (!strcmp(item.buf, \"url\")) {\n+\t} else if (!strcmp(key, \"url\")) {\n \t\tif (!value) {\n \t\t\tret = config_error_nonbool(var);\n \t\t} else if (!me->overwrite && submodule->url) {\n@@ -312,7 +297,7 @@ static int parse_config(const char *var, const char *value, void *data)\n \t\t\tfree((void *) submodule->url);\n \t\t\tsubmodule->url = xstrdup(value);\n \t\t}\n-\t} else if (!strcmp(item.buf, \"update\")) {\n+\t} else if (!strcmp(key, \"update\")) {\n \t\tif (!value)\n \t\t\tret = config_error_nonbool(var);\n \t\telse if (!me->overwrite && submodule->update)\n@@ -324,9 +309,6 @@ static int parse_config(const char *var, const char *value, void *data)\n \t\t}\n \t}\n \n-\tstrbuf_release(&name);\n-\tstrbuf_release(&item);\n-\n \treturn ret;\n }\n \n-- \n2.7.0.rc0.41.gd102984.dirty\n"},{"id":"277476","messageId":"1454623776-3347-5-git-send-email-sbeller@google.com","threadId":"41333","inReplyTo":"1454623776-3347-1-git-send-email-sbeller@google.com","subject":"[PATCHv8 4/9] submodule-config: slightly simplify lookup_or_create_by_name","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-04T22:09:31Z","receivedAt":"2016-02-04T22:09:31Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"No need for a strbuf, when all we use it for, is duplicating a string.\n\nSigned-off-by: Stefan Beller \n---\n submodule-config.c | 5 +----\n 1 file changed, 1 insertion(+), 4 deletions(-)\n\ndiff --git a/submodule-config.c b/submodule-config.c\nindex c08ee7f..e375730 100644\n--- a/submodule-config.c\n+++ b/submodule-config.c\n@@ -165,7 +165,6 @@ static struct submodule *lookup_or_create_by_name(struct submodule_cache *cache,\n \t\tconst unsigned char *gitmodules_sha1, const char *name)\n {\n \tstruct submodule *submodule;\n-\tstruct strbuf name_buf = STRBUF_INIT;\n \n \tsubmodule = cache_lookup_name(cache, gitmodules_sha1, name);\n \tif (submodule)\n@@ -173,9 +172,7 @@ static struct submodule *lookup_or_create_by_name(struct submodule_cache *cache,\n \n \tsubmodule = xmalloc(sizeof(*submodule));\n \n-\tstrbuf_addstr(&name_buf, name);\n-\tsubmodule->name = strbuf_detach(&name_buf, NULL);\n-\n+\tsubmodule->name = xstrdup(name);\n \tsubmodule->path = NULL;\n \tsubmodule->url = NULL;\n \tsubmodule->update = NULL;\n-- \n2.7.0.rc0.41.gd102984.dirty\n"},{"id":"277472","messageId":"1454623776-3347-6-git-send-email-sbeller@google.com","threadId":"41333","inReplyTo":"1454623776-3347-1-git-send-email-sbeller@google.com","subject":"[PATCHv8 5/9] submodule-config: introduce parse_generic_submodule_config","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-04T22:09:32Z","receivedAt":"2016-02-04T22:09:32Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This rewrites parse_config to distinguish between configs specific to\none submodule and configs which apply generically to all submodules.\nWe do not have generic submodule configs yet, but the next patch will\nintroduce \"submodule.fetchJobs\".\n\nSigned-off-by: Stefan Beller \n---\n submodule-config.c | 45 +++++++++++++++++++++++++++++++++++----------\n 1 file changed, 35 insertions(+), 10 deletions(-)\n\ndiff --git a/submodule-config.c b/submodule-config.c\nindex e375730..2841c5e 100644\n--- a/submodule-config.c\n+++ b/submodule-config.c\n@@ -228,18 +228,22 @@ struct parse_config_parameter {\n \tint overwrite;\n };\n \n-static int parse_config(const char *var, const char *value, void *data)\n+static int parse_generic_submodule_config(const char *key,\n+\t\t\t\t\t const char *var,\n+\t\t\t\t\t const char *value,\n+\t\t\t\t\t struct parse_config_parameter *me)\n {\n-\tstruct parse_config_parameter *me = data;\n-\tstruct submodule *submodule;\n-\tint subsection_len, ret = 0;\n-\tconst char *subsection, *key;\n-\n-\tif (parse_config_key(var, \"submodule\", &subsection,\n-\t\t\t &subsection_len, &key) < 0 || !subsection)\n-\t\treturn 0;\n+\treturn 0;\n+}\n \n-\tsubsection = xmemdupz(subsection, subsection_len);\n+static int parse_specific_submodule_config(const char *subsection,\n+\t\t\t\t\t const char *key,\n+\t\t\t\t\t const char *var,\n+\t\t\t\t\t const char *value,\n+\t\t\t\t\t struct parse_config_parameter *me)\n+{\n+\tint ret = 0;\n+\tstruct submodule *submodule;\n \tsubmodule = lookup_or_create_by_name(me->cache,\n \t\t\t\t\t me->gitmodules_sha1,\n \t\t\t\t\t subsection);\n@@ -309,6 +313,27 @@ static int parse_config(const char *var, const char *value, void *data)\n \treturn ret;\n }\n \n+static int parse_config(const char *var, const char *value, void *data)\n+{\n+\tstruct parse_config_parameter *me = data;\n+\tint subsection_len, ret;\n+\tconst char *subsection, *key;\n+\tchar *sub;\n+\n+\tif (parse_config_key(var, \"submodule\", &subsection,\n+\t\t\t &subsection_len, &key) < 0)\n+\t\treturn 0;\n+\n+\tif (!subsection)\n+\t\treturn parse_generic_submodule_config(key, var, value, me);\n+\n+\tsub = xmemdupz(subsection, subsection_len);\n+\tret = parse_specific_submodule_config(sub, key,\n+\t\t\t\t\t var, value, me);\n+\tfree(sub);\n+\treturn ret;\n+}\n+\n static int gitmodule_sha1_from_commit(const unsigned char *commit_sha1,\n \t\t\t\t unsigned char *gitmodules_sha1)\n {\n-- \n2.7.0.rc0.41.gd102984.dirty\n"},{"id":"277469","messageId":"1454623776-3347-7-git-send-email-sbeller@google.com","threadId":"41333","inReplyTo":"1454623776-3347-1-git-send-email-sbeller@google.com","subject":"[PATCHv8 6/9] fetching submodules: respect `submodule.fetchJobs` config option","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-04T22:09:33Z","receivedAt":"2016-02-04T22:09:33Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This allows to configure fetching and updating in parallel\nwithout having the command line option.\n\nThis moved the responsibility to determine how many parallel processes\nto start from builtin/fetch to submodule.c as we need a way to communicate\n\"The user did not specify the number of parallel processes in the command\nline options\" in the builtin fetch. The submodule code takes care of\nthe precedence (CLI > config > default)\n\nSigned-off-by: Stefan Beller \n---\n Documentation/config.txt | 6 ++++++\n builtin/fetch.c | 2 +-\n submodule-config.c | 13 +++++++++++++\n submodule-config.h | 2 ++\n submodule.c | 5 +++++\n t/t5526-fetch-submodules.sh | 14 ++++++++++++++\n 6 files changed, 41 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 2d06b11..3b02732 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2646,6 +2646,12 @@ submodule..ignore::\n \t\"--ignore-submodules\" option. The 'git submodule' commands are not\n \taffected by this setting.\n \n+submodule.fetchJobs::\n+\tSpecifies how many submodules are fetched/cloned at the same time.\n+\tA positive integer allows up to that number of submodules fetched\n+\tin parallel. A value of 0 will give some reasonable default.\n+\tIf unset, it defaults to 1.\n+\n tag.sort::\n \tThis variable controls the sort ordering of tags when displayed by\n \tlinkgit:git-tag[1]. Without the \"--sort=\" option provided, the\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 586840d..5aa1c2d 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -37,7 +37,7 @@ static int prune = -1; /* unspecified */\n static int all, append, dry_run, force, keep, multiple, update_head_ok, verbosity;\n static int progress = -1, recurse_submodules = RECURSE_SUBMODULES_DEFAULT;\n static int tags = TAGS_DEFAULT, unshallow, update_shallow;\n-static int max_children = 1;\n+static int max_children = -1;\n static const char *depth;\n static const char *upload_pack;\n static struct strbuf default_rla = STRBUF_INIT;\ndiff --git a/submodule-config.c b/submodule-config.c\nindex 2841c5e..339b59d 100644\n--- a/submodule-config.c\n+++ b/submodule-config.c\n@@ -32,6 +32,7 @@ enum lookup_type {\n \n static struct submodule_cache cache;\n static int is_cache_init;\n+static unsigned long parallel_jobs = -1;\n \n static int config_path_cmp(const struct submodule_entry *a,\n \t\t\t const struct submodule_entry *b,\n@@ -233,6 +234,13 @@ static int parse_generic_submodule_config(const char *key,\n \t\t\t\t\t const char *value,\n \t\t\t\t\t struct parse_config_parameter *me)\n {\n+\tif (!strcmp(key, \"fetchjobs\")) {\n+\t\tif (!git_parse_ulong(value, ¶llel_jobs)) {\n+\t\t\twarning(_(\"Error parsing submodule.fetchJobs; Defaulting to 1.\"));\n+\t\t\tparallel_jobs = 1;\n+\t\t}\n+\t}\n+\n \treturn 0;\n }\n \n@@ -479,3 +487,8 @@ void submodule_free(void)\n \tcache_free(&cache);\n \tis_cache_init = 0;\n }\n+\n+unsigned long config_parallel_submodules(void)\n+{\n+\treturn parallel_jobs;\n+}\ndiff --git a/submodule-config.h b/submodule-config.h\nindex f9e2a29..bab1e8d 100644\n--- a/submodule-config.h\n+++ b/submodule-config.h\n@@ -27,4 +27,6 @@ const struct submodule *submodule_from_path(const unsigned char *commit_sha1,\n \t\tconst char *path);\n void submodule_free(void);\n \n+unsigned long config_parallel_submodules(void);\n+\n #endif /* SUBMODULE_CONFIG_H */\ndiff --git a/submodule.c b/submodule.c\nindex b83939c..eb7d54b 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -751,6 +751,11 @@ int fetch_populated_submodules(const struct argv_array *options,\n \targv_array_push(&spf.args, \"--recurse-submodules-default\");\n \t/* default value, \"--submodule-prefix\" and its value are added later */\n \n+\tif (max_parallel_jobs < 0)\n+\t\tmax_parallel_jobs = config_parallel_submodules();\n+\tif (max_parallel_jobs < 0)\n+\t\tmax_parallel_jobs = 1;\n+\n \tcalculate_changed_submodule_paths();\n \trun_processes_parallel(max_parallel_jobs,\n \t\t\t get_next_submodule,\ndiff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh\nindex 1241146..954d0e4 100755\n--- a/t/t5526-fetch-submodules.sh\n+++ b/t/t5526-fetch-submodules.sh\n@@ -471,4 +471,18 @@ test_expect_success \"don't fetch submodule when newly recorded commits are alrea\n \ttest_i18ncmp expect.err actual.err\n '\n \n+test_expect_success 'fetching submodules respects parallel settings' '\n+\tgit config fetch.recurseSubmodules true &&\n+\t(\n+\t\tcd downstream &&\n+\t\tGIT_TRACE=$(pwd)/trace.out git fetch --jobs 7 &&\n+\t\tgrep \"7 tasks\" trace.out &&\n+\t\tgit config submodule.fetchJobs 8 &&\n+\t\tGIT_TRACE=$(pwd)/trace.out git fetch &&\n+\t\tgrep \"8 tasks\" trace.out &&\n+\t\tGIT_TRACE=$(pwd)/trace.out git fetch --jobs 9 &&\n+\t\tgrep \"9 tasks\" trace.out\n+\t)\n+'\n+\n test_done\n-- \n2.7.0.rc0.41.gd102984.dirty\n"},{"id":"277473","messageId":"1454623776-3347-8-git-send-email-sbeller@google.com","threadId":"41333","inReplyTo":"1454623776-3347-1-git-send-email-sbeller@google.com","subject":"[PATCHv8 7/9] git submodule update: have a dedicated helper for cloning","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-04T22:09:34Z","receivedAt":"2016-02-04T22:09:34Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This introduces a new helper function in git submodule--helper\nwhich takes care of cloning all submodules, which we want to\nparallelize eventually.\n\nSome tests (such as empty URL, update_mode=none) are required in the\nhelper to make the decision for cloning. These checks have been\nmoved into the C function as well (no need to repeat them in the\nshell script).\n\nAs we can only access the stderr channel from within the parallel\nprocessing engine, we need to reroute the error message for\nspecified but initialized submodules to stderr. As it is an error\nmessage, this should have gone to stderr in the first place, so it\nis a bug fix along the way.\n\nSigned-off-by: Stefan Beller \nSigned-off-by: Junio C Hamano \n---\n builtin/submodule--helper.c | 236 ++++++++++++++++++++++++++++++++++++++++++++\n git-submodule.sh | 45 +++------\n t/t7400-submodule-basic.sh | 4 +-\n 3 files changed, 249 insertions(+), 36 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex f4c3eff..90f9dc6 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -255,6 +255,241 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \treturn 0;\n }\n \n+static int git_submodule_config(const char *var, const char *value, void *cb)\n+{\n+\treturn parse_submodule_config_option(var, value);\n+}\n+\n+struct submodule_update_clone {\n+\t/* states */\n+\tint count;\n+\tint print_unmatched;\n+\t/* configuration */\n+\tint quiet;\n+\tconst char *reference;\n+\tconst char *depth;\n+\tconst char *update;\n+\tconst char *recursive_prefix;\n+\tconst char *prefix;\n+\tstruct module_list list;\n+\tstruct string_list projectlines;\n+\tstruct pathspec pathspec;\n+};\n+#define SUBMODULE_UPDATE_CLONE_INIT {0, 0, 0, NULL, NULL, NULL, NULL, NULL, MODULE_LIST_INIT, STRING_LIST_INIT_DUP}\n+\n+static void fill_clone_command(struct child_process *cp, int quiet,\n+\t\t\t const char *prefix, const char *path,\n+\t\t\t const char *name, const char *url,\n+\t\t\t const char *reference, const char *depth)\n+{\n+\tcp->git_cmd = 1;\n+\tcp->no_stdin = 1;\n+\tcp->stdout_to_stderr = 1;\n+\tcp->err = -1;\n+\targv_array_push(&cp->args, \"submodule--helper\");\n+\targv_array_push(&cp->args, \"clone\");\n+\tif (quiet)\n+\t\targv_array_push(&cp->args, \"--quiet\");\n+\n+\tif (prefix)\n+\t\targv_array_pushl(&cp->args, \"--prefix\", prefix, NULL);\n+\n+\targv_array_pushl(&cp->args, \"--path\", path, NULL);\n+\targv_array_pushl(&cp->args, \"--name\", name, NULL);\n+\targv_array_pushl(&cp->args, \"--url\", url, NULL);\n+\tif (reference)\n+\t\targv_array_push(&cp->args, reference);\n+\tif (depth)\n+\t\targv_array_push(&cp->args, depth);\n+}\n+\n+static int update_clone_get_next_task(struct child_process *cp,\n+\t\t\t\t struct strbuf *err,\n+\t\t\t\t void *pp_cb,\n+\t\t\t\t void **pp_task_cb)\n+{\n+\tstruct submodule_update_clone *pp = pp_cb;\n+\n+\tfor (; pp->count < pp->list.nr; pp->count++) {\n+\t\tconst struct submodule *sub = NULL;\n+\t\tconst struct cache_entry *ce = pp->list.entries[pp->count];\n+\t\tstruct strbuf displaypath_sb = STRBUF_INIT;\n+\t\tstruct strbuf sb = STRBUF_INIT;\n+\t\tconst char *displaypath = NULL;\n+\t\tconst char *update_module = NULL;\n+\t\tchar *url = NULL;\n+\t\tint needs_cloning = 0;\n+\n+\t\tif (ce_stage(ce)) {\n+\t\t\tif (pp->recursive_prefix)\n+\t\t\t\tstrbuf_addf(err,\n+\t\t\t\t\t\"Skipping unmerged submodule %s/%s\\n\",\n+\t\t\t\t\tpp->recursive_prefix, ce->name);\n+\t\t\telse\n+\t\t\t\tstrbuf_addf(err,\n+\t\t\t\t\t\"Skipping unmerged submodule %s\\n\",\n+\t\t\t\t\tce->name);\n+\t\t\tgoto cleanup_and_continue;\n+\t\t}\n+\n+\t\tsub = submodule_from_path(null_sha1, ce->name);\n+\n+\t\tif (pp->recursive_prefix)\n+\t\t\tdisplaypath = relative_path(pp->recursive_prefix,\n+\t\t\t\t\t\t ce->name, &displaypath_sb);\n+\t\telse\n+\t\t\tdisplaypath = ce->name;\n+\n+\t\tif (pp->update)\n+\t\t\tupdate_module = pp->update;\n+\t\tif (!update_module)\n+\t\t\tupdate_module = sub->update;\n+\t\tif (!update_module)\n+\t\t\tupdate_module = \"checkout\";\n+\t\tif (!strcmp(update_module, \"none\")) {\n+\t\t\tstrbuf_addf(err, \"Skipping submodule '%s'\\n\",\n+\t\t\t\t displaypath);\n+\t\t\tgoto cleanup_and_continue;\n+\t\t}\n+\n+\t\t/*\n+\t\t * Looking up the url in .git/config.\n+\t\t * We must not fall back to .gitmodules as we only want\n+\t\t * to process configured submodules.\n+\t\t */\n+\t\tstrbuf_reset(&sb);\n+\t\tstrbuf_addf(&sb, \"submodule.%s.url\", sub->name);\n+\t\tgit_config_get_string(sb.buf, &url);\n+\t\tif (!url) {\n+\t\t\t/*\n+\t\t\t * Only mention uninitialized submodules when its\n+\t\t\t * path have been specified\n+\t\t\t */\n+\t\t\tif (pp->pathspec.nr)\n+\t\t\t\tstrbuf_addf(err,\n+\t\t\t\t\t _(\"Submodule path '%s' not initialized\\n\"\n+\t\t\t\t\t \"Maybe you want to use 'update --init'?\"),\n+\t\t\t\t\t displaypath);\n+\t\t\tgoto cleanup_and_continue;\n+\t\t}\n+\n+\t\tstrbuf_reset(&sb);\n+\t\tstrbuf_addf(&sb, \"%s/.git\", ce->name);\n+\t\tneeds_cloning = !file_exists(sb.buf);\n+\n+\t\tstrbuf_reset(&sb);\n+\t\tstrbuf_addf(&sb, \"%06o %s %d %d\\t%s\\n\", ce->ce_mode,\n+\t\t\t\tsha1_to_hex(ce->sha1), ce_stage(ce),\n+\t\t\t\tneeds_cloning, ce->name);\n+\t\tstring_list_append(&pp->projectlines, sb.buf);\n+\n+\t\tif (needs_cloning) {\n+\t\t\tfill_clone_command(cp, pp->quiet, pp->prefix,\n+\t\t\t\t\t ce->name, sub->name, strdup(url),\n+\t\t\t\t\t pp->reference, pp->depth);\n+\t\t\tpp->count++;\n+\t\t}\n+\n+cleanup_and_continue:\n+\t\tfree(url);\n+\t\tstrbuf_reset(&displaypath_sb);\n+\t\tstrbuf_reset(&sb);\n+\n+\t\tif (needs_cloning)\n+\t\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\n+\n+static int update_clone_start_failure(struct child_process *cp,\n+\t\t\t\t struct strbuf *err,\n+\t\t\t\t void *pp_cb,\n+\t\t\t\t void *pp_task_cb)\n+{\n+\tstruct submodule_update_clone *pp = pp_cb;\n+\n+\tstrbuf_addf(err, \"error when starting a child process\");\n+\tpp->print_unmatched = 1;\n+\n+\treturn 1;\n+}\n+\n+static int update_clone_task_finished(int result,\n+\t\t\t\t struct child_process *cp,\n+\t\t\t\t struct strbuf *err,\n+\t\t\t\t void *pp_cb,\n+\t\t\t\t void *pp_task_cb)\n+{\n+\tstruct submodule_update_clone *pp = pp_cb;\n+\n+\tif (!result) {\n+\t\treturn 0;\n+\t} else {\n+\t\tstrbuf_addf(err, \"error in one child process\");\n+\t\tpp->print_unmatched = 1;\n+\t\treturn 1;\n+\t}\n+}\n+\n+static int update_clone(int argc, const char **argv, const char *prefix)\n+{\n+\tstruct string_list_item *item;\n+\tstruct submodule_update_clone pp = SUBMODULE_UPDATE_CLONE_INIT;\n+\n+\tstruct option module_list_options[] = {\n+\t\tOPT_STRING(0, \"prefix\", &prefix,\n+\t\t\t N_(\"path\"),\n+\t\t\t N_(\"path into the working tree\")),\n+\t\tOPT_STRING(0, \"recursive_prefix\", &pp.recursive_prefix,\n+\t\t\t N_(\"path\"),\n+\t\t\t N_(\"path into the working tree, across nested \"\n+\t\t\t \"submodule boundaries\")),\n+\t\tOPT_STRING(0, \"update\", &pp.update,\n+\t\t\t N_(\"string\"),\n+\t\t\t N_(\"update command for submodules\")),\n+\t\tOPT_STRING(0, \"reference\", &pp.reference, \"\",\n+\t\t\t N_(\"Use the local reference repository \"\n+\t\t\t \"instead of a full clone\")),\n+\t\tOPT_STRING(0, \"depth\", &pp.depth, \"\",\n+\t\t\t N_(\"Create a shallow clone truncated to the \"\n+\t\t\t \"specified number of revisions\")),\n+\t\tOPT__QUIET(&pp.quiet, N_(\"do't print cloning progress\")),\n+\t\tOPT_END()\n+\t};\n+\n+\tconst char *const git_submodule_helper_usage[] = {\n+\t\tN_(\"git submodule--helper list [--prefix=] [...]\"),\n+\t\tNULL\n+\t};\n+\tpp.prefix = prefix;\n+\n+\targc = parse_options(argc, argv, prefix, module_list_options,\n+\t\t\t git_submodule_helper_usage, 0);\n+\n+\tif (module_list_compute(argc, argv, prefix, &pp.pathspec, &pp.list) < 0) {\n+\t\tprintf(\"#unmatched\\n\");\n+\t\treturn 1;\n+\t}\n+\n+\tgitmodules_config();\n+\t/* Overlay the parsed .gitmodules file with .git/config */\n+\tgit_config(git_submodule_config, NULL);\n+\trun_processes_parallel(1, update_clone_get_next_task,\n+\t\t\t\t update_clone_start_failure,\n+\t\t\t\t update_clone_task_finished,\n+\t\t\t\t &pp);\n+\n+\tif (pp.print_unmatched) {\n+\t\tprintf(\"#unmatched\\n\");\n+\t\treturn 1;\n+\t}\n+\n+\tfor_each_string_list_item(item, &pp.projectlines)\n+\t\tutf8_fprintf(stdout, \"%s\", item->string);\n+\n+\treturn 0;\n+}\n+\n struct cmd_struct {\n \tconst char *cmd;\n \tint (*fn)(int, const char **, const char *);\n@@ -264,6 +499,7 @@ static struct cmd_struct commands[] = {\n \t{\"list\", module_list},\n \t{\"name\", module_name},\n \t{\"clone\", module_clone},\n+\t{\"update-clone\", update_clone}\n };\n \n int cmd_submodule__helper(int argc, const char **argv, const char *prefix)\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 9bc5c5f..9f554fb 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -664,17 +664,18 @@ cmd_update()\n \t\tcmd_init \"--\" \"$@\" || return\n \tfi\n \n-\tcloned_modules=\n-\tgit submodule--helper list --prefix \"$wt_prefix\" \"$@\" | {\n+\tgit submodule--helper update-clone ${GIT_QUIET:+--quiet} \\\n+\t\t${wt_prefix:+--prefix \"$wt_prefix\"} \\\n+\t\t${prefix:+--recursive_prefix \"$prefix\"} \\\n+\t\t${update:+--update \"$update\"} \\\n+\t\t${reference:+--reference \"$reference\"} \\\n+\t\t${depth:+--depth \"$depth\"} \\\n+\t\t\"$@\" | {\n \terr=\n-\twhile read mode sha1 stage sm_path\n+\twhile read mode sha1 stage just_cloned sm_path\n \tdo\n \t\tdie_if_unmatched \"$mode\"\n-\t\tif test \"$stage\" = U\n-\t\tthen\n-\t\t\techo >&2 \"Skipping unmerged submodule $prefix$sm_path\"\n-\t\t\tcontinue\n-\t\tfi\n+\n \t\tname=$(git submodule--helper name \"$sm_path\") || exit\n \t\turl=$(git config submodule.\"$name\".url)\n \t\tbranch=$(get_submodule_config \"$name\" branch master)\n@@ -691,27 +692,10 @@ cmd_update()\n \n \t\tdisplaypath=$(relative_path \"$prefix$sm_path\")\n \n-\t\tif test \"$update_module\" = \"none\"\n-\t\tthen\n-\t\t\techo \"Skipping submodule '$displaypath'\"\n-\t\t\tcontinue\n-\t\tfi\n-\n-\t\tif test -z \"$url\"\n-\t\tthen\n-\t\t\t# Only mention uninitialized submodules when its\n-\t\t\t# path have been specified\n-\t\t\ttest \"$#\" != \"0\" &&\n-\t\t\tsay \"$(eval_gettext \"Submodule path '\\$displaypath' not initialized\n-Maybe you want to use 'update --init'?\")\"\n-\t\t\tcontinue\n-\t\tfi\n-\n-\t\tif ! test -d \"$sm_path\"/.git && ! test -f \"$sm_path\"/.git\n+\t\tif test $just_cloned -eq 1\n \t\tthen\n-\t\t\tgit submodule--helper clone ${GIT_QUIET:+--quiet} --prefix \"$prefix\" --path \"$sm_path\" --name \"$name\" --url \"$url\" \"$reference\" \"$depth\" || exit\n-\t\t\tcloned_modules=\"$cloned_modules;$name\"\n \t\t\tsubsha1=\n+\t\t\tupdate_module=checkout\n \t\telse\n \t\t\tsubsha1=$(clear_local_git_env; cd \"$sm_path\" &&\n \t\t\t\tgit rev-parse --verify HEAD) ||\n@@ -751,13 +735,6 @@ Maybe you want to use 'update --init'?\")\"\n \t\t\t\tdie \"$(eval_gettext \"Unable to fetch in submodule path '\\$displaypath'\")\"\n \t\t\tfi\n \n-\t\t\t# Is this something we just cloned?\n-\t\t\tcase \";$cloned_modules;\" in\n-\t\t\t*\";$name;\"*)\n-\t\t\t\t# then there is no local change to integrate\n-\t\t\t\tupdate_module=checkout ;;\n-\t\t\tesac\n-\n \t\t\tmust_die_on_failure=\n \t\t\tcase \"$update_module\" in\n \t\t\tcheckout)\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex 540771c..5991e3c 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -462,7 +462,7 @@ test_expect_success 'update --init' '\n \tgit config --remove-section submodule.example &&\n \ttest_must_fail git config submodule.example.url &&\n \n-\tgit submodule update init > update.out &&\n+\tgit submodule update init 2> update.out &&\n \tcat update.out &&\n \ttest_i18ngrep \"not initialized\" update.out &&\n \ttest_must_fail git rev-parse --resolve-git-dir init/.git &&\n@@ -480,7 +480,7 @@ test_expect_success 'update --init from subdirectory' '\n \tmkdir -p sub &&\n \t(\n \t\tcd sub &&\n-\t\tgit submodule update ../init >update.out &&\n+\t\tgit submodule update ../init 2>update.out &&\n \t\tcat update.out &&\n \t\ttest_i18ngrep \"not initialized\" update.out &&\n \t\ttest_must_fail git rev-parse --resolve-git-dir ../init/.git &&\n-- \n2.7.0.rc0.41.gd102984.dirty\n"},{"id":"277471","messageId":"1454623776-3347-9-git-send-email-sbeller@google.com","threadId":"41333","inReplyTo":"1454623776-3347-1-git-send-email-sbeller@google.com","subject":"[PATCHv8 8/9] submodule update: expose parallelism to the user","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-04T22:09:35Z","receivedAt":"2016-02-04T22:09:35Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Expose possible parallelism either via the \"--jobs\" CLI parameter or\nthe \"submodule.fetchJobs\" setting.\n\nBy having the variable initialized to -1, we make sure 0 can be passed\ninto the parallel processing machine, which will then pick as many parallel\nworkers as there are CPUs.\n\nSigned-off-by: Stefan Beller \n---\n Documentation/git-submodule.txt | 7 ++++++-\n builtin/submodule--helper.c | 18 ++++++++++++++----\n git-submodule.sh | 9 +++++++++\n t/t7406-submodule-update.sh | 12 ++++++++++++\n 4 files changed, 41 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt\nindex 1572f05..13adebf 100644\n--- a/Documentation/git-submodule.txt\n+++ b/Documentation/git-submodule.txt\n@@ -16,7 +16,7 @@ SYNOPSIS\n 'git submodule' [--quiet] deinit [-f|--force] [--] ...\n 'git submodule' [--quiet] update [--init] [--remote] [-N|--no-fetch]\n \t [-f|--force] [--rebase|--merge] [--reference ]\n-\t [--depth ] [--recursive] [--] [...]\n+\t [--depth ] [--recursive] [--jobs ] [--] [...]\n 'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) ]\n \t [commit] [--] [...]\n 'git submodule' [--quiet] foreach [--recursive] \n@@ -377,6 +377,11 @@ for linkgit:git-clone[1]'s `--reference` and `--shared` options carefully.\n \tclone with a history truncated to the specified number of revisions.\n \tSee linkgit:git-clone[1]\n \n+-j ::\n+--jobs ::\n+\tThis option is only valid for the update command.\n+\tClone new submodules in parallel with as many jobs.\n+\tDefaults to the `submodule.fetchJobs` option.\n \n ...::\n \tPaths to submodule(s). When specified this will restrict the command\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 90f9dc6..7e01b85 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -433,6 +433,7 @@ static int update_clone_task_finished(int result,\n \n static int update_clone(int argc, const char **argv, const char *prefix)\n {\n+\tint max_jobs = -1;\n \tstruct string_list_item *item;\n \tstruct submodule_update_clone pp = SUBMODULE_UPDATE_CLONE_INIT;\n \n@@ -453,6 +454,8 @@ static int update_clone(int argc, const char **argv, const char *prefix)\n \t\tOPT_STRING(0, \"depth\", &pp.depth, \"\",\n \t\t\t N_(\"Create a shallow clone truncated to the \"\n \t\t\t \"specified number of revisions\")),\n+\t\tOPT_INTEGER('j', \"jobs\", &max_jobs,\n+\t\t\t N_(\"parallel jobs\")),\n \t\tOPT__QUIET(&pp.quiet, N_(\"do't print cloning progress\")),\n \t\tOPT_END()\n \t};\n@@ -474,10 +477,17 @@ static int update_clone(int argc, const char **argv, const char *prefix)\n \tgitmodules_config();\n \t/* Overlay the parsed .gitmodules file with .git/config */\n \tgit_config(git_submodule_config, NULL);\n-\trun_processes_parallel(1, update_clone_get_next_task,\n-\t\t\t\t update_clone_start_failure,\n-\t\t\t\t update_clone_task_finished,\n-\t\t\t\t &pp);\n+\n+\tif (max_jobs < 0)\n+\t\tmax_jobs = config_parallel_submodules();\n+\tif (max_jobs < 0)\n+\t\tmax_jobs = 1;\n+\n+\trun_processes_parallel(max_jobs,\n+\t\t\t update_clone_get_next_task,\n+\t\t\t update_clone_start_failure,\n+\t\t\t update_clone_task_finished,\n+\t\t\t &pp);\n \n \tif (pp.print_unmatched) {\n \t\tprintf(\"#unmatched\\n\");\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 9f554fb..10c5af9 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -645,6 +645,14 @@ cmd_update()\n \t\t--depth=*)\n \t\t\tdepth=$1\n \t\t\t;;\n+\t\t-j|--jobs)\n+\t\t\tcase \"$2\" in '') usage ;; esac\n+\t\t\tjobs=\"--jobs=$2\"\n+\t\t\tshift\n+\t\t\t;;\n+\t\t--jobs=*)\n+\t\t\tjobs=$1\n+\t\t\t;;\n \t\t--)\n \t\t\tshift\n \t\t\tbreak\n@@ -670,6 +678,7 @@ cmd_update()\n \t\t${update:+--update \"$update\"} \\\n \t\t${reference:+--reference \"$reference\"} \\\n \t\t${depth:+--depth \"$depth\"} \\\n+\t\t${jobs:+$jobs} \\\n \t\t\"$@\" | {\n \terr=\n \twhile read mode sha1 stage just_cloned sm_path\ndiff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh\nindex dda3929..7fd5142 100755\n--- a/t/t7406-submodule-update.sh\n+++ b/t/t7406-submodule-update.sh\n@@ -774,4 +774,16 @@ test_expect_success 'submodule update --recursive drops module name before recur\n \t test_i18ngrep \"Submodule path .deeper/submodule/subsubmodule.: checked out\" actual\n \t)\n '\n+\n+test_expect_success 'submodule update can be run in parallel' '\n+\t(cd super2 &&\n+\t GIT_TRACE=$(pwd)/trace.out git submodule update --jobs 7 &&\n+\t grep \"7 tasks\" trace.out &&\n+\t git config submodule.fetchJobs 8 &&\n+\t GIT_TRACE=$(pwd)/trace.out git submodule update &&\n+\t grep \"8 tasks\" trace.out &&\n+\t GIT_TRACE=$(pwd)/trace.out git submodule update --jobs 9 &&\n+\t grep \"9 tasks\" trace.out\n+\t)\n+'\n test_done\n-- \n2.7.0.rc0.41.gd102984.dirty\n"},{"id":"277470","messageId":"1454623776-3347-10-git-send-email-sbeller@google.com","threadId":"41333","inReplyTo":"1454623776-3347-1-git-send-email-sbeller@google.com","subject":"[PATCHv8 9/9] clone: allow an explicit argument for parallel submodule clones","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-04T22:09:36Z","receivedAt":"2016-02-04T22:09:36Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Just pass it along to \"git submodule update\", which may pick reasonable\ndefaults if you don't specify an explicit number.\n\nSigned-off-by: Stefan Beller \n---\n Documentation/git-clone.txt | 6 +++++-\n builtin/clone.c | 19 +++++++++++++------\n t/t7406-submodule-update.sh | 15 +++++++++++++++\n 3 files changed, 33 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\nindex 6bf000d..6db7b6d 100644\n--- a/Documentation/git-clone.txt\n+++ b/Documentation/git-clone.txt\n@@ -14,7 +14,7 @@ SYNOPSIS\n \t [-o ] [-b ] [-u ] [--reference ]\n \t [--dissociate] [--separate-git-dir ]\n \t [--depth ] [--[no-]single-branch]\n-\t [--recursive | --recurse-submodules] [--] \n+\t [--recursive | --recurse-submodules] [--jobs ] [--] \n \t []\n \n DESCRIPTION\n@@ -221,6 +221,10 @@ objects from the source repository into a pack in the cloned repository.\n \tThe result is Git repository can be separated from working\n \ttree.\n \n+-j ::\n+--jobs ::\n+\tThe number of submodules fetched at the same time.\n+\tDefaults to the `submodule.fetchJobs` option.\n \n ::\n \tThe (possibly remote) repository to clone from. See the\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex a0b3cd9..b004fb4 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -50,6 +50,7 @@ static int option_progress = -1;\n 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 \n static struct option builtin_clone_options[] = {\n \tOPT__VERBOSITY(&option_verbosity),\n@@ -72,6 +73,8 @@ static struct option builtin_clone_options[] = {\n \t\t N_(\"initialize submodules in the clone\")),\n \tOPT_BOOL(0, \"recurse-submodules\", &option_recursive,\n \t\t N_(\"initialize submodules in the clone\")),\n+\tOPT_INTEGER('j', \"jobs\", &max_jobs,\n+\t\t N_(\"number of submodules cloned in parallel\")),\n \tOPT_STRING(0, \"template\", &option_template, N_(\"template-directory\"),\n \t\t N_(\"directory from which templates will be used\")),\n \tOPT_STRING_LIST(0, \"reference\", &option_reference, N_(\"repo\"),\n@@ -95,10 +98,6 @@ static struct option builtin_clone_options[] = {\n \tOPT_END()\n };\n \n-static const char *argv_submodule[] = {\n-\t\"submodule\", \"update\", \"--init\", \"--recursive\", NULL\n-};\n-\n static const char *get_repo_path_1(struct strbuf *path, int *is_bundle)\n {\n \tstatic char *suffix[] = { \"/.git\", \"\", \".git/.git\", \".git\" };\n@@ -724,8 +723,16 @@ 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-\t\terr = run_command_v_opt(argv_submodule, RUN_GIT_CMD);\n+\tif (!err && option_recursive) {\n+\t\tstruct argv_array args = ARGV_ARRAY_INIT;\n+\t\targv_array_pushl(&args, \"submodule\", \"update\", \"--init\", \"--recursive\", NULL);\n+\n+\t\tif (max_jobs != -1)\n+\t\t\targv_array_pushf(&args, \"--jobs=%d\", max_jobs);\n+\n+\t\terr = run_command_v_opt(args.argv, RUN_GIT_CMD);\n+\t\targv_array_clear(&args);\n+\t}\n \n \treturn err;\n }\ndiff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh\nindex 7fd5142..090891e 100755\n--- a/t/t7406-submodule-update.sh\n+++ b/t/t7406-submodule-update.sh\n@@ -786,4 +786,19 @@ test_expect_success 'submodule update can be run in parallel' '\n \t grep \"9 tasks\" trace.out\n \t)\n '\n+\n+test_expect_success 'git clone passes the parallel jobs config on to submodules' '\n+\ttest_when_finished \"rm -rf super4\" &&\n+\tGIT_TRACE=$(pwd)/trace.out git clone --recurse-submodules --jobs 7 . super4 &&\n+\tgrep \"7 tasks\" trace.out &&\n+\trm -rf super4 &&\n+\tgit config --global submodule.fetchJobs 8 &&\n+\tGIT_TRACE=$(pwd)/trace.out git clone --recurse-submodules . super4 &&\n+\tgrep \"8 tasks\" trace.out &&\n+\trm -rf super4 &&\n+\tGIT_TRACE=$(pwd)/trace.out git clone --recurse-submodules --jobs 9 . super4 &&\n+\tgrep \"9 tasks\" trace.out &&\n+\trm -rf super4\n+'\n+\n test_done\n-- \n2.7.0.rc0.41.gd102984.dirty\n"},{"id":"277500","messageId":"20160205005946.GA19501@google.com","threadId":"41333","inReplyTo":"1454623776-3347-2-git-send-email-sbeller@google.com","subject":"Re: [PATCHv8 1/9] submodule-config: keep update strategy around","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2016-02-05T00:59:46Z","receivedAt":"2016-02-05T00:59:46Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nIt's been a while since I looked at this series. Hopefully I can\ncome at it with some fresh eyes. Thanks for your perseverance.\n\nStefan Beller wrote:\n\n> We need the submodule update strategies in a later patch.\n\nThis description doesn't explain what the patch will do or why\nparse_config didn't already retain the value. If I look back\nat this patch later and want to understand why it was written,\nwhat would I want to know?\n\nIt could say\n\n\tCurrently submodule..update is only handled by git-submodule.sh.\n\tC code will start to need to make use of that value as more of the\n\tfunctionality of git-submodule.sh moves into library code in C.\n\n\tAdd the update field to 'struct submodule' and populate it so it can\n\tbe read as sm->update.\n\n[...]\n> +++ b/submodule-config.c\n> @@ -311,6 +312,16 @@ static int parse_config(const char *var, const char *value, void *data)\n> \t\t\tfree((void *) submodule->url);\n> \t\t\tsubmodule->url = xstrdup(value);\n> \t\t}\n> +\t} else if (!strcmp(item.buf, \"update\")) {\n> +\t\tif (!value)\n> +\t\t\tret = config_error_nonbool(var);\n> +\t\telse if (!me->overwrite && submodule->update != NULL)\n> +\t\t\twarn_multiple_config(me->commit_sha1, submodule->name,\n> +\t\t\t\t\t \"update\");\n> +\t\telse {\n> +\t\t\tfree((void *) submodule->update);\n> +\t\t\tsubmodule->update = xstrdup(value);\n> +\t\t}\n\n(not about this patch) This code is repetitive. Would there be a way\nto share code between the parsing of different per-submodule settings?\n\n[...]\n> --- a/submodule-config.h\n> +++ b/submodule-config.h\n> @@ -14,6 +14,7 @@ struct submodule {\n> \tconst char *url;\n> \tint fetch_recurse;\n> \tconst char *ignore;\n> +\tconst char *update;\n\ngitmodules(5) tells me the only allowed values are checkout, rebase,\nmerge, and none. I wouldn't know at a glance how to match against\nthose in calling code. Can this be an enum?\n\nThanks,\nJonathan\n"},{"id":"277501","messageId":"20160205010509.GB19501@google.com","threadId":"41333","inReplyTo":"1454623776-3347-3-git-send-email-sbeller@google.com","subject":"Re: [PATCHv8 2/9] submodule-config: drop check against NULL","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2016-02-05T01:05:09Z","receivedAt":"2016-02-05T01:05:09Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Stefan Beller wrote:\n\n> Adhere to the common coding style of Git and not check explicitly\n> for NULL throughout the file. There are still other occurrences in the\n> code base but that is usually inside of conditions with side effects.\n>\n> Signed-off-by: Stefan Beller \n> ---\n> submodule-config.c | 8 ++++----\n> 1 file changed, 4 insertions(+), 4 deletions(-)\n\nThanks.\nReviewed-by: Jonathan Nieder \n"},{"id":"277502","messageId":"xmqqa8nfeswj.fsf@gitster.mtv.corp.google.com","threadId":"41333","inReplyTo":"1454623776-3347-7-git-send-email-sbeller@google.com","subject":"Re: [PATCHv8 6/9] fetching submodules: respect `submodule.fetchJobs` config option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-02-05T03:29:48Z","receivedAt":"2016-02-05T03:29:48Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> diff --git a/builtin/fetch.c b/builtin/fetch.c\n> index 586840d..5aa1c2d 100644\n> --- a/builtin/fetch.c\n> +++ b/builtin/fetch.c\n> @@ -37,7 +37,7 @@ static int prune = -1; /* unspecified */\n> static int all, append, dry_run, force, keep, multiple, update_head_ok, verbosity;\n> static int progress = -1, recurse_submodules = RECURSE_SUBMODULES_DEFAULT;\n> static int tags = TAGS_DEFAULT, unshallow, update_shallow;\n> -static int max_children = 1;\n> +static int max_children = -1;\n\nThis makes sense as you would later be doing \"If left unspecified,\ni.e. -1, then fall back to blah\" ...\n\ng> diff --git a/submodule-config.c b/submodule-config.c\n> index 2841c5e..339b59d 100644\n> --- a/submodule-config.c\n> +++ b/submodule-config.c\n> @@ -32,6 +32,7 @@ enum lookup_type {\n> \n> static struct submodule_cache cache;\n> static int is_cache_init;\n> +static unsigned long parallel_jobs = -1;\n\n... but I do not think this does. For one thing, you would not be\ndoing \"If parallel_jobs < -1, then that is unspecified yet\" for the\nunsigned long variable, and for another, I do not think you would be\nbehaving differently for the first time (i.e. \"unspecified yet\" case).\n\nI think you want to initialize this to 1, if your \"not configured at\nall\" default is supposed to be 1.\n\n> @@ -233,6 +234,13 @@ static int parse_generic_submodule_config(const char *key,\n> \t\t\t\t\t const char *value,\n> \t\t\t\t\t struct parse_config_parameter *me)\n> {\n> +\tif (!strcmp(key, \"fetchjobs\")) {\n> +\t\tif (!git_parse_ulong(value, ¶llel_jobs)) {\n> +\t\t\twarning(_(\"Error parsing submodule.fetchJobs; Defaulting to 1.\"));\n> +\t\t\tparallel_jobs = 1;\n\nHmph, this is not a die() because...? Not a rhetorical question.\n\n> +unsigned long config_parallel_submodules(void)\n> +{\n> +\treturn parallel_jobs;\n> +}\n\nIt is not a crime to make this return \"int\", as the code that\neventually uses it will assign it to a variable that will be\npassed to run_processes_parallel() that takes an \"int\".\n\nIn fact, returning \"int\" would be preferrable. You are causing\ntruncation somewhere in the callchain anyway. If the truncation\nbothers you, this function or immediately after calling\ngit_parse_ulong() would be a good place to do a range check. The\nvariable parallel_jobs has to stay \"unsigned long\" in any case as\nyou are calling git_parse_ulong().\n\n> diff --git a/submodule.c b/submodule.c\n> index b83939c..eb7d54b 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n> @@ -751,6 +751,11 @@ int fetch_populated_submodules(const struct argv_array *options,\n> \targv_array_push(&spf.args, \"--recurse-submodules-default\");\n> \t/* default value, \"--submodule-prefix\" and its value are added later */\n> \n> +\tif (max_parallel_jobs < 0)\n> +\t\tmax_parallel_jobs = config_parallel_submodules();\n> +\tif (max_parallel_jobs < 0)\n> +\t\tmax_parallel_jobs = 1;\n> +\n> \tcalculate_changed_submodule_paths();\n> \trun_processes_parallel(max_parallel_jobs,\n> \t\t\t get_next_submodule,\n"},{"id":"277557","messageId":"CAGZ79kaamt=7mLv23JyJiWL9cJ8deV4JWH_Kv0Csovvqs5SBcg@mail.gmail.com","threadId":"41333","inReplyTo":"xmqqa8nfeswj.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCHv8 6/9] fetching submodules: respect `submodule.fetchJobs` config option","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-05T18:50:36Z","receivedAt":"2016-02-05T18:50:36Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Feb 4, 2016 at 7:29 PM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>\n>> diff --git a/builtin/fetch.c b/builtin/fetch.c\n>> index 586840d..5aa1c2d 100644\n>> --- a/builtin/fetch.c\n>> +++ b/builtin/fetch.c\n>> @@ -37,7 +37,7 @@ static int prune = -1; /* unspecified */\n>> static int all, append, dry_run, force, keep, multiple, update_head_ok, verbosity;\n>> static int progress = -1, recurse_submodules = RECURSE_SUBMODULES_DEFAULT;\n>> static int tags = TAGS_DEFAULT, unshallow, update_shallow;\n>> -static int max_children = 1;\n>> +static int max_children = -1;\n>\n> This makes sense as you would later be doing \"If left unspecified,\n> i.e. -1, then fall back to blah\" ...\n\nRight,\nmax_children is passed into fetch_populated_submodules, which\nshould handle that....\n\n>\n> g> diff --git a/submodule-config.c b/submodule-config.c\n>> index 2841c5e..339b59d 100644\n>> --- a/submodule-config.c\n>> +++ b/submodule-config.c\n>> @@ -32,6 +32,7 @@ enum lookup_type {\n>>\n>> static struct submodule_cache cache;\n>> static int is_cache_init;\n>> +static unsigned long parallel_jobs = -1;\n>\n> ... but I do not think this does. For one thing, you would not be\n> doing \"If parallel_jobs < -1, then that is unspecified yet\" for the\n> unsigned long variable, and for another, I do not think you would be\n> behaving differently for the first time (i.e. \"unspecified yet\" case).\n>\n> I think you want to initialize this to 1, if your \"not configured at\n> all\" default is supposed to be 1.\n\nPlease read on in the patch, such that you find\n\n@@ -751,6 +751,11 @@ int fetch_populated_submodules(\n(This is where the max_children from builtin/fetch is passed into,\nnamed max_parallel_jobs now)\n\n+ if (max_parallel_jobs < 0)\n+ max_parallel_jobs = config_parallel_submodules();\n+ if (max_parallel_jobs < 0)\n+ max_parallel_jobs = 1;\n+\n\nSo if we don't get the config option from builtin/fetch, we ask for\nconfig_parallel_submodules(), which is what you were seeing above\ninitialized as -1, too. And if that is still -1, we default to 1 there.\n\nSo from a design perspective, you're rather saying we want to move part of\nthis logic into submodule-config.c, such that\nconfig_parallel_submodules never returns -1,\nbut either 1 as the default or the actual configuration?\nThen the code in fetch_populated_submodules, would be as simple as\n\n if (max_parallel_jobs < 0)\n max_parallel_jobs = config_parallel_submodules();\n // done , as config_parallel_submodules(); gives the last\nauthoritive answer.\n\nWell looking at submodule-config.c this might be easier, too.\n\nOk.\n\n>\n>> @@ -233,6 +234,13 @@ static int parse_generic_submodule_config(const char *key,\n>> const char *value,\n>> struct parse_config_parameter *me)\n>> {\n>> + if (!strcmp(key, \"fetchjobs\")) {\n>> + if (!git_parse_ulong(value, ¶llel_jobs)) {\n>> + warning(_(\"Error parsing submodule.fetchJobs; Defaulting to 1.\"));\n>> + parallel_jobs = 1;\n>\n> Hmph, this is not a die() because...? Not a rhetorical question.\n\nBecause this config option doesn't alter the state of the repository.\nAfter the fact you should not be able to tell how much parallel operations\nwere going on.\n\n(As opposed to other options which alter the state of the repository)\n\nI'll change it to die(...), though it sounds overly strict to me in this case.\nBut I guess consistency beats overstrictness here.\n\n>\n>> +unsigned long config_parallel_submodules(void)\n>> +{\n>> + return parallel_jobs;\n>> +}\n>\n> It is not a crime to make this return \"int\", as the code that\n> eventually uses it will assign it to a variable that will be\n> passed to run_processes_parallel() that takes an \"int\".\n\nok\n\n>\n> In fact, returning \"int\" would be preferrable. You are causing\n> truncation somewhere in the callchain anyway. If the truncation\n> bothers you, this function or immediately after calling\n> git_parse_ulong() would be a good place to do a range check. The\n> variable parallel_jobs has to stay \"unsigned long\" in any case as\n> you are calling git_parse_ulong().\n\nok, that should be the best place.\n\n>\n>> diff --git a/submodule.c b/submodule.c\n>> index b83939c..eb7d54b 100644\n>> --- a/submodule.c\n>> +++ b/submodule.c\n>> @@ -751,6 +751,11 @@ int fetch_populated_submodules(const struct argv_array *options,\n>> argv_array_push(&spf.args, \"--recurse-submodules-default\");\n>> /* default value, \"--submodule-prefix\" and its value are added later */\n>>\n>> + if (max_parallel_jobs < 0)\n>> + max_parallel_jobs = config_parallel_submodules();\n>> + if (max_parallel_jobs < 0)\n>> + max_parallel_jobs = 1;\n>> +\n>> calculate_changed_submodule_paths();\n>> run_processes_parallel(max_parallel_jobs,\n>> get_next_submodule,\n"},{"id":"277588","messageId":"xmqqegcrc4j7.fsf@gitster.mtv.corp.google.com","threadId":"41333","inReplyTo":"CAGZ79kaamt=7mLv23JyJiWL9cJ8deV4JWH_Kv0Csovvqs5SBcg@mail.gmail.com","subject":"Re: [PATCHv8 6/9] fetching submodules: respect `submodule.fetchJobs` config option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-02-05T19:59:08Z","receivedAt":"2016-02-05T19:59:08Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> On Thu, Feb 4, 2016 at 7:29 PM, Junio C Hamano wrote:\n>> Stefan Beller writes:\n>> ...\n>>> +static unsigned long parallel_jobs = -1;\n>>\n>> ... but I do not think this does....\n>\n> So if we don't get the config option from builtin/fetch, we ask for\n> config_parallel_submodules(), which is what you were seeing above\n> initialized as -1, too. And if that is still -1, we default to 1 there.\n\nBut that is not really -1, but -1 casted to unsigned long that is\ncasted back to int. \n\n> So from a design perspective, you're rather saying we want to move part of\n> this logic into submodule-config.c, such that\n> config_parallel_submodules never returns -1,\n> but either 1 as the default or the actual configuration?\n\nThat is not my design but yours ;-)\n\nExpecting config_parallel_submodules() to return -1 when there is no\nvariable defined contradicts what you wrote in the documentation, I\nthink, where it says \"this variable defaults to 1 when not set\".\n\n> Then the code in fetch_populated_submodules, would be as simple as\n>\n> if (max_parallel_jobs < 0)\n> max_parallel_jobs = config_parallel_submodules();\n\nYup.\n\n>>> @@ -233,6 +234,13 @@ static int parse_generic_submodule_config(const char *key,\n>>> const char *value,\n>>> struct parse_config_parameter *me)\n>>> {\n>>> + if (!strcmp(key, \"fetchjobs\")) {\n>>> + if (!git_parse_ulong(value, ¶llel_jobs)) {\n>>> + warning(_(\"Error parsing submodule.fetchJobs; Defaulting to 1.\"));\n>>> + parallel_jobs = 1;\n>>\n>> Hmph, this is not a die() because...? Not a rhetorical question.\n>\n> Because this config option doesn't alter the state of the repository.\n> After the fact you should not be able to tell how much parallel operations\n> were going on.\n>\n> (As opposed to other options which alter the state of the repository)\n>\n> I'll change it to die(...), though it sounds overly strict to me in this case.\n> But I guess consistency beats overstrictness here.\n\nI do not see it as being overly strict, though.\n\nIf I were a user of this feature, I'd rather see a problem pointed\nout to me (e.g \"you spelled 'true' but that fetchJobs expects a\npositive integer\") to help me fix it, rather than having to see this\nwarning every time I try to run submodule commands.\n\nWhat benefit does a user get by being loose here? Probably I am\nnot considering some valid use cases...\n\n>>> +unsigned long config_parallel_submodules(void)\n>>> +{\n>>> + return parallel_jobs;\n>>> +}\n>>\n>> It is not a crime to make this return \"int\", as the code that\n>> eventually uses it will assign it to a variable that will be\n>> passed to run_processes_parallel() that takes an \"int\".\n>\n> ok\n>\n>>\n>> In fact, returning \"int\" would be preferrable. You are causing\n>> truncation somewhere in the callchain anyway. If the truncation\n>> bothers you, this function or immediately after calling\n>> git_parse_ulong() would be a good place to do a range check. The\n>> variable parallel_jobs has to stay \"unsigned long\" in any case as\n>> you are calling git_parse_ulong().\n>\n> ok, that should be the best place.\n\nAlternatively (I haven't weighed pros and cons myself), you can make\nparallel_jobs an \"int\", call git_parse_ulong() using a temporary\n\"unsigned long\" and after doing range check assign it to\nparallel_jobs. That would make this function really trivial ;-)\n"},{"id":"277589","messageId":"CAGZ79kYfxYAZif4P0GRdy5LQVXuHAyr3VtwEtX7Uu0sDyD4yoQ@mail.gmail.com","threadId":"41333","inReplyTo":"20160205005946.GA19501@google.com","subject":"Re: [PATCHv8 1/9] submodule-config: keep update strategy around","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-05T20:25:38Z","receivedAt":"2016-02-05T20:25:38Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Feb 4, 2016 at 4:59 PM, Jonathan Nieder wrote:\n> Hi,\n>\n> It's been a while since I looked at this series. Hopefully I can\n> come at it with some fresh eyes. Thanks for your perseverance.\n>\n> Stefan Beller wrote:\n>\n>> We need the submodule update strategies in a later patch.\n>\n> This description doesn't explain what the patch will do or why\n> parse_config didn't already retain the value. If I look back\n> at this patch later and want to understand why it was written,\n> what would I want to know?\n>\n> It could say\n>\n> Currently submodule..update is only handled by git-submodule.sh.\n> C code will start to need to make use of that value as more of the\n> functionality of git-submodule.sh moves into library code in C.\n>\n> Add the update field to 'struct submodule' and populate it so it can\n> be read as sm->update.\n\n\nok\n\n>\n> [...]\n>> +++ b/submodule-config.c\n>> @@ -311,6 +312,16 @@ static int parse_config(const char *var, const char *value, void *data)\n>> free((void *) submodule->url);\n>> submodule->url = xstrdup(value);\n>> }\n>> + } else if (!strcmp(item.buf, \"update\")) {\n>> + if (!value)\n>> + ret = config_error_nonbool(var);\n>> + else if (!me->overwrite && submodule->update != NULL)\n>> + warn_multiple_config(me->commit_sha1, submodule->name,\n>> + \"update\");\n>> + else {\n>> + free((void *) submodule->update);\n>> + submodule->update = xstrdup(value);\n>> + }\n>\n> (not about this patch) This code is repetitive. Would there be a way\n> to share code between the parsing of different per-submodule settings?\n>\n> [...]\n>> --- a/submodule-config.h\n>> +++ b/submodule-config.h\n>> @@ -14,6 +14,7 @@ struct submodule {\n>> const char *url;\n>> int fetch_recurse;\n>> const char *ignore;\n>> + const char *update;\n>\n> gitmodules(5) tells me the only allowed values are checkout, rebase,\n> merge, and none. I wouldn't know at a glance how to match against\n> those in calling code. Can this be an enum?\n\n\"Note that the !command form is intentionally ignored here for\nsecurity reasons.\"\n\nHowever you can overwrite the update field in .git/config to be \"! [foo]\",\nwhich we also need to support, so let's keep it a string for now?\n\n>\n> Thanks,\n> Jonathan\n"},{"id":"277590","messageId":"20160205203309.GA28749@google.com","threadId":"41333","inReplyTo":"CAGZ79kYfxYAZif4P0GRdy5LQVXuHAyr3VtwEtX7Uu0sDyD4yoQ@mail.gmail.com","subject":"Re: [PATCHv8 1/9] submodule-config: keep update strategy around","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2016-02-05T20:33:09Z","receivedAt":"2016-02-05T20:33:09Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Stefan Beller wrote:\n> On Thu, Feb 4, 2016 at 4:59 PM, Jonathan Nieder wrote:\n>> Stefan Beller wrote:\n\n>>> +++ b/submodule-config.h\n>>> @@ -14,6 +14,7 @@ struct submodule {\n>>> + const char *update;\n>>\n>> gitmodules(5) tells me the only allowed values are checkout, rebase,\n>> merge, and none. I wouldn't know at a glance how to match against\n>> those in calling code. Can this be an enum?\n>\n> \"Note that the !command form is intentionally ignored here for\n> security reasons.\"\n>\n> However you can overwrite the update field in .git/config to be \"! [foo]\",\n> which we also need to support, so let's keep it a string for now?\n\nI had forgotten about \"!command\". I think making it a string makes\nthe field hard to use: how do I determine whether it was checkout,\nrebase, merge, custom, or none? Are they case-sensitive? Am I\nsupposed to strip whitespace?\n\nOne possibility would be to have an enum and a string:\n\n\tenum submodule_update_type update;\n\tconst char *update_command;\n\nThanks,\nJonathan\n"},{"id":"277591","messageId":"CAGZ79kaBQx-vTYAoaJe2wjXuEZpq10OtzHbfvRGGYKj2+V=zEg@mail.gmail.com","threadId":"41333","inReplyTo":"20160205203309.GA28749@google.com","subject":"Re: [PATCHv8 1/9] submodule-config: keep update strategy around","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-05T20:36:55Z","receivedAt":"2016-02-05T20:36:55Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Feb 5, 2016 at 12:33 PM, Jonathan Nieder wrote:\n> Stefan Beller wrote:\n>> On Thu, Feb 4, 2016 at 4:59 PM, Jonathan Nieder wrote:\n>>> Stefan Beller wrote:\n>\n>>>> +++ b/submodule-config.h\n>>>> @@ -14,6 +14,7 @@ struct submodule {\n>>>> + const char *update;\n>>>\n>>> gitmodules(5) tells me the only allowed values are checkout, rebase,\n>>> merge, and none. I wouldn't know at a glance how to match against\n>>> those in calling code. Can this be an enum?\n>>\n>> \"Note that the !command form is intentionally ignored here for\n>> security reasons.\"\n>>\n>> However you can overwrite the update field in .git/config to be \"! [foo]\",\n>> which we also need to support, so let's keep it a string for now?\n>\n> I had forgotten about \"!command\". I think making it a string makes\n> the field hard to use: how do I determine whether it was checkout,\n> rebase, merge, custom, or none? Are they case-sensitive? Am I\n> supposed to strip whitespace?\n>\n> One possibility would be to have an enum and a string:\n>\n> enum submodule_update_type update;\n> const char *update_command;\n\nok, that makes sense.\n\n>\n> Thanks,\n> Jonathan\n"},{"id":"277623","messageId":"20160206004615.GB28749@google.com","threadId":"41333","inReplyTo":"1454623776-3347-4-git-send-email-sbeller@google.com","subject":"Re: [PATCHv8 3/9] submodule-config: remove name_and_item_from_var","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2016-02-06T00:46:15Z","receivedAt":"2016-02-06T00:46:15Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nStefan Beller wrote:\n\n> --- a/submodule-config.c\n> +++ b/submodule-config.c\n> @@ -251,18 +235,19 @@ static int parse_config(const char *var, const char *value, void *data)\n> {\n> \tstruct parse_config_parameter *me = data;\n> \tstruct submodule *submodule;\n> -\tstruct strbuf name = STRBUF_INIT, item = STRBUF_INIT;\n> -\tint ret = 0;\n> +\tint subsection_len, ret = 0;\n> +\tconst char *subsection, *key;\n> \n> -\t/* this also ensures that we only parse submodule entries */\n> -\tif (!name_and_item_from_var(var, &name, &item))\n> +\tif (parse_config_key(var, \"submodule\", &subsection,\n> +\t\t\t &subsection_len, &key) < 0 || !subsection)\n> \t\treturn 0;\n> \n> +\tsubsection = xmemdupz(subsection, subsection_len);\n\nThis appears to be leaked.\n\nThanks,\nJonathan\n"},{"id":"277624","messageId":"20160206004852.GC28749@google.com","threadId":"41333","inReplyTo":"1454623776-3347-5-git-send-email-sbeller@google.com","subject":"Re: [PATCHv8 4/9] submodule-config: slightly simplify lookup_or_create_by_name","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2016-02-06T00:48:53Z","receivedAt":"2016-02-06T00:48:53Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Stefan Beller wrote:\n\n> No need for a strbuf, when all we use it for, is duplicating a string.\n>\n> Signed-off-by: Stefan Beller \n> ---\n> submodule-config.c | 5 +----\n> 1 file changed, 1 insertion(+), 4 deletions(-)\n\nReviewed-by: Jonathan Nieder \n"},{"id":"277626","messageId":"CAGZ79kaY7+vfTdtjT_Mty+pu75wX7dktaWoULoUOnzZC5QGDzw@mail.gmail.com","threadId":"41333","inReplyTo":"20160206004615.GB28749@google.com","subject":"Re: [PATCHv8 3/9] submodule-config: remove name_and_item_from_var","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-06T01:21:50Z","receivedAt":"2016-02-06T01:21:50Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Feb 5, 2016 at 4:46 PM, Jonathan Nieder wrote:\n> Hi,\n>\n> Stefan Beller wrote:\n>\n>> --- a/submodule-config.c\n>> +++ b/submodule-config.c\n>> @@ -251,18 +235,19 @@ static int parse_config(const char *var, const char *value, void *data)\n>> {\n>> struct parse_config_parameter *me = data;\n>> struct submodule *submodule;\n>> - struct strbuf name = STRBUF_INIT, item = STRBUF_INIT;\n>> - int ret = 0;\n>> + int subsection_len, ret = 0;\n>> + const char *subsection, *key;\n>>\n>> - /* this also ensures that we only parse submodule entries */\n>> - if (!name_and_item_from_var(var, &name, &item))\n>> + if (parse_config_key(var, \"submodule\", &subsection,\n>> + &subsection_len, &key) < 0 || !subsection)\n>> return 0;\n>>\n>> + subsection = xmemdupz(subsection, subsection_len);\n>\n> This appears to be leaked.\n\nGood point, will fix.\n\nThough the impact is negligible, as this code goes away the next\npatch and there is a free included.\n\nThanks,\nStefan\n\n>\n> Thanks,\n> Jonathan\n"},{"id":"277627","messageId":"20160206012306.GD28749@google.com","threadId":"41333","inReplyTo":"1454623776-3347-6-git-send-email-sbeller@google.com","subject":"Re: [PATCHv8 5/9] submodule-config: introduce parse_generic_submodule_config","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2016-02-06T01:23:06Z","receivedAt":"2016-02-06T01:23:06Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nStefan Beller wrote:\n\n> This rewrites parse_config to distinguish between configs specific to\n> one submodule and configs which apply generically to all submodules.\n> We do not have generic submodule configs yet, but the next patch will\n> introduce \"submodule.fetchJobs\".\n\nDoes this mean that options like submodule.fetchJobs can be specified\nin either .gitmodules or .git/config, like the submodule..*\noptions?\n\nI would expect a good value for fetchJobs to be machine-specific.\nWould it work to put the submodule.fetchJobs handling in\nsubmodule.c::submodule_config alongside fetch.recurseModules instead?\n\nThanks,\nJonathan\n"},{"id":"277628","messageId":"CAGZ79kY4=wQvK1nn0Nv+qbR+n1=aqtY31EGwoHszG=jHP-iNTg@mail.gmail.com","threadId":"41333","inReplyTo":"20160206012306.GD28749@google.com","subject":"Re: [PATCHv8 5/9] submodule-config: introduce parse_generic_submodule_config","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-06T01:36:02Z","receivedAt":"2016-02-06T01:36:02Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Feb 5, 2016 at 5:23 PM, Jonathan Nieder wrote:\n> Hi,\n>\n> Stefan Beller wrote:\n>\n>> This rewrites parse_config to distinguish between configs specific to\n>> one submodule and configs which apply generically to all submodules.\n>> We do not have generic submodule configs yet, but the next patch will\n>> introduce \"submodule.fetchJobs\".\n>\n> Does this mean that options like submodule.fetchJobs can be specified\n> in either .gitmodules or .git/config, like the submodule..*\n> options?\n\noops, yes that would work.\n\nI was diving down the stack when looking where to put the code for\nsubmodule.fetchJobs, and as submodule.c::submodule_config\nmissguided me to submodule-config.c as it told me\n'anything starting with \"submodule.\" will be parsed in the\nsubmodule-config file', so at the time of writing that felt natural to\nput it there as well.\n\nSo generalizing from there, would we expect to\nhave any submodule options with no section in .gitmodules?\n\n>\n> I would expect a good value for fetchJobs to be machine-specific.\n> Would it work to put the submodule.fetchJobs handling in\n> submodule.c::submodule_config alongside fetch.recurseModules instead?\n\nSure that's possible. Although I would not mind having a preset default\nfrom a project. Would it be possible for a project to be malicious with that?\n\n(Answer yes it can, a superproject including itself multiple times and having\nparallel set to >=2 behaves similar like a fork bomb when the parallelism is\nnot overwritten in the actual .git/config, I would think)\n\n>\n> Thanks,\n> Jonathan\n\nThanks,\nStefan\n"}]}