Merge branch 'sb/checkout-recurse-submodules'
authorJunio C Hamano <gitster@pobox.com>
Mon, 29 May 2017 03:34:41 +0000 (12:34 +0900)
committerJunio C Hamano <gitster@pobox.com>
Mon, 29 May 2017 03:34:41 +0000 (12:34 +0900)
"git checkout --recurse-submodules" did not quite work with a
submodule that itself has submodules.

* sb/checkout-recurse-submodules:
submodule: properly recurse for read-tree and checkout
submodule: avoid auto-discovery in new working tree manipulator code
submodule_move_head: reuse child_process structure for futher commands

1  2 
submodule.c
t/lib-submodule-update.sh
diff --combined submodule.c
index 54825100b29bd6bb9614bd2ab2431c6d418945b0,b3ae642f29205655342b08343d440d7f333a39f9..268f25fc2c471d6e84c73403456aaf496c132bf1
@@@ -20,7 -20,7 +20,7 @@@
  static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND;
  static int config_update_recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
  static int parallel_jobs = 1;
 -static struct string_list changed_submodule_paths = STRING_LIST_INIT_NODUP;
 +static struct string_list changed_submodule_paths = STRING_LIST_INIT_DUP;
  static int initialized_fetch_ref_tips;
  static struct oid_array ref_tips_before_fetch;
  static struct oid_array ref_tips_after_fetch;
@@@ -617,94 -617,6 +617,94 @@@ const struct submodule *submodule_from_
        return submodule_from_path(null_sha1, ce->name);
  }
  
 +static struct oid_array *submodule_commits(struct string_list *submodules,
 +                                         const char *path)
 +{
 +      struct string_list_item *item;
 +
 +      item = string_list_insert(submodules, path);
 +      if (item->util)
 +              return (struct oid_array *) item->util;
 +
 +      /* NEEDSWORK: should we have oid_array_init()? */
 +      item->util = xcalloc(1, sizeof(struct oid_array));
 +      return (struct oid_array *) item->util;
 +}
 +
 +static void collect_changed_submodules_cb(struct diff_queue_struct *q,
 +                                        struct diff_options *options,
 +                                        void *data)
 +{
 +      int i;
 +      struct string_list *changed = data;
 +
 +      for (i = 0; i < q->nr; i++) {
 +              struct diff_filepair *p = q->queue[i];
 +              struct oid_array *commits;
 +              if (!S_ISGITLINK(p->two->mode))
 +                      continue;
 +
 +              if (S_ISGITLINK(p->one->mode)) {
 +                      /*
 +                       * NEEDSWORK: We should honor the name configured in
 +                       * the .gitmodules file of the commit we are examining
 +                       * here to be able to correctly follow submodules
 +                       * being moved around.
 +                       */
 +                      commits = submodule_commits(changed, p->two->path);
 +                      oid_array_append(commits, &p->two->oid);
 +              } else {
 +                      /* Submodule is new or was moved here */
 +                      /*
 +                       * NEEDSWORK: When the .git directories of submodules
 +                       * live inside the superprojects .git directory some
 +                       * day we should fetch new submodules directly into
 +                       * that location too when config or options request
 +                       * that so they can be checked out from there.
 +                       */
 +                      continue;
 +              }
 +      }
 +}
 +
 +/*
 + * Collect the paths of submodules in 'changed' which have changed based on
 + * the revisions as specified in 'argv'.  Each entry in 'changed' will also
 + * have a corresponding 'struct oid_array' (in the 'util' field) which lists
 + * what the submodule pointers were updated to during the change.
 + */
 +static void collect_changed_submodules(struct string_list *changed,
 +                                     struct argv_array *argv)
 +{
 +      struct rev_info rev;
 +      const struct commit *commit;
 +
 +      init_revisions(&rev, NULL);
 +      setup_revisions(argv->argc, argv->argv, &rev, NULL);
 +      if (prepare_revision_walk(&rev))
 +              die("revision walk setup failed");
 +
 +      while ((commit = get_revision(&rev))) {
 +              struct rev_info diff_rev;
 +
 +              init_revisions(&diff_rev, NULL);
 +              diff_rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
 +              diff_rev.diffopt.format_callback = collect_changed_submodules_cb;
 +              diff_rev.diffopt.format_callback_data = changed;
 +              diff_tree_combined_merge(commit, 1, &diff_rev);
 +      }
 +
 +      reset_revision_walk();
 +}
 +
 +static void free_submodules_oids(struct string_list *submodules)
 +{
 +      struct string_list_item *item;
 +      for_each_string_list_item(item, submodules)
 +              oid_array_clear((struct oid_array *) item->util);
 +      string_list_clear(submodules, 1);
 +}
 +
  static int has_remote(const char *refname, const struct object_id *oid,
                      int flags, void *cb_data)
  {
@@@ -732,44 -644,10 +732,44 @@@ static int submodule_has_commits(const 
  {
        int has_commit = 1;
  
 +      /*
 +       * Perform a cheap, but incorrect check for the existance of 'commits'.
 +       * This is done by adding the submodule's object store to the in-core
 +       * object store, and then querying for each commit's existance.  If we
 +       * do not have the commit object anywhere, there is no chance we have
 +       * it in the object store of the correct submodule and have it
 +       * reachable from a ref, so we can fail early without spawning rev-list
 +       * which is expensive.
 +       */
        if (add_submodule_odb(path))
                return 0;
  
        oid_array_for_each_unique(commits, check_has_commit, &has_commit);
 +
 +      if (has_commit) {
 +              /*
 +               * Even if the submodule is checked out and the commit is
 +               * present, make sure it exists in the submodule's object store
 +               * and that it is reachable from a ref.
 +               */
 +              struct child_process cp = CHILD_PROCESS_INIT;
 +              struct strbuf out = STRBUF_INIT;
 +
 +              argv_array_pushl(&cp.args, "rev-list", "-n", "1", NULL);
 +              oid_array_for_each_unique(commits, append_oid_to_argv, &cp.args);
 +              argv_array_pushl(&cp.args, "--not", "--all", NULL);
 +
 +              prepare_submodule_repo_env(&cp.env_array);
 +              cp.git_cmd = 1;
 +              cp.no_stdin = 1;
 +              cp.dir = path;
 +
 +              if (capture_command(&cp, &out, GIT_MAX_HEXSZ + 1) || out.len)
 +                      has_commit = 0;
 +
 +              strbuf_release(&out);
 +      }
 +
        return has_commit;
  }
  
@@@ -817,31 -695,91 +817,31 @@@ static int submodule_needs_pushing(cons
        return 0;
  }
  
 -static struct oid_array *submodule_commits(struct string_list *submodules,
 -                                          const char *path)
 -{
 -      struct string_list_item *item;
 -
 -      item = string_list_insert(submodules, path);
 -      if (item->util)
 -              return (struct oid_array *) item->util;
 -
 -      /* NEEDSWORK: should we have oid_array_init()? */
 -      item->util = xcalloc(1, sizeof(struct oid_array));
 -      return (struct oid_array *) item->util;
 -}
 -
 -static void collect_submodules_from_diff(struct diff_queue_struct *q,
 -                                       struct diff_options *options,
 -                                       void *data)
 -{
 -      int i;
 -      struct string_list *submodules = data;
 -
 -      for (i = 0; i < q->nr; i++) {
 -              struct diff_filepair *p = q->queue[i];
 -              struct oid_array *commits;
 -              if (!S_ISGITLINK(p->two->mode))
 -                      continue;
 -              commits = submodule_commits(submodules, p->two->path);
 -              oid_array_append(commits, &p->two->oid);
 -      }
 -}
 -
 -static void find_unpushed_submodule_commits(struct commit *commit,
 -              struct string_list *needs_pushing)
 -{
 -      struct rev_info rev;
 -
 -      init_revisions(&rev, NULL);
 -      rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
 -      rev.diffopt.format_callback = collect_submodules_from_diff;
 -      rev.diffopt.format_callback_data = needs_pushing;
 -      diff_tree_combined_merge(commit, 1, &rev);
 -}
 -
 -static void free_submodules_sha1s(struct string_list *submodules)
 -{
 -      struct string_list_item *item;
 -      for_each_string_list_item(item, submodules)
 -              oid_array_clear((struct oid_array *) item->util);
 -      string_list_clear(submodules, 1);
 -}
 -
  int find_unpushed_submodules(struct oid_array *commits,
                const char *remotes_name, struct string_list *needs_pushing)
  {
 -      struct rev_info rev;
 -      struct commit *commit;
        struct string_list submodules = STRING_LIST_INIT_DUP;
        struct string_list_item *submodule;
        struct argv_array argv = ARGV_ARRAY_INIT;
  
 -      init_revisions(&rev, NULL);
 -
        /* argv.argv[0] will be ignored by setup_revisions */
        argv_array_push(&argv, "find_unpushed_submodules");
        oid_array_for_each_unique(commits, append_oid_to_argv, &argv);
        argv_array_push(&argv, "--not");
        argv_array_pushf(&argv, "--remotes=%s", remotes_name);
  
 -      setup_revisions(argv.argc, argv.argv, &rev, NULL);
 -      if (prepare_revision_walk(&rev))
 -              die("revision walk setup failed");
 -
 -      while ((commit = get_revision(&rev)) != NULL)
 -              find_unpushed_submodule_commits(commit, &submodules);
 -
 -      reset_revision_walk();
 -      argv_array_clear(&argv);
 +      collect_changed_submodules(&submodules, &argv);
  
        for_each_string_list_item(submodule, &submodules) {
 -              struct oid_array *commits = (struct oid_array *) submodule->util;
 +              struct oid_array *commits = submodule->util;
 +              const char *path = submodule->string;
  
 -              if (submodule_needs_pushing(submodule->string, commits))
 -                      string_list_insert(needs_pushing, submodule->string);
 +              if (submodule_needs_pushing(path, commits))
 +                      string_list_insert(needs_pushing, path);
        }
 -      free_submodules_sha1s(&submodules);
 +
 +      free_submodules_oids(&submodules);
 +      argv_array_clear(&argv);
  
        return needs_pushing->nr;
  }
@@@ -958,56 -896,125 +958,56 @@@ int push_unpushed_submodules(struct oid
        return ret;
  }
  
 -static int is_submodule_commit_present(const char *path, unsigned char sha1[20])
 +static int append_oid_to_array(const char *ref, const struct object_id *oid,
 +                             int flags, void *data)
  {
 -      int is_present = 0;
 -      if (!add_submodule_odb(path) && lookup_commit_reference(sha1)) {
 -              /* Even if the submodule is checked out and the commit is
 -               * present, make sure it is reachable from a ref. */
 -              struct child_process cp = CHILD_PROCESS_INIT;
 -              const char *argv[] = {"rev-list", "-n", "1", NULL, "--not", "--all", NULL};
 -              struct strbuf buf = STRBUF_INIT;
 -
 -              argv[3] = sha1_to_hex(sha1);
 -              cp.argv = argv;
 -              prepare_submodule_repo_env(&cp.env_array);
 -              cp.git_cmd = 1;
 -              cp.no_stdin = 1;
 -              cp.dir = path;
 -              if (!capture_command(&cp, &buf, 1024) && !buf.len)
 -                      is_present = 1;
 -
 -              strbuf_release(&buf);
 -      }
 -      return is_present;
 -}
 -
 -static void submodule_collect_changed_cb(struct diff_queue_struct *q,
 -                                       struct diff_options *options,
 -                                       void *data)
 -{
 -      int i;
 -      for (i = 0; i < q->nr; i++) {
 -              struct diff_filepair *p = q->queue[i];
 -              if (!S_ISGITLINK(p->two->mode))
 -                      continue;
 -
 -              if (S_ISGITLINK(p->one->mode)) {
 -                      /* NEEDSWORK: We should honor the name configured in
 -                       * the .gitmodules file of the commit we are examining
 -                       * here to be able to correctly follow submodules
 -                       * being moved around. */
 -                      struct string_list_item *path;
 -                      path = unsorted_string_list_lookup(&changed_submodule_paths, p->two->path);
 -                      if (!path && !is_submodule_commit_present(p->two->path, p->two->oid.hash))
 -                              string_list_append(&changed_submodule_paths, xstrdup(p->two->path));
 -              } else {
 -                      /* Submodule is new or was moved here */
 -                      /* NEEDSWORK: When the .git directories of submodules
 -                       * live inside the superprojects .git directory some
 -                       * day we should fetch new submodules directly into
 -                       * that location too when config or options request
 -                       * that so they can be checked out from there. */
 -                      continue;
 -              }
 -      }
 -}
 -
 -static int add_sha1_to_array(const char *ref, const struct object_id *oid,
 -                           int flags, void *data)
 -{
 -      oid_array_append(data, oid);
 +      struct oid_array *array = data;
 +      oid_array_append(array, oid);
        return 0;
  }
  
  void check_for_new_submodule_commits(struct object_id *oid)
  {
        if (!initialized_fetch_ref_tips) {
 -              for_each_ref(add_sha1_to_array, &ref_tips_before_fetch);
 +              for_each_ref(append_oid_to_array, &ref_tips_before_fetch);
                initialized_fetch_ref_tips = 1;
        }
  
        oid_array_append(&ref_tips_after_fetch, oid);
  }
  
 -static int add_oid_to_argv(const struct object_id *oid, void *data)
 -{
 -      argv_array_push(data, oid_to_hex(oid));
 -      return 0;
 -}
 -
  static void calculate_changed_submodule_paths(void)
  {
 -      struct rev_info rev;
 -      struct commit *commit;
        struct argv_array argv = ARGV_ARRAY_INIT;
 +      struct string_list changed_submodules = STRING_LIST_INIT_DUP;
 +      const struct string_list_item *item;
  
        /* No need to check if there are no submodules configured */
        if (!submodule_from_path(NULL, NULL))
                return;
  
 -      init_revisions(&rev, NULL);
        argv_array_push(&argv, "--"); /* argv[0] program name */
        oid_array_for_each_unique(&ref_tips_after_fetch,
 -                                 add_oid_to_argv, &argv);
 +                                 append_oid_to_argv, &argv);
        argv_array_push(&argv, "--not");
        oid_array_for_each_unique(&ref_tips_before_fetch,
 -                                 add_oid_to_argv, &argv);
 -      setup_revisions(argv.argc, argv.argv, &rev, NULL);
 -      if (prepare_revision_walk(&rev))
 -              die("revision walk setup failed");
 +                                 append_oid_to_argv, &argv);
  
        /*
         * Collect all submodules (whether checked out or not) for which new
         * commits have been recorded upstream in "changed_submodule_paths".
         */
 -      while ((commit = get_revision(&rev))) {
 -              struct commit_list *parent = commit->parents;
 -              while (parent) {
 -                      struct diff_options diff_opts;
 -                      diff_setup(&diff_opts);
 -                      DIFF_OPT_SET(&diff_opts, RECURSIVE);
 -                      diff_opts.output_format |= DIFF_FORMAT_CALLBACK;
 -                      diff_opts.format_callback = submodule_collect_changed_cb;
 -                      diff_setup_done(&diff_opts);
 -                      diff_tree_sha1(parent->item->object.oid.hash, commit->object.oid.hash, "", &diff_opts);
 -                      diffcore_std(&diff_opts);
 -                      diff_flush(&diff_opts);
 -                      parent = parent->next;
 -              }
 +      collect_changed_submodules(&changed_submodules, &argv);
 +
 +      for_each_string_list_item(item, &changed_submodules) {
 +              struct oid_array *commits = item->util;
 +              const char *path = item->string;
 +
 +              if (!submodule_has_commits(path, commits))
 +                      string_list_append(&changed_submodule_paths, path);
        }
  
 +      free_submodules_oids(&changed_submodules);
        argv_array_clear(&argv);
        oid_array_clear(&ref_tips_before_fetch);
        oid_array_clear(&ref_tips_after_fetch);
@@@ -1356,7 -1363,7 +1356,7 @@@ static int submodule_has_dirty_index(co
  {
        struct child_process cp = CHILD_PROCESS_INIT;
  
-       prepare_submodule_repo_env_no_git_dir(&cp.env_array);
+       prepare_submodule_repo_env(&cp.env_array);
  
        cp.git_cmd = 1;
        argv_array_pushl(&cp.args, "diff-index", "--quiet",
  static void submodule_reset_index(const char *path)
  {
        struct child_process cp = CHILD_PROCESS_INIT;
-       prepare_submodule_repo_env_no_git_dir(&cp.env_array);
+       prepare_submodule_repo_env(&cp.env_array);
  
        cp.git_cmd = 1;
        cp.no_stdin = 1;
@@@ -1402,23 -1409,6 +1402,23 @@@ int submodule_move_head(const char *pat
        int ret = 0;
        struct child_process cp = CHILD_PROCESS_INIT;
        const struct submodule *sub;
 +      int *error_code_ptr, error_code;
 +
 +      if (!is_submodule_initialized(path))
 +              return 0;
 +
 +      if (flags & SUBMODULE_MOVE_HEAD_FORCE)
 +              /*
 +               * Pass non NULL pointer to is_submodule_populated_gently
 +               * to prevent die()-ing. We'll use connect_work_tree_and_git_dir
 +               * to fixup the submodule in the force case later.
 +               */
 +              error_code_ptr = &error_code;
 +      else
 +              error_code_ptr = NULL;
 +
 +      if (old && !is_submodule_populated_gently(path, error_code_ptr))
 +              return 0;
  
        sub = submodule_from_path(null_sha1, path);
  
                                absorb_git_dir_into_superproject("", path,
                                        ABSORB_GITDIR_RECURSE_SUBMODULES);
                } else {
 -                      struct strbuf sb = STRBUF_INIT;
 -                      strbuf_addf(&sb, "%s/modules/%s",
 +                      char *gitdir = xstrfmt("%s/modules/%s",
                                    get_git_common_dir(), sub->name);
 -                      connect_work_tree_and_git_dir(path, sb.buf);
 -                      strbuf_release(&sb);
 +                      connect_work_tree_and_git_dir(path, gitdir);
 +                      free(gitdir);
  
                        /* make sure the index is clean as well */
                        submodule_reset_index(path);
                }
 +
 +              if (old && (flags & SUBMODULE_MOVE_HEAD_FORCE)) {
 +                      char *gitdir = xstrfmt("%s/modules/%s",
 +                                  get_git_common_dir(), sub->name);
 +                      connect_work_tree_and_git_dir(path, gitdir);
 +                      free(gitdir);
 +              }
        }
  
-       prepare_submodule_repo_env_no_git_dir(&cp.env_array);
+       prepare_submodule_repo_env(&cp.env_array);
  
        cp.git_cmd = 1;
        cp.no_stdin = 1;
  
        argv_array_pushf(&cp.args, "--super-prefix=%s%s/",
                        get_super_prefix_or_empty(), path);
-       argv_array_pushl(&cp.args, "read-tree", NULL);
+       argv_array_pushl(&cp.args, "read-tree", "--recurse-submodules", NULL);
  
        if (flags & SUBMODULE_MOVE_HEAD_DRY_RUN)
                argv_array_push(&cp.args, "-n");
  
        if (!(flags & SUBMODULE_MOVE_HEAD_DRY_RUN)) {
                if (new) {
-                       struct child_process cp1 = CHILD_PROCESS_INIT;
+                       child_process_init(&cp);
                        /* also set the HEAD accordingly */
-                       cp1.git_cmd = 1;
-                       cp1.no_stdin = 1;
-                       cp1.dir = path;
+                       cp.git_cmd = 1;
+                       cp.no_stdin = 1;
+                       cp.dir = path;
  
-                       argv_array_pushl(&cp1.args, "update-ref", "HEAD", new, NULL);
+                       prepare_submodule_repo_env(&cp.env_array);
+                       argv_array_pushl(&cp.args, "update-ref", "HEAD", new, NULL);
  
-                       if (run_command(&cp1)) {
+                       if (run_command(&cp)) {
                                ret = -1;
                                goto out;
                        }
index f0b1b1820607249929f9d505de3d8c25b5ae92b4,2c17826e950ce221ae9efa10cfa99dc47c042825..58bd4aeb2c52c67831a1dced2ef6b09755d417d3
@@@ -73,7 -73,6 +73,7 @@@ create_lib_submodule_repo () 
  
                git checkout -b "add_sub1" &&
                git submodule add ../submodule_update_sub1 sub1 &&
 +              git submodule add ../submodule_update_sub1 uninitialized_sub &&
                git config -f .gitmodules submodule.sub1.ignore all &&
                git config submodule.sub1.ignore all &&
                git add .gitmodules &&
@@@ -788,11 -787,6 +788,6 @@@ test_submodule_switch_recursing () 
        then
                RESULTDS=failure
        fi
-       RESULTR=success
-       if test "$KNOWN_FAILURE_SUBMODULE_RECURSIVE_NESTED" = 1
-       then
-               RESULTR=failure
-       fi
        RESULTOI=success
        if test "$KNOWN_FAILURE_SUBMODULE_OVERWRITE_IGNORED_UNTRACKED" = 1
        then
        '
  
        # recursing deeper than one level doesn't work yet.
-       test_expect_$RESULTR "$command: modified submodule updates submodule recursively" '
+       test_expect_success "$command: modified submodule updates submodule recursively" '
                prolog &&
                reset_work_tree_to_interested add_nested_sub &&
                (
@@@ -1213,31 -1207,14 +1208,31 @@@ test_submodule_forced_switch_recursing 
                )
        '
        # Updating a submodule from an invalid sha1 updates
 -      test_expect_success "$command: modified submodule does not update submodule work tree from invalid commit" '
 +      test_expect_success "$command: modified submodule does update submodule work tree from invalid commit" '
                prolog &&
                reset_work_tree_to_interested invalid_sub1 &&
                (
                        cd submodule_update &&
                        git branch -t valid_sub1 origin/valid_sub1 &&
 -                      test_must_fail $command valid_sub1 &&
 -                      test_superproject_content origin/invalid_sub1
 +                      $command valid_sub1 &&
 +                      test_superproject_content origin/valid_sub1 &&
 +                      test_submodule_content sub1 origin/valid_sub1
 +              )
 +      '
 +
 +      # Old versions of Git were buggy writing the .git link file
 +      # (e.g. before f8eaa0ba98b and then moving the superproject repo
 +      # whose submodules contained absolute paths)
 +      test_expect_success "$command: updating submodules fixes .git links" '
 +              prolog &&
 +              reset_work_tree_to_interested add_sub1 &&
 +              (
 +                      cd submodule_update &&
 +                      git branch -t modify_sub1 origin/modify_sub1 &&
 +                      echo "gitdir: bogus/path" >sub1/.git &&
 +                      $command modify_sub1 &&
 +                      test_superproject_content origin/modify_sub1 &&
 +                      test_submodule_content sub1 origin/modify_sub1
                )
        '
  }