{"thread":{"id":"41512","subject":"[PATCHv17 02/11] submodule-config: drop check against NULL","startedAt":"2016-02-25T03:06:47Z","lastAt":"2016-02-29T21:55:45Z","messageCount":59,"participants":["Stefan Beller","Junio C Hamano","Jonathan Nieder","Duy Nguyen","Johannes Sixt"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"279267","messageId":"1456369618-18127-1-git-send-email-sbeller@google.com","threadId":"41512","inReplyTo":null,"subject":"[PATCHv17 00/11] Expose submodule parallelism to the user","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-25T03:06:47Z","receivedAt":"2016-02-25T03:06:47Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This replaces origin/sb/submodule-parallel-update and applies on\norigin/sb/submodule-parallel-fetch.\n\nThanks Jonathan for review!\n\n* fixing all the small nits of v16 found by Jonathan!\n\nThanks,\nStefan\n\nInterdiff to v15: (current origin/sb/submodule-parallel-update)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 32254cd..9d94406 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -259,32 +259,36 @@ struct submodule_update_clone {\n \t/* index into 'list', the list of submodules to look into for cloning */\n \tint current;\n \tstruct module_list list;\n-\tint warn_if_uninitialized : 1;\n-\t/* update parameter passed via commandline*/\n+\tunsigned warn_if_uninitialized : 1;\n+\n+\t/* update parameter passed via commandline */\n \tstruct submodule_update_strategy update;\n+\n \t/* configuration parameters which are passed on to the children */\n \tint quiet;\n \tconst char *reference;\n \tconst char *depth;\n \tconst char *recursive_prefix;\n \tconst char *prefix;\n-\t/* lines to be output */\n+\n+\t/* Machine-readable status lines to be consumed by git-submodule.sh */\n \tstruct string_list projectlines;\n+\n \t/* If we want to stop as fast as possible and return an error */\n-\tint quickstop : 1;\n+\tunsigned quickstop : 1;\n };\n #define SUBMODULE_UPDATE_CLONE_INIT {0, MODULE_LIST_INIT, 0, \\\n \tSUBMODULE_UPDATE_STRATEGY_INIT, 0, NULL, NULL, NULL, NULL, \\\n \tSTRING_LIST_INIT_DUP, 0}\n \n /**\n- * Inspect if 'ce' needs to be cloned. If so, prepare the 'child' to be running\n- * the clone and return non zero.\n+ * Determine whether 'ce' needs to be cloned. If so, prepare the 'child' to\n+ * run the clone. Returns 1 if 'ce' needs to be cloned, 0 otherwise.\n  */\n static int prepare_to_clone_next_submodule(const struct cache_entry *ce,\n \t\t\t\t\t   struct child_process *child,\n \t\t\t\t\t   struct submodule_update_clone *suc,\n-\t\t\t\t\t   struct strbuf *err)\n+\t\t\t\t\t   struct strbuf *out)\n {\n \tconst struct submodule *sub = NULL;\n \tstruct strbuf displaypath_sb = STRBUF_INIT;\n@@ -295,10 +299,10 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,\n \n \tif (ce_stage(ce)) {\n \t\tif (suc->recursive_prefix) {\n-\t\t\tstrbuf_addf(err, \"Skipping unmerged submodule %s/%s\\n\",\n+\t\t\tstrbuf_addf(out, \"Skipping unmerged submodule %s/%s\\n\",\n \t\t\t\t    suc->recursive_prefix, ce->name);\n \t\t} else {\n-\t\t\tstrbuf_addf(err, \"Skipping unmerged submodule %s\\n\",\n+\t\t\tstrbuf_addf(out, \"Skipping unmerged submodule %s\\n\",\n \t\t\t\t    ce->name);\n \t\t}\n \t\tgoto cleanup;\n@@ -315,7 +319,7 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,\n \tif (suc->update.type == SM_UPDATE_NONE\n \t    || (suc->update.type == SM_UPDATE_UNSPECIFIED\n \t\t&& sub->update_strategy.type == SM_UPDATE_NONE)) {\n-\t\tstrbuf_addf(err, \"Skipping submodule '%s'\\n\",\n+\t\tstrbuf_addf(out, \"Skipping submodule '%s'\\n\",\n \t\t\t    displaypath);\n \t\tgoto cleanup;\n \t}\n@@ -330,11 +334,11 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,\n \tgit_config_get_string(sb.buf, &url);\n \tif (!url) {\n \t\t/*\n-\t\t * Only mention uninitialized submodules when its\n+\t\t * Only mention uninitialized submodules when their\n \t\t * path have been specified\n \t\t */\n \t\tif (suc->warn_if_uninitialized)\n-\t\t\tstrbuf_addf(err, _(\"Submodule path '%s' not initialized\\n\"\n+\t\t\tstrbuf_addf(out, _(\"Submodule path '%s' not initialized\\n\"\n \t\t\t\t    \"Maybe you want to use 'update --init'?\\n\"),\n \t\t\t\t    displaypath);\n \t\tgoto cleanup;\n@@ -435,7 +439,7 @@ static int update_clone(int argc, const char **argv, const char *prefix)\n \tstruct submodule_update_clone suc = SUBMODULE_UPDATE_CLONE_INIT;\n \n \tstruct option module_update_clone_options[] = {\n-\t\tOPT_STRING(0, \"prefix\", &prefix,\n+\t\tOPT_STRING(0, \"prefix\", &suc.prefix,\n \t\t\t   N_(\"path\"),\n \t\t\t   N_(\"path into the working tree\")),\n \t\tOPT_STRING(0, \"recursive-prefix\", &suc.recursive_prefix,\n@@ -460,7 +464,6 @@ static int update_clone(int argc, const char **argv, const char *prefix)\n \t\tN_(\"git submodule--helper update_clone [--prefix=<path>] [<path>...]\"),\n \t\tNULL\n \t};\n-\tsuc.prefix = prefix;\n \n \targc = parse_options(argc, argv, prefix, module_update_clone_options,\n \t\t\t     git_submodule_helper_usage, 0);\n@@ -475,8 +478,8 @@ static int update_clone(int argc, const char **argv, const char *prefix)\n \tif (pathspec.nr)\n \t\tsuc.warn_if_uninitialized = 1;\n \n-\tgitmodules_config();\n \t/* Overlay the parsed .gitmodules file with .git/config */\n+\tgitmodules_config();\n \tgit_config(submodule_config, NULL);\n \n \tif (max_jobs < 0)\ndiff --git a/run-command.c b/run-command.c\nindex 8abaae4..6fad42f 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -897,29 +897,29 @@ struct parallel_processes {\n \tstruct pollfd *pfd;\n \n \tunsigned shutdown : 1;\n-\tunsigned ended_with_newline: 1;\n \n \tint output_owner;\n \tstruct strbuf buffered_output; /* of finished children */\n };\n \n int default_start_failure(struct child_process *cp,\n-\t\t\t  struct strbuf *err,\n+\t\t\t  struct strbuf *out,\n \t\t\t  void *pp_cb,\n \t\t\t  void *pp_task_cb)\n {\n \tint i;\n \n-\tstrbuf_addstr(err, \"Starting a child failed:\");\n+\tstrbuf_addstr(out, \"Starting a child failed:\");\n \tfor (i = 0; cp->argv[i]; i++)\n-\t\tstrbuf_addf(err, \" %s\", cp->argv[i]);\n+\t\tstrbuf_addf(out, \" %s\", cp->argv[i]);\n+\tstrbuf_addch(out, '\\n');\n \n \treturn 0;\n }\n \n int default_task_finished(int result,\n \t\t\t  struct child_process *cp,\n-\t\t\t  struct strbuf *err,\n+\t\t\t  struct strbuf *out,\n \t\t\t  void *pp_cb,\n \t\t\t  void *pp_task_cb)\n {\n@@ -928,9 +928,10 @@ int default_task_finished(int result,\n \tif (!result)\n \t\treturn 0;\n \n-\tstrbuf_addf(err, \"A child failed with return code %d:\", result);\n+\tstrbuf_addf(out, \"A child failed with return code %d:\", result);\n \tfor (i = 0; cp->argv[i]; i++)\n-\t\tstrbuf_addf(err, \" %s\", cp->argv[i]);\n+\t\tstrbuf_addf(out, \" %s\", cp->argv[i]);\n+\tstrbuf_addch(out, '\\n');\n \n \treturn 0;\n }\n@@ -980,7 +981,6 @@ static void pp_init(struct parallel_processes *pp,\n \tpp->nr_processes = 0;\n \tpp->output_owner = 0;\n \tpp->shutdown = 0;\n-\tpp->ended_with_newline = 1;\n \tpp->children = xcalloc(n, sizeof(*pp->children));\n \tpp->pfd = xcalloc(n, sizeof(*pp->pfd));\n \tstrbuf_init(&pp->buffered_output, 0);\n@@ -1013,7 +1013,7 @@ static void pp_cleanup(struct parallel_processes *pp)\n \t * When get_next_task added messages to the buffer in its last\n \t * iteration, the buffered output is non empty.\n \t */\n-\tfputs(pp->buffered_output.buf, stderr);\n+\tstrbuf_write(&pp->buffered_output, stderr);\n \tstrbuf_release(&pp->buffered_output);\n \n \tsigchain_pop_common();\n@@ -1055,7 +1055,6 @@ static int pp_start_one(struct parallel_processes *pp)\n \t\t\t\t\t pp->data,\n \t\t\t\t\t &pp->children[i].data);\n \t\tstrbuf_addbuf(&pp->buffered_output, &pp->children[i].err);\n-\t\tstrbuf_addch(&pp->buffered_output, '\\n');\n \t\tstrbuf_reset(&pp->children[i].err);\n \t\tif (code)\n \t\t\tpp->shutdown = 1;\n@@ -1098,11 +1097,9 @@ static void pp_buffer_stderr(struct parallel_processes *pp, int output_timeout)\n static void pp_output(struct parallel_processes *pp)\n {\n \tint i = pp->output_owner;\n-\tsize_t len = pp->children[i].err.len;\n-\tif (pp->children[i].state == GIT_CP_WORKING && len) {\n-\t\tfputs(pp->children[i].err.buf, stderr);\n-\t\tpp->ended_with_newline =\n-\t\t\t(pp->children[i].err.buf[len - 1] == '\\n');\n+\tif (pp->children[i].state == GIT_CP_WORKING &&\n+\t    pp->children[i].err.len) {\n+\t\tstrbuf_write(&pp->children[i].err, stderr);\n \t\tstrbuf_reset(&pp->children[i].err);\n \t}\n }\n@@ -1112,7 +1109,6 @@ static int pp_collect_finished(struct parallel_processes *pp)\n \tint i, code;\n \tint n = pp->max_processes;\n \tint result = 0;\n-\tssize_t len;\n \n \twhile (pp->nr_processes > 0) {\n \t\tfor (i = 0; i < pp->max_processes; i++)\n@@ -1137,22 +1133,15 @@ static int pp_collect_finished(struct parallel_processes *pp)\n \t\tpp->pfd[i].fd = -1;\n \t\tchild_process_init(&pp->children[i].process);\n \n-\t\tlen = pp->children[i].err.len - 1;\n-\t\tif (len >= 0 && pp->children[i].err.buf[len] != '\\n')\n-\t\t\tstrbuf_addch(&pp->children[i].err, '\\n');\n-\n \t\tif (i != pp->output_owner) {\n \t\t\tstrbuf_addbuf(&pp->buffered_output, &pp->children[i].err);\n \t\t\tstrbuf_reset(&pp->children[i].err);\n \t\t} else {\n-\t\t\tif (len == -1 && !pp->ended_with_newline)\n-\t\t\t\tstrbuf_addch(&pp->children[i].err, '\\n');\n-\t\t\tfputs(pp->children[i].err.buf, stderr);\n+\t\t\tstrbuf_write(&pp->children[i].err, stderr);\n \t\t\tstrbuf_reset(&pp->children[i].err);\n-\t\t\tpp->ended_with_newline = 1;\n \n \t\t\t/* Output all other finished child processes */\n-\t\t\tfputs(pp->buffered_output.buf, stderr);\n+\t\t\tstrbuf_write(&pp->buffered_output, stderr);\n \t\t\tstrbuf_reset(&pp->buffered_output);\n \n \t\t\t/*\ndiff --git a/run-command.h b/run-command.h\nindex a054fa6..2bd8fee 100644\n--- a/run-command.h\n+++ b/run-command.h\n@@ -139,7 +139,7 @@ int in_async(void);\n  * return the negative signal number.\n  */\n typedef int (*get_next_task_fn)(struct child_process *cp,\n-\t\t\t\tstruct strbuf *err,\n+\t\t\t\tstruct strbuf *out,\n \t\t\t\tvoid *pp_cb,\n \t\t\t\tvoid **pp_task_cb);\n \n@@ -148,7 +148,7 @@ typedef int (*get_next_task_fn)(struct child_process *cp,\n  * a new process.\n  *\n  * You must not write to stdout or stderr in this function. Add your\n- * message to the strbuf err instead, which will be printed without\n+ * message to the strbuf out instead, which will be printed without\n  * messing up the output of the other parallel processes.\n  *\n  * pp_cb is the callback cookie as passed into run_processes_parallel,\n@@ -159,7 +159,7 @@ typedef int (*get_next_task_fn)(struct child_process *cp,\n  * the negative signal number.\n  */\n typedef int (*start_failure_fn)(struct child_process *cp,\n-\t\t\t\tstruct strbuf *err,\n+\t\t\t\tstruct strbuf *out,\n \t\t\t\tvoid *pp_cb,\n \t\t\t\tvoid *pp_task_cb);\n \n@@ -168,7 +168,7 @@ typedef int (*start_failure_fn)(struct child_process *cp,\n  * exact command which failed.\n  */\n int default_start_failure(struct child_process *cp,\n-\t\t\t  struct strbuf *err,\n+\t\t\t  struct strbuf *out,\n \t\t\t  void *pp_cb,\n \t\t\t  void *pp_task_cb);\n \n@@ -176,7 +176,7 @@ int default_start_failure(struct child_process *cp,\n  * This callback is called on every child process that finished processing.\n  *\n  * You must not write to stdout or stderr in this function. Add your\n- * message to the strbuf err instead, which will be printed without\n+ * message to the strbuf out instead, which will be printed without\n  * messing up the output of the other parallel processes.\n  *\n  * pp_cb is the callback cookie as passed into run_processes_parallel,\n@@ -188,7 +188,7 @@ int default_start_failure(struct child_process *cp,\n  */\n typedef int (*task_finished_fn)(int result,\n \t\t\t\tstruct child_process *cp,\n-\t\t\t\tstruct strbuf *err,\n+\t\t\t\tstruct strbuf *out,\n \t\t\t\tvoid *pp_cb,\n \t\t\t\tvoid *pp_task_cb);\n \n@@ -198,7 +198,7 @@ typedef int (*task_finished_fn)(int result,\n  */\n int default_task_finished(int result,\n \t\t\t  struct child_process *cp,\n-\t\t\t  struct strbuf *err,\n+\t\t\t  struct strbuf *out,\n \t\t\t  void *pp_cb,\n \t\t\t  void *pp_task_cb);\n \ndiff --git a/strbuf.c b/strbuf.c\nindex 38686ff..71345cd 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -395,6 +395,12 @@ ssize_t strbuf_read_once(struct strbuf *sb, int fd, size_t hint)\n \treturn cnt;\n }\n \n+ssize_t strbuf_write(struct strbuf *sb, FILE *f)\n+{\n+\treturn fwrite(sb->buf, 1, sb->len, f);\n+}\n+\n+\n #define STRBUF_MAXLINK (2*PATH_MAX)\n \n int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint)\ndiff --git a/strbuf.h b/strbuf.h\nindex 2bf90e7..d4f2aa1 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -387,6 +387,12 @@ extern ssize_t strbuf_read_file(struct strbuf *sb, const char *path, size_t hint\n extern int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint);\n \n /**\n+ * Write the whole content of the strbuf to the stream not stopping at\n+ * NUL bytes.\n+ */\n+extern ssize_t strbuf_write(struct strbuf *sb, FILE *stream);\n+\n+/**\n  * Read a line from a FILE *, overwriting the existing contents\n  * of the strbuf. The second argument specifies the line\n  * terminator character, typically `'\\n'`.\ndiff --git a/submodule.c b/submodule.c\nindex 8fbc847..051f722 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -219,7 +219,7 @@ void gitmodules_config(void)\n int parse_submodule_update_strategy(const char *value,\n \t\tstruct submodule_update_strategy *dst)\n {\n-\tfree((void *)dst->command);\n+\tfree((void*)dst->command);\n \tdst->command = NULL;\n \tif (!strcmp(value, \"none\"))\n \t\tdst->type = SM_UPDATE_NONE;\ndiff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh\nindex 5c6822c..12228b4 100755\n--- a/t/t0061-run-command.sh\n+++ b/t/t0061-run-command.sh\n@@ -77,32 +77,6 @@ test_expect_success 'run_command runs in parallel with more tasks than jobs avai\n \ttest_cmp expect actual\n '\n \n-test_expect_success 'run_command ensures each command ends in LF' '\n-\ttest-run-command run-command-parallel 3 sh -c \"printf \\\"%s\\n%s\\\" Hello World\" 2>actual &&\n-\ttest_cmp expect actual\n-'\n-\n-cat >expect <<-EOF\n-preloaded output of a child\n-preloaded output of a child\n-preloaded output of a child\n-preloaded output of a child\n-EOF\n-\n-test_expect_success 'run_command ensures each command ends in LF when output is only in starting cb' '\n-\ttest-run-command run-command-parallel 3 sh -c true  2>actual &&\n-\ttest_cmp expect actual\n-'\n-\n-cat >expect <<-EOF\n-EOF\n-\n-test_expect_success 'run_command ensures each command ends in LF except when there is no output' '\n-\ttest-run-command run-command-parallel-silent 3 sh -c true  2>actual &&\n-\ttest_cmp expect actual\n-'\n-\n-\n cat >expect <<-EOF\n preloaded output of a child\n asking for a quick stop\ndiff --git a/test-run-command.c b/test-run-command.c\nindex ca1ee97..fbe0a27 100644\n--- a/test-run-command.c\n+++ b/test-run-command.c\n@@ -31,20 +31,6 @@ static int parallel_next(struct child_process *cp,\n \treturn 1;\n }\n \n-static int parallel_next_silent(struct child_process *cp,\n-\t\t\t struct strbuf *err,\n-\t\t\t void *cb,\n-\t\t\t void **task_cb)\n-{\n-\tstruct child_process *d = cb;\n-\tif (number_callbacks >= 4)\n-\t\treturn 0;\n-\n-\targv_array_pushv(&cp->args, d->argv);\n-\tnumber_callbacks++;\n-\treturn 1;\n-}\n-\n static int no_job(struct child_process *cp,\n \t\t  struct strbuf *err,\n \t\t  void *cb,\n@@ -85,10 +71,6 @@ int main(int argc, char **argv)\n \tjobs = atoi(argv[2]);\n \tproc.argv = (const char **)argv + 3;\n \n-\tif (!strcmp(argv[1], \"run-command-parallel-silent\"))\n-\t\texit(run_processes_parallel(jobs, parallel_next_silent,\n-\t\t\t\t\t    NULL, NULL, &proc));\n-\n \tif (!strcmp(argv[1], \"run-command-parallel\"))\n \t\texit(run_processes_parallel(jobs, parallel_next,\n \t\t\t\t\t    NULL, NULL, &proc));\n\n\n\nStefan Beller (11):\n  submodule-config: keep update strategy around\n  submodule-config: drop check against NULL\n  fetching submodules: respect `submodule.fetchJobs` config option\n  submodule update: direct error message to stderr\n  run_processes_parallel: treat output of children as byte array\n  run-command: expose default_{start_failure, task_finished}\n  run_processes_parallel: rename parameters for the callbacks\n  run_processes_parallel: correctly terminate callbacks with an LF\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     | 254 ++++++++++++++++++++++++++++++++++++++++\n git-submodule.sh                |  56 ++++-----\n run-command.c                   |  36 +++---\n run-command.h                   |  29 ++++-\n strbuf.c                        |   6 +\n strbuf.h                        |   6 +\n submodule-config.c              |  19 ++-\n submodule-config.h              |   2 +\n submodule.c                     |  37 +++++-\n submodule.h                     |  18 +++\n t/t5526-fetch-submodules.sh     |  14 +++\n t/t7400-submodule-basic.sh      |   4 +-\n t/t7406-submodule-update.sh     |  27 +++++\n 18 files changed, 477 insertions(+), 71 deletions(-)\n\n-- \n2.7.2.373.g1655498.dirty\n"},{"id":"279275","messageId":"1456369618-18127-2-git-send-email-sbeller@google.com","threadId":"41512","inReplyTo":"1456369618-18127-1-git-send-email-sbeller@google.com","subject":"[PATCHv17 01/11] submodule-config: keep update strategy around","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-25T03:06:48Z","receivedAt":"2016-02-25T03:06:48Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Currently submodule.<name>.update is only handled by git-submodule.sh.\nC code will start to need to make use of that value as more of the\nfunctionality of git-submodule.sh moves into library code in C.\n\nAdd the update field to 'struct submodule' and populate it so it can\nbe read as sm->update or from sm->update_command.\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n submodule-config.c | 13 +++++++++++++\n submodule-config.h |  2 ++\n submodule.c        | 21 +++++++++++++++++++++\n submodule.h        | 16 ++++++++++++++++\n 4 files changed, 52 insertions(+)\n\ndiff --git a/submodule-config.c b/submodule-config.c\nindex afe0ea8..a5cd2ee 100644\n--- a/submodule-config.c\n+++ b/submodule-config.c\n@@ -59,6 +59,7 @@ static void free_one_config(struct submodule_entry *entry)\n {\n \tfree((void *) entry->config->path);\n \tfree((void *) entry->config->name);\n+\tfree((void *) entry->config->update_strategy.command);\n \tfree(entry->config);\n }\n \n@@ -194,6 +195,8 @@ static struct submodule *lookup_or_create_by_name(struct submodule_cache *cache,\n \n \tsubmodule->path = NULL;\n \tsubmodule->url = NULL;\n+\tsubmodule->update_strategy.type = SM_UPDATE_UNSPECIFIED;\n+\tsubmodule->update_strategy.command = NULL;\n \tsubmodule->fetch_recurse = RECURSE_SUBMODULES_NONE;\n \tsubmodule->ignore = NULL;\n \n@@ -311,6 +314,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 &&\n+\t\t\t submodule->update_strategy.type != SM_UPDATE_UNSPECIFIED)\n+\t\t\twarn_multiple_config(me->commit_sha1, submodule->name,\n+\t\t\t\t\t     \"update\");\n+\t\telse if (parse_submodule_update_strategy(value,\n+\t\t\t &submodule->update_strategy) < 0)\n+\t\t\t\tdie(_(\"invalid value for %s\"), var);\n \t}\n \n \tstrbuf_release(&name);\ndiff --git a/submodule-config.h b/submodule-config.h\nindex 9061e4e..092ebfc 100644\n--- a/submodule-config.h\n+++ b/submodule-config.h\n@@ -2,6 +2,7 @@\n #define SUBMODULE_CONFIG_CACHE_H\n \n #include \"hashmap.h\"\n+#include \"submodule.h\"\n #include \"strbuf.h\"\n \n /*\n@@ -14,6 +15,7 @@ struct submodule {\n \tconst char *url;\n \tint fetch_recurse;\n \tconst char *ignore;\n+\tstruct submodule_update_strategy update_strategy;\n \t/* the sha1 blob id of the responsible .gitmodules file */\n \tunsigned char gitmodules_sha1[20];\n };\ndiff --git a/submodule.c b/submodule.c\nindex b83939c..b38dd51 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -210,6 +210,27 @@ void gitmodules_config(void)\n \t}\n }\n \n+int parse_submodule_update_strategy(const char *value,\n+\t\tstruct submodule_update_strategy *dst)\n+{\n+\tfree((void*)dst->command);\n+\tdst->command = NULL;\n+\tif (!strcmp(value, \"none\"))\n+\t\tdst->type = SM_UPDATE_NONE;\n+\telse if (!strcmp(value, \"checkout\"))\n+\t\tdst->type = SM_UPDATE_CHECKOUT;\n+\telse if (!strcmp(value, \"rebase\"))\n+\t\tdst->type = SM_UPDATE_REBASE;\n+\telse if (!strcmp(value, \"merge\"))\n+\t\tdst->type = SM_UPDATE_MERGE;\n+\telse if (skip_prefix(value, \"!\", &value)) {\n+\t\tdst->type = SM_UPDATE_COMMAND;\n+\t\tdst->command = xstrdup(value);\n+\t} else\n+\t\treturn -1;\n+\treturn 0;\n+}\n+\n void handle_ignore_submodules_arg(struct diff_options *diffopt,\n \t\t\t\t  const char *arg)\n {\ndiff --git a/submodule.h b/submodule.h\nindex cbc0003..3464500 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -13,6 +13,20 @@ enum {\n \tRECURSE_SUBMODULES_ON = 2\n };\n \n+enum submodule_update_type {\n+\tSM_UPDATE_UNSPECIFIED = 0,\n+\tSM_UPDATE_CHECKOUT,\n+\tSM_UPDATE_REBASE,\n+\tSM_UPDATE_MERGE,\n+\tSM_UPDATE_NONE,\n+\tSM_UPDATE_COMMAND\n+};\n+\n+struct submodule_update_strategy {\n+\tenum submodule_update_type type;\n+\tconst char *command;\n+};\n+\n int is_staging_gitmodules_ok(void);\n int update_path_in_gitmodules(const char *oldpath, const char *newpath);\n int remove_path_from_gitmodules(const char *path);\n@@ -21,6 +35,8 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,\n \t\tconst char *path);\n int submodule_config(const char *var, const char *value, void *cb);\n void gitmodules_config(void);\n+int parse_submodule_update_strategy(const char *value,\n+\t\tstruct submodule_update_strategy *dst);\n void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *);\n void show_submodule_summary(FILE *f, const char *path,\n \t\tconst char *line_prefix,\n-- \n2.7.2.373.g1655498.dirty\n"},{"id":"279264","messageId":"1456369618-18127-3-git-send-email-sbeller@google.com","threadId":"41512","inReplyTo":"1456369618-18127-1-git-send-email-sbeller@google.com","subject":"[PATCHv17 02/11] submodule-config: drop check against NULL","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-25T03:06:49Z","receivedAt":"2016-02-25T03:06:49Z","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\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n submodule-config.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/submodule-config.c b/submodule-config.c\nindex a5cd2ee..9fa2269 100644\n--- a/submodule-config.c\n+++ b/submodule-config.c\n@@ -267,7 +267,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@@ -291,7 +291,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@@ -307,7 +307,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-- \n2.7.2.373.g1655498.dirty\n"},{"id":"279274","messageId":"1456369618-18127-4-git-send-email-sbeller@google.com","threadId":"41512","inReplyTo":"1456369618-18127-1-git-send-email-sbeller@google.com","subject":"[PATCHv17 03/11] fetching submodules: respect `submodule.fetchJobs` config option","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-25T03:06:50Z","receivedAt":"2016-02-25T03:06:50Z","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\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n Documentation/config.txt    |  6 ++++++\n builtin/fetch.c             |  2 +-\n submodule.c                 | 16 +++++++++++++++-\n submodule.h                 |  2 ++\n t/t5526-fetch-submodules.sh | 14 ++++++++++++++\n 5 files changed, 38 insertions(+), 2 deletions(-)\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.<name>.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=<value>\" 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.c b/submodule.c\nindex b38dd51..051f722 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -15,6 +15,7 @@\n #include \"thread-utils.h\"\n \n static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND;\n+static int parallel_jobs = 1;\n static struct string_list changed_submodule_paths;\n static int initialized_fetch_ref_tips;\n static struct sha1_array ref_tips_before_fetch;\n@@ -169,7 +170,12 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,\n \n int submodule_config(const char *var, const char *value, void *cb)\n {\n-\tif (starts_with(var, \"submodule.\"))\n+\tif (!strcmp(var, \"submodule.fetchjobs\")) {\n+\t\tparallel_jobs = git_config_int(var, value);\n+\t\tif (parallel_jobs < 0)\n+\t\t\tdie(_(\"negative values not allowed for submodule.fetchJobs\"));\n+\t\treturn 0;\n+\t} else if (starts_with(var, \"submodule.\"))\n \t\treturn parse_submodule_config_option(var, value);\n \telse if (!strcmp(var, \"fetch.recursesubmodules\")) {\n \t\tconfig_fetch_recurse_submodules = parse_fetch_recurse_submodules_arg(var, value);\n@@ -772,6 +778,9 @@ 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 = parallel_jobs;\n+\n \tcalculate_changed_submodule_paths();\n \trun_processes_parallel(max_parallel_jobs,\n \t\t\t       get_next_submodule,\n@@ -1118,3 +1127,8 @@ void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir)\n \tstrbuf_release(&rel_path);\n \tfree((void *)real_work_tree);\n }\n+\n+int parallel_submodules(void)\n+{\n+\treturn parallel_jobs;\n+}\ndiff --git a/submodule.h b/submodule.h\nindex 3464500..3166608 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -26,6 +26,7 @@ struct submodule_update_strategy {\n \tenum submodule_update_type type;\n \tconst char *command;\n };\n+#define SUBMODULE_UPDATE_STRATEGY_INIT {SM_UPDATE_UNSPECIFIED, NULL}\n \n int is_staging_gitmodules_ok(void);\n int update_path_in_gitmodules(const char *oldpath, const char *newpath);\n@@ -57,5 +58,6 @@ int find_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_nam\n \t\tstruct string_list *needs_pushing);\n int push_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_name);\n void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);\n+int parallel_submodules(void);\n \n #endif\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.2.373.g1655498.dirty\n"},{"id":"279273","messageId":"1456369618-18127-5-git-send-email-sbeller@google.com","threadId":"41512","inReplyTo":"1456369618-18127-1-git-send-email-sbeller@google.com","subject":"[PATCHv17 04/11] submodule update: direct error message to stderr","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-25T03:06:51Z","receivedAt":"2016-02-25T03:06:51Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Reroute the error message for specified but initialized submodules\nto stderr instead of stdout.\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n git-submodule.sh           | 4 ++--\n t/t7400-submodule-basic.sh | 4 ++--\n 2 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 9bc5c5f..9ee86d4 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -693,7 +693,7 @@ cmd_update()\n \n \t\tif test \"$update_module\" = \"none\"\n \t\tthen\n-\t\t\techo \"Skipping submodule '$displaypath'\"\n+\t\t\techo >&2 \"Skipping submodule '$displaypath'\"\n \t\t\tcontinue\n \t\tfi\n \n@@ -702,7 +702,7 @@ cmd_update()\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+\t\t\tsay >&2 \"$(eval_gettext \"Submodule path '\\$displaypath' not initialized\n Maybe you want to use 'update --init'?\")\"\n \t\t\tcontinue\n \t\tfi\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.2.373.g1655498.dirty\n"},{"id":"279269","messageId":"1456369618-18127-6-git-send-email-sbeller@google.com","threadId":"41512","inReplyTo":"1456369618-18127-1-git-send-email-sbeller@google.com","subject":"[PATCHv17 05/11] run_processes_parallel: treat output of children as byte array","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-25T03:06:52Z","receivedAt":"2016-02-25T03:06:52Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"We do not want the output to be interrupted by a NUL byte, so we\ncannot use raw fputs. Introduce strbuf_write to avoid having long\narguments in run-command.c.\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n run-command.c | 8 ++++----\n strbuf.c      | 6 ++++++\n strbuf.h      | 6 ++++++\n 3 files changed, 16 insertions(+), 4 deletions(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex 51fd72c..2f8f222 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -1011,7 +1011,7 @@ static void pp_cleanup(struct parallel_processes *pp)\n \t * When get_next_task added messages to the buffer in its last\n \t * iteration, the buffered output is non empty.\n \t */\n-\tfputs(pp->buffered_output.buf, stderr);\n+\tstrbuf_write(&pp->buffered_output, stderr);\n \tstrbuf_release(&pp->buffered_output);\n \n \tsigchain_pop_common();\n@@ -1097,7 +1097,7 @@ static void pp_output(struct parallel_processes *pp)\n \tint i = pp->output_owner;\n \tif (pp->children[i].state == GIT_CP_WORKING &&\n \t    pp->children[i].err.len) {\n-\t\tfputs(pp->children[i].err.buf, stderr);\n+\t\tstrbuf_write(&pp->children[i].err, stderr);\n \t\tstrbuf_reset(&pp->children[i].err);\n \t}\n }\n@@ -1135,11 +1135,11 @@ static int pp_collect_finished(struct parallel_processes *pp)\n \t\t\tstrbuf_addbuf(&pp->buffered_output, &pp->children[i].err);\n \t\t\tstrbuf_reset(&pp->children[i].err);\n \t\t} else {\n-\t\t\tfputs(pp->children[i].err.buf, stderr);\n+\t\t\tstrbuf_write(&pp->children[i].err, stderr);\n \t\t\tstrbuf_reset(&pp->children[i].err);\n \n \t\t\t/* Output all other finished child processes */\n-\t\t\tfputs(pp->buffered_output.buf, stderr);\n+\t\t\tstrbuf_write(&pp->buffered_output, stderr);\n \t\t\tstrbuf_reset(&pp->buffered_output);\n \n \t\t\t/*\ndiff --git a/strbuf.c b/strbuf.c\nindex 38686ff..71345cd 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -395,6 +395,12 @@ ssize_t strbuf_read_once(struct strbuf *sb, int fd, size_t hint)\n \treturn cnt;\n }\n \n+ssize_t strbuf_write(struct strbuf *sb, FILE *f)\n+{\n+\treturn fwrite(sb->buf, 1, sb->len, f);\n+}\n+\n+\n #define STRBUF_MAXLINK (2*PATH_MAX)\n \n int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint)\ndiff --git a/strbuf.h b/strbuf.h\nindex 2bf90e7..d4f2aa1 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -387,6 +387,12 @@ extern ssize_t strbuf_read_file(struct strbuf *sb, const char *path, size_t hint\n extern int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint);\n \n /**\n+ * Write the whole content of the strbuf to the stream not stopping at\n+ * NUL bytes.\n+ */\n+extern ssize_t strbuf_write(struct strbuf *sb, FILE *stream);\n+\n+/**\n  * Read a line from a FILE *, overwriting the existing contents\n  * of the strbuf. The second argument specifies the line\n  * terminator character, typically `'\\n'`.\n-- \n2.7.2.373.g1655498.dirty\n"},{"id":"279265","messageId":"1456369618-18127-7-git-send-email-sbeller@google.com","threadId":"41512","inReplyTo":"1456369618-18127-1-git-send-email-sbeller@google.com","subject":"[PATCHv17 06/11] run-command: expose default_{start_failure, task_finished}","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-25T03:06:53Z","receivedAt":"2016-02-25T03:06:53Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"We want to reuse the error reporting facilities in a later patch.\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n run-command.c | 18 +++++++++---------\n run-command.h | 19 +++++++++++++++++++\n 2 files changed, 28 insertions(+), 9 deletions(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex 2f8f222..c9b13cf 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -902,10 +902,10 @@ struct parallel_processes {\n \tstruct strbuf buffered_output; /* of finished children */\n };\n \n-static int default_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+int default_start_failure(struct child_process *cp,\n+\t\t\t  struct strbuf *err,\n+\t\t\t  void *pp_cb,\n+\t\t\t  void *pp_task_cb)\n {\n \tint i;\n \n@@ -916,11 +916,11 @@ static int default_start_failure(struct child_process *cp,\n \treturn 0;\n }\n \n-static int default_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+int default_task_finished(int result,\n+\t\t\t  struct child_process *cp,\n+\t\t\t  struct strbuf *err,\n+\t\t\t  void *pp_cb,\n+\t\t\t  void *pp_task_cb)\n {\n \tint i;\n \ndiff --git a/run-command.h b/run-command.h\nindex d5a57f9..a054fa6 100644\n--- a/run-command.h\n+++ b/run-command.h\n@@ -164,6 +164,15 @@ typedef int (*start_failure_fn)(struct child_process *cp,\n \t\t\t\tvoid *pp_task_cb);\n \n /**\n+ * If a command fails to start, then print an error message stating the\n+ * exact command which failed.\n+ */\n+int default_start_failure(struct child_process *cp,\n+\t\t\t  struct strbuf *err,\n+\t\t\t  void *pp_cb,\n+\t\t\t  void *pp_task_cb);\n+\n+/**\n  * This callback is called on every child process that finished processing.\n  *\n  * You must not write to stdout or stderr in this function. Add your\n@@ -184,6 +193,16 @@ typedef int (*task_finished_fn)(int result,\n \t\t\t\tvoid *pp_task_cb);\n \n /**\n+ * If the child process returns with a non zero error code, print\n+ * an error message of the exact command which failed.\n+ */\n+int default_task_finished(int result,\n+\t\t\t  struct child_process *cp,\n+\t\t\t  struct strbuf *err,\n+\t\t\t  void *pp_cb,\n+\t\t\t  void *pp_task_cb);\n+\n+/**\n  * Runs up to n processes at the same time. Whenever a process can be\n  * started, the callback get_next_task_fn is called to obtain the data\n  * required to start another child process.\n-- \n2.7.2.373.g1655498.dirty\n"},{"id":"279268","messageId":"1456369618-18127-8-git-send-email-sbeller@google.com","threadId":"41512","inReplyTo":"1456369618-18127-1-git-send-email-sbeller@google.com","subject":"[PATCHv17 07/11] run_processes_parallel: rename parameters for the callbacks","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-25T03:06:54Z","receivedAt":"2016-02-25T03:06:54Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The refs code has a similar pattern of passing around 'struct strbuf *err',\nwhich is strictly used for error reporting. This is not the case here,\nas the strbuf is used to accumulate all the output (whether it is error\nor not) for the user. Rename it to 'out'.\n\nSuggested-by: Jonathan Nieder <jrnieder@gmail.com>\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n run-command.c | 12 ++++++------\n run-command.h | 14 +++++++-------\n 2 files changed, 13 insertions(+), 13 deletions(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex c9b13cf..d2964e1 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -903,22 +903,22 @@ struct parallel_processes {\n };\n \n int default_start_failure(struct child_process *cp,\n-\t\t\t  struct strbuf *err,\n+\t\t\t  struct strbuf *out,\n \t\t\t  void *pp_cb,\n \t\t\t  void *pp_task_cb)\n {\n \tint i;\n \n-\tstrbuf_addstr(err, \"Starting a child failed:\");\n+\tstrbuf_addstr(out, \"Starting a child failed:\");\n \tfor (i = 0; cp->argv[i]; i++)\n-\t\tstrbuf_addf(err, \" %s\", cp->argv[i]);\n+\t\tstrbuf_addf(out, \" %s\", cp->argv[i]);\n \n \treturn 0;\n }\n \n int default_task_finished(int result,\n \t\t\t  struct child_process *cp,\n-\t\t\t  struct strbuf *err,\n+\t\t\t  struct strbuf *out,\n \t\t\t  void *pp_cb,\n \t\t\t  void *pp_task_cb)\n {\n@@ -927,9 +927,9 @@ int default_task_finished(int result,\n \tif (!result)\n \t\treturn 0;\n \n-\tstrbuf_addf(err, \"A child failed with return code %d:\", result);\n+\tstrbuf_addf(out, \"A child failed with return code %d:\", result);\n \tfor (i = 0; cp->argv[i]; i++)\n-\t\tstrbuf_addf(err, \" %s\", cp->argv[i]);\n+\t\tstrbuf_addf(out, \" %s\", cp->argv[i]);\n \n \treturn 0;\n }\ndiff --git a/run-command.h b/run-command.h\nindex a054fa6..2bd8fee 100644\n--- a/run-command.h\n+++ b/run-command.h\n@@ -139,7 +139,7 @@ int in_async(void);\n  * return the negative signal number.\n  */\n typedef int (*get_next_task_fn)(struct child_process *cp,\n-\t\t\t\tstruct strbuf *err,\n+\t\t\t\tstruct strbuf *out,\n \t\t\t\tvoid *pp_cb,\n \t\t\t\tvoid **pp_task_cb);\n \n@@ -148,7 +148,7 @@ typedef int (*get_next_task_fn)(struct child_process *cp,\n  * a new process.\n  *\n  * You must not write to stdout or stderr in this function. Add your\n- * message to the strbuf err instead, which will be printed without\n+ * message to the strbuf out instead, which will be printed without\n  * messing up the output of the other parallel processes.\n  *\n  * pp_cb is the callback cookie as passed into run_processes_parallel,\n@@ -159,7 +159,7 @@ typedef int (*get_next_task_fn)(struct child_process *cp,\n  * the negative signal number.\n  */\n typedef int (*start_failure_fn)(struct child_process *cp,\n-\t\t\t\tstruct strbuf *err,\n+\t\t\t\tstruct strbuf *out,\n \t\t\t\tvoid *pp_cb,\n \t\t\t\tvoid *pp_task_cb);\n \n@@ -168,7 +168,7 @@ typedef int (*start_failure_fn)(struct child_process *cp,\n  * exact command which failed.\n  */\n int default_start_failure(struct child_process *cp,\n-\t\t\t  struct strbuf *err,\n+\t\t\t  struct strbuf *out,\n \t\t\t  void *pp_cb,\n \t\t\t  void *pp_task_cb);\n \n@@ -176,7 +176,7 @@ int default_start_failure(struct child_process *cp,\n  * This callback is called on every child process that finished processing.\n  *\n  * You must not write to stdout or stderr in this function. Add your\n- * message to the strbuf err instead, which will be printed without\n+ * message to the strbuf out instead, which will be printed without\n  * messing up the output of the other parallel processes.\n  *\n  * pp_cb is the callback cookie as passed into run_processes_parallel,\n@@ -188,7 +188,7 @@ int default_start_failure(struct child_process *cp,\n  */\n typedef int (*task_finished_fn)(int result,\n \t\t\t\tstruct child_process *cp,\n-\t\t\t\tstruct strbuf *err,\n+\t\t\t\tstruct strbuf *out,\n \t\t\t\tvoid *pp_cb,\n \t\t\t\tvoid *pp_task_cb);\n \n@@ -198,7 +198,7 @@ typedef int (*task_finished_fn)(int result,\n  */\n int default_task_finished(int result,\n \t\t\t  struct child_process *cp,\n-\t\t\t  struct strbuf *err,\n+\t\t\t  struct strbuf *out,\n \t\t\t  void *pp_cb,\n \t\t\t  void *pp_task_cb);\n \n-- \n2.7.2.373.g1655498.dirty\n"},{"id":"279266","messageId":"1456369618-18127-9-git-send-email-sbeller@google.com","threadId":"41512","inReplyTo":"1456369618-18127-1-git-send-email-sbeller@google.com","subject":"[PATCHv17 08/11] run_processes_parallel: correctly terminate callbacks with an LF","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-25T03:06:55Z","receivedAt":"2016-02-25T03:06:55Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"As the strbufs passed around collect all output to the user, and there\nis no post processing involved we need to care about the line ending\nourselves.\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n run-command.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/run-command.c b/run-command.c\nindex d2964e1..6fad42f 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -912,6 +912,7 @@ int default_start_failure(struct child_process *cp,\n \tstrbuf_addstr(out, \"Starting a child failed:\");\n \tfor (i = 0; cp->argv[i]; i++)\n \t\tstrbuf_addf(out, \" %s\", cp->argv[i]);\n+\tstrbuf_addch(out, '\\n');\n \n \treturn 0;\n }\n@@ -930,6 +931,7 @@ int default_task_finished(int result,\n \tstrbuf_addf(out, \"A child failed with return code %d:\", result);\n \tfor (i = 0; cp->argv[i]; i++)\n \t\tstrbuf_addf(out, \" %s\", cp->argv[i]);\n+\tstrbuf_addch(out, '\\n');\n \n \treturn 0;\n }\n-- \n2.7.2.373.g1655498.dirty\n"},{"id":"279272","messageId":"1456369618-18127-10-git-send-email-sbeller@google.com","threadId":"41512","inReplyTo":"1456369618-18127-1-git-send-email-sbeller@google.com","subject":"[PATCHv17 09/11] git submodule update: have a dedicated helper for cloning","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-25T03:06:56Z","receivedAt":"2016-02-25T03:06:56Z","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\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n builtin/submodule--helper.c | 248 ++++++++++++++++++++++++++++++++++++++++++++\n git-submodule.sh            |  47 +++------\n 2 files changed, 261 insertions(+), 34 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex f4c3eff..408ac94 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -255,6 +255,253 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \treturn 0;\n }\n \n+struct submodule_update_clone {\n+\t/* index into 'list', the list of submodules to look into for cloning */\n+\tint current;\n+\tstruct module_list list;\n+\tunsigned warn_if_uninitialized : 1;\n+\n+\t/* update parameter passed via commandline */\n+\tstruct submodule_update_strategy update;\n+\n+\t/* configuration parameters which are passed on to the children */\n+\tint quiet;\n+\tconst char *reference;\n+\tconst char *depth;\n+\tconst char *recursive_prefix;\n+\tconst char *prefix;\n+\n+\t/* Machine-readable status lines to be consumed by git-submodule.sh */\n+\tstruct string_list projectlines;\n+\n+\t/* If we want to stop as fast as possible and return an error */\n+\tunsigned quickstop : 1;\n+};\n+#define SUBMODULE_UPDATE_CLONE_INIT {0, MODULE_LIST_INIT, 0, \\\n+\tSUBMODULE_UPDATE_STRATEGY_INIT, 0, NULL, NULL, NULL, NULL, \\\n+\tSTRING_LIST_INIT_DUP, 0}\n+\n+/**\n+ * Determine whether 'ce' needs to be cloned. If so, prepare the 'child' to\n+ * run the clone. Returns 1 if 'ce' needs to be cloned, 0 otherwise.\n+ */\n+static int prepare_to_clone_next_submodule(const struct cache_entry *ce,\n+\t\t\t\t\t   struct child_process *child,\n+\t\t\t\t\t   struct submodule_update_clone *suc,\n+\t\t\t\t\t   struct strbuf *out)\n+{\n+\tconst struct submodule *sub = NULL;\n+\tstruct strbuf displaypath_sb = STRBUF_INIT;\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tconst char *displaypath = NULL;\n+\tchar *url = NULL;\n+\tint needs_cloning = 0;\n+\n+\tif (ce_stage(ce)) {\n+\t\tif (suc->recursive_prefix) {\n+\t\t\tstrbuf_addf(out, \"Skipping unmerged submodule %s/%s\\n\",\n+\t\t\t\t    suc->recursive_prefix, ce->name);\n+\t\t} else {\n+\t\t\tstrbuf_addf(out, \"Skipping unmerged submodule %s\\n\",\n+\t\t\t\t    ce->name);\n+\t\t}\n+\t\tgoto cleanup;\n+\t}\n+\n+\tsub = submodule_from_path(null_sha1, ce->name);\n+\n+\tif (suc->recursive_prefix)\n+\t\tdisplaypath = relative_path(suc->recursive_prefix,\n+\t\t\t\t\t    ce->name, &displaypath_sb);\n+\telse\n+\t\tdisplaypath = ce->name;\n+\n+\tif (suc->update.type == SM_UPDATE_NONE\n+\t    || (suc->update.type == SM_UPDATE_UNSPECIFIED\n+\t\t&& sub->update_strategy.type == SM_UPDATE_NONE)) {\n+\t\tstrbuf_addf(out, \"Skipping submodule '%s'\\n\",\n+\t\t\t    displaypath);\n+\t\tgoto cleanup;\n+\t}\n+\n+\t/*\n+\t * Looking up the url in .git/config.\n+\t * We must not fall back to .gitmodules as we only want\n+\t * to process configured submodules.\n+\t */\n+\tstrbuf_reset(&sb);\n+\tstrbuf_addf(&sb, \"submodule.%s.url\", sub->name);\n+\tgit_config_get_string(sb.buf, &url);\n+\tif (!url) {\n+\t\t/*\n+\t\t * Only mention uninitialized submodules when their\n+\t\t * path have been specified\n+\t\t */\n+\t\tif (suc->warn_if_uninitialized)\n+\t\t\tstrbuf_addf(out, _(\"Submodule path '%s' not initialized\\n\"\n+\t\t\t\t    \"Maybe you want to use 'update --init'?\\n\"),\n+\t\t\t\t    displaypath);\n+\t\tgoto cleanup;\n+\t}\n+\n+\tstrbuf_reset(&sb);\n+\tstrbuf_addf(&sb, \"%s/.git\", ce->name);\n+\tneeds_cloning = !file_exists(sb.buf);\n+\n+\tstrbuf_reset(&sb);\n+\tstrbuf_addf(&sb, \"%06o %s %d %d\\t%s\\n\", ce->ce_mode,\n+\t\t\tsha1_to_hex(ce->sha1), ce_stage(ce),\n+\t\t\tneeds_cloning, ce->name);\n+\tstring_list_append(&suc->projectlines, sb.buf);\n+\n+\tif (!needs_cloning)\n+\t\tgoto cleanup;\n+\n+\tchild->git_cmd = 1;\n+\tchild->no_stdin = 1;\n+\tchild->stdout_to_stderr = 1;\n+\tchild->err = -1;\n+\targv_array_push(&child->args, \"submodule--helper\");\n+\targv_array_push(&child->args, \"clone\");\n+\tif (suc->quiet)\n+\t\targv_array_push(&child->args, \"--quiet\");\n+\tif (suc->prefix)\n+\t\targv_array_pushl(&child->args, \"--prefix\", suc->prefix, NULL);\n+\targv_array_pushl(&child->args, \"--path\", sub->path, NULL);\n+\targv_array_pushl(&child->args, \"--name\", sub->name, NULL);\n+\targv_array_pushl(&child->args, \"--url\", url, NULL);\n+\tif (suc->reference)\n+\t\targv_array_push(&child->args, suc->reference);\n+\tif (suc->depth)\n+\t\targv_array_push(&child->args, suc->depth);\n+\n+cleanup:\n+\tfree(url);\n+\tstrbuf_reset(&displaypath_sb);\n+\tstrbuf_reset(&sb);\n+\n+\treturn needs_cloning;\n+}\n+\n+static int update_clone_get_next_task(struct child_process *child,\n+\t\t\t\t      struct strbuf *err,\n+\t\t\t\t      void *suc_cb,\n+\t\t\t\t      void **void_task_cb)\n+{\n+\tstruct submodule_update_clone *suc = suc_cb;\n+\n+\tfor (; suc->current < suc->list.nr; suc->current++) {\n+\t\tconst struct cache_entry *ce = suc->list.entries[suc->current];\n+\t\tif (prepare_to_clone_next_submodule(ce, child, suc, err)) {\n+\t\t\tsuc->current++;\n+\t\t\treturn 1;\n+\t\t}\n+\t}\n+\treturn 0;\n+}\n+\n+static int update_clone_start_failure(struct child_process *child,\n+\t\t\t\t      struct strbuf *err,\n+\t\t\t\t      void *suc_cb,\n+\t\t\t\t      void *void_task_cb)\n+{\n+\tstruct submodule_update_clone *suc = suc_cb;\n+\n+\tdefault_start_failure(child, err, suc_cb, void_task_cb);\n+\tsuc->quickstop = 1;\n+\n+\treturn 1;\n+}\n+\n+static int update_clone_task_finished(int result,\n+\t\t\t\t      struct child_process *child,\n+\t\t\t\t      struct strbuf *err,\n+\t\t\t\t      void *suc_cb,\n+\t\t\t\t      void *void_task_cb)\n+{\n+\tstruct submodule_update_clone *suc = suc_cb;\n+\n+\tif (!result)\n+\t\treturn 0;\n+\n+\tdefault_task_finished(result, child, err, suc_cb, void_task_cb);\n+\tsuc->quickstop = 1;\n+\n+\treturn 1;\n+}\n+\n+static int update_clone(int argc, const char **argv, const char *prefix)\n+{\n+\tconst char *update = NULL;\n+\tstruct string_list_item *item;\n+\tstruct pathspec pathspec;\n+\tstruct submodule_update_clone suc = SUBMODULE_UPDATE_CLONE_INIT;\n+\n+\tstruct option module_update_clone_options[] = {\n+\t\tOPT_STRING(0, \"prefix\", &suc.prefix,\n+\t\t\t   N_(\"path\"),\n+\t\t\t   N_(\"path into the working tree\")),\n+\t\tOPT_STRING(0, \"recursive-prefix\", &suc.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\", &update,\n+\t\t\t   N_(\"string\"),\n+\t\t\t   N_(\"rebase, merge, checkout or none\")),\n+\t\tOPT_STRING(0, \"reference\", &suc.reference, N_(\"repo\"),\n+\t\t\t   N_(\"reference repository\")),\n+\t\tOPT_STRING(0, \"depth\", &suc.depth, \"<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(&suc.quiet, N_(\"don'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 update_clone [--prefix=<path>] [<path>...]\"),\n+\t\tNULL\n+\t};\n+\n+\targc = parse_options(argc, argv, prefix, module_update_clone_options,\n+\t\t\t     git_submodule_helper_usage, 0);\n+\n+\tif (update)\n+\t\tif (parse_submodule_update_strategy(update, &suc.update) < 0)\n+\t\t\tdie(_(\"bad value for update parameter\"));\n+\n+\tif (module_list_compute(argc, argv, prefix, &pathspec, &suc.list) < 0)\n+\t\treturn 1;\n+\n+\tif (pathspec.nr)\n+\t\tsuc.warn_if_uninitialized = 1;\n+\n+\t/* Overlay the parsed .gitmodules file with .git/config */\n+\tgitmodules_config();\n+\tgit_config(submodule_config, NULL);\n+\n+\trun_processes_parallel(1,\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       &suc);\n+\n+\t/*\n+\t * We saved the output and put it out all at once now.\n+\t * That means:\n+\t * - the listener does not have to interleave their (checkout)\n+\t *   work with our fetching.  The writes involved in a\n+\t *   checkout involve more straightforward sequential I/O.\n+\t * - the listener can avoid doing any work if fetching failed.\n+\t */\n+\tif (suc.quickstop)\n+\t\treturn 1;\n+\n+\tfor_each_string_list_item(item, &suc.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 +511,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 9ee86d4..a6a82d2 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -664,17 +664,20 @@ cmd_update()\n \t\tcmd_init \"--\" \"$@\" || return\n \tfi\n \n-\tcloned_modules=\n-\tgit submodule--helper list --prefix \"$wt_prefix\" \"$@\" | {\n+\t{\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\"$@\" || echo \"#unmatched\"\n+\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 +694,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 >&2 \"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 >&2 \"$(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 +737,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)\n-- \n2.7.2.373.g1655498.dirty\n"},{"id":"279271","messageId":"1456369618-18127-11-git-send-email-sbeller@google.com","threadId":"41512","inReplyTo":"1456369618-18127-1-git-send-email-sbeller@google.com","subject":"[PATCHv17 10/11] submodule update: expose parallelism to the user","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-25T03:06:57Z","receivedAt":"2016-02-25T03:06:57Z","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\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n Documentation/git-submodule.txt |  7 ++++++-\n builtin/submodule--helper.c     |  8 +++++++-\n git-submodule.sh                |  9 +++++++++\n t/t7406-submodule-update.sh     | 12 ++++++++++++\n 4 files changed, 34 insertions(+), 2 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] [--] <path>...\n 'git submodule' [--quiet] update [--init] [--remote] [-N|--no-fetch]\n \t      [-f|--force] [--rebase|--merge] [--reference <repository>]\n-\t      [--depth <depth>] [--recursive] [--] [<path>...]\n+\t      [--depth <depth>] [--recursive] [--jobs <n>] [--] [<path>...]\n 'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) <n>]\n \t      [commit] [--] [<path>...]\n 'git submodule' [--quiet] foreach [--recursive] <command>\n@@ -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>::\n+--jobs <n>::\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 <path>...::\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 408ac94..9d94406 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 static int update_clone(int argc, const char **argv, const char *prefix)\n {\n \tconst char *update = NULL;\n+\tint max_jobs = -1;\n \tstruct string_list_item *item;\n \tstruct pathspec pathspec;\n \tstruct submodule_update_clone suc = SUBMODULE_UPDATE_CLONE_INIT;\n@@ -453,6 +454,8 @@ static int update_clone(int argc, const char **argv, const char *prefix)\n \t\tOPT_STRING(0, \"depth\", &suc.depth, \"<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(&suc.quiet, N_(\"don't print cloning progress\")),\n \t\tOPT_END()\n \t};\n@@ -479,7 +482,10 @@ static int update_clone(int argc, const char **argv, const char *prefix)\n \tgitmodules_config();\n \tgit_config(submodule_config, NULL);\n \n-\trun_processes_parallel(1,\n+\tif (max_jobs < 0)\n+\t\tmax_jobs = parallel_submodules();\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,\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex a6a82d2..86018ee 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@@ -671,6 +679,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\"$@\" || echo \"#unmatched\"\n \t} | {\n \terr=\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.2.373.g1655498.dirty\n"},{"id":"279270","messageId":"1456369618-18127-12-git-send-email-sbeller@google.com","threadId":"41512","inReplyTo":"1456369618-18127-1-git-send-email-sbeller@google.com","subject":"[PATCHv17 11/11] clone: allow an explicit argument for parallel submodule clones","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-25T03:06:58Z","receivedAt":"2016-02-25T03:06:58Z","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\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\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 <name>] [-b <name>] [-u <upload-pack>] [--reference <repository>]\n \t  [--dissociate] [--separate-git-dir <git dir>]\n \t  [--depth <depth>] [--[no-]single-branch]\n-\t  [--recursive | --recurse-submodules] [--] <repository>\n+\t  [--recursive | --recurse-submodules] [--jobs <n>] [--] <repository>\n \t  [<directory>]\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>::\n+--jobs <n>::\n+\tThe number of submodules fetched at the same time.\n+\tDefaults to the `submodule.fetchJobs` option.\n \n <repository>::\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.2.373.g1655498.dirty\n"},{"id":"279356","messageId":"xmqqwppsoeab.fsf@gitster.mtv.corp.google.com","threadId":"41512","inReplyTo":"1456369618-18127-2-git-send-email-sbeller@google.com","subject":"Re: [PATCHv17 01/11] submodule-config: keep update strategy around","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-02-25T18:06:36Z","receivedAt":"2016-02-25T18:06:36Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> +int parse_submodule_update_strategy(const char *value,\n> +\t\tstruct submodule_update_strategy *dst)\n> +{\n> +\tfree((void*)dst->command);\n\n\tfree((void *)dst->command);\n\n\"git tbdiff\" is quite handy; it didn't spot any other lossage of\nlocal tweak that was in 'pu', which is good.\n\nThanks, will replace.\n"},{"id":"279358","messageId":"xmqqsi0godtp.fsf@gitster.mtv.corp.google.com","threadId":"41512","inReplyTo":"1456369618-18127-6-git-send-email-sbeller@google.com","subject":"Re: [PATCHv17 05/11] run_processes_parallel: treat output of children as byte array","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-02-25T18:16:34Z","receivedAt":"2016-02-25T18:16:34Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> @@ -1135,11 +1135,11 @@ static int pp_collect_finished(struct parallel_processes *pp)\n>  \t\t\tstrbuf_addbuf(&pp->buffered_output, &pp->children[i].err);\n>  \t\t\tstrbuf_reset(&pp->children[i].err);\n>  \t\t} else {\n> -\t\t\tfputs(pp->children[i].err.buf, stderr);\n> +\t\t\tstrbuf_write(&pp->children[i].err, stderr);\n>  \t\t\tstrbuf_reset(&pp->children[i].err);\n>  \n>  \t\t\t/* Output all other finished child processes */\n> -\t\t\tfputs(pp->buffered_output.buf, stderr);\n> +\t\t\tstrbuf_write(&pp->buffered_output, stderr);\n>  \t\t\tstrbuf_reset(&pp->buffered_output);\n>  \n>  \t\t\t/*\n> diff --git a/strbuf.c b/strbuf.c\n> index 38686ff..71345cd 100644\n> --- a/strbuf.c\n> +++ b/strbuf.c\n> @@ -395,6 +395,12 @@ ssize_t strbuf_read_once(struct strbuf *sb, int fd, size_t hint)\n>  \treturn cnt;\n>  }\n>  \n> +ssize_t strbuf_write(struct strbuf *sb, FILE *f)\n> +{\n> +\treturn fwrite(sb->buf, 1, sb->len, f);\n> +}\n\nWhenever I see a call to a function that takes size and nmemb\nseparately, I get worried about the case where nmemb is zero.\nHopefully everybody implements such a fwrite() as a no-op?\n\nThis may not matter in this patch as no caller checks the return\nvalue from this function, but shouldn't the callers be a bit more\ncareful checking errors in the first place?\n"},{"id":"279360","messageId":"CAGZ79kaG+npqYDxABdUGrXH04nBF0_S+VLOHGrsa0jPcWMtiUQ@mail.gmail.com","threadId":"41512","inReplyTo":"xmqqwppsoeab.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCHv17 01/11] submodule-config: keep update strategy around","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-25T18:21:54Z","receivedAt":"2016-02-25T18:21:54Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Feb 25, 2016 at 10:06 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Stefan Beller <sbeller@google.com> writes:\n>\n>> +int parse_submodule_update_strategy(const char *value,\n>> +             struct submodule_update_strategy *dst)\n>> +{\n>> +     free((void*)dst->command);\n>\n>         free((void *)dst->command);\n>\n> \"git tbdiff\" is quite handy; it didn't spot any other lossage of\n> local tweak that was in 'pu', which is good.\n>\n> Thanks, will replace.\n\n\nI usually use git diff <localbranch>.. origin/sb/feature to generate\nthe interdiffs,\nso if you tweak things and I take these tweaks, then the interdiff is\nnot complete.\n\nOn the other hand if I miss the tweak, such a diff shows a revert of the tweak.\n\nI remember applying this tweak; not sure how it got lost again.\n"},{"id":"279384","messageId":"CAGZ79kaQHk+1k48Yrn_2p9SwB8aJO96jUtL-cA-Gr1PASMkUkQ@mail.gmail.com","threadId":"41512","inReplyTo":"xmqqsi0godtp.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCHv17 05/11] run_processes_parallel: treat output of children as byte array","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-25T20:35:04Z","receivedAt":"2016-02-25T20:35:04Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Feb 25, 2016 at 10:16 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Stefan Beller <sbeller@google.com> writes:\n>\n>> @@ -1135,11 +1135,11 @@ static int pp_collect_finished(struct parallel_processes *pp)\n>>                       strbuf_addbuf(&pp->buffered_output, &pp->children[i].err);\n>>                       strbuf_reset(&pp->children[i].err);\n>>               } else {\n>> -                     fputs(pp->children[i].err.buf, stderr);\n>> +                     strbuf_write(&pp->children[i].err, stderr);\n>>                       strbuf_reset(&pp->children[i].err);\n>>\n>>                       /* Output all other finished child processes */\n>> -                     fputs(pp->buffered_output.buf, stderr);\n>> +                     strbuf_write(&pp->buffered_output, stderr);\n>>                       strbuf_reset(&pp->buffered_output);\n>>\n>>                       /*\n>> diff --git a/strbuf.c b/strbuf.c\n>> index 38686ff..71345cd 100644\n>> --- a/strbuf.c\n>> +++ b/strbuf.c\n>> @@ -395,6 +395,12 @@ ssize_t strbuf_read_once(struct strbuf *sb, int fd, size_t hint)\n>>       return cnt;\n>>  }\n>>\n>> +ssize_t strbuf_write(struct strbuf *sb, FILE *f)\n>> +{\n>> +     return fwrite(sb->buf, 1, sb->len, f);\n>> +}\n>\n> Whenever I see a call to a function that takes size and nmemb\n> separately, I get worried about the case where nmemb is zero.\n> Hopefully everybody implements such a fwrite() as a no-op?\n>\n> This may not matter in this patch as no caller checks the return\n> value from this function, but shouldn't the callers be a bit more\n> careful checking errors in the first place?\n\nIf there is no other comment on the series, I plan on sending a patch\nto fix this up afterwards to not further collide with the refs backend series.\n"},{"id":"279394","messageId":"xmqqoab4mnoc.fsf@gitster.mtv.corp.google.com","threadId":"41512","inReplyTo":"1456369618-18127-1-git-send-email-sbeller@google.com","subject":"Re: [PATCHv17 00/11] Expose submodule parallelism to the user","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-02-25T22:26:43Z","receivedAt":"2016-02-25T22:26:43Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> This replaces origin/sb/submodule-parallel-update and applies on\n> origin/sb/submodule-parallel-fetch.\n\nThis round, when applied directly on top of 62104ba1 (submodules:\nallow parallel fetching, add tests and documentation, 2015-12-15),\nseems to break 7400 (43, 45, 78) and 7406 (3-5), at least.\n"},{"id":"279421","messageId":"1456441708-13512-1-git-send-email-sbeller@google.com","threadId":"41512","inReplyTo":"xmqqoab4mnoc.fsf@gitster.mtv.corp.google.com","subject":"[PATCHv18 00/11] Expose","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-25T23:08:17Z","receivedAt":"2016-02-25T23:08:17Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"* fixes the test breakage (which was caused by a last minute fix :/ to the\n  prefix variable handling)\n* include the separately sent patch to not call fwrite in case of length 0\n\nThanks,\nStefan\n\nInterdiff to v17:\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 9d94406..882aeca 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -465,14 +465,14 @@ static int update_clone(int argc, const char **argv, const char *prefix)\n \t\tNULL\n \t};\n \n-\targc = parse_options(argc, argv, prefix, module_update_clone_options,\n+\targc = parse_options(argc, argv, suc.prefix, module_update_clone_options,\n \t\t\t     git_submodule_helper_usage, 0);\n \n \tif (update)\n \t\tif (parse_submodule_update_strategy(update, &suc.update) < 0)\n \t\t\tdie(_(\"bad value for update parameter\"));\n \n-\tif (module_list_compute(argc, argv, prefix, &pathspec, &suc.list) < 0)\n+\tif (module_list_compute(argc, argv, suc.prefix, &pathspec, &suc.list) < 0)\n \t\treturn 1;\n \n \tif (pathspec.nr)\ndiff --git a/strbuf.c b/strbuf.c\nindex 71345cd..5f6da82 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -397,7 +397,7 @@ ssize_t strbuf_read_once(struct strbuf *sb, int fd, size_t hint)\n \n ssize_t strbuf_write(struct strbuf *sb, FILE *f)\n {\n-\treturn fwrite(sb->buf, 1, sb->len, f);\n+\treturn sb->len ? fwrite(sb->buf, 1, sb->len, f) : 0;\n }\n \nStefan Beller (11):\n  submodule-config: keep update strategy around\n  submodule-config: drop check against NULL\n  fetching submodules: respect `submodule.fetchJobs` config option\n  submodule update: direct error message to stderr\n  run_processes_parallel: treat output of children as byte array\n  run-command: expose default_{start_failure, task_finished}\n  run_processes_parallel: rename parameters for the callbacks\n  run_processes_parallel: correctly terminate callbacks with an LF\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     | 254 ++++++++++++++++++++++++++++++++++++++++\n git-submodule.sh                |  56 ++++-----\n run-command.c                   |  36 +++---\n run-command.h                   |  29 ++++-\n strbuf.c                        |   6 +\n strbuf.h                        |   6 +\n submodule-config.c              |  19 ++-\n submodule-config.h              |   2 +\n submodule.c                     |  37 +++++-\n submodule.h                     |  18 +++\n t/t5526-fetch-submodules.sh     |  14 +++\n t/t7400-submodule-basic.sh      |   4 +-\n t/t7406-submodule-update.sh     |  27 +++++\n 18 files changed, 477 insertions(+), 71 deletions(-)\n\n-- \n2.7.0.rc0.36.g75877e4.dirty\n"},{"id":"279420","messageId":"1456441708-13512-2-git-send-email-sbeller@google.com","threadId":"41512","inReplyTo":"1456441708-13512-1-git-send-email-sbeller@google.com","subject":"[PATCHv18 01/11] submodule-config: keep update strategy around","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-25T23:08:18Z","receivedAt":"2016-02-25T23:08:18Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Currently submodule.<name>.update is only handled by git-submodule.sh.\nC code will start to need to make use of that value as more of the\nfunctionality of git-submodule.sh moves into library code in C.\n\nAdd the update field to 'struct submodule' and populate it so it can\nbe read as sm->update or from sm->update_command.\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n submodule-config.c | 13 +++++++++++++\n submodule-config.h |  2 ++\n submodule.c        | 21 +++++++++++++++++++++\n submodule.h        | 16 ++++++++++++++++\n 4 files changed, 52 insertions(+)\n\ndiff --git a/submodule-config.c b/submodule-config.c\nindex afe0ea8..a5cd2ee 100644\n--- a/submodule-config.c\n+++ b/submodule-config.c\n@@ -59,6 +59,7 @@ static void free_one_config(struct submodule_entry *entry)\n {\n \tfree((void *) entry->config->path);\n \tfree((void *) entry->config->name);\n+\tfree((void *) entry->config->update_strategy.command);\n \tfree(entry->config);\n }\n \n@@ -194,6 +195,8 @@ static struct submodule *lookup_or_create_by_name(struct submodule_cache *cache,\n \n \tsubmodule->path = NULL;\n \tsubmodule->url = NULL;\n+\tsubmodule->update_strategy.type = SM_UPDATE_UNSPECIFIED;\n+\tsubmodule->update_strategy.command = NULL;\n \tsubmodule->fetch_recurse = RECURSE_SUBMODULES_NONE;\n \tsubmodule->ignore = NULL;\n \n@@ -311,6 +314,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 &&\n+\t\t\t submodule->update_strategy.type != SM_UPDATE_UNSPECIFIED)\n+\t\t\twarn_multiple_config(me->commit_sha1, submodule->name,\n+\t\t\t\t\t     \"update\");\n+\t\telse if (parse_submodule_update_strategy(value,\n+\t\t\t &submodule->update_strategy) < 0)\n+\t\t\t\tdie(_(\"invalid value for %s\"), var);\n \t}\n \n \tstrbuf_release(&name);\ndiff --git a/submodule-config.h b/submodule-config.h\nindex 9061e4e..092ebfc 100644\n--- a/submodule-config.h\n+++ b/submodule-config.h\n@@ -2,6 +2,7 @@\n #define SUBMODULE_CONFIG_CACHE_H\n \n #include \"hashmap.h\"\n+#include \"submodule.h\"\n #include \"strbuf.h\"\n \n /*\n@@ -14,6 +15,7 @@ struct submodule {\n \tconst char *url;\n \tint fetch_recurse;\n \tconst char *ignore;\n+\tstruct submodule_update_strategy update_strategy;\n \t/* the sha1 blob id of the responsible .gitmodules file */\n \tunsigned char gitmodules_sha1[20];\n };\ndiff --git a/submodule.c b/submodule.c\nindex b83939c..b38dd51 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -210,6 +210,27 @@ void gitmodules_config(void)\n \t}\n }\n \n+int parse_submodule_update_strategy(const char *value,\n+\t\tstruct submodule_update_strategy *dst)\n+{\n+\tfree((void*)dst->command);\n+\tdst->command = NULL;\n+\tif (!strcmp(value, \"none\"))\n+\t\tdst->type = SM_UPDATE_NONE;\n+\telse if (!strcmp(value, \"checkout\"))\n+\t\tdst->type = SM_UPDATE_CHECKOUT;\n+\telse if (!strcmp(value, \"rebase\"))\n+\t\tdst->type = SM_UPDATE_REBASE;\n+\telse if (!strcmp(value, \"merge\"))\n+\t\tdst->type = SM_UPDATE_MERGE;\n+\telse if (skip_prefix(value, \"!\", &value)) {\n+\t\tdst->type = SM_UPDATE_COMMAND;\n+\t\tdst->command = xstrdup(value);\n+\t} else\n+\t\treturn -1;\n+\treturn 0;\n+}\n+\n void handle_ignore_submodules_arg(struct diff_options *diffopt,\n \t\t\t\t  const char *arg)\n {\ndiff --git a/submodule.h b/submodule.h\nindex cbc0003..3464500 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -13,6 +13,20 @@ enum {\n \tRECURSE_SUBMODULES_ON = 2\n };\n \n+enum submodule_update_type {\n+\tSM_UPDATE_UNSPECIFIED = 0,\n+\tSM_UPDATE_CHECKOUT,\n+\tSM_UPDATE_REBASE,\n+\tSM_UPDATE_MERGE,\n+\tSM_UPDATE_NONE,\n+\tSM_UPDATE_COMMAND\n+};\n+\n+struct submodule_update_strategy {\n+\tenum submodule_update_type type;\n+\tconst char *command;\n+};\n+\n int is_staging_gitmodules_ok(void);\n int update_path_in_gitmodules(const char *oldpath, const char *newpath);\n int remove_path_from_gitmodules(const char *path);\n@@ -21,6 +35,8 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,\n \t\tconst char *path);\n int submodule_config(const char *var, const char *value, void *cb);\n void gitmodules_config(void);\n+int parse_submodule_update_strategy(const char *value,\n+\t\tstruct submodule_update_strategy *dst);\n void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *);\n void show_submodule_summary(FILE *f, const char *path,\n \t\tconst char *line_prefix,\n-- \n2.7.0.rc0.36.g75877e4.dirty\n"},{"id":"279426","messageId":"1456441708-13512-3-git-send-email-sbeller@google.com","threadId":"41512","inReplyTo":"1456441708-13512-1-git-send-email-sbeller@google.com","subject":"[PATCHv18 02/11] submodule-config: drop check against NULL","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-25T23:08:19Z","receivedAt":"2016-02-25T23:08:19Z","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\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n submodule-config.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/submodule-config.c b/submodule-config.c\nindex a5cd2ee..9fa2269 100644\n--- a/submodule-config.c\n+++ b/submodule-config.c\n@@ -267,7 +267,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@@ -291,7 +291,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@@ -307,7 +307,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-- \n2.7.0.rc0.36.g75877e4.dirty\n"},{"id":"279431","messageId":"1456441708-13512-4-git-send-email-sbeller@google.com","threadId":"41512","inReplyTo":"1456441708-13512-1-git-send-email-sbeller@google.com","subject":"[PATCHv18 03/11] fetching submodules: respect `submodule.fetchJobs` config option","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-25T23:08:20Z","receivedAt":"2016-02-25T23:08:20Z","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\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n Documentation/config.txt    |  6 ++++++\n builtin/fetch.c             |  2 +-\n submodule.c                 | 16 +++++++++++++++-\n submodule.h                 |  2 ++\n t/t5526-fetch-submodules.sh | 14 ++++++++++++++\n 5 files changed, 38 insertions(+), 2 deletions(-)\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.<name>.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=<value>\" 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.c b/submodule.c\nindex b38dd51..051f722 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -15,6 +15,7 @@\n #include \"thread-utils.h\"\n \n static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND;\n+static int parallel_jobs = 1;\n static struct string_list changed_submodule_paths;\n static int initialized_fetch_ref_tips;\n static struct sha1_array ref_tips_before_fetch;\n@@ -169,7 +170,12 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,\n \n int submodule_config(const char *var, const char *value, void *cb)\n {\n-\tif (starts_with(var, \"submodule.\"))\n+\tif (!strcmp(var, \"submodule.fetchjobs\")) {\n+\t\tparallel_jobs = git_config_int(var, value);\n+\t\tif (parallel_jobs < 0)\n+\t\t\tdie(_(\"negative values not allowed for submodule.fetchJobs\"));\n+\t\treturn 0;\n+\t} else if (starts_with(var, \"submodule.\"))\n \t\treturn parse_submodule_config_option(var, value);\n \telse if (!strcmp(var, \"fetch.recursesubmodules\")) {\n \t\tconfig_fetch_recurse_submodules = parse_fetch_recurse_submodules_arg(var, value);\n@@ -772,6 +778,9 @@ 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 = parallel_jobs;\n+\n \tcalculate_changed_submodule_paths();\n \trun_processes_parallel(max_parallel_jobs,\n \t\t\t       get_next_submodule,\n@@ -1118,3 +1127,8 @@ void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir)\n \tstrbuf_release(&rel_path);\n \tfree((void *)real_work_tree);\n }\n+\n+int parallel_submodules(void)\n+{\n+\treturn parallel_jobs;\n+}\ndiff --git a/submodule.h b/submodule.h\nindex 3464500..3166608 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -26,6 +26,7 @@ struct submodule_update_strategy {\n \tenum submodule_update_type type;\n \tconst char *command;\n };\n+#define SUBMODULE_UPDATE_STRATEGY_INIT {SM_UPDATE_UNSPECIFIED, NULL}\n \n int is_staging_gitmodules_ok(void);\n int update_path_in_gitmodules(const char *oldpath, const char *newpath);\n@@ -57,5 +58,6 @@ int find_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_nam\n \t\tstruct string_list *needs_pushing);\n int push_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_name);\n void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);\n+int parallel_submodules(void);\n \n #endif\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.36.g75877e4.dirty\n"},{"id":"279422","messageId":"1456441708-13512-5-git-send-email-sbeller@google.com","threadId":"41512","inReplyTo":"1456441708-13512-1-git-send-email-sbeller@google.com","subject":"[PATCHv18 04/11] submodule update: direct error message to stderr","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-25T23:08:21Z","receivedAt":"2016-02-25T23:08:21Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Reroute the error message for specified but initialized submodules\nto stderr instead of stdout.\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n git-submodule.sh           | 4 ++--\n t/t7400-submodule-basic.sh | 4 ++--\n 2 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 9bc5c5f..9ee86d4 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -693,7 +693,7 @@ cmd_update()\n \n \t\tif test \"$update_module\" = \"none\"\n \t\tthen\n-\t\t\techo \"Skipping submodule '$displaypath'\"\n+\t\t\techo >&2 \"Skipping submodule '$displaypath'\"\n \t\t\tcontinue\n \t\tfi\n \n@@ -702,7 +702,7 @@ cmd_update()\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+\t\t\tsay >&2 \"$(eval_gettext \"Submodule path '\\$displaypath' not initialized\n Maybe you want to use 'update --init'?\")\"\n \t\t\tcontinue\n \t\tfi\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.36.g75877e4.dirty\n"},{"id":"279423","messageId":"1456441708-13512-6-git-send-email-sbeller@google.com","threadId":"41512","inReplyTo":"1456441708-13512-1-git-send-email-sbeller@google.com","subject":"[PATCHv18 05/11] run_processes_parallel: treat output of children as byte array","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-25T23:08:22Z","receivedAt":"2016-02-25T23:08:22Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"We do not want the output to be interrupted by a NUL byte, so we\ncannot use raw fputs. Introduce strbuf_write to avoid having long\narguments in run-command.c.\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n run-command.c | 8 ++++----\n strbuf.c      | 6 ++++++\n strbuf.h      | 6 ++++++\n 3 files changed, 16 insertions(+), 4 deletions(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex 51fd72c..2f8f222 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -1011,7 +1011,7 @@ static void pp_cleanup(struct parallel_processes *pp)\n \t * When get_next_task added messages to the buffer in its last\n \t * iteration, the buffered output is non empty.\n \t */\n-\tfputs(pp->buffered_output.buf, stderr);\n+\tstrbuf_write(&pp->buffered_output, stderr);\n \tstrbuf_release(&pp->buffered_output);\n \n \tsigchain_pop_common();\n@@ -1097,7 +1097,7 @@ static void pp_output(struct parallel_processes *pp)\n \tint i = pp->output_owner;\n \tif (pp->children[i].state == GIT_CP_WORKING &&\n \t    pp->children[i].err.len) {\n-\t\tfputs(pp->children[i].err.buf, stderr);\n+\t\tstrbuf_write(&pp->children[i].err, stderr);\n \t\tstrbuf_reset(&pp->children[i].err);\n \t}\n }\n@@ -1135,11 +1135,11 @@ static int pp_collect_finished(struct parallel_processes *pp)\n \t\t\tstrbuf_addbuf(&pp->buffered_output, &pp->children[i].err);\n \t\t\tstrbuf_reset(&pp->children[i].err);\n \t\t} else {\n-\t\t\tfputs(pp->children[i].err.buf, stderr);\n+\t\t\tstrbuf_write(&pp->children[i].err, stderr);\n \t\t\tstrbuf_reset(&pp->children[i].err);\n \n \t\t\t/* Output all other finished child processes */\n-\t\t\tfputs(pp->buffered_output.buf, stderr);\n+\t\t\tstrbuf_write(&pp->buffered_output, stderr);\n \t\t\tstrbuf_reset(&pp->buffered_output);\n \n \t\t\t/*\ndiff --git a/strbuf.c b/strbuf.c\nindex 38686ff..5f6da82 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -395,6 +395,12 @@ ssize_t strbuf_read_once(struct strbuf *sb, int fd, size_t hint)\n \treturn cnt;\n }\n \n+ssize_t strbuf_write(struct strbuf *sb, FILE *f)\n+{\n+\treturn sb->len ? fwrite(sb->buf, 1, sb->len, f) : 0;\n+}\n+\n+\n #define STRBUF_MAXLINK (2*PATH_MAX)\n \n int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint)\ndiff --git a/strbuf.h b/strbuf.h\nindex 2bf90e7..d4f2aa1 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -387,6 +387,12 @@ extern ssize_t strbuf_read_file(struct strbuf *sb, const char *path, size_t hint\n extern int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint);\n \n /**\n+ * Write the whole content of the strbuf to the stream not stopping at\n+ * NUL bytes.\n+ */\n+extern ssize_t strbuf_write(struct strbuf *sb, FILE *stream);\n+\n+/**\n  * Read a line from a FILE *, overwriting the existing contents\n  * of the strbuf. The second argument specifies the line\n  * terminator character, typically `'\\n'`.\n-- \n2.7.0.rc0.36.g75877e4.dirty\n"},{"id":"279424","messageId":"1456441708-13512-7-git-send-email-sbeller@google.com","threadId":"41512","inReplyTo":"1456441708-13512-1-git-send-email-sbeller@google.com","subject":"[PATCHv18 06/11] run-command: expose default_{start_failure, task_finished}","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-25T23:08:23Z","receivedAt":"2016-02-25T23:08:23Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"We want to reuse the error reporting facilities in a later patch.\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n run-command.c | 18 +++++++++---------\n run-command.h | 19 +++++++++++++++++++\n 2 files changed, 28 insertions(+), 9 deletions(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex 2f8f222..c9b13cf 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -902,10 +902,10 @@ struct parallel_processes {\n \tstruct strbuf buffered_output; /* of finished children */\n };\n \n-static int default_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+int default_start_failure(struct child_process *cp,\n+\t\t\t  struct strbuf *err,\n+\t\t\t  void *pp_cb,\n+\t\t\t  void *pp_task_cb)\n {\n \tint i;\n \n@@ -916,11 +916,11 @@ static int default_start_failure(struct child_process *cp,\n \treturn 0;\n }\n \n-static int default_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+int default_task_finished(int result,\n+\t\t\t  struct child_process *cp,\n+\t\t\t  struct strbuf *err,\n+\t\t\t  void *pp_cb,\n+\t\t\t  void *pp_task_cb)\n {\n \tint i;\n \ndiff --git a/run-command.h b/run-command.h\nindex d5a57f9..a054fa6 100644\n--- a/run-command.h\n+++ b/run-command.h\n@@ -164,6 +164,15 @@ typedef int (*start_failure_fn)(struct child_process *cp,\n \t\t\t\tvoid *pp_task_cb);\n \n /**\n+ * If a command fails to start, then print an error message stating the\n+ * exact command which failed.\n+ */\n+int default_start_failure(struct child_process *cp,\n+\t\t\t  struct strbuf *err,\n+\t\t\t  void *pp_cb,\n+\t\t\t  void *pp_task_cb);\n+\n+/**\n  * This callback is called on every child process that finished processing.\n  *\n  * You must not write to stdout or stderr in this function. Add your\n@@ -184,6 +193,16 @@ typedef int (*task_finished_fn)(int result,\n \t\t\t\tvoid *pp_task_cb);\n \n /**\n+ * If the child process returns with a non zero error code, print\n+ * an error message of the exact command which failed.\n+ */\n+int default_task_finished(int result,\n+\t\t\t  struct child_process *cp,\n+\t\t\t  struct strbuf *err,\n+\t\t\t  void *pp_cb,\n+\t\t\t  void *pp_task_cb);\n+\n+/**\n  * Runs up to n processes at the same time. Whenever a process can be\n  * started, the callback get_next_task_fn is called to obtain the data\n  * required to start another child process.\n-- \n2.7.0.rc0.36.g75877e4.dirty\n"},{"id":"279427","messageId":"1456441708-13512-8-git-send-email-sbeller@google.com","threadId":"41512","inReplyTo":"1456441708-13512-1-git-send-email-sbeller@google.com","subject":"[PATCHv18 07/11] run_processes_parallel: rename parameters for the callbacks","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-25T23:08:24Z","receivedAt":"2016-02-25T23:08:24Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The refs code has a similar pattern of passing around 'struct strbuf *err',\nwhich is strictly used for error reporting. This is not the case here,\nas the strbuf is used to accumulate all the output (whether it is error\nor not) for the user. Rename it to 'out'.\n\nSuggested-by: Jonathan Nieder <jrnieder@gmail.com>\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n run-command.c | 12 ++++++------\n run-command.h | 14 +++++++-------\n 2 files changed, 13 insertions(+), 13 deletions(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex c9b13cf..d2964e1 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -903,22 +903,22 @@ struct parallel_processes {\n };\n \n int default_start_failure(struct child_process *cp,\n-\t\t\t  struct strbuf *err,\n+\t\t\t  struct strbuf *out,\n \t\t\t  void *pp_cb,\n \t\t\t  void *pp_task_cb)\n {\n \tint i;\n \n-\tstrbuf_addstr(err, \"Starting a child failed:\");\n+\tstrbuf_addstr(out, \"Starting a child failed:\");\n \tfor (i = 0; cp->argv[i]; i++)\n-\t\tstrbuf_addf(err, \" %s\", cp->argv[i]);\n+\t\tstrbuf_addf(out, \" %s\", cp->argv[i]);\n \n \treturn 0;\n }\n \n int default_task_finished(int result,\n \t\t\t  struct child_process *cp,\n-\t\t\t  struct strbuf *err,\n+\t\t\t  struct strbuf *out,\n \t\t\t  void *pp_cb,\n \t\t\t  void *pp_task_cb)\n {\n@@ -927,9 +927,9 @@ int default_task_finished(int result,\n \tif (!result)\n \t\treturn 0;\n \n-\tstrbuf_addf(err, \"A child failed with return code %d:\", result);\n+\tstrbuf_addf(out, \"A child failed with return code %d:\", result);\n \tfor (i = 0; cp->argv[i]; i++)\n-\t\tstrbuf_addf(err, \" %s\", cp->argv[i]);\n+\t\tstrbuf_addf(out, \" %s\", cp->argv[i]);\n \n \treturn 0;\n }\ndiff --git a/run-command.h b/run-command.h\nindex a054fa6..2bd8fee 100644\n--- a/run-command.h\n+++ b/run-command.h\n@@ -139,7 +139,7 @@ int in_async(void);\n  * return the negative signal number.\n  */\n typedef int (*get_next_task_fn)(struct child_process *cp,\n-\t\t\t\tstruct strbuf *err,\n+\t\t\t\tstruct strbuf *out,\n \t\t\t\tvoid *pp_cb,\n \t\t\t\tvoid **pp_task_cb);\n \n@@ -148,7 +148,7 @@ typedef int (*get_next_task_fn)(struct child_process *cp,\n  * a new process.\n  *\n  * You must not write to stdout or stderr in this function. Add your\n- * message to the strbuf err instead, which will be printed without\n+ * message to the strbuf out instead, which will be printed without\n  * messing up the output of the other parallel processes.\n  *\n  * pp_cb is the callback cookie as passed into run_processes_parallel,\n@@ -159,7 +159,7 @@ typedef int (*get_next_task_fn)(struct child_process *cp,\n  * the negative signal number.\n  */\n typedef int (*start_failure_fn)(struct child_process *cp,\n-\t\t\t\tstruct strbuf *err,\n+\t\t\t\tstruct strbuf *out,\n \t\t\t\tvoid *pp_cb,\n \t\t\t\tvoid *pp_task_cb);\n \n@@ -168,7 +168,7 @@ typedef int (*start_failure_fn)(struct child_process *cp,\n  * exact command which failed.\n  */\n int default_start_failure(struct child_process *cp,\n-\t\t\t  struct strbuf *err,\n+\t\t\t  struct strbuf *out,\n \t\t\t  void *pp_cb,\n \t\t\t  void *pp_task_cb);\n \n@@ -176,7 +176,7 @@ int default_start_failure(struct child_process *cp,\n  * This callback is called on every child process that finished processing.\n  *\n  * You must not write to stdout or stderr in this function. Add your\n- * message to the strbuf err instead, which will be printed without\n+ * message to the strbuf out instead, which will be printed without\n  * messing up the output of the other parallel processes.\n  *\n  * pp_cb is the callback cookie as passed into run_processes_parallel,\n@@ -188,7 +188,7 @@ int default_start_failure(struct child_process *cp,\n  */\n typedef int (*task_finished_fn)(int result,\n \t\t\t\tstruct child_process *cp,\n-\t\t\t\tstruct strbuf *err,\n+\t\t\t\tstruct strbuf *out,\n \t\t\t\tvoid *pp_cb,\n \t\t\t\tvoid *pp_task_cb);\n \n@@ -198,7 +198,7 @@ typedef int (*task_finished_fn)(int result,\n  */\n int default_task_finished(int result,\n \t\t\t  struct child_process *cp,\n-\t\t\t  struct strbuf *err,\n+\t\t\t  struct strbuf *out,\n \t\t\t  void *pp_cb,\n \t\t\t  void *pp_task_cb);\n \n-- \n2.7.0.rc0.36.g75877e4.dirty\n"},{"id":"279429","messageId":"1456441708-13512-9-git-send-email-sbeller@google.com","threadId":"41512","inReplyTo":"1456441708-13512-1-git-send-email-sbeller@google.com","subject":"[PATCHv18 08/11] run_processes_parallel: correctly terminate callbacks with an LF","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-25T23:08:25Z","receivedAt":"2016-02-25T23:08:25Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"As the strbufs passed around collect all output to the user, and there\nis no post processing involved we need to care about the line ending\nourselves.\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n run-command.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/run-command.c b/run-command.c\nindex d2964e1..6fad42f 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -912,6 +912,7 @@ int default_start_failure(struct child_process *cp,\n \tstrbuf_addstr(out, \"Starting a child failed:\");\n \tfor (i = 0; cp->argv[i]; i++)\n \t\tstrbuf_addf(out, \" %s\", cp->argv[i]);\n+\tstrbuf_addch(out, '\\n');\n \n \treturn 0;\n }\n@@ -930,6 +931,7 @@ int default_task_finished(int result,\n \tstrbuf_addf(out, \"A child failed with return code %d:\", result);\n \tfor (i = 0; cp->argv[i]; i++)\n \t\tstrbuf_addf(out, \" %s\", cp->argv[i]);\n+\tstrbuf_addch(out, '\\n');\n \n \treturn 0;\n }\n-- \n2.7.0.rc0.36.g75877e4.dirty\n"},{"id":"279430","messageId":"1456441708-13512-10-git-send-email-sbeller@google.com","threadId":"41512","inReplyTo":"1456441708-13512-1-git-send-email-sbeller@google.com","subject":"[PATCHv18 09/11] git submodule update: have a dedicated helper for cloning","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-25T23:08:26Z","receivedAt":"2016-02-25T23:08:26Z","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\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n builtin/submodule--helper.c | 248 ++++++++++++++++++++++++++++++++++++++++++++\n git-submodule.sh            |  47 +++------\n 2 files changed, 261 insertions(+), 34 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex f4c3eff..c852554 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -255,6 +255,253 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \treturn 0;\n }\n \n+struct submodule_update_clone {\n+\t/* index into 'list', the list of submodules to look into for cloning */\n+\tint current;\n+\tstruct module_list list;\n+\tunsigned warn_if_uninitialized : 1;\n+\n+\t/* update parameter passed via commandline */\n+\tstruct submodule_update_strategy update;\n+\n+\t/* configuration parameters which are passed on to the children */\n+\tint quiet;\n+\tconst char *reference;\n+\tconst char *depth;\n+\tconst char *recursive_prefix;\n+\tconst char *prefix;\n+\n+\t/* Machine-readable status lines to be consumed by git-submodule.sh */\n+\tstruct string_list projectlines;\n+\n+\t/* If we want to stop as fast as possible and return an error */\n+\tunsigned quickstop : 1;\n+};\n+#define SUBMODULE_UPDATE_CLONE_INIT {0, MODULE_LIST_INIT, 0, \\\n+\tSUBMODULE_UPDATE_STRATEGY_INIT, 0, NULL, NULL, NULL, NULL, \\\n+\tSTRING_LIST_INIT_DUP, 0}\n+\n+/**\n+ * Determine whether 'ce' needs to be cloned. If so, prepare the 'child' to\n+ * run the clone. Returns 1 if 'ce' needs to be cloned, 0 otherwise.\n+ */\n+static int prepare_to_clone_next_submodule(const struct cache_entry *ce,\n+\t\t\t\t\t   struct child_process *child,\n+\t\t\t\t\t   struct submodule_update_clone *suc,\n+\t\t\t\t\t   struct strbuf *out)\n+{\n+\tconst struct submodule *sub = NULL;\n+\tstruct strbuf displaypath_sb = STRBUF_INIT;\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tconst char *displaypath = NULL;\n+\tchar *url = NULL;\n+\tint needs_cloning = 0;\n+\n+\tif (ce_stage(ce)) {\n+\t\tif (suc->recursive_prefix) {\n+\t\t\tstrbuf_addf(out, \"Skipping unmerged submodule %s/%s\\n\",\n+\t\t\t\t    suc->recursive_prefix, ce->name);\n+\t\t} else {\n+\t\t\tstrbuf_addf(out, \"Skipping unmerged submodule %s\\n\",\n+\t\t\t\t    ce->name);\n+\t\t}\n+\t\tgoto cleanup;\n+\t}\n+\n+\tsub = submodule_from_path(null_sha1, ce->name);\n+\n+\tif (suc->recursive_prefix)\n+\t\tdisplaypath = relative_path(suc->recursive_prefix,\n+\t\t\t\t\t    ce->name, &displaypath_sb);\n+\telse\n+\t\tdisplaypath = ce->name;\n+\n+\tif (suc->update.type == SM_UPDATE_NONE\n+\t    || (suc->update.type == SM_UPDATE_UNSPECIFIED\n+\t\t&& sub->update_strategy.type == SM_UPDATE_NONE)) {\n+\t\tstrbuf_addf(out, \"Skipping submodule '%s'\\n\",\n+\t\t\t    displaypath);\n+\t\tgoto cleanup;\n+\t}\n+\n+\t/*\n+\t * Looking up the url in .git/config.\n+\t * We must not fall back to .gitmodules as we only want\n+\t * to process configured submodules.\n+\t */\n+\tstrbuf_reset(&sb);\n+\tstrbuf_addf(&sb, \"submodule.%s.url\", sub->name);\n+\tgit_config_get_string(sb.buf, &url);\n+\tif (!url) {\n+\t\t/*\n+\t\t * Only mention uninitialized submodules when their\n+\t\t * path have been specified\n+\t\t */\n+\t\tif (suc->warn_if_uninitialized)\n+\t\t\tstrbuf_addf(out, _(\"Submodule path '%s' not initialized\\n\"\n+\t\t\t\t    \"Maybe you want to use 'update --init'?\\n\"),\n+\t\t\t\t    displaypath);\n+\t\tgoto cleanup;\n+\t}\n+\n+\tstrbuf_reset(&sb);\n+\tstrbuf_addf(&sb, \"%s/.git\", ce->name);\n+\tneeds_cloning = !file_exists(sb.buf);\n+\n+\tstrbuf_reset(&sb);\n+\tstrbuf_addf(&sb, \"%06o %s %d %d\\t%s\\n\", ce->ce_mode,\n+\t\t\tsha1_to_hex(ce->sha1), ce_stage(ce),\n+\t\t\tneeds_cloning, ce->name);\n+\tstring_list_append(&suc->projectlines, sb.buf);\n+\n+\tif (!needs_cloning)\n+\t\tgoto cleanup;\n+\n+\tchild->git_cmd = 1;\n+\tchild->no_stdin = 1;\n+\tchild->stdout_to_stderr = 1;\n+\tchild->err = -1;\n+\targv_array_push(&child->args, \"submodule--helper\");\n+\targv_array_push(&child->args, \"clone\");\n+\tif (suc->quiet)\n+\t\targv_array_push(&child->args, \"--quiet\");\n+\tif (suc->prefix)\n+\t\targv_array_pushl(&child->args, \"--prefix\", suc->prefix, NULL);\n+\targv_array_pushl(&child->args, \"--path\", sub->path, NULL);\n+\targv_array_pushl(&child->args, \"--name\", sub->name, NULL);\n+\targv_array_pushl(&child->args, \"--url\", url, NULL);\n+\tif (suc->reference)\n+\t\targv_array_push(&child->args, suc->reference);\n+\tif (suc->depth)\n+\t\targv_array_push(&child->args, suc->depth);\n+\n+cleanup:\n+\tfree(url);\n+\tstrbuf_reset(&displaypath_sb);\n+\tstrbuf_reset(&sb);\n+\n+\treturn needs_cloning;\n+}\n+\n+static int update_clone_get_next_task(struct child_process *child,\n+\t\t\t\t      struct strbuf *err,\n+\t\t\t\t      void *suc_cb,\n+\t\t\t\t      void **void_task_cb)\n+{\n+\tstruct submodule_update_clone *suc = suc_cb;\n+\n+\tfor (; suc->current < suc->list.nr; suc->current++) {\n+\t\tconst struct cache_entry *ce = suc->list.entries[suc->current];\n+\t\tif (prepare_to_clone_next_submodule(ce, child, suc, err)) {\n+\t\t\tsuc->current++;\n+\t\t\treturn 1;\n+\t\t}\n+\t}\n+\treturn 0;\n+}\n+\n+static int update_clone_start_failure(struct child_process *child,\n+\t\t\t\t      struct strbuf *err,\n+\t\t\t\t      void *suc_cb,\n+\t\t\t\t      void *void_task_cb)\n+{\n+\tstruct submodule_update_clone *suc = suc_cb;\n+\n+\tdefault_start_failure(child, err, suc_cb, void_task_cb);\n+\tsuc->quickstop = 1;\n+\n+\treturn 1;\n+}\n+\n+static int update_clone_task_finished(int result,\n+\t\t\t\t      struct child_process *child,\n+\t\t\t\t      struct strbuf *err,\n+\t\t\t\t      void *suc_cb,\n+\t\t\t\t      void *void_task_cb)\n+{\n+\tstruct submodule_update_clone *suc = suc_cb;\n+\n+\tif (!result)\n+\t\treturn 0;\n+\n+\tdefault_task_finished(result, child, err, suc_cb, void_task_cb);\n+\tsuc->quickstop = 1;\n+\n+\treturn 1;\n+}\n+\n+static int update_clone(int argc, const char **argv, const char *prefix)\n+{\n+\tconst char *update = NULL;\n+\tstruct string_list_item *item;\n+\tstruct pathspec pathspec;\n+\tstruct submodule_update_clone suc = SUBMODULE_UPDATE_CLONE_INIT;\n+\n+\tstruct option module_update_clone_options[] = {\n+\t\tOPT_STRING(0, \"prefix\", &suc.prefix,\n+\t\t\t   N_(\"path\"),\n+\t\t\t   N_(\"path into the working tree\")),\n+\t\tOPT_STRING(0, \"recursive-prefix\", &suc.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\", &update,\n+\t\t\t   N_(\"string\"),\n+\t\t\t   N_(\"rebase, merge, checkout or none\")),\n+\t\tOPT_STRING(0, \"reference\", &suc.reference, N_(\"repo\"),\n+\t\t\t   N_(\"reference repository\")),\n+\t\tOPT_STRING(0, \"depth\", &suc.depth, \"<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(&suc.quiet, N_(\"don'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 update_clone [--prefix=<path>] [<path>...]\"),\n+\t\tNULL\n+\t};\n+\n+\targc = parse_options(argc, argv, suc.prefix, module_update_clone_options,\n+\t\t\t     git_submodule_helper_usage, 0);\n+\n+\tif (update)\n+\t\tif (parse_submodule_update_strategy(update, &suc.update) < 0)\n+\t\t\tdie(_(\"bad value for update parameter\"));\n+\n+\tif (module_list_compute(argc, argv, suc.prefix, &pathspec, &suc.list) < 0)\n+\t\treturn 1;\n+\n+\tif (pathspec.nr)\n+\t\tsuc.warn_if_uninitialized = 1;\n+\n+\t/* Overlay the parsed .gitmodules file with .git/config */\n+\tgitmodules_config();\n+\tgit_config(submodule_config, NULL);\n+\n+\trun_processes_parallel(1,\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       &suc);\n+\n+\t/*\n+\t * We saved the output and put it out all at once now.\n+\t * That means:\n+\t * - the listener does not have to interleave their (checkout)\n+\t *   work with our fetching.  The writes involved in a\n+\t *   checkout involve more straightforward sequential I/O.\n+\t * - the listener can avoid doing any work if fetching failed.\n+\t */\n+\tif (suc.quickstop)\n+\t\treturn 1;\n+\n+\tfor_each_string_list_item(item, &suc.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 +511,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 9ee86d4..a6a82d2 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -664,17 +664,20 @@ cmd_update()\n \t\tcmd_init \"--\" \"$@\" || return\n \tfi\n \n-\tcloned_modules=\n-\tgit submodule--helper list --prefix \"$wt_prefix\" \"$@\" | {\n+\t{\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\"$@\" || echo \"#unmatched\"\n+\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 +694,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 >&2 \"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 >&2 \"$(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 +737,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)\n-- \n2.7.0.rc0.36.g75877e4.dirty\n"},{"id":"279428","messageId":"1456441708-13512-11-git-send-email-sbeller@google.com","threadId":"41512","inReplyTo":"1456441708-13512-1-git-send-email-sbeller@google.com","subject":"[PATCHv18 10/11] submodule update: expose parallelism to the user","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-25T23:08:27Z","receivedAt":"2016-02-25T23:08:27Z","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\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n Documentation/git-submodule.txt |  7 ++++++-\n builtin/submodule--helper.c     |  8 +++++++-\n git-submodule.sh                |  9 +++++++++\n t/t7406-submodule-update.sh     | 12 ++++++++++++\n 4 files changed, 34 insertions(+), 2 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] [--] <path>...\n 'git submodule' [--quiet] update [--init] [--remote] [-N|--no-fetch]\n \t      [-f|--force] [--rebase|--merge] [--reference <repository>]\n-\t      [--depth <depth>] [--recursive] [--] [<path>...]\n+\t      [--depth <depth>] [--recursive] [--jobs <n>] [--] [<path>...]\n 'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) <n>]\n \t      [commit] [--] [<path>...]\n 'git submodule' [--quiet] foreach [--recursive] <command>\n@@ -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>::\n+--jobs <n>::\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 <path>...::\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 c852554..882aeca 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 static int update_clone(int argc, const char **argv, const char *prefix)\n {\n \tconst char *update = NULL;\n+\tint max_jobs = -1;\n \tstruct string_list_item *item;\n \tstruct pathspec pathspec;\n \tstruct submodule_update_clone suc = SUBMODULE_UPDATE_CLONE_INIT;\n@@ -453,6 +454,8 @@ static int update_clone(int argc, const char **argv, const char *prefix)\n \t\tOPT_STRING(0, \"depth\", &suc.depth, \"<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(&suc.quiet, N_(\"don't print cloning progress\")),\n \t\tOPT_END()\n \t};\n@@ -479,7 +482,10 @@ static int update_clone(int argc, const char **argv, const char *prefix)\n \tgitmodules_config();\n \tgit_config(submodule_config, NULL);\n \n-\trun_processes_parallel(1,\n+\tif (max_jobs < 0)\n+\t\tmax_jobs = parallel_submodules();\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,\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex a6a82d2..86018ee 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@@ -671,6 +679,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\"$@\" || echo \"#unmatched\"\n \t} | {\n \terr=\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.36.g75877e4.dirty\n"},{"id":"279425","messageId":"1456441708-13512-12-git-send-email-sbeller@google.com","threadId":"41512","inReplyTo":"1456441708-13512-1-git-send-email-sbeller@google.com","subject":"[PATCHv18 11/11] clone: allow an explicit argument for parallel submodule clones","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-25T23:08:28Z","receivedAt":"2016-02-25T23:08:28Z","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\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\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 <name>] [-b <name>] [-u <upload-pack>] [--reference <repository>]\n \t  [--dissociate] [--separate-git-dir <git dir>]\n \t  [--depth <depth>] [--[no-]single-branch]\n-\t  [--recursive | --recurse-submodules] [--] <repository>\n+\t  [--recursive | --recurse-submodules] [--jobs <n>] [--] <repository>\n \t  [<directory>]\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>::\n+--jobs <n>::\n+\tThe number of submodules fetched at the same time.\n+\tDefaults to the `submodule.fetchJobs` option.\n \n <repository>::\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.36.g75877e4.dirty\n"},{"id":"279432","messageId":"CAGZ79kb+znOJNpzJ=jLCxN+PpJ16vbMXZivxsN9bJypRLx2X+Q@mail.gmail.com","threadId":"41512","inReplyTo":"1456441708-13512-1-git-send-email-sbeller@google.com","subject":"Re: [PATCHv18 00/11] Expose","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-25T23:11:12Z","receivedAt":"2016-02-25T23:11:12Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Feb 25, 2016 at 3:08 PM, Stefan Beller <sbeller@google.com> wrote:\nSubject: Expose\n\n...  submodule parallelism to the user\n\n\nDoes git-send-email cache information after invoking it? i.e. Am I not supposed\nto edit the patch files after invoking git send-mail, before\nconfirming to send them\nout actually?\n\nThanks,\nStefan\n"},{"id":"279436","messageId":"20160225231904.GT28749@google.com","threadId":"41512","inReplyTo":"1456441708-13512-1-git-send-email-sbeller@google.com","subject":"Re: [PATCHv18 00/11] Expose","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2016-02-25T23:19:04Z","receivedAt":"2016-02-25T23:19:04Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Stefan Beller wrote:\n\n> --- a/builtin/submodule--helper.c\n> +++ b/builtin/submodule--helper.c\n> @@ -465,14 +465,14 @@ static int update_clone(int argc, const char **argv, const char *prefix)\n>  \t\tNULL\n>  \t};\n>  \n> -\targc = parse_options(argc, argv, prefix, module_update_clone_options,\n> +\targc = parse_options(argc, argv, suc.prefix, module_update_clone_options,\n>  \t\t\t     git_submodule_helper_usage, 0);\n\nI would have expected this to use 'parse_options(argc, argv, prefix, ...' since\nI wouldn't expect a command-specific --prefix= parameter to affect the\ninterpretation of relative filenames in other parameters.\n\nNow that I look around more, it seems that other submodule--helper subcommands\nhave the same strange behavior of overwriting the 'prefix' var.  So I take\nback my suggestion of using a different variable --- that can be addressed in a\nseparate patch that deals with them all at once.\n\nSorry to flip-flop like this,\nJonathan\n"},{"id":"279437","messageId":"CAGZ79kYUkgbHSXYmOXRKhju1QQwiw9EU+VoO-AMZL_nMCf0NNg@mail.gmail.com","threadId":"41512","inReplyTo":"20160225231904.GT28749@google.com","subject":"Re: [PATCHv18 00/11] Expose","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-25T23:25:07Z","receivedAt":"2016-02-25T23:25:07Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Feb 25, 2016 at 3:19 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Stefan Beller wrote:\n>\n>> --- a/builtin/submodule--helper.c\n>> +++ b/builtin/submodule--helper.c\n>> @@ -465,14 +465,14 @@ static int update_clone(int argc, const char **argv, const char *prefix)\n>>               NULL\n>>       };\n>>\n>> -     argc = parse_options(argc, argv, prefix, module_update_clone_options,\n>> +     argc = parse_options(argc, argv, suc.prefix, module_update_clone_options,\n>>                            git_submodule_helper_usage, 0);\n>\n> I would have expected this to use 'parse_options(argc, argv, prefix, ...' since\n> I wouldn't expect a command-specific --prefix= parameter to affect the\n> interpretation of relative filenames in other parameters.\n>\n> Now that I look around more, it seems that other submodule--helper subcommands\n> have the same strange behavior of overwriting the 'prefix' var.  So I take\n> back my suggestion of using a different variable --- that can be addressed in a\n> separate patch that deals with them all at once.\n>\n> Sorry to flip-flop like this,\n> Jonathan\n\nI plan to fix that up in all the submodule--helper commands once this\nlong running series is in,\nbut for now we want to keep it consistently awkward?\n\nThe way all submodule helper commands are currently \"working\" is\n\n    (in git submodule.h:)\n    wt_prefix=$(git rev-parse --show=prefix)\n    cd_to_toplevel\n    git submodule--helper <command> --prefix $wt_prefix\n\nwhich makes the command a bit awkward with having the prefix in the\nsignature and the\nprefix as an option. the prefix as given in the signature ought to be\nempty for now and\nwe always rely on the prefix option.\n\nI plan to replace that to be\n\n    git -C $wt_prefix submodule--helper <command>\n\nwhich then doesn't carry a prefix option, but can rely on the prefix\nfrom its function\nsignature.\n\nSo I flip-flop that change of 'prefix' back to the way all submodule\nhelper operate for\nnow and then send a cleanup once this series is done.\n\nThanks,\nStefan\n"},{"id":"279438","messageId":"20160225232540.GU28749@google.com","threadId":"41512","inReplyTo":"1456441708-13512-1-git-send-email-sbeller@google.com","subject":"Re: [PATCHv18 00/11] Expose","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2016-02-25T23:25:40Z","receivedAt":"2016-02-25T23:25:40Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Stefan Beller wrote:\n\n> Interdiff to v17:\n\nExcept for the bit about 'prefix',\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\n"},{"id":"279439","messageId":"20160225233513.GV28749@google.com","threadId":"41512","inReplyTo":"CAGZ79kYUkgbHSXYmOXRKhju1QQwiw9EU+VoO-AMZL_nMCf0NNg@mail.gmail.com","subject":"Re: [PATCHv18 00/11] Expose","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2016-02-25T23:35:13Z","receivedAt":"2016-02-25T23:35:13Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Stefan Beller wrote:\n> On Thu, Feb 25, 2016 at 3:19 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n\n> > Now that I look around more, it seems that other submodule--helper subcommands\n> > have the same strange behavior of overwriting the 'prefix' var.  So I take\n> > back my suggestion of using a different variable --- that can be addressed in a\n> > separate patch that deals with them all at once.\n[...]\n> I plan to fix that up in all the submodule--helper commands once this\n> long running series is in,\n> but for now we want to keep it consistently awkward?\n\nExactly.\n\n> I plan to replace that to be\n>\n>     git -C $wt_prefix submodule--helper <command>\n>\n> which then doesn't carry a prefix option, but can rely on the prefix from its function\n> signature.\n\nInteresting --- git sets a GIT_PREFIX environment variable (in the\nsame spirit as GIT_WORK_TREE) but doesn't read it.  Using -C is\nprobably the simplest way to go.\n\nThanks,\nJonathan\n"},{"id":"279441","messageId":"CAPc5daWbkNXp8T4U2tiYftB4kSOjf9Cv1fgmbYbpuoKdJPRHGA@mail.gmail.com","threadId":"41512","inReplyTo":"20160225233513.GV28749@google.com","subject":"Re: [PATCHv18 00/11] Expose","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-02-25T23:39:52Z","receivedAt":"2016-02-25T23:39:52Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"On Thu, Feb 25, 2016 at 3:35 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Interesting --- git sets a GIT_PREFIX environment variable (in the\n> same spirit as GIT_WORK_TREE) but doesn't read it.\n\nI think that is because it is meant for helping end-user scripts that\nare called by us;\nthey would not know where the end user started the process in, as one\nof the first\nthings we do is to find the top of the working tree and go there.\n"},{"id":"279447","messageId":"1456444119-6934-1-git-send-email-sbeller@google.com","threadId":"41512","inReplyTo":"CAPc5daWbkNXp8T4U2tiYftB4kSOjf9Cv1fgmbYbpuoKdJPRHGA@mail.gmail.com","subject":"[PATCHv19 00/11] Expose submodule parallelism to the user","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-25T23:48:28Z","receivedAt":"2016-02-25T23:48:28Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Thanks Jonathan (and all the other cc'd reviewers) for bearing\nyet another revision!\n\nI changed the prefix handling back again as it is more consistent.\n\nOnce this series is landed, I plan to refactor the prefix handling for \nall the submodule--helper commands.\n\nThanks,\nStefan\n\nInterdiff to v18:\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 882aeca..0272c98 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -439,7 +439,7 @@ static int update_clone(int argc, const char **argv, const char *prefix)\n \tstruct submodule_update_clone suc = SUBMODULE_UPDATE_CLONE_INIT;\n \n \tstruct option module_update_clone_options[] = {\n-\t\tOPT_STRING(0, \"prefix\", &suc.prefix,\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\", &suc.recursive_prefix,\n@@ -464,15 +464,16 @@ static int update_clone(int argc, const char **argv, const char *prefix)\n \t\tN_(\"git submodule--helper update_clone [--prefix=<path>] [<path>...]\"),\n \t\tNULL\n \t};\n+\tsuc.prefix = prefix;\n \n-\targc = parse_options(argc, argv, suc.prefix, module_update_clone_options,\n+\targc = parse_options(argc, argv, prefix, module_update_clone_options,\n \t\t\t     git_submodule_helper_usage, 0);\n \n \tif (update)\n \t\tif (parse_submodule_update_strategy(update, &suc.update) < 0)\n \t\t\tdie(_(\"bad value for update parameter\"));\n \n-\tif (module_list_compute(argc, argv, suc.prefix, &pathspec, &suc.list) < 0)\n+\tif (module_list_compute(argc, argv, prefix, &pathspec, &suc.list) < 0)\n \t\treturn 1;\n \n \tif (pathspec.nr)\n\n\nStefan Beller (11):\n  submodule-config: keep update strategy around\n  submodule-config: drop check against NULL\n  fetching submodules: respect `submodule.fetchJobs` config option\n  submodule update: direct error message to stderr\n  run_processes_parallel: treat output of children as byte array\n  run-command: expose default_{start_failure, task_finished}\n  run_processes_parallel: rename parameters for the callbacks\n  run_processes_parallel: correctly terminate callbacks with an LF\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     | 255 ++++++++++++++++++++++++++++++++++++++++\n git-submodule.sh                |  56 ++++-----\n run-command.c                   |  36 +++---\n run-command.h                   |  29 ++++-\n strbuf.c                        |   6 +\n strbuf.h                        |   6 +\n submodule-config.c              |  19 ++-\n submodule-config.h              |   2 +\n submodule.c                     |  37 +++++-\n submodule.h                     |  18 +++\n t/t5526-fetch-submodules.sh     |  14 +++\n t/t7400-submodule-basic.sh      |   4 +-\n t/t7406-submodule-update.sh     |  27 +++++\n 18 files changed, 478 insertions(+), 71 deletions(-)\n\n-- \n2.7.0.rc0.36.g75877e4.dirty\n"},{"id":"279455","messageId":"1456444119-6934-2-git-send-email-sbeller@google.com","threadId":"41512","inReplyTo":"1456444119-6934-1-git-send-email-sbeller@google.com","subject":"[PATCHv19 01/11] submodule-config: keep update strategy around","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-25T23:48:29Z","receivedAt":"2016-02-25T23:48:29Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Currently submodule.<name>.update is only handled by git-submodule.sh.\nC code will start to need to make use of that value as more of the\nfunctionality of git-submodule.sh moves into library code in C.\n\nAdd the update field to 'struct submodule' and populate it so it can\nbe read as sm->update or from sm->update_command.\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n submodule-config.c | 13 +++++++++++++\n submodule-config.h |  2 ++\n submodule.c        | 21 +++++++++++++++++++++\n submodule.h        | 16 ++++++++++++++++\n 4 files changed, 52 insertions(+)\n\ndiff --git a/submodule-config.c b/submodule-config.c\nindex afe0ea8..a5cd2ee 100644\n--- a/submodule-config.c\n+++ b/submodule-config.c\n@@ -59,6 +59,7 @@ static void free_one_config(struct submodule_entry *entry)\n {\n \tfree((void *) entry->config->path);\n \tfree((void *) entry->config->name);\n+\tfree((void *) entry->config->update_strategy.command);\n \tfree(entry->config);\n }\n \n@@ -194,6 +195,8 @@ static struct submodule *lookup_or_create_by_name(struct submodule_cache *cache,\n \n \tsubmodule->path = NULL;\n \tsubmodule->url = NULL;\n+\tsubmodule->update_strategy.type = SM_UPDATE_UNSPECIFIED;\n+\tsubmodule->update_strategy.command = NULL;\n \tsubmodule->fetch_recurse = RECURSE_SUBMODULES_NONE;\n \tsubmodule->ignore = NULL;\n \n@@ -311,6 +314,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 &&\n+\t\t\t submodule->update_strategy.type != SM_UPDATE_UNSPECIFIED)\n+\t\t\twarn_multiple_config(me->commit_sha1, submodule->name,\n+\t\t\t\t\t     \"update\");\n+\t\telse if (parse_submodule_update_strategy(value,\n+\t\t\t &submodule->update_strategy) < 0)\n+\t\t\t\tdie(_(\"invalid value for %s\"), var);\n \t}\n \n \tstrbuf_release(&name);\ndiff --git a/submodule-config.h b/submodule-config.h\nindex 9061e4e..092ebfc 100644\n--- a/submodule-config.h\n+++ b/submodule-config.h\n@@ -2,6 +2,7 @@\n #define SUBMODULE_CONFIG_CACHE_H\n \n #include \"hashmap.h\"\n+#include \"submodule.h\"\n #include \"strbuf.h\"\n \n /*\n@@ -14,6 +15,7 @@ struct submodule {\n \tconst char *url;\n \tint fetch_recurse;\n \tconst char *ignore;\n+\tstruct submodule_update_strategy update_strategy;\n \t/* the sha1 blob id of the responsible .gitmodules file */\n \tunsigned char gitmodules_sha1[20];\n };\ndiff --git a/submodule.c b/submodule.c\nindex b83939c..b38dd51 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -210,6 +210,27 @@ void gitmodules_config(void)\n \t}\n }\n \n+int parse_submodule_update_strategy(const char *value,\n+\t\tstruct submodule_update_strategy *dst)\n+{\n+\tfree((void*)dst->command);\n+\tdst->command = NULL;\n+\tif (!strcmp(value, \"none\"))\n+\t\tdst->type = SM_UPDATE_NONE;\n+\telse if (!strcmp(value, \"checkout\"))\n+\t\tdst->type = SM_UPDATE_CHECKOUT;\n+\telse if (!strcmp(value, \"rebase\"))\n+\t\tdst->type = SM_UPDATE_REBASE;\n+\telse if (!strcmp(value, \"merge\"))\n+\t\tdst->type = SM_UPDATE_MERGE;\n+\telse if (skip_prefix(value, \"!\", &value)) {\n+\t\tdst->type = SM_UPDATE_COMMAND;\n+\t\tdst->command = xstrdup(value);\n+\t} else\n+\t\treturn -1;\n+\treturn 0;\n+}\n+\n void handle_ignore_submodules_arg(struct diff_options *diffopt,\n \t\t\t\t  const char *arg)\n {\ndiff --git a/submodule.h b/submodule.h\nindex cbc0003..3464500 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -13,6 +13,20 @@ enum {\n \tRECURSE_SUBMODULES_ON = 2\n };\n \n+enum submodule_update_type {\n+\tSM_UPDATE_UNSPECIFIED = 0,\n+\tSM_UPDATE_CHECKOUT,\n+\tSM_UPDATE_REBASE,\n+\tSM_UPDATE_MERGE,\n+\tSM_UPDATE_NONE,\n+\tSM_UPDATE_COMMAND\n+};\n+\n+struct submodule_update_strategy {\n+\tenum submodule_update_type type;\n+\tconst char *command;\n+};\n+\n int is_staging_gitmodules_ok(void);\n int update_path_in_gitmodules(const char *oldpath, const char *newpath);\n int remove_path_from_gitmodules(const char *path);\n@@ -21,6 +35,8 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,\n \t\tconst char *path);\n int submodule_config(const char *var, const char *value, void *cb);\n void gitmodules_config(void);\n+int parse_submodule_update_strategy(const char *value,\n+\t\tstruct submodule_update_strategy *dst);\n void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *);\n void show_submodule_summary(FILE *f, const char *path,\n \t\tconst char *line_prefix,\n-- \n2.7.0.rc0.36.g75877e4.dirty\n"},{"id":"279444","messageId":"1456444119-6934-3-git-send-email-sbeller@google.com","threadId":"41512","inReplyTo":"1456444119-6934-1-git-send-email-sbeller@google.com","subject":"[PATCHv19 02/11] submodule-config: drop check against NULL","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-25T23:48:30Z","receivedAt":"2016-02-25T23:48:30Z","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\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n submodule-config.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/submodule-config.c b/submodule-config.c\nindex a5cd2ee..9fa2269 100644\n--- a/submodule-config.c\n+++ b/submodule-config.c\n@@ -267,7 +267,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@@ -291,7 +291,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@@ -307,7 +307,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-- \n2.7.0.rc0.36.g75877e4.dirty\n"},{"id":"279454","messageId":"1456444119-6934-4-git-send-email-sbeller@google.com","threadId":"41512","inReplyTo":"1456444119-6934-1-git-send-email-sbeller@google.com","subject":"[PATCHv19 03/11] fetching submodules: respect `submodule.fetchJobs` config option","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-25T23:48:31Z","receivedAt":"2016-02-25T23:48:31Z","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\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n Documentation/config.txt    |  6 ++++++\n builtin/fetch.c             |  2 +-\n submodule.c                 | 16 +++++++++++++++-\n submodule.h                 |  2 ++\n t/t5526-fetch-submodules.sh | 14 ++++++++++++++\n 5 files changed, 38 insertions(+), 2 deletions(-)\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.<name>.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=<value>\" 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.c b/submodule.c\nindex b38dd51..051f722 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -15,6 +15,7 @@\n #include \"thread-utils.h\"\n \n static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND;\n+static int parallel_jobs = 1;\n static struct string_list changed_submodule_paths;\n static int initialized_fetch_ref_tips;\n static struct sha1_array ref_tips_before_fetch;\n@@ -169,7 +170,12 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,\n \n int submodule_config(const char *var, const char *value, void *cb)\n {\n-\tif (starts_with(var, \"submodule.\"))\n+\tif (!strcmp(var, \"submodule.fetchjobs\")) {\n+\t\tparallel_jobs = git_config_int(var, value);\n+\t\tif (parallel_jobs < 0)\n+\t\t\tdie(_(\"negative values not allowed for submodule.fetchJobs\"));\n+\t\treturn 0;\n+\t} else if (starts_with(var, \"submodule.\"))\n \t\treturn parse_submodule_config_option(var, value);\n \telse if (!strcmp(var, \"fetch.recursesubmodules\")) {\n \t\tconfig_fetch_recurse_submodules = parse_fetch_recurse_submodules_arg(var, value);\n@@ -772,6 +778,9 @@ 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 = parallel_jobs;\n+\n \tcalculate_changed_submodule_paths();\n \trun_processes_parallel(max_parallel_jobs,\n \t\t\t       get_next_submodule,\n@@ -1118,3 +1127,8 @@ void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir)\n \tstrbuf_release(&rel_path);\n \tfree((void *)real_work_tree);\n }\n+\n+int parallel_submodules(void)\n+{\n+\treturn parallel_jobs;\n+}\ndiff --git a/submodule.h b/submodule.h\nindex 3464500..3166608 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -26,6 +26,7 @@ struct submodule_update_strategy {\n \tenum submodule_update_type type;\n \tconst char *command;\n };\n+#define SUBMODULE_UPDATE_STRATEGY_INIT {SM_UPDATE_UNSPECIFIED, NULL}\n \n int is_staging_gitmodules_ok(void);\n int update_path_in_gitmodules(const char *oldpath, const char *newpath);\n@@ -57,5 +58,6 @@ int find_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_nam\n \t\tstruct string_list *needs_pushing);\n int push_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_name);\n void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);\n+int parallel_submodules(void);\n \n #endif\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.36.g75877e4.dirty\n"},{"id":"279448","messageId":"1456444119-6934-5-git-send-email-sbeller@google.com","threadId":"41512","inReplyTo":"1456444119-6934-1-git-send-email-sbeller@google.com","subject":"[PATCHv19 04/11] submodule update: direct error message to stderr","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-25T23:48:32Z","receivedAt":"2016-02-25T23:48:32Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Reroute the error message for specified but initialized submodules\nto stderr instead of stdout.\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n git-submodule.sh           | 4 ++--\n t/t7400-submodule-basic.sh | 4 ++--\n 2 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 9bc5c5f..9ee86d4 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -693,7 +693,7 @@ cmd_update()\n \n \t\tif test \"$update_module\" = \"none\"\n \t\tthen\n-\t\t\techo \"Skipping submodule '$displaypath'\"\n+\t\t\techo >&2 \"Skipping submodule '$displaypath'\"\n \t\t\tcontinue\n \t\tfi\n \n@@ -702,7 +702,7 @@ cmd_update()\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+\t\t\tsay >&2 \"$(eval_gettext \"Submodule path '\\$displaypath' not initialized\n Maybe you want to use 'update --init'?\")\"\n \t\t\tcontinue\n \t\tfi\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.36.g75877e4.dirty\n"},{"id":"279450","messageId":"1456444119-6934-6-git-send-email-sbeller@google.com","threadId":"41512","inReplyTo":"1456444119-6934-1-git-send-email-sbeller@google.com","subject":"[PATCHv19 05/11] run_processes_parallel: treat output of children as byte array","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-25T23:48:33Z","receivedAt":"2016-02-25T23:48:33Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"We do not want the output to be interrupted by a NUL byte, so we\ncannot use raw fputs. Introduce strbuf_write to avoid having long\narguments in run-command.c.\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n run-command.c | 8 ++++----\n strbuf.c      | 6 ++++++\n strbuf.h      | 6 ++++++\n 3 files changed, 16 insertions(+), 4 deletions(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex 51fd72c..2f8f222 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -1011,7 +1011,7 @@ static void pp_cleanup(struct parallel_processes *pp)\n \t * When get_next_task added messages to the buffer in its last\n \t * iteration, the buffered output is non empty.\n \t */\n-\tfputs(pp->buffered_output.buf, stderr);\n+\tstrbuf_write(&pp->buffered_output, stderr);\n \tstrbuf_release(&pp->buffered_output);\n \n \tsigchain_pop_common();\n@@ -1097,7 +1097,7 @@ static void pp_output(struct parallel_processes *pp)\n \tint i = pp->output_owner;\n \tif (pp->children[i].state == GIT_CP_WORKING &&\n \t    pp->children[i].err.len) {\n-\t\tfputs(pp->children[i].err.buf, stderr);\n+\t\tstrbuf_write(&pp->children[i].err, stderr);\n \t\tstrbuf_reset(&pp->children[i].err);\n \t}\n }\n@@ -1135,11 +1135,11 @@ static int pp_collect_finished(struct parallel_processes *pp)\n \t\t\tstrbuf_addbuf(&pp->buffered_output, &pp->children[i].err);\n \t\t\tstrbuf_reset(&pp->children[i].err);\n \t\t} else {\n-\t\t\tfputs(pp->children[i].err.buf, stderr);\n+\t\t\tstrbuf_write(&pp->children[i].err, stderr);\n \t\t\tstrbuf_reset(&pp->children[i].err);\n \n \t\t\t/* Output all other finished child processes */\n-\t\t\tfputs(pp->buffered_output.buf, stderr);\n+\t\t\tstrbuf_write(&pp->buffered_output, stderr);\n \t\t\tstrbuf_reset(&pp->buffered_output);\n \n \t\t\t/*\ndiff --git a/strbuf.c b/strbuf.c\nindex 38686ff..5f6da82 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -395,6 +395,12 @@ ssize_t strbuf_read_once(struct strbuf *sb, int fd, size_t hint)\n \treturn cnt;\n }\n \n+ssize_t strbuf_write(struct strbuf *sb, FILE *f)\n+{\n+\treturn sb->len ? fwrite(sb->buf, 1, sb->len, f) : 0;\n+}\n+\n+\n #define STRBUF_MAXLINK (2*PATH_MAX)\n \n int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint)\ndiff --git a/strbuf.h b/strbuf.h\nindex 2bf90e7..d4f2aa1 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -387,6 +387,12 @@ extern ssize_t strbuf_read_file(struct strbuf *sb, const char *path, size_t hint\n extern int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint);\n \n /**\n+ * Write the whole content of the strbuf to the stream not stopping at\n+ * NUL bytes.\n+ */\n+extern ssize_t strbuf_write(struct strbuf *sb, FILE *stream);\n+\n+/**\n  * Read a line from a FILE *, overwriting the existing contents\n  * of the strbuf. The second argument specifies the line\n  * terminator character, typically `'\\n'`.\n-- \n2.7.0.rc0.36.g75877e4.dirty\n"},{"id":"279445","messageId":"1456444119-6934-7-git-send-email-sbeller@google.com","threadId":"41512","inReplyTo":"1456444119-6934-1-git-send-email-sbeller@google.com","subject":"[PATCHv19 06/11] run-command: expose default_{start_failure, task_finished}","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-25T23:48:34Z","receivedAt":"2016-02-25T23:48:34Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"We want to reuse the error reporting facilities in a later patch.\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n run-command.c | 18 +++++++++---------\n run-command.h | 19 +++++++++++++++++++\n 2 files changed, 28 insertions(+), 9 deletions(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex 2f8f222..c9b13cf 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -902,10 +902,10 @@ struct parallel_processes {\n \tstruct strbuf buffered_output; /* of finished children */\n };\n \n-static int default_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+int default_start_failure(struct child_process *cp,\n+\t\t\t  struct strbuf *err,\n+\t\t\t  void *pp_cb,\n+\t\t\t  void *pp_task_cb)\n {\n \tint i;\n \n@@ -916,11 +916,11 @@ static int default_start_failure(struct child_process *cp,\n \treturn 0;\n }\n \n-static int default_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+int default_task_finished(int result,\n+\t\t\t  struct child_process *cp,\n+\t\t\t  struct strbuf *err,\n+\t\t\t  void *pp_cb,\n+\t\t\t  void *pp_task_cb)\n {\n \tint i;\n \ndiff --git a/run-command.h b/run-command.h\nindex d5a57f9..a054fa6 100644\n--- a/run-command.h\n+++ b/run-command.h\n@@ -164,6 +164,15 @@ typedef int (*start_failure_fn)(struct child_process *cp,\n \t\t\t\tvoid *pp_task_cb);\n \n /**\n+ * If a command fails to start, then print an error message stating the\n+ * exact command which failed.\n+ */\n+int default_start_failure(struct child_process *cp,\n+\t\t\t  struct strbuf *err,\n+\t\t\t  void *pp_cb,\n+\t\t\t  void *pp_task_cb);\n+\n+/**\n  * This callback is called on every child process that finished processing.\n  *\n  * You must not write to stdout or stderr in this function. Add your\n@@ -184,6 +193,16 @@ typedef int (*task_finished_fn)(int result,\n \t\t\t\tvoid *pp_task_cb);\n \n /**\n+ * If the child process returns with a non zero error code, print\n+ * an error message of the exact command which failed.\n+ */\n+int default_task_finished(int result,\n+\t\t\t  struct child_process *cp,\n+\t\t\t  struct strbuf *err,\n+\t\t\t  void *pp_cb,\n+\t\t\t  void *pp_task_cb);\n+\n+/**\n  * Runs up to n processes at the same time. Whenever a process can be\n  * started, the callback get_next_task_fn is called to obtain the data\n  * required to start another child process.\n-- \n2.7.0.rc0.36.g75877e4.dirty\n"},{"id":"279446","messageId":"1456444119-6934-8-git-send-email-sbeller@google.com","threadId":"41512","inReplyTo":"1456444119-6934-1-git-send-email-sbeller@google.com","subject":"[PATCHv19 07/11] run_processes_parallel: rename parameters for the callbacks","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-25T23:48:35Z","receivedAt":"2016-02-25T23:48:35Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The refs code has a similar pattern of passing around 'struct strbuf *err',\nwhich is strictly used for error reporting. This is not the case here,\nas the strbuf is used to accumulate all the output (whether it is error\nor not) for the user. Rename it to 'out'.\n\nSuggested-by: Jonathan Nieder <jrnieder@gmail.com>\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n run-command.c | 12 ++++++------\n run-command.h | 14 +++++++-------\n 2 files changed, 13 insertions(+), 13 deletions(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex c9b13cf..d2964e1 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -903,22 +903,22 @@ struct parallel_processes {\n };\n \n int default_start_failure(struct child_process *cp,\n-\t\t\t  struct strbuf *err,\n+\t\t\t  struct strbuf *out,\n \t\t\t  void *pp_cb,\n \t\t\t  void *pp_task_cb)\n {\n \tint i;\n \n-\tstrbuf_addstr(err, \"Starting a child failed:\");\n+\tstrbuf_addstr(out, \"Starting a child failed:\");\n \tfor (i = 0; cp->argv[i]; i++)\n-\t\tstrbuf_addf(err, \" %s\", cp->argv[i]);\n+\t\tstrbuf_addf(out, \" %s\", cp->argv[i]);\n \n \treturn 0;\n }\n \n int default_task_finished(int result,\n \t\t\t  struct child_process *cp,\n-\t\t\t  struct strbuf *err,\n+\t\t\t  struct strbuf *out,\n \t\t\t  void *pp_cb,\n \t\t\t  void *pp_task_cb)\n {\n@@ -927,9 +927,9 @@ int default_task_finished(int result,\n \tif (!result)\n \t\treturn 0;\n \n-\tstrbuf_addf(err, \"A child failed with return code %d:\", result);\n+\tstrbuf_addf(out, \"A child failed with return code %d:\", result);\n \tfor (i = 0; cp->argv[i]; i++)\n-\t\tstrbuf_addf(err, \" %s\", cp->argv[i]);\n+\t\tstrbuf_addf(out, \" %s\", cp->argv[i]);\n \n \treturn 0;\n }\ndiff --git a/run-command.h b/run-command.h\nindex a054fa6..2bd8fee 100644\n--- a/run-command.h\n+++ b/run-command.h\n@@ -139,7 +139,7 @@ int in_async(void);\n  * return the negative signal number.\n  */\n typedef int (*get_next_task_fn)(struct child_process *cp,\n-\t\t\t\tstruct strbuf *err,\n+\t\t\t\tstruct strbuf *out,\n \t\t\t\tvoid *pp_cb,\n \t\t\t\tvoid **pp_task_cb);\n \n@@ -148,7 +148,7 @@ typedef int (*get_next_task_fn)(struct child_process *cp,\n  * a new process.\n  *\n  * You must not write to stdout or stderr in this function. Add your\n- * message to the strbuf err instead, which will be printed without\n+ * message to the strbuf out instead, which will be printed without\n  * messing up the output of the other parallel processes.\n  *\n  * pp_cb is the callback cookie as passed into run_processes_parallel,\n@@ -159,7 +159,7 @@ typedef int (*get_next_task_fn)(struct child_process *cp,\n  * the negative signal number.\n  */\n typedef int (*start_failure_fn)(struct child_process *cp,\n-\t\t\t\tstruct strbuf *err,\n+\t\t\t\tstruct strbuf *out,\n \t\t\t\tvoid *pp_cb,\n \t\t\t\tvoid *pp_task_cb);\n \n@@ -168,7 +168,7 @@ typedef int (*start_failure_fn)(struct child_process *cp,\n  * exact command which failed.\n  */\n int default_start_failure(struct child_process *cp,\n-\t\t\t  struct strbuf *err,\n+\t\t\t  struct strbuf *out,\n \t\t\t  void *pp_cb,\n \t\t\t  void *pp_task_cb);\n \n@@ -176,7 +176,7 @@ int default_start_failure(struct child_process *cp,\n  * This callback is called on every child process that finished processing.\n  *\n  * You must not write to stdout or stderr in this function. Add your\n- * message to the strbuf err instead, which will be printed without\n+ * message to the strbuf out instead, which will be printed without\n  * messing up the output of the other parallel processes.\n  *\n  * pp_cb is the callback cookie as passed into run_processes_parallel,\n@@ -188,7 +188,7 @@ int default_start_failure(struct child_process *cp,\n  */\n typedef int (*task_finished_fn)(int result,\n \t\t\t\tstruct child_process *cp,\n-\t\t\t\tstruct strbuf *err,\n+\t\t\t\tstruct strbuf *out,\n \t\t\t\tvoid *pp_cb,\n \t\t\t\tvoid *pp_task_cb);\n \n@@ -198,7 +198,7 @@ typedef int (*task_finished_fn)(int result,\n  */\n int default_task_finished(int result,\n \t\t\t  struct child_process *cp,\n-\t\t\t  struct strbuf *err,\n+\t\t\t  struct strbuf *out,\n \t\t\t  void *pp_cb,\n \t\t\t  void *pp_task_cb);\n \n-- \n2.7.0.rc0.36.g75877e4.dirty\n"},{"id":"279449","messageId":"1456444119-6934-9-git-send-email-sbeller@google.com","threadId":"41512","inReplyTo":"1456444119-6934-1-git-send-email-sbeller@google.com","subject":"[PATCHv19 08/11] run_processes_parallel: correctly terminate callbacks with an LF","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-25T23:48:36Z","receivedAt":"2016-02-25T23:48:36Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"As the strbufs passed around collect all output to the user, and there\nis no post processing involved we need to care about the line ending\nourselves.\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n run-command.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/run-command.c b/run-command.c\nindex d2964e1..6fad42f 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -912,6 +912,7 @@ int default_start_failure(struct child_process *cp,\n \tstrbuf_addstr(out, \"Starting a child failed:\");\n \tfor (i = 0; cp->argv[i]; i++)\n \t\tstrbuf_addf(out, \" %s\", cp->argv[i]);\n+\tstrbuf_addch(out, '\\n');\n \n \treturn 0;\n }\n@@ -930,6 +931,7 @@ int default_task_finished(int result,\n \tstrbuf_addf(out, \"A child failed with return code %d:\", result);\n \tfor (i = 0; cp->argv[i]; i++)\n \t\tstrbuf_addf(out, \" %s\", cp->argv[i]);\n+\tstrbuf_addch(out, '\\n');\n \n \treturn 0;\n }\n-- \n2.7.0.rc0.36.g75877e4.dirty\n"},{"id":"279452","messageId":"1456444119-6934-10-git-send-email-sbeller@google.com","threadId":"41512","inReplyTo":"1456444119-6934-1-git-send-email-sbeller@google.com","subject":"[PATCHv19 09/11] git submodule update: have a dedicated helper for cloning","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-25T23:48:37Z","receivedAt":"2016-02-25T23:48:37Z","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\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n builtin/submodule--helper.c | 249 ++++++++++++++++++++++++++++++++++++++++++++\n git-submodule.sh            |  47 +++------\n 2 files changed, 262 insertions(+), 34 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex f4c3eff..d119a1d 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -255,6 +255,254 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \treturn 0;\n }\n \n+struct submodule_update_clone {\n+\t/* index into 'list', the list of submodules to look into for cloning */\n+\tint current;\n+\tstruct module_list list;\n+\tunsigned warn_if_uninitialized : 1;\n+\n+\t/* update parameter passed via commandline */\n+\tstruct submodule_update_strategy update;\n+\n+\t/* configuration parameters which are passed on to the children */\n+\tint quiet;\n+\tconst char *reference;\n+\tconst char *depth;\n+\tconst char *recursive_prefix;\n+\tconst char *prefix;\n+\n+\t/* Machine-readable status lines to be consumed by git-submodule.sh */\n+\tstruct string_list projectlines;\n+\n+\t/* If we want to stop as fast as possible and return an error */\n+\tunsigned quickstop : 1;\n+};\n+#define SUBMODULE_UPDATE_CLONE_INIT {0, MODULE_LIST_INIT, 0, \\\n+\tSUBMODULE_UPDATE_STRATEGY_INIT, 0, NULL, NULL, NULL, NULL, \\\n+\tSTRING_LIST_INIT_DUP, 0}\n+\n+/**\n+ * Determine whether 'ce' needs to be cloned. If so, prepare the 'child' to\n+ * run the clone. Returns 1 if 'ce' needs to be cloned, 0 otherwise.\n+ */\n+static int prepare_to_clone_next_submodule(const struct cache_entry *ce,\n+\t\t\t\t\t   struct child_process *child,\n+\t\t\t\t\t   struct submodule_update_clone *suc,\n+\t\t\t\t\t   struct strbuf *out)\n+{\n+\tconst struct submodule *sub = NULL;\n+\tstruct strbuf displaypath_sb = STRBUF_INIT;\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tconst char *displaypath = NULL;\n+\tchar *url = NULL;\n+\tint needs_cloning = 0;\n+\n+\tif (ce_stage(ce)) {\n+\t\tif (suc->recursive_prefix) {\n+\t\t\tstrbuf_addf(out, \"Skipping unmerged submodule %s/%s\\n\",\n+\t\t\t\t    suc->recursive_prefix, ce->name);\n+\t\t} else {\n+\t\t\tstrbuf_addf(out, \"Skipping unmerged submodule %s\\n\",\n+\t\t\t\t    ce->name);\n+\t\t}\n+\t\tgoto cleanup;\n+\t}\n+\n+\tsub = submodule_from_path(null_sha1, ce->name);\n+\n+\tif (suc->recursive_prefix)\n+\t\tdisplaypath = relative_path(suc->recursive_prefix,\n+\t\t\t\t\t    ce->name, &displaypath_sb);\n+\telse\n+\t\tdisplaypath = ce->name;\n+\n+\tif (suc->update.type == SM_UPDATE_NONE\n+\t    || (suc->update.type == SM_UPDATE_UNSPECIFIED\n+\t\t&& sub->update_strategy.type == SM_UPDATE_NONE)) {\n+\t\tstrbuf_addf(out, \"Skipping submodule '%s'\\n\",\n+\t\t\t    displaypath);\n+\t\tgoto cleanup;\n+\t}\n+\n+\t/*\n+\t * Looking up the url in .git/config.\n+\t * We must not fall back to .gitmodules as we only want\n+\t * to process configured submodules.\n+\t */\n+\tstrbuf_reset(&sb);\n+\tstrbuf_addf(&sb, \"submodule.%s.url\", sub->name);\n+\tgit_config_get_string(sb.buf, &url);\n+\tif (!url) {\n+\t\t/*\n+\t\t * Only mention uninitialized submodules when their\n+\t\t * path have been specified\n+\t\t */\n+\t\tif (suc->warn_if_uninitialized)\n+\t\t\tstrbuf_addf(out, _(\"Submodule path '%s' not initialized\\n\"\n+\t\t\t\t    \"Maybe you want to use 'update --init'?\\n\"),\n+\t\t\t\t    displaypath);\n+\t\tgoto cleanup;\n+\t}\n+\n+\tstrbuf_reset(&sb);\n+\tstrbuf_addf(&sb, \"%s/.git\", ce->name);\n+\tneeds_cloning = !file_exists(sb.buf);\n+\n+\tstrbuf_reset(&sb);\n+\tstrbuf_addf(&sb, \"%06o %s %d %d\\t%s\\n\", ce->ce_mode,\n+\t\t\tsha1_to_hex(ce->sha1), ce_stage(ce),\n+\t\t\tneeds_cloning, ce->name);\n+\tstring_list_append(&suc->projectlines, sb.buf);\n+\n+\tif (!needs_cloning)\n+\t\tgoto cleanup;\n+\n+\tchild->git_cmd = 1;\n+\tchild->no_stdin = 1;\n+\tchild->stdout_to_stderr = 1;\n+\tchild->err = -1;\n+\targv_array_push(&child->args, \"submodule--helper\");\n+\targv_array_push(&child->args, \"clone\");\n+\tif (suc->quiet)\n+\t\targv_array_push(&child->args, \"--quiet\");\n+\tif (suc->prefix)\n+\t\targv_array_pushl(&child->args, \"--prefix\", suc->prefix, NULL);\n+\targv_array_pushl(&child->args, \"--path\", sub->path, NULL);\n+\targv_array_pushl(&child->args, \"--name\", sub->name, NULL);\n+\targv_array_pushl(&child->args, \"--url\", url, NULL);\n+\tif (suc->reference)\n+\t\targv_array_push(&child->args, suc->reference);\n+\tif (suc->depth)\n+\t\targv_array_push(&child->args, suc->depth);\n+\n+cleanup:\n+\tfree(url);\n+\tstrbuf_reset(&displaypath_sb);\n+\tstrbuf_reset(&sb);\n+\n+\treturn needs_cloning;\n+}\n+\n+static int update_clone_get_next_task(struct child_process *child,\n+\t\t\t\t      struct strbuf *err,\n+\t\t\t\t      void *suc_cb,\n+\t\t\t\t      void **void_task_cb)\n+{\n+\tstruct submodule_update_clone *suc = suc_cb;\n+\n+\tfor (; suc->current < suc->list.nr; suc->current++) {\n+\t\tconst struct cache_entry *ce = suc->list.entries[suc->current];\n+\t\tif (prepare_to_clone_next_submodule(ce, child, suc, err)) {\n+\t\t\tsuc->current++;\n+\t\t\treturn 1;\n+\t\t}\n+\t}\n+\treturn 0;\n+}\n+\n+static int update_clone_start_failure(struct child_process *child,\n+\t\t\t\t      struct strbuf *err,\n+\t\t\t\t      void *suc_cb,\n+\t\t\t\t      void *void_task_cb)\n+{\n+\tstruct submodule_update_clone *suc = suc_cb;\n+\n+\tdefault_start_failure(child, err, suc_cb, void_task_cb);\n+\tsuc->quickstop = 1;\n+\n+\treturn 1;\n+}\n+\n+static int update_clone_task_finished(int result,\n+\t\t\t\t      struct child_process *child,\n+\t\t\t\t      struct strbuf *err,\n+\t\t\t\t      void *suc_cb,\n+\t\t\t\t      void *void_task_cb)\n+{\n+\tstruct submodule_update_clone *suc = suc_cb;\n+\n+\tif (!result)\n+\t\treturn 0;\n+\n+\tdefault_task_finished(result, child, err, suc_cb, void_task_cb);\n+\tsuc->quickstop = 1;\n+\n+\treturn 1;\n+}\n+\n+static int update_clone(int argc, const char **argv, const char *prefix)\n+{\n+\tconst char *update = NULL;\n+\tstruct string_list_item *item;\n+\tstruct pathspec pathspec;\n+\tstruct submodule_update_clone suc = SUBMODULE_UPDATE_CLONE_INIT;\n+\n+\tstruct option module_update_clone_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\", &suc.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\", &update,\n+\t\t\t   N_(\"string\"),\n+\t\t\t   N_(\"rebase, merge, checkout or none\")),\n+\t\tOPT_STRING(0, \"reference\", &suc.reference, N_(\"repo\"),\n+\t\t\t   N_(\"reference repository\")),\n+\t\tOPT_STRING(0, \"depth\", &suc.depth, \"<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(&suc.quiet, N_(\"don'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 update_clone [--prefix=<path>] [<path>...]\"),\n+\t\tNULL\n+\t};\n+\tsuc.prefix = prefix;\n+\n+\targc = parse_options(argc, argv, prefix, module_update_clone_options,\n+\t\t\t     git_submodule_helper_usage, 0);\n+\n+\tif (update)\n+\t\tif (parse_submodule_update_strategy(update, &suc.update) < 0)\n+\t\t\tdie(_(\"bad value for update parameter\"));\n+\n+\tif (module_list_compute(argc, argv, prefix, &pathspec, &suc.list) < 0)\n+\t\treturn 1;\n+\n+\tif (pathspec.nr)\n+\t\tsuc.warn_if_uninitialized = 1;\n+\n+\t/* Overlay the parsed .gitmodules file with .git/config */\n+\tgitmodules_config();\n+\tgit_config(submodule_config, NULL);\n+\n+\trun_processes_parallel(1,\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       &suc);\n+\n+\t/*\n+\t * We saved the output and put it out all at once now.\n+\t * That means:\n+\t * - the listener does not have to interleave their (checkout)\n+\t *   work with our fetching.  The writes involved in a\n+\t *   checkout involve more straightforward sequential I/O.\n+\t * - the listener can avoid doing any work if fetching failed.\n+\t */\n+\tif (suc.quickstop)\n+\t\treturn 1;\n+\n+\tfor_each_string_list_item(item, &suc.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 +512,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 9ee86d4..a6a82d2 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -664,17 +664,20 @@ cmd_update()\n \t\tcmd_init \"--\" \"$@\" || return\n \tfi\n \n-\tcloned_modules=\n-\tgit submodule--helper list --prefix \"$wt_prefix\" \"$@\" | {\n+\t{\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\"$@\" || echo \"#unmatched\"\n+\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 +694,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 >&2 \"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 >&2 \"$(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 +737,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)\n-- \n2.7.0.rc0.36.g75877e4.dirty\n"},{"id":"279453","messageId":"1456444119-6934-11-git-send-email-sbeller@google.com","threadId":"41512","inReplyTo":"1456444119-6934-1-git-send-email-sbeller@google.com","subject":"[PATCHv19 10/11] submodule update: expose parallelism to the user","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-25T23:48:38Z","receivedAt":"2016-02-25T23:48:38Z","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\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n Documentation/git-submodule.txt |  7 ++++++-\n builtin/submodule--helper.c     |  8 +++++++-\n git-submodule.sh                |  9 +++++++++\n t/t7406-submodule-update.sh     | 12 ++++++++++++\n 4 files changed, 34 insertions(+), 2 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] [--] <path>...\n 'git submodule' [--quiet] update [--init] [--remote] [-N|--no-fetch]\n \t      [-f|--force] [--rebase|--merge] [--reference <repository>]\n-\t      [--depth <depth>] [--recursive] [--] [<path>...]\n+\t      [--depth <depth>] [--recursive] [--jobs <n>] [--] [<path>...]\n 'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) <n>]\n \t      [commit] [--] [<path>...]\n 'git submodule' [--quiet] foreach [--recursive] <command>\n@@ -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>::\n+--jobs <n>::\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 <path>...::\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 d119a1d..0272c98 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 static int update_clone(int argc, const char **argv, const char *prefix)\n {\n \tconst char *update = NULL;\n+\tint max_jobs = -1;\n \tstruct string_list_item *item;\n \tstruct pathspec pathspec;\n \tstruct submodule_update_clone suc = SUBMODULE_UPDATE_CLONE_INIT;\n@@ -453,6 +454,8 @@ static int update_clone(int argc, const char **argv, const char *prefix)\n \t\tOPT_STRING(0, \"depth\", &suc.depth, \"<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(&suc.quiet, N_(\"don't print cloning progress\")),\n \t\tOPT_END()\n \t};\n@@ -480,7 +483,10 @@ static int update_clone(int argc, const char **argv, const char *prefix)\n \tgitmodules_config();\n \tgit_config(submodule_config, NULL);\n \n-\trun_processes_parallel(1,\n+\tif (max_jobs < 0)\n+\t\tmax_jobs = parallel_submodules();\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,\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex a6a82d2..86018ee 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@@ -671,6 +679,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\"$@\" || echo \"#unmatched\"\n \t} | {\n \terr=\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.36.g75877e4.dirty\n"},{"id":"279451","messageId":"1456444119-6934-12-git-send-email-sbeller@google.com","threadId":"41512","inReplyTo":"1456444119-6934-1-git-send-email-sbeller@google.com","subject":"[PATCHv19 11/11] clone: allow an explicit argument for parallel submodule clones","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-25T23:48:39Z","receivedAt":"2016-02-25T23:48:39Z","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\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\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 <name>] [-b <name>] [-u <upload-pack>] [--reference <repository>]\n \t  [--dissociate] [--separate-git-dir <git dir>]\n \t  [--depth <depth>] [--[no-]single-branch]\n-\t  [--recursive | --recurse-submodules] [--] <repository>\n+\t  [--recursive | --recurse-submodules] [--jobs <n>] [--] <repository>\n \t  [<directory>]\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>::\n+--jobs <n>::\n+\tThe number of submodules fetched at the same time.\n+\tDefaults to the `submodule.fetchJobs` option.\n \n <repository>::\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.36.g75877e4.dirty\n"},{"id":"279456","messageId":"20160225235023.GW28749@google.com","threadId":"41512","inReplyTo":"1456444119-6934-1-git-send-email-sbeller@google.com","subject":"Re: [PATCHv19 00/11] Expose submodule parallelism to the user","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2016-02-25T23:50:23Z","receivedAt":"2016-02-25T23:50:23Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Stefan Beller wrote:\n\n> Thanks Jonathan (and all the other cc'd reviewers) for bearing\n> yet another revision!\n\nThanks.  This one looks good.\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\n"},{"id":"279633","messageId":"CACsJy8DmhJrUkp=d4kSst=0yGWn+Hk=MqxCmhH3WqfPzhTt+Uw@mail.gmail.com","threadId":"41512","inReplyTo":"1456444119-6934-10-git-send-email-sbeller@google.com","subject":"Re: [PATCHv19 09/11] git submodule update: have a dedicated helper for cloning","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-02-27T08:40:41Z","receivedAt":"2016-02-27T08:40:41Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Feb 26, 2016 at 6:48 AM, Stefan Beller <sbeller@google.com> wrote:\n> +static int prepare_to_clone_next_submodule(const struct cache_entry *ce,\n> +                                          struct child_process *child,\n> +                                          struct submodule_update_clone *suc,\n> +                                          struct strbuf *out)\n> +{\n> +       const struct submodule *sub = NULL;\n> +       struct strbuf displaypath_sb = STRBUF_INIT;\n> +       struct strbuf sb = STRBUF_INIT;\n> +       const char *displaypath = NULL;\n> +       char *url = NULL;\n> +       int needs_cloning = 0;\n> +\n> +       if (ce_stage(ce)) {\n> +               if (suc->recursive_prefix) {\n> +                       strbuf_addf(out, \"Skipping unmerged submodule %s/%s\\n\",\n> +                                   suc->recursive_prefix, ce->name);\n\nI'm pretty sure this string is for human consumption (because it's\n_()'d elsehwere in this function), please _() this string.\n\n> +               } else {\n> +                       strbuf_addf(out, \"Skipping unmerged submodule %s\\n\",\n> +                                   ce->name);\n\nand this one\n\n> +               }\n> +               goto cleanup;\n> +       }\n> +\n> +       sub = submodule_from_path(null_sha1, ce->name);\n> +\n> +       if (suc->recursive_prefix)\n> +               displaypath = relative_path(suc->recursive_prefix,\n> +                                           ce->name, &displaypath_sb);\n> +       else\n> +               displaypath = ce->name;\n> +\n> +       if (suc->update.type == SM_UPDATE_NONE\n> +           || (suc->update.type == SM_UPDATE_UNSPECIFIED\n> +               && sub->update_strategy.type == SM_UPDATE_NONE)) {\n> +               strbuf_addf(out, \"Skipping submodule '%s'\\n\",\n> +                           displaypath);\n\nand this one\n\n> +               goto cleanup;\n> +       }\n> +\n> +       /*\n> +        * Looking up the url in .git/config.\n> +        * We must not fall back to .gitmodules as we only want\n> +        * to process configured submodules.\n> +        */\n> +       strbuf_reset(&sb);\n> +       strbuf_addf(&sb, \"submodule.%s.url\", sub->name);\n> +       git_config_get_string(sb.buf, &url);\n> +       if (!url) {\n> +               /*\n> +                * Only mention uninitialized submodules when their\n> +                * path have been specified\n> +                */\n> +               if (suc->warn_if_uninitialized)\n> +                       strbuf_addf(out, _(\"Submodule path '%s' not initialized\\n\"\n> +                                   \"Maybe you want to use 'update --init'?\\n\"),\n> +                                   displaypath);\n\noh it's already marked :)\n\nBTW, while you're editing this file, perhaps do this too (maybe in a\nseparate patch)? Because die() already prepends \"fatal:\"\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex a6e54fa..6cf47de 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -731,13 +731,13 @@ int cmd_submodule__helper(int argc, const char\n**argv, const char *prefix)\n {\n        int i;\n        if (argc < 2)\n-               die(_(\"fatal: submodule--helper subcommand must be \"\n+               die(_(\"submodule--helper subcommand must be \"\n                      \"called with a subcommand\"));\n\n        for (i = 0; i < ARRAY_SIZE(commands); i++)\n                if (!strcmp(argv[1], commands[i].cmd))\n                        return commands[i].fn(argc - 1, argv + 1, prefix);\n\n-       die(_(\"fatal: '%s' is not a valid submodule--helper \"\n+       die(_(\"'%s' is not a valid submodule--helper \"\n              \"subcommand\"), argv[1]);\n }\n-- \nDuy\n"},{"id":"279812","messageId":"CAGZ79kZYqXWAqjmYJtHM5rPUPbCS_NxOsrbspkOS1WAVV44wzg@mail.gmail.com","threadId":"41512","inReplyTo":"CACsJy8DmhJrUkp=d4kSst=0yGWn+Hk=MqxCmhH3WqfPzhTt+Uw@mail.gmail.com","subject":"Re: [PATCHv19 09/11] git submodule update: have a dedicated helper for cloning","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-29T19:03:10Z","receivedAt":"2016-02-29T19:03:10Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Sat, Feb 27, 2016 at 12:40 AM, Duy Nguyen <pclouds@gmail.com> wrote:\n> On Fri, Feb 26, 2016 at 6:48 AM, Stefan Beller <sbeller@google.com> wrote:\n>> +static int prepare_to_clone_next_submodule(const struct cache_entry *ce,\n>> +                                          struct child_process *child,\n>> +                                          struct submodule_update_clone *suc,\n>> +                                          struct strbuf *out)\n>> +{\n>> +       const struct submodule *sub = NULL;\n>> +       struct strbuf displaypath_sb = STRBUF_INIT;\n>> +       struct strbuf sb = STRBUF_INIT;\n>> +       const char *displaypath = NULL;\n>> +       char *url = NULL;\n>> +       int needs_cloning = 0;\n>> +\n>> +       if (ce_stage(ce)) {\n>> +               if (suc->recursive_prefix) {\n>> +                       strbuf_addf(out, \"Skipping unmerged submodule %s/%s\\n\",\n>> +                                   suc->recursive_prefix, ce->name);\n>\n> I'm pretty sure this string is for human consumption (because it's\n> _()'d elsehwere in this function), please _() this string.\n>\n>> +               } else {\n>> +                       strbuf_addf(out, \"Skipping unmerged submodule %s\\n\",\n>> +                                   ce->name);\n>\n> and this one\n>\n>> +               }\n>> +               goto cleanup;\n>> +       }\n>> +\n>> +       sub = submodule_from_path(null_sha1, ce->name);\n>> +\n>> +       if (suc->recursive_prefix)\n>> +               displaypath = relative_path(suc->recursive_prefix,\n>> +                                           ce->name, &displaypath_sb);\n>> +       else\n>> +               displaypath = ce->name;\n>> +\n>> +       if (suc->update.type == SM_UPDATE_NONE\n>> +           || (suc->update.type == SM_UPDATE_UNSPECIFIED\n>> +               && sub->update_strategy.type == SM_UPDATE_NONE)) {\n>> +               strbuf_addf(out, \"Skipping submodule '%s'\\n\",\n>> +                           displaypath);\n>\n> and this one\n>\n>> +               goto cleanup;\n>> +       }\n>> +\n>> +       /*\n>> +        * Looking up the url in .git/config.\n>> +        * We must not fall back to .gitmodules as we only want\n>> +        * to process configured submodules.\n>> +        */\n>> +       strbuf_reset(&sb);\n>> +       strbuf_addf(&sb, \"submodule.%s.url\", sub->name);\n>> +       git_config_get_string(sb.buf, &url);\n>> +       if (!url) {\n>> +               /*\n>> +                * Only mention uninitialized submodules when their\n>> +                * path have been specified\n>> +                */\n>> +               if (suc->warn_if_uninitialized)\n>> +                       strbuf_addf(out, _(\"Submodule path '%s' not initialized\\n\"\n>> +                                   \"Maybe you want to use 'update --init'?\\n\"),\n>> +                                   displaypath);\n>\n> oh it's already marked :)\n>\n> BTW, while you're editing this file, perhaps do this too (maybe in a\n> separate patch)? Because die() already prepends \"fatal:\"\n\nMakes sense. As builtin/submodule--helper.c was introduced in 2.7.0\nand translation has already started fr 2.8.0, I'll just pick it up as\npart of this series\ninstead of sending a bugfix patch alone.\n\nThanks for review!\n\n>\n> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\n> index a6e54fa..6cf47de 100644\n> --- a/builtin/submodule--helper.c\n> +++ b/builtin/submodule--helper.c\n> @@ -731,13 +731,13 @@ int cmd_submodule__helper(int argc, const char\n> **argv, const char *prefix)\n>  {\n>         int i;\n>         if (argc < 2)\n> -               die(_(\"fatal: submodule--helper subcommand must be \"\n> +               die(_(\"submodule--helper subcommand must be \"\n>                       \"called with a subcommand\"));\n>\n>         for (i = 0; i < ARRAY_SIZE(commands); i++)\n>                 if (!strcmp(argv[1], commands[i].cmd))\n>                         return commands[i].fn(argc - 1, argv + 1, prefix);\n>\n> -       die(_(\"fatal: '%s' is not a valid submodule--helper \"\n> +       die(_(\"'%s' is not a valid submodule--helper \"\n>               \"subcommand\"), argv[1]);\n>  }\n> --\n> Duy\n"},{"id":"279838","messageId":"56D4AE8A.2050403@kdbg.org","threadId":"41512","inReplyTo":"1456444119-6934-1-git-send-email-sbeller@google.com","subject":"Re: [PATCHv19 00/11] Expose submodule parallelism to the user","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2016-02-29T20:48:10Z","receivedAt":"2016-02-29T20:48:10Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Hi folks,\n\nwe have a major breakage in the parallel tasks infrastructure, and I'm\nafraid it is already in master.\n\nInstrument the code in sb/submodule-parallel-update like this and enjoy\nthe fireworks of './t7400-submodule-basic.sh -v -i -x --debug':\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 0322282..482c7f6 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -690,8 +690,9 @@ cmd_update()\n \t\tcmd_init \"--\" \"$@\" || return\n \tfi\n \n+\tset -x\n \t{\n-\tgit submodule--helper update-clone ${GIT_QUIET:+--quiet} \\\n+\tvalgrind git 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\"} \\\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex 5572327..717e491 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -337,6 +337,7 @@ test_expect_success 'update should fail when path is used by a file' '\n \n \techo \"hello\" >init &&\n \ttest_must_fail git submodule update &&\n+\tfalse &&\n \n \ttest_cmp expect init\n '\n\nThe culprit seems to be default_task_finished(), which accesses argv[]\nof the struct child_process after finish_command has released it,\nprovided the child exited with an error, for example:\n\n==3395== Invalid read of size 8\n==3395==    at 0x54F991: default_task_finished (run-command.c:932)\n==3395==    by 0x49158F: update_clone_task_finished (submodule--helper.c:421)\n==3395==    by 0x5504A2: pp_collect_finished (run-command.c:1122)\n==3395==    by 0x5507C7: run_processes_parallel (run-command.c:1194)\n==3395==    by 0x4918EB: update_clone (submodule--helper.c:483)\n==3395==    by 0x4919D8: cmd_submodule__helper (submodule--helper.c:527)\n==3395==    by 0x405CBE: run_builtin (git.c:353)\n==3395==    by 0x405EAA: handle_builtin (git.c:540)\n==3395==    by 0x405FCC: run_argv (git.c:594)\n==3395==    by 0x4061BF: main (git.c:701)\n==3395==  Address 0x5e49370 is 0 bytes inside a block of size 192 free'd\n==3395==    at 0x4C2A37C: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)\n==3395==    by 0x4A26EE: argv_array_clear (argv-array.c:73)\n==3395==    by 0x54DFC4: child_process_clear (run-command.c:18)\n==3395==    by 0x54EFA7: finish_command (run-command.c:539)\n==3395==    by 0x550413: pp_collect_finished (run-command.c:1120)\n==3395==    by 0x5507C7: run_processes_parallel (run-command.c:1194)\n==3395==    by 0x4918EB: update_clone (submodule--helper.c:483)\n==3395==    by 0x4919D8: cmd_submodule__helper (submodule--helper.c:527)\n==3395==    by 0x405CBE: run_builtin (git.c:353)\n==3395==    by 0x405EAA: handle_builtin (git.c:540)\n==3395==    by 0x405FCC: run_argv (git.c:594)\n==3395==    by 0x4061BF: main (git.c:701)\n\nI haven't thought about a solution, yet. Perhaps you have ideas.\n\n-- Hannes\n"},{"id":"279840","messageId":"CAGZ79kaHm_GDtmjzeBqonS9JbCCjov6W23dNw_EKM5vL6a4=yA@mail.gmail.com","threadId":"41512","inReplyTo":"56D4AE8A.2050403@kdbg.org","subject":"Re: [PATCHv19 00/11] Expose submodule parallelism to the user","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-29T20:59:40Z","receivedAt":"2016-02-29T20:59:40Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Feb 29, 2016 at 12:48 PM, Johannes Sixt <j6t@kdbg.org> wrote:\n> Hi folks,\n>\n> we have a major breakage in the parallel tasks infrastructure, and I'm\n> afraid it is already in master.\n>\n> Instrument the code in sb/submodule-parallel-update like this and enjoy\n> the fireworks of './t7400-submodule-basic.sh -v -i -x --debug':\n>\n> diff --git a/git-submodule.sh b/git-submodule.sh\n> index 0322282..482c7f6 100755\n> --- a/git-submodule.sh\n> +++ b/git-submodule.sh\n> @@ -690,8 +690,9 @@ cmd_update()\n>                 cmd_init \"--\" \"$@\" || return\n>         fi\n>\n> +       set -x\n>         {\n> -       git submodule--helper update-clone ${GIT_QUIET:+--quiet} \\\n> +       valgrind git submodule--helper update-clone ${GIT_QUIET:+--quiet} \\\n>                 ${wt_prefix:+--prefix \"$wt_prefix\"} \\\n>                 ${prefix:+--recursive-prefix \"$prefix\"} \\\n>                 ${update:+--update \"$update\"} \\\n> diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\n> index 5572327..717e491 100755\n> --- a/t/t7400-submodule-basic.sh\n> +++ b/t/t7400-submodule-basic.sh\n> @@ -337,6 +337,7 @@ test_expect_success 'update should fail when path is used by a file' '\n>\n>         echo \"hello\" >init &&\n>         test_must_fail git submodule update &&\n> +       false &&\n>\n>         test_cmp expect init\n>  '\n>\n> The culprit seems to be default_task_finished(), which accesses argv[]\n> of the struct child_process after finish_command has released it,\n> provided the child exited with an error, for example:\n>\n> ==3395== Invalid read of size 8\n> ==3395==    at 0x54F991: default_task_finished (run-command.c:932)\n> ==3395==    by 0x49158F: update_clone_task_finished (submodule--helper.c:421)\n> ==3395==    by 0x5504A2: pp_collect_finished (run-command.c:1122)\n> ==3395==    by 0x5507C7: run_processes_parallel (run-command.c:1194)\n> ==3395==    by 0x4918EB: update_clone (submodule--helper.c:483)\n> ==3395==    by 0x4919D8: cmd_submodule__helper (submodule--helper.c:527)\n> ==3395==    by 0x405CBE: run_builtin (git.c:353)\n> ==3395==    by 0x405EAA: handle_builtin (git.c:540)\n> ==3395==    by 0x405FCC: run_argv (git.c:594)\n> ==3395==    by 0x4061BF: main (git.c:701)\n> ==3395==  Address 0x5e49370 is 0 bytes inside a block of size 192 free'd\n> ==3395==    at 0x4C2A37C: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)\n> ==3395==    by 0x4A26EE: argv_array_clear (argv-array.c:73)\n> ==3395==    by 0x54DFC4: child_process_clear (run-command.c:18)\n> ==3395==    by 0x54EFA7: finish_command (run-command.c:539)\n> ==3395==    by 0x550413: pp_collect_finished (run-command.c:1120)\n> ==3395==    by 0x5507C7: run_processes_parallel (run-command.c:1194)\n> ==3395==    by 0x4918EB: update_clone (submodule--helper.c:483)\n> ==3395==    by 0x4919D8: cmd_submodule__helper (submodule--helper.c:527)\n> ==3395==    by 0x405CBE: run_builtin (git.c:353)\n> ==3395==    by 0x405EAA: handle_builtin (git.c:540)\n> ==3395==    by 0x405FCC: run_argv (git.c:594)\n> ==3395==    by 0x4061BF: main (git.c:701)\n>\n> I haven't thought about a solution, yet. Perhaps you have ideas.\n>\n> -- Hannes\n>\n\nWhat about unfolding finish_command like so:\n\ndiff --git a/run-command.c b/run-command.c\nindex 863dad5..659abd9 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -1115,11 +1115,13 @@ static int pp_collect_finished(struct\nparallel_processes *pp)\n                if (i == pp->max_processes)\n                        break;\n\n-               code = finish_command(&pp->children[i].process);\n+               code = wait_or_whine(pp->children[i].process.pid,\n+                                    pp->children[i].process.argv[0], 0);\n\n                code = pp->task_finished(code, &pp->children[i].process,\n                                         &pp->children[i].err, pp->data,\n                                         &pp->children[i].data);\n+               child_process_clear(&pp->children[i].process);\n\n                if (code)\n                        result = code;\n"},{"id":"279841","messageId":"xmqq1t7v9qou.fsf@gitster.mtv.corp.google.com","threadId":"41512","inReplyTo":"56D4AE8A.2050403@kdbg.org","subject":"Re: [PATCHv19 00/11] Expose submodule parallelism to the user","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-02-29T21:01:21Z","receivedAt":"2016-02-29T21:01:21Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt <j6t@kdbg.org> writes:\n\n> The culprit seems to be default_task_finished(), which accesses argv[]\n> of the struct child_process after finish_command has released it,\n> provided the child exited with an error, for example:\n\nThanks for a report.\n\n> ==3395== Invalid read of size 8\n> ==3395==    at 0x54F991: default_task_finished (run-command.c:932)\n\nThat one and also start_failure() do run after child_process_clear()\nhas cleaned things up, so they shouldn't be looking at argv[] (or\nanything in that structure for that matter).\n\n\n\n> ==3395==    by 0x49158F: update_clone_task_finished (submodule--helper.c:421)\n> ==3395==    by 0x5504A2: pp_collect_finished (run-command.c:1122)\n> ==3395==    by 0x5507C7: run_processes_parallel (run-command.c:1194)\n> ==3395==    by 0x4918EB: update_clone (submodule--helper.c:483)\n> ==3395==    by 0x4919D8: cmd_submodule__helper (submodule--helper.c:527)\n> ==3395==    by 0x405CBE: run_builtin (git.c:353)\n> ==3395==    by 0x405EAA: handle_builtin (git.c:540)\n> ==3395==    by 0x405FCC: run_argv (git.c:594)\n> ==3395==    by 0x4061BF: main (git.c:701)\n> ==3395==  Address 0x5e49370 is 0 bytes inside a block of size 192 free'd\n> ==3395==    at 0x4C2A37C: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)\n> ==3395==    by 0x4A26EE: argv_array_clear (argv-array.c:73)\n> ==3395==    by 0x54DFC4: child_process_clear (run-command.c:18)\n> ==3395==    by 0x54EFA7: finish_command (run-command.c:539)\n> ==3395==    by 0x550413: pp_collect_finished (run-command.c:1120)\n> ==3395==    by 0x5507C7: run_processes_parallel (run-command.c:1194)\n> ==3395==    by 0x4918EB: update_clone (submodule--helper.c:483)\n> ==3395==    by 0x4919D8: cmd_submodule__helper (submodule--helper.c:527)\n> ==3395==    by 0x405CBE: run_builtin (git.c:353)\n> ==3395==    by 0x405EAA: handle_builtin (git.c:540)\n> ==3395==    by 0x405FCC: run_argv (git.c:594)\n> ==3395==    by 0x4061BF: main (git.c:701)\n>\n> I haven't thought about a solution, yet. Perhaps you have ideas.\n>\n> -- Hannes\n"},{"id":"279843","messageId":"CAGZ79kY5nxDQ_o0L_MOYFOx4FPH1apW-TuB88UypuwDaLuKGNQ@mail.gmail.com","threadId":"41512","inReplyTo":"xmqq1t7v9qou.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCHv19 00/11] Expose submodule parallelism to the user","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-29T21:06:53Z","receivedAt":"2016-02-29T21:06:53Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Feb 29, 2016 at 1:01 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Johannes Sixt <j6t@kdbg.org> writes:\n>\n>> The culprit seems to be default_task_finished(), which accesses argv[]\n>> of the struct child_process after finish_command has released it,\n>> provided the child exited with an error, for example:\n>\n> Thanks for a report.\n>\n>> ==3395== Invalid read of size 8\n>> ==3395==    at 0x54F991: default_task_finished (run-command.c:932)\n>\n> That one and also start_failure() do run after child_process_clear()\n> has cleaned things up, so they shouldn't be looking at argv[] (or\n> anything in that structure for that matter).\n\nI am undecided if easy access to the child process struct\nis a benefit or not for the callback. It makes error reporting\neasy, but maybe hacky as you poke around with the argv.\n\nMaybe we want to remove the struct child_process from the\nfunction signature of the callbacks and callbacks need to rely on\nthe data provided solely thru the pointer as passed around for\ncallback purposes, which the user is free to use for any kind\nof data.\n\nAs a rather quickfix for 2.8 (and 2.7) we could however just\nbreakup the finish_command function and call child_process_clear\nafter the callbacks have run.\n\n>\n>\n>\n>> ==3395==    by 0x49158F: update_clone_task_finished (submodule--helper.c:421)\n>> ==3395==    by 0x5504A2: pp_collect_finished (run-command.c:1122)\n>> ==3395==    by 0x5507C7: run_processes_parallel (run-command.c:1194)\n>> ==3395==    by 0x4918EB: update_clone (submodule--helper.c:483)\n>> ==3395==    by 0x4919D8: cmd_submodule__helper (submodule--helper.c:527)\n>> ==3395==    by 0x405CBE: run_builtin (git.c:353)\n>> ==3395==    by 0x405EAA: handle_builtin (git.c:540)\n>> ==3395==    by 0x405FCC: run_argv (git.c:594)\n>> ==3395==    by 0x4061BF: main (git.c:701)\n>> ==3395==  Address 0x5e49370 is 0 bytes inside a block of size 192 free'd\n>> ==3395==    at 0x4C2A37C: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)\n>> ==3395==    by 0x4A26EE: argv_array_clear (argv-array.c:73)\n>> ==3395==    by 0x54DFC4: child_process_clear (run-command.c:18)\n>> ==3395==    by 0x54EFA7: finish_command (run-command.c:539)\n>> ==3395==    by 0x550413: pp_collect_finished (run-command.c:1120)\n>> ==3395==    by 0x5507C7: run_processes_parallel (run-command.c:1194)\n>> ==3395==    by 0x4918EB: update_clone (submodule--helper.c:483)\n>> ==3395==    by 0x4919D8: cmd_submodule__helper (submodule--helper.c:527)\n>> ==3395==    by 0x405CBE: run_builtin (git.c:353)\n>> ==3395==    by 0x405EAA: handle_builtin (git.c:540)\n>> ==3395==    by 0x405FCC: run_argv (git.c:594)\n>> ==3395==    by 0x4061BF: main (git.c:701)\n>>\n>> I haven't thought about a solution, yet. Perhaps you have ideas.\n>>\n>> -- Hannes\n"},{"id":"279844","messageId":"xmqqwppn8ba4.fsf@gitster.mtv.corp.google.com","threadId":"41512","inReplyTo":"CAGZ79kY5nxDQ_o0L_MOYFOx4FPH1apW-TuB88UypuwDaLuKGNQ@mail.gmail.com","subject":"Re: [PATCHv19 00/11] Expose submodule parallelism to the user","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-02-29T21:19:31Z","receivedAt":"2016-02-29T21:19:31Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> Maybe we want to remove the struct child_process from the\n> function signature of the callbacks and callbacks need to rely on\n> the data provided solely thru the pointer as passed around for\n> callback purposes, which the user is free to use for any kind\n> of data.\n\nI think that is the most sensible.\n\n> As a rather quickfix for 2.8 (and 2.7) we could however just\n> breakup the finish_command function and call child_process_clear\n> after the callbacks have run.\n\nThat would not fix the case where run_command() fails, clears child\nand then start failure callback, no?\n"},{"id":"279845","messageId":"CAGZ79kZmuY=-G6votRtX69odiuMq8BDYK8eCqW8q8GDub9p3WA@mail.gmail.com","threadId":"41512","inReplyTo":"xmqqwppn8ba4.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCHv19 00/11] Expose submodule parallelism to the user","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-29T21:22:42Z","receivedAt":"2016-02-29T21:22:42Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Feb 29, 2016 at 1:19 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Stefan Beller <sbeller@google.com> writes:\n>\n>> Maybe we want to remove the struct child_process from the\n>> function signature of the callbacks and callbacks need to rely on\n>> the data provided solely thru the pointer as passed around for\n>> callback purposes, which the user is free to use for any kind\n>> of data.\n>\n> I think that is the most sensible.\n\nOk I'll send the diff above as a patch.\n>\n>> As a rather quickfix for 2.8 (and 2.7) we could however just\n>> breakup the finish_command function and call child_process_clear\n>> after the callbacks have run.\n>\n> That would not fix the case where run_command() fails, clears child\n> and then start failure callback, no?\n\nFor that I was about to propose (broken whitespace below):\n\ncommit a0dcfbf86b6b720529958a4043d3679ed6f340d0\nAuthor: Stefan Beller <sbeller@google.com>\nDate:   Mon Feb 29 13:20:38 2016 -0800\n\n    run-command: factor cleanup out of start_command\n\n    The default callback of the parallel processing infrastructure assumes\n    start_command will not clear the child process upon failing to start a\n    child process. However having access to the child's argument vector\n    enables easy error reporting, so move cleaning the child into a wrapper\n    for general use and the parallel processing infrastructure needs to take\n    care of cleaning up the child itself\n\n    Signed-off-by: Stefan Beller <sbeller@google.com>\n\ndiff --git a/run-command.c b/run-command.c\nindex 863dad5..b1d97d0 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -265,7 +265,7 @@ static int wait_or_whine(pid_t pid, const char\n*argv0, int in_signal)\n         return code;\n }\n\n-int start_command(struct child_process *cmd)\n+static int start_command_uncleaned(struct child_process *cmd)\n {\n         int need_in, need_out, need_err;\n         int fdin[2], fdout[2], fderr[2];\n@@ -326,7 +326,6 @@ int start_command(struct child_process *cmd)\n fail_pipe:\n                         error(\"cannot create %s pipe for %s: %s\",\n                                 str, cmd->argv[0], strerror(failed_errno));\n-                        child_process_clear(cmd);\n                         errno = failed_errno;\n                         return -1;\n                 }\n@@ -510,7 +509,7 @@ fail_pipe:\n                         close_pair(fderr);\n                 else if (cmd->err)\n                         close(cmd->err);\n-                child_process_clear(cmd);\n+\n                 errno = failed_errno;\n                 return -1;\n         }\n@@ -533,6 +532,14 @@ fail_pipe:\n         return 0;\n }\n\n+int start_command(struct child_process *cmd)\n+{\n+        int code = start_command_uncleaned(cmd);\n+        if (code == -1)\n+                child_process_clear(cmd);\n+        return code;\n+}\n+\n int finish_command(struct child_process *cmd)\n {\n         int ret = wait_or_whine(cmd->pid, cmd->argv[0], 0);\n@@ -1047,11 +1054,12 @@ static int pp_start_one(struct parallel_processes *pp)\n         pp->children[i].process.stdout_to_stderr = 1;\n         pp->children[i].process.no_stdin = 1;\n\n-        if (start_command(&pp->children[i].process)) {\n+        if (start_command_uncleaned(&pp->children[i].process)) {\n                 code = pp->start_failure(&pp->children[i].process,\n                                          &pp->children[i].err,\n                                          pp->data,\n                                          &pp->children[i].data);\n+                child_process_clear(&pp->children[i].process);\n                 strbuf_addbuf(&pp->buffered_output, &pp->children[i].err);\n                 strbuf_reset(&pp->children[i].err);\n                 if (code)\n"},{"id":"279846","messageId":"56D4B805.7090702@kdbg.org","threadId":"41512","inReplyTo":"xmqqwppn8ba4.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCHv19 00/11] Expose submodule parallelism to the user","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2016-02-29T21:28:37Z","receivedAt":"2016-02-29T21:28:37Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 29.02.2016 um 22:19 schrieb Junio C Hamano:\n> Stefan Beller <sbeller@google.com> writes:\n>\n>> Maybe we want to remove the struct child_process from the\n>> function signature of the callbacks and callbacks need to rely on\n>> the data provided solely thru the pointer as passed around for\n>> callback purposes, which the user is free to use for any kind\n>> of data.\n>\n> I think that is the most sensible.\n\nI also think that is the better approach. Dumping out the argv array is \nnot the best end-user experience. It is just a debugging aid, and for \nthat we have (or should extend if necessary) GIT_TRACE infrastructure. \nMoreover, a command that failed should have printed error messages, and \nit is not necessary to follow it up with another \"A child process exited \nwith code N\" message.\n\n-- Hannes\n"},{"id":"279853","messageId":"xmqqsi0b89tb.fsf@gitster.mtv.corp.google.com","threadId":"41512","inReplyTo":"56D4B805.7090702@kdbg.org","subject":"Re: [PATCHv19 00/11] Expose submodule parallelism to the user","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-02-29T21:51:12Z","receivedAt":"2016-02-29T21:51:12Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt <j6t@kdbg.org> writes:\n\n> Am 29.02.2016 um 22:19 schrieb Junio C Hamano:\n>> Stefan Beller <sbeller@google.com> writes:\n>>\n>>> Maybe we want to remove the struct child_process from the\n>>> function signature of the callbacks and callbacks need to rely on\n>>> the data provided solely thru the pointer as passed around for\n>>> callback purposes, which the user is free to use for any kind\n>>> of data.\n>>\n>> I think that is the most sensible.\n>\n> I also think that is the better approach. Dumping out the argv array\n> is not the best end-user experience. It is just a debugging aid, and\n> for that we have (or should extend if necessary) GIT_TRACE\n> infrastructure. Moreover, a command that failed should have printed\n> error messages, and it is not necessary to follow it up with another\n> \"A child process exited with code N\" message.\n\nExactly.  Stefan, please do not go in the route of butchering the\nstart_command(), finish_command(), etc. API set with the \"uncleaned\"\nvariants.\n\nThanks.\n"},{"id":"279854","messageId":"CAGZ79kbA5M26MVHymKRo9RTO7uXzN5YR9bKOBpFCM=qW8rDUnA@mail.gmail.com","threadId":"41512","inReplyTo":"xmqqsi0b89tb.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCHv19 00/11] Expose submodule parallelism to the user","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-02-29T21:55:45Z","receivedAt":"2016-02-29T21:55:45Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Feb 29, 2016 at 1:51 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Johannes Sixt <j6t@kdbg.org> writes:\n>\n>> Am 29.02.2016 um 22:19 schrieb Junio C Hamano:\n>>> Stefan Beller <sbeller@google.com> writes:\n>>>\n>>>> Maybe we want to remove the struct child_process from the\n>>>> function signature of the callbacks and callbacks need to rely on\n>>>> the data provided solely thru the pointer as passed around for\n>>>> callback purposes, which the user is free to use for any kind\n>>>> of data.\n>>>\n>>> I think that is the most sensible.\n>>\n>> I also think that is the better approach. Dumping out the argv array\n>> is not the best end-user experience. It is just a debugging aid, and\n>> for that we have (or should extend if necessary) GIT_TRACE\n>> infrastructure. Moreover, a command that failed should have printed\n>> error messages, and it is not necessary to follow it up with another\n>> \"A child process exited with code N\" message.\n>\n> Exactly.  Stefan, please do not go in the route of butchering the\n> start_command(), finish_command(), etc. API set with the \"uncleaned\"\n> variants.\n>\n> Thanks.\n\nAfter rereading your emails just after I had the patches ready to send,\nI realized it's a bad way to go. Thanks for confirming that. :)\n"}]}