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

Alex Riesen wrote:

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.

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. 
And as you say, you're only "kind of" outside the repo when going 
through the symlink; one could argue that cd-ing into a symlink should 
be the semantic equivalent of cd-ing into the thing the link points to, 
and that's certainly the way I use it.

But actually my big objection isn't that it fails, per se, but rather 
that it fails inconsistently. All the C commands work just fine since 
they do getcwd() which returns the real path. It's only the shell 
scripts that fail, e.g. git-pull. With the existing implementation I 
have to remember which commands are shell scripts and which are C 
programs, so I can do "cd `/bin/pwd`" to reset my $PWD before running 
any of the former.

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.

-Steve