Re: [PATCH] setup: translate symlinks in filename when using absolute paths

From: Nguyen Thai Ngoc Duy <pclouds@gmail.com>
Date: Dec 29, 2010, 13:44 UTC
Message-ID: <AANLkTi=VQZH01i9bpem7AhSaNv3BMr3__rLKsQ3bO1Rz@mail.gmail.com>
In-Reply-To: <7vr5d1wuh0.fsf@alter.siamese.dyndns.org>

2010/12/29 Junio C Hamano <gitster@pobox.com>:

Show 23 quoted lines

> Carlo Marcelo Arenas Belon <carenas@sajinet.com.pe> writes:
>
>> otherwise, comparison to validate against work tree will fail when
>> the path includes a symlink and the name passed is not canonical.
>>
>> Signed-off-by: Carlo Marcelo Arenas Belon <carenas@sajinet.com.pe>
>
> I take that "path" and "name passed" refer to the same thing (i.e. "path"
> parameter) in the above.
>
> I think you are trying to handle the case where:
>
>  - you give "/home/carenas/one" from the command line;
>  - $PWD is "/home/carenas"; and
>  - "/home/carenas" is a symlink to "/net/host/home/carenas"
>
> and the scan-from-the-beginning-of-string check done between
> "/home/carenas/one" and the return value of get_git_work_tree() which
> presumably is "/net/host/home/carenas" disagrees.  I wonder if a more
> correct solution might be to help get_git_work_tree() to match the notion
> of where the repository and its worktree are to the idea of where the user
> thinks they are, i.e. not "/net/host/home/carenas" but "/home/carenas", a
> bit better?

I tend to agree. Will cause less surprises (such as this one).

> That would involve tweaking make_absolute_path() I guess?

Hm.. can we avoid converting work_tree to absolute path unless people explicitly set it (via --work-tree and GIT_WORK_TREE)? Basically worktree will be relative to cwd. Usually it's just ".". When people run commands outside worktree, it's the relative "cwd/to/worktree". I'm wondering if we can just avoid the use of make_absolute_path() completely in get_git_work_tree()..

> Note that your patch is the right thing to do either case, i.e. with or
> without such a change to make_absolute_path(), as the function is used to
> set up the return value from get_git_work_tree().  Anything we compare
> with it should have passed make_absolute_path() at least once.

Yes, I think that should that be done inside normalize_path_copy(), not prefix_path().

>>  setup.c |   11 +++++++----
>>  1 files changed, 7 insertions(+), 4 deletions(-)

Also Carlo, tests should be good for illustration and regression purposes. I know you described in detail in another mail. But mails tend to get lost.

-- 
Duy