rm: delete .gitmodules entry of submodules removed from the work tree

Currently using "git rm" on a submodule removes the submodule's work tree from that of the superproject and the gitlink from the index. But the submodule's section in .gitmodules is left untouched, which is a leftover of the now removed submodule and might irritate users (as opposed to the setting in .git/config, this must stay as a reminder that the user showed interest in this submodule so it will be repopulated later when an older commit is checked out). Let "git rm" help the user by not only removing the submodule from the work tree but by also removing the "submodule.<submodule name>" section from the .gitmodules file and stage both. This doesn't happen when the "--cached" option is used, as it would modify the work tree. This also silently does nothing when no .gitmodules file is found and only issues a warning when it doesn't have a section for this submodule. This is because the user might just use plain gitlinks without the .gitmodules file or has already removed the section by hand before issuing the "git rm" command (in which case the warning reminds him that rm would have done that for him). Only when .gitmodules is found and contains merge conflicts the rm command will fail and tell the user to resolve the conflict before trying again. Also extend the man page to inform the user about this new feature. While at it promote the submodule sub-section to a chapter as it made not much sense under "REMOVING FILES THAT HAVE DISAPPEARED FROM THE FILESYSTEM". In t7610 three uses of "git rm submod" had to be replaced with "git rm --cached submod" because that test expects .gitmodules and the work tree to stay untouched. Also in t7400 the tests for the remaining settings in the .gitmodules file had to be changed to assert that these settings are missing. Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jens Lehmann committed Aug 6, 2013 at 21:15 UTC 95c16418f0375e2fc325f32c3d7578fba9cfd7ef
7 files changed +154 -25
Documentation/git-rm.txt
+5 -3
@@ -134,14 +134,16 @@ use the following command:
134 git diff --name-only --diff-filter=D -z | xargs -0 git rm --cached
135 ----------------
136
137 -Submodules
138 -~~~~~~~~~~
137 +SUBMODULES
138 +----------
139 Only submodules using a gitfile (which means they were cloned
140 with a Git version 1.7.8 or newer) will be removed from the work
141 tree, as their repository lives inside the .git directory of the
142 superproject. If a submodule (or one of those nested inside it)
143 still uses a .git directory, `git rm` will fail - no matter if forced
144 -or not - to protect the submodule's history.
144 +or not - to protect the submodule's history. If it exists the
145 +submodule.<name> section in the linkgit:gitmodules[5] file will also
146 +be removed and that file will be staged (unless --cached or -n are used).
147
148 A submodule is considered up-to-date when the HEAD is the same as
149 recorded in the index, no tracked files are modified and no untracked
builtin/rm.c
+16 -3
@@ -283,6 +283,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
283 struct pathspec pathspec;
284 char *seen;
285
286 + gitmodules_config();
287 git_config(git_default_config, NULL);
288
289 argc = parse_options(argc, argv, prefix, builtin_rm_options,
@@ -324,7 +325,10 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
325 continue;
326 ALLOC_GROW(list.entry, list.nr + 1, list.alloc);
327 list.entry[list.nr].name = ce->name;
327 - list.entry[list.nr++].is_submodule = S_ISGITLINK(ce->ce_mode);
328 + list.entry[list.nr].is_submodule = S_ISGITLINK(ce->ce_mode);
329 + if (list.entry[list.nr++].is_submodule &&
330 + !is_staging_gitmodules_ok())
331 + die (_("Please, stage your changes to .gitmodules or stash them to proceed"));
332 }
333
334 if (pathspec.nr) {
@@ -396,13 +400,15 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
400 * in the middle)
401 */
402 if (!index_only) {
399 - int removed = 0;
403 + int removed = 0, gitmodules_modified = 0;
404 for (i = 0; i < list.nr; i++) {
405 const char *path = list.entry[i].name;
406 if (list.entry[i].is_submodule) {
407 if (is_empty_dir(path)) {
408 if (!rmdir(path)) {
409 removed = 1;
410 + if (!remove_path_from_gitmodules(path))
411 + gitmodules_modified = 1;
412 continue;
413 }
414 } else {
@@ -410,9 +416,14 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
416 strbuf_addstr(&buf, path);
417 if (!remove_dir_recursively(&buf, 0)) {
418 removed = 1;
419 + if (!remove_path_from_gitmodules(path))
420 + gitmodules_modified = 1;
421 strbuf_release(&buf);
422 continue;
415 - }
423 + } else if (!file_exists(path))
424 + /* Submodule was removed by user */
425 + if (!remove_path_from_gitmodules(path))
426 + gitmodules_modified = 1;
427 strbuf_release(&buf);
428 /* Fallthrough and let remove_path() fail. */
429 }
@@ -424,6 +435,8 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
435 if (!removed)
436 die_errno("git rm: '%s'", path);
437 }
438 + if (gitmodules_modified)
439 + stage_updated_gitmodules();
440 }
441
442 if (active_cache_changed) {
submodule.c
+33
@@ -81,6 +81,39 @@ int update_path_in_gitmodules(const char *oldpath, const char *newpath)
81 return 0;
82 }
83
84 +/*
85 + * Try to remove the "submodule.<name>" section from .gitmodules where the given
86 + * path is configured. Return 0 only if a .gitmodules file was found, a section
87 + * with the correct path=<path> setting was found and we could remove it.
88 + */
89 +int remove_path_from_gitmodules(const char *path)
90 +{
91 + struct strbuf sect = STRBUF_INIT;
92 + struct string_list_item *path_option;
93 +
94 + if (!file_exists(".gitmodules")) /* Do nothing without .gitmodules */
95 + return -1;
96 +
97 + if (gitmodules_is_unmerged)
98 + die(_("Cannot change unmerged .gitmodules, resolve merge conflicts first"));
99 +
100 + path_option = unsorted_string_list_lookup(&config_name_for_path, path);
101 + if (!path_option) {
102 + warning(_("Could not find section in .gitmodules where path=%s"), path);
103 + return -1;
104 + }
105 + strbuf_addstr(&sect, "submodule.");
106 + strbuf_addstr(&sect, path_option->util);
107 + if (git_config_rename_section_in_file(".gitmodules", sect.buf, NULL) < 0) {
108 + /* Maybe the user already did that, don't error out here */
109 + warning(_("Could not remove .gitmodules entry for %s"), path);
110 + strbuf_release(&sect);
111 + return -1;
112 + }
113 + strbuf_release(&sect);
114 + return 0;
115 +}
116 +
117 void stage_updated_gitmodules(void)
118 {
119 struct strbuf buf = STRBUF_INIT;
submodule.h
+1
@@ -13,6 +13,7 @@ enum {
13
14 int is_staging_gitmodules_ok(void);
15 int update_path_in_gitmodules(const char *oldpath, const char *newpath);
16 +int remove_path_from_gitmodules(const char *path);
17 void stage_updated_gitmodules(void);
18 void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,
19 const char *path);
t/t3600-rm.sh
+92 -6
@@ -263,6 +263,7 @@ test_expect_success 'rm removes subdirectories recursively' '
263 '
264
265 cat >expect <<EOF
266 +M .gitmodules
267 D submod
268 EOF
269
@@ -270,6 +271,15 @@ cat >expect.modified <<EOF
271 M submod
272 EOF
273
274 +cat >expect.cached <<EOF
275 +D submod
276 +EOF
277 +
278 +cat >expect.both_deleted<<EOF
279 +D .gitmodules
280 +D submod
281 +EOF
282 +
283 test_expect_success 'rm removes empty submodules from work tree' '
284 mkdir submod &&
285 git update-index --add --cacheinfo 160000 $(git rev-parse HEAD) submod &&
@@ -281,16 +291,20 @@ test_expect_success 'rm removes empty submodules from work tree' '
291 git rm submod &&
292 test ! -e submod &&
293 git status -s -uno --ignore-submodules=none > actual &&
284 - test_cmp expect actual
294 + test_cmp expect actual &&
295 + test_must_fail git config -f .gitmodules submodule.sub.url &&
296 + test_must_fail git config -f .gitmodules submodule.sub.path
297 '
298
287 -test_expect_success 'rm removes removed submodule from index' '
299 +test_expect_success 'rm removes removed submodule from index and .gitmodules' '
300 git reset --hard &&
301 git submodule update &&
302 rm -rf submod &&
303 git rm submod &&
304 git status -s -uno --ignore-submodules=none > actual &&
293 - test_cmp expect actual
305 + test_cmp expect actual &&
306 + test_must_fail git config -f .gitmodules submodule.sub.url &&
307 + test_must_fail git config -f .gitmodules submodule.sub.path
308 '
309
310 test_expect_success 'rm removes work tree of unmodified submodules' '
@@ -299,7 +313,9 @@ test_expect_success 'rm removes work tree of unmodified submodules' '
313 git rm submod &&
314 test ! -d submod &&
315 git status -s -uno --ignore-submodules=none > actual &&
302 - test_cmp expect actual
316 + test_cmp expect actual &&
317 + test_must_fail git config -f .gitmodules submodule.sub.url &&
318 + test_must_fail git config -f .gitmodules submodule.sub.path
319 '
320
321 test_expect_success 'rm removes a submodule with a trailing /' '
@@ -333,6 +349,72 @@ test_expect_success 'rm of a populated submodule with different HEAD fails unles
349 git rm -f submod &&
350 test ! -d submod &&
351 git status -s -uno --ignore-submodules=none > actual &&
352 + test_cmp expect actual &&
353 + test_must_fail git config -f .gitmodules submodule.sub.url &&
354 + test_must_fail git config -f .gitmodules submodule.sub.path
355 +'
356 +
357 +test_expect_success 'rm --cached leaves work tree of populated submodules and .gitmodules alone' '
358 + git reset --hard &&
359 + git submodule update &&
360 + git rm --cached submod &&
361 + test -d submod &&
362 + test -f submod/.git &&
363 + git status -s -uno >actual &&
364 + test_cmp expect.cached actual &&
365 + git config -f .gitmodules submodule.sub.url &&
366 + git config -f .gitmodules submodule.sub.path
367 +'
368 +
369 +test_expect_success 'rm --dry-run does not touch the submodule or .gitmodules' '
370 + git reset --hard &&
371 + git submodule update &&
372 + git rm -n submod &&
373 + test -f submod/.git &&
374 + git diff-index --exit-code HEAD
375 +'
376 +
377 +test_expect_success 'rm does not complain when no .gitmodules file is found' '
378 + git reset --hard &&
379 + git submodule update &&
380 + git rm .gitmodules &&
381 + git rm submod >actual 2>actual.err &&
382 + ! test -s actual.err &&
383 + ! test -d submod &&
384 + ! test -f submod/.git &&
385 + git status -s -uno >actual &&
386 + test_cmp expect.both_deleted actual
387 +'
388 +
389 +test_expect_success 'rm will error out on a modified .gitmodules file unless staged' '
390 + git reset --hard &&
391 + git submodule update &&
392 + git config -f .gitmodules foo.bar true &&
393 + test_must_fail git rm submod >actual 2>actual.err &&
394 + test -s actual.err &&
395 + test -d submod &&
396 + test -f submod/.git &&
397 + git diff-files --quiet -- submod &&
398 + git add .gitmodules &&
399 + git rm submod >actual 2>actual.err &&
400 + ! test -s actual.err &&
401 + ! test -d submod &&
402 + ! test -f submod/.git &&
403 + git status -s -uno >actual &&
404 + test_cmp expect actual
405 +'
406 +
407 +test_expect_success 'rm issues a warning when section is not found in .gitmodules' '
408 + git reset --hard &&
409 + git submodule update &&
410 + git config -f .gitmodules --remove-section submodule.sub &&
411 + git add .gitmodules &&
412 + echo "warning: Could not find section in .gitmodules where path=submod" >expect.err &&
413 + git rm submod >actual 2>actual.err &&
414 + test_i18ncmp expect.err actual.err &&
415 + ! test -d submod &&
416 + ! test -f submod/.git &&
417 + git status -s -uno >actual &&
418 test_cmp expect actual
419 '
420
@@ -427,7 +509,9 @@ test_expect_success 'rm of a conflicted populated submodule with different HEAD
509 git rm -f submod &&
510 test ! -d submod &&
511 git status -s -uno --ignore-submodules=none > actual &&
430 - test_cmp expect actual
512 + test_cmp expect actual &&
513 + test_must_fail git config -f .gitmodules submodule.sub.url &&
514 + test_must_fail git config -f .gitmodules submodule.sub.path
515 '
516
517 test_expect_success 'rm of a conflicted populated submodule with modifications fails unless forced' '
@@ -446,7 +530,9 @@ test_expect_success 'rm of a conflicted populated submodule with modifications f
530 git rm -f submod &&
531 test ! -d submod &&
532 git status -s -uno --ignore-submodules=none > actual &&
449 - test_cmp expect actual
533 + test_cmp expect actual &&
534 + test_must_fail git config -f .gitmodules submodule.sub.url &&
535 + test_must_fail git config -f .gitmodules submodule.sub.path
536 '
537
538 test_expect_success 'rm of a conflicted populated submodule with untracked files fails unless forced' '
t/t7400-submodule-basic.sh
+4 -10
@@ -773,13 +773,11 @@ test_expect_success 'submodule add --name allows to replace a submodule with ano
773 test_cmp expect .git
774 ) &&
775 echo "repo" >expect &&
776 - git config -f .gitmodules submodule.repo.path >actual &&
777 - test_cmp expect actual &&
776 + test_must_fail git config -f .gitmodules submodule.repo.path &&
777 git config -f .gitmodules submodule.repo_new.path >actual &&
778 test_cmp expect actual&&
779 echo "$submodurl/repo" >expect &&
781 - git config -f .gitmodules submodule.repo.url >actual &&
782 - test_cmp expect actual &&
780 + test_must_fail git config -f .gitmodules submodule.repo.url &&
781 echo "$submodurl/bare.git" >expect &&
782 git config -f .gitmodules submodule.repo_new.url >actual &&
783 test_cmp expect actual &&
@@ -799,12 +797,8 @@ test_expect_success 'submodule add with an existing name fails unless forced' '
797 git rm repo &&
798 test_must_fail git submodule add -q --name repo_new "$submodurl/repo.git" repo &&
799 test ! -d repo &&
802 - echo "repo" >expect &&
803 - git config -f .gitmodules submodule.repo_new.path >actual &&
804 - test_cmp expect actual&&
805 - echo "$submodurl/bare.git" >expect &&
806 - git config -f .gitmodules submodule.repo_new.url >actual &&
807 - test_cmp expect actual &&
800 + test_must_fail git config -f .gitmodules submodule.repo_new.path &&
801 + test_must_fail git config -f .gitmodules submodule.repo_new.url &&
802 echo "$submodurl/bare.git" >expect &&
803 git config submodule.repo_new.url >actual &&
804 test_cmp expect actual &&
t/t7610-mergetool.sh
+3 -3
@@ -253,7 +253,7 @@ test_expect_success 'deleted vs modified submodule' '
253 git checkout -b test6 branch1 &&
254 git submodule update -N &&
255 mv submod submod-movedaside &&
256 - git rm submod &&
256 + git rm --cached submod &&
257 git commit -m "Submodule deleted from branch" &&
258 git checkout -b test6.a test6 &&
259 test_must_fail git merge master &&
@@ -322,7 +322,7 @@ test_expect_success 'file vs modified submodule' '
322 git checkout -b test7 branch1 &&
323 git submodule update -N &&
324 mv submod submod-movedaside &&
325 - git rm submod &&
325 + git rm --cached submod &&
326 echo not a submodule >submod &&
327 git add submod &&
328 git commit -m "Submodule path becomes file" &&
@@ -453,7 +453,7 @@ test_expect_success 'submodule in subdirectory' '
453 test_expect_success 'directory vs modified submodule' '
454 git checkout -b test11 branch1 &&
455 mv submod submod-movedaside &&
456 - git rm submod &&
456 + git rm --cached submod &&
457 mkdir submod &&
458 echo not a submodule >submod/file16 &&
459 git add submod/file16 &&