Re: [PATCH] Change git-rev-parse --show-cdup to output an absolute path

On 4/26/07, Steven Grimm [off-list ref] wrote:

Alex Riesen wrote:

quoted

Your implementation will fail if cwd is longer than PATH_MAX.
Does not happen often, though.

That limitation is already littered through the code, e.g. in setup.c
which would already be failing in the existing implementation. Actually
setup.c goes one better: is_inside_git_dir() hardwires a 1024-character
limit into the code rather than using PATH_MAX. I didn't think I was
introducing a new limit here.

It is yet another place to fix when it breaks.

quoted

quoted

A typical failure case:

$ git clone git://whatever.git foobar
$ ln -s foobar/src/tools/misc/myapp myapp
$ cd myapp

Which is a strange thing to do. What is that for?
myapp is kind of outside the git repo foobar.

For convenience, mostly; obviously that example was a bit contrived but
I do have several symlinks to subdirectories of my repository and it's
faster to type "cd ~/xyz" than "cd ~/repo/src/server/xyz" all the time.

Can't you use your shells variables or aliases (which is even faster to
type)?

That was actually my initial approach to fixing this -- I put "cd
`/bin/pwd`" at the top of the "cd_to_toplevel" function in
git-sh-setup.sh. But it felt cleaner to make git-rev-parse return the
actual correct path so it'd work for shell scripts that didn't happen to
use git-sh-setup.sh. I'm happy to go either way, or of course to keep
this as a local modification if folks find it too distasteful to include
in the official source.

I'm not really objecting. It's just a bit unusual so I suspect unusual
problems coming out of it. Kind of when you try running with your
eyes closed.