* t1503 broken ? @ 2017-03-25 10:46 Torsten Bögershausen 2017-03-25 11:46 ` Duy Nguyen 0 siblings, 1 reply; 11+ messages in thread From: Torsten Bögershausen @ 2017-03-25 10:46 UTC (permalink / raw) To: Git Mailing List, Nguyen Thai Ngoc Duy ./t1305-config-include.sh seems to be broken: not ok 19 - conditional include, $HOME expansion not ok 21 - conditional include, relative path Both Mac and Linux. The problem seems to be the line git config test.two >actual && and git config test.four >actual && In both cases the config "test.two or test.four" is not found, at least that is what my debugging shows. After adding an exit 0 before the test_done gives this: find trash\ directory.t1305-config-include/ -name "bar*" trash directory.t1305-config-include/foo/.git/bar trash directory.t1305-config-include/foo/.git/bar3 trash directory.t1305-config-include/foo/.git/bar5 trash directory.t1305-config-include/foo/.git/bar2 trash directory.t1305-config-include/bar4 Where should bar2 and bar4 be ? ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: t1503 broken ? 2017-03-25 10:46 t1503 broken ? Torsten Bögershausen @ 2017-03-25 11:46 ` Duy Nguyen 2017-03-25 12:26 ` Duy Nguyen 0 siblings, 1 reply; 11+ messages in thread From: Duy Nguyen @ 2017-03-25 11:46 UTC (permalink / raw) To: Torsten Bögershausen; +Cc: Git Mailing List On Sat, Mar 25, 2017 at 5:46 PM, Torsten Bögershausen <tboegi@web.de> wrote: > ./t1305-config-include.sh > seems to be broken: > not ok 19 - conditional include, $HOME expansion > not ok 21 - conditional include, relative path let me guess, your "git" directory is in a symlink path? -- Duy ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: t1503 broken ? 2017-03-25 11:46 ` Duy Nguyen @ 2017-03-25 12:26 ` Duy Nguyen 2017-03-25 13:05 ` Duy Nguyen 0 siblings, 1 reply; 11+ messages in thread From: Duy Nguyen @ 2017-03-25 12:26 UTC (permalink / raw) To: Torsten Bögershausen; +Cc: Git Mailing List On Sat, Mar 25, 2017 at 6:46 PM, Duy Nguyen <pclouds@gmail.com> wrote: > On Sat, Mar 25, 2017 at 5:46 PM, Torsten Bögershausen <tboegi@web.de> wrote: >> ./t1305-config-include.sh >> seems to be broken: >> not ok 19 - conditional include, $HOME expansion >> not ok 21 - conditional include, relative path > > let me guess, your "git" directory is in a symlink path? Yes I could reproduce it when I put my "git" in a symlink. There's a note in document about "Symlinks in `$GIT_DIR` are not resolved before matching" but failing tests is not acceptable. I'll fix it. -- Duy ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: t1503 broken ? 2017-03-25 12:26 ` Duy Nguyen @ 2017-03-25 13:05 ` Duy Nguyen 2017-03-25 19:41 ` Torsten Bögershausen 2017-03-30 11:37 ` [PATCH 1/2] path.c: and an option to call real_path() in expand_user_path() Nguyễn Thái Ngọc Duy 0 siblings, 2 replies; 11+ messages in thread From: Duy Nguyen @ 2017-03-25 13:05 UTC (permalink / raw) To: Torsten Bögershausen; +Cc: Git Mailing List On Sat, Mar 25, 2017 at 07:26:14PM +0700, Duy Nguyen wrote: > On Sat, Mar 25, 2017 at 6:46 PM, Duy Nguyen <pclouds@gmail.com> wrote: > > On Sat, Mar 25, 2017 at 5:46 PM, Torsten Bögershausen <tboegi@web.de> wrote: > >> ./t1305-config-include.sh > >> seems to be broken: > >> not ok 19 - conditional include, $HOME expansion > >> not ok 21 - conditional include, relative path > > > > let me guess, your "git" directory is in a symlink path? > > Yes I could reproduce it when I put my "git" in a symlink. There's a > note in document about "Symlinks in `$GIT_DIR` are not resolved before > matching" but failing tests is not acceptable. I'll fix it. The fix may be something like this. The problem is $GIT_DIR has symlinks resolved, but we don't do the same for other paths in this code. As a result, matching paths fails. I'm a bit concerned about the change in expand_user_path() because I'm not quite sure if it's a completely safe change. But at least could you try the patch and see if it passe the tests on your machine too? -- 8< -- diff --git a/config.c b/config.c index 1a4d855..fc4eae9 100644 --- a/config.c +++ b/config.c @@ -191,7 +191,7 @@ static int prepare_include_condition_pattern(struct strbuf *pat) return error(_("relative config include " "conditionals must come from files")); - strbuf_add_absolute_path(&path, cf->path); + strbuf_realpath(&path, cf->path, 1); slash = find_last_dir_sep(path.buf); if (!slash) die("BUG: how is this possible?"); @@ -213,7 +213,7 @@ static int include_by_gitdir(const char *cond, size_t cond_len, int icase) struct strbuf pattern = STRBUF_INIT; int ret = 0, prefix; - strbuf_add_absolute_path(&text, get_git_dir()); + strbuf_realpath(&text, get_git_dir(), 1); strbuf_add(&pattern, cond, cond_len); prefix = prepare_include_condition_pattern(&pattern); diff --git a/path.c b/path.c index 2224843..18eaac3 100644 --- a/path.c +++ b/path.c @@ -654,7 +654,7 @@ char *expand_user_path(const char *path) const char *home = getenv("HOME"); if (!home) goto return_null; - strbuf_addstr(&user_path, home); + strbuf_addstr(&user_path, real_path(home)); #ifdef GIT_WINDOWS_NATIVE convert_slashes(user_path.buf); #endif -- 8< -- -- Duy ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: t1503 broken ? 2017-03-25 13:05 ` Duy Nguyen @ 2017-03-25 19:41 ` Torsten Bögershausen 2017-03-30 11:37 ` [PATCH 1/2] path.c: and an option to call real_path() in expand_user_path() Nguyễn Thái Ngọc Duy 1 sibling, 0 replies; 11+ messages in thread From: Torsten Bögershausen @ 2017-03-25 19:41 UTC (permalink / raw) To: Duy Nguyen; +Cc: Git Mailing List On 03/25/2017 02:05 PM, Duy Nguyen wrote: > On Sat, Mar 25, 2017 at 07:26:14PM +0700, Duy Nguyen wrote: >> On Sat, Mar 25, 2017 at 6:46 PM, Duy Nguyen <pclouds@gmail.com> wrote: >>> On Sat, Mar 25, 2017 at 5:46 PM, Torsten Bögershausen <tboegi@web.de> wrote: >>>> ./t1305-config-include.sh >>>> seems to be broken: >>>> not ok 19 - conditional include, $HOME expansion >>>> not ok 21 - conditional include, relative path >>> let me guess, your "git" directory is in a symlink path? >> Yes I could reproduce it when I put my "git" in a symlink. There's a >> note in document about "Symlinks in `$GIT_DIR` are not resolved before >> matching" but failing tests is not acceptable. I'll fix it. > The fix may be something like this. The problem is $GIT_DIR has symlinks > resolved, but we don't do the same for other paths in this code. As a > result, matching paths fails. > > I'm a bit concerned about the change in expand_user_path() because I'm > not quite sure if it's a completely safe change. But at least could > you try the patch and see if it passe the tests on your machine too? > > -- 8< -- > diff --git a/config.c b/config.c > index 1a4d855..fc4eae9 100644 > --- a/config.c > +++ b/config.c > @@ -191,7 +191,7 @@ static int prepare_include_condition_pattern(struct strbuf *pat) > return error(_("relative config include " > "conditionals must come from files")); > > - strbuf_add_absolute_path(&path, cf->path); > + strbuf_realpath(&path, cf->path, 1); > slash = find_last_dir_sep(path.buf); > if (!slash) > die("BUG: how is this possible?"); > @@ -213,7 +213,7 @@ static int include_by_gitdir(const char *cond, size_t cond_len, int icase) > struct strbuf pattern = STRBUF_INIT; > int ret = 0, prefix; > > - strbuf_add_absolute_path(&text, get_git_dir()); > + strbuf_realpath(&text, get_git_dir(), 1); > strbuf_add(&pattern, cond, cond_len); > prefix = prepare_include_condition_pattern(&pattern); > > diff --git a/path.c b/path.c > index 2224843..18eaac3 100644 > --- a/path.c > +++ b/path.c > @@ -654,7 +654,7 @@ char *expand_user_path(const char *path) > const char *home = getenv("HOME"); > if (!home) > goto return_null; > - strbuf_addstr(&user_path, home); > + strbuf_addstr(&user_path, real_path(home)); > #ifdef GIT_WINDOWS_NATIVE > convert_slashes(user_path.buf); > #endif > -- 8< -- Thanks for the fast reply. Yes, my path is a softlink - into a directory under NoBackup/ - to make the backup shorter. And I had forgotten about this :-( And yes, your patch fixes it- tested under Linux. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] path.c: and an option to call real_path() in expand_user_path() 2017-03-25 13:05 ` Duy Nguyen 2017-03-25 19:41 ` Torsten Bögershausen @ 2017-03-30 11:37 ` Nguyễn Thái Ngọc Duy 2017-03-30 11:37 ` [PATCH 2/2] config: resolve symlinks in conditional include's patterns Nguyễn Thái Ngọc Duy 2017-04-05 10:24 ` [PATCH v2 1/2] path.c: and an option to call real_path() in expand_user_path() Nguyễn Thái Ngọc Duy 1 sibling, 2 replies; 11+ messages in thread From: Nguyễn Thái Ngọc Duy @ 2017-03-30 11:37 UTC (permalink / raw) To: git; +Cc: tboegi, Junio C Hamano, Nguyễn Thái Ngọc Duy In the next patch we need the ability to expand '~' to real_path($HOME). But we can't do that from outside because '~' is part of a pattern, not a true path. Add an option to expand_user_path() to do so. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> --- builtin/commit.c | 2 +- builtin/config.c | 2 +- cache.h | 2 +- config.c | 8 ++++---- credential-cache.c | 2 +- credential-store.c | 2 +- path.c | 11 ++++++++--- 7 files changed, 17 insertions(+), 12 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index 4e288bc513..ad188fea9e 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1404,7 +1404,7 @@ int cmd_status(int argc, const char **argv, const char *prefix) static const char *implicit_ident_advice(void) { - char *user_config = expand_user_path("~/.gitconfig"); + char *user_config = expand_user_path("~/.gitconfig", 0); char *xdg_config = xdg_config_home("config"); int config_exists = file_exists(user_config) || file_exists(xdg_config); diff --git a/builtin/config.c b/builtin/config.c index 05843a0f96..70bfaaaa1d 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -502,7 +502,7 @@ int cmd_config(int argc, const char **argv, const char *prefix) } if (use_global_config) { - char *user_config = expand_user_path("~/.gitconfig"); + char *user_config = expand_user_path("~/.gitconfig", 0); char *xdg_config = xdg_config_home("config"); if (!user_config) diff --git a/cache.h b/cache.h index 2214d52f61..62e44bfa2f 100644 --- a/cache.h +++ b/cache.h @@ -1146,7 +1146,7 @@ typedef int create_file_fn(const char *path, void *cb); int raceproof_create_file(const char *path, create_file_fn fn, void *cb); int mkdir_in_gitdir(const char *path); -extern char *expand_user_path(const char *path); +extern char *expand_user_path(const char *path, int real_home); const char *enter_repo(const char *path, int strict); static inline int is_absolute_path(const char *path) { diff --git a/config.c b/config.c index 1a4d85537b..f036c721e6 100644 --- a/config.c +++ b/config.c @@ -135,7 +135,7 @@ static int handle_path_include(const char *path, struct config_include_data *inc if (!path) return config_error_nonbool("include.path"); - expanded = expand_user_path(path); + expanded = expand_user_path(path, 0); if (!expanded) return error("could not expand include path '%s'", path); path = expanded; @@ -177,7 +177,7 @@ static int prepare_include_condition_pattern(struct strbuf *pat) char *expanded; int prefix = 0; - expanded = expand_user_path(pat->buf); + expanded = expand_user_path(pat->buf, 0); if (expanded) { strbuf_reset(pat); strbuf_addstr(pat, expanded); @@ -948,7 +948,7 @@ int git_config_pathname(const char **dest, const char *var, const char *value) { if (!value) return config_error_nonbool(var); - *dest = expand_user_path(value); + *dest = expand_user_path(value, 0); if (!*dest) die(_("failed to expand user dir in: '%s'"), value); return 0; @@ -1498,7 +1498,7 @@ static int do_git_config_sequence(config_fn_t fn, void *data) { int ret = 0; char *xdg_config = xdg_config_home("config"); - char *user_config = expand_user_path("~/.gitconfig"); + char *user_config = expand_user_path("~/.gitconfig", 0); char *repo_config = have_git_dir() ? git_pathdup("config") : NULL; current_parsing_scope = CONFIG_SCOPE_SYSTEM; diff --git a/credential-cache.c b/credential-cache.c index 3cbd420019..91550bfb0b 100644 --- a/credential-cache.c +++ b/credential-cache.c @@ -87,7 +87,7 @@ static char *get_socket_path(void) { struct stat sb; char *old_dir, *socket; - old_dir = expand_user_path("~/.git-credential-cache"); + old_dir = expand_user_path("~/.git-credential-cache", 0); if (old_dir && !stat(old_dir, &sb) && S_ISDIR(sb.st_mode)) socket = xstrfmt("%s/socket", old_dir); else diff --git a/credential-store.c b/credential-store.c index 55ca1b1334..ac295420dd 100644 --- a/credential-store.c +++ b/credential-store.c @@ -168,7 +168,7 @@ int cmd_main(int argc, const char **argv) if (file) { string_list_append(&fns, file); } else { - if ((file = expand_user_path("~/.git-credentials"))) + if ((file = expand_user_path("~/.git-credentials", 0))) string_list_append_nodup(&fns, file); file = xdg_config_home("credentials"); if (file) diff --git a/path.c b/path.c index 22248436bf..010c565512 100644 --- a/path.c +++ b/path.c @@ -638,8 +638,10 @@ static struct passwd *getpw_str(const char *username, size_t len) * Return a string with ~ and ~user expanded via getpw*. If buf != NULL, * then it is a newly allocated string. Returns NULL on getpw failure or * if path is NULL. + * + * If real_home is true, real_path($HOME) is used in the expansion. */ -char *expand_user_path(const char *path) +char *expand_user_path(const char *path, int real_home) { struct strbuf user_path = STRBUF_INIT; const char *to_copy = path; @@ -654,7 +656,10 @@ char *expand_user_path(const char *path) const char *home = getenv("HOME"); if (!home) goto return_null; - strbuf_addstr(&user_path, home); + if (real_home) + strbuf_addstr(&user_path, real_path(home)); + else + strbuf_addstr(&user_path, home); #ifdef GIT_WINDOWS_NATIVE convert_slashes(user_path.buf); #endif @@ -723,7 +728,7 @@ const char *enter_repo(const char *path, int strict) strbuf_add(&validated_path, path, len); if (used_path.buf[0] == '~') { - char *newpath = expand_user_path(used_path.buf); + char *newpath = expand_user_path(used_path.buf, 0); if (!newpath) return NULL; strbuf_attach(&used_path, newpath, strlen(newpath), -- 2.11.0.157.gd943d85 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/2] config: resolve symlinks in conditional include's patterns 2017-03-30 11:37 ` [PATCH 1/2] path.c: and an option to call real_path() in expand_user_path() Nguyễn Thái Ngọc Duy @ 2017-03-30 11:37 ` Nguyễn Thái Ngọc Duy 2017-03-30 18:38 ` Junio C Hamano 2017-04-05 10:24 ` [PATCH v2 1/2] path.c: and an option to call real_path() in expand_user_path() Nguyễn Thái Ngọc Duy 1 sibling, 1 reply; 11+ messages in thread From: Nguyễn Thái Ngọc Duy @ 2017-03-30 11:37 UTC (permalink / raw) To: git; +Cc: tboegi, Junio C Hamano, Nguyễn Thái Ngọc Duy $GIT_DIR returned by get_git_dir() is normalized, with all symlinks resolved (see setup_work_tree function). In order to match paths (or patterns) against $GIT_DIR char-by-char, they have to be normalized too. There is a note in config.txt about this, that the user need to resolve symlinks by themselves if needed. The problem is, we allow certain path expansion, '~/' and './', for convenience and can't ask the user to resolve symlinks in these expansions. Make sure the expanded paths have all symlinks resolved. PS. The strbuf_realpath(&text, get_git_dir(), 1) is still needed because get_git_dir() may return relative path. Noticed-by: Torsten Bögershausen <tboegi@web.de> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> --- config.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/config.c b/config.c index f036c721e6..d5ba848b65 100644 --- a/config.c +++ b/config.c @@ -177,7 +177,7 @@ static int prepare_include_condition_pattern(struct strbuf *pat) char *expanded; int prefix = 0; - expanded = expand_user_path(pat->buf, 0); + expanded = expand_user_path(pat->buf, 1); if (expanded) { strbuf_reset(pat); strbuf_addstr(pat, expanded); @@ -191,7 +191,7 @@ static int prepare_include_condition_pattern(struct strbuf *pat) return error(_("relative config include " "conditionals must come from files")); - strbuf_add_absolute_path(&path, cf->path); + strbuf_realpath(&path, cf->path, 1); slash = find_last_dir_sep(path.buf); if (!slash) die("BUG: how is this possible?"); @@ -213,7 +213,7 @@ static int include_by_gitdir(const char *cond, size_t cond_len, int icase) struct strbuf pattern = STRBUF_INIT; int ret = 0, prefix; - strbuf_add_absolute_path(&text, get_git_dir()); + strbuf_realpath(&text, get_git_dir(), 1); strbuf_add(&pattern, cond, cond_len); prefix = prepare_include_condition_pattern(&pattern); -- 2.11.0.157.gd943d85 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] config: resolve symlinks in conditional include's patterns 2017-03-30 11:37 ` [PATCH 2/2] config: resolve symlinks in conditional include's patterns Nguyễn Thái Ngọc Duy @ 2017-03-30 18:38 ` Junio C Hamano 2017-04-04 10:12 ` Duy Nguyen 0 siblings, 1 reply; 11+ messages in thread From: Junio C Hamano @ 2017-03-30 18:38 UTC (permalink / raw) To: Nguyễn Thái Ngọc Duy; +Cc: git, tboegi Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes: > $GIT_DIR returned by get_git_dir() is normalized, with all symlinks > resolved (see setup_work_tree function). In order to match paths (or > patterns) against $GIT_DIR char-by-char, they have to be normalized > too. There is a note in config.txt about this, that the user need to > resolve symlinks by themselves if needed. > > The problem is, we allow certain path expansion, '~/' and './', for > convenience and can't ask the user to resolve symlinks in these > expansions. Make sure the expanded paths have all symlinks resolved. That sounds sensible but I fail to see why 1/2 is the right approach to do this, and I must be missing something. Wouldn't you get the same result if you run realpath() yourself on expanded, after receiving the return value of expand_user_path() in it? Can you add a test to demonstrate the issue (which would need to be protected with SYMLINKS prereq)? Thanks. > PS. The strbuf_realpath(&text, get_git_dir(), 1) is still needed because > get_git_dir() may return relative path. > > Noticed-by: Torsten Bögershausen <tboegi@web.de> > Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> > --- > config.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/config.c b/config.c > index f036c721e6..d5ba848b65 100644 > --- a/config.c > +++ b/config.c > @@ -177,7 +177,7 @@ static int prepare_include_condition_pattern(struct strbuf *pat) > char *expanded; > int prefix = 0; > > - expanded = expand_user_path(pat->buf, 0); > + expanded = expand_user_path(pat->buf, 1); > if (expanded) { > strbuf_reset(pat); > strbuf_addstr(pat, expanded); > @@ -191,7 +191,7 @@ static int prepare_include_condition_pattern(struct strbuf *pat) > return error(_("relative config include " > "conditionals must come from files")); > > - strbuf_add_absolute_path(&path, cf->path); > + strbuf_realpath(&path, cf->path, 1); > slash = find_last_dir_sep(path.buf); > if (!slash) > die("BUG: how is this possible?"); > @@ -213,7 +213,7 @@ static int include_by_gitdir(const char *cond, size_t cond_len, int icase) > struct strbuf pattern = STRBUF_INIT; > int ret = 0, prefix; > > - strbuf_add_absolute_path(&text, get_git_dir()); > + strbuf_realpath(&text, get_git_dir(), 1); > strbuf_add(&pattern, cond, cond_len); > prefix = prepare_include_condition_pattern(&pattern); ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] config: resolve symlinks in conditional include's patterns 2017-03-30 18:38 ` Junio C Hamano @ 2017-04-04 10:12 ` Duy Nguyen 0 siblings, 0 replies; 11+ messages in thread From: Duy Nguyen @ 2017-04-04 10:12 UTC (permalink / raw) To: Junio C Hamano; +Cc: Git Mailing List, Torsten Bögershausen On Fri, Mar 31, 2017 at 1:38 AM, Junio C Hamano <gitster@pobox.com> wrote: > Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes: > >> $GIT_DIR returned by get_git_dir() is normalized, with all symlinks >> resolved (see setup_work_tree function). In order to match paths (or >> patterns) against $GIT_DIR char-by-char, they have to be normalized >> too. There is a note in config.txt about this, that the user need to >> resolve symlinks by themselves if needed. >> >> The problem is, we allow certain path expansion, '~/' and './', for >> convenience and can't ask the user to resolve symlinks in these >> expansions. Make sure the expanded paths have all symlinks resolved. > > That sounds sensible but I fail to see why 1/2 is the right approach > to do this, and I must be missing something. Wouldn't you get the > same result if you run realpath() yourself on expanded, after > receiving the return value of expand_user_path() in it? Because at that point I don't know what part is $HOME (i.e. valid path that real_path can walk through), what part is random wildcards from the pattern. Note that in this case we pass a wildmatch pattern to expand_user_path(), like ~/[ab]foo/*bar*/**. After expansion it becomes /home/pclouds/[ab]foo/*bar*/**. It does not feel right to let real_path() walk the "[ab]foo..." part. In the tests, I hit die("Invalid path") in strbuf_realpath(). Even if I set die_on_error() to avoid that, strbuf_realpath() will not return the resolved path > Can you add a test to demonstrate the issue (which would need to be > protected with SYMLINKS prereq)? Will do. It may look a bit ugly though because I have to force setup_git_directory() to call real_path() because it doesn't always do that. -- Duy ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 1/2] path.c: and an option to call real_path() in expand_user_path() 2017-03-30 11:37 ` [PATCH 1/2] path.c: and an option to call real_path() in expand_user_path() Nguyễn Thái Ngọc Duy 2017-03-30 11:37 ` [PATCH 2/2] config: resolve symlinks in conditional include's patterns Nguyễn Thái Ngọc Duy @ 2017-04-05 10:24 ` Nguyễn Thái Ngọc Duy 2017-04-05 10:24 ` [PATCH v2 2/2] config: resolve symlinks in conditional include's patterns Nguyễn Thái Ngọc Duy 1 sibling, 1 reply; 11+ messages in thread From: Nguyễn Thái Ngọc Duy @ 2017-04-05 10:24 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, tboegi, Nguyễn Thái Ngọc Duy In the next patch we need the ability to expand '~' to real_path($HOME). But we can't do that from outside because '~' is part of a pattern, not a true path. Add an option to expand_user_path() to do so. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> --- No changes in v2. builtin/commit.c | 2 +- builtin/config.c | 2 +- cache.h | 2 +- config.c | 8 ++++---- credential-cache.c | 2 +- credential-store.c | 2 +- path.c | 11 ++++++++--- 7 files changed, 17 insertions(+), 12 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index 4e288bc513..ad188fea9e 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1404,7 +1404,7 @@ int cmd_status(int argc, const char **argv, const char *prefix) static const char *implicit_ident_advice(void) { - char *user_config = expand_user_path("~/.gitconfig"); + char *user_config = expand_user_path("~/.gitconfig", 0); char *xdg_config = xdg_config_home("config"); int config_exists = file_exists(user_config) || file_exists(xdg_config); diff --git a/builtin/config.c b/builtin/config.c index 05843a0f96..70bfaaaa1d 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -502,7 +502,7 @@ int cmd_config(int argc, const char **argv, const char *prefix) } if (use_global_config) { - char *user_config = expand_user_path("~/.gitconfig"); + char *user_config = expand_user_path("~/.gitconfig", 0); char *xdg_config = xdg_config_home("config"); if (!user_config) diff --git a/cache.h b/cache.h index 2214d52f61..62e44bfa2f 100644 --- a/cache.h +++ b/cache.h @@ -1146,7 +1146,7 @@ typedef int create_file_fn(const char *path, void *cb); int raceproof_create_file(const char *path, create_file_fn fn, void *cb); int mkdir_in_gitdir(const char *path); -extern char *expand_user_path(const char *path); +extern char *expand_user_path(const char *path, int real_home); const char *enter_repo(const char *path, int strict); static inline int is_absolute_path(const char *path) { diff --git a/config.c b/config.c index 1a4d85537b..f036c721e6 100644 --- a/config.c +++ b/config.c @@ -135,7 +135,7 @@ static int handle_path_include(const char *path, struct config_include_data *inc if (!path) return config_error_nonbool("include.path"); - expanded = expand_user_path(path); + expanded = expand_user_path(path, 0); if (!expanded) return error("could not expand include path '%s'", path); path = expanded; @@ -177,7 +177,7 @@ static int prepare_include_condition_pattern(struct strbuf *pat) char *expanded; int prefix = 0; - expanded = expand_user_path(pat->buf); + expanded = expand_user_path(pat->buf, 0); if (expanded) { strbuf_reset(pat); strbuf_addstr(pat, expanded); @@ -948,7 +948,7 @@ int git_config_pathname(const char **dest, const char *var, const char *value) { if (!value) return config_error_nonbool(var); - *dest = expand_user_path(value); + *dest = expand_user_path(value, 0); if (!*dest) die(_("failed to expand user dir in: '%s'"), value); return 0; @@ -1498,7 +1498,7 @@ static int do_git_config_sequence(config_fn_t fn, void *data) { int ret = 0; char *xdg_config = xdg_config_home("config"); - char *user_config = expand_user_path("~/.gitconfig"); + char *user_config = expand_user_path("~/.gitconfig", 0); char *repo_config = have_git_dir() ? git_pathdup("config") : NULL; current_parsing_scope = CONFIG_SCOPE_SYSTEM; diff --git a/credential-cache.c b/credential-cache.c index 3cbd420019..91550bfb0b 100644 --- a/credential-cache.c +++ b/credential-cache.c @@ -87,7 +87,7 @@ static char *get_socket_path(void) { struct stat sb; char *old_dir, *socket; - old_dir = expand_user_path("~/.git-credential-cache"); + old_dir = expand_user_path("~/.git-credential-cache", 0); if (old_dir && !stat(old_dir, &sb) && S_ISDIR(sb.st_mode)) socket = xstrfmt("%s/socket", old_dir); else diff --git a/credential-store.c b/credential-store.c index 55ca1b1334..ac295420dd 100644 --- a/credential-store.c +++ b/credential-store.c @@ -168,7 +168,7 @@ int cmd_main(int argc, const char **argv) if (file) { string_list_append(&fns, file); } else { - if ((file = expand_user_path("~/.git-credentials"))) + if ((file = expand_user_path("~/.git-credentials", 0))) string_list_append_nodup(&fns, file); file = xdg_config_home("credentials"); if (file) diff --git a/path.c b/path.c index 22248436bf..010c565512 100644 --- a/path.c +++ b/path.c @@ -638,8 +638,10 @@ static struct passwd *getpw_str(const char *username, size_t len) * Return a string with ~ and ~user expanded via getpw*. If buf != NULL, * then it is a newly allocated string. Returns NULL on getpw failure or * if path is NULL. + * + * If real_home is true, real_path($HOME) is used in the expansion. */ -char *expand_user_path(const char *path) +char *expand_user_path(const char *path, int real_home) { struct strbuf user_path = STRBUF_INIT; const char *to_copy = path; @@ -654,7 +656,10 @@ char *expand_user_path(const char *path) const char *home = getenv("HOME"); if (!home) goto return_null; - strbuf_addstr(&user_path, home); + if (real_home) + strbuf_addstr(&user_path, real_path(home)); + else + strbuf_addstr(&user_path, home); #ifdef GIT_WINDOWS_NATIVE convert_slashes(user_path.buf); #endif @@ -723,7 +728,7 @@ const char *enter_repo(const char *path, int strict) strbuf_add(&validated_path, path, len); if (used_path.buf[0] == '~') { - char *newpath = expand_user_path(used_path.buf); + char *newpath = expand_user_path(used_path.buf, 0); if (!newpath) return NULL; strbuf_attach(&used_path, newpath, strlen(newpath), -- 2.11.0.157.gd943d85 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 2/2] config: resolve symlinks in conditional include's patterns 2017-04-05 10:24 ` [PATCH v2 1/2] path.c: and an option to call real_path() in expand_user_path() Nguyễn Thái Ngọc Duy @ 2017-04-05 10:24 ` Nguyễn Thái Ngọc Duy 0 siblings, 0 replies; 11+ messages in thread From: Nguyễn Thái Ngọc Duy @ 2017-04-05 10:24 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, tboegi, Nguyễn Thái Ngọc Duy $GIT_DIR returned by get_git_dir() is normalized, with all symlinks resolved (see setup_work_tree function). In order to match paths (or patterns) against $GIT_DIR char-by-char, they have to be normalized too. There is a note in config.txt about this, that the user need to resolve symlinks by themselves if needed. The problem is, we allow certain path expansion, '~/' and './', for convenience and can't ask the user to resolve symlinks in these expansions. Make sure the expanded paths have all symlinks resolved. PS. The strbuf_realpath(&text, get_git_dir(), 1) is still needed because get_git_dir() may return relative path. Noticed-by: Torsten Bögershausen <tboegi@web.de> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> --- Tests are added in v2. config.c | 6 +++--- t/t1305-config-include.sh | 54 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/config.c b/config.c index f036c721e6..d5ba848b65 100644 --- a/config.c +++ b/config.c @@ -177,7 +177,7 @@ static int prepare_include_condition_pattern(struct strbuf *pat) char *expanded; int prefix = 0; - expanded = expand_user_path(pat->buf, 0); + expanded = expand_user_path(pat->buf, 1); if (expanded) { strbuf_reset(pat); strbuf_addstr(pat, expanded); @@ -191,7 +191,7 @@ static int prepare_include_condition_pattern(struct strbuf *pat) return error(_("relative config include " "conditionals must come from files")); - strbuf_add_absolute_path(&path, cf->path); + strbuf_realpath(&path, cf->path, 1); slash = find_last_dir_sep(path.buf); if (!slash) die("BUG: how is this possible?"); @@ -213,7 +213,7 @@ static int include_by_gitdir(const char *cond, size_t cond_len, int icase) struct strbuf pattern = STRBUF_INIT; int ret = 0, prefix; - strbuf_add_absolute_path(&text, get_git_dir()); + strbuf_realpath(&text, get_git_dir(), 1); strbuf_add(&pattern, cond, cond_len); prefix = prepare_include_condition_pattern(&pattern); diff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh index e833939320..8fbc7a029f 100755 --- a/t/t1305-config-include.sh +++ b/t/t1305-config-include.sh @@ -3,6 +3,16 @@ test_description='test config file include directives' . ./test-lib.sh +# Force setup_explicit_git_dir() to run until the end. This is needed +# by some tests to make sure real_path() is called on $GIT_DIR. The +# caller needs to make sure git commands are run from a subdirectory +# though or real_path() will not be called. +force_setup_explicit_git_dir() { + GIT_DIR="$(pwd)/.git" + GIT_WORK_TREE="$(pwd)" + export GIT_DIR GIT_WORK_TREE +} + test_expect_success 'include file by absolute path' ' echo "[test]one = 1" >one && echo "[include]path = \"$(pwd)/one\"" >.gitconfig && @@ -208,6 +218,50 @@ test_expect_success 'conditional include, both unanchored, icase' ' ) ' +test_expect_success SYMLINKS 'conditional include, set up symlinked $HOME' ' + mkdir real-home && + ln -s real-home home && + ( + HOME="$TRASH_DIRECTORY/home" && + export HOME && + cd "$HOME" && + + git init foo && + cd foo && + mkdir sub + ) +' + +test_expect_success SYMLINKS 'conditional include, $HOME expansion with symlinks' ' + ( + HOME="$TRASH_DIRECTORY/home" && + export HOME && + cd "$HOME"/foo && + + echo "[includeIf \"gitdir:~/foo/\"]path=bar2" >>.git/config && + echo "[test]two=2" >.git/bar2 && + echo 2 >expect && + force_setup_explicit_git_dir && + git -C sub config test.two >actual && + test_cmp expect actual + ) +' + +test_expect_success SYMLINKS 'conditional include, relative path with symlinks' ' + echo "[includeIf \"gitdir:./foo/.git\"]path=bar4" >home/.gitconfig && + echo "[test]four=4" >home/bar4 && + ( + HOME="$TRASH_DIRECTORY/home" && + export HOME && + cd "$HOME"/foo && + + echo 4 >expect && + force_setup_explicit_git_dir && + git -C sub config test.four >actual && + test_cmp expect actual + ) +' + test_expect_success 'include cycles are detected' ' cat >.gitconfig <<-\EOF && [test]value = gitconfig -- 2.11.0.157.gd943d85 ^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2017-04-05 10:26 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-03-25 10:46 t1503 broken ? Torsten Bögershausen 2017-03-25 11:46 ` Duy Nguyen 2017-03-25 12:26 ` Duy Nguyen 2017-03-25 13:05 ` Duy Nguyen 2017-03-25 19:41 ` Torsten Bögershausen 2017-03-30 11:37 ` [PATCH 1/2] path.c: and an option to call real_path() in expand_user_path() Nguyễn Thái Ngọc Duy 2017-03-30 11:37 ` [PATCH 2/2] config: resolve symlinks in conditional include's patterns Nguyễn Thái Ngọc Duy 2017-03-30 18:38 ` Junio C Hamano 2017-04-04 10:12 ` Duy Nguyen 2017-04-05 10:24 ` [PATCH v2 1/2] path.c: and an option to call real_path() in expand_user_path() Nguyễn Thái Ngọc Duy 2017-04-05 10:24 ` [PATCH v2 2/2] config: resolve symlinks in conditional include's patterns Nguyễn Thái Ngọc Duy
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.