[PATCH v2 0/9] Improve handling of moving and removing submodules

[PATCH v2 0/9] Improve handling of moving and removing submodules

[Peter Collingbourne]

Changes since v1:
 - Generate a more user friendly unique ID
 - Added tests for "git rm" error cases (thanks Jonathan Nieder)

Peter Collingbourne (9):
      Generate unique ID for submodules created using "git submodule add"
      Implement "git mv" for submodules
      git rm: test failure behaviour for multiple removals
      git rm: display a warning for every unremovable file
      git rm: collect file modes
      Add a mode parameter to the remove_path function
      git rm: do not abort due to an initialised submodule
      git submodule: infrastructure for reading .gitmodules files in arbitrary locations
      git rm: remove submodule entries from .gitmodules

 Documentation/git-mv.txt        |    7 ++-
 Documentation/git-rm.txt        |    5 ++-
 Documentation/git-submodule.txt |    8 +++-
 builtin/apply.c                 |    2 +-
 builtin/mv.c                    |   33 +++++++++++--
 builtin/rm.c                    |   45 ++++++++++++++---
 dir.c                           |    4 +-
 dir.h                           |    2 +-
 git-submodule.sh                |   98 ++++++++++++++++++++++++++++++++++--
 merge-recursive.c               |   27 ++++++----
 t/t3600-rm.sh                   |   69 +++++++++++++++++++++++++
 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 +++++++++++++++++++++++++++++++++++++++
 16 files changed, 395 insertions(+), 45 deletions(-)

[PATCH v2 1/9] 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 formed by
concatenating the basename of the submodule path, a dash ("-")
character and the 7-character truncated 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.

The justification for truncating the SHA1 to 7 characters 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.

The justification for including the submodule path basename is that 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>
---
 Documentation/git-submodule.txt |    8 +++++++-
 git-submodule.sh                |   31 +++++++++++++++++++++++++++++--
 t/t7403-submodule-sync.sh       |    2 +-
 t/t7406-submodule-update.sh     |    6 +++---
 t/t7407-submodule-foreach.sh    |   14 +++++++-------
 5 files changed, 47 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 187461c..de29f3a 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -114,6 +114,23 @@ module_clone()
 }
 
 #
+# Generate a unique identifier.  Used to name a submodule.
+#
+gen_uid()
+{
+	path="$1"
+
+	pathbase=$(basename "$path")
+	echo -n "$pathbase"-
+
+	(
+	 echo "$path"
+	 echo $$
+	 date
+	) | git hash-object --stdin | cut -c1-7
+}
+
+#
 # Add a new submodule to the working tree, .gitmodules and the index
 #
 # $@ = repo path
@@ -131,6 +148,11 @@ cmd_add()
 			branch=$2
 			shift
 			;;
+		-n | --name)
+			case "$2" in '') usage ;; esac
+			name=$2
+			shift
+			;;
 		-q|--quiet)
 			GIT_QUIET=1
 			;;
@@ -235,8 +257,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

[PATCH v2 2/9] 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 de29f3a..f1e4e22 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -859,6 +859,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
@@ -869,7 +883,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 v2 3/9] git rm: test failure behaviour for multiple removals

[Peter Collingbourne]

This patch causes the failure cases for the "git rm" command
to be tested.  Specifically it tests that if the first removal
fails the operation is aborted with an error message, and that
if subsequent removals fail, the operation proceeds.

Based-on-work-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Peter Collingbourne <peter@pcc.me.uk>
---
 t/t3600-rm.sh |   54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 54 insertions(+), 0 deletions(-)

diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index 0aaf0ad..5186844 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -126,6 +126,60 @@ 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 '"rm" command printed' '
 	echo frotz > test-file &&
 	git add test-file &&
-- 
1.6.5

[PATCH v2 4/9] git rm: display a warning for every unremovable file

[Peter Collingbourne]

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 the rmdir operation will fail for every initialised submodule.
The removal of the contents of an initialised submodule directory
should always be user controlled, due to the possibility of
unpropagated changes to the submodule.  Therefore, the user should
always be informed of any such removal failures.

Based-on-work-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Peter Collingbourne <peter@pcc.me.uk>
---
 builtin/rm.c  |    2 ++
 t/t3600-rm.sh |   15 +++++++++++++++
 2 files changed, 17 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));
 		}
 	}
 
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index 5186844..ecddd67 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -180,6 +180,21 @@ test_expect_success 'Message when first removal fails' '
 ! 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 &&
-- 
1.6.5

[PATCH v2 5/9] 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 v2 6/9] 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 d56cabf..2f50a53 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3139,7 +3139,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 206c103..f4ac8c9 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;
@@ -739,6 +739,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;
@@ -747,13 +749,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);
@@ -778,7 +780,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);
 }
@@ -794,7 +796,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);
@@ -826,6 +828,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;
@@ -868,6 +871,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;
@@ -892,7 +897,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,
@@ -926,7 +931,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;
@@ -1082,7 +1087,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;
@@ -1129,7 +1134,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);
@@ -1171,7 +1176,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 v2 7/9] 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 v2 8/9] 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 f1e4e22..75c50b8 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 v2 9/9] 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 ++++++++++++++++++-
 t/t7409-submodule-mv-rm.sh |  105 ++++++++++++++++++++++++++++++++++++++++++++
 t/t7409-submodule-mv.sh    |   94 ---------------------------------------
 5 files changed, 177 insertions(+), 97 deletions(-)
 create mode 100755 t/t7409-submodule-mv-rm.sh
 delete mode 100755 t/t7409-submodule-mv.sh

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 75c50b8..baadaa5 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -879,6 +879,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
@@ -889,7 +932,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-rm.sh b/t/t7409-submodule-mv-rm.sh
new file mode 100755
index 0000000..91b7866
--- /dev/null
+++ b/t/t7409-submodule-mv-rm.sh
@@ -0,0 +1,105 @@
+#!/bin/sh
+#
+# Copyright (c) 2010 Peter Collingbourne
+#
+
+test_description='git submodule mv, rm
+
+These tests exercise the "git mv" and "git rm" commands 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_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
diff --git a/t/t7409-submodule-mv.sh b/t/t7409-submodule-mv.sh
deleted file mode 100755
index 9eb3fb1..0000000
--- a/t/t7409-submodule-mv.sh
+++ /dev/null
@@ -1,94 +0,0 @@
-#!/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

Re: [PATCH v2 1/9] Generate unique ID for submodules created using "git submodule add"

[Sverre Rabbelier]

Heya,

On Sat, Apr 10, 2010 at 20:23, Peter Collingbourne <peter@pcc.me.uk> wrote:
> ---

I think this commit message would benefit from having an example of
such a ID in it. I think I understand the idea, but an example would
help me to verify that the idea I have is correct.

-- 
Cheers,

Sverre Rabbelier

Re: [PATCH v2 6/9] Add a mode parameter to the remove_path function

[Junio C Hamano]

Peter Collingbourne <peter@pcc.me.uk> writes:

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

When the mode of the thing on the filesystem doesn't match with what your
callers expect (e.g. the caller "merge-recursive" thought there should be
a gitlink but the filesystem actually had a plain file there), what should
happen?  What happens with your patch?

Re: [PATCH v2 7/9] git rm: do not abort due to an initialised submodule

[Junio C Hamano]

Peter Collingbourne <peter@pcc.me.uk> writes:

> 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,
> ...
> 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.
>  	 */


Your messages both in the commit log and comment talk only about
submodules, ...

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

... but the code does not seem to limit itself to the case where a
submodule removal has failed.

How does this patch affect the failure case for regular files and
directories without any submodules?

Re: [PATCH v2 2/9] Implement "git mv" for submodules

[Junio C Hamano]

Peter Collingbourne <peter@pcc.me.uk> writes:

> +		OPT_BOOLEAN('M', NULL, &skip_module_update, "don't upda...

If you are moving them by default, it is confusing to make up a negative
"skip" option like this.

Instead, add a boolean "module_update" that defaults to true, and give
"--[no-]module-update" without a short single-letter option, i.e.

	OPT_BOOLEAN(0 , "module-update", &module_update, "update submodule entries")

Exactly the same comment applies to your "git rm" patch.

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

This looks like a funny indentation (perhaps a "diff" artifcat???)...

> diff --git a/git-submodule.sh b/git-submodule.sh
> index de29f3a..f1e4e22 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -859,6 +859,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 does not seem to depend on the first "give random name to submodules"
patch in the series, or does it?