Re: [PATCH 2/2] config: resolve symlinks in conditional include's patterns
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 30, 2017, 18:38 UTC
- Message-ID
- <xmqqinmq4nkb.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <20170330113723.20474-2-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
Show 9 quoted lines
> $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.
Show 39 quoted lines
> 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);