[PATCH 00/12] Improve handling of moving and removing submodules

[PATCH 00/12] Improve handling of moving and removing submodules

[Peter Collingbourne]

This patch series deals with a number of issues associated with moving
and removing submodules.  Specifically it improves submodule support
in the following subcommands: apply merge mv rm

This series was inspired by an earlier unmerged series [1] by Petr
Baudis.

[1] http://kerneltrap.org/mailarchive/git/2008/7/16/2533024

 Documentation/git-mv.txt        |    8 ++-
 Documentation/git-rm.txt        |    5 ++-
 Documentation/git-submodule.txt |    8 +++-
 builtin/apply.c                 |    8 +--
 builtin/mv.c                    |   33 +++++++++++--
 builtin/rm.c                    |   45 ++++++++++++++---
 dir.c                           |    4 +-
 dir.h                           |    2 +-
 git-compat-util.h               |    9 +++
 git-submodule.sh                |   95 +++++++++++++++++++++++++++++++++--
 merge-recursive.c               |   27 ++++++----
 t/t4134-apply-submodule.sh      |   38 ++++++++++++++
 t/t7403-submodule-sync.sh       |    2 +-
 t/t7405-submodule-merge.sh      |   13 +++++
 t/t7406-submodule-update.sh     |    6 +-
 t/t7407-submodule-foreach.sh    |   14 +++---
 t/t7409-submodule-mv-rm.sh      |  105 +++++++++++++++++++++++++++++++++++++++
 unpack-trees.c                  |   12 +----
 wrapper.c                       |   22 ++++++--
 19 files changed, 390 insertions(+), 65 deletions(-)

[PATCH 01/12] Generate unique ID for submodules created using "git submodule add"

[Peter Collingbourne]

This patch causes "git submodule add" to generate a unique ID for
the submodule which is used as its name.  The ID is generated by
computing the SHA1 hash of the pid, date and initial path.

The purpose of this patch is to avoid name conflicts which may
arise due to the ability to rename submodules.

Signed-off-by: Peter Collingbourne <peter@pcc.me.uk>
---
 Documentation/git-submodule.txt |    8 +++++++-
 git-submodule.sh                |   28 ++++++++++++++++++++++++++--
 t/t7403-submodule-sync.sh       |    2 +-
 t/t7406-submodule-update.sh     |    6 +++---
 t/t7407-submodule-foreach.sh    |   14 +++++++-------
 5 files changed, 44 insertions(+), 14 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 2502531..1bf78b6 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git submodule' [--quiet] add [-b branch]
-	      [--reference <repository>] [--] <repository> [<path>]
+	      [--reference <repository>] [-n <name>] [--] <repository> [<path>]
 'git submodule' [--quiet] status [--cached] [--recursive] [--] [<path>...]
 'git submodule' [--quiet] init [--] [<path>...]
 'git submodule' [--quiet] update [--init] [-N|--no-fetch] [--rebase]
@@ -199,6 +199,12 @@ OPTIONS
 	(the default). This limit only applies to modified submodules. The
 	size is always limited to 1 for added/deleted/typechanged submodules.
 
+-n <name>::
+--name <name>::
+	This option is only valid for the add command.
+	Name of the new submodule.  By default, a unique identifier
+	is generated.
+
 -N::
 --no-fetch::
 	This option is only valid for the update command.
diff --git a/git-submodule.sh b/git-submodule.sh
index 2dd372a..f05ff4e 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -114,6 +114,20 @@ module_clone()
 }
 
 #
+# Generate a unique identifier.  Used to name a submodule.
+#
+gen_uid()
+{
+	path="$1"
+
+	(
+	 echo "$path"
+	 echo $$
+	 date
+	) | git hash-object --stdin
+}
+
+#
 # Add a new submodule to the working tree, .gitmodules and the index
 #
 # $@ = repo path
@@ -131,6 +145,11 @@ cmd_add()
 			branch=$2
 			shift
 			;;
+		-n | --name)
+			case "$2" in '') usage ;; esac
+			name=$2
+			shift
+			;;
 		-q|--quiet)
 			GIT_QUIET=1
 			;;
@@ -235,8 +254,13 @@ cmd_add()
 	git add "$path" ||
 	die "Failed to add submodule '$path'"
 
-	git config -f .gitmodules submodule."$path".path "$path" &&
-	git config -f .gitmodules submodule."$path".url "$repo" &&
+	if test -z "$name"
+	then
+		name=$(gen_uid "$path")
+	fi
+
+	git config -f .gitmodules submodule."$name".path "$path" &&
+	git config -f .gitmodules submodule."$name".url "$repo" &&
 	git add .gitmodules ||
 	die "Failed to register submodule '$path'"
 }
diff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh
index 7538756..f2c66f8 100755
--- a/t/t7403-submodule-sync.sh
+++ b/t/t7403-submodule-sync.sh
@@ -18,7 +18,7 @@ test_expect_success setup '
 	git clone . super &&
 	git clone super submodule &&
 	(cd super &&
-	 git submodule add ../submodule submodule &&
+	 git submodule add -n submodule ../submodule submodule &&
 	 test_tick &&
 	 git commit -m "submodule"
 	) &&
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index 1382a8e..5bcac8f 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -31,7 +31,7 @@ test_expect_success 'setup a submodule tree' '
 	git clone super rebasing &&
 	git clone super merging &&
 	(cd super &&
-	 git submodule add ../submodule submodule &&
+	 git submodule add -n submodule ../submodule submodule &&
 	 test_tick &&
 	 git commit -m "submodule" &&
 	 git submodule init submodule
@@ -49,12 +49,12 @@ test_expect_success 'setup a submodule tree' '
 	 git commit -m "submodule update"
 	) &&
 	(cd super &&
-	 git submodule add ../rebasing rebasing &&
+	 git submodule add -n rebasing ../rebasing rebasing &&
 	 test_tick &&
 	 git commit -m "rebasing"
 	) &&
 	(cd super &&
-	 git submodule add ../merging merging &&
+	 git submodule add -n merging ../merging merging &&
 	 test_tick &&
 	 git commit -m "rebasing"
 	)
diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh
index 2a52775..a0390dd 100755
--- a/t/t7407-submodule-foreach.sh
+++ b/t/t7407-submodule-foreach.sh
@@ -21,9 +21,9 @@ test_expect_success 'setup a submodule tree' '
 	git clone super submodule &&
 	(
 		cd super &&
-		git submodule add ../submodule sub1 &&
-		git submodule add ../submodule sub2 &&
-		git submodule add ../submodule sub3 &&
+		git submodule add -n sub1 ../submodule sub1 &&
+		git submodule add -n sub2 ../submodule sub2 &&
+		git submodule add -n sub3 ../submodule sub3 &&
 		git config -f .gitmodules --rename-section \
 			submodule.sub1 submodule.foo1 &&
 		git config -f .gitmodules --rename-section \
@@ -82,28 +82,28 @@ test_expect_success 'setup nested submodules' '
 	git clone submodule nested3 &&
 	(
 		cd nested3 &&
-		git submodule add ../submodule submodule &&
+		git submodule add -n submodule ../submodule submodule &&
 		test_tick &&
 		git commit -m "submodule" &&
 		git submodule init submodule
 	) &&
 	(
 		cd nested2 &&
-		git submodule add ../nested3 nested3 &&
+		git submodule add -n nested3 ../nested3 nested3 &&
 		test_tick &&
 		git commit -m "nested3" &&
 		git submodule init nested3
 	) &&
 	(
 		cd nested1 &&
-		git submodule add ../nested2 nested2 &&
+		git submodule add -n nested2 ../nested2 nested2 &&
 		test_tick &&
 		git commit -m "nested2" &&
 		git submodule init nested2
 	) &&
 	(
 		cd super &&
-		git submodule add ../nested1 nested1 &&
+		git submodule add -n nested1 ../nested1 nested1 &&
 		test_tick &&
 		git commit -m "nested1" &&
 		git submodule init nested1
-- 
1.6.5

Re: [PATCH 01/12] Generate unique ID for submodules created using "git submodule add"

[Jonathan Nieder]

Peter Collingbourne wrote:

> This patch causes "git submodule add" to generate a unique ID for
> the submodule which is used as its name.  The ID is generated by
> computing the SHA1 hash of the pid, date and initial path.
> 
> The purpose of this patch is to avoid name conflicts which may
> arise due to the ability to rename submodules.

I assume this is related to Pasky’s patch “git submodule add: Fix
naming clash handling” [1]

	This patch fixes git submodule add behaviour when we add submodule
	living at a same path as logical name of existing submodule. This
	can happen e.g. in case the user git mv's the previous submodule away
	and then git submodule add's another under the same name.
	
	A test-case is obviously included.
	
	This is not completely satisfactory since .git/config cross-commit
	conflicts can still occur. A question is whether this is worth
	handling, maybe it would be worth adding some kind of randomization
	of the autogenerated submodule name, e.g. appending $$ or a timestamp.

The suggestion of _appending_ some nonsense to a submodule name sounds
much more palatable to me than _replacing_ the submodule name with
nonsense.  YMMV, of course.

Regards,
Jonathan

[1] submitted twice: once as part of the series you pointed to and
again in the re-roll at
http://thread.gmane.org/gmane.comp.version-control.git/95763/focus=95769

Re: [PATCH 01/12] Generate unique ID for submodules created using "git submodule add"

[Peter Collingbourne]

> The suggestion of _appending_ some nonsense to a submodule name sounds
> much more palatable to me than _replacing_ the submodule name with
> nonsense.  YMMV, of course.

Indeed.  But we should be careful about which components of the path
we include in the submodule name, in order to minimise confusion.
Here are a couple of suggested patches which could be squashed into
this patch; we may choose to apply one or both of them.

Thanks.
-- 
Peter

[PATCH 1/2] Prefix submodule names with the path basename

[Peter Collingbourne]

We should include some component of the submodule path in the submodule
name to make it possible to identify a particular submodule by its
name.  At the same time we should be conscious of the fact that the
submodule path may change.  Including the entire submodule path in
the submodule name is likely to confuse users if the path changes.
The basename is the component of the path which is least likely
to change, which is a factor in favour of its inclusion in the
submodule name.

Signed-off-by: Peter Collingbourne <peter@pcc.me.uk>
---
 git-submodule.sh |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index f05ff4e..4f0d7df 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -120,6 +120,9 @@ gen_uid()
 {
 	path="$1"
 
+	pathbase=$(basename "$path")
+	echo -n "$pathbase"-
+
 	(
 	 echo "$path"
 	 echo $$
-- 
1.6.5

[PATCH 2/2] Truncate the SHA1 part of the submodule name to 7 characters

[Peter Collingbourne]

The rationale is that a submodule naming clash is a highly infrequent
event (as compared to a file naming clash) so we can minimise the
ugliness of generated submodule names by using a smaller number
of characters.

Signed-off-by: Peter Collingbourne <peter@pcc.me.uk>
---
 git-submodule.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 4f0d7df..e85f2d1 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -127,7 +127,7 @@ gen_uid()
 	 echo "$path"
 	 echo $$
 	 date
-	) | git hash-object --stdin
+	) | git hash-object --stdin | cut -c1-7
 }
 
 #
-- 
1.6.5

[PATCH 02/12] Implement "git mv" for submodules

[Peter Collingbourne]

This patch teaches "git mv" how to handle moving submodules, including
how to update the .gitmodules file.

The .gitmodules update is handled by an undocumented subcommand to
"git submodule" named "mvconfig".

Signed-off-by: Peter Collingbourne <peter@pcc.me.uk>
---
 Documentation/git-mv.txt |    7 ++-
 builtin/mv.c             |   33 ++++++++++++++--
 git-submodule.sh         |   16 +++++++-
 t/t7409-submodule-mv.sh  |   94 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 143 insertions(+), 7 deletions(-)
 create mode 100755 t/t7409-submodule-mv.sh

diff --git a/Documentation/git-mv.txt b/Documentation/git-mv.txt
index bdcb585..632a6a9 100644
--- a/Documentation/git-mv.txt
+++ b/Documentation/git-mv.txt
@@ -14,8 +14,8 @@ DESCRIPTION
 -----------
 This script is used to move or rename a file, directory or symlink.
 
- git mv [-f] [-n] <source> <destination>
- git mv [-f] [-n] [-k] <source> ... <destination directory>
+ git mv [-f] [-n] [-M] <source> <destination>
+ git mv [-f] [-n] [-k] [-M] <source> ... <destination directory>
 
 In the first form, it renames <source>, which must exist and be either
 a file, symlink or directory, to <destination>.
@@ -39,6 +39,9 @@ OPTIONS
 --dry-run::
 	Do nothing; only show what would happen
 
+-M::
+	Do not try to update submodule paths in .gitmodules
+
 
 Author
 ------
diff --git a/builtin/mv.c b/builtin/mv.c
index c07f53b..21fd03f 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -9,6 +9,7 @@
 #include "cache-tree.h"
 #include "string-list.h"
 #include "parse-options.h"
+#include "run-command.h"
 
 static const char * const builtin_mv_usage[] = {
 	"git mv [options] <source>... <destination>",
@@ -53,11 +54,12 @@ static struct lock_file lock_file;
 int cmd_mv(int argc, const char **argv, const char *prefix)
 {
 	int i, newfd;
-	int verbose = 0, show_only = 0, force = 0, ignore_errors = 0;
+	int verbose = 0, show_only = 0, force = 0, ignore_errors = 0, skip_module_update = 0;
 	struct option builtin_mv_options[] = {
 		OPT__DRY_RUN(&show_only),
 		OPT_BOOLEAN('f', "force", &force, "force move/rename even if target exists"),
 		OPT_BOOLEAN('k', NULL, &ignore_errors, "skip move/rename errors"),
+		OPT_BOOLEAN('M', NULL, &skip_module_update, "don't update submodule entries"),
 		OPT_END(),
 	};
 	const char **source, **destination, **dest_path;
@@ -96,13 +98,14 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 	/* Checking */
 	for (i = 0; i < argc; i++) {
 		const char *src = source[i], *dst = destination[i];
-		int length, src_is_dir;
+		int length, src_is_dir, pos;
 		const char *bad = NULL;
 
 		if (show_only)
 			printf("Checking rename of '%s' to '%s'\n", src, dst);
 
 		length = strlen(src);
+		pos = cache_name_pos(src, length);
 		if (lstat(src, &st) < 0)
 			bad = "bad source";
 		else if (!strncmp(src, dst, length) &&
@@ -111,7 +114,9 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 		} else if ((src_is_dir = S_ISDIR(st.st_mode))
 				&& lstat(dst, &st) == 0)
 			bad = "cannot move directory over file";
-		else if (src_is_dir) {
+		else if (src_is_dir &&
+				!(pos >= 0 &&
+				  S_ISGITLINK(active_cache[pos]->ce_mode))) {
 			const char *src_w_slash = add_slash(src);
 			int len_w_slash = length + 1;
 			int first, last;
@@ -162,7 +167,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 				}
 				argc += last - first;
 			}
-		} else if (cache_name_pos(src, length) < 0)
+		} else if (pos < 0)
 			bad = "not under version control";
 		else if (lstat(dst, &st) == 0) {
 			bad = "destination exists";
@@ -223,5 +228,25 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 			die("Unable to write new index file");
 	}
 
+	if (!show_only && !skip_module_update)
+		for (i = 0; i < argc; i++) {
+			const char *src = source[i], *dst = destination[i];
+			int pos;
+
+			if (modes[i] == WORKING_DIRECTORY)
+				continue;
+
+			pos = cache_name_pos(dst, strlen(dst));
+			assert(pos >= 0);
+
+			if (S_ISGITLINK(active_cache[pos]->ce_mode)) {
+				const char *argv_submodule[] = {
+					"submodule", "mvconfig", src, dst, NULL
+				};
+
+				run_command_v_opt(argv_submodule, RUN_GIT_CMD);
+			}
+		}
+
 	return 0;
 }
diff --git a/git-submodule.sh b/git-submodule.sh
index f05ff4e..d0b7a79 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -856,6 +856,20 @@ cmd_sync()
 		fi
 	done
 }
+#
+# Updates the entry in .gitmodules to move a submodule.
+# This command is called by "git mv" for each submodule it moves.
+#
+cmd_mvconfig()
+{
+	src="$1"
+	dst="$2"
+
+	name=$(module_name "$src") || exit
+	git config -f .gitmodules submodule."$name".path "$dst" ||
+		die "Could not update .gitmodules entry for $name"
+	git add .gitmodules || die "Could not add .gitmodules to index"
+}
 
 # This loop parses the command line arguments to find the
 # subcommand name to dispatch.  Parsing of the subcommand specific
@@ -866,7 +880,7 @@ cmd_sync()
 while test $# != 0 && test -z "$command"
 do
 	case "$1" in
-	add | foreach | init | update | status | summary | sync)
+	add | foreach | init | update | status | summary | sync | mvconfig)
 		command=$1
 		;;
 	-q|--quiet)
diff --git a/t/t7409-submodule-mv.sh b/t/t7409-submodule-mv.sh
new file mode 100755
index 0000000..9eb3fb1
--- /dev/null
+++ b/t/t7409-submodule-mv.sh
@@ -0,0 +1,94 @@
+#!/bin/sh
+#
+# Copyright (c) 2010 Peter Collingbourne
+#
+
+test_description='git submodule mv
+
+These tests exercise the "git mv" command for submodules.
+'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+	echo file > file &&
+	git add file &&
+	test_tick &&
+	git commit -m upstream
+	git clone . super &&
+	git clone super submodule &&
+	(cd super &&
+	 git submodule add -n reg ../submodule reg &&
+	 git clone reg unreg &&
+	 git add unreg &&
+	 test_tick &&
+	 git commit -m "submodules"
+	)'
+
+test_expect_success 'move registered submodule' '
+	(cd super &&
+	 git mv reg reg2 &&
+	 test -z "$(git ls-files reg)" &&
+	 test -n "$(git ls-files reg2)" &&
+	 test ! -d reg &&
+	 test -d reg2 &&
+	 test -d reg2/.git &&
+	 test "$(git config -f .gitmodules submodule.reg.path)" = "reg2" &&
+	 test_tick &&
+	 git commit -a -m "move reg"
+	)
+'
+
+test_expect_success 'move unregistered submodule' '
+	(cd super &&
+	 git mv unreg unreg2 &&
+	 test ! -d unreg &&
+	 test -d unreg2 &&
+	 test -d unreg2/.git &&
+	 test_tick &&
+	 git commit -a -m "move unreg"
+	)
+'
+
+test_expect_success 'move unregistered uninitialised submodule' '
+	(cd super &&
+	 rm -rf unreg2 &&
+	 mkdir unreg2 &&
+	 git mv unreg2 unreg &&
+	 test -z "$(git ls-files unreg2)" &&
+	 test -n "$(git ls-files unreg)" &&
+	 test ! -d unreg2 &&
+	 test -d unreg &&
+	 test_tick &&
+	 git commit -a -m "move unreg2"
+	)
+'
+
+test_expect_success 'move registered submodule without changing .gitmodules' '
+	(cd super &&
+	 git mv -M reg2 reg &&
+	 test ! -d reg2 &&
+	 test -d reg &&
+	 test -d reg/.git &&
+	 test "$(git config -f .gitmodules submodule.reg.path)" = "reg2" &&
+	 git mv -M reg reg2
+	)
+'
+
+test_expect_success 'move multiple submodules at once' '
+	(cd super &&
+	 mkdir test\ dir &&
+	 git mv unreg reg2 test\ dir/ &&
+	 test ! -d unreg && 
+	 test ! -d reg2 && 
+	 test -d test\ dir/unreg && 
+	 test -d test\ dir/reg2 && 
+	 test -z "$(git ls-files unreg)" &&
+	 test -n "$(git ls-files test\ dir/unreg)" &&
+	 test -z "$(git ls-files reg2)" &&
+	 test -n "$(git ls-files test\ dir/reg2)" &&
+	 test "$(git config -f .gitmodules submodule.reg.path)" = "test dir/reg2"
+	)
+'
+
+test_done
-- 
1.6.5

[PATCH 03/12] git rm: display a warning for every unremovable file

[Peter Collingbourne]

This patch causes git-rm to display a warning if it was unable to
remove a file other than the first one, rather than silently ignoring
the error, which was the previous behaviour.

Signed-off-by: Peter Collingbourne <peter@pcc.me.uk>
---
 builtin/rm.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/builtin/rm.c b/builtin/rm.c
index f3772c8..05a5158 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -259,6 +259,8 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 			}
 			if (!removed)
 				die_errno("git rm: '%s'", path);
+			else
+				warning("git rm: '%s': %s", path, strerror(errno));
 		}
 	}
 
-- 
1.6.5

Re: [PATCH 03/12] git rm: display a warning for every unremovable file

[Jonathan Nieder]

Peter Collingbourne wrote:

> This patch causes git-rm to display a warning if it was unable to
> remove a file other than the first one, rather than silently ignoring
> the error, which was the previous behaviour.
> 
> Signed-off-by: Peter Collingbourne <peter@pcc.me.uk>
[...]
> @@ -259,6 +259,8 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
>  			}
>  			if (!removed)
>  				die_errno("git rm: '%s'", path);
> +			else
> +				warning("git rm: '%s': %s", path, strerror(errno));

My first reaction was to wonder what is going on.  Clearly someone
had been bothered by the obvious behavior of giving a long stream
of warnings or we wouldn’t have the current behavior.

But in fact, the unchanged code comment before the code you changed
and the message for commit d9b814cc (Add builtin "git rm" command,
2006-05-19) shows me wrong:

    So what happens is that if the _first_ file fails to be removed with "-f", 
    we abort the whole "git rm". But once we've started removing, we don't 
    leave anything half done. If some of the other files don't exist, we'll 
    just ignore errors of removal from the working tree.

    This is only an issue with "-f", of course.

    I think the new behaviour is strictly an improvement, but perhaps more 
    importantly, it is _different_. As a special case, the semantics are 
    identical for the single-file case (which is the only one our test-suite 
    seems to test).

I think the commit message could avoid this confusion.  Something like:

	When ‘git rm’ was built in (d9b814cc, 2006-05-19), its
	semantics changed: before, it just removed files until it
	encountered an error and then would error out, whereas since
	then, it makes an attempt to either remove all files or remove
	none at all.  In particular, if ‘git rm’ fails to remove a
	file after other files have already been removed, it does not
	abort but instead silently accepts the error.

	Better to warn the user in this case!

	This problem is particularly noticeable when dealing with
	submodules because ...

Some tests would be nice as well --- maybe something like the following?

diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index 0aaf0ad..a3861b8 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -126,6 +126,75 @@ test_expect_success 'Remove nonexistent file with --ignore-unmatch' '
 	git rm --ignore-unmatch nonexistent
 '
 
+test_expect_success 'If the first (in alphabetical order) removal fails, rm is cancelled' '
+	touch xyzzy &&
+	mkdir -p plugh &&
+	touch plugh/xyzzy &&
+	git add xyzzy plugh/xyzzy &&
+	git commit --allow-empty -a -m "two files to remove" &&
+	chmod a-w plugh &&
+	git ls-files --stage >before &&
+	test $(grep xyzzy before | wc -l) = 2 &&
+
+	test_must_fail git rm xyzzy plugh/xyzzy &&
+
+	test -e plugh/xyzzy &&
+	test -e xyzzy &&
+	git ls-files --stage >after &&
+	test_cmp before after
+'
+! test -e plugh || chmod 775 plugh
+rm -fr before after plugh xyzzy
+
+test_expect_success 'Best-effort behavior if the second removal fails' '
+	touch plugh &&
+	mkdir -p xyzzy &&
+	touch xyzzy/plugh &&
+	git add plugh xyzzy/plugh &&
+	git commit --allow-empty -a -m "two files to remove" &&
+	chmod a-w xyzzy &&
+	: >expect &&
+
+	git rm plugh xyzzy/plugh &&
+
+	test -e xyzzy/plugh &&
+	! test -e plugh &&
+	git ls-files --stage plugh xyzzy/plugh >actual &&
+	test_cmp expect actual
+'
+! test -e xyzzy || chmod 775 xyzzy
+rm -fr expect actual plugh xyzzy
+
+test_expect_success 'Message when first removal fails' '
+	touch xyzzy &&
+	mkdir -p plugh &&
+	touch plugh/xyzzy &&
+	git add xyzzy plugh/xyzzy &&
+	git commit --allow-empty -a -m "two files to remove" &&
+	chmod a-w plugh &&
+
+	test_must_fail git rm xyzzy plugh/xyzzy 2>msg &&
+
+	grep "git rm: '\''plugh/xyzzy'\'':" msg
+'
+! test -e plugh || chmod 775 plugh
+rm -fr msg plugh xyzzy
+
+test_expect_success 'Message when second removal fails' '
+	touch plugh &&
+	mkdir -p xyzzy &&
+	touch xyzzy/plugh &&
+	git add plugh xyzzy/plugh &&
+	git commit --allow-empty -a -m "two files to remove" &&
+	chmod a-w xyzzy &&
+
+	git rm plugh xyzzy/plugh 2>msg &&
+
+	grep "git rm: '\''xyzzy/plugh'\'':" msg
+'
+! test -e xyzzy || chmod 775 xyzzy
+rm -fr expect actual plugh xyzzy
+
 test_expect_success '"rm" command printed' '
 	echo frotz > test-file &&
 	git add test-file &&

[PATCH 04/12] Generalise the unlink_or_warn function

[Peter Collingbourne]

This patch moves the warning code of the unlink_or_warn function into
a separate function named warn_if_unremovable so that it may be reused.

Signed-off-by: Peter Collingbourne <peter@pcc.me.uk>
---
 wrapper.c |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/wrapper.c b/wrapper.c
index 9c71b21..0bbff56 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -311,18 +311,20 @@ int odb_pack_keep(char *name, size_t namesz, unsigned char *sha1)
 	return open(name, O_RDWR|O_CREAT|O_EXCL, 0600);
 }
 
-int unlink_or_warn(const char *file)
+static int warn_if_unremovable(const char *op, const char *file, int rc)
 {
-	int rc = unlink(file);
-
 	if (rc < 0) {
 		int err = errno;
 		if (ENOENT != err) {
-			warning("unable to unlink %s: %s",
-				file, strerror(errno));
+			warning("unable to %s %s: %s",
+				op, file, strerror(errno));
 			errno = err;
 		}
 	}
 	return rc;
 }
 
+int unlink_or_warn(const char *file)
+{
+	return warn_if_unremovable("unlink", file, unlink(file));
+}
-- 
1.6.5

[PATCH 05/12] Implement the rmdir_or_warn function

[Peter Collingbourne]

This patch implements an rmdir_or_warn function (like unlink_or_warn
but for directories) that uses the generalised warning code in
warn_if_unremovable.

Signed-off-by: Peter Collingbourne <peter@pcc.me.uk>
---
 git-compat-util.h |    4 ++++
 wrapper.c         |    5 +++++
 2 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/git-compat-util.h b/git-compat-util.h
index a3c4537..67ea4c8 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -469,5 +469,9 @@ void git_qsort(void *base, size_t nmemb, size_t size,
  * Always returns the return value of unlink(2).
  */
 int unlink_or_warn(const char *path);
+/*
+ * Likewise for rmdir(2).
+ */
+int rmdir_or_warn(const char *path);
 
 #endif
diff --git a/wrapper.c b/wrapper.c
index 0bbff56..4017bff 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -328,3 +328,8 @@ int unlink_or_warn(const char *file)
 {
 	return warn_if_unremovable("unlink", file, unlink(file));
 }
+
+int rmdir_or_warn(const char *file)
+{
+	return warn_if_unremovable("rmdir", file, rmdir(file));
+}
-- 
1.6.5

[PATCH 06/12] Introduce remove_or_warn function

[Peter Collingbourne]

This patch introduces the remove_or_warn function which is a
generalised version of the {unlink,rmdir}_or_warn functions.  It takes
an additional parameter indicating the mode of the file to be removed.

The patch also modifies certain functions to use remove_or_warn
where appropriate, and adds a test case for a bug fixed by the use
of remove_or_warn.

Signed-off-by: Peter Collingbourne <peter@pcc.me.uk>
---
 builtin/apply.c            |    6 +-----
 git-compat-util.h          |    5 +++++
 t/t4134-apply-submodule.sh |   38 ++++++++++++++++++++++++++++++++++++++
 unpack-trees.c             |   12 ++----------
 wrapper.c                  |    5 +++++
 5 files changed, 51 insertions(+), 15 deletions(-)
 create mode 100755 t/t4134-apply-submodule.sh

diff --git a/builtin/apply.c b/builtin/apply.c
index 7ca9047..65a594c 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3144,11 +3144,7 @@ static void remove_file(struct patch *patch, int rmdir_empty)
 			die("unable to remove %s from index", patch->old_name);
 	}
 	if (!cached) {
-		if (S_ISGITLINK(patch->old_mode)) {
-			if (rmdir(patch->old_name))
-				warning("unable to remove submodule %s",
-					patch->old_name);
-		} else if (!unlink_or_warn(patch->old_name) && rmdir_empty) {
+		if (!remove_or_warn(patch->old_mode, patch->old_name) && rmdir_empty) {
 			remove_path(patch->old_name);
 		}
 	}
diff --git a/git-compat-util.h b/git-compat-util.h
index 67ea4c8..3ebf966 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -473,5 +473,10 @@ int unlink_or_warn(const char *path);
  * Likewise for rmdir(2).
  */
 int rmdir_or_warn(const char *path);
+/*
+ * Calls the correct function out of {unlink,rmdir}_or_warn based on
+ * the supplied file mode.
+ */
+int remove_or_warn(unsigned int mode, const char *path);
 
 #endif
diff --git a/t/t4134-apply-submodule.sh b/t/t4134-apply-submodule.sh
new file mode 100755
index 0000000..1b82f93
--- /dev/null
+++ b/t/t4134-apply-submodule.sh
@@ -0,0 +1,38 @@
+#!/bin/sh
+#
+# Copyright (c) 2010 Peter Collingbourne
+#
+
+test_description='git apply submodule tests'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+	cat > create-sm.patch <<EOF
+diff --git a/dir/sm b/dir/sm
+new file mode 160000
+index 0000000..0123456
+--- /dev/null
++++ b/dir/sm
+@@ -0,0 +1 @@
++Subproject commit 0123456789abcdef0123456789abcdef01234567
+EOF
+	cat > remove-sm.patch <<EOF
+diff --git a/dir/sm b/dir/sm
+deleted file mode 160000
+index 0123456..0000000
+--- a/dir/sm
++++ /dev/null
+@@ -1 +0,0 @@
+-Subproject commit 0123456789abcdef0123456789abcdef01234567
+EOF
+'
+
+test_expect_success 'removing a submodule also removes all leading subdirectories' '
+	git apply --index create-sm.patch &&
+	test -d dir/sm &&
+	git apply --index remove-sm.patch &&
+	test \! -d dir
+'
+
+test_done
diff --git a/unpack-trees.c b/unpack-trees.c
index 75f54ca..c29a9e0 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -67,16 +67,8 @@ static void unlink_entry(struct cache_entry *ce)
 {
 	if (has_symlink_or_noent_leading_path(ce->name, ce_namelen(ce)))
 		return;
-	if (S_ISGITLINK(ce->ce_mode)) {
-		if (rmdir(ce->name)) {
-			warning("unable to rmdir %s: %s",
-				ce->name, strerror(errno));
-			return;
-		}
-	}
-	else
-		if (unlink_or_warn(ce->name))
-			return;
+	if (remove_or_warn(ce->ce_mode, ce->name))
+		return;
 	schedule_dir_for_removal(ce->name, ce_namelen(ce));
 }
 
diff --git a/wrapper.c b/wrapper.c
index 4017bff..10a6750 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -333,3 +333,8 @@ int rmdir_or_warn(const char *file)
 {
 	return warn_if_unremovable("rmdir", file, rmdir(file));
 }
+
+int remove_or_warn(unsigned int mode, const char *file)
+{
+	return S_ISGITLINK(mode) ? rmdir_or_warn(file) : unlink_or_warn(file);
+}
-- 
1.6.5

[PATCH 07/12] Remove a redundant errno test in a usage of remove_path

[Peter Collingbourne]

The errno test is redundant because the same test is carried
out in remove_path itself.

Signed-off-by: Peter Collingbourne <peter@pcc.me.uk>
---
 merge-recursive.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/merge-recursive.c b/merge-recursive.c
index 195ebf9..87232b8 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -409,7 +409,7 @@ static int remove_file(struct merge_options *o, int clean,
 			return -1;
 	}
 	if (update_working_directory) {
-		if (remove_path(path) && errno != ENOENT)
+		if (remove_path(path))
 			return -1;
 	}
 	return 0;
-- 
1.6.5

[PATCH 08/12] git rm: collect file modes

[Peter Collingbourne]

This patch causes git rm to collect file modes alongside file names
in its list data structure.

Signed-off-by: Peter Collingbourne <peter@pcc.me.uk>
---
 builtin/rm.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/builtin/rm.c b/builtin/rm.c
index 05a5158..61ec2cf 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -18,15 +18,18 @@ static const char * const builtin_rm_usage[] = {
 static struct {
 	int nr, alloc;
 	const char **name;
+	unsigned int *mode;
 } list;
 
-static void add_list(const char *name)
+static void add_list(const char *name, unsigned int mode)
 {
 	if (list.nr >= list.alloc) {
 		list.alloc = alloc_nr(list.alloc);
 		list.name = xrealloc(list.name, list.alloc * sizeof(const char *));
+		list.mode = xrealloc(list.mode, list.alloc * sizeof(unsigned int));
 	}
-	list.name[list.nr++] = name;
+	list.name[list.nr] = name;
+	list.mode[list.nr++] = mode;
 }
 
 static int check_local_mod(unsigned char *head, int index_only)
@@ -182,7 +185,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 		struct cache_entry *ce = active_cache[i];
 		if (!match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, seen))
 			continue;
-		add_list(ce->name);
+		add_list(ce->name, ce->ce_mode);
 	}
 
 	if (pathspec) {
-- 
1.6.5

[PATCH 09/12] Add a mode parameter to the remove_path function

[Peter Collingbourne]

This patch adds a mode parameter to remove_path which determines
whether unlink or rmdir is used.  All calls to remove_path have
been modified to supply the mode parameter.

This patch also adds a test case for a bug fixed by the addition
of the mode parameter to remove_path.

Signed-off-by: Peter Collingbourne <peter@pcc.me.uk>
---
 builtin/apply.c            |    2 +-
 builtin/rm.c               |    3 ++-
 dir.c                      |    4 ++--
 dir.h                      |    2 +-
 merge-recursive.c          |   27 ++++++++++++++++-----------
 t/t7405-submodule-merge.sh |   13 +++++++++++++
 6 files changed, 35 insertions(+), 16 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 65a594c..1e9e861 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3145,7 +3145,7 @@ static void remove_file(struct patch *patch, int rmdir_empty)
 	}
 	if (!cached) {
 		if (!remove_or_warn(patch->old_mode, patch->old_name) && rmdir_empty) {
-			remove_path(patch->old_name);
+			remove_path(patch->old_mode, patch->old_name);
 		}
 	}
 }
diff --git a/builtin/rm.c b/builtin/rm.c
index 61ec2cf..6ac5114 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -256,7 +256,8 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 		int removed = 0;
 		for (i = 0; i < list.nr; i++) {
 			const char *path = list.name[i];
-			if (!remove_path(path)) {
+			unsigned int mode = list.mode[i];
+			if (!remove_path(mode, path)) {
 				removed = 1;
 				continue;
 			}
diff --git a/dir.c b/dir.c
index cb83332..2d9cd9a 100644
--- a/dir.c
+++ b/dir.c
@@ -1047,11 +1047,11 @@ void setup_standard_excludes(struct dir_struct *dir)
 		add_excludes_from_file(dir, excludes_file);
 }
 
-int remove_path(const char *name)
+int remove_path(unsigned int mode, const char *name)
 {
 	char *slash;
 
-	if (unlink(name) && errno != ENOENT)
+	if ((S_ISGITLINK(mode) ? rmdir(name) : unlink(name)) && errno != ENOENT)
 		return -1;
 
 	slash = strrchr(name, '/');
diff --git a/dir.h b/dir.h
index 3bead5f..0e48d2a 100644
--- a/dir.h
+++ b/dir.h
@@ -98,6 +98,6 @@ extern void setup_standard_excludes(struct dir_struct *dir);
 extern int remove_dir_recursively(struct strbuf *path, int flag);
 
 /* tries to remove the path with empty directories along it, ignores ENOENT */
-extern int remove_path(const char *path);
+extern int remove_path(unsigned int mode, const char *path);
 
 #endif
diff --git a/merge-recursive.c b/merge-recursive.c
index 87232b8..c5b4149 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -399,7 +399,7 @@ static int update_stages(const char *path, struct diff_filespec *o,
 }
 
 static int remove_file(struct merge_options *o, int clean,
-		       const char *path, int no_wd)
+		       unsigned int mode, const char *path, int no_wd)
 {
 	int update_cache = o->call_depth || clean;
 	int update_working_directory = !o->call_depth && !no_wd;
@@ -409,7 +409,7 @@ static int remove_file(struct merge_options *o, int clean,
 			return -1;
 	}
 	if (update_working_directory) {
-		if (remove_path(path))
+		if (remove_path(mode, path))
 			return -1;
 	}
 	return 0;
@@ -734,6 +734,8 @@ static void conflict_rename_rename(struct merge_options *o,
 {
 	char *del[2];
 	int delp = 0;
+	unsigned int ren1_mode = ren1->pair->two->mode;
+	unsigned int ren2_mode = ren2->pair->two->mode;
 	const char *ren1_dst = ren1->pair->two->path;
 	const char *ren2_dst = ren2->pair->two->path;
 	const char *dst_name1 = ren1_dst;
@@ -742,13 +744,13 @@ static void conflict_rename_rename(struct merge_options *o,
 		dst_name1 = del[delp++] = unique_path(o, ren1_dst, branch1);
 		output(o, 1, "%s is a directory in %s adding as %s instead",
 		       ren1_dst, branch2, dst_name1);
-		remove_file(o, 0, ren1_dst, 0);
+		remove_file(o, 0, ren1_mode, ren1_dst, 0);
 	}
 	if (string_list_has_string(&o->current_directory_set, ren2_dst)) {
 		dst_name2 = del[delp++] = unique_path(o, ren2_dst, branch2);
 		output(o, 1, "%s is a directory in %s adding as %s instead",
 		       ren2_dst, branch1, dst_name2);
-		remove_file(o, 0, ren2_dst, 0);
+		remove_file(o, 0, ren2_mode, ren2_dst, 0);
 	}
 	if (o->call_depth) {
 		remove_file_from_cache(dst_name1);
@@ -773,7 +775,7 @@ static void conflict_rename_dir(struct merge_options *o,
 {
 	char *new_path = unique_path(o, ren1->pair->two->path, branch1);
 	output(o, 1, "Renaming %s to %s instead", ren1->pair->one->path, new_path);
-	remove_file(o, 0, ren1->pair->two->path, 0);
+	remove_file(o, 0, ren1->pair->two->mode, ren1->pair->two->path, 0);
 	update_file(o, 0, ren1->pair->two->sha1, ren1->pair->two->mode, new_path);
 	free(new_path);
 }
@@ -789,7 +791,7 @@ static void conflict_rename_rename_2(struct merge_options *o,
 	output(o, 1, "Renaming %s to %s and %s to %s instead",
 	       ren1->pair->one->path, new_path1,
 	       ren2->pair->one->path, new_path2);
-	remove_file(o, 0, ren1->pair->two->path, 0);
+	remove_file(o, 0, ren1->pair->two->mode, ren1->pair->two->path, 0);
 	update_file(o, 0, ren1->pair->two->sha1, ren1->pair->two->mode, new_path1);
 	update_file(o, 0, ren2->pair->two->sha1, ren2->pair->two->mode, new_path2);
 	free(new_path2);
@@ -821,6 +823,7 @@ static int process_renames(struct merge_options *o,
 		struct rename *ren1 = NULL, *ren2 = NULL;
 		const char *branch1, *branch2;
 		const char *ren1_src, *ren1_dst;
+		unsigned int ren1_srcmode;
 
 		if (i >= a_renames->nr) {
 			ren2 = b_renames->items[j++].util;
@@ -863,6 +866,8 @@ static int process_renames(struct merge_options *o,
 		ren1_src = ren1->pair->one->path;
 		ren1_dst = ren1->pair->two->path;
 
+		ren1_srcmode = ren1->pair->one->mode;
+
 		if (ren2) {
 			const char *ren2_src = ren2->pair->one->path;
 			const char *ren2_dst = ren2->pair->two->path;
@@ -887,7 +892,7 @@ static int process_renames(struct merge_options *o,
 				conflict_rename_rename(o, ren1, branch1, ren2, branch2);
 			} else {
 				struct merge_file_info mfi;
-				remove_file(o, 1, ren1_src, 1);
+				remove_file(o, 1, ren1_srcmode, ren1_src, 1);
 				mfi = merge_file(o,
 						 ren1->pair->one,
 						 ren1->pair->two,
@@ -921,7 +926,7 @@ static int process_renames(struct merge_options *o,
 			struct diff_filespec src_other, dst_other;
 			int try_merge, stage = a_renames == renames1 ? 3: 2;
 
-			remove_file(o, 1, ren1_src, o->call_depth || stage == 3);
+			remove_file(o, 1, ren1_srcmode, ren1_src, o->call_depth || stage == 3);
 
 			hashcpy(src_other.sha1, ren1->src_entry->stages[stage].sha);
 			src_other.mode = ren1->src_entry->stages[stage].mode;
@@ -1077,7 +1082,7 @@ static int process_entry(struct merge_options *o,
 			if (a_sha)
 				output(o, 2, "Removing %s", path);
 			/* do not touch working file if it did not exist */
-			remove_file(o, 1, path, !a_sha);
+			remove_file(o, 1, a_mode, path, !a_sha);
 		} else {
 			/* Deleted in one and changed in the other */
 			clean_merge = 0;
@@ -1124,7 +1129,7 @@ static int process_entry(struct merge_options *o,
 			output(o, 1, "CONFLICT (%s): There is a directory with name %s in %s. "
 			       "Adding %s as %s",
 			       conf, path, other_branch, path, new_path);
-			remove_file(o, 0, path, 0);
+			remove_file(o, 0, mode, path, 0);
 			update_file(o, 0, sha, mode, new_path);
 		} else {
 			output(o, 2, "Adding %s", path);
@@ -1166,7 +1171,7 @@ static int process_entry(struct merge_options *o,
 		 * this entry was deleted altogether. a_mode == 0 means
 		 * we had that path and want to actively remove it.
 		 */
-		remove_file(o, 1, path, !a_mode);
+		remove_file(o, 1, a_mode, path, !a_mode);
 	} else
 		die("Fatal merge failure, shouldn't happen.");
 
diff --git a/t/t7405-submodule-merge.sh b/t/t7405-submodule-merge.sh
index 9a21f78..d87ed9e 100755
--- a/t/t7405-submodule-merge.sh
+++ b/t/t7405-submodule-merge.sh
@@ -71,4 +71,17 @@ test_expect_success 'merging with a modify/modify conflict between merge bases'
 
 '
 
+test_expect_success 'merging a submodule deletion' '
+
+	git reset --hard HEAD &&
+	git checkout -b test3 a &&
+	rm -rf sub &&
+	git update-index --remove sub &&
+	git commit -m empty &&
+	git checkout -b test4 c &&
+	test -d sub &&
+	git merge test3 &&
+	test \! -d sub
+'
+
 test_done
-- 
1.6.5

[PATCH 10/12] git rm: do not abort due to an initialised submodule

[Peter Collingbourne]

This patch causes the "git rm" command to consider "directory not
empty" errors as nonfatal, which will be caused by a submodule being
in an initialised state.  As this is a normal state for a submodule,
it should not cause us to abort.  Neither should we recursively delete
the submodule directory as it may contain unsaved data.

Signed-off-by: Peter Collingbourne <peter@pcc.me.uk>
---
 builtin/rm.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/builtin/rm.c b/builtin/rm.c
index 6ac5114..02ee259 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -250,7 +250,9 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 	 * abort the "git rm" (but once we've successfully removed
 	 * any file at all, we'll go ahead and commit to it all:
 	 * by then we've already committed ourselves and can't fail
-	 * in the middle)
+	 * in the middle).  However failure to remove a submodule
+	 * directory due to the submodule being initialised is never
+	 * a fatal condition.
 	 */
 	if (!index_only) {
 		int removed = 0;
@@ -261,7 +263,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 				removed = 1;
 				continue;
 			}
-			if (!removed)
+			if (!removed && errno != EEXIST && errno != ENOTEMPTY)
 				die_errno("git rm: '%s'", path);
 			else
 				warning("git rm: '%s': %s", path, strerror(errno));
-- 
1.6.5

[PATCH 11/12] git submodule: infrastructure for reading .gitmodules files in arbitrary locations

[Peter Collingbourne]

This patch modifies the module_name function in git-submodule.sh to take
an optional parameter which specifies the path of the .gitmodules
file to read.

Signed-off-by: Peter Collingbourne <peter@pcc.me.uk>
---
 git-submodule.sh |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index d0b7a79..3fd067a 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -64,12 +64,18 @@ module_list()
 # Map submodule path to submodule name
 #
 # $1 = path
+# $2 = .gitmodules file, default ".gitmodules"
 #
 module_name()
 {
+	modfile="$2"
+	if test -z "$modfile"
+	then
+		modfile=".gitmodules"
+	fi
 	# Do we have "submodule.<something>.path = $1" defined in .gitmodules file?
 	re=$(printf '%s\n' "$1" | sed -e 's/[].[^$\\*]/\\&/g')
-	name=$( git config -f .gitmodules --get-regexp '^submodule\..*\.path$' |
+	name=$( git config -f "$modfile" --get-regexp '^submodule\..*\.path$' |
 		sed -n -e 's|^submodule\.\(.*\)\.path '"$re"'$|\1|p' )
        test -z "$name" &&
        die "No submodule mapping found in .gitmodules for path '$path'"
-- 
1.6.5

[PATCH 12/12] git rm: remove submodule entries from .gitmodules

[Peter Collingbourne]

This patch teaches "git rm" how to remove submodules from the
.gitmodules file.  The .gitmodules update is handled by an undocumented
subcommand to "git submodule" named "rmconfig".

Signed-off-by: Peter Collingbourne <peter@pcc.me.uk>
---
 Documentation/git-rm.txt                           |    5 ++-
 builtin/rm.c                                       |   25 ++++++++++-
 git-submodule.sh                                   |   45 +++++++++++++++++++-
 ...09-submodule-mv.sh => t7409-submodule-mv-rm.sh} |   15 ++++++-
 4 files changed, 85 insertions(+), 5 deletions(-)
 rename t/{t7409-submodule-mv.sh => t7409-submodule-mv-rm.sh} (82%)

diff --git a/Documentation/git-rm.txt b/Documentation/git-rm.txt
index c21d19e..81c1bbd 100644
--- a/Documentation/git-rm.txt
+++ b/Documentation/git-rm.txt
@@ -7,7 +7,7 @@ git-rm - Remove files from the working tree and from the index
 
 SYNOPSIS
 --------
-'git rm' [-f | --force] [-n] [-r] [--cached] [--ignore-unmatch] [--quiet] [--] <file>...
+'git rm' [-f | --force] [-n] [-r] [-M] [--cached] [--ignore-unmatch] [--quiet] [--] <file>...
 
 DESCRIPTION
 -----------
@@ -49,6 +49,9 @@ OPTIONS
         Allow recursive removal when a leading directory name is
         given.
 
+-M::
+	Do not try to remove submodule entry in .gitmodules
+
 \--::
 	This option can be used to separate command-line options from
 	the list of files, (useful when filenames might be mistaken
diff --git a/builtin/rm.c b/builtin/rm.c
index 02ee259..3c26a43 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -9,6 +9,7 @@
 #include "cache-tree.h"
 #include "tree-walk.h"
 #include "parse-options.h"
+#include "run-command.h"
 
 static const char * const builtin_rm_usage[] = {
 	"git rm [options] [--] <file>...",
@@ -139,7 +140,7 @@ static int check_local_mod(unsigned char *head, int index_only)
 static struct lock_file lock_file;
 
 static int show_only = 0, force = 0, index_only = 0, recursive = 0, quiet = 0;
-static int ignore_unmatch = 0;
+static int ignore_unmatch = 0, skip_module_update = 0;
 
 static struct option builtin_rm_options[] = {
 	OPT__DRY_RUN(&show_only),
@@ -149,6 +150,8 @@ static struct option builtin_rm_options[] = {
 	OPT_BOOLEAN('r', NULL,             &recursive,  "allow recursive removal"),
 	OPT_BOOLEAN( 0 , "ignore-unmatch", &ignore_unmatch,
 				"exit with a zero status even if nothing matched"),
+	OPT_BOOLEAN('M', NULL,             &skip_module_update,
+				"don't update submodule entries"),
 	OPT_END(),
 };
 
@@ -276,5 +279,25 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 			die("Unable to write new index file");
 	}
 
+	if (!skip_module_update)
+		for (i = 0; i < list.nr; i++) {
+			if (S_ISGITLINK(list.mode[i])) {
+				const char *path = list.name[i];
+
+				const char *argv_submodule[] = {
+					"submodule", "rmconfig", NULL, NULL, NULL, NULL
+				};
+				int argc = 2;
+
+				if (index_only)
+					argv_submodule[argc++] = "--cached";
+
+				argv_submodule[argc++] = "--";
+				argv_submodule[argc++] = path;
+
+				run_command_v_opt(argv_submodule, RUN_GIT_CMD);
+			}
+		}
+
 	return 0;
 }
diff --git a/git-submodule.sh b/git-submodule.sh
index 3fd067a..6d9c08b 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -876,6 +876,49 @@ cmd_mvconfig()
 		die "Could not update .gitmodules entry for $name"
 	git add .gitmodules || die "Could not add .gitmodules to index"
 }
+#
+# Removes the entry in .gitmodules to remove a submodule.
+# This command is called by "git rm" for each submodule it removes.
+#
+cmd_rmconfig()
+{
+	while test $# -ne 0
+	do
+		case "$1" in
+		--cached)
+			index_only=1
+			shift
+			;;
+		--)
+			shift
+			break
+			;;
+		*)
+			break
+			;;
+		esac
+	done
+	path="$1"
+
+	if test -z "$index_only"
+	then
+		name=$(module_name "$path") || exit
+		git config -f .gitmodules --remove-section submodule."$name" ||
+			die "Could not update .gitmodules entry for $name"
+		git add .gitmodules || die "Could not add .gitmodules to index"
+	else
+		git cat-file -p :0:.gitmodules > .git/gitmodules.index ||
+			{ rm .git/gitmodules.index; die "Could not extract .gitmodules from index"; }
+		name=$(module_name "$path" .git/gitmodules.index) || { rm .git/gitmodules.index; exit; }
+		git config -f .git/gitmodules.index --remove-section submodule."$name" ||
+			{ rm .git/gitmodules.index; die "Could not update .gitmodules entry for $name"; }
+		blob=$(git hash-object -w --stdin < .git/gitmodules.index) ||
+			{ rm .git/gitmodules.index; die "Could not create blob for .gitmodules"; }
+		rm .git/gitmodules.index || die "Could not remove temporary .gitmodules file"
+		git update-index --cacheinfo 100644 "$blob" .gitmodules ||
+			die "Could not add .gitmodules to index"
+	fi
+}
 
 # This loop parses the command line arguments to find the
 # subcommand name to dispatch.  Parsing of the subcommand specific
@@ -886,7 +929,7 @@ cmd_mvconfig()
 while test $# != 0 && test -z "$command"
 do
 	case "$1" in
-	add | foreach | init | update | status | summary | sync | mvconfig)
+	add | foreach | init | update | status | summary | sync | mvconfig | rmconfig)
 		command=$1
 		;;
 	-q|--quiet)
diff --git a/t/t7409-submodule-mv.sh b/t/t7409-submodule-mv-rm.sh
similarity index 82%
rename from t/t7409-submodule-mv.sh
rename to t/t7409-submodule-mv-rm.sh
index 9eb3fb1..91b7866 100755
--- a/t/t7409-submodule-mv.sh
+++ b/t/t7409-submodule-mv-rm.sh
@@ -3,9 +3,9 @@
 # Copyright (c) 2010 Peter Collingbourne
 #
 
-test_description='git submodule mv
+test_description='git submodule mv, rm
 
-These tests exercise the "git mv" command for submodules.
+These tests exercise the "git mv" and "git rm" commands for submodules.
 '
 
 . ./test-lib.sh
@@ -91,4 +91,15 @@ test_expect_success 'move multiple submodules at once' '
 	)
 '
 
+test_expect_success 'remove multiple submodules at once' '
+	(cd super &&
+	 git rm -r test\ dir &&
+	 test ! -d test\ dir/unreg && 
+	 test -d test\ dir/reg2 && 
+	 test -z "$(git ls-files test\ dir/unreg)" &&
+	 test -z "$(git ls-files test\ dir/reg2)" &&
+	 test -z "$(git config -f .gitmodules submodule.reg.path)"
+	)
+'
+
 test_done
-- 
1.6.5