{"thread":{"id":"15505","subject":"[PATCH 0/6] Submodule support for git mv and git rm","startedAt":"2008-09-12T21:08:51Z","lastAt":"2008-09-13T11:32:38Z","messageCount":18,"participants":["Petr Baudis","Junio C Hamano","Jakub Narebski","Lars Hjemli"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"90594","messageId":"20080912210817.31628.69014.stgit@localhost","threadId":"15505","inReplyTo":null,"subject":"[PATCH 0/6] Submodule support for git mv and git rm","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2008-09-12T21:08:51Z","receivedAt":"2008-09-12T21:08:51Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"This is a resend of the previously submitted series, with basically\nno changes since the previous submission.\n\n---\n\nPetr Baudis (6):\n      git submodule add: Fix naming clash handling\n      t7400: Add short \"git submodule add\" testsuite\n      t7403: Submodule git mv, git rm testsuite\n      git mv: Support moving submodules\n      git rm: Support for removing submodules\n      submodule.*: Introduce simple C interface for submodule lookup by path\n\n\n Documentation/git-rm.txt   |    6 +\n Makefile                   |    2 \n builtin-mv.c               |   37 ++++++-\n builtin-rm.c               |   65 ++++++++++--\n git-submodule.sh           |   15 ++-\n submodule.c                |   50 +++++++++\n submodule.h                |    8 +\n t/t7400-submodule-basic.sh |   39 +++++++\n t/t7403-submodule-mvrm.sh  |  242 ++++++++++++++++++++++++++++++++++++++++++++\n 9 files changed, 443 insertions(+), 21 deletions(-)\n create mode 100644 submodule.c\n create mode 100644 submodule.h\n create mode 100755 t/t7403-submodule-mvrm.sh\n"},{"id":"90595","messageId":"20080912210857.31628.7605.stgit@localhost","threadId":"15505","inReplyTo":"20080912210817.31628.69014.stgit@localhost","subject":"[PATCH 1/6] submodule.*: Introduce simple C interface for submodule lookup by path","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2008-09-12T21:08:57Z","receivedAt":"2008-09-12T21:08:57Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"The interface will be used for git-mv and git-rm submodule support.\nSo far, only the submodule_by_path() function is defined, however more\ncan be probably expected in the future if/when the git-submodule command\nis ported from shell.\n\nSigned-off-by: Petr Baudis <pasky@suse.cz>\n---\n\n Makefile    |    2 ++\n submodule.c |   50 ++++++++++++++++++++++++++++++++++++++++++++++++++\n submodule.h |    8 ++++++++\n 3 files changed, 60 insertions(+), 0 deletions(-)\n create mode 100644 submodule.c\n create mode 100644 submodule.h\n\ndiff --git a/Makefile b/Makefile\nindex 247cd2d..c7c6478 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -384,6 +384,7 @@ LIB_H += run-command.h\n LIB_H += sha1-lookup.h\n LIB_H += sideband.h\n LIB_H += strbuf.h\n+LIB_H += submodule.h\n LIB_H += tag.h\n LIB_H += transport.h\n LIB_H += tree.h\n@@ -476,6 +477,7 @@ LIB_OBJS += sha1_name.o\n LIB_OBJS += shallow.o\n LIB_OBJS += sideband.o\n LIB_OBJS += strbuf.o\n+LIB_OBJS += submodule.o\n LIB_OBJS += symlinks.o\n LIB_OBJS += tag.o\n LIB_OBJS += trace.o\ndiff --git a/submodule.c b/submodule.c\nnew file mode 100644\nindex 0000000..2883ae6\n--- /dev/null\n+++ b/submodule.c\n@@ -0,0 +1,50 @@\n+#include \"cache.h\"\n+#include \"submodule.h\"\n+\n+\n+struct gitmodules_info {\n+\tconst char *path;\n+\tchar *key;\n+};\n+\n+static int gitmodules_worker(const char *key, const char *value, void *info_)\n+{\n+\tstruct gitmodules_info *info = info_;\n+\tconst char *subkey;\n+\n+\tif (prefixcmp(key, \"submodule.\"))\n+\t\treturn 0;\n+\n+\tsubkey = strrchr(key, '.');\n+\tif (!subkey)\n+\t\treturn 0;\n+\n+\tif (strcmp(subkey, \".path\"))\n+\t\treturn 0;\n+\n+\tif (strcmp(value, info->path))\n+\t\treturn 0;\n+\n+\t/* Found the key to change. */\n+\tif (info->key) {\n+\t\terror(\"multiple submodules live at path `%s'\", info->path);\n+\t\t/* The last one is supposed to win. */\n+\t\tfree(info->key);\n+\t}\n+\tinfo->key = xstrdup(key);\n+\treturn 0;\n+}\n+\n+char *submodule_by_path(const char *path)\n+{\n+\tstruct gitmodules_info info = { path, NULL };\n+\n+\tconfig_exclusive_filename = \".gitmodules\";\n+\tif (git_config(gitmodules_worker, &info))\n+\t\tdie(\"cannot process .gitmodules\");\n+\tif (!info.key)\n+\t\tdie(\"the submodule of `%s' not found in .gitmodules\", path);\n+\tconfig_exclusive_filename = NULL;\n+\n+\treturn info.key;\n+}\ndiff --git a/submodule.h b/submodule.h\nnew file mode 100644\nindex 0000000..bc74fa0\n--- /dev/null\n+++ b/submodule.h\n@@ -0,0 +1,8 @@\n+#ifndef SUBMODULE_H\n+#define SUBMODULE_H\n+\n+/* Find submodule living at given path in .gitmodules and return the key\n+ * of its path config variable (dynamically allocated). */\n+extern char *submodule_by_path(const char *path);\n+\n+#endif\n"},{"id":"90596","messageId":"20080912210902.31628.7325.stgit@localhost","threadId":"15505","inReplyTo":"20080912210817.31628.69014.stgit@localhost","subject":"[PATCH 2/6] git rm: Support for removing submodules","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2008-09-12T21:09:02Z","receivedAt":"2008-09-12T21:09:02Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"This patch adds support for removing submodules to 'git rm', including\nappropriately editing the .gitmodules file to reflect this. Submodule\n_checkouts_ are never removed; that can be potentially catastrophic\nand the user should remove them manually, if really desired.\n\nSigned-off-by: Petr Baudis <pasky@suse.cz>\n---\n\n Documentation/git-rm.txt |    6 +++-\n builtin-rm.c             |   65 ++++++++++++++++++++++++++++++++++++++--------\n 2 files changed, 58 insertions(+), 13 deletions(-)\n\ndiff --git a/Documentation/git-rm.txt b/Documentation/git-rm.txt\nindex 5afb1e7..0c92687 100644\n--- a/Documentation/git-rm.txt\n+++ b/Documentation/git-rm.txt\n@@ -20,7 +20,8 @@ and no updates to their contents can be staged in the index,\n though that default behavior can be overridden with the `-f` option.\n When '--cached' is given, the staged content has to\n match either the tip of the branch or the file on disk,\n-allowing the file to be removed from just the index.\n+allowing the file to be removed from just the index;\n+this is always the case when removing submodules.\n \n \n OPTIONS\n@@ -57,7 +58,8 @@ OPTIONS\n --cached::\n \tUse this option to unstage and remove paths only from the index.\n \tWorking tree files, whether modified or not, will be\n-\tleft alone.\n+\tleft alone.  Note that this is always assumed when removing\n+\ta checked-out submodule.\n \n --ignore-unmatch::\n \tExit with a zero status even if no files matched.\ndiff --git a/builtin-rm.c b/builtin-rm.c\nindex 6bd8211..7475de2 100644\n--- a/builtin-rm.c\n+++ b/builtin-rm.c\n@@ -9,6 +9,7 @@\n #include \"cache-tree.h\"\n #include \"tree-walk.h\"\n #include \"parse-options.h\"\n+#include \"submodule.h\"\n \n static const char * const builtin_rm_usage[] = {\n \t\"git rm [options] [--] <file>...\",\n@@ -17,16 +18,21 @@ static const char * const builtin_rm_usage[] = {\n \n static struct {\n \tint nr, alloc;\n-\tconst char **name;\n+\tstruct {\n+\t\tconst char *name;\n+\t\tint is_gitlink;\n+\t} *info;\n } list;\n \n-static void add_list(const char *name)\n+static void add_list(const char *name, int is_gitlink)\n {\n \tif (list.nr >= list.alloc) {\n \t\tlist.alloc = alloc_nr(list.alloc);\n-\t\tlist.name = xrealloc(list.name, list.alloc * sizeof(const char *));\n+\t\tlist.info = xrealloc(list.info, list.alloc * sizeof(*list.info));\n \t}\n-\tlist.name[list.nr++] = name;\n+\tlist.info[list.nr].name = name;\n+\tlist.info[list.nr].is_gitlink = is_gitlink;\n+\tlist.nr++;\n }\n \n static int remove_file(const char *name)\n@@ -38,6 +44,13 @@ static int remove_file(const char *name)\n \tif (ret && errno == ENOENT)\n \t\t/* The user has removed it from the filesystem by hand */\n \t\tret = errno = 0;\n+\tif (ret && errno == EISDIR) {\n+\t\t/* This is a gitlink entry; try to remove at least the\n+\t\t * directory if the submodule is not checked out; we always\n+\t\t * leave the checked out ones as they are */\n+\t\tif (!rmdir(name) || errno == ENOTEMPTY)\n+\t\t\tret = errno = 0;\n+\t}\n \n \tif (!ret && (slash = strrchr(name, '/'))) {\n \t\tchar *n = xstrdup(name);\n@@ -65,7 +78,7 @@ static int check_local_mod(unsigned char *head, int index_only)\n \t\tstruct stat st;\n \t\tint pos;\n \t\tstruct cache_entry *ce;\n-\t\tconst char *name = list.name[i];\n+\t\tconst char *name = list.info[i].name;\n \t\tunsigned char sha1[20];\n \t\tunsigned mode;\n \t\tint local_changes = 0;\n@@ -83,7 +96,7 @@ static int check_local_mod(unsigned char *head, int index_only)\n \t\t\t/* It already vanished from the working tree */\n \t\t\tcontinue;\n \t\t}\n-\t\telse if (S_ISDIR(st.st_mode)) {\n+\t\telse if (S_ISDIR(st.st_mode) && !S_ISGITLINK(ce->ce_mode)) {\n \t\t\t/* if a file was removed and it is now a\n \t\t\t * directory, that is the same as ENOENT as\n \t\t\t * far as git is concerned; we do not track\n@@ -122,6 +135,22 @@ static int check_local_mod(unsigned char *head, int index_only)\n \treturn errs;\n }\n \n+static void remove_submodule(const char *name)\n+{\n+\tchar *key = submodule_by_path(name);\n+\tchar *sectend = strrchr(key, '.');\n+\n+\tassert(sectend);\n+\t*sectend = 0;\n+\n+\tconfig_exclusive_filename = \".gitmodules\";\n+\tif (git_config_rename_section(key, NULL) <= 0)\n+\t\tdie(\"cannot remove section `%s' from .gitmodules\", key);\n+\tconfig_exclusive_filename = NULL;\n+\n+\tfree(key);\n+}\n+\n static struct lock_file lock_file;\n \n static int show_only = 0, force = 0, index_only = 0, recursive = 0, quiet = 0;\n@@ -140,7 +169,7 @@ static struct option builtin_rm_options[] = {\n \n int cmd_rm(int argc, const char **argv, const char *prefix)\n {\n-\tint i, newfd;\n+\tint i, newfd, subs;\n \tconst char **pathspec;\n \tchar *seen;\n \n@@ -168,7 +197,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)\n \t\tstruct cache_entry *ce = active_cache[i];\n \t\tif (!match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, seen))\n \t\t\tcontinue;\n-\t\tadd_list(ce->name);\n+\t\tadd_list(ce->name, S_ISGITLINK(ce->ce_mode));\n \t}\n \n \tif (pathspec) {\n@@ -216,9 +245,11 @@ int cmd_rm(int argc, const char **argv, const char *prefix)\n \t * the index unless all of them succeed.\n \t */\n \tfor (i = 0; i < list.nr; i++) {\n-\t\tconst char *path = list.name[i];\n+\t\tconst char *path = list.info[i].name;\n \t\tif (!quiet)\n-\t\t\tprintf(\"rm '%s'\\n\", path);\n+\t\t\tprintf(\"rm%s '%s'\\n\",\n+\t\t\t\tlist.info[i].is_gitlink ? \"dir\" : \"\",\n+\t\t\t\tpath);\n \n \t\tif (remove_file_from_cache(path))\n \t\t\tdie(\"git rm: unable to remove %s\", path);\n@@ -238,7 +269,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)\n \tif (!index_only) {\n \t\tint removed = 0;\n \t\tfor (i = 0; i < list.nr; i++) {\n-\t\t\tconst char *path = list.name[i];\n+\t\t\tconst char *path = list.info[i].name;\n \t\t\tif (!remove_file(path)) {\n \t\t\t\tremoved = 1;\n \t\t\t\tcontinue;\n@@ -248,6 +279,18 @@ int cmd_rm(int argc, const char **argv, const char *prefix)\n \t\t}\n \t}\n \n+\t/*\n+\t * Get rid of stale submodule setup.\n+\t */\n+\tsubs = 0;\n+\tfor (i = 0; i < list.nr; i++)\n+\t\tif (list.info[i].is_gitlink) {\n+\t\t\tremove_submodule(list.info[i].name);\n+\t\t\tsubs++;\n+\t\t}\n+\tif (subs && add_file_to_cache(\".gitmodules\", 0))\n+\t\tdie(\"cannot add new .gitmodules to the index\");\n+\n \tif (active_cache_changed) {\n \t\tif (write_cache(newfd, active_cache, active_nr) ||\n \t\t    commit_locked_index(&lock_file))\n"},{"id":"90597","messageId":"20080912210908.31628.50439.stgit@localhost","threadId":"15505","inReplyTo":"20080912210817.31628.69014.stgit@localhost","subject":"[PATCH 3/6] git mv: Support moving submodules","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2008-09-12T21:09:08Z","receivedAt":"2008-09-12T21:09:08Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"This patch adds support for moving submodules to 'git mv', including\nrewriting of the .gitmodules file to reflect the movement.\n\nSigned-off-by: Petr Baudis <pasky@suse.cz>\n---\n\n builtin-mv.c |   37 +++++++++++++++++++++++++++++++++----\n 1 files changed, 33 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin-mv.c b/builtin-mv.c\nindex 4f65b5a..52e3ea5 100644\n--- a/builtin-mv.c\n+++ b/builtin-mv.c\n@@ -9,6 +9,7 @@\n #include \"cache-tree.h\"\n #include \"string-list.h\"\n #include \"parse-options.h\"\n+#include \"submodule.h\"\n \n static const char * const builtin_mv_usage[] = {\n \t\"git mv [options] <source>... <destination>\",\n@@ -49,6 +50,24 @@ static const char *add_slash(const char *path)\n \treturn path;\n }\n \n+static int ce_is_gitlink(int i)\n+{\n+\treturn i < 0 ? 0 : S_ISGITLINK(active_cache[i]->ce_mode);\n+}\n+\n+static void rename_submodule(struct string_list_item *i)\n+{\n+\tchar *key = submodule_by_path(i->path);\n+\n+\tconfig_exclusive_filename = \".gitmodules\";\n+\tif (git_config_set(key, (const char *) i->util))\n+\t\tdie(\"cannot update .gitmodules\");\n+\tconfig_exclusive_filename = NULL;\n+\n+\tfree(key);\n+}\n+\n+\n static struct lock_file lock_file;\n \n int cmd_mv(int argc, const char **argv, const char *prefix)\n@@ -65,6 +84,8 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n \tenum update_mode { BOTH = 0, WORKING_DIRECTORY, INDEX } *modes;\n \tstruct stat st;\n \tstruct string_list src_for_dst = {NULL, 0, 0, 0};\n+\t/* .path is source path, .util is destination path */\n+\tstruct string_list submodules = {NULL, 0, 0, 0};\n \n \tgit_config(git_default_config, NULL);\n \n@@ -84,7 +105,8 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n \t\t/* special case: \".\" was normalized to \"\" */\n \t\tdestination = copy_pathspec(dest_path[0], argv, argc, 1);\n \telse if (!lstat(dest_path[0], &st) &&\n-\t\t\tS_ISDIR(st.st_mode)) {\n+\t\t\tS_ISDIR(st.st_mode) &&\n+\t\t\t!ce_is_gitlink(cache_name_pos(dest_path[0], strlen(dest_path[0])))) {\n \t\tdest_path[0] = add_slash(dest_path[0]);\n \t\tdestination = copy_pathspec(dest_path[0], argv, argc, 1);\n \t} else {\n@@ -96,7 +118,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n \t/* Checking */\n \tfor (i = 0; i < argc; i++) {\n \t\tconst char *src = source[i], *dst = destination[i];\n-\t\tint length, src_is_dir;\n+\t\tint length, src_is_dir, src_cache_pos;\n \t\tconst char *bad = NULL;\n \n \t\tif (show_only)\n@@ -111,7 +133,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n \t\t} else if ((src_is_dir = S_ISDIR(st.st_mode))\n \t\t\t\t&& lstat(dst, &st) == 0)\n \t\t\tbad = \"cannot move directory over file\";\n-\t\telse if (src_is_dir) {\n+\t\telse if ((src_cache_pos = cache_name_pos(src, length)) < 0 && src_is_dir) {\n \t\t\tconst char *src_w_slash = add_slash(src);\n \t\t\tint len_w_slash = length + 1;\n \t\t\tint first, last;\n@@ -177,7 +199,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n \t\t\t\t} else\n \t\t\t\t\tbad = \"Cannot overwrite\";\n \t\t\t}\n-\t\t} else if (cache_name_pos(src, length) < 0)\n+\t\t} else if (src_cache_pos < 0)\n \t\t\tbad = \"not under version control\";\n \t\telse if (string_list_has_string(&src_for_dst, dst))\n \t\t\tbad = \"multiple sources for the same target\";\n@@ -214,10 +236,17 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n \n \t\tpos = cache_name_pos(src, strlen(src));\n \t\tassert(pos >= 0);\n+\t\tif (ce_is_gitlink(pos))\n+\t\t\tstring_list_insert(src, &submodules)->util = (void *) dst;\n \t\tif (!show_only)\n \t\t\trename_cache_entry_at(pos, dst);\n \t}\n \n+\tfor (i = 0; i < submodules.nr; i++)\n+\t\trename_submodule(&submodules.items[i]);\n+\tif (submodules.nr > 0 && add_file_to_cache(\".gitmodules\", 0))\n+\t\tdie(\"cannot add new .gitmodules to the index\");\n+\n \tif (active_cache_changed) {\n \t\tif (write_cache(newfd, active_cache, active_nr) ||\n \t\t    commit_locked_index(&lock_file))\n"},{"id":"90598","messageId":"20080912210913.31628.20562.stgit@localhost","threadId":"15505","inReplyTo":"20080912210817.31628.69014.stgit@localhost","subject":"[PATCH 4/6] t7403: Submodule git mv, git rm testsuite","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2008-09-12T21:09:13Z","receivedAt":"2008-09-12T21:09:13Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"The testsuite for newly added submodule support in git mv, git rm.\n\nSigned-off-by: Petr Baudis <pasky@suse.cz>\n---\n\n t/t7403-submodule-mvrm.sh |  242 +++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 242 insertions(+), 0 deletions(-)\n create mode 100755 t/t7403-submodule-mvrm.sh\n\ndiff --git a/t/t7403-submodule-mvrm.sh b/t/t7403-submodule-mvrm.sh\nnew file mode 100755\nindex 0000000..9b50d6a\n--- /dev/null\n+++ b/t/t7403-submodule-mvrm.sh\n@@ -0,0 +1,242 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2008 Johannes Schindelin\n+#\n+\n+test_description='Test submodules support in git mv and git rm'\n+\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\n+\t(mkdir sub-repo &&\n+\t cd sub-repo &&\n+\t git init &&\n+\t echo file > file &&\n+\t git add file &&\n+\t git commit -m \"sub initial\") &&\n+\t(cp -r sub-repo sub2-repo &&\n+\t cd sub2-repo &&\n+\t echo file2 > file &&\n+\t git add file &&\n+\t git commit -m \"sub commit2\") &&\n+\tgit submodule add \"$(pwd)/sub-repo\" sub &&\n+\tgit submodule add \"$(pwd)/sub2-repo\" sub2 &&\n+\tgit commit -m initial &&\n+\ttest \"$(git config -f .gitmodules submodule.sub.path)\" = \"sub\" &&\n+\ttest \"$(git config -f .gitmodules submodule.sub2.path)\" = \"sub2\"\n+\n+'\n+\n+test_expect_success 'git mv of a submodule' '\n+\n+\tgit mv sub sub.moved &&\n+\t! test -d sub &&\n+\ttest -d sub.moved/.git &&\n+\t! git ls-files --error-unmatch sub &&\n+\ttest \"$(git ls-files --stage --error-unmatch sub.moved | cut -d \" \" -f 1)\" = 160000 &&\n+\ttest \"$(git config -f .gitmodules submodule.sub.path)\" = \"sub.moved\" &&\n+\t! git config -f .gitmodules submodule.sub.moved.path\n+\n+'\n+\n+test_expect_success 'git submodule add vs. git mv' '\n+\n+\t! git submodule add \"$(pwd)/sub2-repo\" sub.moved &&\n+\tgit submodule add \"$(pwd)/sub2-repo\" sub &&\n+\ttest -d sub/.git &&\n+\ttest \"$(git config -f .gitmodules submodule.sub.url)\" = \"$(pwd)/sub-repo\" &&\n+\ttest \"$(git config -f .gitmodules submodule.sub.path)\" = \"sub.moved\" &&\n+\ttest \"$(git config -f .gitmodules submodule.sub~.path)\" = \"sub\"\n+\n+'\n+\n+test_expect_success 'git mv onto existing file' '\n+\n+\techo file > file &&\n+\tgit add file &&\n+\t! git mv sub.moved file &&\n+\ttest -d sub.moved &&\n+\t! test -d file/.git &&\n+\ttest \"$(git ls-files --stage --error-unmatch file | cut -d \" \" -f 1)\" = 100644 &&\n+\ttest \"$(git ls-files --stage --error-unmatch sub.moved | cut -d \" \" -f 1)\" = 160000 &&\n+\ttest \"$(git config -f .gitmodules submodule.sub.path)\" = \"sub.moved\"\n+\n+'\n+\n+test_expect_success 'git mv onto existing directory' '\n+\n+\tmkdir -p dir &&\n+\techo file > dir/file &&\n+\tgit add dir/file &&\n+\tgit mv sub.moved dir &&\n+\t! test -d sub.moved &&\n+\ttest -d dir/sub.moved/.git &&\n+\t! git ls-files --error-unmatch sub.moved &&\n+\ttest \"$(git ls-files --stage --error-unmatch dir/sub.moved | cut -d \" \" -f 1)\" = 160000 &&\n+\ttest \"$(git config -f .gitmodules submodule.sub.path)\" = \"dir/sub.moved\" &&\n+\tgit mv dir/sub.moved . &&\n+\ttest \"$(git config -f .gitmodules submodule.sub.path)\" = \"sub.moved\"\n+\n+'\n+\n+test_expect_success 'git mv onto existing submodule' '\n+\n+\t! git mv sub.moved sub2 &&\n+\ttest -d sub.moved/.git &&\n+\t! test -d sub2/sub.moved &&\n+\ttest \"$(git ls-files --stage --error-unmatch sub2 | cut -d \" \" -f 1)\" = 160000 &&\n+\ttest \"$(git ls-files --stage --error-unmatch sub.moved | cut -d \" \" -f 1)\" = 160000 &&\n+\ttest \"$(git config -f .gitmodules submodule.sub.path)\" = \"sub.moved\"\n+\n+'\n+\n+test_expect_success 'git mv of multiple submodules' '\n+\n+\tmkdir -p dir &&\n+\tgit mv sub.moved sub dir &&\n+\t! test -d sub.moved &&\n+\t! test -d sub &&\n+\ttest -d dir/sub.moved/.git &&\n+\ttest -d dir/sub/.git &&\n+\t! git ls-files --error-unmatch sub.moved sub &&\n+\ttest \"$(git ls-files --stage --error-unmatch dir/sub.moved dir/sub | cut -d \" \" -f 1 | uniq)\" = 160000 &&\n+\t! git config -f .gitmodules submodule.dir.path &&\n+\ttest \"$(git config -f .gitmodules submodule.sub.path)\" = \"dir/sub.moved\" &&\n+\ttest \"$(git config -f .gitmodules submodule.sub~.path)\" = \"dir/sub\"\n+\n+'\n+\n+test_expect_success 'git mv of multiple submodules back from a subdir' '\n+\n+\t(cd dir && git mv sub.moved sub .. && cd ..) &&\n+\ttest -d sub.moved &&\n+\ttest -d sub &&\n+\t! test -d dir/sub.moved/.git &&\n+\t! test -d dir/sub/.git &&\n+\t! git ls-files --error-unmatch dir/sub.moved dir/sub &&\n+\ttest \"$(git ls-files --stage --error-unmatch sub.moved sub | cut -d \" \" -f 1 | uniq)\" = 160000 &&\n+\ttest \"$(git config -f .gitmodules submodule.sub.path)\" = \"sub.moved\" &&\n+\ttest \"$(git config -f .gitmodules submodule.sub~.path)\" = \"sub\"\n+\n+'\n+\n+test_expect_success 'git mv of non-checked-out submodules' '\n+\n+\tgit clone . clone &&\n+\t(cd clone &&\n+\ttest -d sub &&\n+\ttest -d sub2 &&\n+\t! test -d sub/.git &&\n+\t! test -d sub2/.git &&\n+\tgit ls-files --stage --error-unmatch sub sub2 > ls-files.out &&\n+\tmkdir -p dir &&\n+\tgit mv sub sub2 dir &&\n+\t! test -d sub &&\n+\t! test -d sub2 &&\n+\ttest -d dir/sub &&\n+\ttest -d dir/sub2 &&\n+\t! git ls-files --error-unmatch sub sub2 &&\n+\ttest \"$(git ls-files --stage --error-unmatch dir/sub dir/sub2 | cut -d \" \" -f 1 | uniq)\" = 160000 &&\n+\tgit ls-files --stage --error-unmatch dir/sub dir/sub2 | sed \"s#dir/##g\" | diff - ls-files.out &&\n+\ttest \"$(git config -f .gitmodules submodule.sub.path)\" = \"dir/sub\" &&\n+\ttest \"$(git config -f .gitmodules submodule.sub2.path)\" = \"dir/sub2\" &&\n+\t(cd dir && git mv sub2 .. && cd ..) &&\n+\ttest -d sub2 &&\n+\t! test -d dir/sub2 &&\n+\t! git ls-files --error-unmatch dir/sub2 &&\n+\ttest \"$(git ls-files --stage --error-unmatch sub2 | cut -d \" \" -f 1)\" = 160000 &&\n+\ttest \"$(git config -f .gitmodules submodule.sub2.path)\" = \"sub2\")\n+\n+'\n+\n+test_expect_success 'checkpointing state with git commit' '\n+\n+\tgit commit -m\"checkpoint\" -a &&\n+\t(cd clone && git commit -m\"clone checkpoint\" -a)\n+\n+'\n+\n+test_expect_success 'git rm of a regular submodule' '\n+\n+\tgit rm sub2 &&\n+\ttest -d sub2/.git &&\n+\t! git ls-files --error-unmatch sub2 &&\n+\t! git config -f .gitmodules submodule.sub2.path &&\n+\t! git config -f .gitmodules submodule.sub2.url\n+\n+'\n+\n+test_expect_success 'git rm of a submodule with name different from path' '\n+\n+\tgit rm sub.moved &&\n+\ttest -d sub.moved/.git &&\n+\t! git ls-files --error-unmatch sub.moved &&\n+\t! git config -f .gitmodules submodule.sub.path &&\n+\t! git config -f .gitmodules submodule.sub.url\n+\n+'\n+\n+test_expect_success 'git rm of a modified submodule' '\n+\n+\tgit mv sub dir/sub && # more fun with richer path\n+\t(cd dir/sub &&\n+\t echo mod > file &&\n+\t git commit -m \"sub mod\" file) &&\n+\tgit add dir/sub &&\n+\t! git rm dir/sub &&\n+\ttest -d dir/sub/.git &&\n+\ttest \"$(git ls-files --stage --error-unmatch dir/sub | cut -d \" \" -f 1)\" = \"160000\" &&\n+\tgit config -f .gitmodules submodule.sub~.path &&\n+\tgit config -f .gitmodules submodule.sub~.url &&\n+\tgit rm -f dir/sub &&\n+\ttest -d dir/sub/.git &&\n+\t! git ls-files --error-unmatch dir/sub &&\n+\t! git config -f .gitmodules submodule.sub~.path &&\n+\t! git config -f .gitmodules submodule.sub~.url\n+\n+'\n+\n+test_expect_success 'git rm of a submodule from within a subdirectory' '\n+\n+\tgit submodule add \"$(pwd)/sub-repo\" sub-torm &&\n+\tmkdir -p dir &&\n+\t# -f since we did not commit the submodule\n+\t(cd dir && git rm -f ../sub-torm && cd ..) &&\n+\ttest -d sub-torm/.git &&\n+\t! git ls-files --error-unmatch sub-torm &&\n+\t! git config -f .gitmodules submodule.sub-torm.path &&\n+\t! git config -f .gitmodules submodule.sub-torm.url\n+\n+'\n+\n+test_expect_success 'git rm of a non-checked-out submodule' '\n+\n+\t(cd clone &&\n+\ttest -d dir/sub &&\n+\t! test -d dir/sub/.git &&\n+\tgit rm dir/sub &&\n+\t! test -d dir/sub &&\n+\t! git ls-files --error-unmatch dir/sub &&\n+\t! git config -f .gitmodules submodule.sub.path &&\n+\t! git config -f .gitmodules submodule.sub.url)\n+\n+'\n+\n+test_expect_success 'git rm of a non-checked-out submodule w/ different working tree' '\n+\n+\t(cd clone &&\n+\trmdir sub2 &&\n+\techo cunning > sub2 &&\n+\t! git rm sub2 &&\n+\ttest -f sub2 &&\n+\ttest \"$(git ls-files --stage --error-unmatch sub2 | cut -d \" \" -f 1)\" = \"160000\" &&\n+\tgit rm -f sub2 &&\n+\t! test -e sub2 &&\n+\t! git ls-files --error-unmatch sub2 &&\n+\t! git config -f .gitmodules submodule.sub2.path &&\n+\t! git config -f .gitmodules submodule.sub2.url)\n+\n+'\n+\n+test_done\n"},{"id":"90599","messageId":"20080912210918.31628.97855.stgit@localhost","threadId":"15505","inReplyTo":"20080912210817.31628.69014.stgit@localhost","subject":"[PATCH 5/6] t7400: Add short \"git submodule add\" testsuite","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2008-09-12T21:09:19Z","receivedAt":"2008-09-12T21:09:19Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"This patch introduces basic tests for\n\n\tgit submodule add\n\ncovering the basic functionality and the -b parameter.\n\nA trivial update --init test fix freeloads on this commit as well.\n\nSigned-off-by: Petr Baudis <pasky@suse.cz>\n---\n\n t/t7400-submodule-basic.sh |   28 +++++++++++++++++++++++++++-\n 1 files changed, 27 insertions(+), 1 deletions(-)\n\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex be73f7b..8a002bc 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -200,7 +200,7 @@ test_expect_success 'update --init' '\n \n \tmv init init2 &&\n \tgit config -f .gitmodules submodule.example.url \"$(pwd)/init2\" &&\n-\tgit config --remove-section submodule.example\n+\tgit config --remove-section submodule.example &&\n \tgit submodule update init > update.out &&\n \tgrep \"not initialized\" update.out &&\n \ttest ! -d init/.git &&\n@@ -209,4 +209,30 @@ test_expect_success 'update --init' '\n \n '\n \n+test_expect_success 'submodule add' '\n+\n+\tgit submodule add \"$(pwd)/init2\" init-added &&\n+\ttest -d init-added/.git &&\n+\t[ \"$(git config -f .gitmodules submodule.init-added.url)\" = \"$(pwd)/init2\" ] &&\n+\t[ \"$(git config -f .gitmodules submodule.init-added.path)\" = \"init-added\" ]\n+\n+'\n+\n+test_expect_success 'submodule add -b' '\n+\n+\t(\n+\t\tcd init2 &&\n+\t\tgit checkout -b branch &&\n+\t\techo t >s &&\n+\t\tgit add s &&\n+\t\tgit commit -m \"change branch\" &&\n+\t\tgit checkout master\n+\t) &&\n+\tgit submodule add -b branch -- \"$(pwd)/init2\" init-added-b &&\n+\ttest -d init-added-b/.git &&\n+\t[ \"$(git config -f .gitmodules submodule.init-added-b.url)\" = \"$(pwd)/init2\" ] &&\n+\t[ \"$(cd init2 && git rev-parse branch)\" = \"$(cd init-added-b && git rev-parse HEAD)\" ]\n+\n+'\n+\n test_done\n"},{"id":"90600","messageId":"20080912210924.31628.61593.stgit@localhost","threadId":"15505","inReplyTo":"20080912210817.31628.69014.stgit@localhost","subject":"[PATCH 6/6] git submodule add: Fix naming clash handling","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2008-09-12T21:09:24Z","receivedAt":"2008-09-12T21:09:24Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"This patch fixes git submodule add behaviour when we add submodule\nliving at a same path as logical name of existing submodule. This\ncan happen e.g. in case the user git mv's the previous submodule away\nand then git submodule add's another under the same name.\n\nA test-case is obviously included.\n\nThis is not completely satisfactory since .git/config cross-commit\nconflicts can still occur. A question is whether this is worth\nhandling, maybe it would be worth adding some kind of randomization\nof the autogenerated submodule name, e.g. appending $$ or a timestamp.\n\nSigned-off-by: Petr Baudis <pasky@suse.cz>\n---\n\n git-submodule.sh           |   15 ++++++++++++---\n t/t7400-submodule-basic.sh |   11 +++++++++++\n 2 files changed, 23 insertions(+), 3 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 1c39b59..3e4d839 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -201,10 +201,19 @@ cmd_add()\n \tgit add \"$path\" ||\n \tdie \"Failed to add submodule '$path'\"\n \n-\tgit config -f .gitmodules submodule.\"$path\".path \"$path\" &&\n-\tgit config -f .gitmodules submodule.\"$path\".url \"$repo\" &&\n+\tname=\"$path\"\n+\tif git config -f .gitmodules submodule.\"$name\".path; then\n+\t\tname=\"$path~\"; i=1;\n+\t\twhile git config -f .gitmodules submodule.\"$name\".path; do\n+\t\t\tname=\"$path~$i\"\n+\t\t\ti=$((i+1))\n+\t\tdone\n+\tfi\n+\n+\tgit config -f .gitmodules submodule.\"$name\".path \"$path\" &&\n+\tgit config -f .gitmodules submodule.\"$name\".url \"$repo\" &&\n \tgit add .gitmodules ||\n-\tdie \"Failed to register submodule '$path'\"\n+\tdie \"Failed to register submodule '$path' (name '$name')\"\n }\n \n #\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex 8a002bc..0d12a37 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -235,4 +235,15 @@ test_expect_success 'submodule add -b' '\n \n '\n \n+test_expect_success 'submodule add auto-naming clash' '\n+\n+\tgit submodule add \"$(pwd)/init2/.git\" example &&\n+\ttest -d example/.git &&\n+\t[ \"$(git config -f .gitmodules submodule.example.url)\" = \"$(pwd)/init2\" ] &&\n+\t[ \"$(git config -f .gitmodules submodule.example.path)\" = \"init\" ]\n+\t[ \"$(git config -f .gitmodules submodule.example~.url)\" = \"$(pwd)/init2/.git\" ] &&\n+\t[ \"$(git config -f .gitmodules submodule.example~.path)\" = \"example\" ]\n+\n+'\n+\n test_done\n"},{"id":"90601","messageId":"7vsks5njmg.fsf@gitster.siamese.dyndns.org","threadId":"15505","inReplyTo":"20080912210857.31628.7605.stgit@localhost","subject":"Re: [PATCH 1/6] submodule.*: Introduce simple C interface for submodule lookup by path","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-09-12T21:23:51Z","receivedAt":"2008-09-12T21:23:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Petr Baudis <pasky@suse.cz> writes:\n\n> +static int gitmodules_worker(const char *key, const char *value, void *info_)\n\nWon't you ever have different kind of work in the future?\nfind_submodule_by_path(), perhaps?\n\n> +{\n> +\tstruct gitmodules_info *info = info_;\n> +\tconst char *subkey;\n> +\n> +\tif (prefixcmp(key, \"submodule.\"))\n> +\t\treturn 0;\n> +\n> +\tsubkey = strrchr(key, '.');\n> +\tif (!subkey)\n> +\t\treturn 0;\n\nThis cannot happen; you made sure the thing begins with \"submodule.\"\nalready.\n\n> +\tif (strcmp(subkey, \".path\"))\n> +\t\treturn 0;\n\nThis will miss a misconfigured \"submodule.path\" (two level).\n\nI can understand if this part were:\n\n\tsubkey = strrchr(key, '.');\n        if (!subkey || subkey == key + strlen(\"submodule.\") - 1)\n        \treturn 0;\n\n> +\tif (strcmp(value, info->path))\n> +\t\treturn 0;\n\nThis will segfault on a misconfigured:\n\n\t[submodule \"xyzzy\"]\n        \tpath\n\n> +\t/* Found the key to change. */\n> +\tif (info->key) {\n> +\t\terror(\"multiple submodules live at path `%s'\", info->path);\n\nWhy is this \"error()\", not \"warning()\"?\n\n> +\t\t/* The last one is supposed to win. */\n> +\t\tfree(info->key);\n> +\t}\n> +\tinfo->key = xstrdup(key);\n> +\treturn 0;\n\nHave to wonder if it makes easier for the users if this function kept only\n\"xyzzy\" out of \"submodule.xyzzy.path\", not the whole thing.  Cannot judge\nwithout actual callers.\n\n> +}\n> +\n> +char *submodule_by_path(const char *path)\n> +{\n> +\tstruct gitmodules_info info = { path, NULL };\n> +\n> +\tconfig_exclusive_filename = \".gitmodules\";\n> +\tif (git_config(gitmodules_worker, &info))\n> +\t\tdie(\"cannot process .gitmodules\");\n> +\tif (!info.key)\n> +\t\tdie(\"the submodule of `%s' not found in .gitmodules\", path);\n> +\tconfig_exclusive_filename = NULL;\n> +\n> +\treturn info.key;\n> +}\n> diff --git a/submodule.h b/submodule.h\n> new file mode 100644\n> index 0000000..bc74fa0\n> --- /dev/null\n> +++ b/submodule.h\n> @@ -0,0 +1,8 @@\n> +#ifndef SUBMODULE_H\n> +#define SUBMODULE_H\n> +\n> +/* Find submodule living at given path in .gitmodules and return the key\n> + * of its path config variable (dynamically allocated). */\n\nStyle?\n\n> +extern char *submodule_by_path(const char *path);\n> +\n> +#endif\n"},{"id":"90602","messageId":"m3hc8lnj4k.fsf@localhost.localdomain","threadId":"15505","inReplyTo":"7vsks5njmg.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 1/6] submodule.*: Introduce simple C interface for submodule lookup by path","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-09-12T21:35:54Z","receivedAt":"2008-09-12T21:35:54Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Petr Baudis <pasky@suse.cz> writes:\n\n> > +{\n> > +\tstruct gitmodules_info *info = info_;\n> > +\tconst char *subkey;\n> > +\n> > +\tif (prefixcmp(key, \"submodule.\"))\n> > +\t\treturn 0;\n> > +\n> > +\tsubkey = strrchr(key, '.');\n> > +\tif (!subkey)\n> > +\t\treturn 0;\n> \n> This cannot happen; you made sure the thing begins with \"submodule.\"\n> already.\n[cut]\n\nAre there any api to access and manipulate configuration,\nincluding fqvn with the subsection part, instead of having\nto do this again and again?\n\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"90603","messageId":"20080912214129.14829.53058.stgit@localhost","threadId":"15505","inReplyTo":"20080912210908.31628.50439.stgit@localhost","subject":"[PATCH] git mv: Support moving submodules","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2008-09-12T21:42:31Z","receivedAt":"2008-09-12T21:42:31Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"This patch adds support for moving submodules to 'git mv', including\nrewriting of the .gitmodules file to reflect the movement.\n\nSigned-off-by: Petr Baudis <pasky@suse.cz>\n---\n\nI'm sorry, I was careless - this is the correct patch version, referring to\nstring_list_item.string instead of .path.\n\nAlso, the testsuite in patch 4 is for some reason marked as (c)'d by Dscho.\nNot that it would be a big deal.\n\n builtin-mv.c |   37 +++++++++++++++++++++++++++++++++----\n 1 files changed, 33 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin-mv.c b/builtin-mv.c\nindex 4f65b5a..2970acc 100644\n--- a/builtin-mv.c\n+++ b/builtin-mv.c\n@@ -9,6 +9,7 @@\n #include \"cache-tree.h\"\n #include \"string-list.h\"\n #include \"parse-options.h\"\n+#include \"submodule.h\"\n \n static const char * const builtin_mv_usage[] = {\n \t\"git mv [options] <source>... <destination>\",\n@@ -49,6 +50,24 @@ static const char *add_slash(const char *path)\n \treturn path;\n }\n \n+static int ce_is_gitlink(int i)\n+{\n+\treturn i < 0 ? 0 : S_ISGITLINK(active_cache[i]->ce_mode);\n+}\n+\n+static void rename_submodule(struct string_list_item *i)\n+{\n+\tchar *key = submodule_by_path(i->string);\n+\n+\tconfig_exclusive_filename = \".gitmodules\";\n+\tif (git_config_set(key, (const char *) i->util))\n+\t\tdie(\"cannot update .gitmodules\");\n+\tconfig_exclusive_filename = NULL;\n+\n+\tfree(key);\n+}\n+\n+\n static struct lock_file lock_file;\n \n int cmd_mv(int argc, const char **argv, const char *prefix)\n@@ -65,6 +84,8 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n \tenum update_mode { BOTH = 0, WORKING_DIRECTORY, INDEX } *modes;\n \tstruct stat st;\n \tstruct string_list src_for_dst = {NULL, 0, 0, 0};\n+\t/* .string is source path, .util is destination path */\n+\tstruct string_list submodules = {NULL, 0, 0, 0};\n \n \tgit_config(git_default_config, NULL);\n \n@@ -84,7 +105,8 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n \t\t/* special case: \".\" was normalized to \"\" */\n \t\tdestination = copy_pathspec(dest_path[0], argv, argc, 1);\n \telse if (!lstat(dest_path[0], &st) &&\n-\t\t\tS_ISDIR(st.st_mode)) {\n+\t\t\tS_ISDIR(st.st_mode) &&\n+\t\t\t!ce_is_gitlink(cache_name_pos(dest_path[0], strlen(dest_path[0])))) {\n \t\tdest_path[0] = add_slash(dest_path[0]);\n \t\tdestination = copy_pathspec(dest_path[0], argv, argc, 1);\n \t} else {\n@@ -96,7 +118,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n \t/* Checking */\n \tfor (i = 0; i < argc; i++) {\n \t\tconst char *src = source[i], *dst = destination[i];\n-\t\tint length, src_is_dir;\n+\t\tint length, src_is_dir, src_cache_pos;\n \t\tconst char *bad = NULL;\n \n \t\tif (show_only)\n@@ -111,7 +133,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n \t\t} else if ((src_is_dir = S_ISDIR(st.st_mode))\n \t\t\t\t&& lstat(dst, &st) == 0)\n \t\t\tbad = \"cannot move directory over file\";\n-\t\telse if (src_is_dir) {\n+\t\telse if ((src_cache_pos = cache_name_pos(src, length)) < 0 && src_is_dir) {\n \t\t\tconst char *src_w_slash = add_slash(src);\n \t\t\tint len_w_slash = length + 1;\n \t\t\tint first, last;\n@@ -177,7 +199,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n \t\t\t\t} else\n \t\t\t\t\tbad = \"Cannot overwrite\";\n \t\t\t}\n-\t\t} else if (cache_name_pos(src, length) < 0)\n+\t\t} else if (src_cache_pos < 0)\n \t\t\tbad = \"not under version control\";\n \t\telse if (string_list_has_string(&src_for_dst, dst))\n \t\t\tbad = \"multiple sources for the same target\";\n@@ -214,10 +236,17 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n \n \t\tpos = cache_name_pos(src, strlen(src));\n \t\tassert(pos >= 0);\n+\t\tif (ce_is_gitlink(pos))\n+\t\t\tstring_list_insert(src, &submodules)->util = (void *) dst;\n \t\tif (!show_only)\n \t\t\trename_cache_entry_at(pos, dst);\n \t}\n \n+\tfor (i = 0; i < submodules.nr; i++)\n+\t\trename_submodule(&submodules.items[i]);\n+\tif (submodules.nr > 0 && add_file_to_cache(\".gitmodules\", 0))\n+\t\tdie(\"cannot add new .gitmodules to the index\");\n+\n \tif (active_cache_changed) {\n \t\tif (write_cache(newfd, active_cache, active_nr) ||\n \t\t    commit_locked_index(&lock_file))\n"},{"id":"90604","messageId":"7v8wtxniez.fsf@gitster.siamese.dyndns.org","threadId":"15505","inReplyTo":"20080912210902.31628.7325.stgit@localhost","subject":"Re: [PATCH 2/6] git rm: Support for removing submodules","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-09-12T21:49:56Z","receivedAt":"2008-09-12T21:49:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Petr Baudis <pasky@suse.cz> writes:\n\n> @@ -20,7 +20,8 @@ and no updates to their contents can be staged in the index,\n>  though that default behavior can be overridden with the `-f` option.\n>  When '--cached' is given, the staged content has to\n>  match either the tip of the branch or the file on disk,\n> -allowing the file to be removed from just the index.\n> +allowing the file to be removed from just the index;\n> +this is always the case when removing submodules.\n\nSorry, I read this three times but \"this\" is unclear to me.  Different and\nmutually incompatible interpretations I tried to understand it are:\n\n (1) When removing submodules, whether --cached or not, the index can\n     match either HEAD or the work tree; this is different from removing\n     regular blobs where the index must match with HEAD without --cached\n     nor -f;\n\n (2) When removing submodules with --cached, the index can match either\n     HEAD or the work tree and it is removed only from the index.  You\n     cannot remove submodules without --cached;\n\n (3) When removing submodules, the index can match either HEAD or the work\n     tree and it is removed only from the index, even if you did not give\n     --cached;\n\nIt later becomes clear that you meant (3) in the second hunk, but the\nfirst time reader of the resulting document (not this patch) won't be\nreading from bottom to top.\n\nThis is a leftover issue from ealier documentation 25dc720 (Clarify and\nfix English in \"git-rm\" documentation, 2008-04-16), but the description is\nunclear what should happen while working towards the initial commit\n(i.e. no HEAD yet).  I think check_local_mod() allows removal in such a\ncase.  Perhaps you can clarify the point while at it, please?\n\n> diff --git a/builtin-rm.c b/builtin-rm.c\n> index 6bd8211..7475de2 100644\n> --- a/builtin-rm.c\n> +++ b/builtin-rm.c\n> ...\n> -static void add_list(const char *name)\n> +static void add_list(const char *name, int is_gitlink)\n>  {\n>  \tif (list.nr >= list.alloc) {\n>  \t\tlist.alloc = alloc_nr(list.alloc);\n> -\t\tlist.name = xrealloc(list.name, list.alloc * sizeof(const char *));\n> +\t\tlist.info = xrealloc(list.info, list.alloc * sizeof(*list.info));\n>  \t}\n\nALLOC_GROW()?\n\n> @@ -38,6 +44,13 @@ static int remove_file(const char *name)\n>  \tif (ret && errno == ENOENT)\n>  \t\t/* The user has removed it from the filesystem by hand */\n>  \t\tret = errno = 0;\n> +\tif (ret && errno == EISDIR) {\n> +\t\t/* This is a gitlink entry; try to remove at least the\n> +\t\t * directory if the submodule is not checked out; we always\n> +\t\t * leave the checked out ones as they are */\n\n\t/*\n\t * Style?\n         * for a multi-line comment.\n         */\n\n> +static void remove_submodule(const char *name)\n> +{\n> +\tchar *key = submodule_by_path(name);\n> +\tchar *sectend = strrchr(key, '.');\n> +\n> +\tassert(sectend);\n> +\t*sectend = 0;\n\nHere is one caller I questioned in my comments on [1/6].  It is clear this\ncaller wants to use \"submodule.xyzzy\" out of \"submodule.xyzzy.path\".  The\nfunction returning \"submodule.xyzzy.path\" does not feel like a clean and\nreusable interface to me.  I'd suggest either returning \"submodule.xyzzy\"\n(that's too specialized only for this caller to my taste, though), or just\n\"xyzzy\" and have the caller synthesize whatever string it wants to use\n(yes, it loses microoptimization but do we really care about it in this\ncodepath?), if you have other callers that want different strings around\n\"xyzzy\".\n\n> @@ -140,7 +169,7 @@ static struct option builtin_rm_options[] = {\n>  \n>  int cmd_rm(int argc, const char **argv, const char *prefix)\n>  {\n> -\tint i, newfd;\n> +\tint i, newfd, subs;\n\nPerhaps hoist \"int removed\" up one scope level and reuse it?  I misread\nthat you are counting the number of gitlinks in the index, not the number\nof gitlinks that is being removed, on my first read.  The variable is used\nfor the latter.\n\nOther than that I did not see anything questionable in [2/6].\n"},{"id":"90606","messageId":"20080912215814.GG10544@machine.or.cz","threadId":"15505","inReplyTo":"7vsks5njmg.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 1/6] submodule.*: Introduce simple C interface for submodule lookup by path","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2008-09-12T21:58:14Z","receivedAt":"2008-09-12T21:58:14Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"On Fri, Sep 12, 2008 at 02:23:51PM -0700, Junio C Hamano wrote:\n> Petr Baudis <pasky@suse.cz> writes:\n> \n> > +static int gitmodules_worker(const char *key, const char *value, void *info_)\n> \n> Won't you ever have different kind of work in the future?\n> find_submodule_by_path(), perhaps?\n\nGood point.\n\n> > +{\n> > +\tstruct gitmodules_info *info = info_;\n> > +\tconst char *subkey;\n> > +\n> > +\tif (prefixcmp(key, \"submodule.\"))\n> > +\t\treturn 0;\n> > +\n> > +\tsubkey = strrchr(key, '.');\n> > +\tif (!subkey)\n> > +\t\treturn 0;\n> \n> This cannot happen; you made sure the thing begins with \"submodule.\"\n> already.\n> \n> > +\tif (strcmp(subkey, \".path\"))\n> > +\t\treturn 0;\n> \n> This will miss a misconfigured \"submodule.path\" (two level).\n> \n> I can understand if this part were:\n> \n> \tsubkey = strrchr(key, '.');\n>         if (!subkey || subkey == key + strlen(\"submodule.\") - 1)\n>         \treturn 0;\n\nThis looks strange, but I think I see what do you mean. I will use\n\nif (strcmp(subkey, \".path\") || subkey == key + strlen(\"submodule.\") - 1)\n\n> > +\tif (strcmp(value, info->path))\n> > +\t\treturn 0;\n> \n> This will segfault on a misconfigured:\n> \n> \t[submodule \"xyzzy\"]\n>         \tpath\n\nThanks.\n\n> > +\t/* Found the key to change. */\n> > +\tif (info->key) {\n> > +\t\terror(\"multiple submodules live at path `%s'\", info->path);\n> \n> Why is this \"error()\", not \"warning()\"?\n\nMatter of taste, I suppose. I have changed this to warning(), though it\nis dubious configuration at best.\n\n> > +\t\t/* The last one is supposed to win. */\n> > +\t\tfree(info->key);\n> > +\t}\n> > +\tinfo->key = xstrdup(key);\n> > +\treturn 0;\n> \n> Have to wonder if it makes easier for the users if this function kept only\n> \"xyzzy\" out of \"submodule.xyzzy.path\", not the whole thing.  Cannot judge\n> without actual callers.\n\nThey follow up in the next patches. ;-)  They use the submodule name\nonly to access the configuration again, so this format is the most\nconvenient for them.\n\n> > +}\n> > +\n> > +char *submodule_by_path(const char *path)\n> > +{\n> > +\tstruct gitmodules_info info = { path, NULL };\n> > +\n> > +\tconfig_exclusive_filename = \".gitmodules\";\n> > +\tif (git_config(gitmodules_worker, &info))\n> > +\t\tdie(\"cannot process .gitmodules\");\n> > +\tif (!info.key)\n> > +\t\tdie(\"the submodule of `%s' not found in .gitmodules\", path);\n> > +\tconfig_exclusive_filename = NULL;\n> > +\n> > +\treturn info.key;\n> > +}\n> > diff --git a/submodule.h b/submodule.h\n> > new file mode 100644\n> > index 0000000..bc74fa0\n> > --- /dev/null\n> > +++ b/submodule.h\n> > @@ -0,0 +1,8 @@\n> > +#ifndef SUBMODULE_H\n> > +#define SUBMODULE_H\n> > +\n> > +/* Find submodule living at given path in .gitmodules and return the key\n> > + * of its path config variable (dynamically allocated). */\n> \n> Style?\n\nWould you seriously find it prettier with the newline?\n\n$ git grep '^[^/]*[a-z][^/]*\\*/$' *.c | wc -l\n37\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nThe next generation of interesting software will be done\non the Macintosh, not the IBM PC.  -- Bill Gates\n"},{"id":"90607","messageId":"7v1vzpnhzj.fsf@gitster.siamese.dyndns.org","threadId":"15505","inReplyTo":"7v8wtxniez.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 2/6] git rm: Support for removing submodules","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-09-12T21:59:12Z","receivedAt":"2008-09-12T21:59:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n>> +{\n>> +\tchar *key = submodule_by_path(name);\n>> +\tchar *sectend = strrchr(key, '.');\n>> +\n>> +\tassert(sectend);\n>> +\t*sectend = 0;\n>\n> Here is one caller I questioned in my comments on [1/6]...\n\nAnother thing --- can submodule_by_path() ever return NULL saying \"I do\nnot see one in the configuration\"?\n"},{"id":"90612","messageId":"7vprn9m2hw.fsf@gitster.siamese.dyndns.org","threadId":"15505","inReplyTo":"20080912214129.14829.53058.stgit@localhost","subject":"Re: [PATCH] git mv: Support moving submodules","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-09-12T22:19:07Z","receivedAt":"2008-09-12T22:19:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Petr Baudis <pasky@suse.cz> writes:\n\n> diff --git a/builtin-mv.c b/builtin-mv.c\n> index 4f65b5a..2970acc 100644\n> --- a/builtin-mv.c\n> +++ b/builtin-mv.c\n> @@ -9,6 +9,7 @@\n>  #include \"cache-tree.h\"\n>  #include \"string-list.h\"\n>  #include \"parse-options.h\"\n> +#include \"submodule.h\"\n>  \n>  static const char * const builtin_mv_usage[] = {\n>  \t\"git mv [options] <source>... <destination>\",\n> @@ -49,6 +50,24 @@ static const char *add_slash(const char *path)\n>  \treturn path;\n>  }\n>  \n> +static int ce_is_gitlink(int i)\n> +{\n> +\treturn i < 0 ? 0 : S_ISGITLINK(active_cache[i]->ce_mode);\n> +}\n\nThis interface itself is ugly (why should a caller pass \"it is unmerged or\ndoes not exist\" without checking?), and it also makes the hunk that begins\nat 84/105 ugly.  Why not \"path_is_gitlink(const char*)\" and run\ncache_name_pos() here instead?\n\nThe interface, even if it is internal, should be done with a better taste\nthan that, even though I understand that you wanted to reuse the cache_pos\nfor the source one while you check.\n\nOh, by the way, what should happen when you have an unmerged path in the\nindex and say \"git mv path elsewhere\" (this question is *not* limited to\nsubmodules).  Compared to that, what _does_ happen with the current code,\nand with your patch?\n"},{"id":"90613","messageId":"20080912222429.GE10360@machine.or.cz","threadId":"15505","inReplyTo":"7v1vzpnhzj.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 2/6] git rm: Support for removing submodules","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2008-09-12T22:24:29Z","receivedAt":"2008-09-12T22:24:29Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"I will collect your feedback on the whole series, then resend it from\nscratch - sounds good?\n\nOn Fri, Sep 12, 2008 at 02:49:56PM -0700, Junio C Hamano wrote:\n> Petr Baudis <pasky@suse.cz> writes:\n> \n> > @@ -20,7 +20,8 @@ and no updates to their contents can be staged in the index,\n> >  though that default behavior can be overridden with the `-f` option.\n> >  When '--cached' is given, the staged content has to\n> >  match either the tip of the branch or the file on disk,\n> > -allowing the file to be removed from just the index.\n> > +allowing the file to be removed from just the index;\n> > +this is always the case when removing submodules.\n> \n> Sorry, I read this three times but \"this\" is unclear to me.  Different and\n> mutually incompatible interpretations I tried to understand it are:\n> \n>  (1) When removing submodules, whether --cached or not, the index can\n>      match either HEAD or the work tree; this is different from removing\n>      regular blobs where the index must match with HEAD without --cached\n>      nor -f;\n> \n>  (2) When removing submodules with --cached, the index can match either\n>      HEAD or the work tree and it is removed only from the index.  You\n>      cannot remove submodules without --cached;\n> \n>  (3) When removing submodules, the index can match either HEAD or the work\n>      tree and it is removed only from the index, even if you did not give\n>      --cached;\n> \n> It later becomes clear that you meant (3) in the second hunk, but the\n> first time reader of the resulting document (not this patch) won't be\n> reading from bottom to top.\n> \n> This is a leftover issue from ealier documentation 25dc720 (Clarify and\n> fix English in \"git-rm\" documentation, 2008-04-16), but the description is\n> unclear what should happen while working towards the initial commit\n> (i.e. no HEAD yet).  I think check_local_mod() allows removal in such a\n> case.  Perhaps you can clarify the point while at it, please?\n\nI will have a look.\n\n> > diff --git a/builtin-rm.c b/builtin-rm.c\n> > index 6bd8211..7475de2 100644\n> > --- a/builtin-rm.c\n> > +++ b/builtin-rm.c\n> > ...\n> > -static void add_list(const char *name)\n> > +static void add_list(const char *name, int is_gitlink)\n> >  {\n> >  \tif (list.nr >= list.alloc) {\n> >  \t\tlist.alloc = alloc_nr(list.alloc);\n> > -\t\tlist.name = xrealloc(list.name, list.alloc * sizeof(const char *));\n> > +\t\tlist.info = xrealloc(list.info, list.alloc * sizeof(*list.info));\n> >  \t}\n> \n> ALLOC_GROW()?\n\nNeat thing!\n\n> > @@ -38,6 +44,13 @@ static int remove_file(const char *name)\n> >  \tif (ret && errno == ENOENT)\n> >  \t\t/* The user has removed it from the filesystem by hand */\n> >  \t\tret = errno = 0;\n> > +\tif (ret && errno == EISDIR) {\n> > +\t\t/* This is a gitlink entry; try to remove at least the\n> > +\t\t * directory if the submodule is not checked out; we always\n> > +\t\t * leave the checked out ones as they are */\n> \n> \t/*\n> \t * Style?\n>          * for a multi-line comment.\n>          */\n\nRight - I will have to get used to this. ;-)\n\n> > +static void remove_submodule(const char *name)\n> > +{\n> > +\tchar *key = submodule_by_path(name);\n> > +\tchar *sectend = strrchr(key, '.');\n> > +\n> > +\tassert(sectend);\n> > +\t*sectend = 0;\n> \n> Here is one caller I questioned in my comments on [1/6].  It is clear this\n> caller wants to use \"submodule.xyzzy\" out of \"submodule.xyzzy.path\".  The\n> function returning \"submodule.xyzzy.path\" does not feel like a clean and\n> reusable interface to me.  I'd suggest either returning \"submodule.xyzzy\"\n> (that's too specialized only for this caller to my taste, though), or just\n> \"xyzzy\" and have the caller synthesize whatever string it wants to use\n> (yes, it loses microoptimization but do we really care about it in this\n> codepath?), if you have other callers that want different strings around\n> \"xyzzy\".\n\nThis is still easier to use than explicit snprintf(), but in the long\nrun, you're right that returning just the submodule name is the right\nthing. I will change the API.\n\n> > @@ -140,7 +169,7 @@ static struct option builtin_rm_options[] = {\n> >  \n> >  int cmd_rm(int argc, const char **argv, const char *prefix)\n> >  {\n> > -\tint i, newfd;\n> > +\tint i, newfd, subs;\n> \n> Perhaps hoist \"int removed\" up one scope level and reuse it?  I misread\n> that you are counting the number of gitlinks in the index, not the number\n> of gitlinks that is being removed, on my first read.  The variable is used\n> for the latter.\n\nSensible idea.\n\nOn Fri, Sep 12, 2008 at 02:59:12PM -0700, Junio C Hamano wrote:\n> Junio C Hamano <gitster@pobox.com> writes:\n> \n> >> +{\n> >> +\tchar *key = submodule_by_path(name);\n> >> +\tchar *sectend = strrchr(key, '.');\n> >> +\n> >> +\tassert(sectend);\n> >> +\t*sectend = 0;\n> >\n> > Here is one caller I questioned in my comments on [1/6]...\n> \n> Another thing --- can submodule_by_path() ever return NULL saying \"I do\n> not see one in the configuration\"?\n\nNo, it would rather die().\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nThe next generation of interesting software will be done\non the Macintosh, not the IBM PC.  -- Bill Gates\n"},{"id":"90614","messageId":"7vk5dhm1e9.fsf@gitster.siamese.dyndns.org","threadId":"15505","inReplyTo":"20080912222429.GE10360@machine.or.cz","subject":"Re: [PATCH 2/6] git rm: Support for removing submodules","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-09-12T22:42:54Z","receivedAt":"2008-09-12T22:42:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Petr Baudis <pasky@suse.cz> writes:\n\n>> >> +{\n>> >> +\tchar *key = submodule_by_path(name);\n>> >> +\tchar *sectend = strrchr(key, '.');\n>> >> +\n>> >> +\tassert(sectend);\n>> >> +\t*sectend = 0;\n>> >\n>> > Here is one caller I questioned in my comments on [1/6]...\n>> \n>> Another thing --- can submodule_by_path() ever return NULL saying \"I do\n>> not see one in the configuration\"?\n>\n> No, it would rather die().\n\nHmmmm.  If I did...\n\n        $ git init\n\t$ create and add normal paths\n        $ git clone git://..../gitk.git gitk\n        $ git add gitk\n        : heh, I changed my mind\n        $ git rm gitk\n\nthe last step would die, because I changed my mind before fully\ninitializing gitk repository as a proper submodule?\n\nHow would I get rid of the index entry to recover from the mistake?\n\n        $ rm -fr gitk\n        $ git rm gitk\n\nwould presumably fail the same way, no?  I hope I am misreading the\ncode...\n\nWe need to be extremely careful not to break people who do not (yet) have\n[submodule \"xyzzy\"] entries in config and/or .gitmodules when dealing with\nthe gitlink entries in the index.\n"},{"id":"90619","messageId":"7v63p0n5pv.fsf@gitster.siamese.dyndns.org","threadId":"15505","inReplyTo":"20080912210924.31628.61593.stgit@localhost","subject":"Re: [PATCH 6/6] git submodule add: Fix naming clash handling","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-09-13T02:24:12Z","receivedAt":"2008-09-13T02:24:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Petr Baudis <pasky@suse.cz> writes:\n\n> This patch fixes git submodule add behaviour when we add submodule\n> living at a same path as logical name of existing submodule. This\n> can happen e.g. in case the user git mv's the previous submodule away\n> and then git submodule add's another under the same name.\n\nIn short, name \"example\" was used to name the submodule bound at path\n\"init\" in earlier tests, and the new one adds another submodule whose name\nand path are both \"example\" and makes sure they do not get mixed up (and\nthe original code did mix them up).\n\n>  git-submodule.sh           |   15 ++++++++++++---\n>  t/t7400-submodule-basic.sh |   11 +++++++++++\n>  2 files changed, 23 insertions(+), 3 deletions(-)\n>\n> diff --git a/git-submodule.sh b/git-submodule.sh\n> index 1c39b59..3e4d839 100755\n> --- a/git-submodule.sh\n> +++ b/git-submodule.sh\n> @@ -201,10 +201,19 @@ cmd_add()\n>  \tgit add \"$path\" ||\n>  \tdie \"Failed to add submodule '$path'\"\n>  \n> -\tgit config -f .gitmodules submodule.\"$path\".path \"$path\" &&\n> -\tgit config -f .gitmodules submodule.\"$path\".url \"$repo\" &&\n> +\tname=\"$path\"\n> +\tif git config -f .gitmodules submodule.\"$name\".path; then\n> +\t\tname=\"$path~\"; i=1;\n> +\t\twhile git config -f .gitmodules submodule.\"$name\".path; do\n> +\t\t\tname=\"$path~$i\"\n> +\t\t\ti=$((i+1))\n> +\t\tdone\n> +\tfi\n> +\n> +\tgit config -f .gitmodules submodule.\"$name\".path \"$path\" &&\n> +\tgit config -f .gitmodules submodule.\"$name\".url \"$repo\" &&\n>  \tgit add .gitmodules ||\n> -\tdie \"Failed to register submodule '$path'\"\n> +\tdie \"Failed to register submodule '$path' (name '$name')\"\n>  }\n\nThe logic of the fix seems to be correct, but shouldn't the test be like\nthis instead?\n\n\tif git config -f .gitmodules \"submodule.$name.path\" >/dev/null\n        then\n\nThe same thing for \"git config\" used in the \"while\" loop.\n\nAlso I am not sure if name=\"$path~\" is a good idea for two reasons:\n\n - name suffixed with tilde and number looks too much like revision\n   expression.\n\n - A, A~, A~1, A~2... looks ugly; A, A-0, A-1, A-2,... (or start counting\n   from 1 or 2) I would understand.\n\nBy the way, I noticed that cmd_add does not seem to cd_to_toplevel, and\naccesses .gitmodules in the current working directory.  Isn't this a bug?\nHow should \"git submodule add\" work from inside a subdirectory?\n"},{"id":"90629","messageId":"8c5c35580809130432r11e986e5u4c7300d62ca2896f@mail.gmail.com","threadId":"15505","inReplyTo":"7v63p0n5pv.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 6/6] git submodule add: Fix naming clash handling","fromName":"Lars Hjemli","fromEmail":"hjemli@gmail.com","sentAt":"2008-09-13T11:32:38Z","receivedAt":"2008-09-13T11:32:38Z","isPatch":true,"sender":{"key":"hjemli@gmail.com","avatar":null},"body":"On Sat, Sep 13, 2008 at 4:24 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Petr Baudis <pasky@suse.cz> writes:\n>\n>> This patch fixes git submodule add behaviour when we add submodule\n>> living at a same path as logical name of existing submodule. This\n>> can happen e.g. in case the user git mv's the previous submodule away\n>> and then git submodule add's another under the same name.\n>\n> In short, name \"example\" was used to name the submodule bound at path\n> \"init\" in earlier tests, and the new one adds another submodule whose name\n> and path are both \"example\" and makes sure they do not get mixed up (and\n> the original code did mix them up).\n>\n>>  git-submodule.sh           |   15 ++++++++++++---\n>>  t/t7400-submodule-basic.sh |   11 +++++++++++\n>>  2 files changed, 23 insertions(+), 3 deletions(-)\n>>\n>> diff --git a/git-submodule.sh b/git-submodule.sh\n>> index 1c39b59..3e4d839 100755\n>> --- a/git-submodule.sh\n>> +++ b/git-submodule.sh\n>> @@ -201,10 +201,19 @@ cmd_add()\n>>       git add \"$path\" ||\n>>       die \"Failed to add submodule '$path'\"\n>>\n>> -     git config -f .gitmodules submodule.\"$path\".path \"$path\" &&\n>> -     git config -f .gitmodules submodule.\"$path\".url \"$repo\" &&\n>> +     name=\"$path\"\n>> +     if git config -f .gitmodules submodule.\"$name\".path; then\n>> +             name=\"$path~\"; i=1;\n>> +             while git config -f .gitmodules submodule.\"$name\".path; do\n>> +                     name=\"$path~$i\"\n>> +                     i=$((i+1))\n>> +             done\n>> +     fi\n>> +\n>> +     git config -f .gitmodules submodule.\"$name\".path \"$path\" &&\n>> +     git config -f .gitmodules submodule.\"$name\".url \"$repo\" &&\n>>       git add .gitmodules ||\n>> -     die \"Failed to register submodule '$path'\"\n>> +     die \"Failed to register submodule '$path' (name '$name')\"\n>>  }\n>\n> The logic of the fix seems to be correct, but shouldn't the test be like\n> this instead?\n>\n>        if git config -f .gitmodules \"submodule.$name.path\" >/dev/null\n>        then\n>\n> The same thing for \"git config\" used in the \"while\" loop.\n\nYeah, redirecting to /dev/null seems like a good idea.\n\n\n> Also I am not sure if name=\"$path~\" is a good idea for two reasons:\n>\n>  - name suffixed with tilde and number looks too much like revision\n>   expression.\n>\n>  - A, A~, A~1, A~2... looks ugly; A, A-0, A-1, A-2,... (or start counting\n>   from 1 or 2) I would understand.\n\nI'd just exit with an informative error if submodule.$name already\nexists in .git/config.\n\n\n> By the way, I noticed that cmd_add does not seem to cd_to_toplevel, and\n> accesses .gitmodules in the current working directory.  Isn't this a bug?\n\nNot really, since `git submodule` must be executed from the toplevel\n($SUBDIRECTORY_OK is undefined, so the cd_to_toplevel in cmd_sync and\ncmd_summary seems to be noops).\n\n--\nlarsh\n"}]}