From: Junio C Hamano Date: Fri, 4 Mar 2016 21:46:30 +0000 (-0800) Subject: Merge branch 'sb/submodule-parallel-fetch' X-Git-Tag: v2.8.0-rc1~3 X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/bbe90e7950456bf1bb1ab17a9ee626f6fad7a7c6?ds=inline;hp=-c Merge branch 'sb/submodule-parallel-fetch' Simplify the two callback functions that are triggered when the child process terminates to avoid misuse of the child-process structure that has already been cleaned up. * sb/submodule-parallel-fetch: run-command: do not pass child process data into callbacks --- bbe90e7950456bf1bb1ab17a9ee626f6fad7a7c6 diff --combined run-command.c index 863dad52f1,8e3ad07ffe..c72601056c --- a/run-command.c +++ b/run-command.c @@@ -160,41 -160,50 +160,41 @@@ int sane_execvp(const char *file, char return -1; } -static const char **prepare_shell_cmd(const char **argv) +static const char **prepare_shell_cmd(struct argv_array *out, const char **argv) { - int argc, nargc = 0; - const char **nargv; - - for (argc = 0; argv[argc]; argc++) - ; /* just counting */ - /* +1 for NULL, +3 for "sh -c" plus extra $0 */ - nargv = xmalloc(sizeof(*nargv) * (argc + 1 + 3)); - - if (argc < 1) + if (!argv[0]) die("BUG: shell command is empty"); if (strcspn(argv[0], "|&;<>()$`\\\"' \t\n*?[#~=%") != strlen(argv[0])) { #ifndef GIT_WINDOWS_NATIVE - nargv[nargc++] = SHELL_PATH; + argv_array_push(out, SHELL_PATH); #else - nargv[nargc++] = "sh"; + argv_array_push(out, "sh"); #endif - nargv[nargc++] = "-c"; - - if (argc < 2) - nargv[nargc++] = argv[0]; - else { - struct strbuf arg0 = STRBUF_INIT; - strbuf_addf(&arg0, "%s \"$@\"", argv[0]); - nargv[nargc++] = strbuf_detach(&arg0, NULL); - } - } + argv_array_push(out, "-c"); - for (argc = 0; argv[argc]; argc++) - nargv[nargc++] = argv[argc]; - nargv[nargc] = NULL; + /* + * If we have no extra arguments, we do not even need to + * bother with the "$@" magic. + */ + if (!argv[1]) + argv_array_push(out, argv[0]); + else + argv_array_pushf(out, "%s \"$@\"", argv[0]); + } - return nargv; + argv_array_pushv(out, argv); + return out->argv; } #ifndef GIT_WINDOWS_NATIVE static int execv_shell_cmd(const char **argv) { - const char **nargv = prepare_shell_cmd(argv); - trace_argv_printf(nargv, "trace: exec:"); - sane_execvp(nargv[0], (char **)nargv); - free(nargv); + struct argv_array nargv = ARGV_ARRAY_INIT; + prepare_shell_cmd(&nargv, argv); + trace_argv_printf(nargv.argv, "trace: exec:"); + sane_execvp(nargv.argv[0], (char **)nargv.argv); + argv_array_clear(&nargv); return -1; } #endif @@@ -238,7 -247,7 +238,7 @@@ static int wait_or_whine(pid_t pid, con error("waitpid is confused (%s)", argv0); } else if (WIFSIGNALED(status)) { code = WTERMSIG(status); - if (code != SIGINT && code != SIGQUIT) + if (code != SIGINT && code != SIGQUIT && code != SIGPIPE) error("%s died of signal %d", argv0, code); /* * This return value is chosen so that code & 0xff @@@ -448,7 -457,6 +448,7 @@@ fail_pipe { int fhin = 0, fhout = 1, fherr = 2; const char **sargv = cmd->argv; + struct argv_array nargv = ARGV_ARRAY_INIT; if (cmd->no_stdin) fhin = open("/dev/null", O_RDWR); @@@ -474,9 -482,9 +474,9 @@@ fhout = dup(cmd->out); if (cmd->git_cmd) - cmd->argv = prepare_git_cmd(cmd->argv); + cmd->argv = prepare_git_cmd(&nargv, cmd->argv); else if (cmd->use_shell) - cmd->argv = prepare_shell_cmd(cmd->argv); + cmd->argv = prepare_shell_cmd(&nargv, cmd->argv); cmd->pid = mingw_spawnvpe(cmd->argv[0], cmd->argv, (char**) cmd->env, cmd->dir, fhin, fhout, fherr); @@@ -486,7 -494,9 +486,7 @@@ if (cmd->clean_on_exit && cmd->pid >= 0) mark_child_for_cleanup(cmd->pid); - if (cmd->git_cmd) - free(cmd->argv); - + argv_array_clear(&nargv); cmd->argv = sargv; if (fhin != 0) close(fhin); @@@ -625,11 -635,6 +625,11 @@@ int in_async(void return !pthread_equal(main_thread, pthread_self()); } +void NORETURN async_exit(int code) +{ + pthread_exit((void *)(intptr_t)code); +} + #else static struct { @@@ -675,11 -680,6 +675,11 @@@ int in_async(void return process_is_async; } +void NORETURN async_exit(int code) +{ + exit(code); +} + #endif int start_async(struct async *async) @@@ -902,35 -902,18 +902,18 @@@ struct parallel_processes struct strbuf buffered_output; /* of finished children */ }; - static int default_start_failure(struct child_process *cp, - struct strbuf *err, + static int default_start_failure(struct strbuf *err, void *pp_cb, void *pp_task_cb) { - int i; - - strbuf_addstr(err, "Starting a child failed:"); - for (i = 0; cp->argv[i]; i++) - strbuf_addf(err, " %s", cp->argv[i]); - return 0; } static int default_task_finished(int result, - struct child_process *cp, struct strbuf *err, void *pp_cb, void *pp_task_cb) { - int i; - - if (!result) - return 0; - - strbuf_addf(err, "A child failed with return code %d:", result); - for (i = 0; cp->argv[i]; i++) - strbuf_addf(err, " %s", cp->argv[i]); - return 0; } @@@ -1048,8 -1031,7 +1031,7 @@@ static int pp_start_one(struct parallel pp->children[i].process.no_stdin = 1; if (start_command(&pp->children[i].process)) { - code = pp->start_failure(&pp->children[i].process, - &pp->children[i].err, + code = pp->start_failure(&pp->children[i].err, pp->data, &pp->children[i].data); strbuf_addbuf(&pp->buffered_output, &pp->children[i].err); @@@ -1117,7 -1099,7 +1099,7 @@@ static int pp_collect_finished(struct p code = finish_command(&pp->children[i].process); - code = pp->task_finished(code, &pp->children[i].process, + code = pp->task_finished(code, &pp->children[i].err, pp->data, &pp->children[i].data); diff --combined run-command.h index 42917e8618,6e17894f01..3d1e59e26e --- a/run-command.h +++ b/run-command.h @@@ -121,7 -121,6 +121,7 @@@ struct async int start_async(struct async *async); int finish_async(struct async *async); int in_async(void); +void NORETURN async_exit(int code); /** * This callback should initialize the child process and preload the @@@ -159,8 -158,7 +159,7 @@@ typedef int (*get_next_task_fn)(struct * To send a signal to other child processes for abortion, return * the negative signal number. */ - typedef int (*start_failure_fn)(struct child_process *cp, - struct strbuf *err, + typedef int (*start_failure_fn)(struct strbuf *err, void *pp_cb, void *pp_task_cb); @@@ -179,7 -177,6 +178,6 @@@ * the negative signal number. */ typedef int (*task_finished_fn)(int result, - struct child_process *cp, struct strbuf *err, void *pp_cb, void *pp_task_cb); @@@ -193,9 -190,8 +191,8 @@@ * (both stdout and stderr) is routed to stderr in a manner that output * from different tasks does not interleave. * - * If start_failure_fn or task_finished_fn are NULL, default handlers - * will be used. The default handlers will print an error message on - * error without issuing an emergency stop. + * start_failure_fn and task_finished_fn can be NULL to omit any + * special handling. */ int run_processes_parallel(int n, get_next_task_fn, diff --combined submodule.c index 24fb81ac62,916fc84aaa..62c4356c50 --- a/submodule.c +++ b/submodule.c @@@ -69,7 -69,7 +69,7 @@@ int update_path_in_gitmodules(const cha strbuf_addstr(&entry, "submodule."); strbuf_addstr(&entry, submodule->name); strbuf_addstr(&entry, ".path"); - if (git_config_set_in_file(".gitmodules", entry.buf, newpath) < 0) { + if (git_config_set_in_file_gently(".gitmodules", entry.buf, newpath) < 0) { /* Maybe the user already did that, don't error out here */ warning(_("Could not update .gitmodules entry %s"), entry.buf); strbuf_release(&entry); @@@ -123,7 -123,7 +123,7 @@@ static int add_submodule_odb(const cha struct strbuf objects_directory = STRBUF_INIT; struct alternate_object_database *alt_odb; int ret = 0; - int alloc; + size_t alloc; strbuf_git_path_submodule(&objects_directory, path, "objects/"); if (!is_directory(objects_directory.buf)) { @@@ -138,8 -138,8 +138,8 @@@ objects_directory.len)) goto done; - alloc = objects_directory.len + 42; /* for "12/345..." sha1 */ - alt_odb = xmalloc(sizeof(*alt_odb) + alloc); + alloc = st_add(objects_directory.len, 42); /* for "12/345..." sha1 */ + alt_odb = xmalloc(st_add(sizeof(*alt_odb), alloc)); alt_odb->next = alt_odb_list; xsnprintf(alt_odb->base, alloc, "%s", objects_directory.buf); alt_odb->name = alt_odb->base + objects_directory.len; @@@ -705,8 -705,7 +705,7 @@@ static int get_next_submodule(struct ch return 0; } - static int fetch_start_failure(struct child_process *cp, - struct strbuf *err, + static int fetch_start_failure(struct strbuf *err, void *cb, void *task_cb) { struct submodule_parallel_fetch *spf = cb; @@@ -716,8 -715,8 +715,8 @@@ return 0; } - static int fetch_finish(int retvalue, struct child_process *cp, - struct strbuf *err, void *cb, void *task_cb) + static int fetch_finish(int retvalue, struct strbuf *err, + void *cb, void *task_cb) { struct submodule_parallel_fetch *spf = cb; @@@ -1087,9 -1086,11 +1086,9 @@@ void connect_work_tree_and_git_dir(cons /* Update core.worktree setting */ strbuf_reset(&file_name); strbuf_addf(&file_name, "%s/config", git_dir); - if (git_config_set_in_file(file_name.buf, "core.worktree", - relative_path(real_work_tree, git_dir, - &rel_path))) - die(_("Could not set core.worktree in %s"), - file_name.buf); + git_config_set_in_file(file_name.buf, "core.worktree", + relative_path(real_work_tree, git_dir, + &rel_path)); strbuf_release(&file_name); strbuf_release(&rel_path);