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