[PATCH v3] git-sh-setup: Fix scripts whose PWD is a symlink into a git work-dir

From

Marcel M. Cary <marcel@oak.homeunix.org>

Date

Dec 15, 2008, 17:34 UTC

Message-ID

<1229362477-22538-1-git-send-email-marcel@oak.homeunix.org>

In-Reply-To

<7v4p174diu.fsf@gitster.siamese.dyndns.org>

Changes:

* When interpreting a relative upward (../) path in cd_to_toplevel,
  prepend the cwd without symlinks, given by /bin/pwd
* Add tests for cd_to_toplevel and "git pull" in a symlinked
  directory that failed before this fix, plus contrasting scenarios
  that already worked

Signed-off-by: Marcel M. Cary <marcel@oak.homeunix.org>
---

> > I also removed the "pwd -P" from the unit test.
>
> Hmm, really...?

Ouch.  Removing just one occurrence won't help much, will it.

> > Do you think these test cases should run all the time here?
>
> I'd say so.  Your supporting argument could be "See, push works just
> fine with this layout, but pull doesn't because it is a shell script
> that can be fooled, and this change is to fix the inconsistencies
> between them."

> But whether it is inside test_debug or not, the test should check
> not just the exit status from 'git push' but also check what
> happened to the receiving repository at least to make sure it is
> pushing to the location you are expecting it to.

 git-sh-setup.sh           |   23 ++++++++++++-
 t/t2300-cd-to-toplevel.sh |   37 +++++++++++++++++++++
 t/t5521-pull-symlink.sh   |   78 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 136 insertions(+), 2 deletions(-)
 create mode 100755 t/t2300-cd-to-toplevel.sh
 create mode 100755 t/t5521-pull-symlink.sh

diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index dbdf209..f07d96b 100755
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -85,8 +85,27 @@ cd_to_toplevel () {
 	cdup=$(git rev-parse --show-cdup)
 	if test ! -z "$cdup"
 	then
-		cd "$cdup" || {
-			echo >&2 "Cannot chdir to $cdup, the toplevel of the working tree"
+		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="$(/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"
 			exit 1
 		}
 	fi
diff --git a/t/t2300-cd-to-toplevel.sh b/t/t2300-cd-to-toplevel.sh
new file mode 100755
index 0000000..beddb4e
--- /dev/null
+++ b/t/t2300-cd-to-toplevel.sh
@@ -0,0 +1,37 @@
+#!/bin/sh
+
+test_description='cd_to_toplevel'
+
+. ./test-lib.sh
+
+test_cd_to_toplevel () {
+	test_expect_success "$2" '
+		(
+			cd '"'$1'"' &&
+			. git-sh-setup &&
+			cd_to_toplevel &&
+			[ "$(/bin/pwd)" = "$TOPLEVEL" ]
+		)
+	'
+}
+
+TOPLEVEL="$(/bin/pwd)/repo"
+mkdir -p repo/sub/dir
+mv .git repo/
+SUBDIRECTORY_OK=1
+
+test_cd_to_toplevel repo 'at physical root'
+
+test_cd_to_toplevel repo/sub/dir 'at physical subdir'
+
+ln -s repo symrepo
+test_cd_to_toplevel symrepo 'at symbolic root'
+
+ln -s repo/sub/dir subdir-link
+test_cd_to_toplevel subdir-link 'at symbolic subdir'
+
+cd repo
+ln -s sub/dir internal-link
+test_cd_to_toplevel internal-link 'at internal symbolic subdir'
+
+test_done
diff --git a/t/t5521-pull-symlink.sh b/t/t5521-pull-symlink.sh
new file mode 100755
index 0000000..5672b51
--- /dev/null
+++ b/t/t5521-pull-symlink.sh
@@ -0,0 +1,78 @@
+#!/bin/sh
+
+test_description='pulling from symlinked subdir'
+
+. ./test-lib.sh
+
+# The scenario we are building:
+#
+#   trash\ directory/
+#     clone-repo/
+#       subdir/
+#         bar
+#     subdir-link -> clone-repo/subdir/
+#
+# The working directory is subdir-link.
+
+mkdir subdir
+echo file >subdir/file
+git add subdir/file
+git commit -q -m file
+git clone -q . clone-repo
+ln -s clone-repo/subdir/ subdir-link
+
+
+# Demonstrate that things work if we just avoid the symlink
+#
+test_expect_success 'pulling from real subdir' '
+	(
+		echo real >subdir/file &&
+		git commit -m real subdir/file &&
+		cd clone-repo/subdir/ &&
+		git pull &&
+		test real = $(cat file)
+	)
+'
+
+# From subdir-link, pulling should work as it does from
+# clone-repo/subdir/.
+#
+# Instead, the error pull gave was:
+#
+#   fatal: 'origin': unable to chdir or not a git archive
+#   fatal: The remote end hung up unexpectedly
+#
+# because git would find the .git/config for the "trash directory"
+# repo, not for the clone-repo repo.  The "trash directory" repo
+# had no entry for origin.  Git found the wrong .git because
+# git rev-parse --show-cdup printed a path relative to
+# clone-repo/subdir/, not subdir-link/.  Git rev-parse --show-cdup
+# used the correct .git, but when the git pull shell script did
+# "cd `git rev-parse --show-cdup`", it ended up in the wrong
+# directory.  A POSIX shell's "cd" works a little differently
+# than chdir() in C; "cd -P" is much closer to chdir().
+#
+test_expect_success 'pulling from symlinked subdir' '
+	(
+		echo link >subdir/file &&
+		git commit -m link subdir/file &&
+		cd subdir-link/ &&
+		git pull &&
+		test link = $(cat file)
+	)
+'
+
+# Prove that the remote end really is a repo, and other commands
+# work fine in this context.  It's just that "git pull" breaks.
+#
+test_expect_success 'pushing from symlinked subdir' '
+	(
+		cd subdir-link/ &&
+		echo push >file &&
+		git commit -m push ./file &&
+		git push
+	) &&
+	test push = $(git show HEAD:subdir/file)
+'
+
+test_done

-- 
1.6.0.3