patchgrep: simple test for operation in a bare repository

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 t/t7002-grep.sh |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)

Am 03.02.2010 19:16, schrieb René Scharfe:

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:

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.

On Fri, Feb 5, 2010 at 7:24 AM, René Scharfe <rene.scharfe@lsrfire.ath.cx> wrote:

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.

Yes, they should.

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

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 <rene.scharfe@lsrfire.ath.cx>
---
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(-)

René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:

Thanks for a thorough analysis. The patch looks correct and I am very tempted to put it into 1.7.0, though I'd stop at queuing it to 'next'.

René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:

Yeah, shouldn't the other setenv() we see in the context need a similar change as well?