{"thread":{"id":"40345","subject":"[PATCH v9 1/2] submodule refactor: use strbuf_git_path_submodule() in add_submodule_odb()","startedAt":"2015-09-13T22:17:40Z","lastAt":"2015-09-14T18:00:56Z","messageCount":4,"participants":["Max Kirillov","Junio C Hamano"],"isPatch":true,"patchVersion":9,"patchTotal":2},"messages":[{"id":"269895","messageId":"1442182662-28834-1-git-send-email-max@max630.net","threadId":"40345","inReplyTo":"1441922231-18270-1-git-send-email-max@max630.net","subject":"[PATCH v9 0/2] Submodule object path","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2015-09-13T22:17:40Z","receivedAt":"2015-09-13T22:17:40Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"Since v8:\nUse strbuf_git_path_submodule() as Jeff suggested\n\nMax Kirillov (2):\n  submodule refactor: use strbuf_git_path_submodule() in\n    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                      |  8 +-------\n t/t7410-submodule-checkout-to.sh | 10 ++++++++++\n 5 files changed, 42 insertions(+), 16 deletions(-)\n\n-- \n2.3.4.2801.g3d0809b\n"},{"id":"269894","messageId":"1442182662-28834-2-git-send-email-max@max630.net","threadId":"40345","inReplyTo":"1442182662-28834-1-git-send-email-max@max630.net","subject":"[PATCH v9 1/2] submodule refactor: use strbuf_git_path_submodule() in add_submodule_odb()","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2015-09-13T22:17:41Z","receivedAt":"2015-09-13T22:17:41Z","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\nhandle the case when the submodule is linked worktree (which are\nintroduced in c7b3a3d2fe). Instead of fixing the path calculation use\nalready existing strbuf_git_path_submodule() function without changing\noverall behaviour. Then it will be possible to modify only that function\nwhenever we need to change real location of submodule's repository\ncontent.\n\nEdited-by: Jeff King <peff@peff.net>\nSigned-off-by: Max Kirillov <max@max630.net>\n---\n submodule.c | 8 +-------\n 1 file changed, 1 insertion(+), 7 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex 245ed4d..16b7c35 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -124,13 +124,7 @@ static int add_submodule_odb(const char *path)\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-- \n2.3.4.2801.g3d0809b\n"},{"id":"269896","messageId":"1442182662-28834-3-git-send-email-max@max630.net","threadId":"40345","inReplyTo":"1442182662-28834-1-git-send-email-max@max630.net","subject":"[PATCH v9 2/2] path: implement common_dir handling in git_pathdup_submodule()","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2015-09-13T22:17:42Z","receivedAt":"2015-09-13T22:17:42Z","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 <max@max630.net>\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":"269951","messageId":"xmqqoah4x4mf.fsf@gitster.mtv.corp.google.com","threadId":"40345","inReplyTo":"1442182662-28834-2-git-send-email-max@max630.net","subject":"Re: [PATCH v9 1/2] submodule refactor: use strbuf_git_path_submodule() in add_submodule_odb()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-09-14T18:00:56Z","receivedAt":"2015-09-14T18:00:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Max Kirillov <max@max630.net> writes:\n\n> Functions which directly operate submodule's object database do not\n> handle the case when the submodule is linked worktree (which are\n> introduced in c7b3a3d2fe). Instead of fixing the path calculation use\n> already existing strbuf_git_path_submodule() function without changing\n> overall behaviour. Then it will be possible to modify only that function\n> whenever we need to change real location of submodule's repository\n> content.\n>\n> Edited-by: Jeff King <peff@peff.net>\n> Signed-off-by: Max Kirillov <max@max630.net>\n> ---\n>  submodule.c | 8 +-------\n>  1 file changed, 1 insertion(+), 7 deletions(-)\n\nThis certainly is a lot nicer ;-)\n\nThanks.\n\n>\n> diff --git a/submodule.c b/submodule.c\n> index 245ed4d..16b7c35 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n> @@ -124,13 +124,7 @@ static int add_submodule_odb(const char *path)\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"}]}