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

From: Nguyen Thai Ngoc Duy <pclouds@gmail.com>
Date: Feb 5, 2010, 02:40 UTC
Message-ID: <fcaeb9bf1002041840l4d8e8ac9k3ad5e9e8761aa1b@mail.gmail.com>
In-Reply-To: <4B6B653A.8060909@lsrfire.ath.cx>

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

Show 34 quoted lines

> 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