git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Marcel M. Cary" <marcel@oak.homeunix.org>
To: gitster@pobox.com, git@vger.kernel.org
Cc: jnareb@gmail.com, ae@op5.se, j.sixt@viscovery.net,
	"Marcel M. Cary" <marcel@oak.homeunix.org>
Subject: [PATCH] git-sh-setup: Fix scripts whose PWD is a symlink into a git work-dir
Date: Tue,  2 Dec 2008 21:27:00 -0800	[thread overview]
Message-ID: <1228282020-2294-1-git-send-email-marcel@oak.homeunix.org> (raw)
In-Reply-To: <7vtz9vk6uj.fsf@gitster.siamese.dyndns.org>

* Before interpretting an upward path (../) in cd_to_toplevel,
  cd to a path 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 constrasting
  scenarios that already worked

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

I hope this patch will address concerns both about changes
to existing APIs and speed of the new behavior.

A few notes on implementation choices:

I used /bin/pwd because of this precedent for choosing it over
"cd -P" for compatibility.
http://article.gmane.org/gmane.comp.version-control.git/46918

If cd_to_toplevel had concatenated $(/bin/pwd) with $cdup to
avoid the separate "cd", it would require checking for $cdup
being an absolute path.  I wasn't sure how to check that in
a way that is both portable and clearly faster than "cd",
so cd_to_toplevel runs "cd" twice.  I'm assuming that
running an external command like expr or grep is slower than
just doing the "cd".

cd_to_toplevel doesn't check $PWD to see whether to do the
first cd, because some shells allegedly don't update it
reliably.

Since cd_to_toplevel doesn't know whether it's at a
symlinked PWD or not, I wrote it to treat the 
"cd $(/bin/pwd)" as mandatory, even when it might not
actually be.  So on systems without /bin/pwd, it will fail
even when there are no symlinks.  I thought that was better
than inconsistent behavior depending on whether /bin/pwd is
available.

The extra "cd" will be skipped when the script is already at
the top of the working tree.


 git-sh-setup.sh           |   11 +++++++
 t/t2300-cd-to-toplevel.sh |   37 +++++++++++++++++++++++++
 t/t5521-pull-symlink.sh   |   67 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 115 insertions(+), 0 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..377700b 100755
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -85,6 +85,17 @@ cd_to_toplevel () {
 	cdup=$(git rev-parse --show-cdup)
 	if test ! -z "$cdup"
 	then
+		# 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)" || {
+			echo >&2 "Cannot determine the physical path to the current dir"
+			exit 1
+		}
+		cd "$phys" || {
+			echo >&2 "Cannot chdir to the physical path to current dir: $phys"
+			exit 1
+		}
+
 		cd "$cdup" || {
 			echo >&2 "Cannot chdir to $cdup, the toplevel of the working tree"
 			exit 1
diff --git a/t/t2300-cd-to-toplevel.sh b/t/t2300-cd-to-toplevel.sh
new file mode 100755
index 0000000..293dc35
--- /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 &&
+			[ "$(pwd -P)" = "$TOPLEVEL" ]
+		)
+	'
+}
+
+TOPLEVEL="$(pwd -P)/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..f18fec7
--- /dev/null
+++ b/t/t5521-pull-symlink.sh
@@ -0,0 +1,67 @@
+#!/bin/sh
+
+test_description='pulling from symlinked subdir'
+
+. ./test-lib.sh
+
+D=`pwd`
+
+# The scenario we are building:
+#
+#   trash\ directory/
+#     clone-repo/
+#       subdir/
+#         bar
+#     subdir-link -> clone-repo/subdir/
+#
+# The working directory is subdir-link.
+#
+test_expect_success setup '
+
+    mkdir subdir &&
+    touch subdir/bar &&
+    git add subdir/bar &&
+    git commit -m empty &&
+    git clone . clone-repo &&
+    # demonstrate that things work without the symlink
+    test_debug "cd clone-repo/subdir/ && git pull; cd ../.." &&
+    ln -s clone-repo/subdir/ subdir-link &&
+    cd subdir-link/ &&
+    test_debug "set +x"
+'
+
+# 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.  Shell "cd" works a little different from chdir() in C.
+# Bash's "cd -P" works like chdir() in C.
+#
+test_expect_success 'pulling from symlinked subdir' '
+
+    git pull
+'
+
+# Prove that the remote end really is a repo, and other commands
+# work fine in this context.
+#
+test_debug "
+    test_expect_success 'pushing from symlinked subdir' '
+
+        git push
+    '
+"
+cd "$D"
+
+test_done
-- 
1.6.0.3

  reply	other threads:[~2008-12-03  5:28 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-11-15 15:11 [RFC PATCH 0/2] fixing git pull from symlinked directory Marcel M. Cary
2008-11-15 15:11 ` [RFC PATCH 1/2] Add failing test for "git pull" in " Marcel M. Cary
2008-11-15 15:11   ` [RFC PATCH 2/2] Support shell scripts that run from symlinks into a git working dir Marcel M. Cary
2008-11-22 21:33 ` [PATCH] rev-parse: Fix shell scripts whose cwd is a symlink into a git work-dir Marcel M. Cary
2008-11-22 21:54   ` Jakub Narebski
2008-11-23  7:10     ` Andreas Ericsson
2008-11-25  5:54       ` Marcel M. Cary
2008-11-25  6:50         ` Andreas Ericsson
2008-11-25  5:17     ` Marcel M. Cary
2008-11-25  7:30   ` Johannes Sixt
2008-11-25 16:16     ` Marcel M. Cary
2008-11-25 16:30       ` Johannes Sixt
2008-11-25 18:17       ` Junio C Hamano
2008-12-03  5:27         ` Marcel M. Cary [this message]
2008-12-03  7:20           ` [PATCH] git-sh-setup: Fix scripts whose PWD " Junio C Hamano
2008-12-10 15:04             ` [PATCH v2] " Marcel M. Cary
2008-12-10 20:18               ` Junio C Hamano
2008-12-13 20:47                 ` [PATCH v3] " Marcel M. Cary
2008-12-14  3:54                   ` Junio C Hamano
2008-12-15 17:34                     ` Marcel M. Cary
2008-12-15 17:38                       ` [PATCH v3] <-- really v4 Marcel M. Cary
2009-02-07  3:24             ` [RFC PATCH] git-sh-setup: Use "cd" option, not /bin/pwd, for symlinked work tree Marcel M. Cary
2009-02-07 12:25               ` Johannes Schindelin
2009-02-08 18:11                 ` Marcel M. Cary
2009-02-08 20:56                   ` Johannes Schindelin
2009-02-11 14:44                     ` Marcel M. Cary
2009-02-11 18:16                       ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1228282020-2294-1-git-send-email-marcel@oak.homeunix.org \
    --to=marcel@oak.homeunix.org \
    --cc=ae@op5.se \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j.sixt@viscovery.net \
    --cc=jnareb@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).