Subject: Re: [PATCH 2/2] git-submodule: support 'rm' command.

From: Jens Lehmann <[email-masked]>
Date: Tue, 26 Jun 2012 21:59:16 +0200

Am 26.06.2012 21:12, schrieb Phil Hord:
> On Mon, Jun 25, 2012 at 5:09 PM, Jens Lehmann <[email-masked]> wrote:
>> Am 25.06.2012 22:53, schrieb Phil Hord:
>>> On Mon, Jun 25, 2012 at 12:58 PM, Jens Lehmann <[email-masked]> wrote:
>>>> Am 25.06.2012 12:57, schrieb Michał Górny:
>>>>> Add an 'rm' command to git-submodule which provides means to
>>>>> (semi-)easily remove git submodules.
>>>>>
>>>>> Signed-off-by: Michał Górny <[email-masked]>
>>>>> ---
>>>>> Right now, it requires the submodule checkout to be removed manually
>>>>> first (so it does not remove unstaged commits), and just removes
>>>>> the index entry and module information from config.
>>>>>
>>>>> I based it on 'cmd_add' code trying to preserve the original coding
>>>>> standards.
>>>>
>>>> I really like the goal of this patch but would prefer that "git rm"
>>>> learns how to remove submodules instead of adding more code to the
>>>> git-submodule.sh script.
>>>
>>> I would like to see both supported, eventually. That is, git-rm and
>>> git-submodule-rm should both work.  It would make sense to me when I
>>> am looking for the counterpart to 'git submodule add' to find it under
>>> 'git submodule rm', and also under 'git submodule --help'.
>>
>> Hmm, as long as "git submodule rm" would just use "git rm" under the
>> hood and not its own scripting that would be ok.
> 
> Maybe it would be better if 'git-rm' would use 'git submodule rm'
> under the covers.  This would keep the .gitmodules (etc.)
> manipulations out of the hair of the git-rm machinery.

I disagree, me thinks submodules should become first class citizens.

> Also, I hope 'git submodule rm foo' would fail if 'foo' were not a submodule.

Yes, it should. But that'd be easy to test there.

>>> In the special case of a submodule which does not use a gitfile, I am
>>> not even sure if any of the submodule files should be removed. If they
>>> are, what state does that leave the submodule repository in?  A
>>> checked-out workdir whose files are all removed?  'git-status' would
>>> be very noisy in this case.  I'd rather expect this to behave the same
>>> as if I checked out a previous commit which did not have the submodule
>>> added yet.  Today, this leaves the submodule in-place and it shows up
>>> as an untracked file.  I don't know a better way to handle that,
>>> though I expect it would be ok remove all the files even in this case
>>> (if the workdir is not dirty and if the head commit is current in the
>>> superproject).  But it seems extreme to do all of that and then leave
>>> the .git directory lying about in the former submodule directory.
>>
>> Good point. Another option would be to move the git directory into
>> .git/modules of the superproject before removing the files, then next
>> time it's updated it'll use gitfile. But maybe that's a problem which
>> will go away anyways as all submodules cloned with newer git use
>> gitfiles anyway.
> 
> I like this idea, but it seems a little presumptuous.  The new
> behavior might cause a few panicked users to spend the day rebuilding
> their "lost" repository.

Me thinks we should teach "git rm" only to remove the submodule when
the --recurse-submodules option is used with it (which is what "git
submodule rm" would do). Then later the to be added "autoupdate"
submodule config  setting (which I intend to use for automatic
submodule updates during checkout, merge, etc. too) could enable this.
No surprises for users who didn't ask for it.

>  Maybe we can make this an explicit action.
> "git submodule convert-to-gitfile"  :-)

I like it!