Am 27.08.2012 22:59, schrieb Junio C Hamano:
> Jens Lehmann <Jens.Lehmann@xxxxxx> writes:
>> +{
>> + 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)
>> + continue; /* ignore unmerged entry */
>
> Would this cause "git rm -f path" for an unmerged submodule bypass
> the safety check?
Oops, thanks for spotting that. So replacing the "continue;" with
"pos = -pos-1;" should do the trick here, right? Will add some
tests for unmerged submodules ...
>> + ce = active_cache[pos];
>> +
>> + if (!S_ISGITLINK(ce->ce_mode) ||
>> + (lstat(ce->name, &st) < 0) ||
>> + is_empty_dir(name))
>> + continue;
>> +
>> + 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;
>> +}
>
> The call to this function comes at the very end and gives us yes/no
> for the entire set of paths. After getting this error for one
> submodule and bunch of other non-submodule paths, what is the
> procedure for the user to remove it that we want to recommend in our
> documentation? Would it go like this?
>
> $ git rm path1 path2 sub path3
> ... get the above error ...
> $ git submodule --to-gitfile sub
> $ rm -fr sub
> $ git rm sub
> ... then finally ...
> $ git rm path1 path2 path3
With current git I'd recommend:
$ git rm path1 path2 sub path3
... get the above error ...
$ rm -fr sub
... try again ...
$ git rm path1 path2 sub path3
Maybe I should add the hint to repeat the git rm after removing the
submodule to the error output?
Once we implemented "git submodule --to-gitfile" it could be used
instead of "rm -fr sub" to preserve the submodule's repo if the user
wants to.
BTW: I added the same message twice, here for the forced case and in
check_local_mod() when not forced. Is there a recommended way to assign
a localized message to a static variable, so I could define it only once
and reuse it?
>> @@ -80,8 +116,11 @@ static int check_local_mod(unsigned char *head, int index_only)
>>
>> /*
>> * Is the index different from the file in the work tree?
>> + * If it's a submodule, is its work tree modified?
>> */
>> - if (ce_match_stat(ce, &st, 0))
>> + if (ce_match_stat(ce, &st, 0) ||
>> + (S_ISGITLINK(ce->ce_mode) &&
>> + !ok_to_remove_submodule(ce->name)))
>> local_changes = 1;
>
> As noted before, because we also skip these "does it match the
> index? does it match the HEAD?" checks for unmerged paths in this
> function, a submodule that has local changes or new files is
> eligible for removal during a conflicted merge. I have a feeling
> that this should be tightened a bit; wouldn't we want to check at
> least in the checked out version (i.e. stage #2 in the index) if the
> path were a submodule, even if we are in the middle of a conflicted
> merge? After all, the top level merge shouldn't have touched the
> submodule working tree, so the local modes and new files must have
> come from the end user action that was done _before_ the conflicted
> merge started, and not expendable, no?
Right, I'll change that.
--
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