{"thread":{"id":"44534","subject":"[PATCHv2 1/4] submodule: use absolute path for computing relative path connecting","startedAt":"2016-11-22T19:23:17Z","lastAt":"2016-11-30T22:18:45Z","messageCount":14,"participants":["Stefan Beller","Duy Nguyen","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"306339","messageId":"20161122192235.6055-2-sbeller@google.com","threadId":"44534","inReplyTo":"20161122192235.6055-1-sbeller@google.com","subject":"[PATCHv2 1/4] submodule: use absolute path for computing relative path connecting","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-11-22T19:22:32Z","receivedAt":"2016-11-22T19:23:17Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The current caller of connect_work_tree_and_git_dir passes\nan absolute path for the `git_dir` parameter. In the future patch\nwe will also pass in relative path for `git_dir`. Extend the functionality\nof connect_work_tree_and_git_dir to take relative paths for parameters.\n\nWe could work around this in the future patch by computing the absolute\npath for the git_dir in the calling site, however accepting relative\npaths for either parameter makes the API for this function much harder\nto misuse.\n\nSigned-off-by: Stefan Beller \n---\n submodule.c | 12 +++++++-----\n 1 file changed, 7 insertions(+), 5 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex 6f7d883de9..66c5ce5a24 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -1227,23 +1227,25 @@ void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir)\n {\n \tstruct strbuf file_name = STRBUF_INIT;\n \tstruct strbuf rel_path = STRBUF_INIT;\n-\tconst char *real_work_tree = xstrdup(real_path(work_tree));\n+\tchar *real_git_dir = xstrdup(real_path(git_dir));\n+\tchar *real_work_tree = xstrdup(real_path(work_tree));\n \n \t/* Update gitfile */\n \tstrbuf_addf(&file_name, \"%s/.git\", work_tree);\n \twrite_file(file_name.buf, \"gitdir: %s\",\n-\t\t relative_path(git_dir, real_work_tree, &rel_path));\n+\t\t relative_path(real_git_dir, real_work_tree, &rel_path));\n \n \t/* Update core.worktree setting */\n \tstrbuf_reset(&file_name);\n-\tstrbuf_addf(&file_name, \"%s/config\", git_dir);\n+\tstrbuf_addf(&file_name, \"%s/config\", real_git_dir);\n \tgit_config_set_in_file(file_name.buf, \"core.worktree\",\n-\t\t\t relative_path(real_work_tree, git_dir,\n+\t\t\t relative_path(real_work_tree, real_git_dir,\n \t\t\t\t\t &rel_path));\n \n \tstrbuf_release(&file_name);\n \tstrbuf_release(&rel_path);\n-\tfree((void *)real_work_tree);\n+\tfree(real_work_tree);\n+\tfree(real_git_dir);\n }\n \n int parallel_submodules(void)\n-- \n2.11.0.rc2.4.g3396b6f.dirty\n\n"},{"id":"306340","messageId":"20161122192235.6055-4-sbeller@google.com","threadId":"44534","inReplyTo":"20161122192235.6055-1-sbeller@google.com","subject":"[PATCHv2 3/4] test-lib-functions.sh: teach test_commit -C ","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-11-22T19:22:34Z","receivedAt":"2016-11-22T19:23:30Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Specifically when setting up submodule tests, it comes in handy if\nwe can create commits in repositories that are not at the root of\nthe tested trash dir. Add \"-C \" similar to gits -C parameter\nthat will perform the operation in the given directory.\n\nSigned-off-by: Stefan Beller \n---\n t/test-lib-functions.sh | 20 +++++++++++++++-----\n 1 file changed, 15 insertions(+), 5 deletions(-)\n\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex fdaeb3a96b..579e812506 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -157,16 +157,21 @@ debug () {\n \t GIT_TEST_GDB=1 \"$@\"\n }\n \n-# Call test_commit with the arguments \" [ [ []]]\"\n+# Call test_commit with the arguments\n+# [-C ] [ [ []]]\"\n #\n # This will commit a file with the given contents and the given commit\n # message, and tag the resulting commit with the given tag name.\n #\n # , , and all default to .\n+#\n+# If the first argument is \"-C\", the second argument is used as a path for\n+# the git invocations.\n \n test_commit () {\n \tnotick= &&\n \tsignoff= &&\n+\tindir= &&\n \twhile test $# != 0\n \tdo\n \t\tcase \"$1\" in\n@@ -176,21 +181,26 @@ test_commit () {\n \t\t--signoff)\n \t\t\tsignoff=\"$1\"\n \t\t\t;;\n+\t\t-C)\n+\t\t\tindir=\"$2\"\n+\t\t\tshift\n+\t\t\t;;\n \t\t*)\n \t\t\tbreak\n \t\t\t;;\n \t\tesac\n \t\tshift\n \tdone &&\n+\tindir=${indir:+\"$indir\"/} &&\n \tfile=${2:-\"$1.t\"} &&\n-\techo \"${3-$1}\" > \"$file\" &&\n-\tgit add \"$file\" &&\n+\techo \"${3-$1}\" > \"$indir$file\" &&\n+\tgit ${indir:+ -C \"$indir\"} add \"$file\" &&\n \tif test -z \"$notick\"\n \tthen\n \t\ttest_tick\n \tfi &&\n-\tgit commit $signoff -m \"$1\" &&\n-\tgit tag \"${4:-$1}\"\n+\tgit ${indir:+ -C \"$indir\"} commit $signoff -m \"$1\" &&\n+\tgit ${indir:+ -C \"$indir\"} tag \"${4:-$1}\"\n }\n \n # Call test_merge with the arguments \" \", where \n-- \n2.11.0.rc2.4.g3396b6f.dirty\n\n"},{"id":"306341","messageId":"20161122192235.6055-1-sbeller@google.com","threadId":"44534","inReplyTo":null,"subject":"[PATCHv2 0/4] `submodule embedgitdirs` [was: Introduce `submodule interngitdirs`]","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-11-22T19:22:31Z","receivedAt":"2016-11-22T19:23:33Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"v2:\n* fixed commit message for patch:\n \"submodule: use absolute path for computing relative path connecting\"\n* a new patch \"submodule helper: support super prefix\"\n* redid the final patch with more tests and fixing bugs along the way\n* \"test-lib-functions.sh: teach test_commit -C \" unchanged\n\ndiff to v1 below.\n\nStefan Beller (4):\n submodule: use absolute path for computing relative path connecting\n submodule helper: support super prefix\n test-lib-functions.sh: teach test_commit -C \n submodule: add embed-git-dir function\n\n Documentation/git-submodule.txt | 14 +++++++\n builtin/submodule--helper.c | 62 +++++++++++++++++++++++++------\n git-submodule.sh | 7 +++-\n git.c | 2 +-\n submodule.c | 77 +++++++++++++++++++++++++++++++++++---\n submodule.h | 2 +\n t/t7412-submodule-embedgitdirs.sh | 78 +++++++++++++++++++++++++++++++++++++++\n t/test-lib-functions.sh | 20 +++++++---\n 8 files changed, 239 insertions(+), 23 deletions(-)\n create mode 100755 t/t7412-submodule-embedgitdirs.sh\n\nv1:\nThe discussion of the submodule checkout series revealed to me that a command\nis needed to move the git directory from the submodules working tree to be\nembedded into the superprojects git directory.\n\nSo I wrote the code to intern the submodules git dir into the superproject,\nbut whilst writing the code I realized this could be valueable for our use\nin testing too. So I exposed it via the submodule--helper. But as the\nsubmodule helper ought to be just an internal API, we could also\noffer it via the proper submodule command.\n\nThe command as it is has little value to the end user for now, but\nbreaking it out of the submodule checkout series hopefully makes review easier.\n\nThanks,\nStefan\n\ndiff to v1:\ndiff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt\nindex 80d55350eb..34791cfc65 100644\n--- a/Documentation/git-submodule.txt\n+++ b/Documentation/git-submodule.txt\n@@ -22,7 +22,8 @@ SYNOPSIS\n \t [commit] [--] [...]\n 'git submodule' [--quiet] foreach [--recursive] \n 'git submodule' [--quiet] sync [--recursive] [--] [...]\n-'git submodule' [--quiet] interngitdirs [--] [...]\n+'git submodule' [--quiet] embedgitdirs [--] [...]\n+\n \n DESCRIPTION\n -----------\n@@ -245,18 +246,18 @@ sync::\n If `--recursive` is specified, this command will recurse into the\n registered submodules, and sync any nested submodules within.\n \n-interngitdirs::\n+embedgitdirs::\n \tMove the git directory of submodules into its superprojects\n \t`$GIT_DIR/modules` path and then connect the git directory and\n \tits working directory by setting the `core.worktree` and adding\n \ta .git file pointing to the git directory interned into the\n \tsuperproject.\n +\n-\tA repository that was cloned independently and later added\n-\tas a submodule or old setups have the submodules git directory\n-\tinside the submodule instead of the\n+A repository that was cloned independently and later added as a submodule or\n+old setups have the submodules git directory inside the submodule instead of\n+embedded into the superprojects git directory.\n +\n-\tThis command is recursive by default.\n+This command is recursive by default.\n \n OPTIONS\n -------\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 256f8e9439..75cdbf45b8 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -1076,22 +1076,25 @@ static int resolve_remote_submodule_branch(int argc, const char **argv,\n \treturn 0;\n }\n \n-static int intern_git_dir(int argc, const char **argv, const char *prefix)\n+static int embed_git_dir(int argc, const char **argv, const char *prefix)\n {\n \tint i;\n \tstruct pathspec pathspec;\n \tstruct module_list list = MODULE_LIST_INIT;\n \n-\tstruct option intern_gitdir_options[] = {\n+\tstruct option embed_gitdir_options[] = {\n+\t\tOPT_STRING(0, \"prefix\", &prefix,\n+\t\t\t N_(\"path\"),\n+\t\t\t N_(\"path into the working tree\")),\n \t\tOPT_END()\n \t};\n \n \tconst char *const git_submodule_helper_usage[] = {\n-\t\tN_(\"git submodule--helper intern-git-dir [...]\"),\n+\t\tN_(\"git submodule--helper embed-git-dir [...]\"),\n \t\tNULL\n \t};\n \n-\targc = parse_options(argc, argv, prefix, intern_gitdir_options,\n+\targc = parse_options(argc, argv, prefix, embed_gitdir_options,\n \t\t\t git_submodule_helper_usage, 0);\n \n \tgitmodules_config();\n@@ -1101,27 +1104,30 @@ static int intern_git_dir(int argc, const char **argv, const char *prefix)\n \t\treturn 1;\n \n \tfor (i = 0; i < list.nr; i++)\n-\t\tmigrate_submodule_gitdir(list.entries[i]->name);\n+\t\tmigrate_submodule_gitdir(prefix, list.entries[i]->name, 1);\n \n \treturn 0;\n }\n \n+#define SUPPORT_SUPER_PREFIX (1<<0)\n+\n struct cmd_struct {\n \tconst char *cmd;\n \tint (*fn)(int, const char **, const char *);\n+\tint option;\n };\n \n static struct cmd_struct commands[] = {\n-\t{\"list\", module_list},\n-\t{\"name\", module_name},\n-\t{\"clone\", module_clone},\n-\t{\"update-clone\", update_clone},\n-\t{\"relative-path\", resolve_relative_path},\n-\t{\"resolve-relative-url\", resolve_relative_url},\n-\t{\"resolve-relative-url-test\", resolve_relative_url_test},\n-\t{\"init\", module_init},\n-\t{\"remote-branch\", resolve_remote_submodule_branch},\n-\t{\"intern-git-dir\", intern_git_dir}\n+\t{\"list\", module_list, 0},\n+\t{\"name\", module_name, 0},\n+\t{\"clone\", module_clone, 0},\n+\t{\"update-clone\", update_clone, 0},\n+\t{\"relative-path\", resolve_relative_path, 0},\n+\t{\"resolve-relative-url\", resolve_relative_url, 0},\n+\t{\"resolve-relative-url-test\", resolve_relative_url_test, 0},\n+\t{\"init\", module_init, 0},\n+\t{\"remote-branch\", resolve_remote_submodule_branch, 0},\n+\t{\"embed-git-dirs\", embed_git_dir, SUPPORT_SUPER_PREFIX}\n };\n \n int cmd_submodule__helper(int argc, const char **argv, const char *prefix)\n@@ -1131,9 +1137,15 @@ int cmd_submodule__helper(int argc, const char **argv, const char *prefix)\n \t\tdie(_(\"submodule--helper subcommand must be \"\n \t\t \"called with a subcommand\"));\n \n-\tfor (i = 0; i < ARRAY_SIZE(commands); i++)\n-\t\tif (!strcmp(argv[1], commands[i].cmd))\n+\tfor (i = 0; i < ARRAY_SIZE(commands); i++) {\n+\t\tif (!strcmp(argv[1], commands[i].cmd)) {\n+\t\t\tif (get_super_prefix() &&\n+\t\t\t !(commands[i].option & SUPPORT_SUPER_PREFIX))\n+\t\t\t\tdie(\"%s doesn't support --super-prefix\",\n+\t\t\t\t commands[i].cmd);\n \t\t\treturn commands[i].fn(argc - 1, argv + 1, prefix);\n+\t\t}\n+\t}\n \n \tdie(_(\"'%s' is not a valid submodule--helper \"\n \t \"subcommand\"), argv[1]);\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 747e934df2..b7e124f340 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -1131,9 +1131,9 @@ cmd_sync()\n \tdone\n }\n \n-cmd_interngitdirs()\n+cmd_embedgitdirs()\n {\n-\tgit submodule--helper intern-git-dir \"$@\"\n+\tgit submodule--helper embed-git-dirs --prefix \"$wt_prefix\" \"$@\"\n }\n \n # This loop parses the command line arguments to find the\n@@ -1145,7 +1145,7 @@ cmd_interngitdirs()\n while test $# != 0 && test -z \"$command\"\n do\n \tcase \"$1\" in\n-\tadd | foreach | init | deinit | update | status | summary | sync | interngitdirs)\n+\tadd | foreach | init | deinit | update | status | summary | sync | embedgitdirs)\n \t\tcommand=$1\n \t\t;;\n \t-q|--quiet)\ndiff --git a/git.c b/git.c\nindex efa1059fe0..98dcf6c518 100644\n--- a/git.c\n+++ b/git.c\n@@ -493,7 +493,7 @@ static struct cmd_struct commands[] = {\n \t{ \"stage\", cmd_add, RUN_SETUP | NEED_WORK_TREE },\n \t{ \"status\", cmd_status, RUN_SETUP | NEED_WORK_TREE },\n \t{ \"stripspace\", cmd_stripspace },\n-\t{ \"submodule--helper\", cmd_submodule__helper, RUN_SETUP },\n+\t{ \"submodule--helper\", cmd_submodule__helper, RUN_SETUP | SUPPORT_SUPER_PREFIX},\n \t{ \"symbolic-ref\", cmd_symbolic_ref, RUN_SETUP },\n \t{ \"tag\", cmd_tag, RUN_SETUP },\n \t{ \"unpack-file\", cmd_unpack_file, RUN_SETUP },\ndiff --git a/submodule.c b/submodule.c\nindex 99befdba85..d330b567a3 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -1269,22 +1269,12 @@ void prepare_submodule_repo_env(struct argv_array *out)\n * having its git directory within the working tree to the git dir nested\n * in its superprojects git dir under modules/.\n */\n-void migrate_submodule_gitdir(const char *path)\n+void migrate_submodule_gitdir(const char *prefix, const char *path,\n+\t\t\t int recursive)\n {\n \tchar *old_git_dir;\n \tconst char *new_git_dir;\n \tconst struct submodule *sub;\n-\tstruct child_process cp = CHILD_PROCESS_INIT;\n-\n-\tcp.git_cmd = 1;\n-\tcp.no_stdin = 1;\n-\tcp.dir = path;\n-\targv_array_pushl(&cp.args, \"submodule\", \"foreach\", \"--recursive\",\n-\t\t\t\"git\", \"submodule--helper\" \"intern-git-dir\", NULL);\n-\n-\tif (run_command(&cp))\n-\t\tdie(_(\"Could not migrate git directory in submodule '%s'\"),\n-\t\t path);\n \n \told_git_dir = xstrfmt(\"%s/.git\", path);\n \tif (read_gitfile(old_git_dir))\n@@ -1295,14 +1285,46 @@ void migrate_submodule_gitdir(const char *path)\n \tif (!sub)\n \t\tdie(_(\"Could not lookup name for submodule '%s'\"),\n \t\t path);\n+\n \tnew_git_dir = git_common_path(\"modules/%s\", sub->name);\n-\tmkdir_in_gitdir(\".git/modules\");\n+\tif (safe_create_leading_directories_const(new_git_dir) < 0)\n+\t\tdie(_(\"could not create directory '%s'\"), new_git_dir);\n+\n+\tif (!prefix)\n+\t\tprefix = get_super_prefix();\n+\tprintf(\"Migrating git directory of %s%s from\\n'%s' to\\n'%s'\\n\",\n+\t\tprefix ? prefix : \"\", path,\n+\t\treal_path(old_git_dir), new_git_dir);\n \n \tif (rename(old_git_dir, new_git_dir) < 0)\n \t\tdie_errno(_(\"Could not migrate git directory from '%s' to '%s'\"),\n \t\t\told_git_dir, new_git_dir);\n \n \tconnect_work_tree_and_git_dir(path, new_git_dir);\n+\n out:\n+\tif (recursive) {\n+\t\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\t\tstruct strbuf sb = STRBUF_INIT;\n+\n+\t\tif (get_super_prefix())\n+\t\t\tstrbuf_addstr(&sb, get_super_prefix());\n+\t\tstrbuf_addstr(&sb, path);\n+\t\tstrbuf_addch(&sb, '/');\n+\n+\t\tcp.dir = path;\n+\t\tcp.git_cmd = 1;\n+\t\tcp.no_stdin = 1;\n+\t\targv_array_pushl(&cp.args, \"--super-prefix\", sb.buf,\n+\t\t\t\t\t \"submodule--helper\",\n+\t\t\t\t\t \"embed-git-dirs\", NULL);\n+\t\tprepare_submodule_repo_env(&cp.env_array);\n+\t\tif (run_command(&cp))\n+\t\t\tdie(_(\"Could not migrate git directory in submodule '%s'\"),\n+\t\t\t path);\n+\n+\t\tstrbuf_release(&sb);\n+\t}\n+\n \tfree(old_git_dir);\n }\ndiff --git a/submodule.h b/submodule.h\nindex 859026ecfa..e5975d1f3d 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -75,5 +75,6 @@ int parallel_submodules(void);\n */\n void prepare_submodule_repo_env(struct argv_array *out);\n \n-extern void migrate_submodule_gitdir(const char *path);\n+extern void migrate_submodule_gitdir(const char *prefix,\n+\t\t\t\t const char *path, int recursive);\n #endif\ndiff --git a/t/t7412-submodule-embedgitdirs.sh b/t/t7412-submodule-embedgitdirs.sh\nnew file mode 100755\nindex 0000000000..3153dbc069\n--- /dev/null\n+++ b/t/t7412-submodule-embedgitdirs.sh\n@@ -0,0 +1,78 @@\n+#!/bin/sh\n+\n+test_description='Test submodule embedgitdirs\n+\n+This test verifies that `git submodue embedgitdirs` moves a submodules git\n+directory into the superproject.\n+'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup a real submodule' '\n+\tgit init sub1 &&\n+\ttest_commit -C sub1 first &&\n+\tgit submodule add ./sub1 &&\n+\ttest_tick &&\n+\tgit commit -m superproject\n+'\n+\n+test_expect_success 'embed the git dir' '\n+\t>expect.1 &&\n+\t>expect.2 &&\n+\t>actual.1 &&\n+\t>actual.2 &&\n+\tgit status >expect.1 &&\n+\tgit -C sub1 rev-parse HEAD >expect.2 &&\n+\tgit submodule embedgitdirs &&\n+\tgit fsck &&\n+\ttest -f sub1/.git &&\n+\ttest -d .git/modules/sub1 &&\n+\tgit status >actual.1 &&\n+\tgit -C sub1 rev-parse HEAD >actual.2 &&\n+\ttest_cmp expect.1 actual.1 &&\n+\ttest_cmp expect.2 actual.2\n+'\n+\n+test_expect_success 'setup nested submodule' '\n+\tgit init sub1/nested &&\n+\ttest_commit -C sub1/nested first_nested &&\n+\tgit -C sub1 submodule add ./nested &&\n+\ttest_tick &&\n+\tgit -C sub1 commit -m \"add nested\" &&\n+\tgit add sub1 &&\n+\tgit commit -m \"sub1 to include nested submodule\"\n+'\n+\n+test_expect_success 'embed the git dir in a nested submodule' '\n+\tgit status >expect.1 &&\n+\tgit -C sub1/nested rev-parse HEAD >expect.2 &&\n+\tgit submodule embedgitdirs &&\n+\ttest -f sub1/nested/.git &&\n+\ttest -d .git/modules/sub1/modules/nested &&\n+\tgit status >actual.1 &&\n+\tgit -C sub1/nested rev-parse HEAD >actual.2 &&\n+\ttest_cmp expect.1 actual.1 &&\n+\ttest_cmp expect.2 actual.2\n+'\n+\n+test_expect_success 'setup a gitlink with missing .gitmodules entry' '\n+\tgit init sub2 &&\n+\ttest_commit -C sub2 first &&\n+\tgit add sub2 &&\n+\tgit commit -m superproject\n+'\n+\n+test_expect_success 'embedding the git dir fails for incomplete submodules' '\n+\tgit status >expect.1 &&\n+\tgit -C sub2 rev-parse HEAD >expect.2 &&\n+\ttest_must_fail git submodule embedgitdirs &&\n+\tgit -C sub2 fsck &&\n+\ttest -d sub2/.git &&\n+\tgit status >actual &&\n+\tgit -C sub2 rev-parse HEAD >actual.2 &&\n+\ttest_cmp expect.1 actual.1 &&\n+\ttest_cmp expect.2 actual.2\n+'\n+\n+test_done\n+\ndiff --git a/t/t7412-submodule-interngitdirs.sh b/t/t7412-submodule-interngitdirs.sh\ndeleted file mode 100755\nindex 709359155d..0000000000\n--- a/t/t7412-submodule-interngitdirs.sh\n+++ /dev/null\n@@ -1,40 +0,0 @@\n-#!/bin/sh\n-\n-test_description='Test submodule interngitdirs\n-\n-This test verifies that `git submodue interngitdirs` moves a submodules git\n-directory into the superproject.\n-'\n-\n-. ./test-lib.sh\n-\n-test_expect_success 'setup a real submodule' '\n-\tgit init sub1 &&\n-\ttest_commit -C sub1 first &&\n-\tgit submodule add ./sub1 &&\n-\ttest_tick &&\n-\tgit commit -m superproject\n-'\n-\n-test_expect_success 'intern the git dir' '\n-\tgit submodule interngitdirs &&\n-\ttest -f sub1/.git &&\n-\ttest -d .git/modules/sub1 &&\n-\t# check that we did not break the repository:\n-\tgit status\n-'\n-\n-test_expect_success 'setup a gitlink with missing .gitmodules entry' '\n-\tgit init sub2 &&\n-\ttest_commit -C sub2 first &&\n-\tgit add sub2 &&\n-\tgit commit -m superproject\n-'\n-\n-test_expect_success 'intern the git dir fails for incomplete submodules' '\n-\ttest_must_fail git submodule interngitdirs &&\n-\t# check that we did not break the repository:\n-\tgit status\n-'\n-\n-test_done\n\n\n\n-- \n2.11.0.rc2.4.g3396b6f.dirty\n\n"},{"id":"306342","messageId":"20161122192235.6055-3-sbeller@google.com","threadId":"44534","inReplyTo":"20161122192235.6055-1-sbeller@google.com","subject":"[PATCHv2 2/4] submodule helper: support super prefix","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-11-22T19:22:33Z","receivedAt":"2016-11-22T19:23:34Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Just like main commands in Git, the submodule helper needs\naccess to the superproject prefix. Enable this in the git.c\nbut have its own fuse in the helper code by having a flag to\nturn on the super prefix.\n\nSigned-off-by: Stefan Beller \n---\n builtin/submodule--helper.c | 31 ++++++++++++++++++++-----------\n git.c | 2 +-\n 2 files changed, 21 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 4beeda5f9f..806e29ce4e 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -1076,21 +1076,24 @@ static int resolve_remote_submodule_branch(int argc, const char **argv,\n \treturn 0;\n }\n \n+#define SUPPORT_SUPER_PREFIX (1<<0)\n+\n struct cmd_struct {\n \tconst char *cmd;\n \tint (*fn)(int, const char **, const char *);\n+\tint option;\n };\n \n static struct cmd_struct commands[] = {\n-\t{\"list\", module_list},\n-\t{\"name\", module_name},\n-\t{\"clone\", module_clone},\n-\t{\"update-clone\", update_clone},\n-\t{\"relative-path\", resolve_relative_path},\n-\t{\"resolve-relative-url\", resolve_relative_url},\n-\t{\"resolve-relative-url-test\", resolve_relative_url_test},\n-\t{\"init\", module_init},\n-\t{\"remote-branch\", resolve_remote_submodule_branch}\n+\t{\"list\", module_list, 0},\n+\t{\"name\", module_name, 0},\n+\t{\"clone\", module_clone, 0},\n+\t{\"update-clone\", update_clone, 0},\n+\t{\"relative-path\", resolve_relative_path, 0},\n+\t{\"resolve-relative-url\", resolve_relative_url, 0},\n+\t{\"resolve-relative-url-test\", resolve_relative_url_test, 0},\n+\t{\"init\", module_init, 0},\n+\t{\"remote-branch\", resolve_remote_submodule_branch, 0}\n };\n \n int cmd_submodule__helper(int argc, const char **argv, const char *prefix)\n@@ -1100,9 +1103,15 @@ int cmd_submodule__helper(int argc, const char **argv, const char *prefix)\n \t\tdie(_(\"submodule--helper subcommand must be \"\n \t\t \"called with a subcommand\"));\n \n-\tfor (i = 0; i < ARRAY_SIZE(commands); i++)\n-\t\tif (!strcmp(argv[1], commands[i].cmd))\n+\tfor (i = 0; i < ARRAY_SIZE(commands); i++) {\n+\t\tif (!strcmp(argv[1], commands[i].cmd)) {\n+\t\t\tif (get_super_prefix() &&\n+\t\t\t !(commands[i].option & SUPPORT_SUPER_PREFIX))\n+\t\t\t\tdie(\"%s doesn't support --super-prefix\",\n+\t\t\t\t commands[i].cmd);\n \t\t\treturn commands[i].fn(argc - 1, argv + 1, prefix);\n+\t\t}\n+\t}\n \n \tdie(_(\"'%s' is not a valid submodule--helper \"\n \t \"subcommand\"), argv[1]);\ndiff --git a/git.c b/git.c\nindex efa1059fe0..98dcf6c518 100644\n--- a/git.c\n+++ b/git.c\n@@ -493,7 +493,7 @@ static struct cmd_struct commands[] = {\n \t{ \"stage\", cmd_add, RUN_SETUP | NEED_WORK_TREE },\n \t{ \"status\", cmd_status, RUN_SETUP | NEED_WORK_TREE },\n \t{ \"stripspace\", cmd_stripspace },\n-\t{ \"submodule--helper\", cmd_submodule__helper, RUN_SETUP },\n+\t{ \"submodule--helper\", cmd_submodule__helper, RUN_SETUP | SUPPORT_SUPER_PREFIX},\n \t{ \"symbolic-ref\", cmd_symbolic_ref, RUN_SETUP },\n \t{ \"tag\", cmd_tag, RUN_SETUP },\n \t{ \"unpack-file\", cmd_unpack_file, RUN_SETUP },\n-- \n2.11.0.rc2.4.g3396b6f.dirty\n\n"},{"id":"306343","messageId":"20161122192235.6055-5-sbeller@google.com","threadId":"44534","inReplyTo":"20161122192235.6055-1-sbeller@google.com","subject":"[PATCHv2 4/4] submodule: add embed-git-dir function","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-11-22T19:22:35Z","receivedAt":"2016-11-22T19:23:50Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"When a submodule has its git dir inside the working dir, the submodule\nsupport for checkout that we plan to add in a later patch will fail.\n\nAdd functionality to migrate the git directory to be embedded\ninto the superprojects git directory.\n\nSigned-off-by: Stefan Beller \n---\n Documentation/git-submodule.txt | 14 +++++++\n builtin/submodule--helper.c | 33 ++++++++++++++++-\n git-submodule.sh | 7 +++-\n submodule.c | 65 ++++++++++++++++++++++++++++++++\n submodule.h | 2 +\n t/t7412-submodule-embedgitdirs.sh | 78 +++++++++++++++++++++++++++++++++++++++\n 6 files changed, 197 insertions(+), 2 deletions(-)\n create mode 100755 t/t7412-submodule-embedgitdirs.sh\n\ndiff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt\nindex d841573475..34791cfc65 100644\n--- a/Documentation/git-submodule.txt\n+++ b/Documentation/git-submodule.txt\n@@ -22,6 +22,7 @@ SYNOPSIS\n \t [commit] [--] [...]\n 'git submodule' [--quiet] foreach [--recursive] \n 'git submodule' [--quiet] sync [--recursive] [--] [...]\n+'git submodule' [--quiet] embedgitdirs [--] [...]\n \n \n DESCRIPTION\n@@ -245,6 +246,19 @@ sync::\n If `--recursive` is specified, this command will recurse into the\n registered submodules, and sync any nested submodules within.\n \n+embedgitdirs::\n+\tMove the git directory of submodules into its superprojects\n+\t`$GIT_DIR/modules` path and then connect the git directory and\n+\tits working directory by setting the `core.worktree` and adding\n+\ta .git file pointing to the git directory interned into the\n+\tsuperproject.\n++\n+A repository that was cloned independently and later added as a submodule or\n+old setups have the submodules git directory inside the submodule instead of\n+embedded into the superprojects git directory.\n++\n+This command is recursive by default.\n+\n OPTIONS\n -------\n -q::\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 806e29ce4e..e94dd68a0e 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -1076,6 +1076,36 @@ static int resolve_remote_submodule_branch(int argc, const char **argv,\n \treturn 0;\n }\n \n+static int embed_git_dir(int argc, const char **argv, const char *prefix)\n+{\n+\tint i;\n+\tstruct pathspec pathspec;\n+\tstruct module_list list = MODULE_LIST_INIT;\n+\n+\tstruct option embed_gitdir_options[] = {\n+\t\tOPT_END()\n+\t};\n+\n+\tconst char *const git_submodule_helper_usage[] = {\n+\t\tN_(\"git submodule--helper embed-git-dir [...]\"),\n+\t\tNULL\n+\t};\n+\n+\targc = parse_options(argc, argv, prefix, embed_gitdir_options,\n+\t\t\t git_submodule_helper_usage, 0);\n+\n+\tgitmodules_config();\n+\tgit_config(submodule_config, NULL);\n+\n+\tif (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)\n+\t\treturn 1;\n+\n+\tfor (i = 0; i < list.nr; i++)\n+\t\tmigrate_submodule_gitdir(prefix, list.entries[i]->name, 1);\n+\n+\treturn 0;\n+}\n+\n #define SUPPORT_SUPER_PREFIX (1<<0)\n \n struct cmd_struct {\n@@ -1093,7 +1123,8 @@ static struct cmd_struct commands[] = {\n \t{\"resolve-relative-url\", resolve_relative_url, 0},\n \t{\"resolve-relative-url-test\", resolve_relative_url_test, 0},\n \t{\"init\", module_init, 0},\n-\t{\"remote-branch\", resolve_remote_submodule_branch, 0}\n+\t{\"remote-branch\", resolve_remote_submodule_branch, 0},\n+\t{\"embed-git-dirs\", embed_git_dir, SUPPORT_SUPER_PREFIX}\n };\n \n int cmd_submodule__helper(int argc, const char **argv, const char *prefix)\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex a024a135d6..2178248287 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -1131,6 +1131,11 @@ cmd_sync()\n \tdone\n }\n \n+cmd_embedgitdirs()\n+{\n+\tgit submodule--helper --prefix \"$wt_prefix\" embed-git-dirs \"$@\"\n+}\n+\n # This loop parses the command line arguments to find the\n # subcommand name to dispatch. Parsing of the subcommand specific\n # options are primarily done by the subcommand implementations.\n@@ -1140,7 +1145,7 @@ cmd_sync()\n while test $# != 0 && test -z \"$command\"\n do\n \tcase \"$1\" in\n-\tadd | foreach | init | deinit | update | status | summary | sync)\n+\tadd | foreach | init | deinit | update | status | summary | sync | embedgitdirs)\n \t\tcommand=$1\n \t\t;;\n \t-q|--quiet)\ndiff --git a/submodule.c b/submodule.c\nindex 66c5ce5a24..d330b567a3 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -1263,3 +1263,68 @@ void prepare_submodule_repo_env(struct argv_array *out)\n \t}\n \targv_array_push(out, \"GIT_DIR=.git\");\n }\n+\n+/*\n+ * Migrate the given submodule (and all its submodules recursively) from\n+ * having its git directory within the working tree to the git dir nested\n+ * in its superprojects git dir under modules/.\n+ */\n+void migrate_submodule_gitdir(const char *prefix, const char *path,\n+\t\t\t int recursive)\n+{\n+\tchar *old_git_dir;\n+\tconst char *new_git_dir;\n+\tconst struct submodule *sub;\n+\n+\told_git_dir = xstrfmt(\"%s/.git\", path);\n+\tif (read_gitfile(old_git_dir))\n+\t\t/* If it is an actual gitfile, it doesn't need migration. */\n+\t\tgoto out;\n+\n+\tsub = submodule_from_path(null_sha1, path);\n+\tif (!sub)\n+\t\tdie(_(\"Could not lookup name for submodule '%s'\"),\n+\t\t path);\n+\n+\tnew_git_dir = git_common_path(\"modules/%s\", sub->name);\n+\tif (safe_create_leading_directories_const(new_git_dir) < 0)\n+\t\tdie(_(\"could not create directory '%s'\"), new_git_dir);\n+\n+\tif (!prefix)\n+\t\tprefix = get_super_prefix();\n+\tprintf(\"Migrating git directory of %s%s from\\n'%s' to\\n'%s'\\n\",\n+\t\tprefix ? prefix : \"\", path,\n+\t\treal_path(old_git_dir), new_git_dir);\n+\n+\tif (rename(old_git_dir, new_git_dir) < 0)\n+\t\tdie_errno(_(\"Could not migrate git directory from '%s' to '%s'\"),\n+\t\t\told_git_dir, new_git_dir);\n+\n+\tconnect_work_tree_and_git_dir(path, new_git_dir);\n+\n+out:\n+\tif (recursive) {\n+\t\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\t\tstruct strbuf sb = STRBUF_INIT;\n+\n+\t\tif (get_super_prefix())\n+\t\t\tstrbuf_addstr(&sb, get_super_prefix());\n+\t\tstrbuf_addstr(&sb, path);\n+\t\tstrbuf_addch(&sb, '/');\n+\n+\t\tcp.dir = path;\n+\t\tcp.git_cmd = 1;\n+\t\tcp.no_stdin = 1;\n+\t\targv_array_pushl(&cp.args, \"--super-prefix\", sb.buf,\n+\t\t\t\t\t \"submodule--helper\",\n+\t\t\t\t\t \"embed-git-dirs\", NULL);\n+\t\tprepare_submodule_repo_env(&cp.env_array);\n+\t\tif (run_command(&cp))\n+\t\t\tdie(_(\"Could not migrate git directory in submodule '%s'\"),\n+\t\t\t path);\n+\n+\t\tstrbuf_release(&sb);\n+\t}\n+\n+\tfree(old_git_dir);\n+}\ndiff --git a/submodule.h b/submodule.h\nindex d9e197a948..e5975d1f3d 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -75,4 +75,6 @@ int parallel_submodules(void);\n */\n void prepare_submodule_repo_env(struct argv_array *out);\n \n+extern void migrate_submodule_gitdir(const char *prefix,\n+\t\t\t\t const char *path, int recursive);\n #endif\ndiff --git a/t/t7412-submodule-embedgitdirs.sh b/t/t7412-submodule-embedgitdirs.sh\nnew file mode 100755\nindex 0000000000..3153dbc069\n--- /dev/null\n+++ b/t/t7412-submodule-embedgitdirs.sh\n@@ -0,0 +1,78 @@\n+#!/bin/sh\n+\n+test_description='Test submodule embedgitdirs\n+\n+This test verifies that `git submodue embedgitdirs` moves a submodules git\n+directory into the superproject.\n+'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup a real submodule' '\n+\tgit init sub1 &&\n+\ttest_commit -C sub1 first &&\n+\tgit submodule add ./sub1 &&\n+\ttest_tick &&\n+\tgit commit -m superproject\n+'\n+\n+test_expect_success 'embed the git dir' '\n+\t>expect.1 &&\n+\t>expect.2 &&\n+\t>actual.1 &&\n+\t>actual.2 &&\n+\tgit status >expect.1 &&\n+\tgit -C sub1 rev-parse HEAD >expect.2 &&\n+\tgit submodule embedgitdirs &&\n+\tgit fsck &&\n+\ttest -f sub1/.git &&\n+\ttest -d .git/modules/sub1 &&\n+\tgit status >actual.1 &&\n+\tgit -C sub1 rev-parse HEAD >actual.2 &&\n+\ttest_cmp expect.1 actual.1 &&\n+\ttest_cmp expect.2 actual.2\n+'\n+\n+test_expect_success 'setup nested submodule' '\n+\tgit init sub1/nested &&\n+\ttest_commit -C sub1/nested first_nested &&\n+\tgit -C sub1 submodule add ./nested &&\n+\ttest_tick &&\n+\tgit -C sub1 commit -m \"add nested\" &&\n+\tgit add sub1 &&\n+\tgit commit -m \"sub1 to include nested submodule\"\n+'\n+\n+test_expect_success 'embed the git dir in a nested submodule' '\n+\tgit status >expect.1 &&\n+\tgit -C sub1/nested rev-parse HEAD >expect.2 &&\n+\tgit submodule embedgitdirs &&\n+\ttest -f sub1/nested/.git &&\n+\ttest -d .git/modules/sub1/modules/nested &&\n+\tgit status >actual.1 &&\n+\tgit -C sub1/nested rev-parse HEAD >actual.2 &&\n+\ttest_cmp expect.1 actual.1 &&\n+\ttest_cmp expect.2 actual.2\n+'\n+\n+test_expect_success 'setup a gitlink with missing .gitmodules entry' '\n+\tgit init sub2 &&\n+\ttest_commit -C sub2 first &&\n+\tgit add sub2 &&\n+\tgit commit -m superproject\n+'\n+\n+test_expect_success 'embedding the git dir fails for incomplete submodules' '\n+\tgit status >expect.1 &&\n+\tgit -C sub2 rev-parse HEAD >expect.2 &&\n+\ttest_must_fail git submodule embedgitdirs &&\n+\tgit -C sub2 fsck &&\n+\ttest -d sub2/.git &&\n+\tgit status >actual &&\n+\tgit -C sub2 rev-parse HEAD >actual.2 &&\n+\ttest_cmp expect.1 actual.1 &&\n+\ttest_cmp expect.2 actual.2\n+'\n+\n+test_done\n+\n-- \n2.11.0.rc2.4.g3396b6f.dirty\n\n"},{"id":"306345","messageId":"20161122193305.7985-1-sbeller@google.com","threadId":"44534","inReplyTo":"20161122192235.6055-5-sbeller@google.com","subject":"[PATCH] SQUASH","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-11-22T19:33:05Z","receivedAt":"2016-11-22T19:34:37Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller \n---\n\nOn Tue, Nov 22, 2016 at 11:22 AM, Stefan Beller wrote:\n> When a submodule has its git dir inside the working dir, the submodule\n> support for checkout that we plan to add in a later patch will fail.\n>\n> Add functionality to migrate the git directory to be embedded\n> into the superprojects git directory.\n>\n\nI spoke too early, this needs to be squashed. :(\n\nStefan\n\n builtin/submodule--helper.c | 3 +++\n git-submodule.sh | 2 +-\n 2 files changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex e94dd68a0e..75cdbf45b8 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -1083,6 +1083,9 @@ static int embed_git_dir(int argc, const char **argv, const char *prefix)\n \tstruct module_list list = MODULE_LIST_INIT;\n \n \tstruct option embed_gitdir_options[] = {\n+\t\tOPT_STRING(0, \"prefix\", &prefix,\n+\t\t\t N_(\"path\"),\n+\t\t\t N_(\"path into the working tree\")),\n \t\tOPT_END()\n \t};\n \ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 2178248287..b7e124f340 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -1133,7 +1133,7 @@ cmd_sync()\n \n cmd_embedgitdirs()\n {\n-\tgit submodule--helper --prefix \"$wt_prefix\" embed-git-dirs \"$@\"\n+\tgit submodule--helper embed-git-dirs --prefix \"$wt_prefix\" \"$@\"\n }\n \n # This loop parses the command line arguments to find the\n-- \n2.11.0.rc2.4.g3396b6f.dirty\n\n"},{"id":"306586","messageId":"CACsJy8Ce3Oa-xJ4BwgRRy6neM=Jxkfqq7yboHZDXLDG2tu9GzQ@mail.gmail.com","threadId":"44534","inReplyTo":"20161122192235.6055-5-sbeller@google.com","subject":"Re: [PATCHv2 4/4] submodule: add embed-git-dir function","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-11-30T11:14:11Z","receivedAt":"2016-11-30T11:14:48Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Nov 23, 2016 at 2:22 AM, Stefan Beller wrote:\n> +/*\n> + * Migrate the given submodule (and all its submodules recursively) from\n> + * having its git directory within the working tree to the git dir nested\n> + * in its superprojects git dir under modules/.\n> + */\n> +void migrate_submodule_gitdir(const char *prefix, const char *path,\n> + int recursive)\n\nSubmodules and worktrees seem to have many things in common. The first\none is this. \"git worktree move\" on a worktree that contains\nsubmodules .git also benefits from something like this [1]. I suggest\nyou move this function to some neutral place and maybe rename it to\nrelocate_gitdir() or something.\n\nIt probably should take a bit flag instead of \"recursive\" here. One\nthing I would need is the ability to tell this function \"I have moved\nall these .git dirs already (because I move whole worktree in one\noperation), here are the old and new locations of them, fix them up!\".\nIn other words, no rename() could be optionally skipped.\n\n[1] https://public-inbox.org/git/20161128094319.16176-11-pclouds@gmail.com/T/#u\n\n> +{\n> + char *old_git_dir;\n> + const char *new_git_dir;\n> + const struct submodule *sub;\n> +\n> + old_git_dir = xstrfmt(\"%s/.git\", path);\n> + if (read_gitfile(old_git_dir))\n> + /* If it is an actual gitfile, it doesn't need migration. */\n> + goto out;\n> +\n> + sub = submodule_from_path(null_sha1, path);\n> + if (!sub)\n> + die(_(\"Could not lookup name for submodule '%s'\"),\n> + path);\n> +\n> + new_git_dir = git_common_path(\"modules/%s\", sub->name);\n\nWhy doesn't git_path() work here? This would make \"modules\" shared\nbetween worktrees, even though it's not normally. That inconsistency\ncould cause trouble.\n\n> + if (safe_create_leading_directories_const(new_git_dir) < 0)\n> + die(_(\"could not create directory '%s'\"), new_git_dir);\n> +\n> + if (!prefix)\n> + prefix = get_super_prefix();\n> + printf(\"Migrating git directory of %s%s from\\n'%s' to\\n'%s'\\n\",\n> + prefix ? prefix : \"\", path,\n> + real_path(old_git_dir), new_git_dir);\n> +\n> + if (rename(old_git_dir, new_git_dir) < 0)\n> + die_errno(_(\"Could not migrate git directory from '%s' to '%s'\"),\n> + old_git_dir, new_git_dir);\n> +\n> + connect_work_tree_and_git_dir(path, new_git_dir);\n\nAnother thing in common is, both submodules and worktrees use some\nform of textual symlinks. You need to fix up some here. But if this\nsubmodule has multiple worktreee, there may be some \"symlinks\" in\n.git/worktrees which would need fixing up as well.\n\nYou don't have to do the fix up thing right away, but I think we\nshould at least make sure we leave no dangling links behind (by\ndie()ing early if we find a .git dir we can't handle yet)\n-- \nDuy\n"},{"id":"306593","messageId":"CAGZ79kbvey_f8+R16yYT_qsF0RErOh8own8n-RRApTM0dS-+ag@mail.gmail.com","threadId":"44534","inReplyTo":"CACsJy8Ce3Oa-xJ4BwgRRy6neM=Jxkfqq7yboHZDXLDG2tu9GzQ@mail.gmail.com","subject":"Re: [PATCHv2 4/4] submodule: add embed-git-dir function","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-11-30T18:04:52Z","receivedAt":"2016-11-30T18:04:58Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Nov 30, 2016 at 3:14 AM, Duy Nguyen wrote:\n> On Wed, Nov 23, 2016 at 2:22 AM, Stefan Beller wrote:\n>> +/*\n>> + * Migrate the given submodule (and all its submodules recursively) from\n>> + * having its git directory within the working tree to the git dir nested\n>> + * in its superprojects git dir under modules/.\n>> + */\n>> +void migrate_submodule_gitdir(const char *prefix, const char *path,\n>> + int recursive)\n>\n> Submodules and worktrees seem to have many things in common.\n\nYes. :)\n\n> The first\n> one is this. \"git worktree move\" on a worktree that contains\n> submodules .git also benefits from something like this [1].\n\nThat patch is a sensible approach. :)\n(By checking all files to not be submodules a worktree would not run\ninto problems like\na127331cd81, mv: allow moving nested submodules)\n\n> I suggest\n> you move this function to some neutral place and maybe rename it to\n> relocate_gitdir() or something.\n\nok tell me where this neutral place is found?\n(I'd prefer to not clobber it into cache.h *the* most neutral place in git)\nMaybe dir.{c,h} ?\n\n>\n> It probably should take a bit flag instead of \"recursive\" here. One\n> thing I would need is the ability to tell this function \"I have moved\n> all these .git dirs already (because I move whole worktree in one\n> operation), here are the old and new locations of them, fix them up!\".\n> In other words, no rename() could be optionally skipped.\n\nIn the non-main working trees you'd also have a .git file linking\nto the actual git dir and you'd only have to fix them up instead of moving.\n\n\n>\n> [1] https://public-inbox.org/git/20161128094319.16176-11-pclouds@gmail.com/T/#u\n>\n>> +{\n>> + char *old_git_dir;\n>> + const char *new_git_dir;\n>> + const struct submodule *sub;\n>> +\n>> + old_git_dir = xstrfmt(\"%s/.git\", path);\n>> + if (read_gitfile(old_git_dir))\n>> + /* If it is an actual gitfile, it doesn't need migration. */\n>> + goto out;\n>> +\n>> + sub = submodule_from_path(null_sha1, path);\n>> + if (!sub)\n>> + die(_(\"Could not lookup name for submodule '%s'\"),\n>> + path);\n>> +\n>> + new_git_dir = git_common_path(\"modules/%s\", sub->name);\n>\n> Why doesn't git_path() work here? This would make \"modules\" shared\n> between worktrees, even though it's not normally. That inconsistency\n> could cause trouble.\n\nI thought that was a long term goal?\n(I actually think about reviving the series you sent out a few weeks ago\nto make worktree and submodules work well together)\n\nSo for that we'd want to have at least the object store shared across all\nworktrees.\n\n>\n>> + if (safe_create_leading_directories_const(new_git_dir) < 0)\n>> + die(_(\"could not create directory '%s'\"), new_git_dir);\n>> +\n>> + if (!prefix)\n>> + prefix = get_super_prefix();\n>> + printf(\"Migrating git directory of %s%s from\\n'%s' to\\n'%s'\\n\",\n>> + prefix ? prefix : \"\", path,\n>> + real_path(old_git_dir), new_git_dir);\n>> +\n>> + if (rename(old_git_dir, new_git_dir) < 0)\n>> + die_errno(_(\"Could not migrate git directory from '%s' to '%s'\"),\n>> + old_git_dir, new_git_dir);\n>> +\n>> + connect_work_tree_and_git_dir(path, new_git_dir);\n>\n> Another thing in common is, both submodules and worktrees use some\n> form of textual symlinks. You need to fix up some here. But if this\n> submodule has multiple worktreee, there may be some \"symlinks\" in\n> .git/worktrees which would need fixing up as well.\n\nWe could signal that via one of the flag bits?\n(e.g. FIXUP_WORKTREE_SYMLINKS )\n\n>\n> You don't have to do the fix up thing right away, but I think we\n> should at least make sure we leave no dangling links behind (by\n> die()ing early if we find a .git dir we can't handle yet)\n> --\n> Duy\n"},{"id":"306602","messageId":"CAGZ79kar0F7x5U2yZ30ZnWZ9b=EJA=1nT8rxTMRVJPggyFS_XA@mail.gmail.com","threadId":"44534","inReplyTo":"xmqqpolcd73b.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCHv2 4/4] submodule: add embed-git-dir function","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-11-30T21:00:26Z","receivedAt":"2016-11-30T21:04:30Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Nov 30, 2016 at 12:51 PM, Junio C Hamano wrote:\n> Duy Nguyen writes:\n>\n>> On Wed, Nov 23, 2016 at 2:22 AM, Stefan Beller wrote:\n>>> +/*\n>>> + * Migrate the given submodule (and all its submodules recursively) from\n>>> + * having its git directory within the working tree to the git dir nested\n>>> + * in its superprojects git dir under modules/.\n>>> + */\n>>> +void migrate_submodule_gitdir(const char *prefix, const char *path,\n>>> + int recursive)\n>>\n>> Submodules and worktrees seem to have many things in common. The first\n>> one is this. \"git worktree move\" on a worktree that contains\n>> submodules .git also benefits from something like this [1]. I suggest\n>> you move this function to some neutral place and maybe rename it to\n>> relocate_gitdir() or something.\n>\n> Yeah, good suggestion (including name; first round used \"intern\" I\n> had trouble with, then \"embed\" which was OK-ish, but probably\n> \"relocate\" is better choice. If anything, what Stefan's series adds\n> is a command to un-embed embedded one).\n\nHeh, good counter perspective. I thought about embedding it\n\"into the superprojects git directory\" whereas your un-embed sounds\nlike you'd assume embedding is targetd towards the working dir.\n\nrelocate as a fancy name for move sounds like it expects 2 arguments\n(source and destination), but maybe we could go with:\n\n git relocate-git-dir (--into-workingtree|--into-gitdir) \\\n [--recurse-submodules] \\\n [--only-fix-gitfile-links-and-core-setting-as-I-moved-it-myself-already] \\\n [--dryrun] [--verbose] [--unsafe-move]\n\nNo need to have a pathspec here IMHO. Later it could have another flag\nto specify the \"main\" worktree or such as well.\n"},{"id":"306603","messageId":"CAGZ79kYS=GHUvSxDEMLBWZR82j09=Ti4piJu_mJcP7DuBuQZwA@mail.gmail.com","threadId":"44534","inReplyTo":"CAGZ79kbvey_f8+R16yYT_qsF0RErOh8own8n-RRApTM0dS-+ag@mail.gmail.com","subject":"Re: [PATCHv2 4/4] submodule: add embed-git-dir function","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-11-30T20:48:18Z","receivedAt":"2016-11-30T21:07:46Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Nov 30, 2016 at 10:04 AM, Stefan Beller wrote:\n\n>> Submodules and worktrees seem to have many things in common.\n>\n> Yes. :)\n\nI moved the code to dir.{c,h} and renamed it to a more generic \"move_gitdir\",\nbut then I am thinking further about how to align worktrees and submodules here,\nspecifically the recursive part.\n\nWhenever submodules and recursion is involved so far we spawned off a child\nprocess to take care of the issue in the submodule itself. Here we followed\nthe same pattern and called the submodule--helper to embed the git dirs\nin the submodule recursively.\n\nAs this function is not supposed to be submodule specific anymore, I'd\nrather not want to have the recursive childrens code live inside the submodule\nhelper, so we also need a neutral helper for moving git directories?\n\nSo there are a couple ways forward:\n* We declare the \"recursive\" flag to be submodule specific and keep\nthe recursion\n in the submodule helper\n* the recursive flag is generic enough and we invent a plumbing helper.\n Analogous to the submodule--helper, that was originally invented as a C aid\n for the git-submodule shell script, we could have a \"git--helper\" or just\n \"git plumbing \", though that sounds like reinventing the wheel as\n traditionally we'd just have a top level command to do a specific thing.\n (side note: Some parts of git-rev-parse could go into such a plumbing\n command)\n\nFor now I'd think to declare recursion in this function to be\nsubmodule specific.\n\nStefan\n"},{"id":"306604","messageId":"xmqqpolcd73b.fsf@gitster.mtv.corp.google.com","threadId":"44534","inReplyTo":"CACsJy8Ce3Oa-xJ4BwgRRy6neM=Jxkfqq7yboHZDXLDG2tu9GzQ@mail.gmail.com","subject":"Re: [PATCHv2 4/4] submodule: add embed-git-dir function","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-11-30T20:51:36Z","receivedAt":"2016-11-30T21:10:00Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> On Wed, Nov 23, 2016 at 2:22 AM, Stefan Beller wrote:\n>> +/*\n>> + * Migrate the given submodule (and all its submodules recursively) from\n>> + * having its git directory within the working tree to the git dir nested\n>> + * in its superprojects git dir under modules/.\n>> + */\n>> +void migrate_submodule_gitdir(const char *prefix, const char *path,\n>> + int recursive)\n>\n> Submodules and worktrees seem to have many things in common. The first\n> one is this. \"git worktree move\" on a worktree that contains\n> submodules .git also benefits from something like this [1]. I suggest\n> you move this function to some neutral place and maybe rename it to\n> relocate_gitdir() or something.\n\nYeah, good suggestion (including name; first round used \"intern\" I\nhad trouble with, then \"embed\" which was OK-ish, but probably\n\"relocate\" is better choice. If anything, what Stefan's series adds\nis a command to un-embed embedded one).\n\n> It probably should take a bit flag instead of \"recursive\" here. One\n> thing I would need is the ability to tell this function \"I have moved\n> all these .git dirs already (because I move whole worktree in one\n> operation), here are the old and new locations of them, fix them up!\".\n> In other words, no rename() could be optionally skipped.\n\nThanks two of you for working well together ;-)\n"},{"id":"306616","messageId":"xmqqfum8d4w3.fsf@gitster.mtv.corp.google.com","threadId":"44534","inReplyTo":"CAGZ79kar0F7x5U2yZ30ZnWZ9b=EJA=1nT8rxTMRVJPggyFS_XA@mail.gmail.com","subject":"Re: [PATCHv2 4/4] submodule: add embed-git-dir function","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-11-30T21:39:08Z","receivedAt":"2016-11-30T21:39:38Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> git relocate-git-dir (--into-workingtree|--into-gitdir) \\\n\nI am not sure if you meant this as a submodule-specific subcommand\nor more general helper. \"into-workingtree\" suggests to me that it\nis submodule specific, so I'll base my response on that assumption.\n\nWould there ever be a situation where you already have submodule\nrepositories in the right place (according to the more modern\npractice, to keep them in .git/modules/ of superproject) and want to\nmove them to embed them in worktrees of submodules? I do not think\nof any.\n\nIf there is no such situation, I do not think we want a verb that is\ndirection-neutral (e.g. \"move\" or \"relocate\") with two options.\nRather we would want \"git submodule unembed-git-dir\" or something\nlike that.\n"},{"id":"306618","messageId":"CAGZ79kZSAJauwBwrxf+QAhQgyu4ACn+8LrwjpFGVaUQfSzHEAg@mail.gmail.com","threadId":"44534","inReplyTo":"xmqqfum8d4w3.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCHv2 4/4] submodule: add embed-git-dir function","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-11-30T21:56:09Z","receivedAt":"2016-11-30T21:56:16Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Nov 30, 2016 at 1:39 PM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>\n>> git relocate-git-dir (--into-workingtree|--into-gitdir) \\\n>\n> I am not sure if you meant this as a submodule-specific subcommand\n> or more general helper. \"into-workingtree\" suggests to me that it\n> is submodule specific, so I'll base my response on that assumption.\n>\n> Would there ever be a situation where you already have submodule\n> repositories in the right place (according to the more modern\n> practice, to keep them in .git/modules/ of superproject) and want to\n> move them to embed them in worktrees of submodules? I do not think\n> of any.\n\n \"Hi, I made a mistake by using submodules. I don't want to use\n them any more, I rather want to:\n A) make it a separate git repo again and I'll keep them in sync myself\n B) ... \"\n\n \"I abuse submodules for what git-LFS was designed for, and the\n submodule is on a different mount point, please keep the git directory\n also at that mount point\".\n\nNot sure I agree these problems and the proposed solutions are beautiful,\nbut that is what people may think of as a fast hack?\n\n>\n> If there is no such situation, I do not think we want a verb that is\n> direction-neutral (e.g. \"move\" or \"relocate\") with two options.\n> Rather we would want \"git submodule unembed-git-dir\" or something\n> like that.\n\nSo when we want to have a generic function in C (\"relocate_gitdir\")\nfor both worktree and submodules, the recursive flag is not supposed\nto invoke a submodule specific helper, but a generic helper.\n\nAlternatively we make the function not as generic and claim the\nrecursive part is submodule specific and we can happily call\n\"git submodule [un]embed-git-dir\" recursively.\n"},{"id":"306628","messageId":"xmqq37i8d329.fsf@gitster.mtv.corp.google.com","threadId":"44534","inReplyTo":"CAGZ79kZSAJauwBwrxf+QAhQgyu4ACn+8LrwjpFGVaUQfSzHEAg@mail.gmail.com","subject":"Re: [PATCHv2 4/4] submodule: add embed-git-dir function","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-11-30T22:18:38Z","receivedAt":"2016-11-30T22:18:45Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> On Wed, Nov 30, 2016 at 1:39 PM, Junio C Hamano wrote:\n>> Stefan Beller writes:\n>>\n>>> git relocate-git-dir (--into-workingtree|--into-gitdir) \\\n>>\n>> I am not sure if you meant this as a submodule-specific subcommand\n>> or more general helper. \"into-workingtree\" suggests to me that it\n>> is submodule specific, so I'll base my response on that assumption.\n>>\n>> Would there ever be a situation where you already have submodule\n>> repositories in the right place (according to the more modern\n>> practice, to keep them in .git/modules/ of superproject) and want to\n>> move them to embed them in worktrees of submodules? I do not think\n>> of any.\n>\n> \"Hi, I made a mistake by using submodules. I don't want to use\n> them any more, I rather want to:\n> A) make it a separate git repo again and I'll keep them in sync myself\n> B) ... \"\n\nOK, I can buy that. Thanks.\n"}]}