Merge branch 'sb/submodule-parallel-fetch'
authorJunio C Hamano <gitster@pobox.com>
Fri, 4 Mar 2016 21:46:30 +0000 (13:46 -0800)
committerJunio C Hamano <gitster@pobox.com>
Fri, 4 Mar 2016 21:46:30 +0000 (13:46 -0800)
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

1  2 
run-command.c
run-command.h
submodule.c
diff --combined run-command.c
index 863dad52f1913d8fa20fccd23fc527580c6fcaf7,8e3ad07ffe823f9eb29eaf2794ab0549b4afd1ea..c72601056cf5ae7be2593ae89af4effc26a1b043
@@@ -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);
                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);
        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 42917e8618ea469ff56d52704794b6e3bc86cf97,6e17894f010fb59fb5d89ac5f4be5273fbd89162..3d1e59e26e33d062a10698fc139f7fc0f4ae14ec
@@@ -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);
  
   * 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);
   * (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 24fb81ac62e2161e79cbd5ab1f2a241663c2a3a1,916fc84aaabf7cc4d2974d6b930c40fbcbfb8bb1..62c4356c50d4a41381336559f4e9af27e520ad0d
@@@ -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)) {
                                        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;
        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);