git-rev-parse --show-cdup returns a relative path instead of absolute (problem with git pull --rebase not finding the git dir)

Hi,

Im reporting this based on a conversation I had in the #git channel on
freenode.

I have a setup where various subdirectories of a number of git repos are
symlinked into a common directory tree. Something like what the
following would create but with more repositories involved:

cd ~; mkdir foo; chdir foo; git init; mkdir bar; git add bar ; git
commit -m'add bar' ; cd ~; ln -s foo/bar bar ; cd bar; 

[try various git commands, not all will work]

Most git command seem perfectly happy to work on the correct repos from
this symlinked tree. However at least one doesnt, git pull --rebase, in
particular.

Doing a 

	git-rev-parse --git-dir 

seems to behave correctly (always finding the correct location) and 

	git-rev-parse --is-inside-work-dir

reports true. However git-pull --rebase responds with lots of "fatal:
Not a git repository" messages. Example is below.

During discussion about this on #git it was suggested this was because 

	git-rev-parse --show-cdup 

returns a relative path. (../).

Im not on list so id appreciate it if anyone replying to this could cc
me on the mail.

Oh, i am aware of submodules but i have to work with what i have now. 

Cheers,
yves


Example of git pull --rebase failing yet git commit working:

[dmq@somewhere apps]$ echo test > test.txt
[dmq@somewhere apps]$ git add test.txt
[dmq@somewhere apps]$ git commit -m'add a test file -- will remove next
commit'
Created commit 45ab725: add a test file -- will remove next commit
 1 files changed, 1 insertions(+), 0 deletions(-)
 create mode 100644 apps/test.txt
[dmq@somewhere apps]$ git rm test.txt
rm 'apps/test.txt'
[dmq@somewhere apps]$ git commit -m'removed test file'
Created commit 2768e6d: removed test file
 1 files changed, 0 insertions(+), 1 deletions(-)
 delete mode 100644 apps/test.txt
[dmq@somewhere apps]$ git pull --rebase
fatal: Not a git repository
fatal: Not a git repository
fatal: Not a git repository
fatal: Not a git repository

Consider the scenario when someone makes a symlink into a working tree
subdirectory at an unrelated place, then attempts to work inside the
symlinked directory. The scenario is a bit unwieldly, but most of
the Git will handle it fine - except git rev-parse --show-cdup. That
will output a sequence of ../ which will work wrong inside the symlink
using shell cd builtin.

This patch changes --show-cdup to always show absolute workdir path
instead. I think this should hopefully cause no compatibility problems;
the testsuite is passing fine, at least.  The patch also adds
a --show-cdup check and this particular scenartio to the t1500 test.

Signed-off-by: Petr Baudis <redacted>
---

 Documentation/git-rev-parse.txt |    4 ++--
 builtin-rev-parse.c             |   15 +++++----------
 t/t1500-rev-parse.sh            |   18 ++++++++++++++++--
 3 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index 088f971..4c289d0 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt

@@ -103,8 +103,8 @@ OPTIONS
 
 --show-cdup::
 	When the command is invoked from a subdirectory, show the
-	path of the top-level directory relative to the current
-	directory (typically a sequence of "../", or an empty string).
+	path of the top-level directory, or an empty string if the
+	current directory is the top-level directory.
 
 --git-dir::
 	Show `$GIT_DIR` if defined else show the path to the .git directory.

diff --git a/builtin-rev-parse.c b/builtin-rev-parse.c
index a7860ed..011d16c 100644
--- a/builtin-rev-parse.c
+++ b/builtin-rev-parse.c

@@ -500,22 +500,17 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 				continue;
 			}
 			if (!strcmp(arg, "--show-cdup")) {
-				const char *pfx = prefix;
-				if (!is_inside_work_tree()) {
+				if (prefix) {
+					/* We are not at the top level yet */
 					const char *work_tree =
 						get_git_work_tree();
 					if (work_tree)
 						printf("%s\n", work_tree);
 					continue;
+				} else {
+					/* Backwards compatibility */
+					putchar('\n');
 				}
-				while (pfx) {
-					pfx = strchr(pfx, '/');
-					if (pfx) {
-						pfx++;
-						printf("../");
-					}
-				}
-				putchar('\n');
 				continue;
 			}
 			if (!strcmp(arg, "--git-dir")) {

diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh
index 85da4ca..2f0bf15 100755
--- a/t/t1500-rev-parse.sh
+++ b/t/t1500-rev-parse.sh

@@ -26,9 +26,14 @@ test_rev_parse() {
 	"test '$1' = \"\$(git rev-parse --show-prefix)\""
 	shift
 	[ $# -eq 0 ] && return
+
+	test_expect_success "$name: cdup" \
+	"test '$1' = \"\$(git rev-parse --show-cdup)\""
+	shift
+	[ $# -eq 0 ] && return
 }
 
-# label is-bare is-inside-git is-inside-work prefix
+# label is-bare is-inside-git is-inside-work prefix cdup
 
 test_rev_parse toplevel false false true ''
 

@@ -38,11 +43,20 @@ cd objects || exit 1
 test_rev_parse .git/objects/ false true false ''
 cd ../.. || exit 1
 
+basedir=$(pwd)
 mkdir -p sub/dir || exit 1
 cd sub/dir || exit 1
-test_rev_parse subdirectory false false true sub/dir/
+test_rev_parse subdirectory false false true sub/dir/ "$basedir"
 cd ../.. || exit 1
 
+# Scenario: Working within a subdirectory symlinked out of the working tree
+mkdir -p maindir || exit 1
+(mv .git maindir && mkdir -p maindir/sub2 && ln -s maindir/sub2 .) || exit 1
+cd sub2 || exit 1
+test_rev_parse 'symlinked subdirectory' false false true sub2/ "$basedir"/maindir
+cd .. || exit 1
+(rm sub2 && mv maindir/.git . && rm -r maindir) || exit 1
+
 git config core.bare true
 test_rev_parse 'core.bare = true' true false false
 

Hi,

On Tue, 15 Jul 2008, Petr Baudis wrote:

Consider the scenario when someone makes a symlink into a working tree
subdirectory at an unrelated place, then attempts to work inside the
symlinked directory. The scenario is a bit unwieldly, but most of
the Git will handle it fine - except git rev-parse --show-cdup. That
will output a sequence of ../ which will work wrong inside the symlink
using shell cd builtin.

Short version: do not use symlinks in the working directory, if you do not 
want to track the _symlink_.

Long version: there are a lot of problems with that, and --show-cdup is 
the least of the problems.  A checkout, for example, is able to kill the 
symlink and check out a fresh copy of the subdirectory.

AFAICT this is a concious decision: If you want to track a symlink, track 
a symlink, but if you want to track a subdirectory, you will have to track 
a subdirectory, and it cannot be a symlink.

This patch changes --show-cdup to always show absolute workdir path
instead. I think this should hopefully cause no compatibility problems;
the testsuite is passing fine, at least.

See the thread where I proposed a change like this, back with the infamous 
worktree desaster, and Junio NACKed; or the thread where Linus rightfully 
insists that git_dir should be relative if possible, for performance 
reasons.

Hth,
Dscho

  Hi,

On Tue, Jul 15, 2008 at 04:19:30PM +0100, Johannes Schindelin wrote:

On Tue, 15 Jul 2008, Petr Baudis wrote:

quoted

Consider the scenario when someone makes a symlink into a working tree
subdirectory at an unrelated place, then attempts to work inside the
symlinked directory. The scenario is a bit unwieldly, but most of
the Git will handle it fine - except git rev-parse --show-cdup. That
will output a sequence of ../ which will work wrong inside the symlink
using shell cd builtin.

Short version: do not use symlinks in the working directory, if you do not 
want to track the _symlink_.

Long version: there are a lot of problems with that, and --show-cdup is 
the least of the problems.  A checkout, for example, is able to kill the 
symlink and check out a fresh copy of the subdirectory.

AFAICT this is a concious decision: If you want to track a symlink, track 
a symlink, but if you want to track a subdirectory, you will have to track 
a subdirectory, and it cannot be a symlink.

  no, no, this is for the scenario other way around: you have a normal
subdirectory in the working tree, and point a symlink _at_ it from
$somewhere_else. Then you try to work in $somewhere_else/symlink.

quoted

This patch changes --show-cdup to always show absolute workdir path
instead. I think this should hopefully cause no compatibility problems;
the testsuite is passing fine, at least.

See the thread where I proposed a change like this, back with the infamous 
worktree desaster, and Junio NACKed; or the thread where Linus rightfully 
insists that git_dir should be relative if possible, for performance 
reasons.

  I see, [off-list ref]. But noone was aware
of this possible user case. Performance reasons sound reasonable, though
I'm not really sure if for cdup in particular this ever matters.

  P.S.: Either way, there is a possible workaround to tell git about the
working directory manually using git --work-tree=... that I missed to
mention on IRC, Yves.

-- 
				Petr "Pasky" Baudis
GNU, n. An animal of South Africa, which in its domesticated state
resembles a horse, a buffalo and a stag. In its wild condition it is
something like a thunderbolt, an earthquake and a cyclone. -- A. Pierce

On Tue, 2008-07-15 at 17:40 +0200, Petr Baudis wrote:

  no, no, this is for the scenario other way around: you have a normal
subdirectory in the working tree, and point a symlink _at_ it from
$somewhere_else. Then you try to work in $somewhere_else/symlink.

Yes correct. We have a number of different repositories like so:

banana.git/apps
banana.git/lib
orange.git/config
kiwi.git/refdata

and its convenient for many of our existing apps to be able to symlink
them all together into a common tree

joined/apps -> banana.git/apps
joined/lib -> banana.git/lib
joined/config -> orange.git/config
joined/refdata -> kiwi.git/refdata

this way for instance we can swap bits around easily on the fly and say,
restart a webserver or whatever.

Currently we can do this and all our other stuff works, and you
can /mostly/ work with git from the "joined" tree, with the exception of
git pull --rebase and apparently anything else that relies on
--show-cdup

quoted

quoted

This patch changes --show-cdup to always show absolute workdir path
instead. I think this should hopefully cause no compatibility problems;
the testsuite is passing fine, at least.

See the thread where I proposed a change like this, back with the infamous 
worktree desaster, and Junio NACKed; or the thread where Linus rightfully 
insists that git_dir should be relative if possible, for performance 
reasons.

  I see, [off-list ref]. But noone was aware
of this possible user case. Performance reasons sound reasonable, though
I'm not really sure if for cdup in particular this ever matters.

Would it be so bad to detect if the show-cdup actually resolves to the
right place, and if it doesnt go absolute?

  P.S.: Either way, there is a possible workaround to tell git about the
working directory manually using git --work-tree=... that I missed to
mention on IRC, Yves.

Hmm, am i using it wrong then?

[dmq@somewhere apps]$ git-rev-parse --git-dir
/home/dmq/git_tree/main/.git
[dmq@somewhere apps]$ git --work-tree="$(git-rev-parse --git-dir)" pull
--rebase
/usr/bin/git-sh-setup: line 139: cd: .git: No such file or directory
Unable to determine absolute path of git directory

cheers,
yves
ps: not on list, please cc me on replies (sorry for the hassle)

On Tue, 2008-07-15 at 18:41 +0200, Yves Orton wrote:

On Tue, 2008-07-15 at 17:40 +0200, Petr Baudis wrote:

quoted

  P.S.: Either way, there is a possible workaround to tell git about the
working directory manually using git --work-tree=... that I missed to
mention on IRC, Yves.

Hmm, am i using it wrong then?

[dmq@somewhere apps]$ git-rev-parse --git-dir
/home/dmq/git_tree/main/.git
[dmq@somewhere apps]$ git --work-tree="$(git-rev-parse --git-dir)" pull
--rebase
/usr/bin/git-sh-setup: line 139: cd: .git: No such file or directory
Unable to determine absolute path of git directory

Hmm, realizing that was the workdir it wanted i tried it like so:

[dmq@somewhere apps]$ git --work-tree="$(git-rev-parse --git-dir)/.."
pull --rebase
/usr/bin/git-sh-setup: line 139: cd: /home/dmq/git_tree/main/apps/.git:
No such file or directory
Unable to determine absolute path of git directory

Yet:

[dmq@somewhere apps]$ git-rev-parse --git-dir
/home/dmq/git_tree/main/.git

is correct.

cheers,
yves
ps: not on list, please cc me on replies (sorry for the hassle)

Yves Orton wrote:

Hmm, realizing that was the workdir it wanted i tried it like so:

[dmq@somewhere apps]$ git --work-tree="$(git-rev-parse --git-dir)/.."
pull --rebase
/usr/bin/git-sh-setup: line 139: cd: /home/dmq/git_tree/main/apps/.git:
No such file or directory
Unable to determine absolute path of git directory

Yet:

[dmq@somewhere apps]$ git-rev-parse --git-dir
/home/dmq/git_tree/main/.git

is correct.

Are you sure you don't want to specify the --git-dir rather than the 
work dir?

i.e.

git --git-dir="$(git-rev-parse --git-dir)" pull --rebase

Rogan

On Tue, 2008-07-15 at 21:08 +0200, Rogan Dawes wrote:

Yves Orton wrote:

quoted

Hmm, realizing that was the workdir it wanted i tried it like so:

[dmq@somewhere apps]$ git --work-tree="$(git-rev-parse --git-dir)/.."
pull --rebase
/usr/bin/git-sh-setup: line 139: cd: /home/dmq/git_tree/main/apps/.git:
No such file or directory
Unable to determine absolute path of git directory

Yet:

[dmq@somewhere apps]$ git-rev-parse --git-dir
/home/dmq/git_tree/main/.git

is correct.

Are you sure you don't want to specify the --git-dir rather than the 
work dir?

i.e.

git --git-dir="$(git-rev-parse --git-dir)" pull --rebase

That doesnt seem to work correctly either. If i do it from the symlinked
directory i get a notice about each file needing an update. While it
works as expected from the real repo directory.

I think this shows what i mean:

demerphq@gemini:~/git_test/bar$ git status
# On branch master
# Changed but not updated:
#   (use "git add <file>..." to update what will be committed)
#
#       modified:   bar
#
no changes added to commit (use "git add" and/or "git commit -a")
demerphq@gemini:~/git_test/bar$ git commit -a -m'changed bar'
Created commit 7cbbdc9: changed bar
 1 files changed, 1 insertions(+), 0 deletions(-)
demerphq@gemini:~/git_test/bar$ git --git-dir="$(git-rev-parse
--git-dir)" pull --rebase
bar/bar: needs update
refusing to pull with rebase: your working tree is not up-to-date
demerphq@gemini:~/git_test/bar$ cd ../foo2
demerphq@gemini:~/git_test/foo2$ git --git-dir="$(git-rev-parse
--git-dir)" pull --rebase
Current branch master is up to date.
demerphq@gemini:~/git_test/foo2$ cd ..
demerphq@gemini:~/git_test$ ls -lart
total 24
drwxr-xr-x   4 demerphq demerphq  4096 2008-07-15 22:17 foo
drwxr-xr-x 116 demerphq demerphq 12288 2008-07-15 22:18 ..
lrwxrwxrwx   1 demerphq demerphq     8 2008-07-15 22:20 bar -> foo2/bar
drwxr-xr-x   4 demerphq demerphq  4096 2008-07-15 22:20 .
drwxr-xr-x   4 demerphq demerphq  4096 2008-07-15 22:21 foo2



Yves