git.vger.kernel.org archive mirror
 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: Mon, 25 Jun 2012 23:09:20 +0200	[thread overview]
Message-ID: <4FE8D380.20803@web.de> (raw)
In-Reply-To: <CABURp0od-nNFVhLQU9BsiJ=wXkdneJfhxun_PHOfV=sgzOFShg@mail.gmail.com>

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.

>> Also it shouldn't be necessary for the user to remove the directory
>> by hand before running "git rm". At least all files recorded in the
>> submodule can be removed (and if the submodule uses a gitfile that
>> can be removed too). Then all that is left are untracked files the
>> user has to decide what to do with (which might be removed too when
>> running "git rm --recurse-submodules=untracked").
> 
> That sounds like a nice next step.  But I would expect that a 'git
> [submodule] rm foo' where foo has uncommitted changes to complain to
> me (and do nothing else) unless I used --force.  This is similar to
> how git-rm already behaves, I think.  And in the case of a dirty
> submodule it makes sense to treat the submodule files as an atomic
> unit.  That is, if any of the submodule files are dirty and git-rm
> will "leave" them, then it should leave the whole submodule.  It would
> be very inconvenient to have to restore files back into place at the
> correct commit just so I could examine them in context to determine
> what I should have done with them before I used git-rm.

We absolutely agree here, this is pretty much what I wrote further
down in my first response ;-)
(except additionally I consider a submodule dirty if it's HEAD isn't
on any branch to avoid loosing commits)

> 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.

  reply	other threads:[~2012-06-25 21:09 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 [this message]
2012-06-26 19:12         ` Phil Hord
2012-06-26 19:59           ` Jens Lehmann
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=4FE8D380.20803@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).