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

[Duy Nguyen <pclouds@gmail.com>]

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