{"thread":{"id":"38851","subject":"[PATCH v4 1/2] submodule refactor: use git_path_submodule() in add_submodule_odb()","startedAt":"2015-03-18T21:10:50Z","lastAt":"2015-09-14T18:14:19Z","messageCount":23,"participants":["Max Kirillov","Junio C Hamano","Stefan Beller","Jeff King"],"isPatch":true,"patchVersion":4,"patchTotal":2},"messages":[{"id":"257985","messageId":"1426713052-19171-1-git-send-email-max@max630.net","threadId":"38851","inReplyTo":null,"subject":"[PATCH v4 0/2] path: implement common_dir handling in git_path_submodule()","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2015-03-18T21:10:50Z","receivedAt":"2015-03-18T21:10:50Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"After http://thread.gmane.org/gmane.comp.version-control.git/261990\nthe only thing which did not enter the serie is these 2 pathes.\n\nShould be applied over the patches by link, or\necf2ff6ace6a1cc3d55b6f917be5452b5fb0c21b in current pu.\n\nMax Kirillov (2):\n submodule refactor: use git_path_submodule() in add_submodule_odb()\n path: implement common_dir handling in git_path_submodule()\n\n cache.h | 1 +\n path.c | 24 ++++++++++++++++++++----\n setup.c | 17 ++++++++++++-----\n submodule.c | 28 ++++++++++------------------\n t/t7410-submodule-checkout-to.sh | 10 ++++++++++\n 5 files changed, 53 insertions(+), 27 deletions(-)\n\n-- \n2.1.1.391.g7a54a76\n"},{"id":"257984","messageId":"1426713052-19171-2-git-send-email-max@max630.net","threadId":"38851","inReplyTo":"1426713052-19171-1-git-send-email-max@max630.net","subject":"[PATCH v4 1/2] submodule refactor: use git_path_submodule() in add_submodule_odb()","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2015-03-18T21:10:51Z","receivedAt":"2015-03-18T21:10:51Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"Signed-off-by: Max Kirillov \n---\n submodule.c | 28 ++++++++++------------------\n 1 file changed, 10 insertions(+), 18 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex 34094f5..4aad3d4 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -122,43 +122,35 @@ void stage_updated_gitmodules(void)\n \n static int add_submodule_odb(const char *path)\n {\n-\tstruct strbuf objects_directory = STRBUF_INIT;\n \tstruct alternate_object_database *alt_odb;\n+\tconst char* objects_directory;\n \tint ret = 0;\n-\tconst char *git_dir;\n \n-\tstrbuf_addf(&objects_directory, \"%s/.git\", path);\n-\tgit_dir = read_gitfile(objects_directory.buf);\n-\tif (git_dir) {\n-\t\tstrbuf_reset(&objects_directory);\n-\t\tstrbuf_addstr(&objects_directory, git_dir);\n-\t}\n-\tstrbuf_addstr(&objects_directory, \"/objects/\");\n-\tif (!is_directory(objects_directory.buf)) {\n+\tobjects_directory = git_path_submodule(path, \"objects/\");\n+\tif (!is_directory(objects_directory)) {\n \t\tret = -1;\n \t\tgoto done;\n \t}\n+\n \t/* avoid adding it twice */\n \tfor (alt_odb = alt_odb_list; alt_odb; alt_odb = alt_odb->next)\n-\t\tif (alt_odb->name - alt_odb->base == objects_directory.len &&\n-\t\t\t\t!strncmp(alt_odb->base, objects_directory.buf,\n-\t\t\t\t\tobjects_directory.len))\n+\t\tif (alt_odb->name - alt_odb->base == strlen(objects_directory) &&\n+\t\t\t\t!strcmp(alt_odb->base, objects_directory))\n \t\t\tgoto done;\n \n-\talt_odb = xmalloc(objects_directory.len + 42 + sizeof(*alt_odb));\n+\talt_odb = xmalloc(strlen(objects_directory) + 42 + sizeof(*alt_odb));\n \talt_odb->next = alt_odb_list;\n-\tstrcpy(alt_odb->base, objects_directory.buf);\n-\talt_odb->name = alt_odb->base + objects_directory.len;\n+\tstrcpy(alt_odb->base, objects_directory);\n+\talt_odb->name = alt_odb->base + strlen(objects_directory);\n \talt_odb->name[2] = '/';\n \talt_odb->name[40] = '\\0';\n \talt_odb->name[41] = '\\0';\n \talt_odb_list = alt_odb;\n \n \t/* add possible alternates from the submodule */\n-\tread_info_alternates(objects_directory.buf, 0);\n+\tread_info_alternates(objects_directory, 0);\n \tprepare_alt_odb();\n done:\n-\tstrbuf_release(&objects_directory);\n \treturn ret;\n }\n \n-- \n2.1.1.391.g7a54a76\n"},{"id":"257986","messageId":"1426713052-19171-3-git-send-email-max@max630.net","threadId":"38851","inReplyTo":"1426713052-19171-1-git-send-email-max@max630.net","subject":"[PATCH v4 2/2] path: implement common_dir handling in git_path_submodule()","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2015-03-18T21:10:52Z","receivedAt":"2015-03-18T21:10:52Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"This allows making submodules a linked workdirs.\n\nSame as for .git, but ignores the GIT_COMMON_DIR environment variable,\nbecause it would mean common directory for the parent repository and\ndoes not make sense for submodule.\n\nAlso add test for functionality which uses this call.\n\nSigned-off-by: Max Kirillov \n---\n cache.h | 1 +\n path.c | 24 ++++++++++++++++++++----\n setup.c | 17 ++++++++++++-----\n t/t7410-submodule-checkout-to.sh | 10 ++++++++++\n 4 files changed, 43 insertions(+), 9 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 670e861..4cd03f0 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -437,6 +437,7 @@ extern char *get_object_directory(void);\n extern char *get_index_file(void);\n extern char *get_graft_file(void);\n extern int set_git_dir(const char *path);\n+extern int get_common_dir_noenv(struct strbuf *sb, const char *gitdir);\n extern int get_common_dir(struct strbuf *sb, const char *gitdir);\n extern const char *get_git_namespace(void);\n extern const char *strip_namespace(const char *namespaced_ref);\ndiff --git a/path.c b/path.c\nindex a5c51a3..78f718f 100644\n--- a/path.c\n+++ b/path.c\n@@ -98,7 +98,7 @@ static const char *common_list[] = {\n \tNULL\n };\n \n-static void update_common_dir(struct strbuf *buf, int git_dir_len)\n+static void update_common_dir(struct strbuf *buf, int git_dir_len, const char* common_dir)\n {\n \tchar *base = buf->buf + git_dir_len;\n \tconst char **p;\n@@ -115,12 +115,17 @@ static void update_common_dir(struct strbuf *buf, int git_dir_len)\n \t\t\tpath++;\n \t\t\tis_dir = 1;\n \t\t}\n+\n+\t\tif (!common_dir) {\n+\t\t\tcommon_dir = get_git_common_dir();\n+\t\t}\n+\n \t\tif (is_dir && dir_prefix(base, path)) {\n-\t\t\treplace_dir(buf, git_dir_len, get_git_common_dir());\n+\t\t\treplace_dir(buf, git_dir_len, common_dir);\n \t\t\treturn;\n \t\t}\n \t\tif (!is_dir && !strcmp(base, path)) {\n-\t\t\treplace_dir(buf, git_dir_len, get_git_common_dir());\n+\t\t\treplace_dir(buf, git_dir_len, common_dir);\n \t\t\treturn;\n \t\t}\n \t}\n@@ -160,7 +165,7 @@ static void adjust_git_path(struct strbuf *buf, int git_dir_len)\n \telse if (git_db_env && dir_prefix(base, \"objects\"))\n \t\treplace_dir(buf, git_dir_len + 7, get_object_directory());\n \telse if (git_common_dir_env)\n-\t\tupdate_common_dir(buf, git_dir_len);\n+\t\tupdate_common_dir(buf, git_dir_len, NULL);\n }\n \n static void do_git_path(struct strbuf *buf, const char *fmt, va_list args)\n@@ -256,6 +261,8 @@ const char *git_path_submodule(const char *path, const char *fmt, ...)\n {\n \tstruct strbuf *buf = get_pathname();\n \tconst char *git_dir;\n+\tstruct strbuf git_submodule_common_dir = STRBUF_INIT;\n+\tstruct strbuf git_submodule_dir = STRBUF_INIT;\n \tva_list args;\n \n \tstrbuf_addstr(buf, path);\n@@ -269,11 +276,20 @@ const char *git_path_submodule(const char *path, const char *fmt, ...)\n \t\tstrbuf_addstr(buf, git_dir);\n \t}\n \tstrbuf_addch(buf, '/');\n+\tstrbuf_addstr(&git_submodule_dir, buf->buf);\n \n \tva_start(args, fmt);\n \tstrbuf_vaddf(buf, fmt, args);\n \tva_end(args);\n+\n+\tif (get_common_dir_noenv(&git_submodule_common_dir, git_submodule_dir.buf)) {\n+\t\tupdate_common_dir(buf, git_submodule_dir.len, git_submodule_common_dir.buf);\n+\t}\n+\n \tstrbuf_cleanup_path(buf);\n+\n+\tstrbuf_release(&git_submodule_dir);\n+\tstrbuf_release(&git_submodule_common_dir);\n \treturn buf->buf;\n }\n \ndiff --git a/setup.c b/setup.c\nindex fb61860..ffda622 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -226,14 +226,21 @@ void verify_non_filename(const char *prefix, const char *arg)\n \n int get_common_dir(struct strbuf *sb, const char *gitdir)\n {\n+\tconst char *git_env_common_dir = getenv(GIT_COMMON_DIR_ENVIRONMENT);\n+\tif (git_env_common_dir) {\n+\t\tstrbuf_addstr(sb, git_env_common_dir);\n+\t\treturn 1;\n+\t} else {\n+\t\treturn get_common_dir_noenv(sb, gitdir);\n+\t}\n+}\n+\n+int get_common_dir_noenv(struct strbuf *sb, const char *gitdir)\n+{\n \tstruct strbuf data = STRBUF_INIT;\n \tstruct strbuf path = STRBUF_INIT;\n-\tconst char *git_common_dir = getenv(GIT_COMMON_DIR_ENVIRONMENT);\n \tint ret = 0;\n-\tif (git_common_dir) {\n-\t\tstrbuf_addstr(sb, git_common_dir);\n-\t\treturn 1;\n-\t}\n+\n \tstrbuf_addf(&path, \"%s/commondir\", gitdir);\n \tif (file_exists(path.buf)) {\n \t\tif (strbuf_read_file(&data, path.buf, 0) <= 0)\ndiff --git a/t/t7410-submodule-checkout-to.sh b/t/t7410-submodule-checkout-to.sh\nindex 8f30aed..b43391a 100755\n--- a/t/t7410-submodule-checkout-to.sh\n+++ b/t/t7410-submodule-checkout-to.sh\n@@ -47,4 +47,14 @@ test_expect_success 'checkout main and initialize independed clones' \\\n test_expect_success 'can see submodule diffs after independed cloning' \\\n '(cd fully_cloned_submodule/main && git diff --submodule master\"^!\" | grep \"file1 updated\")'\n \n+test_expect_success 'checkout sub manually' \\\n+ 'mkdir linked_submodule &&\n+ (cd clone/main &&\n+\tgit checkout --to \"$base_path/linked_submodule/main\" \"$rev1_hash_main\") &&\n+ (cd clone/main/sub &&\n+\tgit checkout --to \"$base_path/linked_submodule/main/sub\" \"$rev1_hash_sub\")'\n+\n+test_expect_success 'can see submodule diffs after manual checkout of linked submodule' \\\n+ '(cd linked_submodule/main && git diff --submodule master\"^!\" | grep \"file1 updated\")'\n+\n test_done\n-- \n2.1.1.391.g7a54a76\n"},{"id":"267346","messageId":"1438635836-7857-1-git-send-email-max@max630.net","threadId":"38851","inReplyTo":"1426713052-19171-1-git-send-email-max@max630.net","subject":"[PATCH v5 0/2] path: implement common_dir handling in git_path_submodule()","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2015-08-03T21:03:54Z","receivedAt":"2015-08-03T21:03:54Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"Rebased to latest master, also merges cleanly to pu. Otherwise no changes.\n\nMax Kirillov (2):\n submodule refactor: use git_path_submodule() in add_submodule_odb()\n path: implement common_dir handling in git_path_submodule()\n\n cache.h | 1 +\n path.c | 24 ++++++++++++++++++++----\n setup.c | 17 ++++++++++++-----\n submodule.c | 28 ++++++++++------------------\n t/t7410-submodule-checkout-to.sh | 10 ++++++++++\n 5 files changed, 53 insertions(+), 27 deletions(-)\n\n-- \n2.3.4.2801.g3d0809b\n"},{"id":"267347","messageId":"1438635836-7857-2-git-send-email-max@max630.net","threadId":"38851","inReplyTo":"1438635836-7857-1-git-send-email-max@max630.net","subject":"[PATCH v5 1/2] submodule refactor: use git_path_submodule() in add_submodule_odb()","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2015-08-03T21:03:55Z","receivedAt":"2015-08-03T21:03:55Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"Signed-off-by: Max Kirillov \n---\n submodule.c | 28 ++++++++++------------------\n 1 file changed, 10 insertions(+), 18 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex 15e90d1..f6afe0a 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -122,43 +122,35 @@ void stage_updated_gitmodules(void)\n \n static int add_submodule_odb(const char *path)\n {\n-\tstruct strbuf objects_directory = STRBUF_INIT;\n \tstruct alternate_object_database *alt_odb;\n+\tconst char* objects_directory;\n \tint ret = 0;\n-\tconst char *git_dir;\n \n-\tstrbuf_addf(&objects_directory, \"%s/.git\", path);\n-\tgit_dir = read_gitfile(objects_directory.buf);\n-\tif (git_dir) {\n-\t\tstrbuf_reset(&objects_directory);\n-\t\tstrbuf_addstr(&objects_directory, git_dir);\n-\t}\n-\tstrbuf_addstr(&objects_directory, \"/objects/\");\n-\tif (!is_directory(objects_directory.buf)) {\n+\tobjects_directory = git_path_submodule(path, \"objects/\");\n+\tif (!is_directory(objects_directory)) {\n \t\tret = -1;\n \t\tgoto done;\n \t}\n+\n \t/* avoid adding it twice */\n \tfor (alt_odb = alt_odb_list; alt_odb; alt_odb = alt_odb->next)\n-\t\tif (alt_odb->name - alt_odb->base == objects_directory.len &&\n-\t\t\t\t!strncmp(alt_odb->base, objects_directory.buf,\n-\t\t\t\t\tobjects_directory.len))\n+\t\tif (alt_odb->name - alt_odb->base == strlen(objects_directory) &&\n+\t\t\t\t!strcmp(alt_odb->base, objects_directory))\n \t\t\tgoto done;\n \n-\talt_odb = xmalloc(objects_directory.len + 42 + sizeof(*alt_odb));\n+\talt_odb = xmalloc(strlen(objects_directory) + 42 + sizeof(*alt_odb));\n \talt_odb->next = alt_odb_list;\n-\tstrcpy(alt_odb->base, objects_directory.buf);\n-\talt_odb->name = alt_odb->base + objects_directory.len;\n+\tstrcpy(alt_odb->base, objects_directory);\n+\talt_odb->name = alt_odb->base + strlen(objects_directory);\n \talt_odb->name[2] = '/';\n \talt_odb->name[40] = '\\0';\n \talt_odb->name[41] = '\\0';\n \talt_odb_list = alt_odb;\n \n \t/* add possible alternates from the submodule */\n-\tread_info_alternates(objects_directory.buf, 0);\n+\tread_info_alternates(objects_directory, 0);\n \tprepare_alt_odb();\n done:\n-\tstrbuf_release(&objects_directory);\n \treturn ret;\n }\n \n-- \n2.3.4.2801.g3d0809b\n"},{"id":"267348","messageId":"1438635836-7857-3-git-send-email-max@max630.net","threadId":"38851","inReplyTo":"1438635836-7857-1-git-send-email-max@max630.net","subject":"[PATCH v5 2/2] path: implement common_dir handling in git_path_submodule()","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2015-08-03T21:03:56Z","receivedAt":"2015-08-03T21:03:56Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"This allows making submodules a linked workdirs.\n\nSame as for .git, but ignores the GIT_COMMON_DIR environment variable,\nbecause it would mean common directory for the parent repository and\ndoes not make sense for submodule.\n\nAlso add test for functionality which uses this call.\n\nSigned-off-by: Max Kirillov \n---\n cache.h | 1 +\n path.c | 24 ++++++++++++++++++++----\n setup.c | 17 ++++++++++++-----\n t/t7410-submodule-checkout-to.sh | 10 ++++++++++\n 4 files changed, 43 insertions(+), 9 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 4f55466..b87ec75 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -442,6 +442,7 @@ extern char *get_object_directory(void);\n extern char *get_index_file(void);\n extern char *get_graft_file(void);\n extern int set_git_dir(const char *path);\n+extern int get_common_dir_noenv(struct strbuf *sb, const char *gitdir);\n extern int get_common_dir(struct strbuf *sb, const char *gitdir);\n extern const char *get_git_namespace(void);\n extern const char *strip_namespace(const char *namespaced_ref);\ndiff --git a/path.c b/path.c\nindex 10f4cbf..f292d02 100644\n--- a/path.c\n+++ b/path.c\n@@ -98,7 +98,7 @@ static const char *common_list[] = {\n \tNULL\n };\n \n-static void update_common_dir(struct strbuf *buf, int git_dir_len)\n+static void update_common_dir(struct strbuf *buf, int git_dir_len, const char* common_dir)\n {\n \tchar *base = buf->buf + git_dir_len;\n \tconst char **p;\n@@ -115,12 +115,17 @@ static void update_common_dir(struct strbuf *buf, int git_dir_len)\n \t\t\tpath++;\n \t\t\tis_dir = 1;\n \t\t}\n+\n+\t\tif (!common_dir) {\n+\t\t\tcommon_dir = get_git_common_dir();\n+\t\t}\n+\n \t\tif (is_dir && dir_prefix(base, path)) {\n-\t\t\treplace_dir(buf, git_dir_len, get_git_common_dir());\n+\t\t\treplace_dir(buf, git_dir_len, common_dir);\n \t\t\treturn;\n \t\t}\n \t\tif (!is_dir && !strcmp(base, path)) {\n-\t\t\treplace_dir(buf, git_dir_len, get_git_common_dir());\n+\t\t\treplace_dir(buf, git_dir_len, common_dir);\n \t\t\treturn;\n \t\t}\n \t}\n@@ -160,7 +165,7 @@ static void adjust_git_path(struct strbuf *buf, int git_dir_len)\n \telse if (git_db_env && dir_prefix(base, \"objects\"))\n \t\treplace_dir(buf, git_dir_len + 7, get_object_directory());\n \telse if (git_common_dir_env)\n-\t\tupdate_common_dir(buf, git_dir_len);\n+\t\tupdate_common_dir(buf, git_dir_len, NULL);\n }\n \n static void do_git_path(struct strbuf *buf, const char *fmt, va_list args)\n@@ -228,6 +233,8 @@ const char *git_path_submodule(const char *path, const char *fmt, ...)\n {\n \tstruct strbuf *buf = get_pathname();\n \tconst char *git_dir;\n+\tstruct strbuf git_submodule_common_dir = STRBUF_INIT;\n+\tstruct strbuf git_submodule_dir = STRBUF_INIT;\n \tva_list args;\n \n \tstrbuf_addstr(buf, path);\n@@ -241,11 +248,20 @@ const char *git_path_submodule(const char *path, const char *fmt, ...)\n \t\tstrbuf_addstr(buf, git_dir);\n \t}\n \tstrbuf_addch(buf, '/');\n+\tstrbuf_addstr(&git_submodule_dir, buf->buf);\n \n \tva_start(args, fmt);\n \tstrbuf_vaddf(buf, fmt, args);\n \tva_end(args);\n+\n+\tif (get_common_dir_noenv(&git_submodule_common_dir, git_submodule_dir.buf)) {\n+\t\tupdate_common_dir(buf, git_submodule_dir.len, git_submodule_common_dir.buf);\n+\t}\n+\n \tstrbuf_cleanup_path(buf);\n+\n+\tstrbuf_release(&git_submodule_dir);\n+\tstrbuf_release(&git_submodule_common_dir);\n \treturn buf->buf;\n }\n \ndiff --git a/setup.c b/setup.c\nindex 82c0cc2..39ea06b 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -229,14 +229,21 @@ void verify_non_filename(const char *prefix, const char *arg)\n \n int get_common_dir(struct strbuf *sb, const char *gitdir)\n {\n+\tconst char *git_env_common_dir = getenv(GIT_COMMON_DIR_ENVIRONMENT);\n+\tif (git_env_common_dir) {\n+\t\tstrbuf_addstr(sb, git_env_common_dir);\n+\t\treturn 1;\n+\t} else {\n+\t\treturn get_common_dir_noenv(sb, gitdir);\n+\t}\n+}\n+\n+int get_common_dir_noenv(struct strbuf *sb, const char *gitdir)\n+{\n \tstruct strbuf data = STRBUF_INIT;\n \tstruct strbuf path = STRBUF_INIT;\n-\tconst char *git_common_dir = getenv(GIT_COMMON_DIR_ENVIRONMENT);\n \tint ret = 0;\n-\tif (git_common_dir) {\n-\t\tstrbuf_addstr(sb, git_common_dir);\n-\t\treturn 1;\n-\t}\n+\n \tstrbuf_addf(&path, \"%s/commondir\", gitdir);\n \tif (file_exists(path.buf)) {\n \t\tif (strbuf_read_file(&data, path.buf, 0) <= 0)\ndiff --git a/t/t7410-submodule-checkout-to.sh b/t/t7410-submodule-checkout-to.sh\nindex 3f609e8..1acef32 100755\n--- a/t/t7410-submodule-checkout-to.sh\n+++ b/t/t7410-submodule-checkout-to.sh\n@@ -47,4 +47,14 @@ test_expect_success 'checkout main and initialize independed clones' \\\n test_expect_success 'can see submodule diffs after independed cloning' \\\n '(cd fully_cloned_submodule/main && git diff --submodule master\"^!\" | grep \"file1 updated\")'\n \n+test_expect_success 'checkout sub manually' \\\n+ 'mkdir linked_submodule &&\n+ (cd clone/main &&\n+\tgit worktree add \"$base_path/linked_submodule/main\" \"$rev1_hash_main\") &&\n+ (cd clone/main/sub &&\n+\tgit worktree add \"$base_path/linked_submodule/main/sub\" \"$rev1_hash_sub\")'\n+\n+test_expect_success 'can see submodule diffs after manual checkout of linked submodule' \\\n+ '(cd linked_submodule/main && git diff --submodule master\"^!\" | grep \"file1 updated\")'\n+\n test_done\n-- \n2.3.4.2801.g3d0809b\n"},{"id":"267354","messageId":"xmqqmvy86oy0.fsf@gitster.dls.corp.google.com","threadId":"38851","inReplyTo":"1438635836-7857-2-git-send-email-max@max630.net","subject":"Re: [PATCH v5 1/2] submodule refactor: use git_path_submodule() in add_submodule_odb()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-08-03T21:29:27Z","receivedAt":"2015-08-03T21:29:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Max Kirillov writes:\n\nHere is a space to describe why this change is a good thing. Is\nthis a fix to change the behaviour, and if so how is the behaviour\ndifferent with and without the patch? Or is this just to drop the\nblock of code from here and replace it with a call to an existing\nhelper that does exactly the same thing? I _suspect_ that it is the\nlatter, but please do not force reviewers to guess.\n\n> Signed-off-by: Max Kirillov \n> ---\n> submodule.c | 28 ++++++++++------------------\n> 1 file changed, 10 insertions(+), 18 deletions(-)\n>\n> diff --git a/submodule.c b/submodule.c\n> index 15e90d1..f6afe0a 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n> @@ -122,43 +122,35 @@ void stage_updated_gitmodules(void)\n> \n> static int add_submodule_odb(const char *path)\n> {\n> -\tstruct strbuf objects_directory = STRBUF_INIT;\n> \tstruct alternate_object_database *alt_odb;\n> +\tconst char* objects_directory;\n\nStyle; asterisk sticks to the variable, not the type. I think you\nneed to fix this in multiple places (not just 1/2 but also in 2/2).\n\nOther than that I think it is a sensible \"replace bulk of code with\nidentical helper that already exists\" rewrite.\n\nThanks.\n\n> \tint ret = 0;\n> -\tconst char *git_dir;\n> \n> -\tstrbuf_addf(&objects_directory, \"%s/.git\", path);\n> -\tgit_dir = read_gitfile(objects_directory.buf);\n> -\tif (git_dir) {\n> -\t\tstrbuf_reset(&objects_directory);\n> -\t\tstrbuf_addstr(&objects_directory, git_dir);\n> -\t}\n> -\tstrbuf_addstr(&objects_directory, \"/objects/\");\n> -\tif (!is_directory(objects_directory.buf)) {\n> +\tobjects_directory = git_path_submodule(path, \"objects/\");\n> +\tif (!is_directory(objects_directory)) {\n> \t\tret = -1;\n> \t\tgoto done;\n> \t}\n> +\n> \t/* avoid adding it twice */\n> \tfor (alt_odb = alt_odb_list; alt_odb; alt_odb = alt_odb->next)\n> -\t\tif (alt_odb->name - alt_odb->base == objects_directory.len &&\n> -\t\t\t\t!strncmp(alt_odb->base, objects_directory.buf,\n> -\t\t\t\t\tobjects_directory.len))\n> +\t\tif (alt_odb->name - alt_odb->base == strlen(objects_directory) &&\n> +\t\t\t\t!strcmp(alt_odb->base, objects_directory))\n> \t\t\tgoto done;\n> \n> -\talt_odb = xmalloc(objects_directory.len + 42 + sizeof(*alt_odb));\n> +\talt_odb = xmalloc(strlen(objects_directory) + 42 + sizeof(*alt_odb));\n> \talt_odb->next = alt_odb_list;\n> -\tstrcpy(alt_odb->base, objects_directory.buf);\n> -\talt_odb->name = alt_odb->base + objects_directory.len;\n> +\tstrcpy(alt_odb->base, objects_directory);\n> +\talt_odb->name = alt_odb->base + strlen(objects_directory);\n> \talt_odb->name[2] = '/';\n> \talt_odb->name[40] = '\\0';\n> \talt_odb->name[41] = '\\0';\n> \talt_odb_list = alt_odb;\n> \n> \t/* add possible alternates from the submodule */\n> -\tread_info_alternates(objects_directory.buf, 0);\n> +\tread_info_alternates(objects_directory, 0);\n> \tprepare_alt_odb();\n> done:\n> -\tstrbuf_release(&objects_directory);\n> \treturn ret;\n> }\n"},{"id":"267482","messageId":"1438725067-3295-1-git-send-email-max@max630.net","threadId":"38851","inReplyTo":"1438635836-7857-1-git-send-email-max@max630.net","subject":"[PATCH v6 0/2] path: implement common_dir handling in git_path_submodule()","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2015-08-04T21:51:05Z","receivedAt":"2015-08-04T21:51:05Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"Fixed the pointer declaration spacing issue and updated commit messages.\n\nThe 1/2 refactoring still might leave some cases behind. I could grep only function\nconnect_work_tree_and_git_dir() which, as far as I can see, involved in moving repositories.\nHave not touched it because correct moving parts of multiple-worktree\nrepositories probably is a bigger task than just fix config file location.\n\nMax Kirillov (2):\n submodule refactor: use git_path_submodule() in add_submodule_odb()\n path: implement common_dir handling in git_path_submodule()\n\n cache.h | 1 +\n path.c | 24 ++++++++++++++++++++----\n setup.c | 17 ++++++++++++-----\n submodule.c | 28 ++++++++++------------------\n t/t7410-submodule-checkout-to.sh | 10 ++++++++++\n 5 files changed, 53 insertions(+), 27 deletions(-)\n\n-- \n2.3.4.2801.g3d0809b\n"},{"id":"267481","messageId":"1438725067-3295-2-git-send-email-max@max630.net","threadId":"38851","inReplyTo":"1438725067-3295-1-git-send-email-max@max630.net","subject":"[PATCH v6 1/2] submodule refactor: use git_path_submodule() in add_submodule_odb()","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2015-08-04T21:51:06Z","receivedAt":"2015-08-04T21:51:06Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"Functions which directly operate submodule's object database do not handle the\ncase when the submodule is linked worktree (which are introduced in\nc7b3a3d2fe). Instead of fixing the path calculation use already existing\ngit_path_submodule() function, with intention to modify only that function\nwhenever we need to change real location of submodule's repository content.\n\nSigned-off-by: Max Kirillov \n---\n submodule.c | 28 ++++++++++------------------\n 1 file changed, 10 insertions(+), 18 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex 15e90d1..70d18ec 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -122,43 +122,35 @@ void stage_updated_gitmodules(void)\n \n static int add_submodule_odb(const char *path)\n {\n-\tstruct strbuf objects_directory = STRBUF_INIT;\n \tstruct alternate_object_database *alt_odb;\n+\tconst char *objects_directory;\n \tint ret = 0;\n-\tconst char *git_dir;\n \n-\tstrbuf_addf(&objects_directory, \"%s/.git\", path);\n-\tgit_dir = read_gitfile(objects_directory.buf);\n-\tif (git_dir) {\n-\t\tstrbuf_reset(&objects_directory);\n-\t\tstrbuf_addstr(&objects_directory, git_dir);\n-\t}\n-\tstrbuf_addstr(&objects_directory, \"/objects/\");\n-\tif (!is_directory(objects_directory.buf)) {\n+\tobjects_directory = git_path_submodule(path, \"objects/\");\n+\tif (!is_directory(objects_directory)) {\n \t\tret = -1;\n \t\tgoto done;\n \t}\n+\n \t/* avoid adding it twice */\n \tfor (alt_odb = alt_odb_list; alt_odb; alt_odb = alt_odb->next)\n-\t\tif (alt_odb->name - alt_odb->base == objects_directory.len &&\n-\t\t\t\t!strncmp(alt_odb->base, objects_directory.buf,\n-\t\t\t\t\tobjects_directory.len))\n+\t\tif (alt_odb->name - alt_odb->base == strlen(objects_directory) &&\n+\t\t\t\t!strcmp(alt_odb->base, objects_directory))\n \t\t\tgoto done;\n \n-\talt_odb = xmalloc(objects_directory.len + 42 + sizeof(*alt_odb));\n+\talt_odb = xmalloc(strlen(objects_directory) + 42 + sizeof(*alt_odb));\n \talt_odb->next = alt_odb_list;\n-\tstrcpy(alt_odb->base, objects_directory.buf);\n-\talt_odb->name = alt_odb->base + objects_directory.len;\n+\tstrcpy(alt_odb->base, objects_directory);\n+\talt_odb->name = alt_odb->base + strlen(objects_directory);\n \talt_odb->name[2] = '/';\n \talt_odb->name[40] = '\\0';\n \talt_odb->name[41] = '\\0';\n \talt_odb_list = alt_odb;\n \n \t/* add possible alternates from the submodule */\n-\tread_info_alternates(objects_directory.buf, 0);\n+\tread_info_alternates(objects_directory, 0);\n \tprepare_alt_odb();\n done:\n-\tstrbuf_release(&objects_directory);\n \treturn ret;\n }\n \n-- \n2.3.4.2801.g3d0809b\n"},{"id":"267483","messageId":"1438725067-3295-3-git-send-email-max@max630.net","threadId":"38851","inReplyTo":"1438725067-3295-1-git-send-email-max@max630.net","subject":"[PATCH v6 2/2] path: implement common_dir handling in git_path_submodule()","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2015-08-04T21:51:07Z","receivedAt":"2015-08-04T21:51:07Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"When submodule is a linked worktree, \"git diff --submodule\" and other\ncalls which directly access the submodule's object database do not correctly\ncalculate its path. Fix it by changing the git_path_submodule() behavior,\nto use either common or per-worktree directory.\n\nDo it similarly as for parent repository, but ignore the GIT_COMMON_DIR\nenvironment variable, because it would mean common directory for the parent\nrepository and does not make sense for submodule.\n\nAlso add test for functionality which uses this call.\n\nSigned-off-by: Max Kirillov \n---\n cache.h | 1 +\n path.c | 24 ++++++++++++++++++++----\n setup.c | 17 ++++++++++++-----\n t/t7410-submodule-checkout-to.sh | 10 ++++++++++\n 4 files changed, 43 insertions(+), 9 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 4f55466..b87ec75 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -442,6 +442,7 @@ extern char *get_object_directory(void);\n extern char *get_index_file(void);\n extern char *get_graft_file(void);\n extern int set_git_dir(const char *path);\n+extern int get_common_dir_noenv(struct strbuf *sb, const char *gitdir);\n extern int get_common_dir(struct strbuf *sb, const char *gitdir);\n extern const char *get_git_namespace(void);\n extern const char *strip_namespace(const char *namespaced_ref);\ndiff --git a/path.c b/path.c\nindex 10f4cbf..b0cf444 100644\n--- a/path.c\n+++ b/path.c\n@@ -98,7 +98,7 @@ static const char *common_list[] = {\n \tNULL\n };\n \n-static void update_common_dir(struct strbuf *buf, int git_dir_len)\n+static void update_common_dir(struct strbuf *buf, int git_dir_len, const char *common_dir)\n {\n \tchar *base = buf->buf + git_dir_len;\n \tconst char **p;\n@@ -115,12 +115,17 @@ static void update_common_dir(struct strbuf *buf, int git_dir_len)\n \t\t\tpath++;\n \t\t\tis_dir = 1;\n \t\t}\n+\n+\t\tif (!common_dir) {\n+\t\t\tcommon_dir = get_git_common_dir();\n+\t\t}\n+\n \t\tif (is_dir && dir_prefix(base, path)) {\n-\t\t\treplace_dir(buf, git_dir_len, get_git_common_dir());\n+\t\t\treplace_dir(buf, git_dir_len, common_dir);\n \t\t\treturn;\n \t\t}\n \t\tif (!is_dir && !strcmp(base, path)) {\n-\t\t\treplace_dir(buf, git_dir_len, get_git_common_dir());\n+\t\t\treplace_dir(buf, git_dir_len, common_dir);\n \t\t\treturn;\n \t\t}\n \t}\n@@ -160,7 +165,7 @@ static void adjust_git_path(struct strbuf *buf, int git_dir_len)\n \telse if (git_db_env && dir_prefix(base, \"objects\"))\n \t\treplace_dir(buf, git_dir_len + 7, get_object_directory());\n \telse if (git_common_dir_env)\n-\t\tupdate_common_dir(buf, git_dir_len);\n+\t\tupdate_common_dir(buf, git_dir_len, NULL);\n }\n \n static void do_git_path(struct strbuf *buf, const char *fmt, va_list args)\n@@ -228,6 +233,8 @@ const char *git_path_submodule(const char *path, const char *fmt, ...)\n {\n \tstruct strbuf *buf = get_pathname();\n \tconst char *git_dir;\n+\tstruct strbuf git_submodule_common_dir = STRBUF_INIT;\n+\tstruct strbuf git_submodule_dir = STRBUF_INIT;\n \tva_list args;\n \n \tstrbuf_addstr(buf, path);\n@@ -241,11 +248,20 @@ const char *git_path_submodule(const char *path, const char *fmt, ...)\n \t\tstrbuf_addstr(buf, git_dir);\n \t}\n \tstrbuf_addch(buf, '/');\n+\tstrbuf_addstr(&git_submodule_dir, buf->buf);\n \n \tva_start(args, fmt);\n \tstrbuf_vaddf(buf, fmt, args);\n \tva_end(args);\n+\n+\tif (get_common_dir_noenv(&git_submodule_common_dir, git_submodule_dir.buf)) {\n+\t\tupdate_common_dir(buf, git_submodule_dir.len, git_submodule_common_dir.buf);\n+\t}\n+\n \tstrbuf_cleanup_path(buf);\n+\n+\tstrbuf_release(&git_submodule_dir);\n+\tstrbuf_release(&git_submodule_common_dir);\n \treturn buf->buf;\n }\n \ndiff --git a/setup.c b/setup.c\nindex 82c0cc2..39ea06b 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -229,14 +229,21 @@ void verify_non_filename(const char *prefix, const char *arg)\n \n int get_common_dir(struct strbuf *sb, const char *gitdir)\n {\n+\tconst char *git_env_common_dir = getenv(GIT_COMMON_DIR_ENVIRONMENT);\n+\tif (git_env_common_dir) {\n+\t\tstrbuf_addstr(sb, git_env_common_dir);\n+\t\treturn 1;\n+\t} else {\n+\t\treturn get_common_dir_noenv(sb, gitdir);\n+\t}\n+}\n+\n+int get_common_dir_noenv(struct strbuf *sb, const char *gitdir)\n+{\n \tstruct strbuf data = STRBUF_INIT;\n \tstruct strbuf path = STRBUF_INIT;\n-\tconst char *git_common_dir = getenv(GIT_COMMON_DIR_ENVIRONMENT);\n \tint ret = 0;\n-\tif (git_common_dir) {\n-\t\tstrbuf_addstr(sb, git_common_dir);\n-\t\treturn 1;\n-\t}\n+\n \tstrbuf_addf(&path, \"%s/commondir\", gitdir);\n \tif (file_exists(path.buf)) {\n \t\tif (strbuf_read_file(&data, path.buf, 0) <= 0)\ndiff --git a/t/t7410-submodule-checkout-to.sh b/t/t7410-submodule-checkout-to.sh\nindex 3f609e8..1acef32 100755\n--- a/t/t7410-submodule-checkout-to.sh\n+++ b/t/t7410-submodule-checkout-to.sh\n@@ -47,4 +47,14 @@ test_expect_success 'checkout main and initialize independed clones' \\\n test_expect_success 'can see submodule diffs after independed cloning' \\\n '(cd fully_cloned_submodule/main && git diff --submodule master\"^!\" | grep \"file1 updated\")'\n \n+test_expect_success 'checkout sub manually' \\\n+ 'mkdir linked_submodule &&\n+ (cd clone/main &&\n+\tgit worktree add \"$base_path/linked_submodule/main\" \"$rev1_hash_main\") &&\n+ (cd clone/main/sub &&\n+\tgit worktree add \"$base_path/linked_submodule/main/sub\" \"$rev1_hash_sub\")'\n+\n+test_expect_success 'can see submodule diffs after manual checkout of linked submodule' \\\n+ '(cd linked_submodule/main && git diff --submodule master\"^!\" | grep \"file1 updated\")'\n+\n test_done\n-- \n2.3.4.2801.g3d0809b\n"},{"id":"267484","messageId":"1438725925-3689-1-git-send-email-max@max630.net","threadId":"38851","inReplyTo":"1438725067-3295-1-git-send-email-max@max630.net","subject":"[PATCH v7 0/2] path: implement common_dir handling in git_path_submodule()","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2015-08-04T22:05:23Z","receivedAt":"2015-08-04T22:05:23Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"Emphasized that 1/2 does not change behavior.\n\nMax Kirillov (2):\n submodule refactor: use git_path_submodule() in add_submodule_odb()\n path: implement common_dir handling in git_path_submodule()\n\n cache.h | 1 +\n path.c | 24 ++++++++++++++++++++----\n setup.c | 17 ++++++++++++-----\n submodule.c | 28 ++++++++++------------------\n t/t7410-submodule-checkout-to.sh | 10 ++++++++++\n 5 files changed, 53 insertions(+), 27 deletions(-)\n\n-- \n2.3.4.2801.g3d0809b\n"},{"id":"267485","messageId":"1438725925-3689-2-git-send-email-max@max630.net","threadId":"38851","inReplyTo":"1438725925-3689-1-git-send-email-max@max630.net","subject":"[PATCH v7 1/2] submodule refactor: use git_path_submodule() in add_submodule_odb()","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2015-08-04T22:05:24Z","receivedAt":"2015-08-04T22:05:24Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"Functions which directly operate submodule's object database do not handle the\ncase when the submodule is linked worktree (which are introduced in\nc7b3a3d2fe). Instead of fixing the path calculation use already existing\ngit_path_submodule() function without changing overall behavior. Then it will\nbe possible to modify only that function whenever we need to change real\nlocation of submodule's repository content.\n\nSigned-off-by: Max Kirillov \n---\n submodule.c | 28 ++++++++++------------------\n 1 file changed, 10 insertions(+), 18 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex 15e90d1..70d18ec 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -122,43 +122,35 @@ void stage_updated_gitmodules(void)\n \n static int add_submodule_odb(const char *path)\n {\n-\tstruct strbuf objects_directory = STRBUF_INIT;\n \tstruct alternate_object_database *alt_odb;\n+\tconst char *objects_directory;\n \tint ret = 0;\n-\tconst char *git_dir;\n \n-\tstrbuf_addf(&objects_directory, \"%s/.git\", path);\n-\tgit_dir = read_gitfile(objects_directory.buf);\n-\tif (git_dir) {\n-\t\tstrbuf_reset(&objects_directory);\n-\t\tstrbuf_addstr(&objects_directory, git_dir);\n-\t}\n-\tstrbuf_addstr(&objects_directory, \"/objects/\");\n-\tif (!is_directory(objects_directory.buf)) {\n+\tobjects_directory = git_path_submodule(path, \"objects/\");\n+\tif (!is_directory(objects_directory)) {\n \t\tret = -1;\n \t\tgoto done;\n \t}\n+\n \t/* avoid adding it twice */\n \tfor (alt_odb = alt_odb_list; alt_odb; alt_odb = alt_odb->next)\n-\t\tif (alt_odb->name - alt_odb->base == objects_directory.len &&\n-\t\t\t\t!strncmp(alt_odb->base, objects_directory.buf,\n-\t\t\t\t\tobjects_directory.len))\n+\t\tif (alt_odb->name - alt_odb->base == strlen(objects_directory) &&\n+\t\t\t\t!strcmp(alt_odb->base, objects_directory))\n \t\t\tgoto done;\n \n-\talt_odb = xmalloc(objects_directory.len + 42 + sizeof(*alt_odb));\n+\talt_odb = xmalloc(strlen(objects_directory) + 42 + sizeof(*alt_odb));\n \talt_odb->next = alt_odb_list;\n-\tstrcpy(alt_odb->base, objects_directory.buf);\n-\talt_odb->name = alt_odb->base + objects_directory.len;\n+\tstrcpy(alt_odb->base, objects_directory);\n+\talt_odb->name = alt_odb->base + strlen(objects_directory);\n \talt_odb->name[2] = '/';\n \talt_odb->name[40] = '\\0';\n \talt_odb->name[41] = '\\0';\n \talt_odb_list = alt_odb;\n \n \t/* add possible alternates from the submodule */\n-\tread_info_alternates(objects_directory.buf, 0);\n+\tread_info_alternates(objects_directory, 0);\n \tprepare_alt_odb();\n done:\n-\tstrbuf_release(&objects_directory);\n \treturn ret;\n }\n \n-- \n2.3.4.2801.g3d0809b\n"},{"id":"267486","messageId":"1438725925-3689-3-git-send-email-max@max630.net","threadId":"38851","inReplyTo":"1438725925-3689-1-git-send-email-max@max630.net","subject":"[PATCH v7 2/2] path: implement common_dir handling in git_path_submodule()","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2015-08-04T22:05:25Z","receivedAt":"2015-08-04T22:05:25Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"When submodule is a linked worktree, \"git diff --submodule\" and other\ncalls which directly access the submodule's object database do not correctly\ncalculate its path. Fix it by changing the git_path_submodule() behavior,\nto use either common or per-worktree directory.\n\nDo it similarly as for parent repository, but ignore the GIT_COMMON_DIR\nenvironment variable, because it would mean common directory for the parent\nrepository and does not make sense for submodule.\n\nAlso add test for functionality which uses this call.\n\nSigned-off-by: Max Kirillov \n---\n cache.h | 1 +\n path.c | 24 ++++++++++++++++++++----\n setup.c | 17 ++++++++++++-----\n t/t7410-submodule-checkout-to.sh | 10 ++++++++++\n 4 files changed, 43 insertions(+), 9 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 4f55466..b87ec75 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -442,6 +442,7 @@ extern char *get_object_directory(void);\n extern char *get_index_file(void);\n extern char *get_graft_file(void);\n extern int set_git_dir(const char *path);\n+extern int get_common_dir_noenv(struct strbuf *sb, const char *gitdir);\n extern int get_common_dir(struct strbuf *sb, const char *gitdir);\n extern const char *get_git_namespace(void);\n extern const char *strip_namespace(const char *namespaced_ref);\ndiff --git a/path.c b/path.c\nindex 10f4cbf..b0cf444 100644\n--- a/path.c\n+++ b/path.c\n@@ -98,7 +98,7 @@ static const char *common_list[] = {\n \tNULL\n };\n \n-static void update_common_dir(struct strbuf *buf, int git_dir_len)\n+static void update_common_dir(struct strbuf *buf, int git_dir_len, const char *common_dir)\n {\n \tchar *base = buf->buf + git_dir_len;\n \tconst char **p;\n@@ -115,12 +115,17 @@ static void update_common_dir(struct strbuf *buf, int git_dir_len)\n \t\t\tpath++;\n \t\t\tis_dir = 1;\n \t\t}\n+\n+\t\tif (!common_dir) {\n+\t\t\tcommon_dir = get_git_common_dir();\n+\t\t}\n+\n \t\tif (is_dir && dir_prefix(base, path)) {\n-\t\t\treplace_dir(buf, git_dir_len, get_git_common_dir());\n+\t\t\treplace_dir(buf, git_dir_len, common_dir);\n \t\t\treturn;\n \t\t}\n \t\tif (!is_dir && !strcmp(base, path)) {\n-\t\t\treplace_dir(buf, git_dir_len, get_git_common_dir());\n+\t\t\treplace_dir(buf, git_dir_len, common_dir);\n \t\t\treturn;\n \t\t}\n \t}\n@@ -160,7 +165,7 @@ static void adjust_git_path(struct strbuf *buf, int git_dir_len)\n \telse if (git_db_env && dir_prefix(base, \"objects\"))\n \t\treplace_dir(buf, git_dir_len + 7, get_object_directory());\n \telse if (git_common_dir_env)\n-\t\tupdate_common_dir(buf, git_dir_len);\n+\t\tupdate_common_dir(buf, git_dir_len, NULL);\n }\n \n static void do_git_path(struct strbuf *buf, const char *fmt, va_list args)\n@@ -228,6 +233,8 @@ const char *git_path_submodule(const char *path, const char *fmt, ...)\n {\n \tstruct strbuf *buf = get_pathname();\n \tconst char *git_dir;\n+\tstruct strbuf git_submodule_common_dir = STRBUF_INIT;\n+\tstruct strbuf git_submodule_dir = STRBUF_INIT;\n \tva_list args;\n \n \tstrbuf_addstr(buf, path);\n@@ -241,11 +248,20 @@ const char *git_path_submodule(const char *path, const char *fmt, ...)\n \t\tstrbuf_addstr(buf, git_dir);\n \t}\n \tstrbuf_addch(buf, '/');\n+\tstrbuf_addstr(&git_submodule_dir, buf->buf);\n \n \tva_start(args, fmt);\n \tstrbuf_vaddf(buf, fmt, args);\n \tva_end(args);\n+\n+\tif (get_common_dir_noenv(&git_submodule_common_dir, git_submodule_dir.buf)) {\n+\t\tupdate_common_dir(buf, git_submodule_dir.len, git_submodule_common_dir.buf);\n+\t}\n+\n \tstrbuf_cleanup_path(buf);\n+\n+\tstrbuf_release(&git_submodule_dir);\n+\tstrbuf_release(&git_submodule_common_dir);\n \treturn buf->buf;\n }\n \ndiff --git a/setup.c b/setup.c\nindex 82c0cc2..39ea06b 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -229,14 +229,21 @@ void verify_non_filename(const char *prefix, const char *arg)\n \n int get_common_dir(struct strbuf *sb, const char *gitdir)\n {\n+\tconst char *git_env_common_dir = getenv(GIT_COMMON_DIR_ENVIRONMENT);\n+\tif (git_env_common_dir) {\n+\t\tstrbuf_addstr(sb, git_env_common_dir);\n+\t\treturn 1;\n+\t} else {\n+\t\treturn get_common_dir_noenv(sb, gitdir);\n+\t}\n+}\n+\n+int get_common_dir_noenv(struct strbuf *sb, const char *gitdir)\n+{\n \tstruct strbuf data = STRBUF_INIT;\n \tstruct strbuf path = STRBUF_INIT;\n-\tconst char *git_common_dir = getenv(GIT_COMMON_DIR_ENVIRONMENT);\n \tint ret = 0;\n-\tif (git_common_dir) {\n-\t\tstrbuf_addstr(sb, git_common_dir);\n-\t\treturn 1;\n-\t}\n+\n \tstrbuf_addf(&path, \"%s/commondir\", gitdir);\n \tif (file_exists(path.buf)) {\n \t\tif (strbuf_read_file(&data, path.buf, 0) <= 0)\ndiff --git a/t/t7410-submodule-checkout-to.sh b/t/t7410-submodule-checkout-to.sh\nindex 3f609e8..1acef32 100755\n--- a/t/t7410-submodule-checkout-to.sh\n+++ b/t/t7410-submodule-checkout-to.sh\n@@ -47,4 +47,14 @@ test_expect_success 'checkout main and initialize independed clones' \\\n test_expect_success 'can see submodule diffs after independed cloning' \\\n '(cd fully_cloned_submodule/main && git diff --submodule master\"^!\" | grep \"file1 updated\")'\n \n+test_expect_success 'checkout sub manually' \\\n+ 'mkdir linked_submodule &&\n+ (cd clone/main &&\n+\tgit worktree add \"$base_path/linked_submodule/main\" \"$rev1_hash_main\") &&\n+ (cd clone/main/sub &&\n+\tgit worktree add \"$base_path/linked_submodule/main/sub\" \"$rev1_hash_sub\")'\n+\n+test_expect_success 'can see submodule diffs after manual checkout of linked submodule' \\\n+ '(cd linked_submodule/main && git diff --submodule master\"^!\" | grep \"file1 updated\")'\n+\n test_done\n-- \n2.3.4.2801.g3d0809b\n"},{"id":"268237","messageId":"CAGZ79kbCg1x0s_5eRLuxGs-7=-KvxLwpZ1h7ZFeZ-Yhgwfwe_Q@mail.gmail.com","threadId":"38851","inReplyTo":"1438725925-3689-3-git-send-email-max@max630.net","subject":"Re: [PATCH v7 2/2] path: implement common_dir handling in git_path_submodule()","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-08-17T23:56:24Z","receivedAt":"2015-08-17T23:56:24Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Aug 4, 2015 at 3:05 PM, Max Kirillov wrote:\n> When submodule is a linked worktree, \"git diff --submodule\" and other\n> calls which directly access the submodule's object database do not correctly\n> calculate its path. Fix it by changing the git_path_submodule() behavior,\n> to use either common or per-worktree directory.\n>\n> Do it similarly as for parent repository, but ignore the GIT_COMMON_DIR\n> environment variable, because it would mean common directory for the parent\n> repository and does not make sense for submodule.\n>\n> Also add test for functionality which uses this call.\n>\n> Signed-off-by: Max Kirillov \n> ---\n> cache.h | 1 +\n> path.c | 24 ++++++++++++++++++++----\n> setup.c | 17 ++++++++++++-----\n> t/t7410-submodule-checkout-to.sh | 10 ++++++++++\n> 4 files changed, 43 insertions(+), 9 deletions(-)\n>\n> diff --git a/cache.h b/cache.h\n> index 4f55466..b87ec75 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -442,6 +442,7 @@ extern char *get_object_directory(void);\n> extern char *get_index_file(void);\n> extern char *get_graft_file(void);\n> extern int set_git_dir(const char *path);\n> +extern int get_common_dir_noenv(struct strbuf *sb, const char *gitdir);\n> extern int get_common_dir(struct strbuf *sb, const char *gitdir);\n> extern const char *get_git_namespace(void);\n> extern const char *strip_namespace(const char *namespaced_ref);\n> diff --git a/path.c b/path.c\n> index 10f4cbf..b0cf444 100644\n> --- a/path.c\n> +++ b/path.c\n> @@ -98,7 +98,7 @@ static const char *common_list[] = {\n> NULL\n> };\n>\n> -static void update_common_dir(struct strbuf *buf, int git_dir_len)\n> +static void update_common_dir(struct strbuf *buf, int git_dir_len, const char *common_dir)\n> {\n> char *base = buf->buf + git_dir_len;\n> const char **p;\n> @@ -115,12 +115,17 @@ static void update_common_dir(struct strbuf *buf, int git_dir_len)\n> path++;\n> is_dir = 1;\n> }\n> +\n> + if (!common_dir) {\n> + common_dir = get_git_common_dir();\n> + }\n\nUsually we omit the braces for blocks with just one command.\n\n> +\n> if (is_dir && dir_prefix(base, path)) {\n> - replace_dir(buf, git_dir_len, get_git_common_dir());\n> + replace_dir(buf, git_dir_len, common_dir);\n> return;\n> }\n> if (!is_dir && !strcmp(base, path)) {\n> - replace_dir(buf, git_dir_len, get_git_common_dir());\n> + replace_dir(buf, git_dir_len, common_dir);\n> return;\n> }\n> }\n> @@ -160,7 +165,7 @@ static void adjust_git_path(struct strbuf *buf, int git_dir_len)\n> else if (git_db_env && dir_prefix(base, \"objects\"))\n> replace_dir(buf, git_dir_len + 7, get_object_directory());\n> else if (git_common_dir_env)\n> - update_common_dir(buf, git_dir_len);\n> + update_common_dir(buf, git_dir_len, NULL);\n> }\n>\n> static void do_git_path(struct strbuf *buf, const char *fmt, va_list args)\n> @@ -228,6 +233,8 @@ const char *git_path_submodule(const char *path, const char *fmt, ...)\n> {\n> struct strbuf *buf = get_pathname();\n> const char *git_dir;\n> + struct strbuf git_submodule_common_dir = STRBUF_INIT;\n> + struct strbuf git_submodule_dir = STRBUF_INIT;\n> va_list args;\n>\n> strbuf_addstr(buf, path);\n> @@ -241,11 +248,20 @@ const char *git_path_submodule(const char *path, const char *fmt, ...)\n> strbuf_addstr(buf, git_dir);\n> }\n> strbuf_addch(buf, '/');\n> + strbuf_addstr(&git_submodule_dir, buf->buf);\n>\n> va_start(args, fmt);\n> strbuf_vaddf(buf, fmt, args);\n> va_end(args);\n> +\n> + if (get_common_dir_noenv(&git_submodule_common_dir, git_submodule_dir.buf)) {\n> + update_common_dir(buf, git_submodule_dir.len, git_submodule_common_dir.buf);\n> + }\n> +\n> strbuf_cleanup_path(buf);\n> +\n> + strbuf_release(&git_submodule_dir);\n> + strbuf_release(&git_submodule_common_dir);\n> return buf->buf;\n> }\n>\n> diff --git a/setup.c b/setup.c\n> index 82c0cc2..39ea06b 100644\n> --- a/setup.c\n> +++ b/setup.c\n> @@ -229,14 +229,21 @@ void verify_non_filename(const char *prefix, const char *arg)\n>\n> int get_common_dir(struct strbuf *sb, const char *gitdir)\n> {\n> + const char *git_env_common_dir = getenv(GIT_COMMON_DIR_ENVIRONMENT);\n> + if (git_env_common_dir) {\n> + strbuf_addstr(sb, git_env_common_dir);\n> + return 1;\n> + } else {\n> + return get_common_dir_noenv(sb, gitdir);\n> + }\n> +}\n> +\n> +int get_common_dir_noenv(struct strbuf *sb, const char *gitdir)\n> +{\n> struct strbuf data = STRBUF_INIT;\n> struct strbuf path = STRBUF_INIT;\n> - const char *git_common_dir = getenv(GIT_COMMON_DIR_ENVIRONMENT);\n> int ret = 0;\n> - if (git_common_dir) {\n> - strbuf_addstr(sb, git_common_dir);\n> - return 1;\n> - }\n> +\n> strbuf_addf(&path, \"%s/commondir\", gitdir);\n> if (file_exists(path.buf)) {\n> if (strbuf_read_file(&data, path.buf, 0) <= 0)\n> diff --git a/t/t7410-submodule-checkout-to.sh b/t/t7410-submodule-checkout-to.sh\n> index 3f609e8..1acef32 100755\n> --- a/t/t7410-submodule-checkout-to.sh\n> +++ b/t/t7410-submodule-checkout-to.sh\n> @@ -47,4 +47,14 @@ test_expect_success 'checkout main and initialize independed clones' \\\n> test_expect_success 'can see submodule diffs after independed cloning' \\\n> '(cd fully_cloned_submodule/main && git diff --submodule master\"^!\" | grep \"file1 updated\")'\n>\n> +test_expect_success 'checkout sub manually' \\\n> + 'mkdir linked_submodule &&\n> + (cd clone/main &&\n> + git worktree add \"$base_path/linked_submodule/main\" \"$rev1_hash_main\") &&\n> + (cd clone/main/sub &&\n> + git worktree add \"$base_path/linked_submodule/main/sub\" \"$rev1_hash_sub\")'\n> +\n> +test_expect_success 'can see submodule diffs after manual checkout of linked submodule' \\\n> + '(cd linked_submodule/main && git diff --submodule master\"^!\" | grep \"file1 updated\")'\n> +\n> test_done\n> --\n> 2.3.4.2801.g3d0809b\n\nLooks good to me otherwise.\n\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n"},{"id":"268238","messageId":"CAGZ79kYQtcu_YJ57vT=OKh4WTbcfyjTRhn6eDbEO7BrfEru2TA@mail.gmail.com","threadId":"38851","inReplyTo":"1438725925-3689-2-git-send-email-max@max630.net","subject":"Re: [PATCH v7 1/2] submodule refactor: use git_path_submodule() in add_submodule_odb()","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-08-18T00:07:53Z","receivedAt":"2015-08-18T00:07:53Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Aug 4, 2015 at 3:05 PM, Max Kirillov wrote:\n> Functions which directly operate submodule's object database do not handle the\n> case when the submodule is linked worktree (which are introduced in\n> c7b3a3d2fe). Instead of fixing the path calculation use already existing\n> git_path_submodule() function without changing overall behavior. Then it will\n> be possible to modify only that function whenever we need to change real\n> location of submodule's repository content.\n>\n> Signed-off-by: Max Kirillov \n> ---\n> submodule.c | 28 ++++++++++------------------\n> 1 file changed, 10 insertions(+), 18 deletions(-)\n>\n> diff --git a/submodule.c b/submodule.c\n> index 15e90d1..70d18ec 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n> @@ -122,43 +122,35 @@ void stage_updated_gitmodules(void)\n>\n> static int add_submodule_odb(const char *path)\n> {\n> - struct strbuf objects_directory = STRBUF_INIT;\n> struct alternate_object_database *alt_odb;\n> + const char *objects_directory;\n> int ret = 0;\n> - const char *git_dir;\n>\n> - strbuf_addf(&objects_directory, \"%s/.git\", path);\n> - git_dir = read_gitfile(objects_directory.buf);\n> - if (git_dir) {\n> - strbuf_reset(&objects_directory);\n> - strbuf_addstr(&objects_directory, git_dir);\n> - }\n> - strbuf_addstr(&objects_directory, \"/objects/\");\n> - if (!is_directory(objects_directory.buf)) {\n> + objects_directory = git_path_submodule(path, \"objects/\");\n\ngit_path_submodule is going away with jk/git-path (2015-08-10)\nSo you may want to use git_pathdup_submodule and free the\nresult of that function before returning.\n\n> + if (!is_directory(objects_directory)) {\n> ret = -1;\n> goto done;\n> }\n> +\n> /* avoid adding it twice */\n> for (alt_odb = alt_odb_list; alt_odb; alt_odb = alt_odb->next)\n> - if (alt_odb->name - alt_odb->base == objects_directory.len &&\n> - !strncmp(alt_odb->base, objects_directory.buf,\n> - objects_directory.len))\n> + if (alt_odb->name - alt_odb->base == strlen(objects_directory) &&\n> + !strcmp(alt_odb->base, objects_directory))\n> goto done;\n>\n> - alt_odb = xmalloc(objects_directory.len + 42 + sizeof(*alt_odb));\n> + alt_odb = xmalloc(strlen(objects_directory) + 42 + sizeof(*alt_odb));\n> alt_odb->next = alt_odb_list;\n> - strcpy(alt_odb->base, objects_directory.buf);\n> - alt_odb->name = alt_odb->base + objects_directory.len;\n> + strcpy(alt_odb->base, objects_directory);\n> + alt_odb->name = alt_odb->base + strlen(objects_directory);\n> alt_odb->name[2] = '/';\n> alt_odb->name[40] = '\\0';\n> alt_odb->name[41] = '\\0';\n> alt_odb_list = alt_odb;\n>\n> /* add possible alternates from the submodule */\n> - read_info_alternates(objects_directory.buf, 0);\n> + read_info_alternates(objects_directory, 0);\n> prepare_alt_odb();\n> done:\n> - strbuf_release(&objects_directory);\n> return ret;\n> }\n>\n\nThis looks good, though I am diving into the submodule code base\nmyself just now,\nso it's not a strong review. :)\n\n> --\n> 2.3.4.2801.g3d0809b\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n"},{"id":"269743","messageId":"1441922231-18270-1-git-send-email-max@max630.net","threadId":"38851","inReplyTo":"1438725925-3689-1-git-send-email-max@max630.net","subject":"[PATCH v8 0/2] Submodule object path","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2015-09-10T21:57:09Z","receivedAt":"2015-09-10T21:57:09Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"* Rebased to recent master (v2.6.0-rc0).\n* Use git_pathdup_submodule() instead of git_path_submodule()\n* There are more conflicts in pu with [1], not sure\n what should I do about it.\n* Style fixes as Stefan suggested\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/276628\n\nMax Kirillov (2):\n submodule refactor: use git_pathdup_submodule() in add_submodule_odb()\n path: implement common_dir handling in git_pathdup_submodule()\n\n cache.h | 1 +\n path.c | 22 ++++++++++++++++++----\n setup.c | 17 ++++++++++++-----\n submodule.c | 30 ++++++++++++------------------\n t/t7410-submodule-checkout-to.sh | 10 ++++++++++\n 5 files changed, 53 insertions(+), 27 deletions(-)\n\n-- \n2.3.4.2801.g3d0809b\n"},{"id":"269745","messageId":"1441922231-18270-2-git-send-email-max@max630.net","threadId":"38851","inReplyTo":"1441922231-18270-1-git-send-email-max@max630.net","subject":"[PATCH v8 1/2] submodule refactor: use git_pathdup_submodule() in add_submodule_odb()","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2015-09-10T21:57:10Z","receivedAt":"2015-09-10T21:57:10Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"Functions which directly operate submodule's object database do not handle the\ncase when the submodule is linked worktree (which are introduced in\nc7b3a3d2fe). Instead of fixing the path calculation use already existing\ngit_pathdup_submodule() function without changing overall behavior. Then it\nwill be possible to modify only that function whenever we need to change real\nlocation of submodule's repository content.\n\nSigned-off-by: Max Kirillov \n---\n submodule.c | 30 ++++++++++++------------------\n 1 file changed, 12 insertions(+), 18 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex 245ed4d..7340069 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -119,43 +119,37 @@ void stage_updated_gitmodules(void)\n \n static int add_submodule_odb(const char *path)\n {\n-\tstruct strbuf objects_directory = STRBUF_INIT;\n \tstruct alternate_object_database *alt_odb;\n+\tchar *objects_directory;\n \tint ret = 0;\n-\tconst char *git_dir;\n \n-\tstrbuf_addf(&objects_directory, \"%s/.git\", path);\n-\tgit_dir = read_gitfile(objects_directory.buf);\n-\tif (git_dir) {\n-\t\tstrbuf_reset(&objects_directory);\n-\t\tstrbuf_addstr(&objects_directory, git_dir);\n-\t}\n-\tstrbuf_addstr(&objects_directory, \"/objects/\");\n-\tif (!is_directory(objects_directory.buf)) {\n+\tobjects_directory = git_pathdup_submodule(path, \"objects/\");\n+\tif (!is_directory(objects_directory)) {\n \t\tret = -1;\n \t\tgoto done;\n \t}\n+\n \t/* avoid adding it twice */\n \tfor (alt_odb = alt_odb_list; alt_odb; alt_odb = alt_odb->next)\n-\t\tif (alt_odb->name - alt_odb->base == objects_directory.len &&\n-\t\t\t\t!strncmp(alt_odb->base, objects_directory.buf,\n-\t\t\t\t\tobjects_directory.len))\n+\t\tif (alt_odb->name - alt_odb->base == strlen(objects_directory) &&\n+\t\t\t\t!strcmp(alt_odb->base, objects_directory))\n \t\t\tgoto done;\n \n-\talt_odb = xmalloc(objects_directory.len + 42 + sizeof(*alt_odb));\n+\talt_odb = xmalloc(strlen(objects_directory) + 42 + sizeof(*alt_odb));\n \talt_odb->next = alt_odb_list;\n-\tstrcpy(alt_odb->base, objects_directory.buf);\n-\talt_odb->name = alt_odb->base + objects_directory.len;\n+\tstrcpy(alt_odb->base, objects_directory);\n+\talt_odb->name = alt_odb->base + strlen(objects_directory);\n \talt_odb->name[2] = '/';\n \talt_odb->name[40] = '\\0';\n \talt_odb->name[41] = '\\0';\n \talt_odb_list = alt_odb;\n \n \t/* add possible alternates from the submodule */\n-\tread_info_alternates(objects_directory.buf, 0);\n+\tread_info_alternates(objects_directory, 0);\n \tprepare_alt_odb();\n done:\n-\tstrbuf_release(&objects_directory);\n+\tfree(objects_directory);\n+\n \treturn ret;\n }\n \n-- \n2.3.4.2801.g3d0809b\n"},{"id":"269744","messageId":"1441922231-18270-3-git-send-email-max@max630.net","threadId":"38851","inReplyTo":"1441922231-18270-1-git-send-email-max@max630.net","subject":"[PATCH v8 2/2] path: implement common_dir handling in git_pathdup_submodule()","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2015-09-10T21:57:11Z","receivedAt":"2015-09-10T21:57:11Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"When submodule is a linked worktree, \"git diff --submodule\" and other\ncalls which directly access the submodule's object database do not correctly\ncalculate its path. Fix it by changing the git_pathdup_submodule() behavior,\nto use either common or per-worktree directory.\n\nDo it similarly as for parent repository, but ignore the GIT_COMMON_DIR\nenvironment variable, because it would mean common directory for the parent\nrepository and does not make sense for submodule.\n\nAlso add test for functionality which uses this call.\n\nSigned-off-by: Max Kirillov \n---\n cache.h | 1 +\n path.c | 22 ++++++++++++++++++----\n setup.c | 17 ++++++++++++-----\n t/t7410-submodule-checkout-to.sh | 10 ++++++++++\n 4 files changed, 41 insertions(+), 9 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 79066e5..5eb36b4 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -443,6 +443,7 @@ extern char *get_object_directory(void);\n extern char *get_index_file(void);\n extern char *get_graft_file(void);\n extern int set_git_dir(const char *path);\n+extern int get_common_dir_noenv(struct strbuf *sb, const char *gitdir);\n extern int get_common_dir(struct strbuf *sb, const char *gitdir);\n extern const char *get_git_namespace(void);\n extern const char *strip_namespace(const char *namespaced_ref);\ndiff --git a/path.c b/path.c\nindex 95acbaf..22b9e0b 100644\n--- a/path.c\n+++ b/path.c\n@@ -98,7 +98,7 @@ static const char *common_list[] = {\n \tNULL\n };\n \n-static void update_common_dir(struct strbuf *buf, int git_dir_len)\n+static void update_common_dir(struct strbuf *buf, int git_dir_len, const char *common_dir)\n {\n \tchar *base = buf->buf + git_dir_len;\n \tconst char **p;\n@@ -115,12 +115,16 @@ static void update_common_dir(struct strbuf *buf, int git_dir_len)\n \t\t\tpath++;\n \t\t\tis_dir = 1;\n \t\t}\n+\n+\t\tif (!common_dir)\n+\t\t\tcommon_dir = get_git_common_dir();\n+\n \t\tif (is_dir && dir_prefix(base, path)) {\n-\t\t\treplace_dir(buf, git_dir_len, get_git_common_dir());\n+\t\t\treplace_dir(buf, git_dir_len, common_dir);\n \t\t\treturn;\n \t\t}\n \t\tif (!is_dir && !strcmp(base, path)) {\n-\t\t\treplace_dir(buf, git_dir_len, get_git_common_dir());\n+\t\t\treplace_dir(buf, git_dir_len, common_dir);\n \t\t\treturn;\n \t\t}\n \t}\n@@ -160,7 +164,7 @@ static void adjust_git_path(struct strbuf *buf, int git_dir_len)\n \telse if (git_db_env && dir_prefix(base, \"objects\"))\n \t\treplace_dir(buf, git_dir_len + 7, get_object_directory());\n \telse if (git_common_dir_env)\n-\t\tupdate_common_dir(buf, git_dir_len);\n+\t\tupdate_common_dir(buf, git_dir_len, NULL);\n }\n \n static void do_git_path(struct strbuf *buf, const char *fmt, va_list args)\n@@ -228,6 +232,8 @@ static void do_submodule_path(struct strbuf *buf, const char *path,\n \t\t\t const char *fmt, va_list args)\n {\n \tconst char *git_dir;\n+\tstruct strbuf git_submodule_common_dir = STRBUF_INIT;\n+\tstruct strbuf git_submodule_dir = STRBUF_INIT;\n \n \tstrbuf_addstr(buf, path);\n \tif (buf->len && buf->buf[buf->len - 1] != '/')\n@@ -240,9 +246,17 @@ static void do_submodule_path(struct strbuf *buf, const char *path,\n \t\tstrbuf_addstr(buf, git_dir);\n \t}\n \tstrbuf_addch(buf, '/');\n+\tstrbuf_addstr(&git_submodule_dir, buf->buf);\n \n \tstrbuf_vaddf(buf, fmt, args);\n+\n+\tif (get_common_dir_noenv(&git_submodule_common_dir, git_submodule_dir.buf))\n+\t\tupdate_common_dir(buf, git_submodule_dir.len, git_submodule_common_dir.buf);\n+\n \tstrbuf_cleanup_path(buf);\n+\n+\tstrbuf_release(&git_submodule_dir);\n+\tstrbuf_release(&git_submodule_common_dir);\n }\n \n char *git_pathdup_submodule(const char *path, const char *fmt, ...)\ndiff --git a/setup.c b/setup.c\nindex a17c51e..e41e5e1 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -229,14 +229,21 @@ void verify_non_filename(const char *prefix, const char *arg)\n \n int get_common_dir(struct strbuf *sb, const char *gitdir)\n {\n+\tconst char *git_env_common_dir = getenv(GIT_COMMON_DIR_ENVIRONMENT);\n+\tif (git_env_common_dir) {\n+\t\tstrbuf_addstr(sb, git_env_common_dir);\n+\t\treturn 1;\n+\t} else {\n+\t\treturn get_common_dir_noenv(sb, gitdir);\n+\t}\n+}\n+\n+int get_common_dir_noenv(struct strbuf *sb, const char *gitdir)\n+{\n \tstruct strbuf data = STRBUF_INIT;\n \tstruct strbuf path = STRBUF_INIT;\n-\tconst char *git_common_dir = getenv(GIT_COMMON_DIR_ENVIRONMENT);\n \tint ret = 0;\n-\tif (git_common_dir) {\n-\t\tstrbuf_addstr(sb, git_common_dir);\n-\t\treturn 1;\n-\t}\n+\n \tstrbuf_addf(&path, \"%s/commondir\", gitdir);\n \tif (file_exists(path.buf)) {\n \t\tif (strbuf_read_file(&data, path.buf, 0) <= 0)\ndiff --git a/t/t7410-submodule-checkout-to.sh b/t/t7410-submodule-checkout-to.sh\nindex 3f609e8..1acef32 100755\n--- a/t/t7410-submodule-checkout-to.sh\n+++ b/t/t7410-submodule-checkout-to.sh\n@@ -47,4 +47,14 @@ test_expect_success 'checkout main and initialize independed clones' \\\n test_expect_success 'can see submodule diffs after independed cloning' \\\n '(cd fully_cloned_submodule/main && git diff --submodule master\"^!\" | grep \"file1 updated\")'\n \n+test_expect_success 'checkout sub manually' \\\n+ 'mkdir linked_submodule &&\n+ (cd clone/main &&\n+\tgit worktree add \"$base_path/linked_submodule/main\" \"$rev1_hash_main\") &&\n+ (cd clone/main/sub &&\n+\tgit worktree add \"$base_path/linked_submodule/main/sub\" \"$rev1_hash_sub\")'\n+\n+test_expect_success 'can see submodule diffs after manual checkout of linked submodule' \\\n+ '(cd linked_submodule/main && git diff --submodule master\"^!\" | grep \"file1 updated\")'\n+\n test_done\n-- \n2.3.4.2801.g3d0809b\n"},{"id":"269752","messageId":"xmqqoah9222y.fsf@gitster.mtv.corp.google.com","threadId":"38851","inReplyTo":"1441922231-18270-1-git-send-email-max@max630.net","subject":"Re: [PATCH v8 0/2] Submodule object path","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-09-11T01:10:13Z","receivedAt":"2015-09-11T01:10:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Max Kirillov writes:\n\n> * There are more conflicts in pu with [1], not sure\n> what should I do about it.\n>\n> [1] http://thread.gmane.org/gmane.comp.version-control.git/276628\n\nI do not think conflicts got any worse than before. Between the\nresult of merging this round and the previous one to 'pu', the only\ndifference in path.c that conflicts with [1] I see is essentially a\nno-op (the diff attached at the end of this essage).\n\nWhen I push the updated 'pu' out, could you please check\n\n $ git log --oneline --first-parent origin/master..origin/pu\n\nto find the place your topic is merged (I am planning to make it\n\"3af7cd6 Merge branch 'mk/submodule-gitdir-path' into pu\", but\nplease do not rely on the exact commit object name; I may have to\nredo the integration if I find some breakages caused by this or any\nother topic), and then eyeballing (call that merge $m) output of\n\n $ git diff $m^ $m\n\nto see if there is anything I screwed up?\n\nThanks.\n\n\ndiff --git a/path.c b/path.c\nindex 776d8f0..2725bc3 100644\n--- a/path.c\n+++ b/path.c\n@@ -466,11 +466,11 @@ static void do_submodule_path(struct strbuf *buf, const char *path,\n \n \tstrbuf_vaddf(buf, fmt, args);\n \n-\tif (get_common_dir_noenv(&git_submodule_common_dir, git_submodule_dir.buf)) {\n+\tif (get_common_dir_noenv(&git_submodule_common_dir, git_submodule_dir.buf))\n \t\tupdate_common_dir(buf, git_submodule_dir.len, git_submodule_common_dir.buf);\n-\t}\n \n \tstrbuf_cleanup_path(buf);\n+\n \tstrbuf_release(&git_submodule_dir);\n \tstrbuf_release(&git_submodule_common_dir);\n }\n"},{"id":"269754","messageId":"20150911075340.GA18917@sigill.intra.peff.net","threadId":"38851","inReplyTo":"1441922231-18270-2-git-send-email-max@max630.net","subject":"Re: [PATCH v8 1/2] submodule refactor: use git_pathdup_submodule() in add_submodule_odb()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-09-11T07:53:40Z","receivedAt":"2015-09-11T07:53:40Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Sep 11, 2015 at 12:57:10AM +0300, Max Kirillov wrote:\n\n> diff --git a/submodule.c b/submodule.c\n> index 245ed4d..7340069 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n> @@ -119,43 +119,37 @@ void stage_updated_gitmodules(void)\n> \n> static int add_submodule_odb(const char *path)\n> {\n> -\tstruct strbuf objects_directory = STRBUF_INIT;\n> \tstruct alternate_object_database *alt_odb;\n> +\tchar *objects_directory;\n\nNow that we have strbuf_git_path_submodule(), is there any reason to\nswitch this away from a strbuf?\n\nThat saves us a bunch of strlen calls, and it makes the diff way\nshorter. My ulterior motive is that the result also conflicts a lot less\nwith some patches I'm about to post to harden the malloc and strcpy\ncalls below.\n\nThat would make your patch look something like:\n\ndiff --git a/submodule.c b/submodule.c\nindex 245ed4d..5e5a46f 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -122,15 +122,8 @@ static int add_submodule_odb(const char *path)\n \tstruct strbuf objects_directory = STRBUF_INIT;\n \tstruct alternate_object_database *alt_odb;\n \tint ret = 0;\n-\tconst char *git_dir;\n \n-\tstrbuf_addf(&objects_directory, \"%s/.git\", path);\n-\tgit_dir = read_gitfile(objects_directory.buf);\n-\tif (git_dir) {\n-\t\tstrbuf_reset(&objects_directory);\n-\t\tstrbuf_addstr(&objects_directory, git_dir);\n-\t}\n-\tstrbuf_addstr(&objects_directory, \"/objects/\");\n+\tstrbuf_git_path_submodule(&objects_directory, path, \"objects/\");\n \tif (!is_directory(objects_directory.buf)) {\n \t\tret = -1;\n \t\tgoto done;\n\n-Peff\n"},{"id":"269897","messageId":"20150913222619.GA8042@wheezy.local","threadId":"38851","inReplyTo":"20150911075340.GA18917@sigill.intra.peff.net","subject":"Re: [PATCH v8 1/2] submodule refactor: use git_pathdup_submodule() in add_submodule_odb()","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2015-09-13T22:26:29Z","receivedAt":"2015-09-13T22:26:29Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"On Fri, Sep 11, 2015 at 03:53:40AM -0400, Jeff King wrote:\n> \n> Now that we have strbuf_git_path_submodule(), is there any reason to\n> switch this away from a strbuf?\n> \n> That saves us a bunch of strlen calls, and it makes the diff way\n> shorter. My ulterior motive is that the result also conflicts a lot less\n> with some patches I'm about to post to harden the malloc and strcpy\n> calls below.\n> \n> That would make your patch look something like:\n> \n> diff --git a/submodule.c b/submodule.c\n> index 245ed4d..5e5a46f 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n> @@ -122,15 +122,8 @@ static int add_submodule_odb(const char *path)\n> \tstruct strbuf objects_directory = STRBUF_INIT;\n> \tstruct alternate_object_database *alt_odb;\n> \tint ret = 0;\n> -\tconst char *git_dir;\n> \n> -\tstrbuf_addf(&objects_directory, \"%s/.git\", path);\n> -\tgit_dir = read_gitfile(objects_directory.buf);\n> -\tif (git_dir) {\n> -\t\tstrbuf_reset(&objects_directory);\n> -\t\tstrbuf_addstr(&objects_directory, git_dir);\n> -\t}\n> -\tstrbuf_addstr(&objects_directory, \"/objects/\");\n> +\tstrbuf_git_path_submodule(&objects_directory, path, \"objects/\");\n> \tif (!is_directory(objects_directory.buf)) {\n> \t\tret = -1;\n> \t\tgoto done;\n> \n> -Peff\n\nThank you. This is how it looks now.\n\n-- \nMax\n"},{"id":"269898","messageId":"20150913223256.GB8042@wheezy.local","threadId":"38851","inReplyTo":"xmqqoah9222y.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v8 0/2] Submodule object path","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2015-09-13T22:32:56Z","receivedAt":"2015-09-13T22:32:56Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"On Thu, Sep 10, 2015 at 06:10:13PM -0700, Junio C Hamano wrote:\n> When I push the updated 'pu' out, could you please check\n\nI follow the pu merges. So far resolutions seem correct and\nall tests pass. If you don't mind re-resolving it as I send\nnewer versions I will base them on master.\n\nThank you.\n\n-- \nMax\n"},{"id":"269953","messageId":"xmqqk2rsx404.fsf@gitster.mtv.corp.google.com","threadId":"38851","inReplyTo":"20150913223256.GB8042@wheezy.local","subject":"Re: [PATCH v8 0/2] Submodule object path","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-09-14T18:14:19Z","receivedAt":"2015-09-14T18:14:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Max Kirillov writes:\n\n> On Thu, Sep 10, 2015 at 06:10:13PM -0700, Junio C Hamano wrote:\n>> When I push the updated 'pu' out, could you please check\n>\n> I follow the pu merges. So far resolutions seem correct and\n> all tests pass.\n\nThe primary thing I ask from \"eyeballing by the original author\" is\nto see if \"git diff M^ M\" for the merge M of the topic is something\nthe original author would have done if he was given a starting point\nof M^ to build his or her topic (which is what your \"resolutions\nseem correct\" really is), so we are good.\n\n> If you don't mind re-resolving it as I send newer versions I will\n> base them on master.\n\nSo far rerere seems to be happy, so basing on the same commit is\nfine, but I suspect this round should already be ready for 'next'.\n\nI had to remove the now-unused variable \"const char *git_dir;\" in\n[PATCH 1/2], but other than that it looked OK from a cursory re-read.\n\nThanks.\n"}]}