# [PATCH v9 1/2] submodule refactor: use strbuf_git_path_submodule() in add_submodule_odb() 4 messages from 2015-09-13 to 2015-09-14. Participants: Max Kirillov, Junio C Hamano. Thread: https://gitlist.dev/t/40345 ## Max Kirillov, 2015-09-13 22:17 Subject: [PATCH v9 0/2] Submodule object path Message-ID: <1442182662-28834-1-git-send-email-max@max630.net> URL: https://gitlist.dev/e/1442182662-28834-1-git-send-email-max%40max630.net In-Reply-To: <1441922231-18270-1-git-send-email-max@max630.net> ``` Since v8: Use strbuf_git_path_submodule() as Jeff suggested Max Kirillov (2): submodule refactor: use strbuf_git_path_submodule() in add_submodule_odb() path: implement common_dir handling in git_pathdup_submodule() cache.h | 1 + path.c | 22 ++++++++++++++++++---- setup.c | 17 ++++++++++++----- submodule.c | 8 +------- t/t7410-submodule-checkout-to.sh | 10 ++++++++++ 5 files changed, 42 insertions(+), 16 deletions(-) -- 2.3.4.2801.g3d0809b ``` ## Max Kirillov, 2015-09-13 22:17 Subject: [PATCH v9 1/2] submodule refactor: use strbuf_git_path_submodule() in add_submodule_odb() Message-ID: <1442182662-28834-2-git-send-email-max@max630.net> URL: https://gitlist.dev/e/1442182662-28834-2-git-send-email-max%40max630.net In-Reply-To: <1442182662-28834-1-git-send-email-max@max630.net> ``` Functions which directly operate submodule's object database do not handle the case when the submodule is linked worktree (which are introduced in c7b3a3d2fe). Instead of fixing the path calculation use already existing strbuf_git_path_submodule() function without changing overall behaviour. Then it will be possible to modify only that function whenever we need to change real location of submodule's repository content. Edited-by: Jeff King Signed-off-by: Max Kirillov --- submodule.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/submodule.c b/submodule.c index 245ed4d..16b7c35 100644 --- a/submodule.c +++ b/submodule.c @@ -124,13 +124,7 @@ static int add_submodule_odb(const char *path) int ret = 0; const char *git_dir; - strbuf_addf(&objects_directory, "%s/.git", path); - git_dir = read_gitfile(objects_directory.buf); - if (git_dir) { - strbuf_reset(&objects_directory); - strbuf_addstr(&objects_directory, git_dir); - } - strbuf_addstr(&objects_directory, "/objects/"); + strbuf_git_path_submodule(&objects_directory, path, "objects/"); if (!is_directory(objects_directory.buf)) { ret = -1; goto done; -- 2.3.4.2801.g3d0809b ``` ## Max Kirillov, 2015-09-13 22:17 Subject: [PATCH v9 2/2] path: implement common_dir handling in git_pathdup_submodule() Message-ID: <1442182662-28834-3-git-send-email-max@max630.net> URL: https://gitlist.dev/e/1442182662-28834-3-git-send-email-max%40max630.net In-Reply-To: <1442182662-28834-1-git-send-email-max@max630.net> ``` When submodule is a linked worktree, "git diff --submodule" and other calls which directly access the submodule's object database do not correctly calculate its path. Fix it by changing the git_pathdup_submodule() behavior, to use either common or per-worktree directory. Do it similarly as for parent repository, but ignore the GIT_COMMON_DIR environment variable, because it would mean common directory for the parent repository and does not make sense for submodule. Also add test for functionality which uses this call. Signed-off-by: Max Kirillov --- cache.h | 1 + path.c | 22 ++++++++++++++++++---- setup.c | 17 ++++++++++++----- t/t7410-submodule-checkout-to.sh | 10 ++++++++++ 4 files changed, 41 insertions(+), 9 deletions(-) diff --git a/cache.h b/cache.h index 79066e5..5eb36b4 100644 --- a/cache.h +++ b/cache.h @@ -443,6 +443,7 @@ extern char *get_object_directory(void); extern char *get_index_file(void); extern char *get_graft_file(void); extern int set_git_dir(const char *path); +extern int get_common_dir_noenv(struct strbuf *sb, const char *gitdir); extern int get_common_dir(struct strbuf *sb, const char *gitdir); extern const char *get_git_namespace(void); extern const char *strip_namespace(const char *namespaced_ref); diff --git a/path.c b/path.c index 95acbaf..22b9e0b 100644 --- a/path.c +++ b/path.c @@ -98,7 +98,7 @@ static const char *common_list[] = { NULL }; -static void update_common_dir(struct strbuf *buf, int git_dir_len) +static void update_common_dir(struct strbuf *buf, int git_dir_len, const char *common_dir) { char *base = buf->buf + git_dir_len; const char **p; @@ -115,12 +115,16 @@ static void update_common_dir(struct strbuf *buf, int git_dir_len) path++; is_dir = 1; } + + if (!common_dir) + common_dir = get_git_common_dir(); + if (is_dir && dir_prefix(base, path)) { - replace_dir(buf, git_dir_len, get_git_common_dir()); + replace_dir(buf, git_dir_len, common_dir); return; } if (!is_dir && !strcmp(base, path)) { - replace_dir(buf, git_dir_len, get_git_common_dir()); + replace_dir(buf, git_dir_len, common_dir); return; } } @@ -160,7 +164,7 @@ static void adjust_git_path(struct strbuf *buf, int git_dir_len) else if (git_db_env && dir_prefix(base, "objects")) replace_dir(buf, git_dir_len + 7, get_object_directory()); else if (git_common_dir_env) - update_common_dir(buf, git_dir_len); + update_common_dir(buf, git_dir_len, NULL); } static void do_git_path(struct strbuf *buf, const char *fmt, va_list args) @@ -228,6 +232,8 @@ static void do_submodule_path(struct strbuf *buf, const char *path, const char *fmt, va_list args) { const char *git_dir; + struct strbuf git_submodule_common_dir = STRBUF_INIT; + struct strbuf git_submodule_dir = STRBUF_INIT; strbuf_addstr(buf, path); if (buf->len && buf->buf[buf->len - 1] != '/') @@ -240,9 +246,17 @@ static void do_submodule_path(struct strbuf *buf, const char *path, strbuf_addstr(buf, git_dir); } strbuf_addch(buf, '/'); + strbuf_addstr(&git_submodule_dir, buf->buf); strbuf_vaddf(buf, fmt, args); + + if (get_common_dir_noenv(&git_submodule_common_dir, git_submodule_dir.buf)) + update_common_dir(buf, git_submodule_dir.len, git_submodule_common_dir.buf); + strbuf_cleanup_path(buf); + + strbuf_release(&git_submodule_dir); + strbuf_release(&git_submodule_common_dir); } char *git_pathdup_submodule(const char *path, const char *fmt, ...) diff --git a/setup.c b/setup.c index a17c51e..e41e5e1 100644 --- a/setup.c +++ b/setup.c @@ -229,14 +229,21 @@ void verify_non_filename(const char *prefix, const char *arg) int get_common_dir(struct strbuf *sb, const char *gitdir) { + const char *git_env_common_dir = getenv(GIT_COMMON_DIR_ENVIRONMENT); + if (git_env_common_dir) { + strbuf_addstr(sb, git_env_common_dir); + return 1; + } else { + return get_common_dir_noenv(sb, gitdir); + } +} + +int get_common_dir_noenv(struct strbuf *sb, const char *gitdir) +{ struct strbuf data = STRBUF_INIT; struct strbuf path = STRBUF_INIT; - const char *git_common_dir = getenv(GIT_COMMON_DIR_ENVIRONMENT); int ret = 0; - if (git_common_dir) { - strbuf_addstr(sb, git_common_dir); - return 1; - } + strbuf_addf(&path, "%s/commondir", gitdir); if (file_exists(path.buf)) { if (strbuf_read_file(&data, path.buf, 0) <= 0) diff --git a/t/t7410-submodule-checkout-to.sh b/t/t7410-submodule-checkout-to.sh index 3f609e8..1acef32 100755 --- a/t/t7410-submodule-checkout-to.sh +++ b/t/t7410-submodule-checkout-to.sh @@ -47,4 +47,14 @@ test_expect_success 'checkout main and initialize independed clones' \ test_expect_success 'can see submodule diffs after independed cloning' \ '(cd fully_cloned_submodule/main && git diff --submodule master"^!" | grep "file1 updated")' +test_expect_success 'checkout sub manually' \ + 'mkdir linked_submodule && + (cd clone/main && + git worktree add "$base_path/linked_submodule/main" "$rev1_hash_main") && + (cd clone/main/sub && + git worktree add "$base_path/linked_submodule/main/sub" "$rev1_hash_sub")' + +test_expect_success 'can see submodule diffs after manual checkout of linked submodule' \ + '(cd linked_submodule/main && git diff --submodule master"^!" | grep "file1 updated")' + test_done -- 2.3.4.2801.g3d0809b ``` ## Junio C Hamano, 2015-09-14 18:00 Subject: Re: [PATCH v9 1/2] submodule refactor: use strbuf_git_path_submodule() in add_submodule_odb() Message-ID: URL: https://gitlist.dev/e/xmqqoah4x4mf.fsf%40gitster.mtv.corp.google.com In-Reply-To: <1442182662-28834-2-git-send-email-max@max630.net> ``` Max Kirillov writes: > Functions which directly operate submodule's object database do not > handle the case when the submodule is linked worktree (which are > introduced in c7b3a3d2fe). Instead of fixing the path calculation use > already existing strbuf_git_path_submodule() function without changing > overall behaviour. Then it will be possible to modify only that function > whenever we need to change real location of submodule's repository > content. > > Edited-by: Jeff King > Signed-off-by: Max Kirillov > --- > submodule.c | 8 +------- > 1 file changed, 1 insertion(+), 7 deletions(-) This certainly is a lot nicer ;-) Thanks. > > diff --git a/submodule.c b/submodule.c > index 245ed4d..16b7c35 100644 > --- a/submodule.c > +++ b/submodule.c > @@ -124,13 +124,7 @@ static int add_submodule_odb(const char *path) > int ret = 0; > const char *git_dir; > > - strbuf_addf(&objects_directory, "%s/.git", path); > - git_dir = read_gitfile(objects_directory.buf); > - if (git_dir) { > - strbuf_reset(&objects_directory); > - strbuf_addstr(&objects_directory, git_dir); > - } > - strbuf_addstr(&objects_directory, "/objects/"); > + strbuf_git_path_submodule(&objects_directory, path, "objects/"); > if (!is_directory(objects_directory.buf)) { > ret = -1; > goto done; ```