* `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).