threads / patch / 32004

patchAdd rm to submodule command.

Subject: [PATCH] Add rm to submodule command.

## tl;dr

3 messages between Nov 2, 2012 and Nov 4, 2012. Diffs are folded; open one to read it.

replies: 2people: 3as markdown or json

Alex Linden Levy· Nov 2, 2012, 17:26 UTC · lore

This change removes the config entries in .gitmodules and adds it.
---
 git-submodule.sh | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 61 insertions(+), 1 deletion(-)

Show changes to git-submodule.sh +61 −1

diff --git a/git-submodule.sh b/git-submodule.sh
index ab6b110..29d950f 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 [-b branch] [-f|--force] [--] [<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>...]
@@ -370,6 +371,65 @@ Use -f if you really want to add it." >&2
 	die "$(eval_gettext "Failed to register submodule '\$sm_path'")"
 }
 
+cmd_rm()
+{
+	# parse $args after "submodule ... rm".
+	while test $# -ne 0
+	do
+		case "$1" in
+		-b | --branch)
+			case "$2" in '') usage ;; esac
+			branch=$2
+			shift
+			;;
+		-f | --force)
+			force=$1
+			;;
+		-q|--quiet)
+			GIT_QUIET=1
+			;;
+		--)
+			shift
+			break
+			;;
+		-*)
+			usage
+			;;
+		*)
+			break
+			;;
+		esac
+		shift
+	done
+	
+	sm_path=$1
+
+	# normalize path:
+	# multiple //; leading ./; /./; /../; trailing /
+	sm_path=$(printf '%s/\n' "$sm_path" |
+		sed -e '
+			s|//*|/|g
+			s|^\(\./\)*||
+			s|/\./|/|g
+			:start
+			s|\([^/]*\)/\.\./||
+			tstart
+			s|/*$||
+		')
+
+
+	#edit .gitmodules
+	git config -f .gitmodules --remove-section submodule."$sm_path" ||
+	die "$(eval_gettext "Failed to rm submodule '\$sm_path' section")"
+
+	#get rid of the submodule
+	git rm --cached $force "$sm_path" ||
+	die "$(eval_gettext "Failed to rm submodule '\$sm_path'")"
+	
+	#now add the .gitmodules change
+	git add ${force} .gitmodules ||
+	die "$(eval_gettext "Failed to remove submodule '\$sm_path'")"
+}
 #
 # Execute an arbitrary command sequence in each checked out
 # submodule
@@ -1076,7 +1136,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.8.0.1.g3039071.dirty

Jeff King· Nov 4, 2012, 13:43 UTC · re: Alex Linden Levy · lore

Re: [PATCH] Add rm to submodule command.

On Fri, Nov 02, 2012 at 10:26:11AM -0700, Alex Linden Levy wrote:

> This change removes the config entries in .gitmodules and adds it.
> ---

Signoff?

>  git-submodule.sh | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 61 insertions(+), 1 deletion(-)

No documentation or tests?

> diff --git a/git-submodule.sh b/git-submodule.sh
> index ab6b110..29d950f 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh

I'd defer to submodule experts on whether the steps to 'rm' the submodule make sense. Jens?

-Peff

Jens Lehmann· Nov 4, 2012, 14:29 UTC · re: Jeff King · lore

Re: [PATCH] Add rm to submodule command.

Am 04.11.2012 14:43, schrieb Jeff King:

Show 19 quoted lines

> On Fri, Nov 02, 2012 at 10:26:11AM -0700, Alex Linden Levy wrote:
> 
>> This change removes the config entries in .gitmodules and adds it.
>> ---
> 
> Signoff?
> 
>>  git-submodule.sh | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 61 insertions(+), 1 deletion(-)
> 
> No documentation or tests?
> 
>> diff --git a/git-submodule.sh b/git-submodule.sh
>> index ab6b110..29d950f 100755
>> --- a/git-submodule.sh
>> +++ b/git-submodule.sh
> 
> I'd defer to submodule experts on whether the steps to 'rm' the
> submodule make sense. Jens?

Hmm, this change adds the --quiet and --branch options to rm which aren't used (and at least --branch makes no sense to me here). Remainders of copy & paste? It also only affects the .gitmodules setting and leaves the submodules work tree alone, while I think it should - at least optionally - remove the work tree just like "git rm" removes files too (of course only if there is no .git directory found in it and no modifications are present, as that would possibly lose data).

Me also thinks such a command should use my recent rm submodule work to remove the work tree (found in your current master and Junio's next branch), which does all necessary checks before it removes the work tree together with the index entry. This could be tweaked via a --cached option or such if the user wants to keep the work tree.

But apart from those issues I'm not convinced that adding a "git submodule rm" command is the best option. While working on teaching "git mv" to move submodules I came to the conclusion that it might be a better solution to let "git rm" remove the submodule entry from the .gitmodules file itself (but of course only if that file is found and contains such an entry, if not it will silently do nothing to not disturb submodule users who don't have a .gitmodules file and are using plain gitlinks).

The reason is that git core commands like status, diff and fetch already use the path -> name mapping found in .gitmodules and will behave strange when this is out of sync with the work tree. So I strongly believe that doing a "git mv" should change the path -> name mapping in .gitmodules too while moving the submodule's work tree and updating the index (of course again only if .gitmodules is found and contains such an entry, if not it'll just move the work tree and update the index). Then we won't need a new "git submodule mv" command as everything is done inside the mv command. And for consistency I think "git rm" should also remove the path -> name mapping (even though that is not required for a rm to do its job, as no one will use that setting later when the submodule is gone from the index). Then we won't need a new "git submodule rm" at all.

Does that make sense?