From: Nguyen Thai Ngoc Duy Date: Wed, 29 Dec 2010 13:44:11 GMT Subject: Re: [PATCH] setup: translate symlinks in filename when using absolute paths Message-ID: In-Reply-To: <7vr5d1wuh0.fsf@alter.siamese.dyndns.org> 2010/12/29 Junio C Hamano : > Carlo Marcelo Arenas Belon 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 > > 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