{"thread":{"id":"40446","subject":"[PATCH 1/8] submodule.c: write \"Fetching submodule \" to stderr","startedAt":"2015-09-28T23:13:58Z","lastAt":"2015-09-30T18:48:32Z","messageCount":13,"participants":["Stefan Beller","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":8},"messages":[{"id":"270890","messageId":"1443482046-25569-1-git-send-email-sbeller@google.com","threadId":"40446","inReplyTo":null,"subject":"[PATCH 0/8] fetch submodules in parallel","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-09-28T23:13:58Z","receivedAt":"2015-09-28T23:13:58Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Changes to v4: (diff below)\n* Some functions wanted to be static (Thanks Ramsay!)\n* The patch to factor out return code handling has been dropped as\n the return code handling is slightly different in finish_command and\n the parallel case.\n* We can handle signals a bit more gracefully now.\n* More documentation in run-command.h \n* I thought it is a good idea to introduce `sigchain_pop_common`.\n\nJonathan Nieder (1):\n submodule.c: write \"Fetching submodule \" to stderr\n\nStefan Beller (7):\n xread: poll on non blocking fds\n xread_nonblock: add functionality to read from fds without blocking\n strbuf: add strbuf_read_once to read without blocking\n sigchain: add command to pop all common signals\n run-command: add an asynchronous parallel child processor\n fetch_populated_submodules: use new parallel job processing\n submodules: allow parallel fetching, add tests and documentation\n\n Documentation/fetch-options.txt | 7 +\n builtin/fetch.c | 6 +-\n builtin/pull.c | 6 +\n git-compat-util.h | 1 +\n run-command.c | 348 ++++++++++++++++++++++++++++++++++++++++\n run-command.h | 63 ++++++++\n sigchain.c | 9 ++\n sigchain.h | 1 +\n strbuf.c | 11 ++\n strbuf.h | 9 ++\n submodule.c | 127 +++++++++++----\n submodule.h | 2 +-\n t/t0061-run-command.sh | 20 +++\n t/t5526-fetch-submodules.sh | 70 +++++---\n test-run-command.c | 24 +++\n wrapper.c | 35 +++-\n 16 files changed, 675 insertions(+), 64 deletions(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex 494e1f8..df84985 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -234,35 +234,6 @@ static inline void set_cloexec(int fd)\n \t\tfcntl(fd, F_SETFD, flags | FD_CLOEXEC);\n }\n \n-static int determine_return_value(int wait_status,\n-\t\t\t\t int *result,\n-\t\t\t\t int *error_code,\n-\t\t\t\t const char *argv0)\n-{\n-\tif (WIFSIGNALED(wait_status)) {\n-\t\t*result = WTERMSIG(wait_status);\n-\t\tif (*result != SIGINT && *result != SIGQUIT)\n-\t\t\terror(\"%s died of signal %d\", argv0, *result);\n-\t\t/*\n-\t\t * This return value is chosen so that code & 0xff\n-\t\t * mimics the exit code that a POSIX shell would report for\n-\t\t * a program that died from this signal.\n-\t\t */\n-\t\t*result += 128;\n-\t} else if (WIFEXITED(wait_status)) {\n-\t\t*result = WEXITSTATUS(wait_status);\n-\t\t/*\n-\t\t * Convert special exit code when execvp failed.\n-\t\t */\n-\t\tif (*result == 127) {\n-\t\t\t*result = -1;\n-\t\t\t*error_code = ENOENT;\n-\t\t}\n-\t} else\n-\t\treturn -1;\n-\treturn 0;\n-}\n-\n static int wait_or_whine(pid_t pid, const char *argv0)\n {\n \tint status, code = -1;\n@@ -275,12 +246,29 @@ static int wait_or_whine(pid_t pid, const char *argv0)\n \tif (waiting < 0) {\n \t\tfailed_errno = errno;\n \t\terror(\"waitpid for %s failed: %s\", argv0, strerror(errno));\n+\t} else if (waiting != pid) {\n+\t\terror(\"waitpid is confused (%s)\", argv0);\n+\t} else if (WIFSIGNALED(status)) {\n+\t\tcode = WTERMSIG(status);\n+\t\tif (code != SIGINT && code != SIGQUIT)\n+\t\t\terror(\"%s died of signal %d\", argv0, code);\n+\t\t/*\n+\t\t * This return value is chosen so that code & 0xff\n+\t\t * mimics the exit code that a POSIX shell would report for\n+\t\t * a program that died from this signal.\n+\t\t */\n+\t\tcode += 128;\n+\t} else if (WIFEXITED(status)) {\n+\t\tcode = WEXITSTATUS(status);\n+\t\t/*\n+\t\t * Convert special exit code when execvp failed.\n+\t\t */\n+\t\tif (code == 127) {\n+\t\t\tcode = -1;\n+\t\t\tfailed_errno = ENOENT;\n+\t\t}\n \t} else {\n-\t\tif (waiting != pid || (determine_return_value(status,\n-\t\t\t\t\t\t\t &code,\n-\t\t\t\t\t\t\t &failed_errno,\n-\t\t\t\t\t\t\t argv0) < 0))\n-\t\t\terror(\"waitpid is confused (%s)\", argv0);\n+\t\terror(\"waitpid is confused (%s)\", argv0);\n \t}\n \n \tclear_child_for_cleanup(pid);\n@@ -888,46 +876,67 @@ struct parallel_processes {\n \t */\n \tstruct pollfd *pfd;\n \n+\tunsigned shutdown : 1;\n+\n \tint output_owner;\n \tstruct strbuf buffered_output; /* of finished children */\n-};\n+} parallel_processes_struct;\n \n-void default_start_failure(void *data,\n-\t\t\t struct child_process *cp,\n-\t\t\t struct strbuf *err)\n+static int default_start_failure(void *data,\n+\t\t\t\t struct child_process *cp,\n+\t\t\t\t struct strbuf *err)\n {\n \tint i;\n-\tstruct strbuf sb = STRBUF_INIT;\n \n+\tstrbuf_addstr(err, \"Starting a child failed:\");\n \tfor (i = 0; cp->argv[i]; i++)\n-\t\tstrbuf_addf(&sb, \" %s\", cp->argv[i]);\n+\t\tstrbuf_addf(err, \" %s\", cp->argv[i]);\n \n-\tdie_errno(\"Starting a child failed:%s\", sb.buf);\n+\treturn 0;\n }\n \n-void default_return_value(void *data,\n-\t\t\t struct child_process *cp,\n-\t\t\t int result)\n+static int default_return_value(void *data,\n+\t\t\t\tstruct child_process *cp,\n+\t\t\t\tstruct strbuf *err,\n+\t\t\t\tint result)\n {\n \tint i;\n-\tstruct strbuf sb = STRBUF_INIT;\n \n \tif (!result)\n-\t\treturn;\n+\t\treturn 0;\n \n+\tstrbuf_addf(err, \"A child failed with return code %d:\", result);\n \tfor (i = 0; cp->argv[i]; i++)\n-\t\tstrbuf_addf(&sb, \" %s\", cp->argv[i]);\n+\t\tstrbuf_addf(err, \" %s\", cp->argv[i]);\n \n-\tdie_errno(\"A child failed with return code %d:%s\", result, sb.buf);\n+\treturn 0;\n }\n \n-static void pp_init(struct parallel_processes *pp,\n-\t\t\t\t\tint n, void *data,\n-\t\t\t\t\tget_next_task_fn get_next_task,\n-\t\t\t\t\tstart_failure_fn start_failure,\n-\t\t\t\t\treturn_value_fn return_value)\n+static void kill_children(struct parallel_processes *pp, int signo)\n+{\n+\tint i, n = pp->max_processes;\n+\n+\tfor (i = 0; i < n; i++)\n+\t\tif (pp->children[i].in_use)\n+\t\t\tkill(pp->children[i].process.pid, signo);\n+}\n+\n+static void handle_children_on_signal(int signo)\n+{\n+\tstruct parallel_processes *pp = ¶llel_processes_struct;\n+\n+\tkill_children(pp, signo);\n+\tsigchain_pop(signo);\n+\traise(signo);\n+}\n+\n+static struct parallel_processes *pp_init(int n, void *data,\n+\t\t\t\t\t get_next_task_fn get_next_task,\n+\t\t\t\t\t start_failure_fn start_failure,\n+\t\t\t\t\t return_value_fn return_value)\n {\n \tint i;\n+\tstruct parallel_processes *pp = ¶llel_processes_struct;\n \n \tif (n < 1)\n \t\tn = online_cpus();\n@@ -952,6 +961,8 @@ static void pp_init(struct parallel_processes *pp,\n \t\tpp->pfd[i].events = POLLIN;\n \t\tpp->pfd[i].fd = -1;\n \t}\n+\tsigchain_push_common(handle_children_on_signal);\n+\treturn pp;\n }\n \n static void pp_cleanup(struct parallel_processes *pp)\n@@ -964,6 +975,8 @@ static void pp_cleanup(struct parallel_processes *pp)\n \tfree(pp->children);\n \tfree(pp->pfd);\n \tstrbuf_release(&pp->buffered_output);\n+\n+\tsigchain_pop_common();\n }\n \n static void set_nonblocking(int fd)\n@@ -977,7 +990,12 @@ static void set_nonblocking(int fd)\n \t\t\t\"output will be degraded\");\n }\n \n-/* returns 1 if a process was started, 0 otherwise */\n+/* returns\n+ * 0 if a new task was started.\n+ * 1 if no new jobs was started (get_next_task ran out of work, non critical\n+ * problem with starting a new command)\n+ * -1 no new job was started, user wishes to shutdown early.\n+ */\n static int pp_start_one(struct parallel_processes *pp)\n {\n \tint i;\n@@ -993,10 +1011,14 @@ static int pp_start_one(struct parallel_processes *pp)\n \t\t\t &pp->children[i].err))\n \t\treturn 1;\n \n-\tif (start_command(&pp->children[i].process))\n-\t\tpp->start_failure(pp->data,\n-\t\t\t\t &pp->children[i].process,\n-\t\t\t\t &pp->children[i].err);\n+\tif (start_command(&pp->children[i].process)) {\n+\t\tint code = pp->start_failure(pp->data,\n+\t\t\t\t\t &pp->children[i].process,\n+\t\t\t\t\t &pp->children[i].err);\n+\t\tstrbuf_addbuf(&pp->buffered_output, &pp->children[i].err);\n+\t\tstrbuf_reset(&pp->children[i].err);\n+\t\treturn code ? -1 : 1;\n+\t}\n \n \tset_nonblocking(pp->children[i].process.err);\n \n@@ -1006,11 +1028,11 @@ static int pp_start_one(struct parallel_processes *pp)\n \treturn 0;\n }\n \n-static void pp_buffer_stderr(struct parallel_processes *pp)\n+static void pp_buffer_stderr(struct parallel_processes *pp, int output_timeout)\n {\n \tint i;\n \n-\twhile ((i = poll(pp->pfd, pp->max_processes, 100)) < 0) {\n+\twhile ((i = poll(pp->pfd, pp->max_processes, output_timeout)) < 0) {\n \t\tif (errno == EINTR)\n \t\t\tcontinue;\n \t\tpp_cleanup(pp);\n@@ -1038,17 +1060,18 @@ static void pp_output(struct parallel_processes *pp)\n \t}\n }\n \n-static void pp_collect_finished(struct parallel_processes *pp)\n+static int pp_collect_finished(struct parallel_processes *pp)\n {\n \tint i = 0;\n \tpid_t pid;\n \tint wait_status, code;\n \tint n = pp->max_processes;\n+\tint result = 0;\n \n \twhile (pp->nr_processes > 0) {\n \t\tpid = waitpid(-1, &wait_status, WNOHANG);\n \t\tif (pid == 0)\n-\t\t\treturn;\n+\t\t\treturn 0;\n \n \t\tif (pid < 0)\n \t\t\tdie_errno(\"wait\");\n@@ -1064,12 +1087,38 @@ static void pp_collect_finished(struct parallel_processes *pp)\n \t\t\t\tpp->children[i].process.err, 0) < 0)\n \t\t\tdie_errno(\"strbuf_read\");\n \n-\t\tif (determine_return_value(wait_status, &code, &errno,\n-\t\t\t\t\t pp->children[i].process.argv[0]) < 0)\n-\t\t\terror(\"waitpid is confused (%s)\",\n-\t\t\t pp->children[i].process.argv[0]);\n+\t\tif (WIFSIGNALED(wait_status)) {\n+\t\t\tcode = WTERMSIG(wait_status);\n+\t\t\tif (!pp->shutdown &&\n+\t\t\t code != SIGINT && code != SIGQUIT)\n+\t\t\t\tstrbuf_addf(&pp->children[i].err,\n+\t\t\t\t\t \"%s died of signal %d\",\n+\t\t\t\t\t pp->children[i].process.argv[0],\n+\t\t\t\t\t code);\n+\t\t\t/*\n+\t\t\t * This return value is chosen so that code & 0xff\n+\t\t\t * mimics the exit code that a POSIX shell would report for\n+\t\t\t * a program that died from this signal.\n+\t\t\t */\n+\t\t\tcode += 128;\n+\t\t} else if (WIFEXITED(wait_status)) {\n+\t\t\tcode = WEXITSTATUS(wait_status);\n+\t\t\t/*\n+\t\t\t * Convert special exit code when execvp failed.\n+\t\t\t */\n+\t\t\tif (code == 127) {\n+\t\t\t\tcode = -1;\n+\t\t\t\terrno = ENOENT;\n+\t\t\t}\n+\t\t} else\n+\t\t\tstrbuf_addf(&pp->children[i].err,\n+\t\t\t\t \"waitpid is confused (%s)\",\n+\t\t\t\t pp->children[i].process.argv[0]);\n+\n \n-\t\tpp->return_value(pp->data, &pp->children[i].process, code);\n+\t\tif (pp->return_value(pp->data, &pp->children[i].process,\n+\t\t\t\t &pp->children[i].err, code))\n+\t\t\tresult = 1;\n \n \t\targv_array_clear(&pp->children[i].process.args);\n \t\targv_array_clear(&pp->children[i].process.env_array);\n@@ -1103,6 +1152,7 @@ static void pp_collect_finished(struct parallel_processes *pp)\n \t\t\tpp->output_owner = (pp->output_owner + i) % n;\n \t\t}\n \t}\n+\treturn result;\n }\n \n int run_processes_parallel(int n, void *data,\n@@ -1110,21 +1160,43 @@ int run_processes_parallel(int n, void *data,\n \t\t\t start_failure_fn start_failure,\n \t\t\t return_value_fn return_value)\n {\n-\tstruct parallel_processes pp;\n-\tpp_init(&pp, n, data, get_next_task, start_failure, return_value);\n+\tint no_more_task = 0;\n+\tstruct parallel_processes *pp;\n \n+\tpp = pp_init(n, data, get_next_task, start_failure, return_value);\n \twhile (1) {\n-\t\twhile (pp.nr_processes < pp.max_processes &&\n-\t\t !pp_start_one(&pp))\n-\t\t\t; /* nothing */\n-\t\tif (!pp.nr_processes)\n+\t\tint i;\n+\t\tint output_timeout = 100;\n+\t\tint spawn_cap = 4;\n+\n+\t\tif (!no_more_task) {\n+\t\t\tfor (i = 0; i < spawn_cap; i++) {\n+\t\t\t\tint code;\n+\t\t\t\tif (pp->nr_processes == pp->max_processes)\n+\t\t\t\t\tbreak;\n+\n+\t\t\t\tcode = pp_start_one(pp);\n+\t\t\t\tif (!code)\n+\t\t\t\t\tcontinue;\n+\t\t\t\tif (code < 0) {\n+\t\t\t\t\tpp->shutdown = 1;\n+\t\t\t\t\tkill_children(pp, SIGTERM);\n+\t\t\t\t}\n+\t\t\t\tno_more_task = 1;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t}\n+\t\tif (no_more_task && !pp->nr_processes)\n \t\t\tbreak;\n-\t\tpp_buffer_stderr(&pp);\n-\t\tpp_output(&pp);\n-\t\tpp_collect_finished(&pp);\n+\t\tpp_buffer_stderr(pp, output_timeout);\n+\t\tpp_output(pp);\n+\t\tif (pp_collect_finished(pp)) {\n+\t\t\tkill_children(pp, SIGTERM);\n+\t\t\tpp->shutdown = 1;\n+\t\t\tno_more_task = 1;\n+\t\t}\n \t}\n \n-\tpp_cleanup(&pp);\n-\n+\tpp_cleanup(pp);\n \treturn 0;\n }\ndiff --git a/run-command.h b/run-command.h\nindex 3807fd1..1179cb0 100644\n--- a/run-command.h\n+++ b/run-command.h\n@@ -132,13 +132,36 @@ typedef int (*get_next_task_fn)(void *data,\n \t\t\t\tstruct child_process *cp,\n \t\t\t\tstruct strbuf *err);\n \n-typedef void (*start_failure_fn)(void *data,\n-\t\t\t\t struct child_process *cp,\n-\t\t\t\t struct strbuf *err);\n-\n-typedef void (*return_value_fn)(void *data,\n+/**\n+ * This callback is called whenever there are problems starting\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+ * messing up the output of the other parallel processes.\n+ *\n+ * Return 0 to continue the parallel processing. To abort gracefully,\n+ * return non zero.\n+ */\n+typedef int (*start_failure_fn)(void *data,\n \t\t\t\tstruct child_process *cp,\n-\t\t\t\tint result);\n+\t\t\t\tstruct strbuf *err);\n+\n+/**\n+ * This callback is called on every there are problems starting\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+ * messing up the output of the other parallel processes.\n+ *\n+ * Return 0 to continue the parallel processing. To abort gracefully,\n+ * return non zero.\n+ */\n+typedef int (*return_value_fn)(void *data,\n+\t\t\t struct child_process *cp,\n+\t\t\t struct strbuf *err,\n+\t\t\t int result);\n \n /**\n * Runs up to n processes at the same time. Whenever a process can be\n@@ -148,6 +171,10 @@ typedef void (*return_value_fn)(void *data,\n * The children started via this function run in parallel and their output\n * to stderr is buffered, while one of the children will directly output\n * to stderr.\n+ *\n+ * If start_failure_fn and return_value_fn are NULL, default handlers\n+ * will be used. The default handlers will print an error message on\n+ * error without issuing an emergency stop.\n */\n \n int run_processes_parallel(int n, void *data,\ndiff --git a/sigchain.c b/sigchain.c\nindex faa375d..9262307 100644\n--- a/sigchain.c\n+++ b/sigchain.c\n@@ -50,3 +50,12 @@ void sigchain_push_common(sigchain_fun f)\n \tsigchain_push(SIGQUIT, f);\n \tsigchain_push(SIGPIPE, f);\n }\n+\n+void sigchain_pop_common(void)\n+{\n+\tsigchain_pop(SIGINT);\n+\tsigchain_pop(SIGHUP);\n+\tsigchain_pop(SIGTERM);\n+\tsigchain_pop(SIGQUIT);\n+\tsigchain_pop(SIGPIPE);\n+}\ndiff --git a/sigchain.h b/sigchain.h\nindex 618083b..138b20f 100644\n--- a/sigchain.h\n+++ b/sigchain.h\n@@ -7,5 +7,6 @@ int sigchain_push(int sig, sigchain_fun f);\n int sigchain_pop(int sig);\n \n void sigchain_push_common(sigchain_fun f);\n+void sigchain_pop_common(void);\n \n #endif /* SIGCHAIN_H */\ndiff --git a/submodule.c b/submodule.c\nindex fdaf3e4..7ab89f4 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -630,18 +630,25 @@ struct submodule_parallel_fetch {\n int get_next_submodule(void *data, struct child_process *cp,\n \t\t struct strbuf *err);\n \n-void handle_submodule_fetch_start_err(void *data, struct child_process *cp, struct strbuf *err)\n+static int fetch_start_failure(void *data, struct child_process *cp,\n+\t\t\t struct strbuf *err)\n {\n \tstruct submodule_parallel_fetch *spf = data;\n+\n \tspf->result = 1;\n+\n+\treturn 0;\n }\n \n-void handle_submodule_fetch_finish( void *data, struct child_process *cp, int retvalue)\n+static int fetch_finish(void *data, struct child_process *cp,\n+\t\t\tstruct strbuf *err, int retvalue)\n {\n \tstruct submodule_parallel_fetch *spf = data;\n \n \tif (retvalue)\n \t\tspf->result = 1;\n+\n+\treturn 0;\n }\n \n int fetch_populated_submodules(const struct argv_array *options,\n@@ -671,8 +678,8 @@ int fetch_populated_submodules(const struct argv_array *options,\n \tcalculate_changed_submodule_paths();\n \trun_processes_parallel(max_parallel_jobs, &spf,\n \t\t\t get_next_submodule,\n-\t\t\t handle_submodule_fetch_start_err,\n-\t\t\t handle_submodule_fetch_finish);\n+\t\t\t fetch_start_failure,\n+\t\t\t fetch_finish);\n \n \targv_array_clear(&spf.args);\n out:\ndiff --git a/test-run-command.c b/test-run-command.c\nindex 94c6eee..2555791 100644\n--- a/test-run-command.c\n+++ b/test-run-command.c\n@@ -16,9 +16,9 @@\n #include \n \n static int number_callbacks;\n-int parallel_next(void *data,\n-\t\t struct child_process *cp,\n-\t\t struct strbuf *err)\n+static int parallel_next(void *data,\n+\t\t\t struct child_process *cp,\n+\t\t\t struct strbuf *err)\n {\n \tstruct child_process *d = data;\n \tif (number_callbacks >= 4)\n\n\n-- \n2.5.0.273.g6fa2560.dirty\n"},{"id":"270889","messageId":"1443482046-25569-2-git-send-email-sbeller@google.com","threadId":"40446","inReplyTo":"1443482046-25569-1-git-send-email-sbeller@google.com","subject":"[PATCH 1/8] submodule.c: write \"Fetching submodule \" to stderr","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-09-28T23:13:59Z","receivedAt":"2015-09-28T23:13:59Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Jonathan Nieder \n\nThe \"Pushing submodule \" progress output correctly goes to\nstderr, but \"Fetching submodule \" is going to stdout by\nmistake. Fix it to write to stderr.\n\nNoticed while trying to implement a parallel submodule fetch. When\nthis particular output line went to a different file descriptor, it\nwas buffered separately, resulting in wrongly interleaved output if\nwe copied it to the terminal naively.\n\nSigned-off-by: Jonathan Nieder \nSigned-off-by: Stefan Beller \nSigned-off-by: Junio C Hamano \n---\n submodule.c | 2 +-\n t/t5526-fetch-submodules.sh | 51 +++++++++++++++++++++++----------------------\n 2 files changed, 27 insertions(+), 26 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex 9fcc86f..1d64e57 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -694,7 +694,7 @@ int fetch_populated_submodules(const struct argv_array *options,\n \t\t\tgit_dir = submodule_git_dir.buf;\n \t\tif (is_directory(git_dir)) {\n \t\t\tif (!quiet)\n-\t\t\t\tprintf(\"Fetching submodule %s%s\\n\", prefix, ce->name);\n+\t\t\t\tfprintf(stderr, \"Fetching submodule %s%s\\n\", prefix, ce->name);\n \t\t\tcp.dir = submodule_path.buf;\n \t\t\targv_array_push(&argv, default_argv);\n \t\t\targv_array_push(&argv, \"--submodule-prefix\");\ndiff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh\nindex a4532b0..17759b1 100755\n--- a/t/t5526-fetch-submodules.sh\n+++ b/t/t5526-fetch-submodules.sh\n@@ -16,7 +16,8 @@ add_upstream_commit() {\n \t\tgit add subfile &&\n \t\tgit commit -m new subfile &&\n \t\thead2=$(git rev-parse --short HEAD) &&\n-\t\techo \"From $pwd/submodule\" > ../expect.err &&\n+\t\techo \"Fetching submodule submodule\" > ../expect.err &&\n+\t\techo \"From $pwd/submodule\" >> ../expect.err &&\n \t\techo \" $head1..$head2 master -> origin/master\" >> ../expect.err\n \t) &&\n \t(\n@@ -27,6 +28,7 @@ add_upstream_commit() {\n \t\tgit add deepsubfile &&\n \t\tgit commit -m new deepsubfile &&\n \t\thead2=$(git rev-parse --short HEAD) &&\n+\t\techo \"Fetching submodule submodule/subdir/deepsubmodule\" >> ../expect.err\n \t\techo \"From $pwd/deepsubmodule\" >> ../expect.err &&\n \t\techo \" $head1..$head2 master -> origin/master\" >> ../expect.err\n \t)\n@@ -56,9 +58,7 @@ test_expect_success setup '\n \t(\n \t\tcd downstream &&\n \t\tgit submodule update --init --recursive\n-\t) &&\n-\techo \"Fetching submodule submodule\" > expect.out &&\n-\techo \"Fetching submodule submodule/subdir/deepsubmodule\" >> expect.out\n+\t)\n '\n \n test_expect_success \"fetch --recurse-submodules recurses into submodules\" '\n@@ -67,7 +67,7 @@ test_expect_success \"fetch --recurse-submodules recurses into submodules\" '\n \t\tcd downstream &&\n \t\tgit fetch --recurse-submodules >../actual.out 2>../actual.err\n \t) &&\n-\ttest_i18ncmp expect.out actual.out &&\n+\ttest_must_be_empty actual.out &&\n \ttest_i18ncmp expect.err actual.err\n '\n \n@@ -96,7 +96,7 @@ test_expect_success \"using fetchRecurseSubmodules=true in .gitmodules recurses i\n \t\tgit config -f .gitmodules submodule.submodule.fetchRecurseSubmodules true &&\n \t\tgit fetch >../actual.out 2>../actual.err\n \t) &&\n-\ttest_i18ncmp expect.out actual.out &&\n+\ttest_must_be_empty actual.out &&\n \ttest_i18ncmp expect.err actual.err\n '\n \n@@ -127,7 +127,7 @@ test_expect_success \"--recurse-submodules overrides fetchRecurseSubmodules setti\n \t\tgit config --unset -f .gitmodules submodule.submodule.fetchRecurseSubmodules &&\n \t\tgit config --unset submodule.submodule.fetchRecurseSubmodules\n \t) &&\n-\ttest_i18ncmp expect.out actual.out &&\n+\ttest_must_be_empty actual.out &&\n \ttest_i18ncmp expect.err actual.err\n '\n \n@@ -146,7 +146,7 @@ test_expect_success \"--dry-run propagates to submodules\" '\n \t\tcd downstream &&\n \t\tgit fetch --recurse-submodules --dry-run >../actual.out 2>../actual.err\n \t) &&\n-\ttest_i18ncmp expect.out actual.out &&\n+\ttest_must_be_empty actual.out &&\n \ttest_i18ncmp expect.err actual.err\n '\n \n@@ -155,7 +155,7 @@ test_expect_success \"Without --dry-run propagates to submodules\" '\n \t\tcd downstream &&\n \t\tgit fetch --recurse-submodules >../actual.out 2>../actual.err\n \t) &&\n-\ttest_i18ncmp expect.out actual.out &&\n+\ttest_must_be_empty actual.out &&\n \ttest_i18ncmp expect.err actual.err\n '\n \n@@ -166,7 +166,7 @@ test_expect_success \"recurseSubmodules=true propagates into submodules\" '\n \t\tgit config fetch.recurseSubmodules true\n \t\tgit fetch >../actual.out 2>../actual.err\n \t) &&\n-\ttest_i18ncmp expect.out actual.out &&\n+\ttest_must_be_empty actual.out &&\n \ttest_i18ncmp expect.err actual.err\n '\n \n@@ -180,7 +180,7 @@ test_expect_success \"--recurse-submodules overrides config in submodule\" '\n \t\t) &&\n \t\tgit fetch --recurse-submodules >../actual.out 2>../actual.err\n \t) &&\n-\ttest_i18ncmp expect.out actual.out &&\n+\ttest_must_be_empty actual.out &&\n \ttest_i18ncmp expect.err actual.err\n '\n \n@@ -214,16 +214,15 @@ test_expect_success \"Recursion stops when no new submodule commits are fetched\"\n \tgit add submodule &&\n \tgit commit -m \"new submodule\" &&\n \thead2=$(git rev-parse --short HEAD) &&\n-\techo \"Fetching submodule submodule\" > expect.out.sub &&\n \techo \"From $pwd/.\" > expect.err.sub &&\n \techo \" $head1..$head2 master -> origin/master\" >>expect.err.sub &&\n-\thead -2 expect.err >> expect.err.sub &&\n+\thead -3 expect.err >> expect.err.sub &&\n \t(\n \t\tcd downstream &&\n \t\tgit fetch >../actual.out 2>../actual.err\n \t) &&\n \ttest_i18ncmp expect.err.sub actual.err &&\n-\ttest_i18ncmp expect.out.sub actual.out\n+\ttest_must_be_empty actual.out\n '\n \n test_expect_success \"Recursion doesn't happen when new superproject commits don't change any submodules\" '\n@@ -269,7 +268,7 @@ test_expect_success \"Recursion picks up config in submodule\" '\n \t\t)\n \t) &&\n \ttest_i18ncmp expect.err.sub actual.err &&\n-\ttest_i18ncmp expect.out actual.out\n+\ttest_must_be_empty actual.out\n '\n \n test_expect_success \"Recursion picks up all submodules when necessary\" '\n@@ -285,7 +284,8 @@ test_expect_success \"Recursion picks up all submodules when necessary\" '\n \t\tgit add subdir/deepsubmodule &&\n \t\tgit commit -m \"new deepsubmodule\"\n \t\thead2=$(git rev-parse --short HEAD) &&\n-\t\techo \"From $pwd/submodule\" > ../expect.err.sub &&\n+\t\techo \"Fetching submodule submodule\" > ../expect.err.sub &&\n+\t\techo \"From $pwd/submodule\" >> ../expect.err.sub &&\n \t\techo \" $head1..$head2 master -> origin/master\" >> ../expect.err.sub\n \t) &&\n \thead1=$(git rev-parse --short HEAD) &&\n@@ -295,13 +295,13 @@ test_expect_success \"Recursion picks up all submodules when necessary\" '\n \techo \"From $pwd/.\" > expect.err.2 &&\n \techo \" $head1..$head2 master -> origin/master\" >> expect.err.2 &&\n \tcat expect.err.sub >> expect.err.2 &&\n-\ttail -2 expect.err >> expect.err.2 &&\n+\ttail -3 expect.err >> expect.err.2 &&\n \t(\n \t\tcd downstream &&\n \t\tgit fetch >../actual.out 2>../actual.err\n \t) &&\n \ttest_i18ncmp expect.err.2 actual.err &&\n-\ttest_i18ncmp expect.out actual.out\n+\ttest_must_be_empty actual.out\n '\n \n test_expect_success \"'--recurse-submodules=on-demand' doesn't recurse when no new commits are fetched in the superproject (and ignores config)\" '\n@@ -317,7 +317,8 @@ test_expect_success \"'--recurse-submodules=on-demand' doesn't recurse when no ne\n \t\tgit add subdir/deepsubmodule &&\n \t\tgit commit -m \"new deepsubmodule\" &&\n \t\thead2=$(git rev-parse --short HEAD) &&\n-\t\techo \"From $pwd/submodule\" > ../expect.err.sub &&\n+\t\techo Fetching submodule submodule > ../expect.err.sub &&\n+\t\techo \"From $pwd/submodule\" >> ../expect.err.sub &&\n \t\techo \" $head1..$head2 master -> origin/master\" >> ../expect.err.sub\n \t) &&\n \t(\n@@ -335,7 +336,7 @@ test_expect_success \"'--recurse-submodules=on-demand' recurses as deep as necess\n \tgit add submodule &&\n \tgit commit -m \"new submodule\" &&\n \thead2=$(git rev-parse --short HEAD) &&\n-\ttail -2 expect.err > expect.err.deepsub &&\n+\ttail -3 expect.err > expect.err.deepsub &&\n \techo \"From $pwd/.\" > expect.err &&\n \techo \" $head1..$head2 master -> origin/master\" >>expect.err &&\n \tcat expect.err.sub >> expect.err &&\n@@ -354,7 +355,7 @@ test_expect_success \"'--recurse-submodules=on-demand' recurses as deep as necess\n \t\t\tgit config --unset -f .gitmodules submodule.subdir/deepsubmodule.fetchRecursive\n \t\t)\n \t) &&\n-\ttest_i18ncmp expect.out actual.out &&\n+\ttest_must_be_empty actual.out &&\n \ttest_i18ncmp expect.err actual.err\n '\n \n@@ -388,7 +389,7 @@ test_expect_success \"'fetch.recurseSubmodules=on-demand' overrides global config\n \thead2=$(git rev-parse --short HEAD) &&\n \techo \"From $pwd/.\" > expect.err.2 &&\n \techo \" $head1..$head2 master -> origin/master\" >>expect.err.2 &&\n-\thead -2 expect.err >> expect.err.2 &&\n+\thead -3 expect.err >> expect.err.2 &&\n \t(\n \t\tcd downstream &&\n \t\tgit config fetch.recurseSubmodules on-demand &&\n@@ -399,7 +400,7 @@ test_expect_success \"'fetch.recurseSubmodules=on-demand' overrides global config\n \t\tcd downstream &&\n \t\tgit config --unset fetch.recurseSubmodules\n \t) &&\n-\ttest_i18ncmp expect.out.sub actual.out &&\n+\ttest_must_be_empty actual.out &&\n \ttest_i18ncmp expect.err.2 actual.err\n '\n \n@@ -416,7 +417,7 @@ test_expect_success \"'submodule..fetchRecurseSubmodules=on-demand' override\n \thead2=$(git rev-parse --short HEAD) &&\n \techo \"From $pwd/.\" > expect.err.2 &&\n \techo \" $head1..$head2 master -> origin/master\" >>expect.err.2 &&\n-\thead -2 expect.err >> expect.err.2 &&\n+\thead -3 expect.err >> expect.err.2 &&\n \t(\n \t\tcd downstream &&\n \t\tgit config submodule.submodule.fetchRecurseSubmodules on-demand &&\n@@ -427,7 +428,7 @@ test_expect_success \"'submodule..fetchRecurseSubmodules=on-demand' override\n \t\tcd downstream &&\n \t\tgit config --unset submodule.submodule.fetchRecurseSubmodules\n \t) &&\n-\ttest_i18ncmp expect.out.sub actual.out &&\n+\ttest_must_be_empty actual.out &&\n \ttest_i18ncmp expect.err.2 actual.err\n '\n \n-- \n2.5.0.273.g6fa2560.dirty\n"},{"id":"270894","messageId":"1443482046-25569-3-git-send-email-sbeller@google.com","threadId":"40446","inReplyTo":"1443482046-25569-1-git-send-email-sbeller@google.com","subject":"[PATCH 2/8] xread: poll on non blocking fds","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-09-28T23:14:00Z","receivedAt":"2015-09-28T23:14:00Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":">From the man page:\nEAGAIN The file descriptor fd refers to a file other than a socket\n and has been marked nonblocking (O_NONBLOCK), and the read\n would block.\n\nEAGAIN or EWOULDBLOCK\n The file descriptor fd refers to a socket and has been marked\n nonblocking (O_NONBLOCK), and the read would block. POSIX.1-2001\n allows either error to be returned for this case, and does not\n require these constants to have the same value, so a portable\n application should check for both possibilities.\n\nIf we get an EAGAIN or EWOULDBLOCK the fd must have set O_NONBLOCK.\nAs the intent of xread is to read as much as possible either until the\nfd is EOF or an actual error occurs, we can ease the feeder of the fd\nby not spinning the whole time, but rather wait for it politely by not\nbusy waiting.\n\nWe should not care if the call to poll failed, as we're in an infinite\nloop and can only get out with the correct read().\n\nSigned-off-by: Stefan Beller \nSigned-off-by: Junio C Hamano \n---\n wrapper.c | 13 +++++++++++--\n 1 file changed, 11 insertions(+), 2 deletions(-)\n\ndiff --git a/wrapper.c b/wrapper.c\nindex ff49807..5517928 100644\n--- a/wrapper.c\n+++ b/wrapper.c\n@@ -201,8 +201,17 @@ ssize_t xread(int fd, void *buf, size_t len)\n \t len = MAX_IO_SIZE;\n \twhile (1) {\n \t\tnr = read(fd, buf, len);\n-\t\tif ((nr < 0) && (errno == EAGAIN || errno == EINTR))\n-\t\t\tcontinue;\n+\t\tif (nr < 0) {\n+\t\t\tif (errno == EINTR)\n+\t\t\t\tcontinue;\n+\t\t\tif (errno == EAGAIN || errno == EWOULDBLOCK) {\n+\t\t\t\tstruct pollfd pfd;\n+\t\t\t\tpfd.events = POLLIN;\n+\t\t\t\tpfd.fd = fd;\n+\t\t\t\t/* We deliberately ignore the return value */\n+\t\t\t\tpoll(&pfd, 1, -1);\n+\t\t\t}\n+\t\t}\n \t\treturn nr;\n \t}\n }\n-- \n2.5.0.273.g6fa2560.dirty\n"},{"id":"270891","messageId":"1443482046-25569-4-git-send-email-sbeller@google.com","threadId":"40446","inReplyTo":"1443482046-25569-1-git-send-email-sbeller@google.com","subject":"[PATCH 3/8] xread_nonblock: add functionality to read from fds without blocking","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-09-28T23:14:01Z","receivedAt":"2015-09-28T23:14:01Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Provide a wrapper to read(), similar to xread(), that restarts on\nEINTR but not EAGAIN (or EWOULDBLOCK). This enables the caller to\nhandle polling itself, possibly polling multiple sockets or performing\nsome other action.\n\nHelped-by: Jacob Keller \nHelped-by: Jeff King ,\nHelped-by: Junio C Hamano \nSigned-off-by: Stefan Beller \nSigned-off-by: Junio C Hamano \n---\n git-compat-util.h | 1 +\n wrapper.c | 22 ++++++++++++++++++++++\n 2 files changed, 23 insertions(+)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex c6d391f..9ccea85 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -718,6 +718,7 @@ extern void *xcalloc(size_t nmemb, size_t size);\n extern void *xmmap(void *start, size_t length, int prot, int flags, int fd, off_t offset);\n extern void *xmmap_gently(void *start, size_t length, int prot, int flags, int fd, off_t offset);\n extern ssize_t xread(int fd, void *buf, size_t len);\n+extern ssize_t xread_nonblock(int fd, void *buf, size_t len);\n extern ssize_t xwrite(int fd, const void *buf, size_t len);\n extern ssize_t xpread(int fd, void *buf, size_t len, off_t offset);\n extern int xdup(int fd);\ndiff --git a/wrapper.c b/wrapper.c\nindex 5517928..41a21e1 100644\n--- a/wrapper.c\n+++ b/wrapper.c\n@@ -217,6 +217,28 @@ ssize_t xread(int fd, void *buf, size_t len)\n }\n \n /*\n+ * xread_nonblock() is the same a read(), but it automatically restarts read()\n+ * interrupted operations (EINTR). xread_nonblock() DOES NOT GUARANTEE that\n+ * \"len\" bytes is read. EWOULDBLOCK is turned into EAGAIN.\n+ */\n+ssize_t xread_nonblock(int fd, void *buf, size_t len)\n+{\n+\tssize_t nr;\n+\tif (len > MAX_IO_SIZE)\n+\t\tlen = MAX_IO_SIZE;\n+\twhile (1) {\n+\t\tnr = read(fd, buf, len);\n+\t\tif (nr < 0) {\n+\t\t\tif (errno == EINTR)\n+\t\t\t\tcontinue;\n+\t\t\tif (errno == EWOULDBLOCK)\n+\t\t\t\terrno = EAGAIN;\n+\t\t}\n+\t\treturn nr;\n+\t}\n+}\n+\n+/*\n * xwrite() is the same a write(), but it automatically restarts write()\n * operations with a recoverable error (EAGAIN and EINTR). xwrite() DOES NOT\n * GUARANTEE that \"len\" bytes is written even if the operation is successful.\n-- \n2.5.0.273.g6fa2560.dirty\n"},{"id":"270895","messageId":"1443482046-25569-5-git-send-email-sbeller@google.com","threadId":"40446","inReplyTo":"1443482046-25569-1-git-send-email-sbeller@google.com","subject":"[PATCH 4/8] strbuf: add strbuf_read_once to read without blocking","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-09-28T23:14:02Z","receivedAt":"2015-09-28T23:14:02Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The new call will read from a file descriptor into a strbuf once. The\nunderlying call xread_nonblock is meant to execute without blocking if\nthe file descriptor is set to O_NONBLOCK. It is a bug to call\nstrbuf_read_once on a file descriptor which would block.\n\nSigned-off-by: Stefan Beller \nSigned-off-by: Junio C Hamano \n---\n strbuf.c | 11 +++++++++++\n strbuf.h | 9 +++++++++\n 2 files changed, 20 insertions(+)\n\ndiff --git a/strbuf.c b/strbuf.c\nindex cce5eed..35e71b8 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -384,6 +384,17 @@ ssize_t strbuf_read(struct strbuf *sb, int fd, size_t hint)\n \treturn sb->len - oldlen;\n }\n \n+ssize_t strbuf_read_once(struct strbuf *sb, int fd, size_t hint)\n+{\n+\tssize_t cnt;\n+\n+\tstrbuf_grow(sb, hint ? hint : 8192);\n+\tcnt = xread_nonblock(fd, sb->buf + sb->len, sb->alloc - sb->len - 1);\n+\tif (cnt > 0)\n+\t\tstrbuf_setlen(sb, sb->len + cnt);\n+\treturn cnt;\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 aef2794..ea69665 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -367,6 +367,15 @@ extern size_t strbuf_fread(struct strbuf *, size_t, FILE *);\n extern ssize_t strbuf_read(struct strbuf *, int fd, size_t hint);\n \n /**\n+ * Read from a file descriptor that is marked as O_NONBLOCK without\n+ * blocking. Returns the number of new bytes appended to the sb.\n+ * Negative return value signals there was an error returned from\n+ * underlying read(2), in which case the caller should check errno.\n+ * e.g. errno == EAGAIN when the read may have blocked.\n+ */\n+extern ssize_t strbuf_read_once(struct strbuf *, int fd, size_t hint);\n+\n+/**\n * Read the contents of a file, specified by its path. The third argument\n * can be used to give a hint about the file size, to avoid reallocs.\n */\n-- \n2.5.0.273.g6fa2560.dirty\n"},{"id":"270892","messageId":"1443482046-25569-6-git-send-email-sbeller@google.com","threadId":"40446","inReplyTo":"1443482046-25569-1-git-send-email-sbeller@google.com","subject":"[PATCH 5/8] sigchain: add command to pop all common signals","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-09-28T23:14:03Z","receivedAt":"2015-09-28T23:14:03Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The new method removes all common signal handlers that were installed\nby sigchain_push.\n\nCC: Jeff King \nSigned-off-by: Stefan Beller \n---\n sigchain.c | 9 +++++++++\n sigchain.h | 1 +\n 2 files changed, 10 insertions(+)\n\ndiff --git a/sigchain.c b/sigchain.c\nindex faa375d..9262307 100644\n--- a/sigchain.c\n+++ b/sigchain.c\n@@ -50,3 +50,12 @@ void sigchain_push_common(sigchain_fun f)\n \tsigchain_push(SIGQUIT, f);\n \tsigchain_push(SIGPIPE, f);\n }\n+\n+void sigchain_pop_common(void)\n+{\n+\tsigchain_pop(SIGINT);\n+\tsigchain_pop(SIGHUP);\n+\tsigchain_pop(SIGTERM);\n+\tsigchain_pop(SIGQUIT);\n+\tsigchain_pop(SIGPIPE);\n+}\ndiff --git a/sigchain.h b/sigchain.h\nindex 618083b..138b20f 100644\n--- a/sigchain.h\n+++ b/sigchain.h\n@@ -7,5 +7,6 @@ int sigchain_push(int sig, sigchain_fun f);\n int sigchain_pop(int sig);\n \n void sigchain_push_common(sigchain_fun f);\n+void sigchain_pop_common(void);\n \n #endif /* SIGCHAIN_H */\n-- \n2.5.0.273.g6fa2560.dirty\n"},{"id":"270897","messageId":"1443482046-25569-7-git-send-email-sbeller@google.com","threadId":"40446","inReplyTo":"1443482046-25569-1-git-send-email-sbeller@google.com","subject":"[PATCH 6/8] run-command: add an asynchronous parallel child processor","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-09-28T23:14:04Z","receivedAt":"2015-09-28T23:14:04Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This allows to run external commands in parallel with ordered output\non stderr.\n\nIf we run external commands in parallel we cannot pipe the output directly\nto the our stdout/err as it would mix up. So each process's output will\nflow through a pipe, which we buffer. One subprocess can be directly\npiped to out stdout/err for a low latency feedback to the user.\n\nExample:\nLet's assume we have 5 submodules A,B,C,D,E and each fetch takes a\ndifferent amount of time as the different submodules vary in size, then\nthe output of fetches in sequential order might look like this:\n\n time -->\n output: |---A---| |-B-| |-------C-------| |-D-| |-E-|\n\nWhen we schedule these submodules into maximal two parallel processes,\na schedule and sample output over time may look like this:\n\nprocess 1: |---A---| |-D-| |-E-|\n\nprocess 2: |-B-| |-------C-------|\n\noutput: |---A---|B|---C-------|DE\n\nSo A will be perceived as it would run normally in the single child\nversion. As B has finished by the time A is done, we can dump its whole\nprogress buffer on stderr, such that it looks like it finished in no\ntime. Once that is done, C is determined to be the visible child and\nits progress will be reported in real time.\n\nSo this way of output is really good for human consumption, as it only\nchanges the timing, not the actual output.\n\nFor machine consumption the output needs to be prepared in the tasks,\nby either having a prefix per line or per block to indicate whose tasks\noutput is displayed, because the output order may not follow the\noriginal sequential ordering:\n\n |----A----| |--B--| |-C-|\n\nwill be scheduled to be all parallel:\n\nprocess 1: |----A----|\nprocess 2: |--B--|\nprocess 3: |-C-|\noutput: |----A----|CB\n\nThis happens because C finished before B did, so it will be queued for\noutput before B.\n\nSigned-off-by: Stefan Beller \nSigned-off-by: Junio C Hamano \n---\n run-command.c | 348 +++++++++++++++++++++++++++++++++++++++++++++++++\n run-command.h | 63 +++++++++\n t/t0061-run-command.sh | 20 +++\n test-run-command.c | 24 ++++\n 4 files changed, 455 insertions(+)\n\ndiff --git a/run-command.c b/run-command.c\nindex 28e1d55..df84985 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -3,6 +3,8 @@\n #include \"exec_cmd.h\"\n #include \"sigchain.h\"\n #include \"argv-array.h\"\n+#include \"thread-utils.h\"\n+#include \"strbuf.h\"\n \n void child_process_init(struct child_process *child)\n {\n@@ -852,3 +854,349 @@ int capture_command(struct child_process *cmd, struct strbuf *buf, size_t hint)\n \tclose(cmd->out);\n \treturn finish_command(cmd);\n }\n+\n+struct parallel_processes {\n+\tvoid *data;\n+\n+\tint max_processes;\n+\tint nr_processes;\n+\n+\tget_next_task_fn get_next_task;\n+\tstart_failure_fn start_failure;\n+\treturn_value_fn return_value;\n+\n+\tstruct {\n+\t\tunsigned in_use : 1;\n+\t\tstruct child_process process;\n+\t\tstruct strbuf err;\n+\t} *children;\n+\t/*\n+\t * The struct pollfd is logically part of *children,\n+\t * but the system call expects it as its own array.\n+\t */\n+\tstruct pollfd *pfd;\n+\n+\tunsigned shutdown : 1;\n+\n+\tint output_owner;\n+\tstruct strbuf buffered_output; /* of finished children */\n+} parallel_processes_struct;\n+\n+static int default_start_failure(void *data,\n+\t\t\t\t struct child_process *cp,\n+\t\t\t\t struct strbuf *err)\n+{\n+\tint i;\n+\n+\tstrbuf_addstr(err, \"Starting a child failed:\");\n+\tfor (i = 0; cp->argv[i]; i++)\n+\t\tstrbuf_addf(err, \" %s\", cp->argv[i]);\n+\n+\treturn 0;\n+}\n+\n+static int default_return_value(void *data,\n+\t\t\t\tstruct child_process *cp,\n+\t\t\t\tstruct strbuf *err,\n+\t\t\t\tint result)\n+{\n+\tint i;\n+\n+\tif (!result)\n+\t\treturn 0;\n+\n+\tstrbuf_addf(err, \"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+\n+\treturn 0;\n+}\n+\n+static void kill_children(struct parallel_processes *pp, int signo)\n+{\n+\tint i, n = pp->max_processes;\n+\n+\tfor (i = 0; i < n; i++)\n+\t\tif (pp->children[i].in_use)\n+\t\t\tkill(pp->children[i].process.pid, signo);\n+}\n+\n+static void handle_children_on_signal(int signo)\n+{\n+\tstruct parallel_processes *pp = ¶llel_processes_struct;\n+\n+\tkill_children(pp, signo);\n+\tsigchain_pop(signo);\n+\traise(signo);\n+}\n+\n+static struct parallel_processes *pp_init(int n, void *data,\n+\t\t\t\t\t get_next_task_fn get_next_task,\n+\t\t\t\t\t start_failure_fn start_failure,\n+\t\t\t\t\t return_value_fn return_value)\n+{\n+\tint i;\n+\tstruct parallel_processes *pp = ¶llel_processes_struct;\n+\n+\tif (n < 1)\n+\t\tn = online_cpus();\n+\n+\tpp->max_processes = n;\n+\tpp->data = data;\n+\tif (!get_next_task)\n+\t\tdie(\"BUG: you need to specify a get_next_task function\");\n+\tpp->get_next_task = get_next_task;\n+\n+\tpp->start_failure = start_failure ? start_failure : default_start_failure;\n+\tpp->return_value = return_value ? return_value : default_return_value;\n+\n+\tpp->nr_processes = 0;\n+\tpp->output_owner = 0;\n+\tpp->children = xcalloc(n, sizeof(*pp->children));\n+\tpp->pfd = xcalloc(n, sizeof(*pp->pfd));\n+\tstrbuf_init(&pp->buffered_output, 0);\n+\n+\tfor (i = 0; i < n; i++) {\n+\t\tstrbuf_init(&pp->children[i].err, 0);\n+\t\tpp->pfd[i].events = POLLIN;\n+\t\tpp->pfd[i].fd = -1;\n+\t}\n+\tsigchain_push_common(handle_children_on_signal);\n+\treturn pp;\n+}\n+\n+static void pp_cleanup(struct parallel_processes *pp)\n+{\n+\tint i;\n+\n+\tfor (i = 0; i < pp->max_processes; i++)\n+\t\tstrbuf_release(&pp->children[i].err);\n+\n+\tfree(pp->children);\n+\tfree(pp->pfd);\n+\tstrbuf_release(&pp->buffered_output);\n+\n+\tsigchain_pop_common();\n+}\n+\n+static void set_nonblocking(int fd)\n+{\n+\tint flags = fcntl(fd, F_GETFL);\n+\tif (flags < 0)\n+\t\twarning(\"Could not get file status flags, \"\n+\t\t\t\"output will be degraded\");\n+\telse if (fcntl(fd, F_SETFL, flags | O_NONBLOCK))\n+\t\twarning(\"Could not set file status flags, \"\n+\t\t\t\"output will be degraded\");\n+}\n+\n+/* returns\n+ * 0 if a new task was started.\n+ * 1 if no new jobs was started (get_next_task ran out of work, non critical\n+ * problem with starting a new command)\n+ * -1 no new job was started, user wishes to shutdown early.\n+ */\n+static int pp_start_one(struct parallel_processes *pp)\n+{\n+\tint i;\n+\n+\tfor (i = 0; i < pp->max_processes; i++)\n+\t\tif (!pp->children[i].in_use)\n+\t\t\tbreak;\n+\tif (i == pp->max_processes)\n+\t\tdie(\"BUG: bookkeeping is hard\");\n+\n+\tif (!pp->get_next_task(pp->data,\n+\t\t\t &pp->children[i].process,\n+\t\t\t &pp->children[i].err))\n+\t\treturn 1;\n+\n+\tif (start_command(&pp->children[i].process)) {\n+\t\tint code = pp->start_failure(pp->data,\n+\t\t\t\t\t &pp->children[i].process,\n+\t\t\t\t\t &pp->children[i].err);\n+\t\tstrbuf_addbuf(&pp->buffered_output, &pp->children[i].err);\n+\t\tstrbuf_reset(&pp->children[i].err);\n+\t\treturn code ? -1 : 1;\n+\t}\n+\n+\tset_nonblocking(pp->children[i].process.err);\n+\n+\tpp->nr_processes++;\n+\tpp->children[i].in_use = 1;\n+\tpp->pfd[i].fd = pp->children[i].process.err;\n+\treturn 0;\n+}\n+\n+static void pp_buffer_stderr(struct parallel_processes *pp, int output_timeout)\n+{\n+\tint i;\n+\n+\twhile ((i = poll(pp->pfd, pp->max_processes, output_timeout)) < 0) {\n+\t\tif (errno == EINTR)\n+\t\t\tcontinue;\n+\t\tpp_cleanup(pp);\n+\t\tdie_errno(\"poll\");\n+\t}\n+\n+\t/* Buffer output from all pipes. */\n+\tfor (i = 0; i < pp->max_processes; i++) {\n+\t\tif (pp->children[i].in_use &&\n+\t\t pp->pfd[i].revents & POLLIN)\n+\t\t\tif (strbuf_read_once(&pp->children[i].err,\n+\t\t\t\t\t pp->children[i].process.err, 0) < 0)\n+\t\t\t\tif (errno != EAGAIN)\n+\t\t\t\t\tdie_errno(\"read\");\n+\t}\n+}\n+\n+static void pp_output(struct parallel_processes *pp)\n+{\n+\tint i = pp->output_owner;\n+\tif (pp->children[i].in_use &&\n+\t pp->children[i].err.len) {\n+\t\tfputs(pp->children[i].err.buf, stderr);\n+\t\tstrbuf_reset(&pp->children[i].err);\n+\t}\n+}\n+\n+static int pp_collect_finished(struct parallel_processes *pp)\n+{\n+\tint i = 0;\n+\tpid_t pid;\n+\tint wait_status, code;\n+\tint n = pp->max_processes;\n+\tint result = 0;\n+\n+\twhile (pp->nr_processes > 0) {\n+\t\tpid = waitpid(-1, &wait_status, WNOHANG);\n+\t\tif (pid == 0)\n+\t\t\treturn 0;\n+\n+\t\tif (pid < 0)\n+\t\t\tdie_errno(\"wait\");\n+\n+\t\tfor (i = 0; i < pp->max_processes; i++)\n+\t\t\tif (pp->children[i].in_use &&\n+\t\t\t pid == pp->children[i].process.pid)\n+\t\t\t\tbreak;\n+\t\tif (i == pp->max_processes)\n+\t\t\tdie(\"BUG: found a child process we were not aware of\");\n+\n+\t\tif (strbuf_read(&pp->children[i].err,\n+\t\t\t\tpp->children[i].process.err, 0) < 0)\n+\t\t\tdie_errno(\"strbuf_read\");\n+\n+\t\tif (WIFSIGNALED(wait_status)) {\n+\t\t\tcode = WTERMSIG(wait_status);\n+\t\t\tif (!pp->shutdown &&\n+\t\t\t code != SIGINT && code != SIGQUIT)\n+\t\t\t\tstrbuf_addf(&pp->children[i].err,\n+\t\t\t\t\t \"%s died of signal %d\",\n+\t\t\t\t\t pp->children[i].process.argv[0],\n+\t\t\t\t\t code);\n+\t\t\t/*\n+\t\t\t * This return value is chosen so that code & 0xff\n+\t\t\t * mimics the exit code that a POSIX shell would report for\n+\t\t\t * a program that died from this signal.\n+\t\t\t */\n+\t\t\tcode += 128;\n+\t\t} else if (WIFEXITED(wait_status)) {\n+\t\t\tcode = WEXITSTATUS(wait_status);\n+\t\t\t/*\n+\t\t\t * Convert special exit code when execvp failed.\n+\t\t\t */\n+\t\t\tif (code == 127) {\n+\t\t\t\tcode = -1;\n+\t\t\t\terrno = ENOENT;\n+\t\t\t}\n+\t\t} else\n+\t\t\tstrbuf_addf(&pp->children[i].err,\n+\t\t\t\t \"waitpid is confused (%s)\",\n+\t\t\t\t pp->children[i].process.argv[0]);\n+\n+\n+\t\tif (pp->return_value(pp->data, &pp->children[i].process,\n+\t\t\t\t &pp->children[i].err, code))\n+\t\t\tresult = 1;\n+\n+\t\targv_array_clear(&pp->children[i].process.args);\n+\t\targv_array_clear(&pp->children[i].process.env_array);\n+\n+\t\tpp->nr_processes--;\n+\t\tpp->children[i].in_use = 0;\n+\t\tpp->pfd[i].fd = -1;\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\tfputs(pp->children[i].err.buf, 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_reset(&pp->buffered_output);\n+\n+\t\t\t/*\n+\t\t\t * Pick next process to output live.\n+\t\t\t * NEEDSWORK:\n+\t\t\t * For now we pick it randomly by doing a round\n+\t\t\t * robin. Later we may want to pick the one with\n+\t\t\t * the most output or the longest or shortest\n+\t\t\t * running process time.\n+\t\t\t */\n+\t\t\tfor (i = 0; i < n; i++)\n+\t\t\t\tif (pp->children[(pp->output_owner + i) % n].in_use)\n+\t\t\t\t\tbreak;\n+\t\t\tpp->output_owner = (pp->output_owner + i) % n;\n+\t\t}\n+\t}\n+\treturn result;\n+}\n+\n+int run_processes_parallel(int n, void *data,\n+\t\t\t get_next_task_fn get_next_task,\n+\t\t\t start_failure_fn start_failure,\n+\t\t\t return_value_fn return_value)\n+{\n+\tint no_more_task = 0;\n+\tstruct parallel_processes *pp;\n+\n+\tpp = pp_init(n, data, get_next_task, start_failure, return_value);\n+\twhile (1) {\n+\t\tint i;\n+\t\tint output_timeout = 100;\n+\t\tint spawn_cap = 4;\n+\n+\t\tif (!no_more_task) {\n+\t\t\tfor (i = 0; i < spawn_cap; i++) {\n+\t\t\t\tint code;\n+\t\t\t\tif (pp->nr_processes == pp->max_processes)\n+\t\t\t\t\tbreak;\n+\n+\t\t\t\tcode = pp_start_one(pp);\n+\t\t\t\tif (!code)\n+\t\t\t\t\tcontinue;\n+\t\t\t\tif (code < 0) {\n+\t\t\t\t\tpp->shutdown = 1;\n+\t\t\t\t\tkill_children(pp, SIGTERM);\n+\t\t\t\t}\n+\t\t\t\tno_more_task = 1;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t}\n+\t\tif (no_more_task && !pp->nr_processes)\n+\t\t\tbreak;\n+\t\tpp_buffer_stderr(pp, output_timeout);\n+\t\tpp_output(pp);\n+\t\tif (pp_collect_finished(pp)) {\n+\t\t\tkill_children(pp, SIGTERM);\n+\t\t\tpp->shutdown = 1;\n+\t\t\tno_more_task = 1;\n+\t\t}\n+\t}\n+\n+\tpp_cleanup(pp);\n+\treturn 0;\n+}\ndiff --git a/run-command.h b/run-command.h\nindex 5b4425a..1179cb0 100644\n--- a/run-command.h\n+++ b/run-command.h\n@@ -119,4 +119,67 @@ struct async {\n int start_async(struct async *async);\n int finish_async(struct async *async);\n \n+/**\n+ * This callback should initialize the child process and preload the\n+ * error channel. The preloading of is useful if you want to have a message\n+ * printed directly before the output of the child process.\n+ * You MUST set stdout_to_stderr.\n+ *\n+ * Return 1 if the next child is ready to run.\n+ * Return 0 if there are no more tasks to be processed.\n+ */\n+typedef int (*get_next_task_fn)(void *data,\n+\t\t\t\tstruct child_process *cp,\n+\t\t\t\tstruct strbuf *err);\n+\n+/**\n+ * This callback is called whenever there are problems starting\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+ * messing up the output of the other parallel processes.\n+ *\n+ * Return 0 to continue the parallel processing. To abort gracefully,\n+ * return non zero.\n+ */\n+typedef int (*start_failure_fn)(void *data,\n+\t\t\t\tstruct child_process *cp,\n+\t\t\t\tstruct strbuf *err);\n+\n+/**\n+ * This callback is called on every there are problems starting\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+ * messing up the output of the other parallel processes.\n+ *\n+ * Return 0 to continue the parallel processing. To abort gracefully,\n+ * return non zero.\n+ */\n+typedef int (*return_value_fn)(void *data,\n+\t\t\t struct child_process *cp,\n+\t\t\t struct strbuf *err,\n+\t\t\t int result);\n+\n+/**\n+ * Runs up to n processes at the same time. Whenever a process can be\n+ * started, the callback `get_next_task` is called to obtain the data\n+ * fed to the child process.\n+ *\n+ * The children started via this function run in parallel and their output\n+ * to stderr is buffered, while one of the children will directly output\n+ * to stderr.\n+ *\n+ * If start_failure_fn and return_value_fn are NULL, default handlers\n+ * will be used. The default handlers will print an error message on\n+ * error without issuing an emergency stop.\n+ */\n+\n+int run_processes_parallel(int n, void *data,\n+\t\t\t get_next_task_fn,\n+\t\t\t start_failure_fn,\n+\t\t\t return_value_fn);\n+\n #endif\ndiff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh\nindex 9acf628..49aa3db 100755\n--- a/t/t0061-run-command.sh\n+++ b/t/t0061-run-command.sh\n@@ -47,4 +47,24 @@ test_expect_success POSIXPERM,SANITY 'unreadable directory in PATH' '\n \ttest_cmp expect actual\n '\n \n+cat >expect <<-EOF\n+preloaded output of a child\n+Hello\n+World\n+preloaded output of a child\n+Hello\n+World\n+preloaded output of a child\n+Hello\n+World\n+preloaded output of a child\n+Hello\n+World\n+EOF\n+\n+test_expect_success 'run_command runs in parallel' '\n+\ttest-run-command run-command-parallel-4 sh -c \"printf \\\"%s\\n%s\\n\\\" Hello World\" 2>actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\ndiff --git a/test-run-command.c b/test-run-command.c\nindex 89c7de2..2555791 100644\n--- a/test-run-command.c\n+++ b/test-run-command.c\n@@ -10,9 +10,29 @@\n \n #include \"git-compat-util.h\"\n #include \"run-command.h\"\n+#include \"argv-array.h\"\n+#include \"strbuf.h\"\n #include \n #include \n \n+static int number_callbacks;\n+static int parallel_next(void *data,\n+\t\t\t struct child_process *cp,\n+\t\t\t struct strbuf *err)\n+{\n+\tstruct child_process *d = data;\n+\tif (number_callbacks >= 4)\n+\t\treturn 0;\n+\n+\targv_array_pushv(&cp->args, d->argv);\n+\tcp->stdout_to_stderr = 1;\n+\tcp->no_stdin = 1;\n+\tcp->err = -1;\n+\tstrbuf_addf(err, \"preloaded output of a child\\n\");\n+\tnumber_callbacks++;\n+\treturn 1;\n+}\n+\n int main(int argc, char **argv)\n {\n \tstruct child_process proc = CHILD_PROCESS_INIT;\n@@ -30,6 +50,10 @@ int main(int argc, char **argv)\n \tif (!strcmp(argv[1], \"run-command\"))\n \t\texit(run_command(&proc));\n \n+\tif (!strcmp(argv[1], \"run-command-parallel-4\"))\n+\t\texit(run_processes_parallel(4, &proc, parallel_next,\n+\t\t\t\t\t NULL, NULL));\n+\n \tfprintf(stderr, \"check usage\\n\");\n \treturn 1;\n }\n-- \n2.5.0.273.g6fa2560.dirty\n"},{"id":"270896","messageId":"1443482046-25569-8-git-send-email-sbeller@google.com","threadId":"40446","inReplyTo":"1443482046-25569-1-git-send-email-sbeller@google.com","subject":"[PATCH 7/8] fetch_populated_submodules: use new parallel job processing","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-09-28T23:14:05Z","receivedAt":"2015-09-28T23:14:05Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"In a later patch we enable parallel processing of submodules, this\nonly adds the possibility for it. So this change should not change\nany user facing behavior.\n\nSigned-off-by: Stefan Beller \nSigned-off-by: Junio C Hamano \n---\n submodule.c | 126 ++++++++++++++++++++++++++++++++++++++++++++----------------\n 1 file changed, 92 insertions(+), 34 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex 1d64e57..8134fe7 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -12,6 +12,7 @@\n #include \"sha1-array.h\"\n #include \"argv-array.h\"\n #include \"blob.h\"\n+#include \"thread-utils.h\"\n \n static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND;\n static struct string_list changed_submodule_paths;\n@@ -615,37 +616,89 @@ static void calculate_changed_submodule_paths(void)\n \tinitialized_fetch_ref_tips = 0;\n }\n \n+struct submodule_parallel_fetch {\n+\tint count;\n+\tstruct argv_array args;\n+\tconst char *work_tree;\n+\tconst char *prefix;\n+\tint command_line_option;\n+\tint quiet;\n+\tint result;\n+};\n+#define SPF_INIT {0, ARGV_ARRAY_INIT, NULL, NULL, 0, 0, 0}\n+\n+int get_next_submodule(void *data, struct child_process *cp,\n+\t\t struct strbuf *err);\n+\n+static int fetch_start_failure(void *data, struct child_process *cp,\n+\t\t\t struct strbuf *err)\n+{\n+\tstruct submodule_parallel_fetch *spf = data;\n+\n+\tspf->result = 1;\n+\n+\treturn 0;\n+}\n+\n+static int fetch_finish(void *data, struct child_process *cp,\n+\t\t\tstruct strbuf *err, int retvalue)\n+{\n+\tstruct submodule_parallel_fetch *spf = data;\n+\n+\tif (retvalue)\n+\t\tspf->result = 1;\n+\n+\treturn 0;\n+}\n+\n int fetch_populated_submodules(const struct argv_array *options,\n \t\t\t const char *prefix, int command_line_option,\n \t\t\t int quiet)\n {\n-\tint i, result = 0;\n-\tstruct child_process cp = CHILD_PROCESS_INIT;\n-\tstruct argv_array argv = ARGV_ARRAY_INIT;\n-\tconst char *work_tree = get_git_work_tree();\n-\tif (!work_tree)\n+\tint i;\n+\tint max_parallel_jobs = 1;\n+\tstruct submodule_parallel_fetch spf = SPF_INIT;\n+\n+\tspf.work_tree = get_git_work_tree();\n+\tspf.command_line_option = command_line_option;\n+\tspf.quiet = quiet;\n+\tspf.prefix = prefix;\n+\n+\tif (!spf.work_tree)\n \t\tgoto out;\n \n \tif (read_cache() < 0)\n \t\tdie(\"index file corrupt\");\n \n-\targv_array_push(&argv, \"fetch\");\n+\targv_array_push(&spf.args, \"fetch\");\n \tfor (i = 0; i < options->argc; i++)\n-\t\targv_array_push(&argv, options->argv[i]);\n-\targv_array_push(&argv, \"--recurse-submodules-default\");\n+\t\targv_array_push(&spf.args, options->argv[i]);\n+\targv_array_push(&spf.args, \"--recurse-submodules-default\");\n \t/* default value, \"--submodule-prefix\" and its value are added later */\n \n-\tcp.env = local_repo_env;\n-\tcp.git_cmd = 1;\n-\tcp.no_stdin = 1;\n-\n \tcalculate_changed_submodule_paths();\n+\trun_processes_parallel(max_parallel_jobs, &spf,\n+\t\t\t get_next_submodule,\n+\t\t\t fetch_start_failure,\n+\t\t\t fetch_finish);\n+\n+\targv_array_clear(&spf.args);\n+out:\n+\tstring_list_clear(&changed_submodule_paths, 1);\n+\treturn spf.result;\n+}\n \n-\tfor (i = 0; i < active_nr; i++) {\n+int get_next_submodule(void *data, struct child_process *cp,\n+\t\t struct strbuf *err)\n+{\n+\tint ret = 0;\n+\tstruct submodule_parallel_fetch *spf = data;\n+\n+\tfor ( ; spf->count < active_nr; spf->count++) {\n \t\tstruct strbuf submodule_path = STRBUF_INIT;\n \t\tstruct strbuf submodule_git_dir = STRBUF_INIT;\n \t\tstruct strbuf submodule_prefix = STRBUF_INIT;\n-\t\tconst struct cache_entry *ce = active_cache[i];\n+\t\tconst struct cache_entry *ce = active_cache[spf->count];\n \t\tconst char *git_dir, *default_argv;\n \t\tconst struct submodule *submodule;\n \n@@ -657,7 +710,7 @@ int fetch_populated_submodules(const struct argv_array *options,\n \t\t\tsubmodule = submodule_from_name(null_sha1, ce->name);\n \n \t\tdefault_argv = \"yes\";\n-\t\tif (command_line_option == RECURSE_SUBMODULES_DEFAULT) {\n+\t\tif (spf->command_line_option == RECURSE_SUBMODULES_DEFAULT) {\n \t\t\tif (submodule &&\n \t\t\t submodule->fetch_recurse !=\n \t\t\t\t\t\tRECURSE_SUBMODULES_NONE) {\n@@ -680,40 +733,45 @@ int fetch_populated_submodules(const struct argv_array *options,\n \t\t\t\t\tdefault_argv = \"on-demand\";\n \t\t\t\t}\n \t\t\t}\n-\t\t} else if (command_line_option == RECURSE_SUBMODULES_ON_DEMAND) {\n+\t\t} else if (spf->command_line_option == RECURSE_SUBMODULES_ON_DEMAND) {\n \t\t\tif (!unsorted_string_list_lookup(&changed_submodule_paths, ce->name))\n \t\t\t\tcontinue;\n \t\t\tdefault_argv = \"on-demand\";\n \t\t}\n \n-\t\tstrbuf_addf(&submodule_path, \"%s/%s\", work_tree, ce->name);\n+\t\tstrbuf_addf(&submodule_path, \"%s/%s\", spf->work_tree, ce->name);\n \t\tstrbuf_addf(&submodule_git_dir, \"%s/.git\", submodule_path.buf);\n-\t\tstrbuf_addf(&submodule_prefix, \"%s%s/\", prefix, ce->name);\n+\t\tstrbuf_addf(&submodule_prefix, \"%s%s/\", spf->prefix, ce->name);\n \t\tgit_dir = read_gitfile(submodule_git_dir.buf);\n \t\tif (!git_dir)\n \t\t\tgit_dir = submodule_git_dir.buf;\n \t\tif (is_directory(git_dir)) {\n-\t\t\tif (!quiet)\n-\t\t\t\tfprintf(stderr, \"Fetching submodule %s%s\\n\", prefix, ce->name);\n-\t\t\tcp.dir = submodule_path.buf;\n-\t\t\targv_array_push(&argv, default_argv);\n-\t\t\targv_array_push(&argv, \"--submodule-prefix\");\n-\t\t\targv_array_push(&argv, submodule_prefix.buf);\n-\t\t\tcp.argv = argv.argv;\n-\t\t\tif (run_command(&cp))\n-\t\t\t\tresult = 1;\n-\t\t\targv_array_pop(&argv);\n-\t\t\targv_array_pop(&argv);\n-\t\t\targv_array_pop(&argv);\n+\t\t\tchild_process_init(cp);\n+\t\t\tcp->dir = strbuf_detach(&submodule_path, NULL);\n+\t\t\tcp->env = local_repo_env;\n+\t\t\tcp->git_cmd = 1;\n+\t\t\tcp->no_stdin = 1;\n+\t\t\tcp->stdout_to_stderr = 1;\n+\t\t\tcp->err = -1;\n+\t\t\tif (!spf->quiet)\n+\t\t\t\tstrbuf_addf(err, \"Fetching submodule %s%s\\n\",\n+\t\t\t\t\t spf->prefix, ce->name);\n+\t\t\targv_array_init(&cp->args);\n+\t\t\targv_array_pushv(&cp->args, spf->args.argv);\n+\t\t\targv_array_push(&cp->args, default_argv);\n+\t\t\targv_array_push(&cp->args, \"--submodule-prefix\");\n+\t\t\targv_array_push(&cp->args, submodule_prefix.buf);\n+\t\t\tret = 1;\n \t\t}\n \t\tstrbuf_release(&submodule_path);\n \t\tstrbuf_release(&submodule_git_dir);\n \t\tstrbuf_release(&submodule_prefix);\n+\t\tif (ret) {\n+\t\t\tspf->count++;\n+\t\t\treturn 1;\n+\t\t}\n \t}\n-\targv_array_clear(&argv);\n-out:\n-\tstring_list_clear(&changed_submodule_paths, 1);\n-\treturn result;\n+\treturn 0;\n }\n \n unsigned is_submodule_modified(const char *path, int ignore_untracked)\n-- \n2.5.0.273.g6fa2560.dirty\n"},{"id":"270893","messageId":"1443482046-25569-9-git-send-email-sbeller@google.com","threadId":"40446","inReplyTo":"1443482046-25569-1-git-send-email-sbeller@google.com","subject":"[PATCH 8/8] submodules: allow parallel fetching, add tests and documentation","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-09-28T23:14:06Z","receivedAt":"2015-09-28T23:14:06Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This enables the work of the previous patches.\n\nSigned-off-by: Stefan Beller \nSigned-off-by: Junio C Hamano \n---\n Documentation/fetch-options.txt | 7 +++++++\n builtin/fetch.c | 6 +++++-\n builtin/pull.c | 6 ++++++\n submodule.c | 3 +--\n submodule.h | 2 +-\n t/t5526-fetch-submodules.sh | 19 +++++++++++++++++++\n 6 files changed, 39 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt\nindex 45583d8..6b109f6 100644\n--- a/Documentation/fetch-options.txt\n+++ b/Documentation/fetch-options.txt\n@@ -100,6 +100,13 @@ ifndef::git-pull[]\n \treference to a commit that isn't already in the local submodule\n \tclone.\n \n+-j::\n+--jobs=::\n+\tNumber of parallel children to be used for fetching submodules.\n+\tEach will fetch from different submodules, such that fetching many\n+\tsubmodules will be faster. By default submodules will be fetched\n+\tone at a time.\n+\n --no-recurse-submodules::\n \tDisable recursive fetching of submodules (this has the same effect as\n \tusing the '--recurse-submodules=no' option).\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex ee1f1a9..f28eac6 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -37,6 +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 const char *depth;\n static const char *upload_pack;\n static struct strbuf default_rla = STRBUF_INIT;\n@@ -99,6 +100,8 @@ static struct option builtin_fetch_options[] = {\n \t\t N_(\"fetch all tags and associated objects\"), TAGS_SET),\n \tOPT_SET_INT('n', NULL, &tags,\n \t\t N_(\"do not fetch all tags (--no-tags)\"), TAGS_UNSET),\n+\tOPT_INTEGER('j', \"jobs\", &max_children,\n+\t\t N_(\"number of submodules fetched in parallel\")),\n \tOPT_BOOL('p', \"prune\", &prune,\n \t\t N_(\"prune remote-tracking branches no longer on remote\")),\n \t{ OPTION_CALLBACK, 0, \"recurse-submodules\", NULL, N_(\"on-demand\"),\n@@ -1217,7 +1220,8 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)\n \t\tresult = fetch_populated_submodules(&options,\n \t\t\t\t\t\t submodule_prefix,\n \t\t\t\t\t\t recurse_submodules,\n-\t\t\t\t\t\t verbosity < 0);\n+\t\t\t\t\t\t verbosity < 0,\n+\t\t\t\t\t\t max_children);\n \t\targv_array_clear(&options);\n \t}\n \ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex 722a83c..f0af196 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -94,6 +94,7 @@ static int opt_force;\n static char *opt_tags;\n static char *opt_prune;\n static char *opt_recurse_submodules;\n+static char *max_children;\n static int opt_dry_run;\n static char *opt_keep;\n static char *opt_depth;\n@@ -177,6 +178,9 @@ static struct option pull_options[] = {\n \t\tN_(\"on-demand\"),\n \t\tN_(\"control recursive fetching of submodules\"),\n \t\tPARSE_OPT_OPTARG),\n+\tOPT_PASSTHRU('j', \"jobs\", &max_children, N_(\"n\"),\n+\t\tN_(\"number of submodules pulled in parallel\"),\n+\t\tPARSE_OPT_OPTARG),\n \tOPT_BOOL(0, \"dry-run\", &opt_dry_run,\n \t\tN_(\"dry run\")),\n \tOPT_PASSTHRU('k', \"keep\", &opt_keep, NULL,\n@@ -524,6 +528,8 @@ static int run_fetch(const char *repo, const char **refspecs)\n \t\targv_array_push(&args, opt_prune);\n \tif (opt_recurse_submodules)\n \t\targv_array_push(&args, opt_recurse_submodules);\n+\tif (max_children)\n+\t\targv_array_push(&args, max_children);\n \tif (opt_dry_run)\n \t\targv_array_push(&args, \"--dry-run\");\n \tif (opt_keep)\ndiff --git a/submodule.c b/submodule.c\nindex 8134fe7..7ab89f4 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -653,10 +653,9 @@ static int fetch_finish(void *data, struct child_process *cp,\n \n int fetch_populated_submodules(const struct argv_array *options,\n \t\t\t const char *prefix, int command_line_option,\n-\t\t\t int quiet)\n+\t\t\t int quiet, int max_parallel_jobs)\n {\n \tint i;\n-\tint max_parallel_jobs = 1;\n \tstruct submodule_parallel_fetch spf = SPF_INIT;\n \n \tspf.work_tree = get_git_work_tree();\ndiff --git a/submodule.h b/submodule.h\nindex 5507c3d..cbc0003 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -31,7 +31,7 @@ void set_config_fetch_recurse_submodules(int value);\n void check_for_new_submodule_commits(unsigned char new_sha1[20]);\n int fetch_populated_submodules(const struct argv_array *options,\n \t\t\t const char *prefix, int command_line_option,\n-\t\t\t int quiet);\n+\t\t\t int quiet, int max_parallel_jobs);\n unsigned is_submodule_modified(const char *path, int ignore_untracked);\n int submodule_uses_gitfile(const char *path);\n int ok_to_remove_submodule(const char *path);\ndiff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh\nindex 17759b1..1b4ce69 100755\n--- a/t/t5526-fetch-submodules.sh\n+++ b/t/t5526-fetch-submodules.sh\n@@ -71,6 +71,16 @@ test_expect_success \"fetch --recurse-submodules recurses into submodules\" '\n \ttest_i18ncmp expect.err actual.err\n '\n \n+test_expect_success \"fetch --recurse-submodules -j2 has the same output behaviour\" '\n+\tadd_upstream_commit &&\n+\t(\n+\t\tcd downstream &&\n+\t\tgit fetch --recurse-submodules -j2 2>../actual.err\n+\t) &&\n+\ttest_must_be_empty actual.out &&\n+\ttest_i18ncmp expect.err actual.err\n+'\n+\n test_expect_success \"fetch alone only fetches superproject\" '\n \tadd_upstream_commit &&\n \t(\n@@ -140,6 +150,15 @@ test_expect_success \"--quiet propagates to submodules\" '\n \t! test -s actual.err\n '\n \n+test_expect_success \"--quiet propagates to parallel submodules\" '\n+\t(\n+\t\tcd downstream &&\n+\t\tgit fetch --recurse-submodules -j 2 --quiet >../actual.out 2>../actual.err\n+\t) &&\n+\t! test -s actual.out &&\n+\t! test -s actual.err\n+'\n+\n test_expect_success \"--dry-run propagates to submodules\" '\n \tadd_upstream_commit &&\n \t(\n-- \n2.5.0.273.g6fa2560.dirty\n"},{"id":"270925","messageId":"xmqqa8s4a9cw.fsf@gitster.mtv.corp.google.com","threadId":"40446","inReplyTo":"1443482046-25569-7-git-send-email-sbeller@google.com","subject":"Re: [PATCH 6/8] run-command: add an asynchronous parallel child processor","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-09-30T03:12:31Z","receivedAt":"2015-09-30T03:12:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> +\twhile (1) {\n> +\t\tint i;\n> +\t\tint output_timeout = 100;\n> +\t\tint spawn_cap = 4;\n> +\n> +\t\tif (!no_more_task) {\n> +\t\t\tfor (i = 0; i < spawn_cap; i++) {\n> +\t\t\t\tint code;\n> +\t\t\t\tif (pp->nr_processes == pp->max_processes)\n> +\t\t\t\t\tbreak;\n> +\n> +\t\t\t\tcode = pp_start_one(pp);\n> +\t\t\t\tif (!code)\n> +\t\t\t\t\tcontinue;\n> +\t\t\t\tif (code < 0) {\n> +\t\t\t\t\tpp->shutdown = 1;\n> +\t\t\t\t\tkill_children(pp, SIGTERM);\n> +\t\t\t\t}\n> +\t\t\t\tno_more_task = 1;\n> +\t\t\t\tbreak;\n> +\t\t\t}\n> +\t\t}\n> +\t\tif (no_more_task && !pp->nr_processes)\n> +\t\t\tbreak;\n\nI may have comments on other parts of this patch, but I noticed this\na bit hard to read while reading the end result.\n\nLosing the outer \"if (!no_more_task)\" and replacing the above with\n\n\tfor (no_more_task = 0, i = 0;\n !no_more_task && i < spawn_cap;\n i++) {\n \t... do things that may or may not set\n ... no_more_task\n\t}\n\tif (no_more_task && ...)\n \tbreak;\n\nwould make it clear that regardless of spawn_cap, no_more_task is\nhonored.\n\nAlso I think that having the outer \"if (!no_more_task)\" and not\nhaving \"no_more_task = 0\" after each iteration is buggy. Even when\nnext_task() told start_one() that it does not have more tasks for\nnow, as long as there are running processes, it is entirely plausible\nthat next call to next_task() can return \"now we have some more task\nto do\".\n\nAlthough I think it would make it unsightly, if you want to have the\nlarge indentation that protects the spawn_cap loop from getting\nentered, the condition would be \n\n\tif (!pp->shutdown) {\n\t\tfor (... spawn_cap loop ...) {\n \t...\n\t\t}\n\t}\n\nThat structure could make sense. But even then I would probably\nwrite it more like\n\n\t...\n\tint spawn_cap = 4;\n\n\tpp = pp_init(...);\n while (1) {\n \tint no_more_task = 0;\n\n for (i = 0;\n !no_more_task && !pp->shutdown && i < spawn_cap;\n i++) {\n\t\t\t...\n code = start_one();\n ... set no_more_task to 1 as needed\n ... set pp->shutdown to 1 as needed\n\t\t}\n if (no_more_task && !pp->nr_processes)\n\t\t\tbreak;\n\t\tbuffer_stderr(...);\n output(...);\n collect(...);\n\t}\n\nThat is, you need to have two independent conditions that tell you\nnot to spawn any new task:\n\n (1) You called start_one() repeatedly and next_task() said \"nothing\n more for now\", so you know calling start_one() one more time\n without changing other conditions (like draining output from\n running processes and culling finished ones) will not help.\n\n Letting other parts of the application that uses this scheduler\n loop (i.e. drain output, cull finished process, etc.) may\n change the situation and you _do_ need to call start_one() when\n the next_task() merely said \"nothing more for now\".\n\n That is what no_more_task controls.\n\n (2) The application said \"I want the system to be gracefully shut\n down\". next_task() may also have said \"nothing more for now\"\n and you may have set no_more_task in response to it, but unlike\n (1) above, draining and culling must be done only to shut the\n system down, the application does not want new processes to be\n added. You do not want to enter the spawn_cap loop when it\n happens.\n\n That is what pp->shutdown controls.\n"},{"id":"270926","messageId":"xmqq37xwa3au.fsf@gitster.mtv.corp.google.com","threadId":"40446","inReplyTo":"1443482046-25569-6-git-send-email-sbeller@google.com","subject":"Re: [PATCH 5/8] sigchain: add command to pop all common signals","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-09-30T05:23:21Z","receivedAt":"2015-09-30T05:23:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> The new method removes all common signal handlers that were installed\n> by sigchain_push.\n>\n> CC: Jeff King \n> Signed-off-by: Stefan Beller \n> ---\n> sigchain.c | 9 +++++++++\n> sigchain.h | 1 +\n> 2 files changed, 10 insertions(+)\n\nSounds like a good idea, as you need to clean them all up if you did\npush_common() and ended up not receiving any signal.\n\nThis is merely pure aesthetics, but I somehow thought that ordering\nthem in the reverse as listed in push_common() might make more\nsense, though.\n\nThanks.\n\n>\n> diff --git a/sigchain.c b/sigchain.c\n> index faa375d..9262307 100644\n> --- a/sigchain.c\n> +++ b/sigchain.c\n> @@ -50,3 +50,12 @@ void sigchain_push_common(sigchain_fun f)\n> \tsigchain_push(SIGQUIT, f);\n> \tsigchain_push(SIGPIPE, f);\n> }\n> +\n> +void sigchain_pop_common(void)\n> +{\n> +\tsigchain_pop(SIGINT);\n> +\tsigchain_pop(SIGHUP);\n> +\tsigchain_pop(SIGTERM);\n> +\tsigchain_pop(SIGQUIT);\n> +\tsigchain_pop(SIGPIPE);\n> +}\n> diff --git a/sigchain.h b/sigchain.h\n> index 618083b..138b20f 100644\n> --- a/sigchain.h\n> +++ b/sigchain.h\n> @@ -7,5 +7,6 @@ int sigchain_push(int sig, sigchain_fun f);\n> int sigchain_pop(int sig);\n> \n> void sigchain_push_common(sigchain_fun f);\n> +void sigchain_pop_common(void);\n> \n> #endif /* SIGCHAIN_H */\n"},{"id":"270938","messageId":"CAGZ79kbp5v-bMXGJh1KcXTb=Umb_2G_2taPRXhoPs0y_0cdUzw@mail.gmail.com","threadId":"40446","inReplyTo":"xmqqa8s4a9cw.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 6/8] run-command: add an asynchronous parallel child processor","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-09-30T18:28:47Z","receivedAt":"2015-09-30T18:28:47Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Sep 29, 2015 at 8:12 PM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>\n>> + while (1) {\n>> + int i;\n>> + int output_timeout = 100;\n>> + int spawn_cap = 4;\n>> +\n>> + if (!no_more_task) {\n>> + for (i = 0; i < spawn_cap; i++) {\n>> + int code;\n>> + if (pp->nr_processes == pp->max_processes)\n>> + break;\n>> +\n>> + code = pp_start_one(pp);\n>> + if (!code)\n>> + continue;\n>> + if (code < 0) {\n>> + pp->shutdown = 1;\n>> + kill_children(pp, SIGTERM);\n>> + }\n>> + no_more_task = 1;\n>> + break;\n>> + }\n>> + }\n>> + if (no_more_task && !pp->nr_processes)\n>> + break;\n>\n> I may have comments on other parts of this patch, but I noticed this\n> a bit hard to read while reading the end result.\n>\n> Losing the outer \"if (!no_more_task)\" and replacing the above with\n>\n> for (no_more_task = 0, i = 0;\n> !no_more_task && i < spawn_cap;\n> i++) {\n> ... do things that may or may not set\n> ... no_more_task\n> }\n> if (no_more_task && ...)\n> break;\n>\n> would make it clear that regardless of spawn_cap, no_more_task is\n> honored.\n>\n> Also I think that having the outer \"if (!no_more_task)\" and not\n> having \"no_more_task = 0\" after each iteration is buggy. Even when\n> next_task() told start_one() that it does not have more tasks for\n> now, as long as there are running processes, it is entirely plausible\n> that next call to next_task() can return \"now we have some more task\n> to do\".\n>\n> Although I think it would make it unsightly, if you want to have the\n> large indentation that protects the spawn_cap loop from getting\n> entered, the condition would be\n>\n> if (!pp->shutdown) {\n> for (... spawn_cap loop ...) {\n> ...\n> }\n> }\n>\n> That structure could make sense. But even then I would probably\n> write it more like\n>\n> ...\n> int spawn_cap = 4;\n>\n> pp = pp_init(...);\n> while (1) {\n> int no_more_task = 0;\n>\n> for (i = 0;\n> !no_more_task && !pp->shutdown && i < spawn_cap;\n> i++) {\n> ...\n> code = start_one();\n> ... set no_more_task to 1 as needed\n> ... set pp->shutdown to 1 as needed\n> }\n> if (no_more_task && !pp->nr_processes)\n> break;\n> buffer_stderr(...);\n> output(...);\n> collect(...);\n> }\n\nThat is indeed better to read.\nThough if we reset `no_more_task` each iteration of the loop by\nhaving its declaration inside the loop, the condition for exiting the\nloop needs to be:\n\n if ((no_more_task || pp->shutdown) && !pp->nr_processes)\n break;\n\nfor correctness.\n\nWhen looking at that condition, I realized that I implicitly assumed\nthe workflow, where get_next_task returns 0 intermittently, to be a\nsecond class citizen. I reworded the documentation as well there.\n\n>\n> That is, you need to have two independent conditions that tell you\n> not to spawn any new task:\n>\n> (1) You called start_one() repeatedly and next_task() said \"nothing\n> more for now\", so you know calling start_one() one more time\n> without changing other conditions (like draining output from\n> running processes and culling finished ones) will not help.\n>\n> Letting other parts of the application that uses this scheduler\n> loop (i.e. drain output, cull finished process, etc.) may\n> change the situation and you _do_ need to call start_one() when\n> the next_task() merely said \"nothing more for now\".\n>\n> That is what no_more_task controls.\n>\n> (2) The application said \"I want the system to be gracefully shut\n> down\". next_task() may also have said \"nothing more for now\"\n> and you may have set no_more_task in response to it, but unlike\n> (1) above, draining and culling must be done only to shut the\n> system down, the application does not want new processes to be\n> added. You do not want to enter the spawn_cap loop when it\n> happens.\n>\n> That is what pp->shutdown controls.\n>\n"},{"id":"270939","messageId":"xmqq4mib920v.fsf@gitster.mtv.corp.google.com","threadId":"40446","inReplyTo":"xmqqa8s4a9cw.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 6/8] run-command: add an asynchronous parallel child processor","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-09-30T18:48:32Z","receivedAt":"2015-09-30T18:48:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> I may have comments on other parts of this patch, but I noticed this\n> a bit hard to read while reading the end result.\n> ...\n\nI finished reading the remainder. Other than the above all look\nsensible.\n\nWill replace what had been queued.\n\nThanks.\n"}]}