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

From: Steven Grimm <koreth@midwinter.com>
Date: Apr 26, 2007, 09:00 UTC
Message-ID: <46306A29.4010608@midwinter.com>
In-Reply-To: <81b0412b0704260120mda8a2abhe343f5c127945939@mail.gmail.com>

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.

Show 8 quoted lines

>> 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