Re: [PATCH] rev-parse: Fix shell scripts whose cwd is a symlink into a git work-dir

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxx
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231
--
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

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux