Subject: Re: [PATCH] Add rm to submodule command.

From: Jens Lehmann <[email-masked]>
Date: Sun, 04 Nov 2012 15:29:02 +0100

Am 04.11.2012 14:43, schrieb Jeff King:
> On Fri, Nov 02, 2012 at 10:26:11AM -0700, Alex Linden Levy wrote:
> 
>> This change removes the config entries in .gitmodules and adds it.
>> ---
> 
> Signoff?
> 
>>  git-submodule.sh | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 61 insertions(+), 1 deletion(-)
> 
> No documentation or tests?
> 
>> diff --git a/git-submodule.sh b/git-submodule.sh
>> index ab6b110..29d950f 100755
>> --- a/git-submodule.sh
>> +++ b/git-submodule.sh
> 
> I'd defer to submodule experts on whether the steps to 'rm' the
> submodule make sense. Jens?

Hmm, this change adds the --quiet and --branch options to rm
which aren't used (and at least --branch makes no sense to me
here). Remainders of copy & paste? It also only affects the
.gitmodules setting and leaves the submodules work tree alone,
while I think it should - at least optionally - remove the work
tree just like "git rm" removes files too (of course only if
there is no .git directory found in it and no modifications are
present, as that would possibly lose data).

Me also thinks such a command should use my recent rm submodule
work to remove the work tree (found in your current master and
Junio's next branch), which does all necessary checks before it
removes the work tree together with the index entry. This could
be tweaked via a --cached option or such if the user wants to
keep the work tree.

But apart from those issues I'm not convinced that adding a
"git submodule rm" command is the best option. While working on
teaching "git mv" to move submodules I came to the conclusion
that it might be a better solution to let "git rm" remove the
submodule entry from the .gitmodules file itself (but of course
only if that file is found and contains such an entry, if not
it will silently do nothing to not disturb submodule users who
don't have a .gitmodules file and are using plain gitlinks).

The reason is that git core commands like status, diff and
fetch already use the path -> name mapping found in .gitmodules
and will behave strange when this is out of sync with the work
tree. So I strongly believe that doing a "git mv" should change
the path -> name mapping in .gitmodules too while moving the
submodule's work tree and updating the index (of course again
only if .gitmodules is found and contains such an entry, if not
it'll just move the work tree and update the index). Then we
won't need a new "git submodule mv" command as everything is
done inside the mv command. And for consistency I think "git rm"
should also remove the path -> name mapping (even though that is
not required for a rm to do its job, as no one will use that
setting later when the submodule is gone from the index). Then
we won't need a new "git submodule rm" at all.

Does that make sense?