rm: absorb a submodules git dir before deletion
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>
Stefan Beller committed
Dec 27, 2016 at 11:03 UTC
55856a35b20dae2499d4e0b23551c7ba9a33baf2
2 files changed
+36
-87
builtin/rm.c
+19
-61
@@ -59,27 +59,9 @@ static void print_error_files(struct string_list *files_list,
59
}
60
}
61
62
-static void error_removing_concrete_submodules(struct string_list *files, int *errs)
63
-{
64
- print_error_files(files,
65
- Q_("the following submodule (or one of its nested "
66
- "submodules)\n"
67
- "uses a .git directory:",
68
- "the following submodules (or one of their nested "
69
- "submodules)\n"
70
- "use a .git directory:", files->nr),
71
- _("\n(use 'rm -rf' if you really want to remove "
72
- "it including all of its history)"),
73
- errs);
74
- string_list_clear(files, 0);
75
-}
76
-
77
-static int check_submodules_use_gitfiles(void)
62
+static void submodules_absorb_gitdir_if_needed(const char *prefix)
63
{
64
int i;
80
- int errs = 0;
81
- struct string_list files = STRING_LIST_INIT_NODUP;
82
-
65
for (i = 0; i < list.nr; i++) {
66
const char *name = list.entry[i].name;
67
int pos;
@@ -99,12 +81,9 @@ static int check_submodules_use_gitfiles(void)
81
continue;
82
83
if (!submodule_uses_gitfile(name))
102
- string_list_append(&files, name);
84
+ absorb_git_dir_into_superproject(prefix, name,
85
+ ABSORB_GITDIR_RECURSE_SUBMODULES);
86
}
104
-
105
- error_removing_concrete_submodules(&files, &errs);
106
-
107
- return errs;
87
}
88
89
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)
99
int errs = 0;
100
struct string_list files_staged = STRING_LIST_INIT_NODUP;
101
struct string_list files_cached = STRING_LIST_INIT_NODUP;
123
- struct string_list files_submodule = STRING_LIST_INIT_NODUP;
102
struct string_list files_local = STRING_LIST_INIT_NODUP;
103
104
no_head = is_null_oid(head);
@@ -219,13 +197,8 @@ static int check_local_mod(struct object_id *head, int index_only)
197
else if (!index_only) {
198
if (staged_changes)
199
string_list_append(&files_cached, name);
222
- if (local_changes) {
223
- if (S_ISGITLINK(ce->ce_mode) &&
224
- !submodule_uses_gitfile(name))
225
- string_list_append(&files_submodule, name);
226
- else
227
- string_list_append(&files_local, name);
228
- }
200
+ if (local_changes)
201
+ string_list_append(&files_local, name);
202
}
203
}
204
print_error_files(&files_staged,
@@ -247,8 +220,6 @@ static int check_local_mod(struct object_id *head, int index_only)
220
&errs);
221
string_list_clear(&files_cached, 0);
222
250
- error_removing_concrete_submodules(&files_submodule, &errs);
251
-
223
print_error_files(&files_local,
224
Q_("the following file has local modifications:",
225
"the following files have local modifications:",
@@ -342,6 +313,9 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
313
exit(0);
314
}
315
316
+ if (!index_only)
317
+ submodules_absorb_gitdir_if_needed(prefix);
318
+
319
/*
320
* If not forced, the file, the index and the HEAD (if exists)
321
* 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)
332
oidclr(&oid);
333
if (check_local_mod(&oid, index_only))
334
exit(1);
361
- } else if (!index_only) {
362
- if (check_submodules_use_gitfiles())
363
- exit(1);
335
}
336
337
/*
@@ -389,32 +360,20 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
360
*/
361
if (!index_only) {
362
int removed = 0, gitmodules_modified = 0;
392
- struct strbuf buf = STRBUF_INIT;
363
for (i = 0; i < list.nr; i++) {
364
const char *path = list.entry[i].name;
365
if (list.entry[i].is_submodule) {
396
- if (is_empty_dir(path)) {
397
- if (!rmdir(path)) {
398
- removed = 1;
399
- if (!remove_path_from_gitmodules(path))
400
- gitmodules_modified = 1;
401
- continue;
402
- }
403
- } else {
404
- strbuf_reset(&buf);
405
- strbuf_addstr(&buf, path);
406
- if (!remove_dir_recursively(&buf, 0)) {
407
- removed = 1;
408
- if (!remove_path_from_gitmodules(path))
409
- gitmodules_modified = 1;
410
- strbuf_release(&buf);
411
- continue;
412
- } else if (!file_exists(path))
413
- /* Submodule was removed by user */
414
- if (!remove_path_from_gitmodules(path))
415
- gitmodules_modified = 1;
416
- /* Fallthrough and let remove_path() fail. */
417
- }
366
+ struct strbuf buf = STRBUF_INIT;
367
+
368
+ strbuf_addstr(&buf, path);
369
+ if (remove_dir_recursively(&buf, 0))
370
+ die(_("could not remove '%s'"), path);
371
+ strbuf_release(&buf);
372
+
373
+ removed = 1;
374
+ if (!remove_path_from_gitmodules(path))
375
+ gitmodules_modified = 1;
376
+ continue;
377
}
378
if (!remove_path(path)) {
379
removed = 1;
@@ -423,7 +382,6 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
382
if (!removed)
383
die_errno("git rm: '%s'", path);
384
}
426
- strbuf_release(&buf);
385
if (gitmodules_modified)
386
stage_updated_gitmodules();
387
}
t/t3600-rm.sh
+17
-26
@@ -585,26 +585,22 @@ test_expect_success 'rm of a conflicted unpopulated submodule succeeds' '
585
test_cmp expect actual
586
'
587
588
-test_expect_success 'rm of a populated submodule with a .git directory fails even when forced' '
588
+test_expect_success 'rm of a populated submodule with a .git directory migrates git dir' '
589
git checkout -f master &&
590
git reset --hard &&
591
git submodule update &&
592
(cd submod &&
593
rm .git &&
594
cp -R ../.git/modules/sub .git &&
595
- GIT_WORK_TREE=. git config --unset core.worktree
595
+ GIT_WORK_TREE=. git config --unset core.worktree &&
596
+ rm -r ../.git/modules/sub
597
) &&
597
- test_must_fail git rm submod &&
598
- test -d submod &&
599
- test -d submod/.git &&
600
- git status -s -uno --ignore-submodules=none > actual &&
601
- ! test -s actual &&
602
- test_must_fail git rm -f submod &&
603
- test -d submod &&
604
- test -d submod/.git &&
605
- git status -s -uno --ignore-submodules=none > actual &&
606
- ! test -s actual &&
607
- rm -rf submod
598
+ git rm submod 2>output.err &&
599
+ ! test -d submod &&
600
+ ! test -d submod/.git &&
601
+ git status -s -uno --ignore-submodules=none >actual &&
602
+ test -s actual &&
603
+ test_i18ngrep Migrating output.err
604
'
605
606
cat >expect.deepmodified <<EOF
@@ -689,24 +685,19 @@ test_expect_success 'rm of a populated nested submodule with a nested .git direc
685
git submodule update --recursive &&
686
(cd submod/subsubmod &&
687
rm .git &&
692
- cp -R ../../.git/modules/sub/modules/sub .git &&
688
+ mv ../../.git/modules/sub/modules/sub .git &&
689
GIT_WORK_TREE=. git config --unset core.worktree
690
) &&
695
- test_must_fail git rm submod &&
696
- test -d submod &&
697
- test -d submod/subsubmod/.git &&
698
- git status -s -uno --ignore-submodules=none > actual &&
699
- ! test -s actual &&
700
- test_must_fail git rm -f submod &&
701
- test -d submod &&
702
- test -d submod/subsubmod/.git &&
703
- git status -s -uno --ignore-submodules=none > actual &&
704
- ! test -s actual &&
705
- rm -rf submod
691
+ git rm submod 2>output.err &&
692
+ ! test -d submod &&
693
+ ! test -d submod/subsubmod/.git &&
694
+ git status -s -uno --ignore-submodules=none >actual &&
695
+ test -s actual &&
696
+ test_i18ngrep Migrating output.err
697
'
698
699
test_expect_success 'checking out a commit after submodule removal needs manual updates' '
709
- git commit -m "submodule removal" submod &&
700
+ git commit -m "submodule removal" submod .gitmodules &&
701
git checkout HEAD^ &&
702
git submodule update &&
703
git checkout -q HEAD^ 2>actual &&