On miÃ, 2011-03-16 at 21:16 +0700, Nguyen Thai Ngoc Duy wrote: > 2011/3/16 Carlos MartÃn Nieto <cmn@xxxxxxxx>: > > 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. > > > 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. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html