Jens Lehmann <Jens.Lehmann@xxxxxx> writes: > 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. Correct in principle, but the definition of "better" could be open to discussion. > It should remove the gitlink > from the index no matter if it is populated or not. Why? If you have a regular file that has changes with respect to the index entry, the index version is kept as well as the working tree version. You can "rm --cached" to remove the index entry, and you can "rm --force" to remove both, nuking the change you made to the working tree, but we do not touch such a "dirty" entry without any explicit option. > 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. You are repeating yourself without justification. > Change "git rm" so it only issues a warning if a populated submodule > cannot be removed. I find this part iffy due to the above. With --cached, perhaps, but without any option, I do not think I heard a sound justification behind this change. > 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. I think the "policy" for regular files is that "git rm $path" errors out to avoid losing information in $path. Even if the HEAD in the submodule points at the same commit as recorded in the index, if the submodule directory has other changes that (cd $path && git status) would report, we would not want to remove it, no? I am not sure if the difference between $path/.git/HEAD and :$path (the version in the index) matters. Maybe it does, maybe it doesn't. One possible sane behaviour of "git rm $path" might be: - If --force is given, remove it from the index and from the working tree (i.e. "rm -rf $path"), but use the "gitfile" facility to save $path/.git away to $GIT_DIR/modules/$name; error out if the submodule directory $path cannot be removed like this. We would probably want to remove "submodule.<name>.*" entries in .gitmodules for <name> for which "submodule.<name>.path" matches the $path. - If --cached is given, remove it from the index if the version in the index match either HEAD or the $path/.git/HEAD, without touching the working tree. This is consistent with what happens to a regular file. - If neither --force nor --cached is given, run an equivalent of (cd $path && git status), and also check if $path/.git/HEAD matches the index version. Error out if the submodule directory is dirty (again I am not sure about this part). If the submodule directory is clean, do the same as the case with --force. > While this changes behavior of "git rm", it only fixes an error where it > never worked properly. It stops an error from being issued, but I am not convinced that the new behaviour is necessarily a sensible one. A change that stops an error and performs a random operation is not necessarily a "fix". -- 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