Re: [PATCH 1/3] make_absolute_path: Don't try to copy a string to itself
On mié, 2011-03-16 at 21:16 +0700, Nguyen Thai Ngoc Duy wrote:
Show 18 quoted lines
> 2011/3/16 Carlos Martín Nieto <cmn@elego.de>:
> > I've been changing this a bit, trying to make all the paths normalized,
> > but it'll take a bit longer. I'll send a partial patch when I've
> > finished something worth seeing (for the moment, the test fail if there
> > is a symlink somewhere in the tree, as I've mixed
> > real_path/make_absolute_path and absolute_path/make_nonrelative_path a
> > bit).
> >
> > Is it a good idea to normalize the paths? Otherwise, everything could
> > be replaced by real_path/make_absolute_path (as most calls already are).
> > As it's transitive and these paths aren't stored permanently (other than
> > with clone), as long as we agree on one representation, it should be
> > fine.
>
> I think the question is whether it's _necessary_ to do that. Any gain?
> make_absolute_path() calls are not in critical path, I don't think we
> should bother much, unless there are bugs like one you fixed in your
> patch.
I was under the wrong impression that non-normalized paths were what
was causing is_inside_dir not to recognize the paths (this with a patch
using non-resolved absolute paths as given by make_nonrelative_path
rather than make_absolute_path).
As it turns out, getcwd resolves the links for us, so is_inside_dir
would say e.g. that /home/cmn/two/git/t wasn't under /home/cmn/two/git,
because getcwd said the cwd was /home/cmn/one/git (two is a symlink to
one).
At any rate, I think make_absolute_path is mis-named and it should be
called real_path (or make_real_path). The difference between
make_nonrelative_path and make_absolute_path is certainly not clear
without looking at the implementation.
Show 9 quoted lines
>
> > Is there a performance hit if we resolve links all the time? If we run
> > everything through normalize_path(_copy), is it slower than resolving
> > links?
>
> What paths are you talking about? If they are inside $GIT_DIR, we
> touch them quite often. But there are not many of them (unless you
> spread loose objects all over the place), resolving links should not
> be an issue.
There aren't in fact that many calls to these functions, so resolving
should be fine. More on this as an answer to your other mail.
Carlos Martín Nieto <cmn@elego.de>