From: Andreas Ericsson <ae@op5.se>
Date: Tue, 25 Nov 2008 06:50:12 GMT
Subject: Re: [PATCH] rev-parse: Fix shell scripts whose cwd is a symlink into a git work-dir
Message-ID: <492BA024.7080506@op5.se>
In-Reply-To: <492B9321.8090706@oak.homeunix.org>

Marcel M. Cary wrote:
>> AFAIR, it was introduced to make test-builds of really large projects in
>> really deep directories with a ton of symlinks leading to the path work a
>> lot faster.
> 
> Andreas,
> 
> I see value in keeping Git very fast.  That is, after all, why I chose
> Git over Mercurial.  Do you know where that discussion was, if was in
> the archives?  I found these reasons to avoid absolute paths in the git
> archives:
> 
> * paths with more components are slower to work with (in the context of
>   add and diff, which deal with many many paths)
> * absolute paths may exceed PATH_MAX while relative ones didn't
> * getcwd() will fail if parent directories are not executable, or on
>   some platforms, if parent directories are not readable
> 
> My impression is that the performance issue is probably not significant
> for cd_to_toplevel since it's not in a tight inner loop, and dito for
> other potential callers of --show-cdup.  The PATH_MAX seems to be a
> restriction elsewhere in the code already.
> 

The performance issue does not come from cd_to_toplevel itself, but from
its callers. That is, if scripts start to use absolute paths from *other*
tight loops, that's when we hit a problem.

> Even if there were a scenario that put --show-cdup in a tight loop, I
> wonder whether current implementation provides much performance benefit,
> at least when bash is the calling language: bash seems to make the
> relative path absolute anyway inside the "cd" builtin.
> 
> The commit (5f94c730) that introduces that code doesn't mention
> performance.  It compares to:
> 
>   git rev-parse --show-prefix | sed -e 's|[^/][^/]*|..|g'
> 
> 
> I also noticed that this failure case with "--show-cdup" in a symlinked
> directory has come up more than once before.
> http://marc.info/?l=git&m=122452534912000&w=2
> http://marc.info/?l=git&m=121613416212958&w=2
> https://kerneltrap.org/mailarchive/git/2007/4/25/244653/thread
> 

I can imagine. However, --show-cdup has a different use too. It's nifty
for printing relative paths from commands running inside a subdirectory
of the repository. If you need the absolute path to the root of the repo,
I'd suggest you add "--show-absolute-path" instead.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231