Jens Lehmann <Jens.Lehmann@xxxxxx> writes: >> * jl/submodule-rm (2012-08-27) 1 commit >> - Teach rm to remove submodules unless they contain a git directory >> >> "git rm submodule" cannot blindly remove a submodule directory as >> its working tree may have local changes, and worse yet, it may even >> have its repository embedded in it. Teach it some special cases >> where it is safe to remove a submodule, specifically, when there is >> no local changes in the submodule working tree, and its repository >> is not embedded in its working tree but is elsewhere and uses the >> gitfile mechanism to point at it. >> >> I lost track; what is the doneness of the discussion on this patch? > > The review of v2 revealed that in case of submodule merge conflicts > the necessary checks weren't done. This (and the minor issues raised > in http://permalink.gmane.org/gmane.comp.version-control.git/204370) > is fixed in this version. Thanks. I wish all others paid attention to "What's cooking" like you did here. And if it is hard to do so for whatever reason, suggest a better way for me to publish "What's cooking" or an equivalent (I am interested in finding the least bureaucratic way to help people and keep the balls rolling). > +static int check_submodules_use_gitfiles(void) > +{ > + int i; > + int errs = 0; > + > + for (i = 0; i < list.nr; i++) { > + const char *name = list.entry[i].name; > + int pos; > + struct cache_entry *ce; > + struct stat st; > + > + pos = cache_name_pos(name, strlen(name)); > + if (pos < 0) > + pos = -pos-1; > + ce = active_cache[pos]; > + > + if (!S_ISGITLINK(ce->ce_mode) || > + (lstat(ce->name, &st) < 0) || > + is_empty_dir(name)) > + continue; If the name doesn't exist in the index (i.e. "list" has names that do not exist in the index for whatever reason), a negative pos is returned to tell you where it _would_ be inserted if you said "git add" the path. But these names in the "list" are guaranteed to exist in the index in _some_ form, so for a negative pos, (-pos-1) will have the conflicted entry at the lowest stage (typically the common ancestor's version). I am not sure checking only that one is sufficient, though. Wouldn't you want to at least check stage #2 (ours, which should most resemble the working tree)? If this were "common ancestor had it as a submodule, our side removed it and created something else, their side updated the submodule" conflict, the stage #2 would not be a gitlink (it would be a blob if that something else is a file, or may be missing if the submodule was replaced with a directory), and the path ce->name would definitely not be a submodule. > + if (!submodule_uses_gitfile(name)) > + errs = error(_("submodule '%s' (or one of its nested " > + "submodules) uses a .git directory\n" > + "(use 'rm -rf' if you really want to remove " > + "it including all of its history)"), name); > + } > + > + return errs; > +} > + > static int check_local_mod(unsigned char *head, int index_only) > { > /* > @@ -37,15 +72,23 @@ 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; > int staged_changes = 0; > > pos = cache_name_pos(name, strlen(name)); > - if (pos < 0) > - continue; /* removing unmerged entry */ > + if (pos < 0) { > + /* > + * Skip unmerged entries except for populated submodules > + * that could loose history when removed. s/loose/lose/ > + */ > + pos = -pos-1; > + if (!S_ISGITLINK(active_cache[pos]->ce_mode) || > + is_empty_dir(name)) > + continue; > + } Lilewise. It may make sense to introduce a helper function to tell if it is a submodule on our side by checking only the stage #2 entry when you see a nagetive pos returned from cache_name_pos() and call it "is_ours_submodule?()" or something. -- 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