git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: John Keeping <john@keeping.me.uk>
To: Jeff King <peff@peff.net>
Cc: "Elliott Cable" <me@ell.io>,
	"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>,
	git@vger.kernel.org
Subject: Re: `git rev-parse --is-inside-work-tree` and $GIT_WORK_TREE
Date: Tue, 29 Mar 2016 23:00:03 +0100	[thread overview]
Message-ID: <20160329220003.GG1578@serenity.lan> (raw)
In-Reply-To: <20160329212143.GA30116@sigill.intra.peff.net>

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

  reply	other threads:[~2016-03-29 22:00 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-29 11:42 `git rev-parse --is-inside-work-tree` and $GIT_WORK_TREE Elliott Cable
2016-03-29 11:53 ` Elliott Cable
2016-03-29 12:33   ` John Keeping
2016-03-29 15:08     ` Junio C Hamano
2016-03-29 19:41       ` Jeff King
2016-03-29 19:56         ` Junio C Hamano
2016-03-29 20:26           ` Jeff King
2016-03-29 20:34 ` Jeff King
2016-03-29 20:52   ` John Keeping
2016-03-29 21:21     ` Jeff King
2016-03-29 22:00       ` John Keeping [this message]
2016-03-29 22:14         ` John Keeping
2016-03-29 22:16         ` Jeff King
2016-03-29 22:35           ` Junio C Hamano
2016-03-30  0:53   ` Duy Nguyen
2016-04-01  0:49     ` Elliott Cable

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160329220003.GG1578@serenity.lan \
    --to=john@keeping.me.uk \
    --cc=git@vger.kernel.org \
    --cc=me@ell.io \
    --cc=pclouds@gmail.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).