Merge branch 'jl/submodule-rm'
authorJeff King <peff@peff.net>
Mon, 29 Oct 2012 08:12:07 +0000 (04:12 -0400)
committerJeff King <peff@peff.net>
Mon, 29 Oct 2012 08:12:07 +0000 (04:12 -0400)
"git rm submodule" cannot blindly remove a submodule directory as
its working tree may have local changes, and worse yet, it may even
have its repository embedded in it. Teach it some special cases
where it is safe to remove a submodule, specifically, when there is
no local changes in the submodule working tree, and its repository
is not embedded in its working tree but is elsewhere and uses the
gitfile mechanism to point at it.

* jl/submodule-rm:
submodule: teach rm to remove submodules unless they contain a git directory

1  2 
builtin/rm.c
submodule.c
submodule.h
diff --combined builtin/rm.c
index b384c4c3cfe973346b5b295416f3883ae2d73c94,4a881ab27fd4efa0bb16955bb2e8944c1461b11c..2aea3b565318e11bdcc63f175631388f4f20ba44
@@@ -9,17 -9,67 +9,67 @@@
  #include "cache-tree.h"
  #include "tree-walk.h"
  #include "parse-options.h"
+ #include "submodule.h"
  
  static const char * const builtin_rm_usage[] = {
 -      "git rm [options] [--] <file>...",
 +      N_("git rm [options] [--] <file>..."),
        NULL
  };
  
  static struct {
        int nr, alloc;
-       const char **name;
+       struct {
+               const char *name;
+               char is_submodule;
+       } *entry;
  } list;
  
+ static int get_ours_cache_pos(const char *path, int pos)
+ {
+       int i = -pos - 1;
+       while ((i < active_nr) && !strcmp(active_cache[i]->name, path)) {
+               if (ce_stage(active_cache[i]) == 2)
+                       return i;
+               i++;
+       }
+       return -1;
+ }
+ static int check_submodules_use_gitfiles(void)
+ {
+       int i;
+       int errs = 0;
+       for (i = 0; i < list.nr; i++) {
+               const char *name = list.entry[i].name;
+               int pos;
+               struct cache_entry *ce;
+               struct stat st;
+               pos = cache_name_pos(name, strlen(name));
+               if (pos < 0) {
+                       pos = get_ours_cache_pos(name, pos);
+                       if (pos < 0)
+                               continue;
+               }
+               ce = active_cache[pos];
+               if (!S_ISGITLINK(ce->ce_mode) ||
+                   (lstat(ce->name, &st) < 0) ||
+                   is_empty_dir(name))
+                       continue;
+               if (!submodule_uses_gitfile(name))
+                       errs = error(_("submodule '%s' (or one of its nested "
+                                    "submodules) uses a .git directory\n"
+                                    "(use 'rm -rf' if you really want to remove "
+                                    "it including all of its history)"), name);
+       }
+       return errs;
+ }
  static int check_local_mod(unsigned char *head, int index_only)
  {
        /*
                struct stat st;
                int pos;
                struct cache_entry *ce;
-               const char *name = list.name[i];
+               const char *name = list.entry[i].name;
                unsigned char sha1[20];
                unsigned mode;
                int local_changes = 0;
                int staged_changes = 0;
  
                pos = cache_name_pos(name, strlen(name));
-               if (pos < 0)
-                       continue; /* removing unmerged entry */
+               if (pos < 0) {
+                       /*
+                        * Skip unmerged entries except for populated submodules
+                        * that could lose history when removed.
+                        */
+                       pos = get_ours_cache_pos(name, pos);
+                       if (pos < 0)
+                               continue;
+                       if (!S_ISGITLINK(active_cache[pos]->ce_mode) ||
+                           is_empty_dir(name))
+                               continue;
+               }
                ce = active_cache[pos];
  
                if (lstat(ce->name, &st) < 0) {
                        /* if a file was removed and it is now a
                         * directory, that is the same as ENOENT as
                         * far as git is concerned; we do not track
-                        * directories.
+                        * directories unless they are submodules.
                         */
-                       continue;
+                       if (!S_ISGITLINK(ce->ce_mode))
+                               continue;
                }
  
                /*
  
                /*
                 * Is the index different from the file in the work tree?
+                * If it's a submodule, is its work tree modified?
                 */
-               if (ce_match_stat(ce, &st, 0))
+               if (ce_match_stat(ce, &st, 0) ||
+                   (S_ISGITLINK(ce->ce_mode) &&
+                    !ok_to_remove_submodule(ce->name)))
                        local_changes = 1;
  
                /*
                                errs = error(_("'%s' has changes staged in the index\n"
                                             "(use --cached to keep the file, "
                                             "or -f to force removal)"), name);
-                       if (local_changes)
-                               errs = error(_("'%s' has local modifications\n"
-                                            "(use --cached to keep the file, "
-                                            "or -f to force removal)"), name);
+                       if (local_changes) {
+                               if (S_ISGITLINK(ce->ce_mode) &&
+                                   !submodule_uses_gitfile(name)) {
+                                       errs = error(_("submodule '%s' (or one of its nested "
+                                                    "submodules) uses a .git directory\n"
+                                                    "(use 'rm -rf' if you really want to remove "
+                                                    "it including all of its history)"), name);
+                               } else
+                                       errs = error(_("'%s' has local modifications\n"
+                                                    "(use --cached to keep the file, "
+                                                    "or -f to force removal)"), name);
+                       }
                }
        }
        return errs;
@@@ -130,13 -203,13 +203,13 @@@ static int show_only = 0, force = 0, in
  static int ignore_unmatch = 0;
  
  static struct option builtin_rm_options[] = {
 -      OPT__DRY_RUN(&show_only, "dry run"),
 -      OPT__QUIET(&quiet, "do not list removed files"),
 -      OPT_BOOLEAN( 0 , "cached",         &index_only, "only remove from the index"),
 -      OPT__FORCE(&force, "override the up-to-date check"),
 -      OPT_BOOLEAN('r', NULL,             &recursive,  "allow recursive removal"),
 +      OPT__DRY_RUN(&show_only, N_("dry run")),
 +      OPT__QUIET(&quiet, N_("do not list removed files")),
 +      OPT_BOOLEAN( 0 , "cached",         &index_only, N_("only remove from the index")),
 +      OPT__FORCE(&force, N_("override the up-to-date check")),
 +      OPT_BOOLEAN('r', NULL,             &recursive,  N_("allow recursive removal")),
        OPT_BOOLEAN( 0 , "ignore-unmatch", &ignore_unmatch,
 -                              "exit with a zero status even if nothing matched"),
 +                              N_("exit with a zero status even if nothing matched")),
        OPT_END(),
  };
  
@@@ -173,8 -246,9 +246,9 @@@ int cmd_rm(int argc, const char **argv
                struct cache_entry *ce = active_cache[i];
                if (!match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, seen))
                        continue;
-               ALLOC_GROW(list.name, list.nr + 1, list.alloc);
-               list.name[list.nr++] = ce->name;
+               ALLOC_GROW(list.entry, list.nr + 1, list.alloc);
+               list.entry[list.nr].name = ce->name;
+               list.entry[list.nr++].is_submodule = S_ISGITLINK(ce->ce_mode);
        }
  
        if (pathspec) {
                        hashclr(sha1);
                if (check_local_mod(sha1, index_only))
                        exit(1);
+       } else if (!index_only) {
+               if (check_submodules_use_gitfiles())
+                       exit(1);
        }
  
        /*
         * the index unless all of them succeed.
         */
        for (i = 0; i < list.nr; i++) {
-               const char *path = list.name[i];
+               const char *path = list.entry[i].name;
                if (!quiet)
                        printf("rm '%s'\n", path);
  
        if (!index_only) {
                int removed = 0;
                for (i = 0; i < list.nr; i++) {
-                       const char *path = list.name[i];
+                       const char *path = list.entry[i].name;
+                       if (list.entry[i].is_submodule) {
+                               if (is_empty_dir(path)) {
+                                       if (!rmdir(path)) {
+                                               removed = 1;
+                                               continue;
+                                       }
+                               } else {
+                                       struct strbuf buf = STRBUF_INIT;
+                                       strbuf_addstr(&buf, path);
+                                       if (!remove_dir_recursively(&buf, 0)) {
+                                               removed = 1;
+                                               strbuf_release(&buf);
+                                               continue;
+                                       }
+                                       strbuf_release(&buf);
+                                       /* Fallthrough and let remove_path() fail. */
+                               }
+                       }
                        if (!remove_path(path)) {
                                removed = 1;
                                continue;
diff --combined submodule.c
index 50f213e926374e86c23cdbfc730f6d81af4816b8,acb2fe09a77ba9ee99a629e70f3aa9a95bbd174c..e3e0b455eae925996aa8ecf8bf931edd0638d1cc
@@@ -588,13 -588,13 +588,13 @@@ static void calculate_changed_submodule
        initialized_fetch_ref_tips = 0;
  }
  
 -int fetch_populated_submodules(int num_options, const char **options,
 +int fetch_populated_submodules(const struct argv_array *options,
                               const char *prefix, int command_line_option,
                               int quiet)
  {
 -      int i, result = 0, argc = 0, default_argc;
 +      int i, result = 0;
        struct child_process cp;
 -      const char **argv;
 +      struct argv_array argv = ARGV_ARRAY_INIT;
        struct string_list_item *name_for_path;
        const char *work_tree = get_git_work_tree();
        if (!work_tree)
                if (read_cache() < 0)
                        die("index file corrupt");
  
 -      /* 6: "fetch" (options) --recurse-submodules-default default "--submodule-prefix" prefix NULL */
 -      argv = xcalloc(num_options + 6, sizeof(const char *));
 -      argv[argc++] = "fetch";
 -      for (i = 0; i < num_options; i++)
 -              argv[argc++] = options[i];
 -      argv[argc++] = "--recurse-submodules-default";
 -      default_argc = argc++;
 -      argv[argc++] = "--submodule-prefix";
 +      argv_array_push(&argv, "fetch");
 +      for (i = 0; i < options->argc; i++)
 +              argv_array_push(&argv, options->argv[i]);
 +      argv_array_push(&argv, "--recurse-submodules-default");
 +      /* default value, "--submodule-prefix" and its value are added later */
  
        memset(&cp, 0, sizeof(cp));
 -      cp.argv = argv;
        cp.env = local_repo_env;
        cp.git_cmd = 1;
        cp.no_stdin = 1;
                        if (!quiet)
                                printf("Fetching submodule %s%s\n", prefix, ce->name);
                        cp.dir = submodule_path.buf;
 -                      argv[default_argc] = default_argv;
 -                      argv[argc] = submodule_prefix.buf;
 +                      argv_array_push(&argv, default_argv);
 +                      argv_array_push(&argv, "--submodule-prefix");
 +                      argv_array_push(&argv, submodule_prefix.buf);
 +                      cp.argv = argv.argv;
                        if (run_command(&cp))
                                result = 1;
 +                      argv_array_pop(&argv);
 +                      argv_array_pop(&argv);
 +                      argv_array_pop(&argv);
                }
                strbuf_release(&submodule_path);
                strbuf_release(&submodule_git_dir);
                strbuf_release(&submodule_prefix);
        }
 -      free(argv);
 +      argv_array_clear(&argv);
  out:
        string_list_clear(&changed_submodule_paths, 1);
        return result;
@@@ -759,6 -758,86 +759,86 @@@ unsigned is_submodule_modified(const ch
        return dirty_submodule;
  }
  
+ int submodule_uses_gitfile(const char *path)
+ {
+       struct child_process cp;
+       const char *argv[] = {
+               "submodule",
+               "foreach",
+               "--quiet",
+               "--recursive",
+               "test -f .git",
+               NULL,
+       };
+       struct strbuf buf = STRBUF_INIT;
+       const char *git_dir;
+       strbuf_addf(&buf, "%s/.git", path);
+       git_dir = read_gitfile(buf.buf);
+       if (!git_dir) {
+               strbuf_release(&buf);
+               return 0;
+       }
+       strbuf_release(&buf);
+       /* Now test that all nested submodules use a gitfile too */
+       memset(&cp, 0, sizeof(cp));
+       cp.argv = argv;
+       cp.env = local_repo_env;
+       cp.git_cmd = 1;
+       cp.no_stdin = 1;
+       cp.no_stderr = 1;
+       cp.no_stdout = 1;
+       cp.dir = path;
+       if (run_command(&cp))
+               return 0;
+       return 1;
+ }
+ int ok_to_remove_submodule(const char *path)
+ {
+       struct stat st;
+       ssize_t len;
+       struct child_process cp;
+       const char *argv[] = {
+               "status",
+               "--porcelain",
+               "-u",
+               "--ignore-submodules=none",
+               NULL,
+       };
+       struct strbuf buf = STRBUF_INIT;
+       int ok_to_remove = 1;
+       if ((lstat(path, &st) < 0) || is_empty_dir(path))
+               return 1;
+       if (!submodule_uses_gitfile(path))
+               return 0;
+       memset(&cp, 0, sizeof(cp));
+       cp.argv = argv;
+       cp.env = local_repo_env;
+       cp.git_cmd = 1;
+       cp.no_stdin = 1;
+       cp.out = -1;
+       cp.dir = path;
+       if (start_command(&cp))
+               die("Could not run 'git status --porcelain -uall --ignore-submodules=none' in submodule %s", path);
+       len = strbuf_read(&buf, cp.out, 1024);
+       if (len > 2)
+               ok_to_remove = 0;
+       close(cp.out);
+       if (finish_command(&cp))
+               die("'git status --porcelain -uall --ignore-submodules=none' failed in submodule %s", path);
+       strbuf_release(&buf);
+       return ok_to_remove;
+ }
  static int find_first_merges(struct object_array *result, const char *path,
                struct commit *a, struct commit *b)
  {
                die("revision walk setup failed");
        while ((commit = get_revision(&revs)) != NULL) {
                struct object *o = &(commit->object);
 -              if (in_merge_bases(b, &commit, 1))
 +              if (in_merge_bases(b, commit))
                        add_object_array(o, NULL, &merges);
        }
        reset_revision_walk();
                contains_another = 0;
                for (j = 0; j < merges.nr; j++) {
                        struct commit *m2 = (struct commit *) merges.objects[j].item;
 -                      if (i != j && in_merge_bases(m2, &m1, 1)) {
 +                      if (i != j && in_merge_bases(m2, m1)) {
                                contains_another = 1;
                                break;
                        }
@@@ -866,18 -945,18 +946,18 @@@ int merge_submodule(unsigned char resul
        }
  
        /* check whether both changes are forward */
 -      if (!in_merge_bases(commit_base, &commit_a, 1) ||
 -          !in_merge_bases(commit_base, &commit_b, 1)) {
 +      if (!in_merge_bases(commit_base, commit_a) ||
 +          !in_merge_bases(commit_base, commit_b)) {
                MERGE_WARNING(path, "commits don't follow merge-base");
                return 0;
        }
  
        /* Case #1: a is contained in b or vice versa */
 -      if (in_merge_bases(commit_a, &commit_b, 1)) {
 +      if (in_merge_bases(commit_a, commit_b)) {
                hashcpy(result, b);
                return 1;
        }
 -      if (in_merge_bases(commit_b, &commit_a, 1)) {
 +      if (in_merge_bases(commit_b, commit_a)) {
                hashcpy(result, a);
                return 1;
        }
diff --combined submodule.h
index 594b50d51066be35d2f9dc9aa795f3ecdf131573,9c0f6a454ae0fdc88109f55e20ba7e39af17399c..f2e8271fc68c3b1e78d200e2af374a77f253f1ee
@@@ -2,7 -2,6 +2,7 @@@
  #define SUBMODULE_H
  
  struct diff_options;
 +struct argv_array;
  
  enum {
        RECURSE_SUBMODULES_ON_DEMAND = -1,
@@@ -24,10 -23,12 +24,12 @@@ void show_submodule_summary(FILE *f, co
                const char *del, const char *add, const char *reset);
  void set_config_fetch_recurse_submodules(int value);
  void check_for_new_submodule_commits(unsigned char new_sha1[20]);
 -int fetch_populated_submodules(int num_options, const char **options,
 +int fetch_populated_submodules(const struct argv_array *options,
                               const char *prefix, int command_line_option,
                               int quiet);
  unsigned is_submodule_modified(const char *path, int ignore_untracked);
+ int submodule_uses_gitfile(const char *path);
+ int ok_to_remove_submodule(const char *path);
  int merge_submodule(unsigned char result[20], const char *path, const unsigned char base[20],
                    const unsigned char a[20], const unsigned char b[20], int search);
  int find_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_name,