git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* git-rev-parse --show-cdup returns a relative path instead of absolute (problem with git pull --rebase not finding the git dir)
@ 2008-07-15 14:13 Yves Orton
  2008-07-15 14:59 ` [PATCH] git rev-parse: Fix --show-cdup inside symlinked directory Petr Baudis
  0 siblings, 1 reply; 8+ messages in thread
From: Yves Orton @ 2008-07-15 14:13 UTC (permalink / raw)
  To: git

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH] git rev-parse: Fix --show-cdup inside symlinked directory
  2008-07-15 14:13 git-rev-parse --show-cdup returns a relative path instead of absolute (problem with git pull --rebase not finding the git dir) Yves Orton
@ 2008-07-15 14:59 ` Petr Baudis
  2008-07-15 15:19   ` Johannes Schindelin
  0 siblings, 1 reply; 8+ messages in thread
From: Petr Baudis @ 2008-07-15 14:59 UTC (permalink / raw)
  To: gitster; +Cc: git

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 <pasky@suse.cz>
---

 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
 

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] git rev-parse: Fix --show-cdup inside symlinked directory
  2008-07-15 14:59 ` [PATCH] git rev-parse: Fix --show-cdup inside symlinked directory Petr Baudis
@ 2008-07-15 15:19   ` Johannes Schindelin
  2008-07-15 15:40     ` Petr Baudis
  0 siblings, 1 reply; 8+ messages in thread
From: Johannes Schindelin @ 2008-07-15 15:19 UTC (permalink / raw)
  To: Petr Baudis; +Cc: gitster, git

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] git rev-parse: Fix --show-cdup inside symlinked directory
  2008-07-15 15:19   ` Johannes Schindelin
@ 2008-07-15 15:40     ` Petr Baudis
  2008-07-15 16:41       ` Yves Orton
  0 siblings, 1 reply; 8+ messages in thread
From: Petr Baudis @ 2008-07-15 15:40 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: gitster, git, yves.orton

  Hi,

On Tue, Jul 15, 2008 at 04:19:30PM +0100, Johannes Schindelin wrote:
> 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.

  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.

> > 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, <7vk5sly3h9.fsf@assigned-by-dhcp.cox.net>. 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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] git rev-parse: Fix --show-cdup inside symlinked directory
  2008-07-15 15:40     ` Petr Baudis
@ 2008-07-15 16:41       ` Yves Orton
  2008-07-15 16:58         ` Yves Orton
  0 siblings, 1 reply; 8+ messages in thread
From: Yves Orton @ 2008-07-15 16:41 UTC (permalink / raw)
  To: Petr Baudis; +Cc: Johannes Schindelin, gitster, git

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

> > > 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, <7vk5sly3h9.fsf@assigned-by-dhcp.cox.net>. 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)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] git rev-parse: Fix --show-cdup inside symlinked directory
  2008-07-15 16:41       ` Yves Orton
@ 2008-07-15 16:58         ` Yves Orton
  2008-07-15 19:08           ` Rogan Dawes
  0 siblings, 1 reply; 8+ messages in thread
From: Yves Orton @ 2008-07-15 16:58 UTC (permalink / raw)
  To: Petr Baudis; +Cc: Johannes Schindelin, gitster, git

On Tue, 2008-07-15 at 18:41 +0200, Yves Orton wrote:
> On Tue, 2008-07-15 at 17:40 +0200, Petr Baudis wrote:
> > 
> >   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)
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] git rev-parse: Fix --show-cdup inside symlinked   directory
  2008-07-15 16:58         ` Yves Orton
@ 2008-07-15 19:08           ` Rogan Dawes
  2008-07-15 20:26             ` Yves Orton
  0 siblings, 1 reply; 8+ messages in thread
From: Rogan Dawes @ 2008-07-15 19:08 UTC (permalink / raw)
  To: Yves Orton; +Cc: Petr Baudis, Johannes Schindelin, gitster, git

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] git rev-parse: Fix --show-cdup inside symlinked   directory
  2008-07-15 19:08           ` Rogan Dawes
@ 2008-07-15 20:26             ` Yves Orton
  0 siblings, 0 replies; 8+ messages in thread
From: Yves Orton @ 2008-07-15 20:26 UTC (permalink / raw)
  To: Rogan Dawes; +Cc: Petr Baudis, Johannes Schindelin, gitster, git

On Tue, 2008-07-15 at 21:08 +0200, Rogan Dawes wrote:
> 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

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2008-07-15 20:27 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-07-15 14:13 git-rev-parse --show-cdup returns a relative path instead of absolute (problem with git pull --rebase not finding the git dir) Yves Orton
2008-07-15 14:59 ` [PATCH] git rev-parse: Fix --show-cdup inside symlinked directory Petr Baudis
2008-07-15 15:19   ` Johannes Schindelin
2008-07-15 15:40     ` Petr Baudis
2008-07-15 16:41       ` Yves Orton
2008-07-15 16:58         ` Yves Orton
2008-07-15 19:08           ` Rogan Dawes
2008-07-15 20:26             ` Yves Orton

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