Merge branch 'sb/submodule-helper-clone-regression-fix' into sb/submodule-init
authorJunio C Hamano <gitster@pobox.com>
Thu, 14 Apr 2016 19:46:11 +0000 (12:46 -0700)
committerJunio C Hamano <gitster@pobox.com>
Thu, 14 Apr 2016 19:46:11 +0000 (12:46 -0700)
* sb/submodule-helper-clone-regression-fix:
submodule--helper, module_clone: catch fprintf failure
submodule--helper: do not borrow absolute_path() result for too long
submodule--helper, module_clone: always operate on absolute paths
submodule--helper clone: create the submodule path just once
submodule--helper: fix potential NULL-dereference
recursive submodules: test for relative paths

1  2 
builtin/submodule--helper.c
t/t7400-submodule-basic.sh
index a484945d37911828554769a38affddaf3067dc54,b3a60f56c345fce8dabc31826769ff879fbde931..a39ad1be995da7e46be8db3db2173d323c6beae9
@@@ -153,11 -153,11 +153,11 @@@ static int clone_submodule(const char *
  
  static int module_clone(int argc, const char **argv, const char *prefix)
  {
-       const char *path = NULL, *name = NULL, *url = NULL;
+       const char *name = NULL, *url = NULL;
        const char *reference = NULL, *depth = NULL;
        int quiet = 0;
        FILE *submodule_dot_git;
-       char *sm_gitdir, *cwd, *p;
+       char *p, *path = NULL, *sm_gitdir;
        struct strbuf rel_path = STRBUF_INIT;
        struct strbuf sb = STRBUF_INIT;
  
        argc = parse_options(argc, argv, prefix, module_clone_options,
                             git_submodule_helper_usage, 0);
  
+       if (!path || !*path)
+               die(_("submodule--helper: unspecified or empty --path"));
        strbuf_addf(&sb, "%s/modules/%s", get_git_dir(), name);
-       sm_gitdir = strbuf_detach(&sb, NULL);
+       sm_gitdir = xstrdup(absolute_path(sb.buf));
+       strbuf_reset(&sb);
+       if (!is_absolute_path(path)) {
+               strbuf_addf(&sb, "%s/%s", get_git_work_tree(), path);
+               path = strbuf_detach(&sb, NULL);
+       } else
+               path = xstrdup(path);
  
        if (!file_exists(sm_gitdir)) {
                if (safe_create_leading_directories_const(sm_gitdir) < 0)
        }
  
        /* Write a .git file in the submodule to redirect to the superproject. */
-       if (safe_create_leading_directories_const(path) < 0)
-               die(_("could not create directory '%s'"), path);
-       if (path && *path)
-               strbuf_addf(&sb, "%s/.git", path);
-       else
-               strbuf_addstr(&sb, ".git");
+       strbuf_addf(&sb, "%s/.git", path);
        if (safe_create_leading_directories_const(sb.buf) < 0)
                die(_("could not create leading directories of '%s'"), sb.buf);
        submodule_dot_git = fopen(sb.buf, "w");
        if (!submodule_dot_git)
                die_errno(_("cannot open file '%s'"), sb.buf);
  
-       fprintf(submodule_dot_git, "gitdir: %s\n",
-               relative_path(sm_gitdir, path, &rel_path));
+       fprintf_or_die(submodule_dot_git, "gitdir: %s\n",
+                      relative_path(sm_gitdir, path, &rel_path));
        if (fclose(submodule_dot_git))
                die(_("could not close file %s"), sb.buf);
        strbuf_reset(&sb);
        strbuf_reset(&rel_path);
  
-       cwd = xgetcwd();
        /* Redirect the worktree of the submodule in the superproject's config */
-       if (!is_absolute_path(sm_gitdir)) {
-               strbuf_addf(&sb, "%s/%s", cwd, sm_gitdir);
-               free(sm_gitdir);
-               sm_gitdir = strbuf_detach(&sb, NULL);
-       }
-       strbuf_addf(&sb, "%s/%s", cwd, path);
        p = git_pathdup_submodule(path, "config");
        if (!p)
                die(_("could not get submodule directory for '%s'"), path);
        git_config_set_in_file(p, "core.worktree",
-                              relative_path(sb.buf, sm_gitdir, &rel_path));
+                              relative_path(path, sm_gitdir, &rel_path));
        strbuf_release(&sb);
        strbuf_release(&rel_path);
        free(sm_gitdir);
-       free(cwd);
+       free(path);
        free(p);
        return 0;
  }
  
 +struct submodule_update_clone {
 +      /* index into 'list', the list of submodules to look into for cloning */
 +      int current;
 +      struct module_list list;
 +      unsigned warn_if_uninitialized : 1;
 +
 +      /* update parameter passed via commandline */
 +      struct submodule_update_strategy update;
 +
 +      /* configuration parameters which are passed on to the children */
 +      int quiet;
 +      const char *reference;
 +      const char *depth;
 +      const char *recursive_prefix;
 +      const char *prefix;
 +
 +      /* Machine-readable status lines to be consumed by git-submodule.sh */
 +      struct string_list projectlines;
 +
 +      /* If we want to stop as fast as possible and return an error */
 +      unsigned quickstop : 1;
 +};
 +#define SUBMODULE_UPDATE_CLONE_INIT {0, MODULE_LIST_INIT, 0, \
 +      SUBMODULE_UPDATE_STRATEGY_INIT, 0, NULL, NULL, NULL, NULL, \
 +      STRING_LIST_INIT_DUP, 0}
 +
 +/**
 + * Determine whether 'ce' needs to be cloned. If so, prepare the 'child' to
 + * run the clone. Returns 1 if 'ce' needs to be cloned, 0 otherwise.
 + */
 +static int prepare_to_clone_next_submodule(const struct cache_entry *ce,
 +                                         struct child_process *child,
 +                                         struct submodule_update_clone *suc,
 +                                         struct strbuf *out)
 +{
 +      const struct submodule *sub = NULL;
 +      struct strbuf displaypath_sb = STRBUF_INIT;
 +      struct strbuf sb = STRBUF_INIT;
 +      const char *displaypath = NULL;
 +      char *url = NULL;
 +      int needs_cloning = 0;
 +
 +      if (ce_stage(ce)) {
 +              if (suc->recursive_prefix)
 +                      strbuf_addf(&sb, "%s/%s", suc->recursive_prefix, ce->name);
 +              else
 +                      strbuf_addf(&sb, "%s", ce->name);
 +              strbuf_addf(out, _("Skipping unmerged submodule %s"), sb.buf);
 +              strbuf_addch(out, '\n');
 +              goto cleanup;
 +      }
 +
 +      sub = submodule_from_path(null_sha1, ce->name);
 +
 +      if (suc->recursive_prefix)
 +              displaypath = relative_path(suc->recursive_prefix,
 +                                          ce->name, &displaypath_sb);
 +      else
 +              displaypath = ce->name;
 +
 +      if (suc->update.type == SM_UPDATE_NONE
 +          || (suc->update.type == SM_UPDATE_UNSPECIFIED
 +              && sub->update_strategy.type == SM_UPDATE_NONE)) {
 +              strbuf_addf(out, _("Skipping submodule '%s'"), displaypath);
 +              strbuf_addch(out, '\n');
 +              goto cleanup;
 +      }
 +
 +      /*
 +       * Looking up the url in .git/config.
 +       * We must not fall back to .gitmodules as we only want
 +       * to process configured submodules.
 +       */
 +      strbuf_reset(&sb);
 +      strbuf_addf(&sb, "submodule.%s.url", sub->name);
 +      git_config_get_string(sb.buf, &url);
 +      if (!url) {
 +              /*
 +               * Only mention uninitialized submodules when their
 +               * path have been specified
 +               */
 +              if (suc->warn_if_uninitialized) {
 +                      strbuf_addf(out,
 +                              _("Submodule path '%s' not initialized"),
 +                              displaypath);
 +                      strbuf_addch(out, '\n');
 +                      strbuf_addstr(out,
 +                              _("Maybe you want to use 'update --init'?"));
 +                      strbuf_addch(out, '\n');
 +              }
 +              goto cleanup;
 +      }
 +
 +      strbuf_reset(&sb);
 +      strbuf_addf(&sb, "%s/.git", ce->name);
 +      needs_cloning = !file_exists(sb.buf);
 +
 +      strbuf_reset(&sb);
 +      strbuf_addf(&sb, "%06o %s %d %d\t%s\n", ce->ce_mode,
 +                      sha1_to_hex(ce->sha1), ce_stage(ce),
 +                      needs_cloning, ce->name);
 +      string_list_append(&suc->projectlines, sb.buf);
 +
 +      if (!needs_cloning)
 +              goto cleanup;
 +
 +      child->git_cmd = 1;
 +      child->no_stdin = 1;
 +      child->stdout_to_stderr = 1;
 +      child->err = -1;
 +      argv_array_push(&child->args, "submodule--helper");
 +      argv_array_push(&child->args, "clone");
 +      if (suc->quiet)
 +              argv_array_push(&child->args, "--quiet");
 +      if (suc->prefix)
 +              argv_array_pushl(&child->args, "--prefix", suc->prefix, NULL);
 +      argv_array_pushl(&child->args, "--path", sub->path, NULL);
 +      argv_array_pushl(&child->args, "--name", sub->name, NULL);
 +      argv_array_pushl(&child->args, "--url", url, NULL);
 +      if (suc->reference)
 +              argv_array_push(&child->args, suc->reference);
 +      if (suc->depth)
 +              argv_array_push(&child->args, suc->depth);
 +
 +cleanup:
 +      free(url);
 +      strbuf_reset(&displaypath_sb);
 +      strbuf_reset(&sb);
 +
 +      return needs_cloning;
 +}
 +
 +static int update_clone_get_next_task(struct child_process *child,
 +                                    struct strbuf *err,
 +                                    void *suc_cb,
 +                                    void **void_task_cb)
 +{
 +      struct submodule_update_clone *suc = suc_cb;
 +
 +      for (; suc->current < suc->list.nr; suc->current++) {
 +              const struct cache_entry *ce = suc->list.entries[suc->current];
 +              if (prepare_to_clone_next_submodule(ce, child, suc, err)) {
 +                      suc->current++;
 +                      return 1;
 +              }
 +      }
 +      return 0;
 +}
 +
 +static int update_clone_start_failure(struct strbuf *err,
 +                                    void *suc_cb,
 +                                    void *void_task_cb)
 +{
 +      struct submodule_update_clone *suc = suc_cb;
 +      suc->quickstop = 1;
 +      return 1;
 +}
 +
 +static int update_clone_task_finished(int result,
 +                                    struct strbuf *err,
 +                                    void *suc_cb,
 +                                    void *void_task_cb)
 +{
 +      struct submodule_update_clone *suc = suc_cb;
 +
 +      if (!result)
 +              return 0;
 +
 +      suc->quickstop = 1;
 +      return 1;
 +}
 +
 +static int update_clone(int argc, const char **argv, const char *prefix)
 +{
 +      const char *update = NULL;
 +      int max_jobs = -1;
 +      struct string_list_item *item;
 +      struct pathspec pathspec;
 +      struct submodule_update_clone suc = SUBMODULE_UPDATE_CLONE_INIT;
 +
 +      struct option module_update_clone_options[] = {
 +              OPT_STRING(0, "prefix", &prefix,
 +                         N_("path"),
 +                         N_("path into the working tree")),
 +              OPT_STRING(0, "recursive-prefix", &suc.recursive_prefix,
 +                         N_("path"),
 +                         N_("path into the working tree, across nested "
 +                            "submodule boundaries")),
 +              OPT_STRING(0, "update", &update,
 +                         N_("string"),
 +                         N_("rebase, merge, checkout or none")),
 +              OPT_STRING(0, "reference", &suc.reference, N_("repo"),
 +                         N_("reference repository")),
 +              OPT_STRING(0, "depth", &suc.depth, "<depth>",
 +                         N_("Create a shallow clone truncated to the "
 +                            "specified number of revisions")),
 +              OPT_INTEGER('j', "jobs", &max_jobs,
 +                          N_("parallel jobs")),
 +              OPT__QUIET(&suc.quiet, N_("don't print cloning progress")),
 +              OPT_END()
 +      };
 +
 +      const char *const git_submodule_helper_usage[] = {
 +              N_("git submodule--helper update_clone [--prefix=<path>] [<path>...]"),
 +              NULL
 +      };
 +      suc.prefix = prefix;
 +
 +      argc = parse_options(argc, argv, prefix, module_update_clone_options,
 +                           git_submodule_helper_usage, 0);
 +
 +      if (update)
 +              if (parse_submodule_update_strategy(update, &suc.update) < 0)
 +                      die(_("bad value for update parameter"));
 +
 +      if (module_list_compute(argc, argv, prefix, &pathspec, &suc.list) < 0)
 +              return 1;
 +
 +      if (pathspec.nr)
 +              suc.warn_if_uninitialized = 1;
 +
 +      /* Overlay the parsed .gitmodules file with .git/config */
 +      gitmodules_config();
 +      git_config(submodule_config, NULL);
 +
 +      if (max_jobs < 0)
 +              max_jobs = parallel_submodules();
 +
 +      run_processes_parallel(max_jobs,
 +                             update_clone_get_next_task,
 +                             update_clone_start_failure,
 +                             update_clone_task_finished,
 +                             &suc);
 +
 +      /*
 +       * We saved the output and put it out all at once now.
 +       * That means:
 +       * - the listener does not have to interleave their (checkout)
 +       *   work with our fetching.  The writes involved in a
 +       *   checkout involve more straightforward sequential I/O.
 +       * - the listener can avoid doing any work if fetching failed.
 +       */
 +      if (suc.quickstop)
 +              return 1;
 +
 +      for_each_string_list_item(item, &suc.projectlines)
 +              utf8_fprintf(stdout, "%s", item->string);
 +
 +      return 0;
 +}
 +
  struct cmd_struct {
        const char *cmd;
        int (*fn)(int, const char **, const char *);
@@@ -515,20 -259,19 +510,20 @@@ static struct cmd_struct commands[] = 
        {"list", module_list},
        {"name", module_name},
        {"clone", module_clone},
 +      {"update-clone", update_clone}
  };
  
  int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
  {
        int i;
        if (argc < 2)
 -              die(_("fatal: submodule--helper subcommand must be "
 +              die(_("submodule--helper subcommand must be "
                      "called with a subcommand"));
  
        for (i = 0; i < ARRAY_SIZE(commands); i++)
                if (!strcmp(argv[1], commands[i].cmd))
                        return commands[i].fn(argc - 1, argv + 1, prefix);
  
 -      die(_("fatal: '%s' is not a valid submodule--helper "
 +      die(_("'%s' is not a valid submodule--helper "
              "subcommand"), argv[1]);
  }
index 5991e3c015dc8e21c483ab74e858b508ef37a061,ea3fabbed87a6bd3e91f4d1d9a640411fd04b9d6..033d034f5d69cb9fb7c70f1330a2e6cc7343ca92
@@@ -462,7 -462,7 +462,7 @@@ test_expect_success 'update --init' 
        git config --remove-section submodule.example &&
        test_must_fail git config submodule.example.url &&
  
 -      git submodule update init > update.out &&
 +      git submodule update init 2> update.out &&
        cat update.out &&
        test_i18ngrep "not initialized" update.out &&
        test_must_fail git rev-parse --resolve-git-dir init/.git &&
@@@ -480,7 -480,7 +480,7 @@@ test_expect_success 'update --init fro
        mkdir -p sub &&
        (
                cd sub &&
 -              git submodule update ../init >update.out &&
 +              git submodule update ../init 2>update.out &&
                cat update.out &&
                test_i18ngrep "not initialized" update.out &&
                test_must_fail git rev-parse --resolve-git-dir ../init/.git &&
@@@ -818,6 -818,47 +818,47 @@@ test_expect_success 'submodule add --na
        )
  '
  
+ test_expect_success 'recursive relative submodules stay relative' '
+       test_when_finished "rm -rf super clone2 subsub sub3" &&
+       mkdir subsub &&
+       (
+               cd subsub &&
+               git init &&
+               >t &&
+               git add t &&
+               git commit -m "initial commit"
+       ) &&
+       mkdir sub3 &&
+       (
+               cd sub3 &&
+               git init &&
+               >t &&
+               git add t &&
+               git commit -m "initial commit" &&
+               git submodule add ../subsub dirdir/subsub &&
+               git commit -m "add submodule subsub"
+       ) &&
+       mkdir super &&
+       (
+               cd super &&
+               git init &&
+               >t &&
+               git add t &&
+               git commit -m "initial commit" &&
+               git submodule add ../sub3 &&
+               git commit -m "add submodule sub"
+       ) &&
+       git clone super clone2 &&
+       (
+               cd clone2 &&
+               git submodule update --init --recursive &&
+               echo "gitdir: ../.git/modules/sub3" >./sub3/.git_expect &&
+               echo "gitdir: ../../../.git/modules/sub3/modules/dirdir/subsub" >./sub3/dirdir/subsub/.git_expect
+       ) &&
+       test_cmp clone2/sub3/.git_expect clone2/sub3/.git &&
+       test_cmp clone2/sub3/dirdir/subsub/.git_expect clone2/sub3/dirdir/subsub/.git
+ '
  test_expect_success 'submodule add with an existing name fails unless forced' '
        (
                cd addtest2 &&