Currently using "git rm" on a populated submodule produces this error:
fatal: git rm: '<submodule path>': Is a directory
Using it on an unpopulated submodule removes the empty directory silently
and removes the gitlink from the index, while it doesn't do the latter
when the submodule is populated but errors out.
While the error technically correct (the submodule directory can't be
removed because it still contains the checked out work tree) rm could do
better because it knows it is a submodule. It should remove the gitlink
from the index no matter if it is populated or not. Also not being able to
remove a submodule directory isn't an error but should only issue a
warning to inform the user about that fact while removing the gitlink from
the index nonetheless.
Change "git rm" so it only issues a warning if a populated submodule
cannot be removed. Also apply the same policy as for regular files and
require forcing when the submodules HEAD is different than what is
recorded in the index. To achieve that the list storing to be deleted
files and submodules is extended by a boolean recording if the entry is a
submodule or not to reuse that information in the removal phase.
While this changes behavior of "git rm", it only fixes an error where it
never worked properly.
Signed-off-by: Jens Lehmann <Jens.Lehmann@xxxxxx>
---
builtin/rm.c | 34 +++++++++++++++++-----------
t/t3600-rm.sh | 71 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 92 insertions(+), 13 deletions(-)
diff --git a/builtin/rm.c b/builtin/rm.c
index 90c8a50..1c73dcf 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -17,7 +17,10 @@ static const char * const builtin_rm_usage[] = {
static struct {
int nr, alloc;
- const char **name;
+ struct {
+ const char *name;
+ char is_submodule;
+ } *entry;
} list;
static int check_local_mod(unsigned char *head, int index_only)
@@ -37,7 +40,7 @@ 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;
@@ -54,11 +57,11 @@ static int check_local_mod(unsigned char *head, int index_only)
/* It already vanished from the working tree */
continue;
}
- else if (S_ISDIR(st.st_mode)) {
+ else if (S_ISDIR(st.st_mode) && !S_ISGITLINK(ce->ce_mode)) {
/* 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;
}
@@ -173,8 +176,9 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
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) {
@@ -222,7 +226,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
* 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);
@@ -244,13 +248,17 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
if (!index_only) {
int removed = 0;
for (i = 0; i < list.nr; i++) {
- const char *path = list.name[i];
- if (!remove_path(path)) {
- removed = 1;
- continue;
+ const char *path = list.entry[i].name;
+ if (!list.entry[i].is_submodule) {
+ if (!remove_path(path)) {
+ removed = 1;
+ continue;
+ }
+ if (!removed)
+ die_errno("git rm: '%s'", path);
+ } else {
+ rmdir_or_warn(path);
}
- if (!removed)
- die_errno("git rm: '%s'", path);
}
}
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index 9fd28bc..2af8bb9 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -262,4 +262,75 @@ test_expect_success 'rm removes subdirectories recursively' '
! test -d dir
'
+cat >expect <<EOF
+D submod
+EOF
+
+test_expect_success 'rm removes empty submodules from work tree' '
+ mkdir submod &&
+ git update-index --add --cacheinfo 160000 $(git rev-parse HEAD) submod &&
+ git config -f .gitmodules submodule.sub.url ./. &&
+ git config -f .gitmodules submodule.sub.path submod &&
+ git submodule init &&
+ git add .gitmodules &&
+ git commit -m "add submodule" &&
+ git rm submod &&
+ test ! -e submod &&
+ git status -s -uno > actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'rm leaves work tree of populated submodules alone' '
+ git reset --hard &&
+ git submodule update &&
+ git rm submod &&
+ test -d submod &&
+ test -f submod/.git &&
+ git status -s -uno > actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'rm of a populated submodule with different HEAD requires forcing' '
+ git reset --hard &&
+ git submodule update &&
+ (cd submod &&
+ git checkout HEAD^
+ ) &&
+ test_must_fail git rm submod &&
+ git status -s -uno > actual &&
+ git rm --force submod &&
+ test -s actual &&
+ test -d submod &&
+ test -f submod/.git &&
+ git status -s -uno > actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'rm of a populated submodule with modifications succeeds' '
+ git reset --hard &&
+ git submodule update &&
+ (cd submod &&
+ echo X >empty
+ ) &&
+ git rm submod &&
+ test -d submod &&
+ test -f submod/.git &&
+ git status -s -uno > actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'rm of a populated submodule with untracked files succeeds' '
+ git reset --hard &&
+ git submodule update &&
+ (cd submod &&
+ echo X >untracked
+ ) &&
+ git rm submod &&
+ test -d submod &&
+ test -f submod/.git &&
+ git status -s -uno > actual &&
+ test_cmp expect actual &&
+ rm submod/untracked
+'
+
test_done
--
1.7.11.1.105.g9f6831b
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html