Re: [PATCH 2/2] config: resolve symlinks in conditional include's patterns

From: Duy Nguyen <pclouds@gmail.com>
Date: Apr 4, 2017, 10:12 UTC
Message-ID: <CACsJy8CbOFAviPo9tCJ-4+ritd5WQboGHDyxaCxBK1UMZe0+kQ@mail.gmail.com>
In-Reply-To: <xmqqinmq4nkb.fsf@gitster.mtv.corp.google.com>

On Fri, Mar 31, 2017 at 1:38 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 16 quoted lines

> 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