undef/git-commit-vandalism
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

rm: absorb a submodules git dir before deletion

Browse Source 
When deleting a submodule, we need to keep the actual git directory around,
such that we do not lose local changes in there and at a later checkout
of the submodule we don't need to clone it again.

Now that the functionality is available to absorb the git directory of a
submodule, rewrite the checking in git-rm to not complain, but rather
relocate the git directories inside the superproject.

An alternative solution was discussed to have a function
`depopulate_submodule`. That would couple the check for its git directory
and possible relocation before the the removal, such that it is less
likely to miss the check in the future.  But the indirection with such
a function added seemed also complex. The reason for that was that this
possible move of the git directory was also implemented in
`ok_to_remove_submodule`, such that this function could truthfully
answer whether it is ok to remove the submodule.

The solution proposed here defers all these checks to the caller.

Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
Stefan Beller 2016-12-27 11:03:14 -08:00 committed by Junio C Hamano
parent 83b7696605
commit 55856a35b2
2 changed files with 36 additions and 87 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

80
builtin/rm.c
View File

@ -59,27 +59,9 @@ static void print_error_files(struct string_list *files_list,
} }
} }
static void error_removing_concrete_submodules(struct string_list *files, int *errs) static void submodules_absorb_gitdir_if_needed(const char *prefix)
{
print_error_files(files,
Q_("the following submodule (or one of its nested "
"submodules)\n"
"uses a .git directory:",
"the following submodules (or one of their nested "
"submodules)\n"
"use a .git directory:", files->nr),
_("\n(use 'rm -rf' if you really want to remove "
"it including all of its history)"),
errs);
string_list_clear(files, 0);
}
static int check_submodules_use_gitfiles(void)
{ {
int i; int i;
int errs = 0;
struct string_list files = STRING_LIST_INIT_NODUP;
for (i = 0; i < list.nr; i++) { for (i = 0; i < list.nr; i++) {
const char *name = list.entry[i].name; const char *name = list.entry[i].name;
int pos; int pos;
@ -99,12 +81,9 @@ static int check_submodules_use_gitfiles(void)
continue; continue;
if (!submodule_uses_gitfile(name)) if (!submodule_uses_gitfile(name))
string_list_append(&files, name); absorb_git_dir_into_superproject(prefix, name,
ABSORB_GITDIR_RECURSE_SUBMODULES);
} }
error_removing_concrete_submodules(&files, &errs);
return errs;
} }
static int check_local_mod(struct object_id *head, int index_only) static int check_local_mod(struct object_id *head, int index_only)
@ -120,7 +99,6 @@ static int check_local_mod(struct object_id *head, int index_only)
int errs = 0; int errs = 0;
struct string_list files_staged = STRING_LIST_INIT_NODUP; struct string_list files_staged = STRING_LIST_INIT_NODUP;
struct string_list files_cached = STRING_LIST_INIT_NODUP; struct string_list files_cached = STRING_LIST_INIT_NODUP;
struct string_list files_submodule = STRING_LIST_INIT_NODUP;
struct string_list files_local = STRING_LIST_INIT_NODUP; struct string_list files_local = STRING_LIST_INIT_NODUP;
no_head = is_null_oid(head); no_head = is_null_oid(head);
@ -219,13 +197,8 @@ static int check_local_mod(struct object_id *head, int index_only)
else if (!index_only) { else if (!index_only) {
if (staged_changes) if (staged_changes)
string_list_append(&files_cached, name); string_list_append(&files_cached, name);
if (local_changes) { if (local_changes)
if (S_ISGITLINK(ce->ce_mode) && string_list_append(&files_local, name);
!submodule_uses_gitfile(name))
string_list_append(&files_submodule, name);
else
string_list_append(&files_local, name);
}
} }
} }
print_error_files(&files_staged, print_error_files(&files_staged,
@ -247,8 +220,6 @@ static int check_local_mod(struct object_id *head, int index_only)
&errs); &errs);
string_list_clear(&files_cached, 0); string_list_clear(&files_cached, 0);
error_removing_concrete_submodules(&files_submodule, &errs);
print_error_files(&files_local, print_error_files(&files_local,
Q_("the following file has local modifications:", Q_("the following file has local modifications:",
"the following files have local modifications:", "the following files have local modifications:",
@ -342,6 +313,9 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
exit(0); exit(0);
} }
if (!index_only)
submodules_absorb_gitdir_if_needed(prefix);
/* /*
* If not forced, the file, the index and the HEAD (if exists) * If not forced, the file, the index and the HEAD (if exists)
* must match; but the file can already been removed, since * must match; but the file can already been removed, since
@ -358,9 +332,6 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
oidclr(&oid); oidclr(&oid);
if (check_local_mod(&oid, index_only)) if (check_local_mod(&oid, index_only))
exit(1); exit(1);
} else if (!index_only) {
if (check_submodules_use_gitfiles())
exit(1);
} }
/* /*
@ -389,32 +360,20 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
*/ */
if (!index_only) { if (!index_only) {
int removed = 0, gitmodules_modified = 0; int removed = 0, gitmodules_modified = 0;
struct strbuf buf = STRBUF_INIT;
for (i = 0; i < list.nr; i++) { for (i = 0; i < list.nr; i++) {
const char *path = list.entry[i].name; const char *path = list.entry[i].name;
if (list.entry[i].is_submodule) { if (list.entry[i].is_submodule) {
if (is_empty_dir(path)) { struct strbuf buf = STRBUF_INIT;
if (!rmdir(path)) {
removed = 1; strbuf_addstr(&buf, path);
if (!remove_path_from_gitmodules(path)) if (remove_dir_recursively(&buf, 0))
gitmodules_modified = 1; die(_("could not remove '%s'"), path);
continue; strbuf_release(&buf);
}
} else { removed = 1;
strbuf_reset(&buf); if (!remove_path_from_gitmodules(path))
strbuf_addstr(&buf, path); gitmodules_modified = 1;
if (!remove_dir_recursively(&buf, 0)) { continue;
removed = 1;
if (!remove_path_from_gitmodules(path))
gitmodules_modified = 1;
strbuf_release(&buf);
continue;
} else if (!file_exists(path))
/* Submodule was removed by user */
if (!remove_path_from_gitmodules(path))
gitmodules_modified = 1;
/* Fallthrough and let remove_path() fail. */
}
} }
if (!remove_path(path)) { if (!remove_path(path)) {
removed = 1; removed = 1;
@ -423,7 +382,6 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
if (!removed) if (!removed)
die_errno("git rm: '%s'", path); die_errno("git rm: '%s'", path);
} }
strbuf_release(&buf);
if (gitmodules_modified) if (gitmodules_modified)
stage_updated_gitmodules(); stage_updated_gitmodules();
} }

43
t/t3600-rm.sh
View File

@ -585,26 +585,22 @@ test_expect_success 'rm of a conflicted unpopulated submodule succeeds' '
test_cmp expect actual test_cmp expect actual
' '
test_expect_success 'rm of a populated submodule with a .git directory fails even when forced' ' test_expect_success 'rm of a populated submodule with a .git directory migrates git dir' '
git checkout -f master && git checkout -f master &&
git reset --hard && git reset --hard &&
git submodule update && git submodule update &&
(cd submod && (cd submod &&
rm .git && rm .git &&
cp -R ../.git/modules/sub .git && cp -R ../.git/modules/sub .git &&
GIT_WORK_TREE=. git config --unset core.worktree GIT_WORK_TREE=. git config --unset core.worktree &&
rm -r ../.git/modules/sub
) && ) &&
test_must_fail git rm submod && git rm submod 2>output.err &&
test -d submod && ! test -d submod &&
test -d submod/.git && ! test -d submod/.git &&
git status -s -uno --ignore-submodules=none > actual && git status -s -uno --ignore-submodules=none >actual &&
! test -s actual && test -s actual &&
test_must_fail git rm -f submod && test_i18ngrep Migrating output.err
test -d submod &&
test -d submod/.git &&
git status -s -uno --ignore-submodules=none > actual &&
! test -s actual &&
rm -rf submod
' '
cat >expect.deepmodified <<EOF cat >expect.deepmodified <<EOF
@ -689,24 +685,19 @@ test_expect_success 'rm of a populated nested submodule with a nested .git direc
git submodule update --recursive && git submodule update --recursive &&
(cd submod/subsubmod && (cd submod/subsubmod &&
rm .git && rm .git &&
cp -R ../../.git/modules/sub/modules/sub .git && mv ../../.git/modules/sub/modules/sub .git &&
GIT_WORK_TREE=. git config --unset core.worktree GIT_WORK_TREE=. git config --unset core.worktree
) && ) &&
test_must_fail git rm submod && git rm submod 2>output.err &&
test -d submod && ! test -d submod &&
test -d submod/subsubmod/.git && ! test -d submod/subsubmod/.git &&
git status -s -uno --ignore-submodules=none > actual && git status -s -uno --ignore-submodules=none >actual &&
! test -s actual && test -s actual &&
test_must_fail git rm -f submod && test_i18ngrep Migrating output.err
test -d submod &&
test -d submod/subsubmod/.git &&
git status -s -uno --ignore-submodules=none > actual &&
! test -s actual &&
rm -rf submod
' '
test_expect_success 'checking out a commit after submodule removal needs manual updates' ' test_expect_success 'checking out a commit after submodule removal needs manual updates' '
git commit -m "submodule removal" submod && git commit -m "submodule removal" submod .gitmodules &&
git checkout HEAD^ && git checkout HEAD^ &&
git submodule update && git submodule update &&
git checkout -q HEAD^ 2>actual && git checkout -q HEAD^ 2>actual &&