All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jens Lehmann <Jens.Lehmann@web.de>
To: Phil Hord <phil.hord@gmail.com>
Cc: "Michał Górny" <mgorny@gentoo.org>, git@vger.kernel.org
Subject: Re: [PATCH 2/2] git-submodule: support 'rm' command.
Date: Tue, 26 Jun 2012 21:59:16 +0200	[thread overview]
Message-ID: <4FEA1494.404@web.de> (raw)
In-Reply-To: <CABURp0qFXGs6wqFbz28OKywVsFu23JKfhS8uLsen-nqhBvDAiw@mail.gmail.com>

Am 26.06.2012 21:12, schrieb Phil Hord:
> On Mon, Jun 25, 2012 at 5:09 PM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
>> Am 25.06.2012 22:53, schrieb Phil Hord:
>>> On Mon, Jun 25, 2012 at 12:58 PM, Jens Lehmann <Jens.Lehmann@web.de> 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 <mgorny@gentoo.org>
>>>>> ---
>>>>> 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!

  reply	other threads:[~2012-06-26 19:59 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-06-25 10:56 [PATCH 1/2] git-submodule.sh: fix filename in comment Michał Górny
2012-06-25 10:57 ` [PATCH 2/2] git-submodule: support 'rm' command Michał Górny
2012-06-25 16:58   ` Jens Lehmann
2012-06-25 20:53     ` Phil Hord
2012-06-25 21:09       ` Jens Lehmann
2012-06-26 19:12         ` Phil Hord
2012-06-26 19:59           ` Jens Lehmann [this message]
2012-06-27 18:48             ` Phil Hord
2012-06-28 20:31     ` Michał Górny

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4FEA1494.404@web.de \
    --to=jens.lehmann@web.de \
    --cc=git@vger.kernel.org \
    --cc=mgorny@gentoo.org \
    --cc=phil.hord@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.