Jens Lehmann <[email protected]> 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
