Re: `git rev-parse --is-inside-work-tree` and $GIT_WORK_TREE

From: John Keeping <john@keeping.me.uk>
Date: Mar 29, 2016, 22:00 UTC
Message-ID: <20160329220003.GG1578@serenity.lan>
In-Reply-To: <20160329212143.GA30116@sigill.intra.peff.net>

On Tue, Mar 29, 2016 at 05:21:43PM -0400, Jeff King wrote:

Show 40 quoted lines

> On Tue, Mar 29, 2016 at 09:52:08PM +0100, John Keeping wrote:
> 
> > > Yeah, I think this is a bug. Presumably what is happening is that we are
> > > too eager to "cd $GIT_WORK_TREE" inside git-rev-parse, and by the time
> > > we ask "are we in a work tree", the answer has become yes. But the
> > > caller really wants to know "am _I_ inside the work tree".
> > 
> > I don't think that's what's happening.  Try:
> > 
> > 	$ cd .git/
> > 	$ GIT_WORK_TREE=.. git rev-parse --is-inside-work-tree
> > 	true
> > 
> > so I think it's that we refuse to assume that the directory above a Git
> > directory is a working tree (something similar happens when the
> > "core.worktree" config variable is set).  I'm not convinced that's
> > unreasonable.
> 
> Yeah, you're right, but I'm not sure how your example shows that, (isn't
> it basically the same as Elliott's original, except using a relative
> path?). A more compelling counter-example to my hypothesis is:
> 
>   $ cd .git
>   $ GIT_WORK_TREE=/tmp git rev-parse --is-inside-work-tree
>   false
> 
> So it is not that we chdir too early, but just that we blindly check "is
> $(pwd) inside $GIT_WORK_TREE". And it does not create a problem for the
> normal discovered-path cases, because either:
> 
>   - we discovered .git by walking up the directory tree, which means we
>     must be in a work-tree
> 
>   - we discovered that we are inside a .git directory, and therefore
>     take it to be bare (and thus there is no work tree, and we cannot be
>     inside it). This is what happens in Elliott's original example that
>     behaves differently than the $GIT_WORK_TREE case.
> 
> I'd be tempted to say that "inside the work tree" is further clarified
> to "not inside the $GIT_DIR".

Yes, I think that's reasonable.  But...

Show 17 quoted lines

> > However, the case above also gives:
> > 
> > 	$ GIT_WORK_TREE=.. git rev-parse --is-inside-git-dir
> > 	false
> > 	$ test $(pwd) = $(GIT_WORK_TREE=.. git rev-parse --git-dir); echo $?
> > 	0
> > 
> > so even though $PWD *is* the Git directory, we're not in the Git
> > directory!  Setting GIT_DIR=$(pwd) makes no different to that.
> 
> We seem to get that wrong. I'm also not sure if it would make sense if
> you explicitly set the two to be equal, like:
> 
>   # checking in your own refs?
>   GIT_WORK_TREE=$(pwd) GIT_DIR=$(pwd) git add refs packed-refs
> 
> So the current behavior may just be weird-but-true.

This case definitely feels wrong:

	$ GIT_WORK_TREE=$(cd ..; pwd) GIT_DIR=$(pwd) git rev-parse --is-inside-git-dir
	false

Shouldn't that be the same as if GIT_WORK_TREE and GIT_DIR aren't set? (It's also potentially surprising since "git rev-parse --git-dir" does give the right answer in this case.)

If GIT_WORK_TREE points somewhere unrelated then it is correct:

	$ GIT_WORK_TREE=/tmp GIT_DIR=$(pwd) git rev-parse --is-inside-git-dir
	true