* [PATCH v4 0/2] path: implement common_dir handling in git_path_submodule()
@ 2015-03-18 21:10 Max Kirillov
2015-03-18 21:10 ` [PATCH v4 1/2] submodule refactor: use git_path_submodule() in add_submodule_odb() Max Kirillov
` (2 more replies)
0 siblings, 3 replies; 27+ messages in thread
From: Max Kirillov @ 2015-03-18 21:10 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy, Jens Lehmann
Cc: Junio C Hamano, git, Max Kirillov
After http://thread.gmane.org/gmane.comp.version-control.git/261990
the only thing which did not enter the serie is these 2 pathes.
Should be applied over the patches by link, or
ecf2ff6ace6a1cc3d55b6f917be5452b5fb0c21b in current pu.
Max Kirillov (2):
submodule refactor: use git_path_submodule() in add_submodule_odb()
path: implement common_dir handling in git_path_submodule()
cache.h | 1 +
path.c | 24 ++++++++++++++++++++----
setup.c | 17 ++++++++++++-----
submodule.c | 28 ++++++++++------------------
t/t7410-submodule-checkout-to.sh | 10 ++++++++++
5 files changed, 53 insertions(+), 27 deletions(-)
--
2.1.1.391.g7a54a76
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH v4 1/2] submodule refactor: use git_path_submodule() in add_submodule_odb() 2015-03-18 21:10 [PATCH v4 0/2] path: implement common_dir handling in git_path_submodule() Max Kirillov @ 2015-03-18 21:10 ` Max Kirillov 2015-03-18 21:10 ` [PATCH v4 2/2] path: implement common_dir handling in git_path_submodule() Max Kirillov 2015-08-03 21:03 ` [PATCH v5 0/2] " Max Kirillov 2 siblings, 0 replies; 27+ messages in thread From: Max Kirillov @ 2015-03-18 21:10 UTC (permalink / raw) To: Nguyễn Thái Ngọc Duy, Jens Lehmann Cc: Junio C Hamano, git, Max Kirillov Signed-off-by: Max Kirillov <max@max630.net> --- submodule.c | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/submodule.c b/submodule.c index 34094f5..4aad3d4 100644 --- a/submodule.c +++ b/submodule.c @@ -122,43 +122,35 @@ void stage_updated_gitmodules(void) static int add_submodule_odb(const char *path) { - struct strbuf objects_directory = STRBUF_INIT; struct alternate_object_database *alt_odb; + const char* objects_directory; 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/"); - if (!is_directory(objects_directory.buf)) { + objects_directory = git_path_submodule(path, "objects/"); + if (!is_directory(objects_directory)) { ret = -1; goto done; } + /* avoid adding it twice */ for (alt_odb = alt_odb_list; alt_odb; alt_odb = alt_odb->next) - if (alt_odb->name - alt_odb->base == objects_directory.len && - !strncmp(alt_odb->base, objects_directory.buf, - objects_directory.len)) + if (alt_odb->name - alt_odb->base == strlen(objects_directory) && + !strcmp(alt_odb->base, objects_directory)) goto done; - alt_odb = xmalloc(objects_directory.len + 42 + sizeof(*alt_odb)); + alt_odb = xmalloc(strlen(objects_directory) + 42 + sizeof(*alt_odb)); alt_odb->next = alt_odb_list; - strcpy(alt_odb->base, objects_directory.buf); - alt_odb->name = alt_odb->base + objects_directory.len; + strcpy(alt_odb->base, objects_directory); + alt_odb->name = alt_odb->base + strlen(objects_directory); alt_odb->name[2] = '/'; alt_odb->name[40] = '\0'; alt_odb->name[41] = '\0'; alt_odb_list = alt_odb; /* add possible alternates from the submodule */ - read_info_alternates(objects_directory.buf, 0); + read_info_alternates(objects_directory, 0); prepare_alt_odb(); done: - strbuf_release(&objects_directory); return ret; } -- 2.1.1.391.g7a54a76 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v4 2/2] path: implement common_dir handling in git_path_submodule() 2015-03-18 21:10 [PATCH v4 0/2] path: implement common_dir handling in git_path_submodule() Max Kirillov 2015-03-18 21:10 ` [PATCH v4 1/2] submodule refactor: use git_path_submodule() in add_submodule_odb() Max Kirillov @ 2015-03-18 21:10 ` Max Kirillov 2015-08-03 21:03 ` [PATCH v5 0/2] " Max Kirillov 2 siblings, 0 replies; 27+ messages in thread From: Max Kirillov @ 2015-03-18 21:10 UTC (permalink / raw) To: Nguyễn Thái Ngọc Duy, Jens Lehmann Cc: Junio C Hamano, git, Max Kirillov This allows making submodules a linked workdirs. Same as for .git, but ignores 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 <max@max630.net> --- cache.h | 1 + path.c | 24 ++++++++++++++++++++---- setup.c | 17 ++++++++++++----- t/t7410-submodule-checkout-to.sh | 10 ++++++++++ 4 files changed, 43 insertions(+), 9 deletions(-) diff --git a/cache.h b/cache.h index 670e861..4cd03f0 100644 --- a/cache.h +++ b/cache.h @@ -437,6 +437,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 a5c51a3..78f718f 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,17 @@ 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 +165,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) @@ -256,6 +261,8 @@ const char *git_path_submodule(const char *path, const char *fmt, ...) { struct strbuf *buf = get_pathname(); const char *git_dir; + struct strbuf git_submodule_common_dir = STRBUF_INIT; + struct strbuf git_submodule_dir = STRBUF_INIT; va_list args; strbuf_addstr(buf, path); @@ -269,11 +276,20 @@ const char *git_path_submodule(const char *path, const char *fmt, ...) strbuf_addstr(buf, git_dir); } strbuf_addch(buf, '/'); + strbuf_addstr(&git_submodule_dir, buf->buf); va_start(args, fmt); strbuf_vaddf(buf, fmt, args); va_end(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); return buf->buf; } diff --git a/setup.c b/setup.c index fb61860..ffda622 100644 --- a/setup.c +++ b/setup.c @@ -226,14 +226,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 8f30aed..b43391a 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 checkout --to "$base_path/linked_submodule/main" "$rev1_hash_main") && + (cd clone/main/sub && + git checkout --to "$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.1.1.391.g7a54a76 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v5 0/2] path: implement common_dir handling in git_path_submodule() 2015-03-18 21:10 [PATCH v4 0/2] path: implement common_dir handling in git_path_submodule() Max Kirillov 2015-03-18 21:10 ` [PATCH v4 1/2] submodule refactor: use git_path_submodule() in add_submodule_odb() Max Kirillov 2015-03-18 21:10 ` [PATCH v4 2/2] path: implement common_dir handling in git_path_submodule() Max Kirillov @ 2015-08-03 21:03 ` Max Kirillov 2015-08-03 21:03 ` [PATCH v5 1/2] submodule refactor: use git_path_submodule() in add_submodule_odb() Max Kirillov ` (2 more replies) 2 siblings, 3 replies; 27+ messages in thread From: Max Kirillov @ 2015-08-03 21:03 UTC (permalink / raw) To: Jens Lehmann, Duy Nguyen; +Cc: Max Kirillov, Junio C Hamano, git Rebased to latest master, also merges cleanly to pu. Otherwise no changes. Max Kirillov (2): submodule refactor: use git_path_submodule() in add_submodule_odb() path: implement common_dir handling in git_path_submodule() cache.h | 1 + path.c | 24 ++++++++++++++++++++---- setup.c | 17 ++++++++++++----- submodule.c | 28 ++++++++++------------------ t/t7410-submodule-checkout-to.sh | 10 ++++++++++ 5 files changed, 53 insertions(+), 27 deletions(-) -- 2.3.4.2801.g3d0809b ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v5 1/2] submodule refactor: use git_path_submodule() in add_submodule_odb() 2015-08-03 21:03 ` [PATCH v5 0/2] " Max Kirillov @ 2015-08-03 21:03 ` Max Kirillov 2015-08-03 21:29 ` Junio C Hamano 2015-08-03 21:03 ` [PATCH v5 2/2] path: implement common_dir handling in git_path_submodule() Max Kirillov 2015-08-04 21:51 ` [PATCH v6 0/2] " Max Kirillov 2 siblings, 1 reply; 27+ messages in thread From: Max Kirillov @ 2015-08-03 21:03 UTC (permalink / raw) To: Jens Lehmann, Duy Nguyen; +Cc: Max Kirillov, Junio C Hamano, git Signed-off-by: Max Kirillov <max@max630.net> --- submodule.c | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/submodule.c b/submodule.c index 15e90d1..f6afe0a 100644 --- a/submodule.c +++ b/submodule.c @@ -122,43 +122,35 @@ void stage_updated_gitmodules(void) static int add_submodule_odb(const char *path) { - struct strbuf objects_directory = STRBUF_INIT; struct alternate_object_database *alt_odb; + const char* objects_directory; 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/"); - if (!is_directory(objects_directory.buf)) { + objects_directory = git_path_submodule(path, "objects/"); + if (!is_directory(objects_directory)) { ret = -1; goto done; } + /* avoid adding it twice */ for (alt_odb = alt_odb_list; alt_odb; alt_odb = alt_odb->next) - if (alt_odb->name - alt_odb->base == objects_directory.len && - !strncmp(alt_odb->base, objects_directory.buf, - objects_directory.len)) + if (alt_odb->name - alt_odb->base == strlen(objects_directory) && + !strcmp(alt_odb->base, objects_directory)) goto done; - alt_odb = xmalloc(objects_directory.len + 42 + sizeof(*alt_odb)); + alt_odb = xmalloc(strlen(objects_directory) + 42 + sizeof(*alt_odb)); alt_odb->next = alt_odb_list; - strcpy(alt_odb->base, objects_directory.buf); - alt_odb->name = alt_odb->base + objects_directory.len; + strcpy(alt_odb->base, objects_directory); + alt_odb->name = alt_odb->base + strlen(objects_directory); alt_odb->name[2] = '/'; alt_odb->name[40] = '\0'; alt_odb->name[41] = '\0'; alt_odb_list = alt_odb; /* add possible alternates from the submodule */ - read_info_alternates(objects_directory.buf, 0); + read_info_alternates(objects_directory, 0); prepare_alt_odb(); done: - strbuf_release(&objects_directory); return ret; } -- 2.3.4.2801.g3d0809b ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v5 1/2] submodule refactor: use git_path_submodule() in add_submodule_odb() 2015-08-03 21:03 ` [PATCH v5 1/2] submodule refactor: use git_path_submodule() in add_submodule_odb() Max Kirillov @ 2015-08-03 21:29 ` Junio C Hamano 0 siblings, 0 replies; 27+ messages in thread From: Junio C Hamano @ 2015-08-03 21:29 UTC (permalink / raw) To: Max Kirillov; +Cc: Jens Lehmann, Duy Nguyen, git Max Kirillov <max@max630.net> writes: Here is a space to describe why this change is a good thing. Is this a fix to change the behaviour, and if so how is the behaviour different with and without the patch? Or is this just to drop the block of code from here and replace it with a call to an existing helper that does exactly the same thing? I _suspect_ that it is the latter, but please do not force reviewers to guess. > Signed-off-by: Max Kirillov <max@max630.net> > --- > submodule.c | 28 ++++++++++------------------ > 1 file changed, 10 insertions(+), 18 deletions(-) > > diff --git a/submodule.c b/submodule.c > index 15e90d1..f6afe0a 100644 > --- a/submodule.c > +++ b/submodule.c > @@ -122,43 +122,35 @@ void stage_updated_gitmodules(void) > > static int add_submodule_odb(const char *path) > { > - struct strbuf objects_directory = STRBUF_INIT; > struct alternate_object_database *alt_odb; > + const char* objects_directory; Style; asterisk sticks to the variable, not the type. I think you need to fix this in multiple places (not just 1/2 but also in 2/2). Other than that I think it is a sensible "replace bulk of code with identical helper that already exists" rewrite. Thanks. > 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/"); > - if (!is_directory(objects_directory.buf)) { > + objects_directory = git_path_submodule(path, "objects/"); > + if (!is_directory(objects_directory)) { > ret = -1; > goto done; > } > + > /* avoid adding it twice */ > for (alt_odb = alt_odb_list; alt_odb; alt_odb = alt_odb->next) > - if (alt_odb->name - alt_odb->base == objects_directory.len && > - !strncmp(alt_odb->base, objects_directory.buf, > - objects_directory.len)) > + if (alt_odb->name - alt_odb->base == strlen(objects_directory) && > + !strcmp(alt_odb->base, objects_directory)) > goto done; > > - alt_odb = xmalloc(objects_directory.len + 42 + sizeof(*alt_odb)); > + alt_odb = xmalloc(strlen(objects_directory) + 42 + sizeof(*alt_odb)); > alt_odb->next = alt_odb_list; > - strcpy(alt_odb->base, objects_directory.buf); > - alt_odb->name = alt_odb->base + objects_directory.len; > + strcpy(alt_odb->base, objects_directory); > + alt_odb->name = alt_odb->base + strlen(objects_directory); > alt_odb->name[2] = '/'; > alt_odb->name[40] = '\0'; > alt_odb->name[41] = '\0'; > alt_odb_list = alt_odb; > > /* add possible alternates from the submodule */ > - read_info_alternates(objects_directory.buf, 0); > + read_info_alternates(objects_directory, 0); > prepare_alt_odb(); > done: > - strbuf_release(&objects_directory); > return ret; > } ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v5 2/2] path: implement common_dir handling in git_path_submodule() 2015-08-03 21:03 ` [PATCH v5 0/2] " Max Kirillov 2015-08-03 21:03 ` [PATCH v5 1/2] submodule refactor: use git_path_submodule() in add_submodule_odb() Max Kirillov @ 2015-08-03 21:03 ` Max Kirillov 2015-08-04 21:51 ` [PATCH v6 0/2] " Max Kirillov 2 siblings, 0 replies; 27+ messages in thread From: Max Kirillov @ 2015-08-03 21:03 UTC (permalink / raw) To: Jens Lehmann, Duy Nguyen; +Cc: Max Kirillov, Junio C Hamano, git This allows making submodules a linked workdirs. Same as for .git, but ignores 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 <max@max630.net> --- cache.h | 1 + path.c | 24 ++++++++++++++++++++---- setup.c | 17 ++++++++++++----- t/t7410-submodule-checkout-to.sh | 10 ++++++++++ 4 files changed, 43 insertions(+), 9 deletions(-) diff --git a/cache.h b/cache.h index 4f55466..b87ec75 100644 --- a/cache.h +++ b/cache.h @@ -442,6 +442,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 10f4cbf..f292d02 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,17 @@ 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 +165,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 +233,8 @@ const char *git_path_submodule(const char *path, const char *fmt, ...) { struct strbuf *buf = get_pathname(); const char *git_dir; + struct strbuf git_submodule_common_dir = STRBUF_INIT; + struct strbuf git_submodule_dir = STRBUF_INIT; va_list args; strbuf_addstr(buf, path); @@ -241,11 +248,20 @@ const char *git_path_submodule(const char *path, const char *fmt, ...) strbuf_addstr(buf, git_dir); } strbuf_addch(buf, '/'); + strbuf_addstr(&git_submodule_dir, buf->buf); va_start(args, fmt); strbuf_vaddf(buf, fmt, args); va_end(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); return buf->buf; } diff --git a/setup.c b/setup.c index 82c0cc2..39ea06b 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 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v6 0/2] path: implement common_dir handling in git_path_submodule() 2015-08-03 21:03 ` [PATCH v5 0/2] " Max Kirillov 2015-08-03 21:03 ` [PATCH v5 1/2] submodule refactor: use git_path_submodule() in add_submodule_odb() Max Kirillov 2015-08-03 21:03 ` [PATCH v5 2/2] path: implement common_dir handling in git_path_submodule() Max Kirillov @ 2015-08-04 21:51 ` Max Kirillov 2015-08-04 21:51 ` [PATCH v6 1/2] submodule refactor: use git_path_submodule() in add_submodule_odb() Max Kirillov ` (2 more replies) 2 siblings, 3 replies; 27+ messages in thread From: Max Kirillov @ 2015-08-04 21:51 UTC (permalink / raw) To: Jens Lehmann, Duy Nguyen; +Cc: Max Kirillov, Junio C Hamano, git Fixed the pointer declaration spacing issue and updated commit messages. The 1/2 refactoring still might leave some cases behind. I could grep only function connect_work_tree_and_git_dir() which, as far as I can see, involved in moving repositories. Have not touched it because correct moving parts of multiple-worktree repositories probably is a bigger task than just fix config file location. Max Kirillov (2): submodule refactor: use git_path_submodule() in add_submodule_odb() path: implement common_dir handling in git_path_submodule() cache.h | 1 + path.c | 24 ++++++++++++++++++++---- setup.c | 17 ++++++++++++----- submodule.c | 28 ++++++++++------------------ t/t7410-submodule-checkout-to.sh | 10 ++++++++++ 5 files changed, 53 insertions(+), 27 deletions(-) -- 2.3.4.2801.g3d0809b ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v6 1/2] submodule refactor: use git_path_submodule() in add_submodule_odb() 2015-08-04 21:51 ` [PATCH v6 0/2] " Max Kirillov @ 2015-08-04 21:51 ` Max Kirillov 2015-08-04 21:51 ` [PATCH v6 2/2] path: implement common_dir handling in git_path_submodule() Max Kirillov 2015-08-04 22:05 ` [PATCH v7 0/2] " Max Kirillov 2 siblings, 0 replies; 27+ messages in thread From: Max Kirillov @ 2015-08-04 21:51 UTC (permalink / raw) To: Jens Lehmann, Duy Nguyen; +Cc: Max Kirillov, Junio C Hamano, git 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 git_path_submodule() function, with intention to modify only that function whenever we need to change real location of submodule's repository content. Signed-off-by: Max Kirillov <max@max630.net> --- submodule.c | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/submodule.c b/submodule.c index 15e90d1..70d18ec 100644 --- a/submodule.c +++ b/submodule.c @@ -122,43 +122,35 @@ void stage_updated_gitmodules(void) static int add_submodule_odb(const char *path) { - struct strbuf objects_directory = STRBUF_INIT; struct alternate_object_database *alt_odb; + const char *objects_directory; 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/"); - if (!is_directory(objects_directory.buf)) { + objects_directory = git_path_submodule(path, "objects/"); + if (!is_directory(objects_directory)) { ret = -1; goto done; } + /* avoid adding it twice */ for (alt_odb = alt_odb_list; alt_odb; alt_odb = alt_odb->next) - if (alt_odb->name - alt_odb->base == objects_directory.len && - !strncmp(alt_odb->base, objects_directory.buf, - objects_directory.len)) + if (alt_odb->name - alt_odb->base == strlen(objects_directory) && + !strcmp(alt_odb->base, objects_directory)) goto done; - alt_odb = xmalloc(objects_directory.len + 42 + sizeof(*alt_odb)); + alt_odb = xmalloc(strlen(objects_directory) + 42 + sizeof(*alt_odb)); alt_odb->next = alt_odb_list; - strcpy(alt_odb->base, objects_directory.buf); - alt_odb->name = alt_odb->base + objects_directory.len; + strcpy(alt_odb->base, objects_directory); + alt_odb->name = alt_odb->base + strlen(objects_directory); alt_odb->name[2] = '/'; alt_odb->name[40] = '\0'; alt_odb->name[41] = '\0'; alt_odb_list = alt_odb; /* add possible alternates from the submodule */ - read_info_alternates(objects_directory.buf, 0); + read_info_alternates(objects_directory, 0); prepare_alt_odb(); done: - strbuf_release(&objects_directory); return ret; } -- 2.3.4.2801.g3d0809b ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v6 2/2] path: implement common_dir handling in git_path_submodule() 2015-08-04 21:51 ` [PATCH v6 0/2] " Max Kirillov 2015-08-04 21:51 ` [PATCH v6 1/2] submodule refactor: use git_path_submodule() in add_submodule_odb() Max Kirillov @ 2015-08-04 21:51 ` Max Kirillov 2015-08-04 22:05 ` [PATCH v7 0/2] " Max Kirillov 2 siblings, 0 replies; 27+ messages in thread From: Max Kirillov @ 2015-08-04 21:51 UTC (permalink / raw) To: Jens Lehmann, Duy Nguyen; +Cc: Max Kirillov, Junio C Hamano, git 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_path_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 <max@max630.net> --- cache.h | 1 + path.c | 24 ++++++++++++++++++++---- setup.c | 17 ++++++++++++----- t/t7410-submodule-checkout-to.sh | 10 ++++++++++ 4 files changed, 43 insertions(+), 9 deletions(-) diff --git a/cache.h b/cache.h index 4f55466..b87ec75 100644 --- a/cache.h +++ b/cache.h @@ -442,6 +442,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 10f4cbf..b0cf444 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,17 @@ 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 +165,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 +233,8 @@ const char *git_path_submodule(const char *path, const char *fmt, ...) { struct strbuf *buf = get_pathname(); const char *git_dir; + struct strbuf git_submodule_common_dir = STRBUF_INIT; + struct strbuf git_submodule_dir = STRBUF_INIT; va_list args; strbuf_addstr(buf, path); @@ -241,11 +248,20 @@ const char *git_path_submodule(const char *path, const char *fmt, ...) strbuf_addstr(buf, git_dir); } strbuf_addch(buf, '/'); + strbuf_addstr(&git_submodule_dir, buf->buf); va_start(args, fmt); strbuf_vaddf(buf, fmt, args); va_end(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); return buf->buf; } diff --git a/setup.c b/setup.c index 82c0cc2..39ea06b 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 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v7 0/2] path: implement common_dir handling in git_path_submodule() 2015-08-04 21:51 ` [PATCH v6 0/2] " Max Kirillov 2015-08-04 21:51 ` [PATCH v6 1/2] submodule refactor: use git_path_submodule() in add_submodule_odb() Max Kirillov 2015-08-04 21:51 ` [PATCH v6 2/2] path: implement common_dir handling in git_path_submodule() Max Kirillov @ 2015-08-04 22:05 ` Max Kirillov 2015-08-04 22:05 ` [PATCH v7 1/2] submodule refactor: use git_path_submodule() in add_submodule_odb() Max Kirillov ` (2 more replies) 2 siblings, 3 replies; 27+ messages in thread From: Max Kirillov @ 2015-08-04 22:05 UTC (permalink / raw) To: Jens Lehmann, Duy Nguyen; +Cc: Max Kirillov, Junio C Hamano, git Emphasized that 1/2 does not change behavior. Max Kirillov (2): submodule refactor: use git_path_submodule() in add_submodule_odb() path: implement common_dir handling in git_path_submodule() cache.h | 1 + path.c | 24 ++++++++++++++++++++---- setup.c | 17 ++++++++++++----- submodule.c | 28 ++++++++++------------------ t/t7410-submodule-checkout-to.sh | 10 ++++++++++ 5 files changed, 53 insertions(+), 27 deletions(-) -- 2.3.4.2801.g3d0809b ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v7 1/2] submodule refactor: use git_path_submodule() in add_submodule_odb() 2015-08-04 22:05 ` [PATCH v7 0/2] " Max Kirillov @ 2015-08-04 22:05 ` Max Kirillov 2015-08-18 0:07 ` Stefan Beller 2015-08-04 22:05 ` [PATCH v7 2/2] path: implement common_dir handling in git_path_submodule() Max Kirillov 2015-09-10 21:57 ` [PATCH v8 0/2] Submodule object path Max Kirillov 2 siblings, 1 reply; 27+ messages in thread From: Max Kirillov @ 2015-08-04 22:05 UTC (permalink / raw) To: Jens Lehmann, Duy Nguyen; +Cc: Max Kirillov, Junio C Hamano, git 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 git_path_submodule() function without changing overall behavior. Then it will be possible to modify only that function whenever we need to change real location of submodule's repository content. Signed-off-by: Max Kirillov <max@max630.net> --- submodule.c | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/submodule.c b/submodule.c index 15e90d1..70d18ec 100644 --- a/submodule.c +++ b/submodule.c @@ -122,43 +122,35 @@ void stage_updated_gitmodules(void) static int add_submodule_odb(const char *path) { - struct strbuf objects_directory = STRBUF_INIT; struct alternate_object_database *alt_odb; + const char *objects_directory; 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/"); - if (!is_directory(objects_directory.buf)) { + objects_directory = git_path_submodule(path, "objects/"); + if (!is_directory(objects_directory)) { ret = -1; goto done; } + /* avoid adding it twice */ for (alt_odb = alt_odb_list; alt_odb; alt_odb = alt_odb->next) - if (alt_odb->name - alt_odb->base == objects_directory.len && - !strncmp(alt_odb->base, objects_directory.buf, - objects_directory.len)) + if (alt_odb->name - alt_odb->base == strlen(objects_directory) && + !strcmp(alt_odb->base, objects_directory)) goto done; - alt_odb = xmalloc(objects_directory.len + 42 + sizeof(*alt_odb)); + alt_odb = xmalloc(strlen(objects_directory) + 42 + sizeof(*alt_odb)); alt_odb->next = alt_odb_list; - strcpy(alt_odb->base, objects_directory.buf); - alt_odb->name = alt_odb->base + objects_directory.len; + strcpy(alt_odb->base, objects_directory); + alt_odb->name = alt_odb->base + strlen(objects_directory); alt_odb->name[2] = '/'; alt_odb->name[40] = '\0'; alt_odb->name[41] = '\0'; alt_odb_list = alt_odb; /* add possible alternates from the submodule */ - read_info_alternates(objects_directory.buf, 0); + read_info_alternates(objects_directory, 0); prepare_alt_odb(); done: - strbuf_release(&objects_directory); return ret; } -- 2.3.4.2801.g3d0809b ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v7 1/2] submodule refactor: use git_path_submodule() in add_submodule_odb() 2015-08-04 22:05 ` [PATCH v7 1/2] submodule refactor: use git_path_submodule() in add_submodule_odb() Max Kirillov @ 2015-08-18 0:07 ` Stefan Beller 0 siblings, 0 replies; 27+ messages in thread From: Stefan Beller @ 2015-08-18 0:07 UTC (permalink / raw) To: Max Kirillov Cc: Jens Lehmann, Duy Nguyen, Junio C Hamano, git@vger.kernel.org On Tue, Aug 4, 2015 at 3:05 PM, Max Kirillov <max@max630.net> wrote: > 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 > git_path_submodule() function without changing overall behavior. Then it will > be possible to modify only that function whenever we need to change real > location of submodule's repository content. > > Signed-off-by: Max Kirillov <max@max630.net> > --- > submodule.c | 28 ++++++++++------------------ > 1 file changed, 10 insertions(+), 18 deletions(-) > > diff --git a/submodule.c b/submodule.c > index 15e90d1..70d18ec 100644 > --- a/submodule.c > +++ b/submodule.c > @@ -122,43 +122,35 @@ void stage_updated_gitmodules(void) > > static int add_submodule_odb(const char *path) > { > - struct strbuf objects_directory = STRBUF_INIT; > struct alternate_object_database *alt_odb; > + const char *objects_directory; > 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/"); > - if (!is_directory(objects_directory.buf)) { > + objects_directory = git_path_submodule(path, "objects/"); git_path_submodule is going away with jk/git-path (2015-08-10) So you may want to use git_pathdup_submodule and free the result of that function before returning. > + if (!is_directory(objects_directory)) { > ret = -1; > goto done; > } > + > /* avoid adding it twice */ > for (alt_odb = alt_odb_list; alt_odb; alt_odb = alt_odb->next) > - if (alt_odb->name - alt_odb->base == objects_directory.len && > - !strncmp(alt_odb->base, objects_directory.buf, > - objects_directory.len)) > + if (alt_odb->name - alt_odb->base == strlen(objects_directory) && > + !strcmp(alt_odb->base, objects_directory)) > goto done; > > - alt_odb = xmalloc(objects_directory.len + 42 + sizeof(*alt_odb)); > + alt_odb = xmalloc(strlen(objects_directory) + 42 + sizeof(*alt_odb)); > alt_odb->next = alt_odb_list; > - strcpy(alt_odb->base, objects_directory.buf); > - alt_odb->name = alt_odb->base + objects_directory.len; > + strcpy(alt_odb->base, objects_directory); > + alt_odb->name = alt_odb->base + strlen(objects_directory); > alt_odb->name[2] = '/'; > alt_odb->name[40] = '\0'; > alt_odb->name[41] = '\0'; > alt_odb_list = alt_odb; > > /* add possible alternates from the submodule */ > - read_info_alternates(objects_directory.buf, 0); > + read_info_alternates(objects_directory, 0); > prepare_alt_odb(); > done: > - strbuf_release(&objects_directory); > return ret; > } > This looks good, though I am diving into the submodule code base myself just now, so it's not a strong review. :) > -- > 2.3.4.2801.g3d0809b > > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v7 2/2] path: implement common_dir handling in git_path_submodule() 2015-08-04 22:05 ` [PATCH v7 0/2] " Max Kirillov 2015-08-04 22:05 ` [PATCH v7 1/2] submodule refactor: use git_path_submodule() in add_submodule_odb() Max Kirillov @ 2015-08-04 22:05 ` Max Kirillov 2015-08-17 23:56 ` Stefan Beller 2015-09-10 21:57 ` [PATCH v8 0/2] Submodule object path Max Kirillov 2 siblings, 1 reply; 27+ messages in thread From: Max Kirillov @ 2015-08-04 22:05 UTC (permalink / raw) To: Jens Lehmann, Duy Nguyen; +Cc: Max Kirillov, Junio C Hamano, git 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_path_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 <max@max630.net> --- cache.h | 1 + path.c | 24 ++++++++++++++++++++---- setup.c | 17 ++++++++++++----- t/t7410-submodule-checkout-to.sh | 10 ++++++++++ 4 files changed, 43 insertions(+), 9 deletions(-) diff --git a/cache.h b/cache.h index 4f55466..b87ec75 100644 --- a/cache.h +++ b/cache.h @@ -442,6 +442,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 10f4cbf..b0cf444 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,17 @@ 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 +165,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 +233,8 @@ const char *git_path_submodule(const char *path, const char *fmt, ...) { struct strbuf *buf = get_pathname(); const char *git_dir; + struct strbuf git_submodule_common_dir = STRBUF_INIT; + struct strbuf git_submodule_dir = STRBUF_INIT; va_list args; strbuf_addstr(buf, path); @@ -241,11 +248,20 @@ const char *git_path_submodule(const char *path, const char *fmt, ...) strbuf_addstr(buf, git_dir); } strbuf_addch(buf, '/'); + strbuf_addstr(&git_submodule_dir, buf->buf); va_start(args, fmt); strbuf_vaddf(buf, fmt, args); va_end(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); return buf->buf; } diff --git a/setup.c b/setup.c index 82c0cc2..39ea06b 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 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v7 2/2] path: implement common_dir handling in git_path_submodule() 2015-08-04 22:05 ` [PATCH v7 2/2] path: implement common_dir handling in git_path_submodule() Max Kirillov @ 2015-08-17 23:56 ` Stefan Beller 0 siblings, 0 replies; 27+ messages in thread From: Stefan Beller @ 2015-08-17 23:56 UTC (permalink / raw) To: Max Kirillov Cc: Jens Lehmann, Duy Nguyen, Junio C Hamano, git@vger.kernel.org On Tue, Aug 4, 2015 at 3:05 PM, Max Kirillov <max@max630.net> wrote: > 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_path_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 <max@max630.net> > --- > cache.h | 1 + > path.c | 24 ++++++++++++++++++++---- > setup.c | 17 ++++++++++++----- > t/t7410-submodule-checkout-to.sh | 10 ++++++++++ > 4 files changed, 43 insertions(+), 9 deletions(-) > > diff --git a/cache.h b/cache.h > index 4f55466..b87ec75 100644 > --- a/cache.h > +++ b/cache.h > @@ -442,6 +442,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 10f4cbf..b0cf444 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,17 @@ 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(); > + } Usually we omit the braces for blocks with just one command. > + > 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 +165,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 +233,8 @@ const char *git_path_submodule(const char *path, const char *fmt, ...) > { > struct strbuf *buf = get_pathname(); > const char *git_dir; > + struct strbuf git_submodule_common_dir = STRBUF_INIT; > + struct strbuf git_submodule_dir = STRBUF_INIT; > va_list args; > > strbuf_addstr(buf, path); > @@ -241,11 +248,20 @@ const char *git_path_submodule(const char *path, const char *fmt, ...) > strbuf_addstr(buf, git_dir); > } > strbuf_addch(buf, '/'); > + strbuf_addstr(&git_submodule_dir, buf->buf); > > va_start(args, fmt); > strbuf_vaddf(buf, fmt, args); > va_end(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); > return buf->buf; > } > > diff --git a/setup.c b/setup.c > index 82c0cc2..39ea06b 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 Looks good to me otherwise. > > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v8 0/2] Submodule object path 2015-08-04 22:05 ` [PATCH v7 0/2] " Max Kirillov 2015-08-04 22:05 ` [PATCH v7 1/2] submodule refactor: use git_path_submodule() in add_submodule_odb() Max Kirillov 2015-08-04 22:05 ` [PATCH v7 2/2] path: implement common_dir handling in git_path_submodule() Max Kirillov @ 2015-09-10 21:57 ` Max Kirillov 2015-09-10 21:57 ` [PATCH v8 1/2] submodule refactor: use git_pathdup_submodule() in add_submodule_odb() Max Kirillov ` (3 more replies) 2 siblings, 4 replies; 27+ messages in thread From: Max Kirillov @ 2015-09-10 21:57 UTC (permalink / raw) To: Jens Lehmann, Duy Nguyen, Jeff King, Johannes Schindelin, Heiko Voigt, Stefan Beller Cc: Max Kirillov, Junio C Hamano, git * Rebased to recent master (v2.6.0-rc0). * Use git_pathdup_submodule() instead of git_path_submodule() * There are more conflicts in pu with [1], not sure what should I do about it. * Style fixes as Stefan suggested [1] http://thread.gmane.org/gmane.comp.version-control.git/276628 Max Kirillov (2): submodule refactor: use git_pathdup_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 | 30 ++++++++++++------------------ t/t7410-submodule-checkout-to.sh | 10 ++++++++++ 5 files changed, 53 insertions(+), 27 deletions(-) -- 2.3.4.2801.g3d0809b ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v8 1/2] submodule refactor: use git_pathdup_submodule() in add_submodule_odb() 2015-09-10 21:57 ` [PATCH v8 0/2] Submodule object path Max Kirillov @ 2015-09-10 21:57 ` Max Kirillov 2015-09-11 7:53 ` Jeff King 2015-09-10 21:57 ` [PATCH v8 2/2] path: implement common_dir handling in git_pathdup_submodule() Max Kirillov ` (2 subsequent siblings) 3 siblings, 1 reply; 27+ messages in thread From: Max Kirillov @ 2015-09-10 21:57 UTC (permalink / raw) To: Jens Lehmann, Duy Nguyen, Jeff King, Johannes Schindelin, Heiko Voigt, Stefan Beller Cc: Max Kirillov, Junio C Hamano, git 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 git_pathdup_submodule() function without changing overall behavior. Then it will be possible to modify only that function whenever we need to change real location of submodule's repository content. Signed-off-by: Max Kirillov <max@max630.net> --- submodule.c | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/submodule.c b/submodule.c index 245ed4d..7340069 100644 --- a/submodule.c +++ b/submodule.c @@ -119,43 +119,37 @@ void stage_updated_gitmodules(void) static int add_submodule_odb(const char *path) { - struct strbuf objects_directory = STRBUF_INIT; struct alternate_object_database *alt_odb; + char *objects_directory; 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/"); - if (!is_directory(objects_directory.buf)) { + objects_directory = git_pathdup_submodule(path, "objects/"); + if (!is_directory(objects_directory)) { ret = -1; goto done; } + /* avoid adding it twice */ for (alt_odb = alt_odb_list; alt_odb; alt_odb = alt_odb->next) - if (alt_odb->name - alt_odb->base == objects_directory.len && - !strncmp(alt_odb->base, objects_directory.buf, - objects_directory.len)) + if (alt_odb->name - alt_odb->base == strlen(objects_directory) && + !strcmp(alt_odb->base, objects_directory)) goto done; - alt_odb = xmalloc(objects_directory.len + 42 + sizeof(*alt_odb)); + alt_odb = xmalloc(strlen(objects_directory) + 42 + sizeof(*alt_odb)); alt_odb->next = alt_odb_list; - strcpy(alt_odb->base, objects_directory.buf); - alt_odb->name = alt_odb->base + objects_directory.len; + strcpy(alt_odb->base, objects_directory); + alt_odb->name = alt_odb->base + strlen(objects_directory); alt_odb->name[2] = '/'; alt_odb->name[40] = '\0'; alt_odb->name[41] = '\0'; alt_odb_list = alt_odb; /* add possible alternates from the submodule */ - read_info_alternates(objects_directory.buf, 0); + read_info_alternates(objects_directory, 0); prepare_alt_odb(); done: - strbuf_release(&objects_directory); + free(objects_directory); + return ret; } -- 2.3.4.2801.g3d0809b ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v8 1/2] submodule refactor: use git_pathdup_submodule() in add_submodule_odb() 2015-09-10 21:57 ` [PATCH v8 1/2] submodule refactor: use git_pathdup_submodule() in add_submodule_odb() Max Kirillov @ 2015-09-11 7:53 ` Jeff King 2015-09-13 22:26 ` Max Kirillov 0 siblings, 1 reply; 27+ messages in thread From: Jeff King @ 2015-09-11 7:53 UTC (permalink / raw) To: Max Kirillov Cc: Jens Lehmann, Duy Nguyen, Johannes Schindelin, Heiko Voigt, Stefan Beller, Junio C Hamano, git On Fri, Sep 11, 2015 at 12:57:10AM +0300, Max Kirillov wrote: > diff --git a/submodule.c b/submodule.c > index 245ed4d..7340069 100644 > --- a/submodule.c > +++ b/submodule.c > @@ -119,43 +119,37 @@ void stage_updated_gitmodules(void) > > static int add_submodule_odb(const char *path) > { > - struct strbuf objects_directory = STRBUF_INIT; > struct alternate_object_database *alt_odb; > + char *objects_directory; Now that we have strbuf_git_path_submodule(), is there any reason to switch this away from a strbuf? That saves us a bunch of strlen calls, and it makes the diff way shorter. My ulterior motive is that the result also conflicts a lot less with some patches I'm about to post to harden the malloc and strcpy calls below. That would make your patch look something like: diff --git a/submodule.c b/submodule.c index 245ed4d..5e5a46f 100644 --- a/submodule.c +++ b/submodule.c @@ -122,15 +122,8 @@ static int add_submodule_odb(const char *path) struct strbuf objects_directory = STRBUF_INIT; struct alternate_object_database *alt_odb; 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; -Peff ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v8 1/2] submodule refactor: use git_pathdup_submodule() in add_submodule_odb() 2015-09-11 7:53 ` Jeff King @ 2015-09-13 22:26 ` Max Kirillov 0 siblings, 0 replies; 27+ messages in thread From: Max Kirillov @ 2015-09-13 22:26 UTC (permalink / raw) To: Jeff King Cc: Max Kirillov, Jens Lehmann, Duy Nguyen, Johannes Schindelin, Heiko Voigt, Stefan Beller, Junio C Hamano, git On Fri, Sep 11, 2015 at 03:53:40AM -0400, Jeff King wrote: > > Now that we have strbuf_git_path_submodule(), is there any reason to > switch this away from a strbuf? > > That saves us a bunch of strlen calls, and it makes the diff way > shorter. My ulterior motive is that the result also conflicts a lot less > with some patches I'm about to post to harden the malloc and strcpy > calls below. > > That would make your patch look something like: > > diff --git a/submodule.c b/submodule.c > index 245ed4d..5e5a46f 100644 > --- a/submodule.c > +++ b/submodule.c > @@ -122,15 +122,8 @@ static int add_submodule_odb(const char *path) > struct strbuf objects_directory = STRBUF_INIT; > struct alternate_object_database *alt_odb; > 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; > > -Peff Thank you. This is how it looks now. -- Max ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v8 2/2] path: implement common_dir handling in git_pathdup_submodule() 2015-09-10 21:57 ` [PATCH v8 0/2] Submodule object path Max Kirillov 2015-09-10 21:57 ` [PATCH v8 1/2] submodule refactor: use git_pathdup_submodule() in add_submodule_odb() Max Kirillov @ 2015-09-10 21:57 ` Max Kirillov 2015-09-11 1:10 ` [PATCH v8 0/2] Submodule object path Junio C Hamano 2015-09-13 22:17 ` [PATCH v9 " Max Kirillov 3 siblings, 0 replies; 27+ messages in thread From: Max Kirillov @ 2015-09-10 21:57 UTC (permalink / raw) To: Jens Lehmann, Duy Nguyen, Jeff King, Johannes Schindelin, Heiko Voigt, Stefan Beller Cc: Max Kirillov, Junio C Hamano, git 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 <max@max630.net> --- 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 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v8 0/2] Submodule object path 2015-09-10 21:57 ` [PATCH v8 0/2] Submodule object path Max Kirillov 2015-09-10 21:57 ` [PATCH v8 1/2] submodule refactor: use git_pathdup_submodule() in add_submodule_odb() Max Kirillov 2015-09-10 21:57 ` [PATCH v8 2/2] path: implement common_dir handling in git_pathdup_submodule() Max Kirillov @ 2015-09-11 1:10 ` Junio C Hamano 2015-09-13 22:32 ` Max Kirillov 2015-09-13 22:17 ` [PATCH v9 " Max Kirillov 3 siblings, 1 reply; 27+ messages in thread From: Junio C Hamano @ 2015-09-11 1:10 UTC (permalink / raw) To: Max Kirillov Cc: Jens Lehmann, Duy Nguyen, Jeff King, Johannes Schindelin, Heiko Voigt, Stefan Beller, git Max Kirillov <max@max630.net> writes: > * There are more conflicts in pu with [1], not sure > what should I do about it. > > [1] http://thread.gmane.org/gmane.comp.version-control.git/276628 I do not think conflicts got any worse than before. Between the result of merging this round and the previous one to 'pu', the only difference in path.c that conflicts with [1] I see is essentially a no-op (the diff attached at the end of this essage). When I push the updated 'pu' out, could you please check $ git log --oneline --first-parent origin/master..origin/pu to find the place your topic is merged (I am planning to make it "3af7cd6 Merge branch 'mk/submodule-gitdir-path' into pu", but please do not rely on the exact commit object name; I may have to redo the integration if I find some breakages caused by this or any other topic), and then eyeballing (call that merge $m) output of $ git diff $m^ $m to see if there is anything I screwed up? Thanks. diff --git a/path.c b/path.c index 776d8f0..2725bc3 100644 --- a/path.c +++ b/path.c @@ -466,11 +466,11 @@ static void do_submodule_path(struct strbuf *buf, const char *path, strbuf_vaddf(buf, fmt, args); - if (get_common_dir_noenv(&git_submodule_common_dir, git_submodule_dir.buf)) { + 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); } ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v8 0/2] Submodule object path 2015-09-11 1:10 ` [PATCH v8 0/2] Submodule object path Junio C Hamano @ 2015-09-13 22:32 ` Max Kirillov 2015-09-14 18:14 ` Junio C Hamano 0 siblings, 1 reply; 27+ messages in thread From: Max Kirillov @ 2015-09-13 22:32 UTC (permalink / raw) To: Junio C Hamano Cc: Max Kirillov, Jens Lehmann, Duy Nguyen, Jeff King, Johannes Schindelin, Heiko Voigt, Stefan Beller, git On Thu, Sep 10, 2015 at 06:10:13PM -0700, Junio C Hamano wrote: > When I push the updated 'pu' out, could you please check I follow the pu merges. So far resolutions seem correct and all tests pass. If you don't mind re-resolving it as I send newer versions I will base them on master. Thank you. -- Max ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v8 0/2] Submodule object path 2015-09-13 22:32 ` Max Kirillov @ 2015-09-14 18:14 ` Junio C Hamano 0 siblings, 0 replies; 27+ messages in thread From: Junio C Hamano @ 2015-09-14 18:14 UTC (permalink / raw) To: Max Kirillov Cc: Jens Lehmann, Duy Nguyen, Jeff King, Johannes Schindelin, Heiko Voigt, Stefan Beller, git Max Kirillov <max@max630.net> writes: > On Thu, Sep 10, 2015 at 06:10:13PM -0700, Junio C Hamano wrote: >> When I push the updated 'pu' out, could you please check > > I follow the pu merges. So far resolutions seem correct and > all tests pass. The primary thing I ask from "eyeballing by the original author" is to see if "git diff M^ M" for the merge M of the topic is something the original author would have done if he was given a starting point of M^ to build his or her topic (which is what your "resolutions seem correct" really is), so we are good. > If you don't mind re-resolving it as I send newer versions I will > base them on master. So far rerere seems to be happy, so basing on the same commit is fine, but I suspect this round should already be ready for 'next'. I had to remove the now-unused variable "const char *git_dir;" in [PATCH 1/2], but other than that it looked OK from a cursory re-read. Thanks. ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v9 0/2] Submodule object path 2015-09-10 21:57 ` [PATCH v8 0/2] Submodule object path Max Kirillov ` (2 preceding siblings ...) 2015-09-11 1:10 ` [PATCH v8 0/2] Submodule object path Junio C Hamano @ 2015-09-13 22:17 ` Max Kirillov 2015-09-13 22:17 ` [PATCH v9 1/2] submodule refactor: use strbuf_git_path_submodule() in add_submodule_odb() Max Kirillov 2015-09-13 22:17 ` [PATCH v9 2/2] path: implement common_dir handling in git_pathdup_submodule() Max Kirillov 3 siblings, 2 replies; 27+ messages in thread From: Max Kirillov @ 2015-09-13 22:17 UTC (permalink / raw) To: Jens Lehmann, Duy Nguyen, Jeff King, Johannes Schindelin, Heiko Voigt, Stefan Beller Cc: Max Kirillov, Junio C Hamano, git 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v9 1/2] submodule refactor: use strbuf_git_path_submodule() in add_submodule_odb() 2015-09-13 22:17 ` [PATCH v9 " Max Kirillov @ 2015-09-13 22:17 ` Max Kirillov 2015-09-14 18:00 ` Junio C Hamano 2015-09-13 22:17 ` [PATCH v9 2/2] path: implement common_dir handling in git_pathdup_submodule() Max Kirillov 1 sibling, 1 reply; 27+ messages in thread From: Max Kirillov @ 2015-09-13 22:17 UTC (permalink / raw) To: Jens Lehmann, Duy Nguyen, Jeff King, Johannes Schindelin, Heiko Voigt, Stefan Beller Cc: Max Kirillov, Junio C Hamano, git 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 <peff@peff.net> Signed-off-by: Max Kirillov <max@max630.net> --- 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 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v9 1/2] submodule refactor: use strbuf_git_path_submodule() in add_submodule_odb() 2015-09-13 22:17 ` [PATCH v9 1/2] submodule refactor: use strbuf_git_path_submodule() in add_submodule_odb() Max Kirillov @ 2015-09-14 18:00 ` Junio C Hamano 0 siblings, 0 replies; 27+ messages in thread From: Junio C Hamano @ 2015-09-14 18:00 UTC (permalink / raw) To: Max Kirillov Cc: Jens Lehmann, Duy Nguyen, Jeff King, Johannes Schindelin, Heiko Voigt, Stefan Beller, git Max Kirillov <max@max630.net> 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 <peff@peff.net> > Signed-off-by: Max Kirillov <max@max630.net> > --- > 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; ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v9 2/2] path: implement common_dir handling in git_pathdup_submodule() 2015-09-13 22:17 ` [PATCH v9 " Max Kirillov 2015-09-13 22:17 ` [PATCH v9 1/2] submodule refactor: use strbuf_git_path_submodule() in add_submodule_odb() Max Kirillov @ 2015-09-13 22:17 ` Max Kirillov 1 sibling, 0 replies; 27+ messages in thread From: Max Kirillov @ 2015-09-13 22:17 UTC (permalink / raw) To: Jens Lehmann, Duy Nguyen, Jeff King, Johannes Schindelin, Heiko Voigt, Stefan Beller Cc: Max Kirillov, Junio C Hamano, git 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 <max@max630.net> --- 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 ^ permalink raw reply related [flat|nested] 27+ messages in thread
end of thread, other threads:[~2015-09-14 18:14 UTC | newest] Thread overview: 27+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-03-18 21:10 [PATCH v4 0/2] path: implement common_dir handling in git_path_submodule() Max Kirillov 2015-03-18 21:10 ` [PATCH v4 1/2] submodule refactor: use git_path_submodule() in add_submodule_odb() Max Kirillov 2015-03-18 21:10 ` [PATCH v4 2/2] path: implement common_dir handling in git_path_submodule() Max Kirillov 2015-08-03 21:03 ` [PATCH v5 0/2] " Max Kirillov 2015-08-03 21:03 ` [PATCH v5 1/2] submodule refactor: use git_path_submodule() in add_submodule_odb() Max Kirillov 2015-08-03 21:29 ` Junio C Hamano 2015-08-03 21:03 ` [PATCH v5 2/2] path: implement common_dir handling in git_path_submodule() Max Kirillov 2015-08-04 21:51 ` [PATCH v6 0/2] " Max Kirillov 2015-08-04 21:51 ` [PATCH v6 1/2] submodule refactor: use git_path_submodule() in add_submodule_odb() Max Kirillov 2015-08-04 21:51 ` [PATCH v6 2/2] path: implement common_dir handling in git_path_submodule() Max Kirillov 2015-08-04 22:05 ` [PATCH v7 0/2] " Max Kirillov 2015-08-04 22:05 ` [PATCH v7 1/2] submodule refactor: use git_path_submodule() in add_submodule_odb() Max Kirillov 2015-08-18 0:07 ` Stefan Beller 2015-08-04 22:05 ` [PATCH v7 2/2] path: implement common_dir handling in git_path_submodule() Max Kirillov 2015-08-17 23:56 ` Stefan Beller 2015-09-10 21:57 ` [PATCH v8 0/2] Submodule object path Max Kirillov 2015-09-10 21:57 ` [PATCH v8 1/2] submodule refactor: use git_pathdup_submodule() in add_submodule_odb() Max Kirillov 2015-09-11 7:53 ` Jeff King 2015-09-13 22:26 ` Max Kirillov 2015-09-10 21:57 ` [PATCH v8 2/2] path: implement common_dir handling in git_pathdup_submodule() Max Kirillov 2015-09-11 1:10 ` [PATCH v8 0/2] Submodule object path Junio C Hamano 2015-09-13 22:32 ` Max Kirillov 2015-09-14 18:14 ` Junio C Hamano 2015-09-13 22:17 ` [PATCH v9 " Max Kirillov 2015-09-13 22:17 ` [PATCH v9 1/2] submodule refactor: use strbuf_git_path_submodule() in add_submodule_odb() Max Kirillov 2015-09-14 18:00 ` Junio C Hamano 2015-09-13 22:17 ` [PATCH v9 2/2] path: implement common_dir handling in git_pathdup_submodule() Max Kirillov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).