threads / patch / 18937

patchGit.pm: Set GIT_WORK_TREE if we set GIT_DIR

Subject: [PATCH] Git.pm: Set GIT_WORK_TREE if we set GIT_DIR

## tl;dr

4 messages between Apr 18, 2009 and Apr 18, 2009. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Frank Lichtenheld· Apr 18, 2009, 15:58 UTC · lore
From: Frank Lichtenheld <flichtenheld@astaro.com>

Otherwise git will use the current directory as work tree which will lead to unexpected results if we operate in sub directory of the work tree.

Signed-off-by: Frank Lichtenheld <flichtenheld@astaro.com>
---
 perl/Git.pm         |    2 ++
 t/t9700-perl-git.sh |    4 ++++
 t/t9700/test.pl     |   13 +++++++++++++
 3 files changed, 19 insertions(+), 0 deletions(-)

One example for the unexpected results is that "git log ." doesn't work correctly, as demonstrated by my test.

Show changes to 3 files +19 −0

perl/Git.pm, t/t9700-perl-git.sh, t/t9700/test.pl

diff --git a/perl/Git.pm b/perl/Git.pm
index 291ff5b..4313db7 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -1280,6 +1280,8 @@ sub _cmd_exec {
 	my ($self, @args) = @_;
 	if ($self) {
 		$self->repo_path() and $ENV{'GIT_DIR'} = $self->repo_path();
+		$self->repo_path() and $self->wc_path()
+			and $ENV{'GIT_WORK_TREE'} = $self->wc_path();
 		$self->wc_path() and chdir($self->wc_path());
 		$self->wc_subdir() and chdir($self->wc_subdir());
 	}
diff --git a/t/t9700-perl-git.sh b/t/t9700-perl-git.sh
index b4ca244..4eb7d3f 100755
--- a/t/t9700-perl-git.sh
+++ b/t/t9700-perl-git.sh
@@ -29,6 +29,10 @@ test_expect_success \
      git add . &&
      git commit -m "first commit" &&
 
+     echo "new file in subdir 2" > directory2/file2 &&
+     git add . &&
+     git commit -m "commit in directory2" &&
+
      echo "changed file 1" > file1 &&
      git commit -a -m "second commit" &&
 
diff --git a/t/t9700/test.pl b/t/t9700/test.pl
index 697daf3..d9b29ea 100755
--- a/t/t9700/test.pl
+++ b/t/t9700/test.pl
@@ -98,3 +98,16 @@ TODO: {
 	todo_skip 'config after wc_chdir', 1;
 	is($r->config("color.string"), "value", "config after wc_chdir");
 }
+
+# Object generation in sub directory
+chdir("directory2");
+my $r2 = Git->repository();
+is($r2->repo_path, $abs_repo_dir . "/.git", "repo_path (2)");
+is($r2->wc_path, $abs_repo_dir . "/", "wc_path (2)");
+is($r2->wc_subdir, "directory2/", "wc_subdir initial (2)");
+
+# commands in sub directory
+my $last_commit = $r2->command_oneline(qw(rev-parse --verify HEAD));
+like($last_commit, qr/^[0-9a-fA-F]{40}$/, 'rev-parse returned hash');
+my $dir_commit = $r2->command_oneline('log', '-n1', '--pretty=format:%H', '.');
+isnt($last_commit, $dir_commit, 'log . does not show last commit');
-- 
1.6.2.1
Frank Lichtenheld· Apr 18, 2009, 15:58 UTC · re: Frank Lichtenheld · lore

[PATCH] Git.pm: Always set Repository to absolute path if autodetecting

From: Frank Lichtenheld <flichtenheld@astaro.com>

So far we only set it to absolute paths in some cases which lead to problems like wc_chdir not working.

Signed-off-by: Frank Lichtenheld <flichtenheld@astaro.com>
---
 perl/Git.pm     |    2 +-
 t/t9700/test.pl |   10 ++--------
 2 files changed, 3 insertions(+), 9 deletions(-)
Show changes to 2 files +3 −9

perl/Git.pm, t/t9700/test.pl

diff --git a/perl/Git.pm b/perl/Git.pm
index 4313db7..e8df55d 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -185,7 +185,7 @@ sub repository {
 
 		if ($dir) {
 			$dir =~ m#^/# or $dir = $opts{Directory} . '/' . $dir;
-			$opts{Repository} = $dir;
+			$opts{Repository} = abs_path($dir);
 
 			# If --git-dir went ok, this shouldn't die either.
 			my $prefix = $search->command_oneline('rev-parse', '--show-prefix');
diff --git a/t/t9700/test.pl b/t/t9700/test.pl
index d9b29ea..6c70aec 100755
--- a/t/t9700/test.pl
+++ b/t/t9700/test.pl
@@ -86,18 +86,12 @@ close TEMPFILE;
 unlink $tmpfile;
 
 # paths
-is($r->repo_path, "./.git", "repo_path");
+is($r->repo_path, $abs_repo_dir . "/.git", "repo_path");
 is($r->wc_path, $abs_repo_dir . "/", "wc_path");
 is($r->wc_subdir, "", "wc_subdir initial");
 $r->wc_chdir("directory1");
 is($r->wc_subdir, "directory1", "wc_subdir after wc_chdir");
-TODO: {
-	local $TODO = "commands do not work after wc_chdir";
-	# Failure output is active even in non-verbose mode and thus
-	# annoying.  Hence we skip these tests as long as they fail.
-	todo_skip 'config after wc_chdir', 1;
-	is($r->config("color.string"), "value", "config after wc_chdir");
-}
+is($r->config("test.string"), "value", "config after wc_chdir");
 
 # Object generation in sub directory
 chdir("directory2");
-- 
1.6.2.1
Johannes Schindelin· Apr 18, 2009, 18:13 UTC · re: Frank Lichtenheld · lore

Re: [PATCH] Git.pm: Always set Repository to absolute path if autodetecting

Hi,
On Sat, 18 Apr 2009, Frank Lichtenheld wrote:
> So far we only set it to absolute paths in some cases which lead to 
> problems like wc_chdir not working.

We had something similar in setup.c until Linus pointed out that it deteriorates performance.

Ciao, Dscho

Frank Lichtenheld· Apr 18, 2009, 21:02 UTC · re: Johannes Schindelin · lore

Re: [PATCH] Git.pm: Always set Repository to absolute path if autodetecting

On Sat, Apr 18, 2009 at 08:13:05PM +0200, Johannes Schindelin wrote:
Show 7 quoted lines
> On Sat, 18 Apr 2009, Frank Lichtenheld wrote:
> 
> > So far we only set it to absolute paths in some cases which lead to 
> > problems like wc_chdir not working.
> 
> We had something similar in setup.c until Linus pointed out that it 
> deteriorates performance.
You mean 044bbbcb63281dfdb78344ada2c44c96122dc822 ?

I don't think we're talking about the same issue here. The code in setup.c prepares a variable for usage inside the running git command, while the Perl code needs to prepare a variable for calling a git command, i.e. the content of the GIT_DIR environment variable. As you can see from the code in setup.c git will in fact make that content absolute before making it relative again, so no harm should come from the fact that we're giving it an already absolute value.

Also if we're giving it a relative path it will interpret it relative to the current working directory and not GIT_WORK_TREE, so we should in fact not store it relative because we would need to recalculate it after wc_chdir, which is exactly what's causing problems.

Gruesse,
-- 
Frank Lichtenheld <frank@lichtenheld.de>
www: http://www.djpig.de/

← back to recent threads