[PATCH] grep: simple test for operation in a bare repository

Signed-off-by: Rene Scharfe <redacted>
---
 t/t7002-grep.sh |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/t/t7002-grep.sh b/t/t7002-grep.sh
index bf4d4dc..8cf958d 100755
--- a/t/t7002-grep.sh
+++ b/t/t7002-grep.sh

@@ -45,6 +45,23 @@ test_expect_success 'grep should not segfault with a bad input' '
 	test_must_fail git grep "("
 '
 
+bare_repo=.git/bare_test_repo
+test_expect_success 'setup bare repo' '
+	git clone --bare . $bare_repo
+'
+
+test_expect_success "grep HEAD (t-1), bare repo" '(
+	cd $bare_repo &&
+	echo "HEAD:t/t:1:test" >expected &&
+	git grep -n -e test HEAD >actual &&
+	diff expected actual
+)'
+
+test_expect_success "grep (t-1), bare repo, must fail" '(
+	cd $bare_repo &&
+	test_must_fail git grep -n -e test
+)'
+
 for H in HEAD ''
 do
 	case "$H" in

-- 
1.7.0.rc1

Am 03.02.2010 19:16, schrieb René Scharfe:

Signed-off-by: Rene Scharfe <redacted>
---
 t/t7002-grep.sh |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)

Err, no, that won't do.  Sorry.

The test script fails to demonstrate the issue I've run into.  It runs
successfully, but running git grep manually fails:

	$ cd t/trash\ directory.t7002-grep/.git/bare_test_repo/
	$ git grep bla HEAD
	fatal: This operation must be run in a work tree

I have to dig a bit deeper and try to come back with a better test script.

René

Am 04.02.2010 00:50, schrieb René Scharfe:

Am 03.02.2010 19:16, schrieb René Scharfe:

quoted

Signed-off-by: Rene Scharfe <redacted>
---
 t/t7002-grep.sh |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)

Err, no, that won't do.  Sorry.

The test script fails to demonstrate the issue I've run into.  It runs
successfully, but running git grep manually fails:

	$ cd t/trash\ directory.t7002-grep/.git/bare_test_repo/
	$ git grep bla HEAD
	fatal: This operation must be run in a work tree

I have to dig a bit deeper and try to come back with a better test script.

OK, I have to admit defeat: I can't come up with a test script.  But
the issue is reproducible: git grep in a bare repository fails when
run with a pager.

	$ mkdir /tmp/a
	$ cd /tmp/a
	$ git init
	Initialized empty Git repository in /tmp/a/.git/
	$ echo a >a
	$ git add a
	$ git commit -m.
	[master (root-commit) e11f955] .
	 1 files changed, 1 insertions(+), 0 deletions(-)
	 create mode 100644 a

	$ git clone --bare . ../b
	Initialized empty Git repository in /tmp/b/
	$ cd /tmp/b

	$ git grep a HEAD
	fatal: This operation must be run in a work tree
	$ git grep a HEAD | cat
	HEAD:a:a
	$ git --no-pager grep a HEAD
	HEAD:a:a

Reverting 7e622650 (grep: prepare to run outside of a work tree), or
rather just setting the flag RUN_SETUP for grep in git.c again, makes
the first git grep call succeed, too.

As does the following patch, but I don't know why.  The call chain is
quite deep.  It seems that without the patch the static variable
git_dir in environment.c isn't updated when git finds out that it runs
in a bare repo -- but only if a pager is used.

There are five more sites in git.c, path.c and setup.c where $GIT_DIR
is set directly with setenv().  I wonder if they should better call
set_git_dir() instead, too.

diff --git a/setup.c b/setup.c
index 710e2f3..5fb9b25 100644
--- a/setup.c
+++ b/setup.c

@@ -406,7 +406,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
 				cwd[offset] = '\0';
 				setenv(GIT_DIR_ENVIRONMENT, cwd, 1);
 			} else
-				setenv(GIT_DIR_ENVIRONMENT, ".", 1);
+				set_git_dir(".");
 			check_repository_format_gently(nongit_ok);
 			return NULL;
 		}

On Fri, Feb 5, 2010 at 7:24 AM, René Scharfe
[off-list ref] wrote:

OK, I have to admit defeat: I can't come up with a test script.  But
the issue is reproducible: git grep in a bare repository fails when
run with a pager.

       $ mkdir /tmp/a
       $ cd /tmp/a
       $ git init
       Initialized empty Git repository in /tmp/a/.git/
       $ echo a >a
       $ git add a
       $ git commit -m.
       [master (root-commit) e11f955] .
        1 files changed, 1 insertions(+), 0 deletions(-)
        create mode 100644 a

       $ git clone --bare . ../b
       Initialized empty Git repository in /tmp/b/
       $ cd /tmp/b

       $ git grep a HEAD
       fatal: This operation must be run in a work tree
       $ git grep a HEAD | cat
       HEAD:a:a
       $ git --no-pager grep a HEAD
       HEAD:a:a

Reverting 7e622650 (grep: prepare to run outside of a work tree), or
rather just setting the flag RUN_SETUP for grep in git.c again, makes
the first git grep call succeed, too.

As does the following patch, but I don't know why.  The call chain is
quite deep.  It seems that without the patch the static variable
git_dir in environment.c isn't updated when git finds out that it runs
in a bare repo -- but only if a pager is used.

setup_pager() calls git_config(), which indirectly calls get_git_dir()
and sets git_dir in stone. Changing GIT_DIR environment variable alone
won't work, as you have seen.

When RUN_SETUP is set, setup_git_directory() would be called before
setup_pager() can kick in, so everything is properly set.

There are five more sites in git.c, path.c and setup.c where $GIT_DIR
is set directly with setenv().  I wonder if they should better call
set_git_dir() instead, too.

Yes, they should.
-- 
Duy

Am 05.02.2010 03:40, schrieb Nguyen Thai Ngoc Duy:

setup_pager() calls git_config(), which indirectly calls get_git_dir()
and sets git_dir in stone. Changing GIT_DIR environment variable alone
won't work, as you have seen.

When RUN_SETUP is set, setup_git_directory() would be called before
setup_pager() can kick in, so everything is properly set.

quoted

There are five more sites in git.c, path.c and setup.c where $GIT_DIR
is set directly with setenv().  I wonder if they should better call
set_git_dir() instead, too.

Yes, they should.

This patch converts the setenv() calls in path.c and setup.c.  After
the call, git grep with a pager works again in bare repos.

It leaves the setenv(GIT_DIR_ENVIRONMENT, ...) calls in git.c alone, as
they respond to command line switches that emulate the effect of setting
the environment variable directly.

The remaining site in environment.c is in set_git_dir() and is left
alone, too, of course.  Finally, builtin-init-db.c is left changed
because the repo is still being carefully constructed when the
environment variable is set.

This fixes git shortlog when run inside a git directory, which had been
broken by abe549e1.

Signed-off-by: Rene Scharfe <redacted>
---
Since it's doesn't fix a regression (abe549e1 was committed in March
2008), this patch doesn't have to go in at this point in the release
cycle.  And perhaps it's even superseded by the more general fix Duy
is working on?

 path.c  |    2 +-
 setup.c |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/path.c b/path.c
index 79aa104..0005df3 100644
--- a/path.c
+++ b/path.c

@@ -336,7 +336,7 @@ char *enter_repo(char *path, int strict)
  	if (access("objects", X_OK) == 0 && access("refs", X_OK) == 0 &&
 	    validate_headref("HEAD") == 0) {
-		setenv(GIT_DIR_ENVIRONMENT, ".", 1);
+		set_git_dir(".");
 		check_repository_format();
 		return path;
 	}

diff --git a/setup.c b/setup.c
index 710e2f3..b38cbee 100644
--- a/setup.c
+++ b/setup.c

@@ -404,9 +404,9 @@ const char *setup_git_directory_gently(int *nongit_ok)
 				inside_work_tree = 0;
 			if (offset != len) {
 				cwd[offset] = '\0';
-				setenv(GIT_DIR_ENVIRONMENT, cwd, 1);
+				set_git_dir(cwd);
 			} else
-				setenv(GIT_DIR_ENVIRONMENT, ".", 1);
+				set_git_dir(".");
 			check_repository_format_gently(nongit_ok);
 			return NULL;
 		}