[RFC PATCH] git-sh-setup: Use "cd" option, not /bin/pwd, for symlinked work tree

From: Marcel M. Cary <marcel@oak.homeunix.org>
Date: Feb 7, 2009, 03:24 UTC
Message-ID: <1233977068-24861-1-git-send-email-marcel@oak.homeunix.org>
In-Reply-To: <7viqq1hghw.fsf@gitster.siamese.dyndns.org>

In cd_to_toplevel, instead of 'cd $(unset PWD; /bin/pwd)/$path' use 'cd -P $path'. The "-P" option yields a desirable similarity to C chdir.

While the "-P" option may be slightly less commonly supported than /bin/pwd, it is more concise, better tested, and less error prone. I've already added the 'unset PWD' to fix the /bin/pwd solution on BSD; there may be more edge cases out there.

This still passes all the same test cases in t5521-pull-symlink.sh and t2300-cd-to-toplevel.sh, even before updating them to use 'pwd -P'. ---

> ... I think it would probably be better to bite the bullet
> and start using "cd -P" soon after 1.6.1 goes final, ...

Here's a post-1.6.1 way to make Git shell scripts work like C programs when in symlinked directories.

> ... and at the same time
> existing places that use "cd `pwd`" as a workaround if there are some.

I haven't found other places that use the "cd `/bin/pwd`" workaround.

I also wasn't able to think of a test case that fails under the previous way and works under this new way either.

Any opinions on whether this is worthwhile? It seems more standardized if less supported, odd as that seems to me.

Marcel

 git-sh-setup.sh           |   29 ++++++++---------------------
 t/t2300-cd-to-toplevel.sh |    4 ++--
 2 files changed, 10 insertions(+), 23 deletions(-)

diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 2142308..8382339 100755
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -85,27 +85,14 @@ cd_to_toplevel () {
 	cdup=$(git rev-parse --show-cdup)
 	if test ! -z "$cdup"
 	then
-		case "$cdup" in
-		/*)
-			# Not quite the same as if we did "cd -P '$cdup'" when
-			# $cdup contains ".." after symlink path components.
-			# Don't fix that case at least until Git switches to
-			# "cd -P" across the board.
-			phys="$cdup"
-			;;
-		..|../*|*/..|*/../*)
-			# Interpret $cdup relative to the physical, not logical, cwd.
-			# Probably /bin/pwd is more portable than passing -P to cd or pwd.
-			phys="$(unset PWD; /bin/pwd)/$cdup"
-			;;
-		*)
-			# There's no "..", so no need to make things absolute.
-			phys="$cdup"
-			;;
-		esac
-
-		cd "$phys" || {
-			echo >&2 "Cannot chdir to $phys, the toplevel of the working tree"
+		# The "-P" option says to follow "physical" directory
+		# structure instead of following symbolic links.  When cdup is
+		# "../", this means following the ".." entry in the current
+		# directory instead textually removing a symlink path element
+		# from the PWD shell variable.  The "-P" behavior is more
+		# consistent with the C-style chdir used by most of Git.
+		cd -P "$cdup" || {
+			echo >&2 "Cannot chdir to $cdup, the toplevel of the working tree"
 			exit 1
 		}
 	fi
diff --git a/t/t2300-cd-to-toplevel.sh b/t/t2300-cd-to-toplevel.sh
index e42cbfe..293dc35 100755
--- a/t/t2300-cd-to-toplevel.sh
+++ b/t/t2300-cd-to-toplevel.sh
@@ -10,12 +10,12 @@ test_cd_to_toplevel () {
 			cd '"'$1'"' &&
 			. git-sh-setup &&
 			cd_to_toplevel &&
-			[ "$(unset PWD; /bin/pwd)" = "$TOPLEVEL" ]
+			[ "$(pwd -P)" = "$TOPLEVEL" ]
 		)
 	'
 }
 
-TOPLEVEL="$(unset PWD; /bin/pwd)/repo"
+TOPLEVEL="$(pwd -P)/repo"
 mkdir -p repo/sub/dir
 mv .git repo/
 SUBDIRECTORY_OK=1

-- 
1.6.1