git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] git-submodule.sh: fix filename in comment.
@ 2012-06-25 10:56 Michał Górny
  2012-06-25 10:57 ` [PATCH 2/2] git-submodule: support 'rm' command Michał Górny
  0 siblings, 1 reply; 9+ messages in thread
From: Michał Górny @ 2012-06-25 10:56 UTC (permalink / raw)
  To: git; +Cc: Michał Górny

Signed-off-by: Michał Górny <mgorny@gentoo.org>
---
 git-submodule.sh |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 5c61ae2..fbf2faf 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -1,6 +1,6 @@
 #!/bin/sh
 #
-# git-submodules.sh: add, init, update or list git submodules
+# git-submodule.sh: add, init, update or list git submodules
 #
 # Copyright (c) 2007 Lars Hjemli
 
-- 
1.7.10.2

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/2] git-submodule: support 'rm' command.
  2012-06-25 10:56 [PATCH 1/2] git-submodule.sh: fix filename in comment Michał Górny
@ 2012-06-25 10:57 ` Michał Górny
  2012-06-25 16:58   ` Jens Lehmann
  0 siblings, 1 reply; 9+ messages in thread
From: Michał Górny @ 2012-06-25 10:57 UTC (permalink / raw)
  To: git; +Cc: 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.

 Documentation/git-submodule.txt |   12 +++++++
 git-submodule.sh                |   68 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 79 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index fbbbcb2..293c1bf 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -11,6 +11,7 @@ SYNOPSIS
 [verse]
 'git submodule' [--quiet] add [-b branch] [-f|--force]
 	      [--reference <repository>] [--] <repository> [<path>]
+'git submodule' [--quiet] rm <path>...
 'git submodule' [--quiet] status [--cached] [--recursive] [--] [<path>...]
 'git submodule' [--quiet] init [--] [<path>...]
 'git submodule' [--quiet] update [--init] [-N|--no-fetch] [--rebase]
@@ -104,6 +105,17 @@ together in the same relative location, and only the
 superproject's URL needs to be provided: git-submodule will correctly
 locate the submodule using the relative URL in .gitmodules.
 
+rm::
+	Remove and unregister the submodules at given paths.
++
+This requires at least one <path> argument. The repository checkout
+existing at that directory needs to be removed manually from
+the filesystem prior to calling this command. Note that all local
+changes will be lost.
++
+This command removes the submodule from the current git index,
+the .gitmodules file and the local repository config.
+
 status::
 	Show the status of the submodules. This will print the SHA-1 of the
 	currently checked out commit for each submodule, along with the
diff --git a/git-submodule.sh b/git-submodule.sh
index fbf2faf..88fd414 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -6,6 +6,7 @@
 
 dashless=$(basename "$0" | sed -e 's/-/ /')
 USAGE="[--quiet] add [-b branch] [-f|--force] [--reference <repository>] [--] <repository> [<path>]
+   or: $dashless [--quiet] rm [--] <path>...
    or: $dashless [--quiet] status [--cached] [--recursive] [--] [<path>...]
    or: $dashless [--quiet] init [--] [<path>...]
    or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
@@ -308,6 +309,71 @@ Use -f if you really want to add it." >&2
 }
 
 #
+# Remove submodules from the working tree, .gitmodules and the index
+#
+# $@ = submodule paths
+#
+cmd_rm()
+{
+	# parse $args after "submodule ... rm".
+	while test $# -ne 0
+	do
+		case "$1" in
+		--)
+			shift
+			break
+			;;
+		-*)
+			usage
+			;;
+		*)
+			break
+			;;
+		esac
+		shift
+	done
+
+	if test -z "$1"; then
+		usage
+	fi
+
+	while test $# -ne 0
+	do
+		sm_path=$1
+		shift
+
+		# normalize path:
+		# multiple //; leading ./; /./; /../; trailing /
+		sm_path=$(printf '%s/\n' "$sm_path" |
+			sed -e '
+				s|//*|/|g
+				s|^\(\./\)*||
+				s|/\./|/|g
+				:start
+				s|\([^/]*\)/\.\./||
+				tstart
+				s|/*$||
+			')
+		git ls-files --error-unmatch "$sm_path" > /dev/null 2>&1 ||
+		die "$(eval_gettext "'\$sm_path' does not exist in the index")"
+
+		if test -e "$sm_path"
+		then
+			die "$(eval_gettext "'\$sm_path' needs to be removed manually first")"
+		fi
+
+		git rm --cached "$sm_path" ||
+		die "$(eval_gettext "Failed to remove submodule '\$sm_path'")"
+
+		git config -f .gitmodules --remove-section submodule."$sm_path" &&
+		git add --force .gitmodules ||
+		die "$(eval_gettext "Failed to unregister submodule '\$sm_path'")"
+
+		git config --remove-section submodule."$sm_path"
+	done
+}
+
+#
 # Execute an arbitrary command sequence in each checked out
 # submodule
 #
@@ -996,7 +1062,7 @@ cmd_sync()
 while test $# != 0 && test -z "$command"
 do
 	case "$1" in
-	add | foreach | init | update | status | summary | sync)
+	add | rm | foreach | init | update | status | summary | sync)
 		command=$1
 		;;
 	-q|--quiet)
-- 
1.7.10.2

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] git-submodule: support 'rm' command.
  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-28 20:31     ` Michał Górny
  0 siblings, 2 replies; 9+ messages in thread
From: Jens Lehmann @ 2012-06-25 16:58 UTC (permalink / raw)
  To: Michał Górny; +Cc: git

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.

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

>  Documentation/git-submodule.txt |   12 +++++++
>  git-submodule.sh                |   68 ++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 79 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
> index fbbbcb2..293c1bf 100644
> --- a/Documentation/git-submodule.txt
> +++ b/Documentation/git-submodule.txt
> @@ -11,6 +11,7 @@ SYNOPSIS
>  [verse]
>  'git submodule' [--quiet] add [-b branch] [-f|--force]
>  	      [--reference <repository>] [--] <repository> [<path>]
> +'git submodule' [--quiet] rm <path>...
>  'git submodule' [--quiet] status [--cached] [--recursive] [--] [<path>...]
>  'git submodule' [--quiet] init [--] [<path>...]
>  'git submodule' [--quiet] update [--init] [-N|--no-fetch] [--rebase]
> @@ -104,6 +105,17 @@ together in the same relative location, and only the
>  superproject's URL needs to be provided: git-submodule will correctly
>  locate the submodule using the relative URL in .gitmodules.
>  
> +rm::
> +	Remove and unregister the submodules at given paths.
> ++
> +This requires at least one <path> argument. The repository checkout
> +existing at that directory needs to be removed manually from
> +the filesystem prior to calling this command. Note that all local
> +changes will be lost.

Me thinks without -f a "git rm" should only then remove a submodule
if no local modifications exist and current HEAD is part of a remote
branch (so you can't loose unpushed commits by accident). If the
submodule uses a gitfile a local branch might be sufficient for that,
as the git directory lives on.

> ++
> +This command removes the submodule from the current git index,
> +the .gitmodules file and the local repository config.

It should not be removed from .git/config by default. The user may
have special settings there and the presence in .git/config shows
he cared about having the submodule checked out, which should not
be revoked by just removing the submodule from the work tree.
Unless he removes the config from there himself he should get back
a populated submodule when he checks out an earlier commit and says
"git submodule update".

>  status::
>  	Show the status of the submodules. This will print the SHA-1 of the
>  	currently checked out commit for each submodule, along with the
> diff --git a/git-submodule.sh b/git-submodule.sh
> index fbf2faf..88fd414 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -6,6 +6,7 @@
>  
>  dashless=$(basename "$0" | sed -e 's/-/ /')
>  USAGE="[--quiet] add [-b branch] [-f|--force] [--reference <repository>] [--] <repository> [<path>]
> +   or: $dashless [--quiet] rm [--] <path>...
>     or: $dashless [--quiet] status [--cached] [--recursive] [--] [<path>...]
>     or: $dashless [--quiet] init [--] [<path>...]
>     or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
> @@ -308,6 +309,71 @@ Use -f if you really want to add it." >&2
>  }
>  
>  #
> +# Remove submodules from the working tree, .gitmodules and the index
> +#
> +# $@ = submodule paths
> +#
> +cmd_rm()
> +{
> +	# parse $args after "submodule ... rm".
> +	while test $# -ne 0
> +	do
> +		case "$1" in
> +		--)
> +			shift
> +			break
> +			;;
> +		-*)
> +			usage
> +			;;
> +		*)
> +			break
> +			;;
> +		esac
> +		shift
> +	done
> +
> +	if test -z "$1"; then
> +		usage
> +	fi
> +
> +	while test $# -ne 0
> +	do
> +		sm_path=$1
> +		shift
> +
> +		# normalize path:
> +		# multiple //; leading ./; /./; /../; trailing /
> +		sm_path=$(printf '%s/\n' "$sm_path" |
> +			sed -e '
> +				s|//*|/|g
> +				s|^\(\./\)*||
> +				s|/\./|/|g
> +				:start
> +				s|\([^/]*\)/\.\./||
> +				tstart
> +				s|/*$||
> +			')
> +		git ls-files --error-unmatch "$sm_path" > /dev/null 2>&1 ||
> +		die "$(eval_gettext "'\$sm_path' does not exist in the index")"
> +
> +		if test -e "$sm_path"
> +		then
> +			die "$(eval_gettext "'\$sm_path' needs to be removed manually first")"
> +		fi
> +
> +		git rm --cached "$sm_path" ||
> +		die "$(eval_gettext "Failed to remove submodule '\$sm_path'")"
> +
> +		git config -f .gitmodules --remove-section submodule."$sm_path" &&
> +		git add --force .gitmodules ||
> +		die "$(eval_gettext "Failed to unregister submodule '\$sm_path'")"
> +
> +		git config --remove-section submodule."$sm_path"
> +	done
> +}
> +
> +#
>  # Execute an arbitrary command sequence in each checked out
>  # submodule
>  #
> @@ -996,7 +1062,7 @@ cmd_sync()
>  while test $# != 0 && test -z "$command"
>  do
>  	case "$1" in
> -	add | foreach | init | update | status | summary | sync)
> +	add | rm | foreach | init | update | status | summary | sync)
>  		command=$1
>  		;;
>  	-q|--quiet)
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] git-submodule: support 'rm' command.
  2012-06-25 16:58   ` Jens Lehmann
@ 2012-06-25 20:53     ` Phil Hord
  2012-06-25 21:09       ` Jens Lehmann
  2012-06-28 20:31     ` Michał Górny
  1 sibling, 1 reply; 9+ messages in thread
From: Phil Hord @ 2012-06-25 20:53 UTC (permalink / raw)
  To: Jens Lehmann; +Cc: Michał Górny, git

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


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

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.

Phil

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] git-submodule: support 'rm' command.
  2012-06-25 20:53     ` Phil Hord
@ 2012-06-25 21:09       ` Jens Lehmann
  2012-06-26 19:12         ` Phil Hord
  0 siblings, 1 reply; 9+ messages in thread
From: Jens Lehmann @ 2012-06-25 21:09 UTC (permalink / raw)
  To: Phil Hord; +Cc: Michał Górny, git

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.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] git-submodule: support 'rm' command.
  2012-06-25 21:09       ` Jens Lehmann
@ 2012-06-26 19:12         ` Phil Hord
  2012-06-26 19:59           ` Jens Lehmann
  0 siblings, 1 reply; 9+ messages in thread
From: Phil Hord @ 2012-06-26 19:12 UTC (permalink / raw)
  To: Jens Lehmann; +Cc: Michał Górny, git

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.

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

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

So you did.  What bothered me was the suggestion that you could
partially remove a submodule's files.

>> 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.  Maybe we can make this an explicit action.
"git submodule convert-to-gitfile"  :-)

Phil

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] git-submodule: support 'rm' command.
  2012-06-26 19:12         ` Phil Hord
@ 2012-06-26 19:59           ` Jens Lehmann
  2012-06-27 18:48             ` Phil Hord
  0 siblings, 1 reply; 9+ messages in thread
From: Jens Lehmann @ 2012-06-26 19:59 UTC (permalink / raw)
  To: Phil Hord; +Cc: Michał Górny, git

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!

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] git-submodule: support 'rm' command.
  2012-06-26 19:59           ` Jens Lehmann
@ 2012-06-27 18:48             ` Phil Hord
  0 siblings, 0 replies; 9+ messages in thread
From: Phil Hord @ 2012-06-27 18:48 UTC (permalink / raw)
  To: Jens Lehmann; +Cc: Michał Górny, git

On Tue, Jun 26, 2012 at 3:59 PM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
> 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.

Sounds ok to me.  But maybe keep it in a separate module just for
manipulating submodules; you know, to reduce our spaghetti score.

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

I like that, though I despise the --recurse-submodules option because
A) it is too long, and B) it is sometimes spelled "--recurse" (for
good reasons, I'm sure, but it's irritating nonetheless).

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] git-submodule: support 'rm' command.
  2012-06-25 16:58   ` Jens Lehmann
  2012-06-25 20:53     ` Phil Hord
@ 2012-06-28 20:31     ` Michał Górny
  1 sibling, 0 replies; 9+ messages in thread
From: Michał Górny @ 2012-06-28 20:31 UTC (permalink / raw)
  To: Jens Lehmann; +Cc: git

[-- Attachment #1: Type: text/plain, Size: 2664 bytes --]

On Mon, 25 Jun 2012 18:58:36 +0200
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.

My main intent was, like Phil already pointed out, to provide
a counterpart to 'git submodule add'. I don't mind 'git rm' supporting
removing submodules but that's not exactly what I would consider...
friendly?

What I'm trying to express is that if I added a submodule using 'git
submodule add', I would expect to have 'git submodule rm'. Honestly, I
didn't even think about trying using 'git rm' on a submodule.
The correct results of such a call are hard to predict to me.

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

Except for the untracked files there may also be commits which were not
pushed anywhere. Honestly, I didn't want to get into that risky area in
the first patch.

As the next step, I'd see adding a '--force' option to actually enforce
removing the directory. But I'd like the basics to start working first,
then consider harder things.

> > ++
> > +This command removes the submodule from the current git index,
> > +the .gitmodules file and the local repository config.
> 
> It should not be removed from .git/config by default. The user may
> have special settings there and the presence in .git/config shows
> he cared about having the submodule checked out, which should not
> be revoked by just removing the submodule from the work tree.
> Unless he removes the config from there himself he should get back
> a populated submodule when he checks out an earlier commit and says
> "git submodule update".

Ok, I will change that.

-- 
Best regards,
Michał Górny

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 316 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2012-06-28 20:31 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
2012-06-27 18:48             ` Phil Hord
2012-06-28 20:31     ` Michał Górny

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