{"thread":{"id":"33371","subject":"[PATCH/RFC 0/3] Teach mv to move submodules","startedAt":"2013-04-03T19:54:38Z","lastAt":"2013-04-11T18:43:10Z","messageCount":14,"participants":["Jens Lehmann","Junio C Hamano","Ramkumar Ramachandra"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"213061","messageId":"515C88FE.9020203@web.de","threadId":"33371","inReplyTo":null,"subject":"[PATCH/RFC 0/3] Teach mv to move submodules","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2013-04-03T19:54:38Z","receivedAt":"2013-04-03T19:54:38Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"This is the last topic I intend to finish before preparing the recursive\nsubmodule update. The other prerequisites for that next step are Heiko's\n\"allow more sources for config values\" and \"fetch moved submodules\non-demand\" topics, which are necessary to populate submodules that were\nnot present in the currently checked out commit and to have all commits\nof moved or currently not present submodules fetched for later checkout\nrespectively. But these are other threads ...\n\nLet's enable users to move submodules (just as easy as they can already\nremove them with current Git with \"git rm\") by using a plain \"git mv\".\nWhile using \"git submodule update\" on a moved submodule cloned by a\ncoworker will still leave the submodule's work tree lying around at the\nold path, that will be fixed by the recursive submodule update (and is\nnot different from what Git currently does with removed submodules).\nReplacing a directory containing files tracked by Git or vice versa is\nstill not possible, another issue to be fixed by the recursive submodule\nupdate. I tried to CC all parties which showed interest in this topic,\nhopefully I didn't forget anyone.\n\nJens Lehmann (3):\n Teach mv to move submodules together with their work trees\n Teach mv to move submodules using a gitfile\n Teach mv to update the path entry in .gitmodules for moved submodules\n\n builtin/mv.c | 126 ++++++++++++++++++++++++++++++++++------------------------\n submodule.c | 126 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n submodule.h | 3 ++\n t/t7001-mv.sh | 94 +++++++++++++++++++++++++++++++++++++++++++\n 4 files changed, 297 insertions(+), 52 deletions(-)\n\n-- \n1.8.2.377.g1bdb7d0\n"},{"id":"213064","messageId":"515C8958.4080704@web.de","threadId":"33371","inReplyTo":"515C88FE.9020203@web.de","subject":"[PATCH/RFC 1/3] Teach mv to move submodules together with their work trees","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2013-04-03T19:56:08Z","receivedAt":"2013-04-03T19:56:08Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Currently the attempt to use \"git mv\" on a submodule errors out with:\n fatal: source directory is empty, source=, destination=\nThe reason is that mv searches for the submodule with a trailing slash in\nthe index, which it doesn't find (because it is stored without a trailing\nslash). As it doesn't find any index entries inside the submodule it\nclaims the directory would be empty even though it isn't.\n\nFix that by searching for the name without a trailing slash and continue\nif it is a submodule. Then rename() will move the submodule work tree just\nlike it moves a file.\n\nSigned-off-by: Jens Lehmann \n---\n builtin/mv.c | 99 +++++++++++++++++++++++++++++++----------------------------\n t/t7001-mv.sh | 34 ++++++++++++++++++++\n 2 files changed, 86 insertions(+), 47 deletions(-)\n\ndiff --git a/builtin/mv.c b/builtin/mv.c\nindex 034fec9..361028d 100644\n--- a/builtin/mv.c\n+++ b/builtin/mv.c\n@@ -117,55 +117,60 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\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\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-\n-\t\t\tmodes[i] = WORKING_DIRECTORY;\n-\n-\t\t\tfirst = cache_name_pos(src_w_slash, len_w_slash);\n-\t\t\tif (first >= 0)\n-\t\t\t\tdie (_(\"Huh? %.*s is in index?\"),\n-\t\t\t\t\t\tlen_w_slash, src_w_slash);\n-\n-\t\t\tfirst = -1 - first;\n-\t\t\tfor (last = first; last < active_nr; last++) {\n-\t\t\t\tconst char *path = active_cache[last]->name;\n-\t\t\t\tif (strncmp(path, src_w_slash, len_w_slash))\n-\t\t\t\t\tbreak;\n-\t\t\t}\n-\t\t\tfree((char *)src_w_slash);\n-\n-\t\t\tif (last - first < 1)\n-\t\t\t\tbad = _(\"source directory is empty\");\n-\t\t\telse {\n-\t\t\t\tint j, dst_len;\n-\n-\t\t\t\tif (last - first > 0) {\n-\t\t\t\t\tsource = xrealloc(source,\n-\t\t\t\t\t\t\t(argc + last - first)\n-\t\t\t\t\t\t\t* sizeof(char *));\n-\t\t\t\t\tdestination = xrealloc(destination,\n-\t\t\t\t\t\t\t(argc + last - first)\n-\t\t\t\t\t\t\t* sizeof(char *));\n-\t\t\t\t\tmodes = xrealloc(modes,\n-\t\t\t\t\t\t\t(argc + last - first)\n-\t\t\t\t\t\t\t* sizeof(enum update_mode));\n+\t\t\tint first = cache_name_pos(src, length);\n+\t\t\tif (first >= 0) {\n+\t\t\t\tif (!S_ISGITLINK(active_cache[first]->ce_mode))\n+\t\t\t\t\tdie (_(\"Huh? Directory %s is in index and no submodule?\"), src);\n+\t\t\t} else {\n+\t\t\t\tconst char *src_w_slash = add_slash(src);\n+\t\t\t\tint last, len_w_slash = length + 1;\n+\n+\t\t\t\tmodes[i] = WORKING_DIRECTORY;\n+\n+\t\t\t\tfirst = cache_name_pos(src_w_slash, len_w_slash);\n+\t\t\t\tif (first >= 0)\n+\t\t\t\t\tdie (_(\"Huh? %.*s is in index?\"),\n+\t\t\t\t\t\t\tlen_w_slash, src_w_slash);\n+\n+\t\t\t\tfirst = -1 - first;\n+\t\t\t\tfor (last = first; last < active_nr; last++) {\n+\t\t\t\t\tconst char *path = active_cache[last]->name;\n+\t\t\t\t\tif (strncmp(path, src_w_slash, len_w_slash))\n+\t\t\t\t\t\tbreak;\n \t\t\t\t}\n-\n-\t\t\t\tdst = add_slash(dst);\n-\t\t\t\tdst_len = strlen(dst);\n-\n-\t\t\t\tfor (j = 0; j < last - first; j++) {\n-\t\t\t\t\tconst char *path =\n-\t\t\t\t\t\tactive_cache[first + j]->name;\n-\t\t\t\t\tsource[argc + j] = path;\n-\t\t\t\t\tdestination[argc + j] =\n-\t\t\t\t\t\tprefix_path(dst, dst_len,\n-\t\t\t\t\t\t\tpath + length + 1);\n-\t\t\t\t\tmodes[argc + j] = INDEX;\n+\t\t\t\tfree((char *)src_w_slash);\n+\n+\t\t\t\tif (last - first < 1)\n+\t\t\t\t\tbad = _(\"source directory is empty\");\n+\t\t\t\telse {\n+\t\t\t\t\tint j, dst_len;\n+\n+\t\t\t\t\tif (last - first > 0) {\n+\t\t\t\t\t\tsource = xrealloc(source,\n+\t\t\t\t\t\t\t\t(argc + last - first)\n+\t\t\t\t\t\t\t\t* sizeof(char *));\n+\t\t\t\t\t\tdestination = xrealloc(destination,\n+\t\t\t\t\t\t\t\t(argc + last - first)\n+\t\t\t\t\t\t\t\t* sizeof(char *));\n+\t\t\t\t\t\tmodes = xrealloc(modes,\n+\t\t\t\t\t\t\t\t(argc + last - first)\n+\t\t\t\t\t\t\t\t* sizeof(enum update_mode));\n+\t\t\t\t\t}\n+\n+\t\t\t\t\tdst = add_slash(dst);\n+\t\t\t\t\tdst_len = strlen(dst);\n+\n+\t\t\t\t\tfor (j = 0; j < last - first; j++) {\n+\t\t\t\t\t\tconst char *path =\n+\t\t\t\t\t\t\tactive_cache[first + j]->name;\n+\t\t\t\t\t\tsource[argc + j] = path;\n+\t\t\t\t\t\tdestination[argc + j] =\n+\t\t\t\t\t\t\tprefix_path(dst, dst_len,\n+\t\t\t\t\t\t\t\tpath + length + 1);\n+\t\t\t\t\t\tmodes[argc + j] = INDEX;\n+\t\t\t\t\t}\n+\t\t\t\t\targc += last - first;\n \t\t\t\t}\n-\t\t\t\targc += last - first;\n \t\t\t}\n \t\t} else if (cache_name_pos(src, length) < 0)\n \t\t\tbad = _(\"not under version control\");\ndiff --git a/t/t7001-mv.sh b/t/t7001-mv.sh\nindex a845b15..4c57f61 100755\n--- a/t/t7001-mv.sh\n+++ b/t/t7001-mv.sh\n@@ -255,4 +255,38 @@ test_expect_success SYMLINKS 'git mv should overwrite file with a symlink' '\n\n rm -f moved symlink\n\n+test_expect_success 'setup submodule' '\n+\tgit commit -m initial &&\n+\tgit reset --hard &&\n+\tgit submodule add ./. sub &&\n+\techo content >file &&\n+\tgit add file &&\n+\tgit commit -m \"added sub and file\"\n+'\n+\n+test_expect_success 'git mv cannot move a submodule in a file' '\n+\ttest_must_fail git mv sub file\n+'\n+\n+test_expect_success 'git mv moves a submodule with a .git directory and no .gitmodules' '\n+\tentry=\"$(git ls-files --stage sub | cut -f 1)\" &&\n+\tgit rm .gitmodules &&\n+\t(\n+\t\tcd sub &&\n+\t\trm -f .git &&\n+\t\tcp -a ../.git/modules/sub .git &&\n+\t\tGIT_WORK_TREE=. git config --unset core.worktree\n+\t) &&\n+\tmkdir mod &&\n+\tgit mv sub mod/sub &&\n+\t! test -e sub &&\n+\t[ \"$entry\" = \"$(git ls-files --stage mod/sub | cut -f 1)\" ] &&\n+\t(\n+\t\tcd mod/sub &&\n+\t\tgit status\n+\t) &&\n+\tgit update-index --refresh &&\n+\tgit diff-files --quiet\n+'\n+\n test_done\n-- \n1.8.2.377.g1bdb7d0\n"},{"id":"213062","messageId":"515C8987.2060804@web.de","threadId":"33371","inReplyTo":"515C88FE.9020203@web.de","subject":"[PATCH/RFC 2/3] Teach mv to move submodules using a gitfile","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2013-04-03T19:56:55Z","receivedAt":"2013-04-03T19:56:55Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"When moving a submodule which uses a gitfile to point to the git directory\nstored in .git/modules/ of the superproject two changes must be made\nto make the submodule work: the .git file and the core.worktree setting\nmust be adjusted to point from work tree to git directory and back.\n\nAchieve that by remembering which submodule uses a gitfile by storing the\nresult of read_gitfile() of each submodule. If that is not NULL the new\nfunction connect_work_tree_and_git_dir() is called after renaming the\nsubmodule's work tree which updates the two settings to the new values.\n\nSigned-off-by: Jens Lehmann \n---\n builtin/mv.c | 19 ++++++++++++++----\n submodule.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n submodule.h | 1 +\n t/t7001-mv.sh | 19 ++++++++++++++++++\n 4 files changed, 99 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/mv.c b/builtin/mv.c\nindex 361028d..609bbb8 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 \tN_(\"git mv [options] ... \"),\n@@ -65,7 +66,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOLEAN('k', NULL, &ignore_errors, N_(\"skip move/rename errors\")),\n \t\tOPT_END(),\n \t};\n-\tconst char **source, **destination, **dest_path;\n+\tconst char **source, **destination, **dest_path, **submodule_gitfile;\n \tenum update_mode { BOTH = 0, WORKING_DIRECTORY, INDEX } *modes;\n \tstruct stat st;\n \tstruct string_list src_for_dst = STRING_LIST_INIT_NODUP;\n@@ -84,6 +85,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n \tsource = copy_pathspec(prefix, argv, argc, 0);\n \tmodes = xcalloc(argc, sizeof(enum update_mode));\n \tdest_path = copy_pathspec(prefix, argv + argc, 1, 0);\n+\tsubmodule_gitfile = xcalloc(argc, sizeof(char *));\n\n \tif (dest_path[0][0] == '\\0')\n \t\t/* special case: \".\" was normalized to \"\" */\n@@ -119,8 +121,14 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n \t\telse if (src_is_dir) {\n \t\t\tint first = cache_name_pos(src, length);\n \t\t\tif (first >= 0) {\n+\t\t\t\tstruct strbuf submodule_dotgit = STRBUF_INIT;\n \t\t\t\tif (!S_ISGITLINK(active_cache[first]->ce_mode))\n \t\t\t\t\tdie (_(\"Huh? Directory %s is in index and no submodule?\"), src);\n+\t\t\t\tstrbuf_addf(&submodule_dotgit, \"%s/.git\", src);\n+\t\t\t\tsubmodule_gitfile[i] = read_gitfile(submodule_dotgit.buf);\n+\t\t\t\tif (submodule_gitfile[i])\n+\t\t\t\t\tsubmodule_gitfile[i] = xstrdup(submodule_gitfile[i]);\n+\t\t\t\tstrbuf_release(&submodule_dotgit);\n \t\t\t} else {\n \t\t\t\tconst char *src_w_slash = add_slash(src);\n \t\t\t\tint last, len_w_slash = length + 1;\n@@ -215,9 +223,12 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n \t\tint pos;\n \t\tif (show_only || verbose)\n \t\t\tprintf(_(\"Renaming %s to %s\\n\"), src, dst);\n-\t\tif (!show_only && mode != INDEX &&\n-\t\t\t\trename(src, dst) < 0 && !ignore_errors)\n-\t\t\tdie_errno (_(\"renaming '%s' failed\"), src);\n+\t\tif (!show_only && mode != INDEX) {\n+\t\t\tif (rename(src, dst) < 0 && !ignore_errors)\n+\t\t\t\tdie_errno (_(\"renaming '%s' failed\"), src);\n+\t\t\tif (submodule_gitfile[i])\n+\t\t\t\tconnect_work_tree_and_git_dir(dst, submodule_gitfile[i]);\n+\t\t}\n\n \t\tif (mode == WORKING_DIRECTORY)\n \t\t\tcontinue;\ndiff --git a/submodule.c b/submodule.c\nindex 975bc87..eba9b42 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -1001,3 +1001,67 @@ int merge_submodule(unsigned char result[20], const char *path,\n \tfree(merges.objects);\n \treturn 0;\n }\n+\n+/* Update gitfile and core.worktree setting to connect work tree and git dir */\n+void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir)\n+{\n+\tstruct strbuf core_worktree_setting = STRBUF_INIT;\n+\tstruct strbuf configfile_name = STRBUF_INIT;\n+\tstruct strbuf gitfile_content = STRBUF_INIT;\n+\tstruct strbuf gitfile_name = STRBUF_INIT;\n+\tconst char *real_work_tree = real_path(work_tree);\n+\tconst char *pathspec[] = { real_work_tree, git_dir, NULL };\n+\tconst char *max_prefix = common_prefix(pathspec);\n+\tFILE *fp;\n+\n+\tif (max_prefix) { /* skip common prefix */\n+\t\tsize_t max_prefix_len = strlen(max_prefix);\n+\t\treal_work_tree += max_prefix_len;\n+\t\tgit_dir += max_prefix_len;\n+\t}\n+\n+\t/*\n+\t * Update gitfile\n+\t */\n+\tstrbuf_addstr(&gitfile_content, \"gitdir: \");\n+\tif (real_work_tree[0]) {\n+\t\tconst char *s = real_work_tree;\n+\t\tdo {\n+\t\t\tstrbuf_addstr(&gitfile_content, \"../\");\n+\t\t\ts++;\n+\t\t} while ((s = strchr(s, '/')));\n+\t}\n+\tstrbuf_addstr(&gitfile_content, git_dir);\n+\tstrbuf_addch(&gitfile_content, '\\n');\n+\n+\tstrbuf_addf(&gitfile_name, \"%s/.git\", work_tree);\n+\tfp = fopen(gitfile_name.buf, \"w\");\n+\tif (!fp)\n+\t\tdie(_(\"Could not create git link %s\"), gitfile_name.buf);\n+\tfprintf(fp, gitfile_content.buf);\n+\tfclose(fp);\n+\n+\tstrbuf_release(&gitfile_content);\n+\tstrbuf_release(&gitfile_name);\n+\n+\t/*\n+\t * Update core.worktree setting\n+\t */\n+\tif (git_dir[0]) {\n+\t\tconst char *s = git_dir;\n+\t\tdo {\n+\t\t\tstrbuf_addstr(&core_worktree_setting, \"../\");\n+\t\t\ts++;\n+\t\t} while ((s = strchr(s, '/')));\n+\t}\n+\tstrbuf_addstr(&core_worktree_setting, real_work_tree);\n+\n+\tstrbuf_addf(&configfile_name, \"%s/config\", git_dir);\n+\tif (git_config_set_in_file(configfile_name.buf, \"core.worktree\",\n+\t\t\t\t core_worktree_setting.buf))\n+\t\tdie(_(\"Could not set core.worktree in %s\"),\n+\t\t configfile_name.buf);\n+\n+\tstrbuf_release(&core_worktree_setting);\n+\tstrbuf_release(&configfile_name);\n+}\ndiff --git a/submodule.h b/submodule.h\nindex 3dc1b3f..0c27c53 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -35,5 +35,6 @@ int merge_submodule(unsigned char result[20], const char *path, const unsigned c\n int find_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_name,\n \t\tstruct string_list *needs_pushing);\n int push_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_name);\n+void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);\n\n #endif\ndiff --git a/t/t7001-mv.sh b/t/t7001-mv.sh\nindex 4c57f61..d824464 100755\n--- a/t/t7001-mv.sh\n+++ b/t/t7001-mv.sh\n@@ -289,4 +289,23 @@ test_expect_success 'git mv moves a submodule with a .git directory and no .gitm\n \tgit diff-files --quiet\n '\n\n+test_expect_success 'git mv moves a submodule with gitfile' '\n+\trm -rf mod/sub &&\n+\tgit reset --hard &&\n+\tgit submodule update &&\n+\tentry=\"$(git ls-files --stage sub | cut -f 1)\" &&\n+\t(\n+\t\tcd mod &&\n+\t\tgit mv ../sub/ .\n+\t) &&\n+\t! test -e sub &&\n+\t[ \"$entry\" = \"$(git ls-files --stage mod/sub | cut -f 1)\" ] &&\n+\t(\n+\t\tcd mod/sub &&\n+\t\tgit status\n+\t) &&\n+\tgit update-index --refresh &&\n+\tgit diff-files --quiet\n+'\n+\n test_done\n-- \n1.8.2.377.g1bdb7d0\n"},{"id":"213063","messageId":"515C89BA.9020806@web.de","threadId":"33371","inReplyTo":"515C88FE.9020203@web.de","subject":"[PATCH/RFC 3/3] Teach mv to update the path entry in .gitmodules for moved submodules","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2013-04-03T19:57:46Z","receivedAt":"2013-04-03T19:57:46Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Currently using \"git mv\" on a submodule moves the submodule's work tree in\nthat of the superproject. But the submodule's path setting in .gitmodules\nis left untouched, which is now inconsistent with the work tree and makes\ngit commands that rely on the proper path -> name mapping (like status and\ndiff) behave strangely.\n\nLet \"git mv\" help here by not only moving the submodule's work tree but\nalso updating the \"submodule..path\" setting from the\n.gitmodules file and stage both. This doesn't happen when no .gitmodules\nfile is found and only issues a warning when it doesn't have a section for\nthis submodule. This is because the user might just use plain gitlinks\nwithout the .gitmodules file or has already updated the path setting by\nhand before issuing the \"git mv\" command (in which case the warning\nreminds him that mv would have done that for him). Only when .gitmodules\nis found and contains merge conflicts the mv command will fail and tell\nthe user to resolve the conflict before trying again.\n\nSigned-off-by: Jens Lehmann \n---\n builtin/mv.c | 8 +++++++-\n submodule.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n submodule.h | 2 ++\n t/t7001-mv.sh | 41 +++++++++++++++++++++++++++++++++++++++\n 4 files changed, 112 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/mv.c b/builtin/mv.c\nindex 609bbb8..36e5605 100644\n--- a/builtin/mv.c\n+++ b/builtin/mv.c\n@@ -57,7 +57,7 @@ static struct lock_file lock_file;\n\n int cmd_mv(int argc, const char **argv, const char *prefix)\n {\n-\tint i, newfd;\n+\tint i, newfd, gitmodules_modified = 0;\n \tint verbose = 0, show_only = 0, force = 0, ignore_errors = 0;\n \tstruct option builtin_mv_options[] = {\n \t\tOPT__VERBOSE(&verbose, N_(\"be verbose\")),\n@@ -71,6 +71,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n \tstruct stat st;\n \tstruct string_list src_for_dst = STRING_LIST_INIT_NODUP;\n\n+\tgitmodules_config();\n \tgit_config(git_default_config, NULL);\n\n \targc = parse_options(argc, argv, prefix, builtin_mv_options,\n@@ -228,6 +229,8 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n \t\t\t\tdie_errno (_(\"renaming '%s' failed\"), src);\n \t\t\tif (submodule_gitfile[i])\n \t\t\t\tconnect_work_tree_and_git_dir(dst, submodule_gitfile[i]);\n+\t\t\tif (!update_path_in_gitmodules(src, dst))\n+\t\t\t\tgitmodules_modified = 1;\n \t\t}\n\n \t\tif (mode == WORKING_DIRECTORY)\n@@ -239,6 +242,9 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n \t\t\trename_cache_entry_at(pos, dst);\n \t}\n\n+\tif (gitmodules_modified)\n+\t\tstage_updated_gitmodules();\n+\n \tif (active_cache_changed) {\n \t\tif (write_cache(newfd, active_cache, active_nr) ||\n \t\t commit_locked_index(&lock_file))\ndiff --git a/submodule.c b/submodule.c\nindex eba9b42..fb742b4 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -10,6 +10,7 @@\n #include \"string-list.h\"\n #include \"sha1-array.h\"\n #include \"argv-array.h\"\n+#include \"blob.h\"\n\n static struct string_list config_name_for_path;\n static struct string_list config_fetch_recurse_submodules_for_name;\n@@ -30,6 +31,67 @@ static struct sha1_array ref_tips_after_fetch;\n */\n static int gitmodules_is_unmerged;\n\n+/*\n+ * Try to update the \"path\" entry in the \"submodule.\" section of the\n+ * .gitmodules file.\n+ */\n+int update_path_in_gitmodules(const char *oldpath, const char *newpath)\n+{\n+\tstruct strbuf entry = STRBUF_INIT;\n+\tstruct string_list_item *path_option;\n+\n+\tif (!file_exists(\".gitmodules\")) /* Do nothing whithout .gitmodules */\n+\t\treturn -1;\n+\n+\tif (gitmodules_is_unmerged)\n+\t\tdie(_(\"Cannot change unmerged .gitmodules, resolve merge conflicts first\"));\n+\n+\tpath_option = unsorted_string_list_lookup(&config_name_for_path, oldpath);\n+\tif (!path_option) {\n+\t\twarning(_(\"Could not find section in .gitmodules where path=%s\"), oldpath);\n+\t\treturn -1;\n+\t}\n+\tstrbuf_addstr(&entry, \"submodule.\");\n+\tstrbuf_addstr(&entry, path_option->util);\n+\tstrbuf_addstr(&entry, \".path\");\n+\tif (git_config_set_in_file(\".gitmodules\", entry.buf, newpath) < 0) {\n+\t\t/* Maybe the user already did that, don't error out here */\n+\t\twarning(_(\"Could not update .gitmodules entry %s\"), entry.buf);\n+\t\treturn -1;\n+\t}\n+\tstrbuf_release(&entry);\n+\treturn 0;\n+}\n+\n+void stage_updated_gitmodules(void)\n+{\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tstruct stat st;\n+\tint pos;\n+\tstruct cache_entry *ce;\n+\tint namelen = strlen(\".gitmodules\");\n+\n+\tpos = cache_name_pos(\".gitmodules\", strlen(\".gitmodules\"));\n+\tif (pos < 0) {\n+\t\twarning(_(\"could not find .gitmodules in index\"));\n+\t\treturn;\n+\t}\n+\tce = active_cache[pos];\n+\tce->ce_flags = namelen;\n+\tif (strbuf_read_file(&buf, \".gitmodules\", 0) < 0)\n+\t\tdie(_(\"reading updated .gitmodules failed\"));\n+\tif (lstat(\".gitmodules\", &st) < 0)\n+\t\tdie_errno(_(\"unable to stat updated .gitmodules\"));\n+\tfill_stat_cache_info(ce, &st);\n+\tce->ce_mode = ce_mode_from_stat(ce, st.st_mode);\n+\tif (remove_file_from_cache(\".gitmodules\") < 0)\n+\t\tdie(_(\"unable to remove .gitmodules from index\"));\n+\tif (write_sha1_file(buf.buf, buf.len, blob_type, ce->sha1))\n+\t\tdie(_(\"adding updated .gitmodules failed\"));\n+\tif (add_cache_entry(ce, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE))\n+\t\tdie(_(\"staging updated .gitmodules failed\"));\n+}\n+\n static int add_submodule_odb(const char *path)\n {\n \tstruct strbuf objects_directory = STRBUF_INIT;\ndiff --git a/submodule.h b/submodule.h\nindex 0c27c53..39c0321 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -11,6 +11,8 @@ enum {\n \tRECURSE_SUBMODULES_ON = 2\n };\n\n+int update_path_in_gitmodules(const char *oldpath, const char *newpath);\n+void stage_updated_gitmodules(void);\n void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,\n \t\tconst char *path);\n int submodule_config(const char *var, const char *value, void *cb);\ndiff --git a/t/t7001-mv.sh b/t/t7001-mv.sh\nindex d824464..c144d21 100755\n--- a/t/t7001-mv.sh\n+++ b/t/t7001-mv.sh\n@@ -304,6 +304,47 @@ test_expect_success 'git mv moves a submodule with gitfile' '\n \t\tcd mod/sub &&\n \t\tgit status\n \t) &&\n+\techo mod/sub >expected &&\n+\tgit config -f .gitmodules submodule.sub.path >actual &&\n+\ttest_cmp expected actual &&\n+\tgit update-index --refresh &&\n+\tgit diff-files --quiet\n+'\n+\n+test_expect_success 'mv does not complain when no .gitmodules file is found' '\n+\trm -rf mod/sub &&\n+\tgit reset --hard &&\n+\tgit submodule update &&\n+\tgit rm .gitmodules &&\n+\tentry=\"$(git ls-files --stage sub | cut -f 1)\" &&\n+\tgit mv sub mod/sub 2>actual.err &&\n+\t! test -s actual.err &&\n+\t! test -e sub &&\n+\t[ \"$entry\" = \"$(git ls-files --stage mod/sub | cut -f 1)\" ] &&\n+\t(\n+\t\tcd mod/sub &&\n+\t\tgit status\n+\t) &&\n+\tgit update-index --refresh &&\n+\tgit diff-files --quiet\n+'\n+\n+test_expect_success 'mv issues a warning when section is not found in .gitmodules' '\n+\trm -rf mod/sub &&\n+\tgit reset --hard &&\n+\tgit submodule update &&\n+\tgit config -f .gitmodules --remove-section submodule.sub &&\n+\tgit add .gitmodules &&\n+\tentry=\"$(git ls-files --stage sub | cut -f 1)\" &&\n+\techo \"warning: Could not find section in .gitmodules where path=sub\" >expect.err &&\n+\tgit mv sub mod/sub 2>actual.err &&\n+\ttest_i18ncmp expect.err actual.err &&\n+\t! test -e sub &&\n+\t[ \"$entry\" = \"$(git ls-files --stage mod/sub | cut -f 1)\" ] &&\n+\t(\n+\t\tcd mod/sub &&\n+\t\tgit status\n+\t) &&\n \tgit update-index --refresh &&\n \tgit diff-files --quiet\n '\n-- \n1.8.2.377.g1bdb7d0\n"},{"id":"213735","messageId":"7vwqsbnvxz.fsf@alter.siamese.dyndns.org","threadId":"33371","inReplyTo":"515C8987.2060804@web.de","subject":"Re: [PATCH/RFC 2/3] Teach mv to move submodules using a gitfile","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-09T23:08:56Z","receivedAt":"2013-04-09T23:08:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jens Lehmann writes:\n\n> diff --git a/submodule.c b/submodule.c\n> index 975bc87..eba9b42 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n> @@ -1001,3 +1001,67 @@ int merge_submodule(unsigned char result[20], const char *path,\n> ...\n> +\tif (!fp)\n> +\t\tdie(_(\"Could not create git link %s\"), gitfile_name.buf);\n> +\tfprintf(fp, gitfile_content.buf);\n\nPerhaps.\n\n\tfprintf(fp, \"%s\", gitfile_content.buf);\n"},{"id":"213786","messageId":"51659A8A.80605@web.de","threadId":"33371","inReplyTo":"7vwqsbnvxz.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC 2/3] Teach mv to move submodules using a gitfile","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2013-04-10T16:59:54Z","receivedAt":"2013-04-10T16:59:54Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 10.04.2013 01:08, schrieb Junio C Hamano:\n> Jens Lehmann writes:\n> \n>> diff --git a/submodule.c b/submodule.c\n>> index 975bc87..eba9b42 100644\n>> --- a/submodule.c\n>> +++ b/submodule.c\n>> @@ -1001,3 +1001,67 @@ int merge_submodule(unsigned char result[20], const char *path,\n>> ...\n>> +\tif (!fp)\n>> +\t\tdie(_(\"Could not create git link %s\"), gitfile_name.buf);\n>> +\tfprintf(fp, gitfile_content.buf);\n> \n> Perhaps.\n> \n> \tfprintf(fp, \"%s\", gitfile_content.buf);\n\nSure. Will fix in v2.\n"},{"id":"213844","messageId":"5165D469.2020603@web.de","threadId":"33371","inReplyTo":"51659A8A.80605@web.de","subject":"[PATCH v2 2/3] Teach mv to move submodules using a gitfile","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2013-04-10T21:06:49Z","receivedAt":"2013-04-10T21:06:49Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"When moving a submodule which uses a gitfile to point to the git directory\nstored in .git/modules/ of the superproject two changes must be made\nto make the submodule work: the .git file and the core.worktree setting\nmust be adjusted to point from work tree to git directory and back.\n\nAchieve that by remembering which submodule uses a gitfile by storing the\nresult of read_gitfile() of each submodule. If that is not NULL the new\nfunction connect_work_tree_and_git_dir() is called after renaming the\nsubmodule's work tree which updates the two settings to the new values.\n\nSigned-off-by: Jens Lehmann \n---\n\nAm 10.04.2013 18:59, schrieb Jens Lehmann:\n> Am 10.04.2013 01:08, schrieb Junio C Hamano:\n>> Jens Lehmann writes:\n>>\n>>> diff --git a/submodule.c b/submodule.c\n>>> index 975bc87..eba9b42 100644\n>>> --- a/submodule.c\n>>> +++ b/submodule.c\n>>> @@ -1001,3 +1001,67 @@ int merge_submodule(unsigned char result[20], const char *path,\n>>> ...\n>>> +\tif (!fp)\n>>> +\t\tdie(_(\"Could not create git link %s\"), gitfile_name.buf);\n>>> +\tfprintf(fp, gitfile_content.buf);\n>>\n>> Perhaps.\n>>\n>> \tfprintf(fp, \"%s\", gitfile_content.buf);\n> \n> Sure. Will fix in v2.\n\nHere we go.\n\n\n builtin/mv.c | 19 ++++++++++++++----\n submodule.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n submodule.h | 1 +\n t/t7001-mv.sh | 19 ++++++++++++++++++\n 4 files changed, 99 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/mv.c b/builtin/mv.c\nindex 361028d..609bbb8 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 \tN_(\"git mv [options] ... \"),\n@@ -65,7 +66,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOLEAN('k', NULL, &ignore_errors, N_(\"skip move/rename errors\")),\n \t\tOPT_END(),\n \t};\n-\tconst char **source, **destination, **dest_path;\n+\tconst char **source, **destination, **dest_path, **submodule_gitfile;\n \tenum update_mode { BOTH = 0, WORKING_DIRECTORY, INDEX } *modes;\n \tstruct stat st;\n \tstruct string_list src_for_dst = STRING_LIST_INIT_NODUP;\n@@ -84,6 +85,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n \tsource = copy_pathspec(prefix, argv, argc, 0);\n \tmodes = xcalloc(argc, sizeof(enum update_mode));\n \tdest_path = copy_pathspec(prefix, argv + argc, 1, 0);\n+\tsubmodule_gitfile = xcalloc(argc, sizeof(char *));\n\n \tif (dest_path[0][0] == '\\0')\n \t\t/* special case: \".\" was normalized to \"\" */\n@@ -119,8 +121,14 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n \t\telse if (src_is_dir) {\n \t\t\tint first = cache_name_pos(src, length);\n \t\t\tif (first >= 0) {\n+\t\t\t\tstruct strbuf submodule_dotgit = STRBUF_INIT;\n \t\t\t\tif (!S_ISGITLINK(active_cache[first]->ce_mode))\n \t\t\t\t\tdie (_(\"Huh? Directory %s is in index and no submodule?\"), src);\n+\t\t\t\tstrbuf_addf(&submodule_dotgit, \"%s/.git\", src);\n+\t\t\t\tsubmodule_gitfile[i] = read_gitfile(submodule_dotgit.buf);\n+\t\t\t\tif (submodule_gitfile[i])\n+\t\t\t\t\tsubmodule_gitfile[i] = xstrdup(submodule_gitfile[i]);\n+\t\t\t\tstrbuf_release(&submodule_dotgit);\n \t\t\t} else {\n \t\t\t\tconst char *src_w_slash = add_slash(src);\n \t\t\t\tint last, len_w_slash = length + 1;\n@@ -215,9 +223,12 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n \t\tint pos;\n \t\tif (show_only || verbose)\n \t\t\tprintf(_(\"Renaming %s to %s\\n\"), src, dst);\n-\t\tif (!show_only && mode != INDEX &&\n-\t\t\t\trename(src, dst) < 0 && !ignore_errors)\n-\t\t\tdie_errno (_(\"renaming '%s' failed\"), src);\n+\t\tif (!show_only && mode != INDEX) {\n+\t\t\tif (rename(src, dst) < 0 && !ignore_errors)\n+\t\t\t\tdie_errno (_(\"renaming '%s' failed\"), src);\n+\t\t\tif (submodule_gitfile[i])\n+\t\t\t\tconnect_work_tree_and_git_dir(dst, submodule_gitfile[i]);\n+\t\t}\n\n \t\tif (mode == WORKING_DIRECTORY)\n \t\t\tcontinue;\ndiff --git a/submodule.c b/submodule.c\nindex 975bc87..9a3eb85 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -1001,3 +1001,67 @@ int merge_submodule(unsigned char result[20], const char *path,\n \tfree(merges.objects);\n \treturn 0;\n }\n+\n+/* Update gitfile and core.worktree setting to connect work tree and git dir */\n+void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir)\n+{\n+\tstruct strbuf core_worktree_setting = STRBUF_INIT;\n+\tstruct strbuf configfile_name = STRBUF_INIT;\n+\tstruct strbuf gitfile_content = STRBUF_INIT;\n+\tstruct strbuf gitfile_name = STRBUF_INIT;\n+\tconst char *real_work_tree = real_path(work_tree);\n+\tconst char *pathspec[] = { real_work_tree, git_dir, NULL };\n+\tconst char *max_prefix = common_prefix(pathspec);\n+\tFILE *fp;\n+\n+\tif (max_prefix) { /* skip common prefix */\n+\t\tsize_t max_prefix_len = strlen(max_prefix);\n+\t\treal_work_tree += max_prefix_len;\n+\t\tgit_dir += max_prefix_len;\n+\t}\n+\n+\t/*\n+\t * Update gitfile\n+\t */\n+\tstrbuf_addstr(&gitfile_content, \"gitdir: \");\n+\tif (real_work_tree[0]) {\n+\t\tconst char *s = real_work_tree;\n+\t\tdo {\n+\t\t\tstrbuf_addstr(&gitfile_content, \"../\");\n+\t\t\ts++;\n+\t\t} while ((s = strchr(s, '/')));\n+\t}\n+\tstrbuf_addstr(&gitfile_content, git_dir);\n+\tstrbuf_addch(&gitfile_content, '\\n');\n+\n+\tstrbuf_addf(&gitfile_name, \"%s/.git\", work_tree);\n+\tfp = fopen(gitfile_name.buf, \"w\");\n+\tif (!fp)\n+\t\tdie(_(\"Could not create git link %s\"), gitfile_name.buf);\n+\tfprintf(fp, \"%s\", gitfile_content.buf);\n+\tfclose(fp);\n+\n+\tstrbuf_release(&gitfile_content);\n+\tstrbuf_release(&gitfile_name);\n+\n+\t/*\n+\t * Update core.worktree setting\n+\t */\n+\tif (git_dir[0]) {\n+\t\tconst char *s = git_dir;\n+\t\tdo {\n+\t\t\tstrbuf_addstr(&core_worktree_setting, \"../\");\n+\t\t\ts++;\n+\t\t} while ((s = strchr(s, '/')));\n+\t}\n+\tstrbuf_addstr(&core_worktree_setting, real_work_tree);\n+\n+\tstrbuf_addf(&configfile_name, \"%s/config\", git_dir);\n+\tif (git_config_set_in_file(configfile_name.buf, \"core.worktree\",\n+\t\t\t\t core_worktree_setting.buf))\n+\t\tdie(_(\"Could not set core.worktree in %s\"),\n+\t\t configfile_name.buf);\n+\n+\tstrbuf_release(&core_worktree_setting);\n+\tstrbuf_release(&configfile_name);\n+}\ndiff --git a/submodule.h b/submodule.h\nindex 3dc1b3f..0c27c53 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -35,5 +35,6 @@ int merge_submodule(unsigned char result[20], const char *path, const unsigned c\n int find_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_name,\n \t\tstruct string_list *needs_pushing);\n int push_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_name);\n+void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);\n\n #endif\ndiff --git a/t/t7001-mv.sh b/t/t7001-mv.sh\nindex 4c57f61..d824464 100755\n--- a/t/t7001-mv.sh\n+++ b/t/t7001-mv.sh\n@@ -289,4 +289,23 @@ test_expect_success 'git mv moves a submodule with a .git directory and no .gitm\n \tgit diff-files --quiet\n '\n\n+test_expect_success 'git mv moves a submodule with gitfile' '\n+\trm -rf mod/sub &&\n+\tgit reset --hard &&\n+\tgit submodule update &&\n+\tentry=\"$(git ls-files --stage sub | cut -f 1)\" &&\n+\t(\n+\t\tcd mod &&\n+\t\tgit mv ../sub/ .\n+\t) &&\n+\t! test -e sub &&\n+\t[ \"$entry\" = \"$(git ls-files --stage mod/sub | cut -f 1)\" ] &&\n+\t(\n+\t\tcd mod/sub &&\n+\t\tgit status\n+\t) &&\n+\tgit update-index --refresh &&\n+\tgit diff-files --quiet\n+'\n+\n test_done\n-- \n1.8.2.1.345.gb37ac0e\n"},{"id":"213913","messageId":"CALkWK0kUdKfAp36HSdoTOK_Wt-Rs_4f3HKHLbLi8e3pzx0iEGw@mail.gmail.com","threadId":"33371","inReplyTo":"5165D469.2020603@web.de","subject":"Re: [PATCH v2 2/3] Teach mv to move submodules using a gitfile","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-04-11T08:37:11Z","receivedAt":"2013-04-11T08:37:11Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Jens Lehmann wrote:\n> When moving a submodule which uses a gitfile to point to the git directory\n> stored in .git/modules/ of the superproject two changes must be made\n> to make the submodule work: the .git file and the core.worktree setting\n> must be adjusted to point from work tree to git directory and back.\n\nIsn't it untrue that the git directory is stored in\n.git/modules/: it is stored in .git/modules/path/to/module. I\nthought the whole point of this complex scheme was to avoid name\nconflicts with submodules with the same name in other directories.\nThen why aren't you moving the object store as well? What happens if\nI try to create a submodule in the old path of this moved submodule in\nthe future? How will you fail, and how will the user recover from\nthis?\n\nA nit on the wording. Perhaps: \"the .git file in the worktree must be\nadjusted to point to $GITDIR, and core.worktree must be set to point\nto the worktree.\"\n\n> Achieve that by remembering which submodule uses a gitfile by storing the\n> result of read_gitfile() of each submodule. If that is not NULL the new\n> function connect_work_tree_and_git_dir() is called after renaming the\n> submodule's work tree which updates the two settings to the new values.\n\nOh God. Can't you figure out at mv-time if the submodule uses a\ngitfile? Why are you storing it? Where are you storing it exactly?\n\n> diff --git a/builtin/mv.c b/builtin/mv.c\n> index 361028d..609bbb8 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> N_(\"git mv [options] ... \"),\n> @@ -65,7 +66,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n> OPT_BOOLEAN('k', NULL, &ignore_errors, N_(\"skip move/rename errors\")),\n> OPT_END(),\n> };\n> - const char **source, **destination, **dest_path;\n> + const char **source, **destination, **dest_path, **submodule_gitfile;\n> enum update_mode { BOTH = 0, WORKING_DIRECTORY, INDEX } *modes;\n> struct stat st;\n> struct string_list src_for_dst = STRING_LIST_INIT_NODUP;\n> @@ -84,6 +85,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n> source = copy_pathspec(prefix, argv, argc, 0);\n> modes = xcalloc(argc, sizeof(enum update_mode));\n> dest_path = copy_pathspec(prefix, argv + argc, 1, 0);\n> + submodule_gitfile = xcalloc(argc, sizeof(char *));\n\nThis is cmd_mv, and you're allocating argc number of char * pointers\nto submodule_gitfile. Why? Are you guaranteed that there are argc\nnumber of submodule_gitfiles?\n\n> if (dest_path[0][0] == '\\0')\n> /* special case: \".\" was normalized to \"\" */\n> @@ -119,8 +121,14 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n> else if (src_is_dir) {\n> int first = cache_name_pos(src, length);\n> if (first >= 0) {\n> + struct strbuf submodule_dotgit = STRBUF_INIT;\n> if (!S_ISGITLINK(active_cache[first]->ce_mode))\n> die (_(\"Huh? Directory %s is in index and no submodule?\"), src);\n> + strbuf_addf(&submodule_dotgit, \"%s/.git\", src);\n> + submodule_gitfile[i] = read_gitfile(submodule_dotgit.buf);\n\nWhat?! read_gitfile() returns the path to the git directory, if it is\nfound. How are you assigning the path to a $GITDIR to a variable\nnamed submodule_gitfile?\n\n> + if (submodule_gitfile[i])\n> + submodule_gitfile[i] = xstrdup(submodule_gitfile[i]);\n\nDoesn't read as smoothly, but saves you an extra char *. Okay.\n\n> + strbuf_release(&submodule_dotgit);\n> } else {\n> const char *src_w_slash = add_slash(src);\n> int last, len_w_slash = length + 1;\n> @@ -215,9 +223,12 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n> int pos;\n> if (show_only || verbose)\n> printf(_(\"Renaming %s to %s\\n\"), src, dst);\n> - if (!show_only && mode != INDEX &&\n> - rename(src, dst) < 0 && !ignore_errors)\n> - die_errno (_(\"renaming '%s' failed\"), src);\n> + if (!show_only && mode != INDEX) {\n> + if (rename(src, dst) < 0 && !ignore_errors)\n> + die_errno (_(\"renaming '%s' failed\"), src);\n> + if (submodule_gitfile[i])\n> + connect_work_tree_and_git_dir(dst, submodule_gitfile[i]);\n> + }\n\nOkay, scratch my previous comment about allocating argc char *\npointers for submodule_gitfile(). Since your logic is in a loop that\nloops argc times, you really have no choice.\n\n> diff --git a/submodule.c b/submodule.c\n> index 975bc87..9a3eb85 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n> @@ -1001,3 +1001,67 @@ int merge_submodule(unsigned char result[20], const char *path,\n> free(merges.objects);\n> return 0;\n> }\n> +\n> +/* Update gitfile and core.worktree setting to connect work tree and git dir */\n> +void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir)\n> +{\n> + struct strbuf core_worktree_setting = STRBUF_INIT;\n> + struct strbuf configfile_name = STRBUF_INIT;\n> + struct strbuf gitfile_content = STRBUF_INIT;\n> + struct strbuf gitfile_name = STRBUF_INIT;\n> + const char *real_work_tree = real_path(work_tree);\n> + const char *pathspec[] = { real_work_tree, git_dir, NULL };\n> + const char *max_prefix = common_prefix(pathspec);\n> + FILE *fp;\n> +\n> + if (max_prefix) { /* skip common prefix */\n> + size_t max_prefix_len = strlen(max_prefix);\n> + real_work_tree += max_prefix_len;\n> + git_dir += max_prefix_len;\n> + }\n> +\n> + /*\n> + * Update gitfile\n> + */\n> + strbuf_addstr(&gitfile_content, \"gitdir: \");\n> + if (real_work_tree[0]) {\n> + const char *s = real_work_tree;\n> + do {\n> + strbuf_addstr(&gitfile_content, \"../\");\n> + s++;\n> + } while ((s = strchr(s, '/')));\n> + }\n> + strbuf_addstr(&gitfile_content, git_dir);\n> + strbuf_addch(&gitfile_content, '\\n');\n\nYuck. Just yuck. Why don't you just fopen the gitfile and write one\nline to it directly (ie. the absolute path of the worktree)? Why do\nyou need a strbuf for this at all? Ah, absolute paths would break if\nyou moved the entire superproject to a different location. This is\nreason #347 I don't like keeping GITDIRs in the superproject's\n.git/modules/. In any case, why don't you use relative_path() in\npath.c instead of reinventing the wheel?\n\n> + strbuf_addf(&gitfile_name, \"%s/.git\", work_tree);\n\nWhy work_tree, not real_work_tree?\nstrbuf is an overkill for this: why not mkpath()?\n\n> + fp = fopen(gitfile_name.buf, \"w\");\n> + if (!fp)\n> + die(_(\"Could not create git link %s\"), gitfile_name.buf);\n> + fprintf(fp, \"%s\", gitfile_content.buf);\n\nNot checking the return value of fprintf() for possible failure?\n\n> + fclose(fp);\n> +\n> + strbuf_release(&gitfile_content);\n> + strbuf_release(&gitfile_name);\n> +\n> + /*\n> + * Update core.worktree setting\n> + */\n> + if (git_dir[0]) {\n> + const char *s = git_dir;\n> + do {\n> + strbuf_addstr(&core_worktree_setting, \"../\");\n> + s++;\n> + } while ((s = strchr(s, '/')));\n> + }\n\nCan't use relative_path()?\n\n> + strbuf_addstr(&core_worktree_setting, real_work_tree);\n> +\n> + strbuf_addf(&configfile_name, \"%s/config\", git_dir);\n\nmkpath() to avoid the strbuf. If you have to use strbufs, why aren't\nyou re-using a \"pathbuf\" strbuf?\n"},{"id":"213914","messageId":"CALkWK0mWHwTV8dc9F3tyq9HYOnC2S56x_efr1+eRsqJqTFutXA@mail.gmail.com","threadId":"33371","inReplyTo":"515C8958.4080704@web.de","subject":"Re: [PATCH/RFC 1/3] Teach mv to move submodules together with their work trees","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-04-11T09:12:51Z","receivedAt":"2013-04-11T09:12:51Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Jens Lehmann wrote:\n> Currently the attempt to use \"git mv\" on a submodule errors out with:\n> fatal: source directory is empty, source=, destination=\n> The reason is that mv searches for the submodule with a trailing slash in\n> the index, which it doesn't find (because it is stored without a trailing\n> slash). As it doesn't find any index entries inside the submodule it\n> claims the directory would be empty even though it isn't.\n\nWhy does it search for a submodule with a trailing slash in the index?\n You make it sound like it's doing something unnatural; in reality, it\ndoes this because it executes lstat() on the filesystem path\nspecified, and the stat mode matches S_ISDIR (because it _is_ a\ndirectory on the filesystem). It is stored in the index as an entry\n(without a trailing slash) with the mode 160000, gitlink.\n\nWhat happens if I attempt to git mv oldpath/ newpath/ (with the\nliteral slashes, probably because I'm using a stupid shell\ncompletion)?\n\n> Fix that by searching for the name without a trailing slash and continue\n> if it is a submodule.\n\nSo, the correct solution is not to \"search for a name without a\ntrailing slash\", but rather to handle the gitlink entry in the index\nappropriately.\n\n> Then rename() will move the submodule work tree just\n> like it moves a file.\n\nWhat is this rename() function you're talking about? I don't see it anywhere.\n\n> diff --git a/builtin/mv.c b/builtin/mv.c\n> index 034fec9..361028d 100644\n> --- a/builtin/mv.c\n> +++ b/builtin/mv.c\n> @@ -117,55 +117,60 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n> && lstat(dst, &st) == 0)\n> bad = _(\"cannot move directory over file\");\n> else if (src_is_dir) {\n> - const char *src_w_slash = add_slash(src);\n> - int len_w_slash = length + 1;\n> - int first, last;\n> -\n> - modes[i] = WORKING_DIRECTORY;\n> -\n> - first = cache_name_pos(src_w_slash, len_w_slash);\n> - if (first >= 0)\n> - die (_(\"Huh? %.*s is in index?\"),\n> - len_w_slash, src_w_slash);\n> -\n> - first = -1 - first;\n> - for (last = first; last < active_nr; last++) {\n> - const char *path = active_cache[last]->name;\n> - if (strncmp(path, src_w_slash, len_w_slash))\n> - break;\n> - }\n> - free((char *)src_w_slash);\n> -\n> - if (last - first < 1)\n> - bad = _(\"source directory is empty\");\n> - else {\n> - int j, dst_len;\n> -\n> - if (last - first > 0) {\n> - source = xrealloc(source,\n> - (argc + last - first)\n> - * sizeof(char *));\n> - destination = xrealloc(destination,\n> - (argc + last - first)\n> - * sizeof(char *));\n> - modes = xrealloc(modes,\n> - (argc + last - first)\n> - * sizeof(enum update_mode));\n> + int first = cache_name_pos(src, length);\n> + if (first >= 0) {\n> + if (!S_ISGITLINK(active_cache[first]->ce_mode))\n> + die (_(\"Huh? Directory %s is in index and no submodule?\"), src);\n\nI didn't understand this. Why does it have to be a gitlink if it is\nstored at index position >= 0?\nI'm assuming the else-case has nothing to do with the actual moving\nbut rather something specific to directories (enumerating entries in\nit?), which is why it doesn't get executed when we find a gitlink.\n\n> + } else {\n> + const char *src_w_slash = add_slash(src);\n> + int last, len_w_slash = length + 1;\n> +\n> + modes[i] = WORKING_DIRECTORY;\n> +\n> + first = cache_name_pos(src_w_slash, len_w_slash);\n> + if (first >= 0)\n> + die (_(\"Huh? %.*s is in index?\"),\n> + len_w_slash, src_w_slash);\n> +\n> + first = -1 - first;\n> + for (last = first; last < active_nr; last++) {\n> + const char *path = active_cache[last]->name;\n> + if (strncmp(path, src_w_slash, len_w_slash))\n> + break;\n> }\n\nMostly unchanged, but I didn't understand the line first = -1 - first\nin the original. What is it doing? So, I'm guessing first is the\ncache position of the directory itself, and last stores the index of\nthe \"last\" entry in the cache? What does that even mean?\n\n> -\n> - dst = add_slash(dst);\n> - dst_len = strlen(dst);\n> -\n> - for (j = 0; j < last - first; j++) {\n> - const char *path =\n> - active_cache[first + j]->name;\n> - source[argc + j] = path;\n> - destination[argc + j] =\n> - prefix_path(dst, dst_len,\n> - path + length + 1);\n> - modes[argc + j] = INDEX;\n> + free((char *)src_w_slash);\n> +\n> + if (last - first < 1)\n> + bad = _(\"source directory is empty\");\n\nThis is exactly what was tripping us up earlier. Can you explain what\nlast - first < 1 means?\n\n> + else {\n> + int j, dst_len;\n> +\n> + if (last - first > 0) {\n> + source = xrealloc(source,\n> + (argc + last - first)\n> + * sizeof(char *));\n> + destination = xrealloc(destination,\n> + (argc + last - first)\n> + * sizeof(char *));\n> + modes = xrealloc(modes,\n> + (argc + last - first)\n> + * sizeof(enum update_mode));\n> + }\n> +\n> + dst = add_slash(dst);\n> + dst_len = strlen(dst);\n> +\n> + for (j = 0; j < last - first; j++) {\n> + const char *path =\n> + active_cache[first + j]->name;\n> + source[argc + j] = path;\n> + destination[argc + j] =\n> + prefix_path(dst, dst_len,\n> + path + length + 1);\n> + modes[argc + j] = INDEX;\n> + }\n> + argc += last - first;\n> }\n\nMostly unchanged, but hard to review because I can't easily see what\nchanged and what didn't.\n\n> - argc += last - first;\n\nWhy did you remove this line?\n"},{"id":"213917","messageId":"CALkWK0m08_FmrMqcdRgy6-_7Pc8LyWeZqdtfJ3GGTGNTLaR0QA@mail.gmail.com","threadId":"33371","inReplyTo":"515C89BA.9020806@web.de","subject":"Re: [PATCH/RFC 3/3] Teach mv to update the path entry in .gitmodules for moved submodules","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-04-11T10:06:51Z","receivedAt":"2013-04-11T10:06:51Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Jens Lehmann wrote:\n> Currently using \"git mv\" on a submodule moves the submodule's work tree in\n> that of the superproject.\n\nIt's not \"Currently\": it's the result of your last two patches. The\nwording is very unclear. Perhaps: As a result of the last two patches\nin this series, a 'git mv' moves the submodule directory to a new path\nin the superproject's worktree, and updates the cache entry in the\nindex appropriately.\n\n> But the submodule's path setting in .gitmodules\n> is left untouched, which is now inconsistent with the work tree and makes\n> git commands that rely on the proper path -> name mapping (like status and\n> diff) behave strangely.\n\nI'm frankly a little surprised that your previous two patches didn't\nbreak any tests. That's probably because we don't have enough tests\nto exercise git-core with submodules. I'm not saying that it's a bad\nthing though: submodules are still immature, and tight tests would get\nin the way of new patches.\n\n> Let \"git mv\" help here by not only moving the submodule's work tree but\n> also updating the \"submodule..path\" setting from the\n> .gitmodules file and stage both.\n\n> This doesn't happen when no .gitmodules\n> file is found and only issues a warning when it doesn't have a section for\n> this submodule. This is because the user might just use plain gitlinks\n> without the .gitmodules file\n\nHow will git-core work without a .gitmodules? Shouldn't we create a\nfresh .gitmodules here?\n\n> or has already updated the path setting by\n> hand before issuing the \"git mv\" command (in which case the warning\n> reminds him that mv would have done that for him).\n\nShouldn't these two cases issue different warnings?\n\nBesides, why is this explanation missing in your rm patch's commit message?\n\n> diff --git a/builtin/mv.c b/builtin/mv.c\n> index 609bbb8..36e5605 100644\n> --- a/builtin/mv.c\n> +++ b/builtin/mv.c\n> @@ -239,6 +242,9 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n> rename_cache_entry_at(pos, dst);\n> }\n>\n> + if (gitmodules_modified)\n> + stage_updated_gitmodules();\n> +\n\nI'm unhappy with this, but we I'll have to live with my\ndissatisfaction until we get rid of \".gitmodules\" (if that ever\nhappens). mv has a clear task: it should perform a filesystem mv,\nmanipulate the index, and write out the changed index. Adding an\nunrelated file to the index has nothing to do with its primary task.\n\n> diff --git a/submodule.c b/submodule.c\n> index eba9b42..fb742b4 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n\nAll my comments in the rm review apply here too, so I won't repeat myself.\n\n> @@ -10,6 +10,7 @@\n> #include \"string-list.h\"\n> #include \"sha1-array.h\"\n> #include \"argv-array.h\"\n> +#include \"blob.h\"\n\nInteresting. Let's see why you need blob.h.\n\n> static struct string_list config_name_for_path;\n> static struct string_list config_fetch_recurse_submodules_for_name;\n> @@ -30,6 +31,67 @@ static struct sha1_array ref_tips_after_fetch;\n> */\n> static int gitmodules_is_unmerged;\n>\n> +/*\n> + * Try to update the \"path\" entry in the \"submodule.\" section of the\n> + * .gitmodules file.\n> + */\n> +int update_path_in_gitmodules(const char *oldpath, const char *newpath)\n> +{\n> + struct strbuf entry = STRBUF_INIT;\n> + struct string_list_item *path_option;\n> +\n> + if (!file_exists(\".gitmodules\")) /* Do nothing whithout .gitmodules */\n> + return -1;\n\ns/whithout/without. Why are you not returning an error()? Don't you\nwant to tell the user that no \".gitmodules\" was found?\n\n> +void stage_updated_gitmodules(void)\n\nvoid return? Don't you want the caller to know whether we were\nsuccessful or not?\n\nWhy does the name contain \"updated\"? Why does the function care if I\nupdated the .gitmodules or not? It's just staging .gitmodules, as it\nis on the filesystem.\n\n> +{\n> + struct strbuf buf = STRBUF_INIT;\n> + struct stat st;\n> + int pos;\n> + struct cache_entry *ce;\n> + int namelen = strlen(\".gitmodules\");\n> +\n> + pos = cache_name_pos(\".gitmodules\", strlen(\".gitmodules\"));\n\nAnd you forgot about namelen just like that.\n\n> + if (pos < 0) {\n> + warning(_(\"could not find .gitmodules in index\"));\n> + return;\n> + }\n\nWhat could this mean? I might have an untracked .gitmodules file in\nmy worktree, or I might not have a .gitmodules file at all. Since\nyou're already checking the latter case in the previous function,\ncan't you persist the result, and return a more sensible error?\n\n> + ce = active_cache[pos];\n> + ce->ce_flags = namelen;\n> + if (strbuf_read_file(&buf, \".gitmodules\", 0) < 0)\n> + die(_(\"reading updated .gitmodules failed\"));\n\nYou're reading it because? What does \"_updated_ .gitmodules\" mean here?\n\n> + if (lstat(\".gitmodules\", &st) < 0)\n> + die_errno(_(\"unable to stat updated .gitmodules\"));\n> + fill_stat_cache_info(ce, &st);\n> + ce->ce_mode = ce_mode_from_stat(ce, st.st_mode);\n\nI don't understand why you're taking the pains to fill out a cache_entry.\n\n> + if (remove_file_from_cache(\".gitmodules\") < 0)\n> + die(_(\"unable to remove .gitmodules from index\"));\n\nYou already have pos, so why not just remove_cache_entry_at()?\n\n> + if (write_sha1_file(buf.buf, buf.len, blob_type, ce->sha1))\n> + die(_(\"adding updated .gitmodules failed\"));\n\nAh, you need blob.h for blob_type.\n\nWait, why are you just reading and writing .gitmodules? What changed?\n And why are you manually writing a blob to the object store? Doesn't\ngit-core already do this if you just add it to the index? See the\nS_IFLNK case in index_path().\n\nWhat happens if there's a race? Shouldn't you be locking .gitmodules\nbefore updating it, like we do with the index and just about\neverything else?\n\n> + if (add_cache_entry(ce, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE))\n> + die(_(\"staging updated .gitmodules failed\"));\n\nThis is all you need: it calls index_path() and writes your blob\nobject to the database.\n"},{"id":"213965","messageId":"7vk3o9jan8.fsf@alter.siamese.dyndns.org","threadId":"33371","inReplyTo":"CALkWK0mWHwTV8dc9F3tyq9HYOnC2S56x_efr1+eRsqJqTFutXA@mail.gmail.com","subject":"Re: [PATCH/RFC 1/3] Teach mv to move submodules together with their work trees","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-11T16:27:07Z","receivedAt":"2013-04-11T16:27:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra writes:\n\n>> Then rename() will move the submodule work tree just\n>> like it moves a file.\n>\n> What is this rename() function you're talking about? I don't see it anywhere.\n\n\"man 2 rename\"; it is called from a generic part of builtin/mv.c to\nrename one path to another and can move both files and directories.\n\n>> + if (first >= 0) {\n>> + if (!S_ISGITLINK(active_cache[first]->ce_mode))\n>> + die (_(\"Huh? Directory %s is in index and no submodule?\"), src);\n>\n> I didn't understand this. Why does it have to be a gitlink if it is\n> stored at index position >= 0?\n\nThe path is not in the middle of the conflict, but the index records\nsomething that is not a gitlink. E.g. you start from A (regular file)\nin the index, \"rm A && mkdir A\" and make it the top of a working\ntree of a separate project. \"git mv A elsewhere\" will say \"src_is_dir\"\nbut the index still thinks A should be a regular file blob.\n\n> I'm assuming the else-case has nothing to do with the actual moving\n> but rather something specific to directories (enumerating entries in\n> it?), which is why it doesn't get executed when we find a gitlink.\n\nIt wants to move all the paths in the directory to a new\ndestination, e.g. \"git mv srcdir dstdir\", and update \n\n\n>> + } else {\n>> + const char *src_w_slash = add_slash(src);\n>> + int last, len_w_slash = length + 1;\n>> +\n>> + modes[i] = WORKING_DIRECTORY;\n>> +\n>> + first = cache_name_pos(src_w_slash, len_w_slash);\n>> + if (first >= 0)\n>> + die (_(\"Huh? %.*s is in index?\"),\n>> + len_w_slash, src_w_slash);\n>> +\n>> + first = -1 - first;\n>> + for (last = first; last < active_nr; last++) {\n>> + const char *path = active_cache[last]->name;\n>> + if (strncmp(path, src_w_slash, len_w_slash))\n>> + break;\n>> }\n>\n> Mostly unchanged, but I didn't understand the line first = -1 - first\n> in the original. What is it doing? So, I'm guessing first is the\n> cache position of the directory itself, and last stores the index of\n> the \"last\" entry in the cache? What does that even mean?\n\ncache_name_pos() either returns the position for a path at stage #0\nthat exists in the index, or the position the given path _would_ be\ninserted if you were to add it to the index for a path that does not\nexist in the index at stage #0 (I think this part of the code does\nnot consider that the given path is unmerged, either by being sloppy\nor detecting that case much earlier---I didn't check), and when\ndoing the latter, it encodes the position by negating it and\noffsetting it by 1 (otherwise you cannot tell \"it would come at the\nvery beginning\" and \"it is at the very beginning\", because negated\nzero is still zero). The \"-1 - first\" is an idiom used everywhere\nby callers of cache_name_pos() to recover the latter from the\nreturned value.\n\nIf you start at a position \"src/\" would have been inserted, and\niterate over the index while the entry's path prefix-matches with\n\"src/\", you will find where the entries in the \"src/\" directory\nends.\n\n>> + if (last - first < 1)\n>> + bad = _(\"source directory is empty\");\n>\n> This is exactly what was tripping us up earlier. Can you explain what\n> last - first < 1 means?\n\nI think the above covers it.\n\nAsking questions to learn the basic part of Git internals on this\nlist, e.g. \"I found this existing code, and I do not understand what\nit is doing. Can somebody shed a light on it?\", is perfectly fine,\nbut can you do so outside the review discussion?\n\nIt clutters the review discussions when such a request for education\nis labeled as if it were a review or mixed with a message with\ngenuine review comments.\n"},{"id":"213967","messageId":"7vd2u1j9q5.fsf@alter.siamese.dyndns.org","threadId":"33371","inReplyTo":"CALkWK0mWHwTV8dc9F3tyq9HYOnC2S56x_efr1+eRsqJqTFutXA@mail.gmail.com","subject":"Re: [PATCH/RFC 1/3] Teach mv to move submodules together with their work trees","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-11T16:46:58Z","receivedAt":"2013-04-11T16:46:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra writes:\n\n> Why does it search for a submodule with a trailing slash in the index?\n> You make it sound like it's doing something unnatural; in reality, it\n> does this because it executes lstat() on the filesystem path\n> specified, and the stat mode matches S_ISDIR (because it _is_ a\n> directory on the filesystem). It is stored in the index as an entry\n> (without a trailing slash) with the mode 160000, gitlink.\n>\n> What happens if I attempt to git mv oldpath/ newpath/ (with the\n> literal slashes, probably because I'm using a stupid shell\n> completion)?\n\nI think it should work.\n\n\tmkdir a && >a/f && git add a/f && git mv a/ b/\n\n>> Fix that by searching for the name without a trailing slash and continue\n>> if it is a submodule.\n>\n> So, the correct solution is not to \"search for a name without a\n> trailing slash\", but rather to handle the gitlink entry in the index\n> appropriately.\n\nFor moving an entire directory's contents, because the index is\nflat, you would look for \"name/\", because you know all of the paths\ncontained in it will share that prefix.\n\nBut when dealing with a submodule, you need to see if the path that\nfound to be a directory in the working tree is a gitlink in the\nindex. And the way to do so is to look for it without trailing\nslash, because there is nothing hanging under it in the index.\n\nSo the right implementation of \"handle appropriately\" is to \"search\nwithout slash\". They are saying the same thing, and the latter is a\nmore specific way to point out in what way the existing code that\nwanted to delegate moving to \"submodule mv\" and not having to worry\nabout gitlinks was unprepared for it, and what change is needed to\nmake it \"handle appropriately\".\n\nSo I think there is no need to rephrase here, but your comment makes\nme wonder if the patch covers the case where you have a submodule at\n\"a/x\" and the user does \"git mv a/ b/\". The src_is_dir thing will\nnotice \"a/\" is a directory, finds all the paths inside a/ including\n\"a/x\" (and we won't see any paths inside the submodule's working\ntree) to \"b/\", and update the cache entries and the working tree.\nDoes the adjusting of the path for that moved submodule, which is a\ntheme for [PATCH 3/3], cover that case, too?\n\nAnother thing to wonder is if we correctly reject \"git mv a/x/f here\"\nin the same example where a/x is a directory. The path is beyond\nthe lower boundary of our working tree and should not be touched.\n"},{"id":"213971","messageId":"7v61ztj94u.fsf@alter.siamese.dyndns.org","threadId":"33371","inReplyTo":"515C89BA.9020806@web.de","subject":"Re: [PATCH/RFC 3/3] Teach mv to update the path entry in .gitmodules for moved submodules","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-11T16:59:45Z","receivedAt":"2013-04-11T16:59:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jens Lehmann writes:\n\n> Currently using \"git mv\" on a submodule moves the submodule's work tree in\n> that of the superproject. But the submodule's path setting in .gitmodules\n> is left untouched, which is now inconsistent with the work tree and makes\n> git commands that rely on the proper path -> name mapping (like status and\n> diff) behave strangely.\n>\n> Let \"git mv\" help here by not only moving the submodule's work tree but\n> also updating the \"submodule..path\" setting from the\n> .gitmodules file and stage both. This doesn't happen when no .gitmodules\n> file is found and only issues a warning when it doesn't have a section for\n> this submodule.\n\nShould it happen when the user has other changes to .gitmodules that\nhasn't been updated to the index?\n\nAs this is done in the same final \"per-path\" loop as adjusting the\ngitfile links, the worry I expressed in an earlier message about\n\"git mv a/ b/\" when \"a/x\" is a submodule turns out to be unfounded,\nwhich is good ;-)\n"},{"id":"213989","messageId":"7vmwt4j4ch.fsf@alter.siamese.dyndns.org","threadId":"33371","inReplyTo":"CALkWK0kUdKfAp36HSdoTOK_Wt-Rs_4f3HKHLbLi8e3pzx0iEGw@mail.gmail.com","subject":"Re: [PATCH v2 2/3] Teach mv to move submodules using a gitfile","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-11T18:43:10Z","receivedAt":"2013-04-11T18:43:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra writes:\n\n> Jens Lehmann wrote:\n>> When moving a submodule which uses a gitfile to point to the git directory\n>> stored in .git/modules/ of the superproject two changes must be made\n>> to make the submodule work: the .git file and the core.worktree setting\n>> must be adjusted to point from work tree to git directory and back.\n>\n> Isn't it untrue that the git directory is stored in\n> .git/modules/: it is stored in .git/modules/path/to/module.\n>\n> I thought the whole point of this complex scheme was to avoid name\n> conflicts with submodules with the same name in other directories.\n\nI think Jens is right on this one. There are three things the code needs\nto know about a submodule: its name, path and URL.\n\nA canonical use case to think about is a project that builds an\nappliance:\n\n * Its zero-th version only has the sources to the userspace for the\n appliance.\n\n * The first version adds the Linux kernel as a submodule bound at\n kernel/, taking it from git://k.org/linux-2.6.git/.\n\n * The second version adds a choice to build the appliance with the\n BSD kernel, and the project reorganizes the source tree to have\n Linux kernel at path linux/ and adds the bsd kernel at path bsd/.\n\n * By the time the third version is released, the URL to the Linux\n has migrated to git://k.org/linux.git/; it is still logically\n the same (i.e. continuation of) the old 2.6.git repository [*1*].\n\nWe would want to make it possible to \"git checkout\" smoothly between\nthese four versions. Switching from v1 to v0 would have to remove\nthe submodule working tree at kernel/ but the user may want to\nswitch back to v1 without having to re-download the kernel\nsubmodule, so the kernel/.git repository needs to be stashed away\nsomewhere. Somewhere in $GIT_DIR of the superproject, but where?\nSwitching from v1 to v2 would need to move kernel/ to linux/ and\nmove kernel/.git to linux/.git. The design choice made long time\nago (if you recall the collection of old threads I gave you some\ntime ago, this is what was called \"Steven's three-level thing\") was\nto give a stable \"logical\" name for the Linux kernel component, so\nthat no matter where in the working tree the version that happens to\nbe at the tip of the current branch has it, we know where in the\nsuperproject's .git/modules it is found.\n\nSo at the second version when we move the submodule that used to be\nat kernel/ to linux/, we move the working tree of it, adjust the\n\"path\" of the submodule, but keep the name. And that name gives an\nidentity to the submodule, and that is what is used as a key inside\n$GIT_DIR of the superproject to decide where the repository\n(together with its object store) of the submodule is stashed away.\n\n\n[Footnote]\n\n*1* I mentioned the URL thing only for completeness; it does not\ncome into play in the \"checkout\" scenario, but when you start\nthinking about remote interactions, you need to be aware of how that\nvalue and the one copied to the configuration upon \"submodule init\"\nneed to be managed. Which is a separate topic but is an integral\npart of the canonical example.\n\nIdeally, a user who has followed along the life of this project\nshould:\n\n * first encounter git://k.org/linux-2.6.git/ in v1; \"git submodule\n init\" would copy it to her .git/config in the superproject.\n\n * later notice that .gitmodules has git://k.org/linux.git/ location\n that she hasn't seen for the submodule, and is given a chance to\n have the URL entry updated in her .git/config.\n\nThis is becuase even when she checks out an older branch that has\n2.6 in .gitmodules, \"git submodule update\" _should_ go to the new\nURL, not to the defunct 2.6 URL. We do the \"copy initially\", but do\nnot do the latter \"offer a chance to update when seeing a new one\"\n(at least, not yet).\n"}]}