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