* `git rev-parse --is-inside-work-tree` and $GIT_WORK_TREE
@ 2016-03-29 11:42 Elliott Cable
2016-03-29 11:53 ` Elliott Cable
2016-03-29 20:34 ` Jeff King
0 siblings, 2 replies; 16+ messages in thread
From: Elliott Cable @ 2016-03-29 11:42 UTC (permalink / raw)
To: git
So, I find this behaviour a little strange; I can't determine if it's
a subtle bug, or intentionally undefined/‘fuzzy’ behaviour:
$ cd a-repo/.git/
$ pwd
/path/to/a-repo/.git
$ git rev-parse --is-inside-work-tree
false
$ export GIT_WORK_TREE=/path/to/a-repo
$ git rev-parse --is-inside-work-tree
true
i.e. when within the repository (the `.git` directory), and when that
directory is a sub-directory of the working-tree, `rev-parse
--is-inside-work-tree` reports *false* (reasonable enough, I suppose);
but then if `$GIT_WORK_TREE` is set to precisely the directory that
git was *already* assuming was the working-directory, then the same
command, in the same location, reports *true*.
This should probably be made consistent: either `rev-parse
--is-inside-work-tree` should report “true”, even inside the `.git`
dir, as long as that directory is a sub-directory of the working-tree
… or repository-directories / `$GIT_DIR` / `.git` directories should
be excluded from truthy responses to `rev-parse
--is-inside-work-tree`.
⁓ ELLIOTTCABLE — fly safe.
http://ell.io/tt
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: `git rev-parse --is-inside-work-tree` and $GIT_WORK_TREE
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 20:34 ` Jeff King
1 sibling, 1 reply; 16+ messages in thread
From: Elliott Cable @ 2016-03-29 11:53 UTC (permalink / raw)
To: git
On Tue, Mar 29, 2016 at 6:42 AM, Elliott Cable <me@ell.io> wrote:
> So, I find this behaviour a little strange; I can't determine if it's
> a subtle bug, or intentionally undefined/‘fuzzy’ behaviour ...
Oh lord, it gets worse ...
$ cd a-repo
$ git rev-parse --is-inside-work-tree; git rev-parse --is-inside-git-dir
true
false
$ cd .git
$ git rev-parse --is-inside-work-tree; git rev-parse --is-inside-git-dir
false
true
$ export GIT_WORK_TREE="$(git rev-parse --show-toplevel)" # !!!
$ git rev-parse --is-inside-work-tree; git rev-parse --is-inside-git-dir
true
false
$ # !!?!?
So, basically, if `$GIT_WORK_TREE` is set at all, it appears that the
`rev-parse --is-inside...` flags don't function reliably at all.
⁓ ELLIOTTCABLE — fly safe.
http://ell.io/tt
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: `git rev-parse --is-inside-work-tree` and $GIT_WORK_TREE
2016-03-29 11:53 ` Elliott Cable
@ 2016-03-29 12:33 ` John Keeping
2016-03-29 15:08 ` Junio C Hamano
0 siblings, 1 reply; 16+ messages in thread
From: John Keeping @ 2016-03-29 12:33 UTC (permalink / raw)
To: Elliott Cable; +Cc: git
On Tue, Mar 29, 2016 at 06:53:35AM -0500, Elliott Cable wrote:
> On Tue, Mar 29, 2016 at 6:42 AM, Elliott Cable <me@ell.io> wrote:
> > So, I find this behaviour a little strange; I can't determine if it's
> > a subtle bug, or intentionally undefined/‘fuzzy’ behaviour ...
>
> Oh lord, it gets worse ...
>
> $ cd a-repo
> $ git rev-parse --is-inside-work-tree; git rev-parse --is-inside-git-dir
> true
> false
> $ cd .git
> $ git rev-parse --is-inside-work-tree; git rev-parse --is-inside-git-dir
> false
> true
I believe these are working correctly, the .git directory is not part of
the working tree.
> $ export GIT_WORK_TREE="$(git rev-parse --show-toplevel)" # !!!
Did you check the value of GIT_WORK_TREE here? When I try it's the
empty string.
If I set the core.worktree config variable to ".." then rev-parse does
find the working tree correctly. I recall some previous discussion
about this but I can't find it in the list archives from a quick search.
> $ git rev-parse --is-inside-work-tree; git rev-parse --is-inside-git-dir
> true
> false
> $ # !!?!?
>
> So, basically, if `$GIT_WORK_TREE` is set at all, it appears that the
> `rev-parse --is-inside...` flags don't function reliably at all.
If you set GIT_WORK_TREE you're telling Git to override all of the
normal detection logic. What version of Git are you using? When I try
this it says:
fatal: The empty string is not a valid path
If I set GIT_WORK_TREE to the correct value for this repository then it
behaves the same as with the auto-detection logic.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: `git rev-parse --is-inside-work-tree` and $GIT_WORK_TREE
2016-03-29 12:33 ` John Keeping
@ 2016-03-29 15:08 ` Junio C Hamano
2016-03-29 19:41 ` Jeff King
0 siblings, 1 reply; 16+ messages in thread
From: Junio C Hamano @ 2016-03-29 15:08 UTC (permalink / raw)
To: John Keeping; +Cc: Elliott Cable, git
John Keeping <john@keeping.me.uk> writes:
> If you set GIT_WORK_TREE you're telling Git to override all of the
> normal detection logic.
I didn't carefully read the other parts of the discussion, but this
is not entirely correct.
The actual/intended rules are fairly simple.
* With GIT_DIR (without GIT_WORK_TREE), you specify where the .git
directory is, and refuse the "normal detection logic". As you
refuse the normal "where is the root of the working tree and its
.git directory?" logic, Git uses the current directory as the
root of the working tree.
* But that means if you set GIT_DIR, you cannot work from a
subdirectory of your working tree--you always have to run Git
from the root level of your working tree. This may be
inconvenient.
* That is why GIT_WORK_TREE exists. It allows you to say "No, I am
not at the root level but am in a subdirectory somewhere inside
the working tree. The root level of the working tree is there
above, not here".
So it is a misconfiguration if you only set GIT_WORK_TREE without
setting GIT_DIR.
Also, if you set both and run Git from outside $GIT_WORK_TREE, even
though Git may try to do its best to give you a reasonable behaviour
[*1*], it is working by accident not by design (see the statement
you are making by setting GIT_WORK_TREE in the third bullet above).
IOW, running from outside GIT_WORK_TREE is a misconfiguration.
[Footnote]
*1* Think what should happen when you are outside GIT_WORK_TREE and
say this:
$ git grep foo
As you are not even inside the working tree, the command would
not know in which subdirectory you want to find the string foo;
the "reasonable behaviour" is to work on the whole working tree
in this case.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: `git rev-parse --is-inside-work-tree` and $GIT_WORK_TREE
2016-03-29 15:08 ` Junio C Hamano
@ 2016-03-29 19:41 ` Jeff King
2016-03-29 19:56 ` Junio C Hamano
0 siblings, 1 reply; 16+ messages in thread
From: Jeff King @ 2016-03-29 19:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: John Keeping, Elliott Cable, git
On Tue, Mar 29, 2016 at 08:08:29AM -0700, Junio C Hamano wrote:
> So it is a misconfiguration if you only set GIT_WORK_TREE without
> setting GIT_DIR.
Hmm. I have frequently done this when my cwd is a git repository (e.g.,
a bare one), and it works as you'd expect (find the git-dir in the
current path, then the working tree via $GIT_WORK_TREE).
I always assumed that was the intended behavior, as it is the only one
that makes sense. I suspect I am not alone in having relied on this.
> Also, if you set both and run Git from outside $GIT_WORK_TREE, even
> though Git may try to do its best to give you a reasonable behaviour
> [*1*], it is working by accident not by design (see the statement
> you are making by setting GIT_WORK_TREE in the third bullet above).
>
> IOW, running from outside GIT_WORK_TREE is a misconfiguration.
>
> [Footnote]
>
> *1* Think what should happen when you are outside GIT_WORK_TREE and
> say this:
>
> $ git grep foo
>
> As you are not even inside the working tree, the command would
> not know in which subdirectory you want to find the string foo;
> the "reasonable behaviour" is to work on the whole working tree
> in this case.
Likewise, I always assumed this "reasonable behavior" was intended. When
we setup_git_directory(), we end up in the root of the working tree as
usual. The "prefix" must be empty, as we were not in the work tree at
all, and we do a whole-tree operation.
Those behaviors may not have been fully designed, but as they do the
only reasonable thing (besides dying with an error), and people may have
baked that assumption into their scripts, I think we should avoid
changing them unless there is a compelling reason.
-Peff
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: `git rev-parse --is-inside-work-tree` and $GIT_WORK_TREE
2016-03-29 19:41 ` Jeff King
@ 2016-03-29 19:56 ` Junio C Hamano
2016-03-29 20:26 ` Jeff King
0 siblings, 1 reply; 16+ messages in thread
From: Junio C Hamano @ 2016-03-29 19:56 UTC (permalink / raw)
To: Jeff King; +Cc: John Keeping, Elliott Cable, git
Jeff King <peff@peff.net> writes:
> On Tue, Mar 29, 2016 at 08:08:29AM -0700, Junio C Hamano wrote:
>
>> So it is a misconfiguration if you only set GIT_WORK_TREE without
>> setting GIT_DIR.
>
> Hmm. I have frequently done this when my cwd is a git repository (e.g.,
> a bare one), and it works as you'd expect (find the git-dir in the
> current path, then the working tree via $GIT_WORK_TREE).
Hmm, does what is done by "git add HEAD" in such a situation match
what you'd expect?
git init work
cd work; date >HEAD; git commit -m initial
git push ../bare master:master
date >>HEAD
export GIT_WORK_TREE=$(pwd)
cd ..
git --bare init bare
cd bare
git add HEAD
I'd have to say that this invites unnecessary confusion, even though
I agree that "go to the GIT_WORK_TREE and take pathspecs relative to
that directory" is the only sensible thing for us to be doing.
But that is not an issue about "set only work-tree" (it is about
"run from outside the work-tree").
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: `git rev-parse --is-inside-work-tree` and $GIT_WORK_TREE
2016-03-29 19:56 ` Junio C Hamano
@ 2016-03-29 20:26 ` Jeff King
0 siblings, 0 replies; 16+ messages in thread
From: Jeff King @ 2016-03-29 20:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: John Keeping, Elliott Cable, git
On Tue, Mar 29, 2016 at 12:56:41PM -0700, Junio C Hamano wrote:
> >> So it is a misconfiguration if you only set GIT_WORK_TREE without
> >> setting GIT_DIR.
> >
> > Hmm. I have frequently done this when my cwd is a git repository (e.g.,
> > a bare one), and it works as you'd expect (find the git-dir in the
> > current path, then the working tree via $GIT_WORK_TREE).
>
> Hmm, does what is done by "git add HEAD" in such a situation match
> what you'd expect?
>
> git init work
> cd work; date >HEAD; git commit -m initial
> git push ../bare master:master
> date >>HEAD
> export GIT_WORK_TREE=$(pwd)
> cd ..
> git --bare init bare
> cd bare
> git add HEAD
I had to tweak your commands a little, but I assume the part you are
interested in is the end, when git-add finds HEAD in $GIT_WORK_TREE and
not the bare repository.
And yes, that is exactly what I'd expect, and why it is useful (if you
wanted to add arbitrary cruft from the bare repo, you'd set
$GIT_WORK_TREE to point there).
> I'd have to say that this invites unnecessary confusion, even though
> I agree that "go to the GIT_WORK_TREE and take pathspecs relative to
> that directory" is the only sensible thing for us to be doing.
>
> But that is not an issue about "set only work-tree" (it is about
> "run from outside the work-tree").
Yeah, there are two things going on:
1. Without $GIT_DIR but with $GIT_WORK_TREE, we find $GIT_DIR via the
usual discovery path.
2. When outside $GIT_WORK_TREE, any work-tree operations work as if
they were started from $GIT_WORK_TREE.
And relying on (1) almost always relies on (2), unless your work-tree
happens to be inside the discovery path for your $GIT_DIR. So you could
do:
git init repo
mkdir repo/subdir
echo content >file
GIT_WORK_TREE=$(pwd) git add .
which adds "file" at the top-level. And we used only rule (1), not rule
(2). I don't know whether people actually do that or not (I guess it
could be useful for tricky subtree things).
-Peff
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: `git rev-parse --is-inside-work-tree` and $GIT_WORK_TREE
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 20:34 ` Jeff King
2016-03-29 20:52 ` John Keeping
2016-03-30 0:53 ` Duy Nguyen
1 sibling, 2 replies; 16+ messages in thread
From: Jeff King @ 2016-03-29 20:34 UTC (permalink / raw)
To: Elliott Cable; +Cc: John Keeping, Nguyễn Thái Ngọc Duy, git
On Tue, Mar 29, 2016 at 06:42:44AM -0500, Elliott Cable wrote:
> So, I find this behaviour a little strange; I can't determine if it's
> a subtle bug, or intentionally undefined/‘fuzzy’ behaviour:
>
> $ cd a-repo/.git/
> $ pwd
> /path/to/a-repo/.git
> $ git rev-parse --is-inside-work-tree
> false
> $ export GIT_WORK_TREE=/path/to/a-repo
> $ git rev-parse --is-inside-work-tree
> true
>
> i.e. when within the repository (the `.git` directory), and when that
> directory is a sub-directory of the working-tree, `rev-parse
> --is-inside-work-tree` reports *false* (reasonable enough, I suppose);
> but then if `$GIT_WORK_TREE` is set to precisely the directory that
> git was *already* assuming was the working-directory, then the same
> command, in the same location, reports *true*.
>
> This should probably be made consistent: either `rev-parse
> --is-inside-work-tree` should report “true”, even inside the `.git`
> dir, as long as that directory is a sub-directory of the working-tree
> … or repository-directories / `$GIT_DIR` / `.git` directories should
> be excluded from truthy responses to `rev-parse
> --is-inside-work-tree`.
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".
Unfortunately, I think the fix is likely to be rather tricky, as the
work-tree stuff is happening deep inside setup_git_directory().
-Peff
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: `git rev-parse --is-inside-work-tree` and $GIT_WORK_TREE
2016-03-29 20:34 ` Jeff King
@ 2016-03-29 20:52 ` John Keeping
2016-03-29 21:21 ` Jeff King
2016-03-30 0:53 ` Duy Nguyen
1 sibling, 1 reply; 16+ messages in thread
From: John Keeping @ 2016-03-29 20:52 UTC (permalink / raw)
To: Jeff King; +Cc: Elliott Cable, Nguyễn Thái Ngọc Duy, git
On Tue, Mar 29, 2016 at 04:34:25PM -0400, Jeff King wrote:
> On Tue, Mar 29, 2016 at 06:42:44AM -0500, Elliott Cable wrote:
>
> > So, I find this behaviour a little strange; I can't determine if it's
> > a subtle bug, or intentionally undefined/‘fuzzy’ behaviour:
> >
> > $ cd a-repo/.git/
> > $ pwd
> > /path/to/a-repo/.git
> > $ git rev-parse --is-inside-work-tree
> > false
> > $ export GIT_WORK_TREE=/path/to/a-repo
> > $ git rev-parse --is-inside-work-tree
> > true
> >
> > i.e. when within the repository (the `.git` directory), and when that
> > directory is a sub-directory of the working-tree, `rev-parse
> > --is-inside-work-tree` reports *false* (reasonable enough, I suppose);
> > but then if `$GIT_WORK_TREE` is set to precisely the directory that
> > git was *already* assuming was the working-directory, then the same
> > command, in the same location, reports *true*.
> >
> > This should probably be made consistent: either `rev-parse
> > --is-inside-work-tree` should report “true”, even inside the `.git`
> > dir, as long as that directory is a sub-directory of the working-tree
> > … or repository-directories / `$GIT_DIR` / `.git` directories should
> > be excluded from truthy responses to `rev-parse
> > --is-inside-work-tree`.
>
> 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.
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.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: `git rev-parse --is-inside-work-tree` and $GIT_WORK_TREE
2016-03-29 20:52 ` John Keeping
@ 2016-03-29 21:21 ` Jeff King
2016-03-29 22:00 ` John Keeping
0 siblings, 1 reply; 16+ messages in thread
From: Jeff King @ 2016-03-29 21:21 UTC (permalink / raw)
To: John Keeping; +Cc: Elliott Cable, Nguyễn Thái Ngọc Duy, git
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". But as you note:
> 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.
-Peff
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: `git rev-parse --is-inside-work-tree` and $GIT_WORK_TREE
2016-03-29 21:21 ` Jeff King
@ 2016-03-29 22:00 ` John Keeping
2016-03-29 22:14 ` John Keeping
2016-03-29 22:16 ` Jeff King
0 siblings, 2 replies; 16+ messages in thread
From: John Keeping @ 2016-03-29 22:00 UTC (permalink / raw)
To: Jeff King; +Cc: Elliott Cable, Nguyễn Thái Ngọc Duy, git
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: `git rev-parse --is-inside-work-tree` and $GIT_WORK_TREE
2016-03-29 22:00 ` John Keeping
@ 2016-03-29 22:14 ` John Keeping
2016-03-29 22:16 ` Jeff King
1 sibling, 0 replies; 16+ messages in thread
From: John Keeping @ 2016-03-29 22:14 UTC (permalink / raw)
To: Jeff King; +Cc: Elliott Cable, Nguyễn Thái Ngọc Duy, git
On Tue, Mar 29, 2016 at 11:00:03PM +0100, John Keeping wrote:
> 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.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: `git rev-parse --is-inside-work-tree` and $GIT_WORK_TREE
2016-03-29 22:00 ` John Keeping
2016-03-29 22:14 ` John Keeping
@ 2016-03-29 22:16 ` Jeff King
2016-03-29 22:35 ` Junio C Hamano
1 sibling, 1 reply; 16+ messages in thread
From: Jeff King @ 2016-03-29 22:16 UTC (permalink / raw)
To: John Keeping; +Cc: Elliott Cable, Nguyễn Thái Ngọc Duy, git
On Tue, Mar 29, 2016 at 11:00:03PM +0100, John Keeping wrote:
> > 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
Yeah, and not just the is-inside-git-dir test:
$ echo content >../file
$ GIT_WORK_TREE=$(cd ..; pwd) GIT_DIR=$(pwd) git add file
fatal: pathspec 'file' did not match any files
I'd expect that to work, and it doesn't, because we pass ".git/" as the
"prefix" to cmd_add(). Which I guess is true, but it feels kind of weird
(I think most people who set both variables like that would generally
point to some other directory entirely, and we would have a NULL
prefix).
The --is-inside-git-dir thing is related, but a different problem. I
just got your follow-up mentioning that it doesn't take the prefix into
account, which I agree it probably should.
-Peff
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: `git rev-parse --is-inside-work-tree` and $GIT_WORK_TREE
2016-03-29 22:16 ` Jeff King
@ 2016-03-29 22:35 ` Junio C Hamano
0 siblings, 0 replies; 16+ messages in thread
From: Junio C Hamano @ 2016-03-29 22:35 UTC (permalink / raw)
To: Jeff King
Cc: John Keeping, Elliott Cable,
Nguyễn Thái Ngọc Duy, git
Jeff King <peff@peff.net> writes:
> $ echo content >../file
> $ GIT_WORK_TREE=$(cd ..; pwd) GIT_DIR=$(pwd) git add file
> fatal: pathspec 'file' did not match any files
>
> I'd expect that to work, and it doesn't, because we pass ".git/" as the
> "prefix" to cmd_add(). Which I guess is true, but it feels kind of weird
> (I think most people who set both variables like that would generally
> point to some other directory entirely, and we would have a NULL
> prefix).
That reminds me of a related tangent. If we really want to properly
support running from outside the working tree (or from inside .git
for that matter), I suspect we need two separate "prefix" for two
different uses. The "we would have a NULL prefix" is what was
considered the true "prefix" traditionally, i.e. it is the directory
to which any pathspecs and relative paths that name paths in the
history are taken relative to. E.g. if you run "git add HEAD" from
inside your GIT_DIR but you have GIT_WORK_TREE set up correctly, you
would want to add HEAD from the root of the working tree.
Another is the base directory for a relative filename that names a
file that does not have anything to do with the paths in the
history. E.g. if you run "git grep --file patterns" from outside
the working tree but with GIT_DIR/GIT_WORK_TREE correctly set up,
you would still want to read the "patterns" file from the current
directory.
The former can be done by using prefix=NULL to say "we may or may
not have come from outside a working tree but we no longer care
after we chdir(2) to the root of the working tree. Any path is
relative to the root of the working tree." But then we may lose the
clue to read from the latter (the OPT_FILENAME option is handled by
prefix_filename() using the prefix).
The distinction between the two does not exist as long as you start
inside GIT_WORK_TREE and outside GIT_DIR.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: `git rev-parse --is-inside-work-tree` and $GIT_WORK_TREE
2016-03-29 20:34 ` Jeff King
2016-03-29 20:52 ` John Keeping
@ 2016-03-30 0:53 ` Duy Nguyen
2016-04-01 0:49 ` Elliott Cable
1 sibling, 1 reply; 16+ messages in thread
From: Duy Nguyen @ 2016-03-30 0:53 UTC (permalink / raw)
To: Jeff King; +Cc: Elliott Cable, John Keeping, Git Mailing List
On Wed, Mar 30, 2016 at 3:34 AM, Jeff King <peff@peff.net> wrote:
> On Tue, Mar 29, 2016 at 06:42:44AM -0500, Elliott Cable wrote:
>
>> So, I find this behaviour a little strange; I can't determine if it's
>> a subtle bug, or intentionally undefined/‘fuzzy’ behaviour:
>>
>> $ cd a-repo/.git/
>> $ pwd
>> /path/to/a-repo/.git
>> $ git rev-parse --is-inside-work-tree
>> false
>> $ export GIT_WORK_TREE=/path/to/a-repo
>> $ git rev-parse --is-inside-work-tree
>> true
>>
>> i.e. when within the repository (the `.git` directory), and when that
>> directory is a sub-directory of the working-tree, `rev-parse
>> --is-inside-work-tree` reports *false* (reasonable enough, I suppose);
>> but then if `$GIT_WORK_TREE` is set to precisely the directory that
>> git was *already* assuming was the working-directory, then the same
>> command, in the same location, reports *true*.
>>
>> This should probably be made consistent: either `rev-parse
>> --is-inside-work-tree` should report “true”, even inside the `.git`
>> dir, as long as that directory is a sub-directory of the working-tree
>> … or repository-directories / `$GIT_DIR` / `.git` directories should
>> be excluded from truthy responses to `rev-parse
>> --is-inside-work-tree`.
No. Once you set GIT_WORK_TREE you tell git that worktree exists. That
overrides the "bare repo" attribute (i.e. no worktree) that's
automatically set when we try to find .git directory.
> 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".
On relative GIT_WORK_TREE some mail down this thread, there's this
note in t1510 that you might find interesting
5. GIT_WORK_TREE/core.worktree was originally meant to work only if
GIT_DIR is set, but earlier git didn't enforce it, and some scripts
depend on the implementation that happened to first discover .git by
going up from the users $cwd and then using the specified working tree
that may or may not have any relation to where .git was found in. This
historical behaviour must be kept.
Basically if you set GIT_WORK_TREE you better set GIT_DIR too to keep
things sane.
--
Duy
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: `git rev-parse --is-inside-work-tree` and $GIT_WORK_TREE
2016-03-30 0:53 ` Duy Nguyen
@ 2016-04-01 0:49 ` Elliott Cable
0 siblings, 0 replies; 16+ messages in thread
From: Elliott Cable @ 2016-04-01 0:49 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Jeff King, John Keeping, Git Mailing List
oh, wow, this got over my head *real* fast. Okay,
1. Yeah, my `$GIT_WORK_TREE` was def. an absolute path; I typed that
example code without running it *precisely* that way (entirely my
mistake! I'm so sorry for the confusion it caused, and all that typing
you did!); if I remember correctly (not at the machine right now), I
had run `git rev-parse --show-toplevel` from a different directory,
with `$GIT_DIR` set, while trying to narrow down this bug, so it gave
me an absolute path … and then copy-pasted that path, and then
replaced my copy-paste with the original command to make the
bug-report example as concise as possible? oops. But, yeah, it fails
in the manner described above with an absolute path. (Which it looks
like you two figured out above.)
2. Re: intentions, again, it seems like you've changed your mind, but …
> So it is a misconfiguration if you only set GIT_WORK_TREE
without setting GIT_DIR.
I really, really hope not! Half the usefulness of `$GIT_WORK_TREE`
existing is in that mode. In fact, that's how I found `$GIT_WORK_TREE`
documented and explained, in [this blog
post](https://git-scm.com/blog/2010/04/11/environment.html) on the Git
site. That usage seems pretty damn useful, so I do hope it's
eventually explicitly supported … (and if that's *not* going to be the
case, it should be explicitly documented in the `GIT(1)` manpage,
alongside the other documentation of the environment-variables, that
the behaviour is undefined is `$GIT_DIR` isn't set first. =)
⁓ ELLIOTTCABLE — fly safe.
http://ell.io/tt
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2016-04-01 0:49 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
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
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).