Jens Lehmann <jens.lehm...@web.de> 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to