git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
John Keeping <john@keeping.me.uk>
Date
Mar 29, 2016, 22:14 UTC
Message-ID
<20160329221409.GH1578@serenity.lan>
In-Reply-To
<20160329220003.GG1578@serenity.lan>
On Tue, Mar 29, 2016 at 11:00:03PM +0100, John Keeping wrote:
Show 75 quoted lines
> On Tue, Mar 29, 2016 at 05:21:43PM -0400, Jeff King wrote:
> > 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...
> 
> > > 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

It seems that this is a result of changing the working directory to the root of the working tree if we're inside it. is_inside_dir() doesn't take account of startup_info->prefix and changing to:

	real_path(startup_info->prefix)
instead of xgetcwd() means that these tests are less surprising.

But I haven't run the test suite or thought about what else this could break.

Previous: John KeepingNext: Jeff King
Message 12 of 16 in “`git rev-parse --is-inside-work-tree` and $GIT_WORK_TREE”
  1. Elliott CableMar 29, 2016
  2. Elliott CableMar 29, 2016
  3. John KeepingMar 29, 2016
  4. Junio C HamanoMar 29, 2016
  5. Jeff KingMar 29, 2016
  6. Junio C HamanoMar 29, 2016
  7. Jeff KingMar 29, 2016
  8. Jeff KingMar 29, 2016
  9. John KeepingMar 29, 2016
  10. Jeff KingMar 29, 2016
  11. John KeepingMar 29, 2016
  12. John KeepingMar 29, 2016
  13. Jeff KingMar 29, 2016
  14. Junio C HamanoMar 29, 2016
  15. Duy NguyenMar 30, 2016
  16. Elliott CableApr 1, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.