{"thread":{"id":"41858","subject":"`git rev-parse --is-inside-work-tree` and $GIT_WORK_TREE","startedAt":"2016-03-29T11:42:44Z","lastAt":"2016-04-01T00:49:26Z","messageCount":16,"participants":["Elliott Cable","John Keeping","Junio C Hamano","Jeff King","Duy Nguyen"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"282042","messageId":"CAPZ477NxXVNNwDvzaFt7GoUGuJwnOuX3y1N+aPtVRFD3E8dQBA@mail.gmail.com","threadId":"41858","inReplyTo":null,"subject":"`git rev-parse --is-inside-work-tree` and $GIT_WORK_TREE","fromName":"Elliott Cable","fromEmail":"me@ell.io","sentAt":"2016-03-29T11:42:44Z","receivedAt":"2016-03-29T11:42:44Z","isPatch":false,"sender":{"key":"me@ell.io","avatar":"https://gravatar.com/avatar/0d35bb7d7c29b6bbcaafdf13ec70745573d31cb222130cc754a064e707c08d63?d=mp&s=160"},"body":"So, I find this behaviour a little strange; I can't determine if it's\na subtle bug, or intentionally undefined/‘fuzzy’ behaviour:\n\n    $ cd a-repo/.git/\n    $ pwd\n    /path/to/a-repo/.git\n    $ git rev-parse --is-inside-work-tree\n    false\n    $ export GIT_WORK_TREE=/path/to/a-repo\n    $ git rev-parse --is-inside-work-tree\n    true\n\ni.e. when within the repository (the `.git` directory), and when that\ndirectory is a sub-directory of the working-tree, `rev-parse\n--is-inside-work-tree` reports *false* (reasonable enough, I suppose);\nbut then if `$GIT_WORK_TREE` is set to precisely the directory that\ngit was *already* assuming was the working-directory, then the same\ncommand, in the same location, reports *true*.\n\nThis should probably be made consistent: either `rev-parse\n--is-inside-work-tree` should report “true”, even inside the `.git`\ndir, as long as that directory is a sub-directory of the working-tree\n… or repository-directories / `$GIT_DIR` / `.git` directories should\nbe excluded from truthy responses to `rev-parse\n--is-inside-work-tree`.\n\n\n⁓ ELLIOTTCABLE — fly safe.\n  http://ell.io/tt\n"},{"id":"282044","messageId":"CAPZ477PD7SkRg7T_Y_n27Hjw5TeW6Sh0-vtoP6-4xUDraC7OiA@mail.gmail.com","threadId":"41858","inReplyTo":"CAPZ477NxXVNNwDvzaFt7GoUGuJwnOuX3y1N+aPtVRFD3E8dQBA@mail.gmail.com","subject":"Re: `git rev-parse --is-inside-work-tree` and $GIT_WORK_TREE","fromName":"Elliott Cable","fromEmail":"me@ell.io","sentAt":"2016-03-29T11:53:35Z","receivedAt":"2016-03-29T11:53:35Z","isPatch":false,"sender":{"key":"me@ell.io","avatar":"https://gravatar.com/avatar/0d35bb7d7c29b6bbcaafdf13ec70745573d31cb222130cc754a064e707c08d63?d=mp&s=160"},"body":"On Tue, Mar 29, 2016 at 6:42 AM, Elliott Cable <me@ell.io> wrote:\n> So, I find this behaviour a little strange; I can't determine if it's\n> a subtle bug, or intentionally undefined/‘fuzzy’ behaviour ...\n\nOh lord, it gets worse ...\n\n$ cd a-repo\n$ git rev-parse --is-inside-work-tree; git rev-parse --is-inside-git-dir\ntrue\nfalse\n$ cd .git\n$ git rev-parse --is-inside-work-tree; git rev-parse --is-inside-git-dir\nfalse\ntrue\n$ export GIT_WORK_TREE=\"$(git rev-parse --show-toplevel)\"   # !!!\n$ git rev-parse --is-inside-work-tree; git rev-parse --is-inside-git-dir\ntrue\nfalse\n$ # !!?!?\n\nSo, basically, if `$GIT_WORK_TREE` is set at all, it appears that the\n`rev-parse --is-inside...` flags don't function reliably at all.\n\n\n⁓ ELLIOTTCABLE — fly safe.\n  http://ell.io/tt\n"},{"id":"282046","messageId":"20160329123306.GD1578@serenity.lan","threadId":"41858","inReplyTo":"CAPZ477PD7SkRg7T_Y_n27Hjw5TeW6Sh0-vtoP6-4xUDraC7OiA@mail.gmail.com","subject":"Re: `git rev-parse --is-inside-work-tree` and $GIT_WORK_TREE","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2016-03-29T12:33:07Z","receivedAt":"2016-03-29T12:33:07Z","isPatch":false,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Tue, Mar 29, 2016 at 06:53:35AM -0500, Elliott Cable wrote:\n> On Tue, Mar 29, 2016 at 6:42 AM, Elliott Cable <me@ell.io> wrote:\n> > So, I find this behaviour a little strange; I can't determine if it's\n> > a subtle bug, or intentionally undefined/‘fuzzy’ behaviour ...\n> \n> Oh lord, it gets worse ...\n> \n> $ cd a-repo\n> $ git rev-parse --is-inside-work-tree; git rev-parse --is-inside-git-dir\n> true\n> false\n> $ cd .git\n> $ git rev-parse --is-inside-work-tree; git rev-parse --is-inside-git-dir\n> false\n> true\n\nI believe these are working correctly, the .git directory is not part of\nthe working tree.\n\n> $ export GIT_WORK_TREE=\"$(git rev-parse --show-toplevel)\"   # !!!\n\nDid you check the value of GIT_WORK_TREE here?  When I try it's the\nempty string.\n\nIf I set the core.worktree config variable to \"..\" then rev-parse does\nfind the working tree correctly.  I recall some previous discussion\nabout this but I can't find it in the list archives from a quick search.\n\n> $ git rev-parse --is-inside-work-tree; git rev-parse --is-inside-git-dir\n> true\n> false\n> $ # !!?!?\n> \n> So, basically, if `$GIT_WORK_TREE` is set at all, it appears that the\n> `rev-parse --is-inside...` flags don't function reliably at all.\n\nIf you set GIT_WORK_TREE you're telling Git to override all of the\nnormal detection logic.  What version of Git are you using?  When I try\nthis it says:\n\n\tfatal: The empty string is not a valid path\n\nIf I set GIT_WORK_TREE to the correct value for this repository then it\nbehaves the same as with the auto-detection logic.\n"},{"id":"282065","messageId":"xmqqshz9z5hu.fsf@gitster.mtv.corp.google.com","threadId":"41858","inReplyTo":"20160329123306.GD1578@serenity.lan","subject":"Re: `git rev-parse --is-inside-work-tree` and $GIT_WORK_TREE","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-29T15:08:29Z","receivedAt":"2016-03-29T15:08:29Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"John Keeping <john@keeping.me.uk> writes:\n\n> If you set GIT_WORK_TREE you're telling Git to override all of the\n> normal detection logic.\n\nI didn't carefully read the other parts of the discussion, but this\nis not entirely correct.\n\nThe actual/intended rules are fairly simple.\n\n * With GIT_DIR (without GIT_WORK_TREE), you specify where the .git\n   directory is, and refuse the \"normal detection logic\".  As you\n   refuse the normal \"where is the root of the working tree and its\n   .git directory?\" logic, Git uses the current directory as the\n   root of the working tree.\n\n * But that means if you set GIT_DIR, you cannot work from a\n   subdirectory of your working tree--you always have to run Git\n   from the root level of your working tree.  This may be\n   inconvenient.\n\n * That is why GIT_WORK_TREE exists.  It allows you to say \"No, I am\n   not at the root level but am in a subdirectory somewhere inside\n   the working tree. The root level of the working tree is there\n   above, not here\".\n\nSo it is a misconfiguration if you only set GIT_WORK_TREE without\nsetting GIT_DIR.\n\nAlso, if you set both and run Git from outside $GIT_WORK_TREE, even\nthough Git may try to do its best to give you a reasonable behaviour\n[*1*], it is working by accident not by design (see the statement\nyou are making by setting GIT_WORK_TREE in the third bullet above).\n\nIOW, running from outside GIT_WORK_TREE is a misconfiguration.\n\n[Footnote]\n\n*1* Think what should happen when you are outside GIT_WORK_TREE and\n    say this:\n\n\t$ git grep foo\n\n    As you are not even inside the working tree, the command would\n    not know in which subdirectory you want to find the string foo;\n    the \"reasonable behaviour\" is to work on the whole working tree\n    in this case.\n"},{"id":"282096","messageId":"20160329194156.GA9527@sigill.intra.peff.net","threadId":"41858","inReplyTo":"xmqqshz9z5hu.fsf@gitster.mtv.corp.google.com","subject":"Re: `git rev-parse --is-inside-work-tree` and $GIT_WORK_TREE","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-03-29T19:41:56Z","receivedAt":"2016-03-29T19:41:56Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 29, 2016 at 08:08:29AM -0700, Junio C Hamano wrote:\n\n> So it is a misconfiguration if you only set GIT_WORK_TREE without\n> setting GIT_DIR.\n\nHmm. I have frequently done this when my cwd is a git repository (e.g.,\na bare one), and it works as you'd expect (find the git-dir in the\ncurrent path, then the working tree via $GIT_WORK_TREE).\n\nI always assumed that was the intended behavior, as it is the only one\nthat makes sense. I suspect I am not alone in having relied on this.\n\n> Also, if you set both and run Git from outside $GIT_WORK_TREE, even\n> though Git may try to do its best to give you a reasonable behaviour\n> [*1*], it is working by accident not by design (see the statement\n> you are making by setting GIT_WORK_TREE in the third bullet above).\n> \n> IOW, running from outside GIT_WORK_TREE is a misconfiguration.\n> \n> [Footnote]\n> \n> *1* Think what should happen when you are outside GIT_WORK_TREE and\n>     say this:\n> \n> \t$ git grep foo\n> \n>     As you are not even inside the working tree, the command would\n>     not know in which subdirectory you want to find the string foo;\n>     the \"reasonable behaviour\" is to work on the whole working tree\n>     in this case.\n\nLikewise, I always assumed this \"reasonable behavior\" was intended. When\nwe setup_git_directory(), we end up in the root of the working tree as\nusual. The \"prefix\" must be empty, as we were not in the work tree at\nall, and we do a whole-tree operation.\n\nThose behaviors may not have been fully designed, but as they do the\nonly reasonable thing (besides dying with an error), and people may have\nbaked that assumption into their scripts, I think we should avoid\nchanging them unless there is a compelling reason.\n\n-Peff\n"},{"id":"282101","messageId":"xmqq60w5xdl2.fsf@gitster.mtv.corp.google.com","threadId":"41858","inReplyTo":"20160329194156.GA9527@sigill.intra.peff.net","subject":"Re: `git rev-parse --is-inside-work-tree` and $GIT_WORK_TREE","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-29T19:56:41Z","receivedAt":"2016-03-29T19:56:41Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Tue, Mar 29, 2016 at 08:08:29AM -0700, Junio C Hamano wrote:\n>\n>> So it is a misconfiguration if you only set GIT_WORK_TREE without\n>> setting GIT_DIR.\n>\n> Hmm. I have frequently done this when my cwd is a git repository (e.g.,\n> a bare one), and it works as you'd expect (find the git-dir in the\n> current path, then the working tree via $GIT_WORK_TREE).\n\nHmm, does what is done by \"git add HEAD\" in such a situation match\nwhat you'd expect?\n\n        git init work\n        cd work; date >HEAD; git commit -m initial\n        git push ../bare master:master\n\tdate >>HEAD\n        export GIT_WORK_TREE=$(pwd)\n\tcd ..\n\tgit --bare init bare\n\tcd bare\n\tgit add HEAD\n\nI'd have to say that this invites unnecessary confusion, even though\nI agree that \"go to the GIT_WORK_TREE and take pathspecs relative to\nthat directory\" is the only sensible thing for us to be doing.\n\nBut that is not an issue about \"set only work-tree\" (it is about\n\"run from outside the work-tree\").\n"},{"id":"282111","messageId":"20160329202626.GC9527@sigill.intra.peff.net","threadId":"41858","inReplyTo":"xmqq60w5xdl2.fsf@gitster.mtv.corp.google.com","subject":"Re: `git rev-parse --is-inside-work-tree` and $GIT_WORK_TREE","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-03-29T20:26:27Z","receivedAt":"2016-03-29T20:26:27Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 29, 2016 at 12:56:41PM -0700, Junio C Hamano wrote:\n\n> >> So it is a misconfiguration if you only set GIT_WORK_TREE without\n> >> setting GIT_DIR.\n> >\n> > Hmm. I have frequently done this when my cwd is a git repository (e.g.,\n> > a bare one), and it works as you'd expect (find the git-dir in the\n> > current path, then the working tree via $GIT_WORK_TREE).\n> \n> Hmm, does what is done by \"git add HEAD\" in such a situation match\n> what you'd expect?\n> \n>         git init work\n>         cd work; date >HEAD; git commit -m initial\n>         git push ../bare master:master\n> \tdate >>HEAD\n>         export GIT_WORK_TREE=$(pwd)\n> \tcd ..\n> \tgit --bare init bare\n> \tcd bare\n> \tgit add HEAD\n\nI had to tweak your commands a little, but I assume the part you are\ninterested in is the end, when git-add finds HEAD in $GIT_WORK_TREE and\nnot the bare repository.\n\nAnd yes, that is exactly what I'd expect, and why it is useful (if you\nwanted to add arbitrary cruft from the bare repo, you'd set\n$GIT_WORK_TREE to point there).\n\n> I'd have to say that this invites unnecessary confusion, even though\n> I agree that \"go to the GIT_WORK_TREE and take pathspecs relative to\n> that directory\" is the only sensible thing for us to be doing.\n> \n> But that is not an issue about \"set only work-tree\" (it is about\n> \"run from outside the work-tree\").\n\nYeah, there are two things going on:\n\n  1. Without $GIT_DIR but with $GIT_WORK_TREE, we find $GIT_DIR via the\n     usual discovery path.\n\n  2. When outside $GIT_WORK_TREE, any work-tree operations work as if\n     they were started from $GIT_WORK_TREE.\n\nAnd relying on (1) almost always relies on (2), unless your work-tree\nhappens to be inside the discovery path for your $GIT_DIR. So you could\ndo:\n\n  git init repo\n  mkdir repo/subdir\n  echo content >file\n  GIT_WORK_TREE=$(pwd) git add .\n\nwhich adds \"file\" at the top-level. And we used only rule (1), not rule\n(2). I don't know whether people actually do that or not (I guess it\ncould be useful for tricky subtree things).\n\n-Peff\n"},{"id":"282115","messageId":"20160329203425.GA24027@sigill.intra.peff.net","threadId":"41858","inReplyTo":"CAPZ477NxXVNNwDvzaFt7GoUGuJwnOuX3y1N+aPtVRFD3E8dQBA@mail.gmail.com","subject":"Re: `git rev-parse --is-inside-work-tree` and $GIT_WORK_TREE","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-03-29T20:34:25Z","receivedAt":"2016-03-29T20:34:25Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 29, 2016 at 06:42:44AM -0500, Elliott Cable wrote:\n\n> So, I find this behaviour a little strange; I can't determine if it's\n> a subtle bug, or intentionally undefined/‘fuzzy’ behaviour:\n> \n>     $ cd a-repo/.git/\n>     $ pwd\n>     /path/to/a-repo/.git\n>     $ git rev-parse --is-inside-work-tree\n>     false\n>     $ export GIT_WORK_TREE=/path/to/a-repo\n>     $ git rev-parse --is-inside-work-tree\n>     true\n> \n> i.e. when within the repository (the `.git` directory), and when that\n> directory is a sub-directory of the working-tree, `rev-parse\n> --is-inside-work-tree` reports *false* (reasonable enough, I suppose);\n> but then if `$GIT_WORK_TREE` is set to precisely the directory that\n> git was *already* assuming was the working-directory, then the same\n> command, in the same location, reports *true*.\n> \n> This should probably be made consistent: either `rev-parse\n> --is-inside-work-tree` should report “true”, even inside the `.git`\n> dir, as long as that directory is a sub-directory of the working-tree\n> … or repository-directories / `$GIT_DIR` / `.git` directories should\n> be excluded from truthy responses to `rev-parse\n> --is-inside-work-tree`.\n\nYeah, I think this is a bug. Presumably what is happening is that we are\ntoo eager to \"cd $GIT_WORK_TREE\" inside git-rev-parse, and by the time\nwe ask \"are we in a work tree\", the answer has become yes. But the\ncaller really wants to know \"am _I_ inside the work tree\".\n\nUnfortunately, I think the fix is likely to be rather tricky, as the\nwork-tree stuff is happening deep inside setup_git_directory().\n\n-Peff\n"},{"id":"282117","messageId":"20160329205208.GF1578@serenity.lan","threadId":"41858","inReplyTo":"20160329203425.GA24027@sigill.intra.peff.net","subject":"Re: `git rev-parse --is-inside-work-tree` and $GIT_WORK_TREE","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2016-03-29T20:52:08Z","receivedAt":"2016-03-29T20:52:08Z","isPatch":false,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Tue, Mar 29, 2016 at 04:34:25PM -0400, Jeff King wrote:\n> On Tue, Mar 29, 2016 at 06:42:44AM -0500, Elliott Cable wrote:\n> \n> > So, I find this behaviour a little strange; I can't determine if it's\n> > a subtle bug, or intentionally undefined/‘fuzzy’ behaviour:\n> > \n> >     $ cd a-repo/.git/\n> >     $ pwd\n> >     /path/to/a-repo/.git\n> >     $ git rev-parse --is-inside-work-tree\n> >     false\n> >     $ export GIT_WORK_TREE=/path/to/a-repo\n> >     $ git rev-parse --is-inside-work-tree\n> >     true\n> > \n> > i.e. when within the repository (the `.git` directory), and when that\n> > directory is a sub-directory of the working-tree, `rev-parse\n> > --is-inside-work-tree` reports *false* (reasonable enough, I suppose);\n> > but then if `$GIT_WORK_TREE` is set to precisely the directory that\n> > git was *already* assuming was the working-directory, then the same\n> > command, in the same location, reports *true*.\n> > \n> > This should probably be made consistent: either `rev-parse\n> > --is-inside-work-tree` should report “true”, even inside the `.git`\n> > dir, as long as that directory is a sub-directory of the working-tree\n> > … or repository-directories / `$GIT_DIR` / `.git` directories should\n> > be excluded from truthy responses to `rev-parse\n> > --is-inside-work-tree`.\n> \n> Yeah, I think this is a bug. Presumably what is happening is that we are\n> too eager to \"cd $GIT_WORK_TREE\" inside git-rev-parse, and by the time\n> we ask \"are we in a work tree\", the answer has become yes. But the\n> caller really wants to know \"am _I_ inside the work tree\".\n\nI don't think that's what's happening.  Try:\n\n\t$ cd .git/\n\t$ GIT_WORK_TREE=.. git rev-parse --is-inside-work-tree\n\ttrue\n\nso I think it's that we refuse to assume that the directory above a Git\ndirectory is a working tree (something similar happens when the\n\"core.worktree\" config variable is set).  I'm not convinced that's\nunreasonable.\n\nHowever, the case above also gives:\n\n\t$ GIT_WORK_TREE=.. git rev-parse --is-inside-git-dir\n\tfalse\n\t$ test $(pwd) = $(GIT_WORK_TREE=.. git rev-parse --git-dir); echo $?\n\t0\n\nso even though $PWD *is* the Git directory, we're not in the Git\ndirectory!  Setting GIT_DIR=$(pwd) makes no different to that.\n"},{"id":"282120","messageId":"20160329212143.GA30116@sigill.intra.peff.net","threadId":"41858","inReplyTo":"20160329205208.GF1578@serenity.lan","subject":"Re: `git rev-parse --is-inside-work-tree` and $GIT_WORK_TREE","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-03-29T21:21:43Z","receivedAt":"2016-03-29T21:21:43Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 29, 2016 at 09:52:08PM +0100, John Keeping wrote:\n\n> > Yeah, I think this is a bug. Presumably what is happening is that we are\n> > too eager to \"cd $GIT_WORK_TREE\" inside git-rev-parse, and by the time\n> > we ask \"are we in a work tree\", the answer has become yes. But the\n> > caller really wants to know \"am _I_ inside the work tree\".\n> \n> I don't think that's what's happening.  Try:\n> \n> \t$ cd .git/\n> \t$ GIT_WORK_TREE=.. git rev-parse --is-inside-work-tree\n> \ttrue\n> \n> so I think it's that we refuse to assume that the directory above a Git\n> directory is a working tree (something similar happens when the\n> \"core.worktree\" config variable is set).  I'm not convinced that's\n> unreasonable.\n\nYeah, you're right, but I'm not sure how your example shows that, (isn't\nit basically the same as Elliott's original, except using a relative\npath?). A more compelling counter-example to my hypothesis is:\n\n  $ cd .git\n  $ GIT_WORK_TREE=/tmp git rev-parse --is-inside-work-tree\n  false\n\nSo it is not that we chdir too early, but just that we blindly check \"is\n$(pwd) inside $GIT_WORK_TREE\". And it does not create a problem for the\nnormal discovered-path cases, because either:\n\n  - we discovered .git by walking up the directory tree, which means we\n    must be in a work-tree\n\n  - we discovered that we are inside a .git directory, and therefore\n    take it to be bare (and thus there is no work tree, and we cannot be\n    inside it). This is what happens in Elliott's original example that\n    behaves differently than the $GIT_WORK_TREE case.\n\nI'd be tempted to say that \"inside the work tree\" is further clarified\nto \"not inside the $GIT_DIR\". But as you note:\n\n> However, the case above also gives:\n> \n> \t$ GIT_WORK_TREE=.. git rev-parse --is-inside-git-dir\n> \tfalse\n> \t$ test $(pwd) = $(GIT_WORK_TREE=.. git rev-parse --git-dir); echo $?\n> \t0\n> \n> so even though $PWD *is* the Git directory, we're not in the Git\n> directory!  Setting GIT_DIR=$(pwd) makes no different to that.\n\nWe seem to get that wrong. I'm also not sure if it would make sense if\nyou explicitly set the two to be equal, like:\n\n  # checking in your own refs?\n  GIT_WORK_TREE=$(pwd) GIT_DIR=$(pwd) git add refs packed-refs\n\nSo the current behavior may just be weird-but-true.\n\n-Peff\n"},{"id":"282124","messageId":"20160329220003.GG1578@serenity.lan","threadId":"41858","inReplyTo":"20160329212143.GA30116@sigill.intra.peff.net","subject":"Re: `git rev-parse --is-inside-work-tree` and $GIT_WORK_TREE","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2016-03-29T22:00:03Z","receivedAt":"2016-03-29T22:00:03Z","isPatch":false,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Tue, Mar 29, 2016 at 05:21:43PM -0400, Jeff King wrote:\n> On Tue, Mar 29, 2016 at 09:52:08PM +0100, John Keeping wrote:\n> \n> > > Yeah, I think this is a bug. Presumably what is happening is that we are\n> > > too eager to \"cd $GIT_WORK_TREE\" inside git-rev-parse, and by the time\n> > > we ask \"are we in a work tree\", the answer has become yes. But the\n> > > caller really wants to know \"am _I_ inside the work tree\".\n> > \n> > I don't think that's what's happening.  Try:\n> > \n> > \t$ cd .git/\n> > \t$ GIT_WORK_TREE=.. git rev-parse --is-inside-work-tree\n> > \ttrue\n> > \n> > so I think it's that we refuse to assume that the directory above a Git\n> > directory is a working tree (something similar happens when the\n> > \"core.worktree\" config variable is set).  I'm not convinced that's\n> > unreasonable.\n> \n> Yeah, you're right, but I'm not sure how your example shows that, (isn't\n> it basically the same as Elliott's original, except using a relative\n> path?). A more compelling counter-example to my hypothesis is:\n> \n>   $ cd .git\n>   $ GIT_WORK_TREE=/tmp git rev-parse --is-inside-work-tree\n>   false\n> \n> So it is not that we chdir too early, but just that we blindly check \"is\n> $(pwd) inside $GIT_WORK_TREE\". And it does not create a problem for the\n> normal discovered-path cases, because either:\n> \n>   - we discovered .git by walking up the directory tree, which means we\n>     must be in a work-tree\n> \n>   - we discovered that we are inside a .git directory, and therefore\n>     take it to be bare (and thus there is no work tree, and we cannot be\n>     inside it). This is what happens in Elliott's original example that\n>     behaves differently than the $GIT_WORK_TREE case.\n> \n> I'd be tempted to say that \"inside the work tree\" is further clarified\n> to \"not inside the $GIT_DIR\".\n\nYes, I think that's reasonable.  But...\n\n> > However, the case above also gives:\n> > \n> > \t$ GIT_WORK_TREE=.. git rev-parse --is-inside-git-dir\n> > \tfalse\n> > \t$ test $(pwd) = $(GIT_WORK_TREE=.. git rev-parse --git-dir); echo $?\n> > \t0\n> > \n> > so even though $PWD *is* the Git directory, we're not in the Git\n> > directory!  Setting GIT_DIR=$(pwd) makes no different to that.\n> \n> We seem to get that wrong. I'm also not sure if it would make sense if\n> you explicitly set the two to be equal, like:\n> \n>   # checking in your own refs?\n>   GIT_WORK_TREE=$(pwd) GIT_DIR=$(pwd) git add refs packed-refs\n> \n> So the current behavior may just be weird-but-true.\n\nThis case definitely feels wrong:\n\n\t$ GIT_WORK_TREE=$(cd ..; pwd) GIT_DIR=$(pwd) git rev-parse --is-inside-git-dir\n\tfalse\n\nShouldn't that be the same as if GIT_WORK_TREE and GIT_DIR aren't set?\n(It's also potentially surprising since \"git rev-parse --git-dir\" does\ngive the right answer in this case.)\n\nIf GIT_WORK_TREE points somewhere unrelated then it is correct:\n\n\t$ GIT_WORK_TREE=/tmp GIT_DIR=$(pwd) git rev-parse --is-inside-git-dir\n\ttrue\n"},{"id":"282125","messageId":"20160329221409.GH1578@serenity.lan","threadId":"41858","inReplyTo":"20160329220003.GG1578@serenity.lan","subject":"Re: `git rev-parse --is-inside-work-tree` and $GIT_WORK_TREE","fromName":"John Keeping","fromEmail":"john@keeping.me.uk","sentAt":"2016-03-29T22:14:09Z","receivedAt":"2016-03-29T22:14:09Z","isPatch":false,"sender":{"key":"john@keeping.me.uk","avatar":"https://avatars.githubusercontent.com/u/1702081?v=4"},"body":"On Tue, Mar 29, 2016 at 11:00:03PM +0100, John Keeping wrote:\n> On Tue, Mar 29, 2016 at 05:21:43PM -0400, Jeff King wrote:\n> > On Tue, Mar 29, 2016 at 09:52:08PM +0100, John Keeping wrote:\n> > \n> > > > Yeah, I think this is a bug. Presumably what is happening is that we are\n> > > > too eager to \"cd $GIT_WORK_TREE\" inside git-rev-parse, and by the time\n> > > > we ask \"are we in a work tree\", the answer has become yes. But the\n> > > > caller really wants to know \"am _I_ inside the work tree\".\n> > > \n> > > I don't think that's what's happening.  Try:\n> > > \n> > > \t$ cd .git/\n> > > \t$ GIT_WORK_TREE=.. git rev-parse --is-inside-work-tree\n> > > \ttrue\n> > > \n> > > so I think it's that we refuse to assume that the directory above a Git\n> > > directory is a working tree (something similar happens when the\n> > > \"core.worktree\" config variable is set).  I'm not convinced that's\n> > > unreasonable.\n> > \n> > Yeah, you're right, but I'm not sure how your example shows that, (isn't\n> > it basically the same as Elliott's original, except using a relative\n> > path?). A more compelling counter-example to my hypothesis is:\n> > \n> >   $ cd .git\n> >   $ GIT_WORK_TREE=/tmp git rev-parse --is-inside-work-tree\n> >   false\n> > \n> > So it is not that we chdir too early, but just that we blindly check \"is\n> > $(pwd) inside $GIT_WORK_TREE\". And it does not create a problem for the\n> > normal discovered-path cases, because either:\n> > \n> >   - we discovered .git by walking up the directory tree, which means we\n> >     must be in a work-tree\n> > \n> >   - we discovered that we are inside a .git directory, and therefore\n> >     take it to be bare (and thus there is no work tree, and we cannot be\n> >     inside it). This is what happens in Elliott's original example that\n> >     behaves differently than the $GIT_WORK_TREE case.\n> > \n> > I'd be tempted to say that \"inside the work tree\" is further clarified\n> > to \"not inside the $GIT_DIR\".\n> \n> Yes, I think that's reasonable.  But...\n> \n> > > However, the case above also gives:\n> > > \n> > > \t$ GIT_WORK_TREE=.. git rev-parse --is-inside-git-dir\n> > > \tfalse\n> > > \t$ test $(pwd) = $(GIT_WORK_TREE=.. git rev-parse --git-dir); echo $?\n> > > \t0\n> > > \n> > > so even though $PWD *is* the Git directory, we're not in the Git\n> > > directory!  Setting GIT_DIR=$(pwd) makes no different to that.\n> > \n> > We seem to get that wrong. I'm also not sure if it would make sense if\n> > you explicitly set the two to be equal, like:\n> > \n> >   # checking in your own refs?\n> >   GIT_WORK_TREE=$(pwd) GIT_DIR=$(pwd) git add refs packed-refs\n> > \n> > So the current behavior may just be weird-but-true.\n> \n> This case definitely feels wrong:\n> \n> \t$ GIT_WORK_TREE=$(cd ..; pwd) GIT_DIR=$(pwd) git rev-parse --is-inside-git-dir\n> \tfalse\n> \n> Shouldn't that be the same as if GIT_WORK_TREE and GIT_DIR aren't set?\n> (It's also potentially surprising since \"git rev-parse --git-dir\" does\n> give the right answer in this case.)\n> \n> If GIT_WORK_TREE points somewhere unrelated then it is correct:\n> \n> \t$ GIT_WORK_TREE=/tmp GIT_DIR=$(pwd) git rev-parse --is-inside-git-dir\n> \ttrue\n\nIt seems that this is a result of changing the working directory to the\nroot of the working tree if we're inside it.  is_inside_dir() doesn't\ntake account of startup_info->prefix and changing to:\n\n\treal_path(startup_info->prefix)\n\ninstead of xgetcwd() means that these tests are less surprising.\n\nBut I haven't run the test suite or thought about what else this could\nbreak.\n"},{"id":"282126","messageId":"20160329221657.GA31811@sigill.intra.peff.net","threadId":"41858","inReplyTo":"20160329220003.GG1578@serenity.lan","subject":"Re: `git rev-parse --is-inside-work-tree` and $GIT_WORK_TREE","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-03-29T22:16:57Z","receivedAt":"2016-03-29T22:16:57Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 29, 2016 at 11:00:03PM +0100, John Keeping wrote:\n\n> > We seem to get that wrong. I'm also not sure if it would make sense if\n> > you explicitly set the two to be equal, like:\n> > \n> >   # checking in your own refs?\n> >   GIT_WORK_TREE=$(pwd) GIT_DIR=$(pwd) git add refs packed-refs\n> > \n> > So the current behavior may just be weird-but-true.\n> \n> This case definitely feels wrong:\n> \n> \t$ GIT_WORK_TREE=$(cd ..; pwd) GIT_DIR=$(pwd) git rev-parse --is-inside-git-dir\n> \tfalse\n\nYeah, and not just the is-inside-git-dir test:\n\n  $ echo content >../file\n  $ GIT_WORK_TREE=$(cd ..; pwd) GIT_DIR=$(pwd) git add file\n  fatal: pathspec 'file' did not match any files\n\nI'd expect that to work, and it doesn't, because we pass \".git/\" as the\n\"prefix\" to cmd_add(). Which I guess is true, but it feels kind of weird\n(I think most people who set both variables like that would generally\npoint to some other directory entirely, and we would have a NULL\nprefix).\n\nThe --is-inside-git-dir thing is related, but a different problem. I\njust got your follow-up mentioning that it doesn't take the prefix into\naccount, which I agree it probably should.\n\n-Peff\n"},{"id":"282135","messageId":"xmqq1t6sx685.fsf@gitster.mtv.corp.google.com","threadId":"41858","inReplyTo":"20160329221657.GA31811@sigill.intra.peff.net","subject":"Re: `git rev-parse --is-inside-work-tree` and $GIT_WORK_TREE","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-03-29T22:35:38Z","receivedAt":"2016-03-29T22:35:38Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n>   $ echo content >../file\n>   $ GIT_WORK_TREE=$(cd ..; pwd) GIT_DIR=$(pwd) git add file\n>   fatal: pathspec 'file' did not match any files\n>\n> I'd expect that to work, and it doesn't, because we pass \".git/\" as the\n> \"prefix\" to cmd_add(). Which I guess is true, but it feels kind of weird\n> (I think most people who set both variables like that would generally\n> point to some other directory entirely, and we would have a NULL\n> prefix).\n\nThat reminds me of a related tangent.  If we really want to properly\nsupport running from outside the working tree (or from inside .git\nfor that matter), I suspect we need two separate \"prefix\" for two\ndifferent uses.  The \"we would have a NULL prefix\" is what was\nconsidered the true \"prefix\" traditionally, i.e. it is the directory\nto which any pathspecs and relative paths that name paths in the\nhistory are taken relative to.  E.g. if you run \"git add HEAD\" from\ninside your GIT_DIR but you have GIT_WORK_TREE set up correctly, you\nwould want to add HEAD from the root of the working tree.\n\nAnother is the base directory for a relative filename that names a\nfile that does not have anything to do with the paths in the\nhistory.  E.g. if you run \"git grep --file patterns\" from outside\nthe working tree but with GIT_DIR/GIT_WORK_TREE correctly set up,\nyou would still want to read the \"patterns\" file from the current\ndirectory.\n\nThe former can be done by using prefix=NULL to say \"we may or may\nnot have come from outside a working tree but we no longer care\nafter we chdir(2) to the root of the working tree.  Any path is\nrelative to the root of the working tree.\"  But then we may lose the\nclue to read from the latter (the OPT_FILENAME option is handled by\nprefix_filename() using the prefix).\n\nThe distinction between the two does not exist as long as you start\ninside GIT_WORK_TREE and outside GIT_DIR.\n"},{"id":"282162","messageId":"CACsJy8BAW0E36qKjJqvLL0ZHKdh3+7G1axt1jD46Yv3atfL7fw@mail.gmail.com","threadId":"41858","inReplyTo":"20160329203425.GA24027@sigill.intra.peff.net","subject":"Re: `git rev-parse --is-inside-work-tree` and $GIT_WORK_TREE","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-03-30T00:53:58Z","receivedAt":"2016-03-30T00:53:58Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Mar 30, 2016 at 3:34 AM, Jeff King <peff@peff.net> wrote:\n> On Tue, Mar 29, 2016 at 06:42:44AM -0500, Elliott Cable wrote:\n>\n>> So, I find this behaviour a little strange; I can't determine if it's\n>> a subtle bug, or intentionally undefined/‘fuzzy’ behaviour:\n>>\n>>     $ cd a-repo/.git/\n>>     $ pwd\n>>     /path/to/a-repo/.git\n>>     $ git rev-parse --is-inside-work-tree\n>>     false\n>>     $ export GIT_WORK_TREE=/path/to/a-repo\n>>     $ git rev-parse --is-inside-work-tree\n>>     true\n>>\n>> i.e. when within the repository (the `.git` directory), and when that\n>> directory is a sub-directory of the working-tree, `rev-parse\n>> --is-inside-work-tree` reports *false* (reasonable enough, I suppose);\n>> but then if `$GIT_WORK_TREE` is set to precisely the directory that\n>> git was *already* assuming was the working-directory, then the same\n>> command, in the same location, reports *true*.\n>>\n>> This should probably be made consistent: either `rev-parse\n>> --is-inside-work-tree` should report “true”, even inside the `.git`\n>> dir, as long as that directory is a sub-directory of the working-tree\n>> … or repository-directories / `$GIT_DIR` / `.git` directories should\n>> be excluded from truthy responses to `rev-parse\n>> --is-inside-work-tree`.\n\nNo. Once you set GIT_WORK_TREE you tell git that worktree exists. That\noverrides the \"bare repo\" attribute (i.e. no worktree) that's\nautomatically set when we try to find .git directory.\n\n> Yeah, I think this is a bug. Presumably what is happening is that we are\n> too eager to \"cd $GIT_WORK_TREE\" inside git-rev-parse, and by the time\n> we ask \"are we in a work tree\", the answer has become yes. But the\n> caller really wants to know \"am _I_ inside the work tree\".\n\nOn relative GIT_WORK_TREE some mail down this thread, there's this\nnote in t1510 that you might find interesting\n\n5. GIT_WORK_TREE/core.worktree was originally meant to work only if\n   GIT_DIR is set, but earlier git didn't enforce it, and some scripts\n   depend on the implementation that happened to first discover .git by\n   going up from the users $cwd and then using the specified working tree\n   that may or may not have any relation to where .git was found in.  This\n   historical behaviour must be kept.\n\nBasically if you set GIT_WORK_TREE you better set GIT_DIR too to keep\nthings sane.\n-- \nDuy\n"},{"id":"282425","messageId":"CAPZ477MmUCmTF+Pyn0wAHVjj3LsZ9M3-v2fPb3+vD9r+rEfwBw@mail.gmail.com","threadId":"41858","inReplyTo":"CACsJy8BAW0E36qKjJqvLL0ZHKdh3+7G1axt1jD46Yv3atfL7fw@mail.gmail.com","subject":"Re: `git rev-parse --is-inside-work-tree` and $GIT_WORK_TREE","fromName":"Elliott Cable","fromEmail":"me@ell.io","sentAt":"2016-04-01T00:49:26Z","receivedAt":"2016-04-01T00:49:26Z","isPatch":false,"sender":{"key":"me@ell.io","avatar":"https://gravatar.com/avatar/0d35bb7d7c29b6bbcaafdf13ec70745573d31cb222130cc754a064e707c08d63?d=mp&s=160"},"body":"oh, wow, this got over my head *real* fast. Okay,\n\n1. Yeah, my `$GIT_WORK_TREE` was def. an absolute path; I typed that\nexample code without running it *precisely* that way (entirely my\nmistake! I'm so sorry for the confusion it caused, and all that typing\nyou did!); if I remember correctly (not at the machine right now), I\nhad run `git rev-parse --show-toplevel` from a different directory,\nwith `$GIT_DIR` set, while trying to narrow down this bug, so it gave\nme an absolute path … and then copy-pasted that path, and then\nreplaced my copy-paste with the original command to make the\nbug-report example as concise as possible? oops. But, yeah, it fails\nin the manner described above with an absolute path. (Which it looks\nlike you two figured out above.)\n\n2. Re: intentions, again, it seems like you've changed your mind, but …\n\n   >  So it is a misconfiguration if you only set GIT_WORK_TREE\nwithout setting GIT_DIR.\n\n   I really, really hope not! Half the usefulness of `$GIT_WORK_TREE`\nexisting is in that mode. In fact, that's how I found `$GIT_WORK_TREE`\ndocumented and explained, in [this blog\npost](https://git-scm.com/blog/2010/04/11/environment.html) on the Git\nsite. That usage seems pretty damn useful, so I do hope it's\neventually explicitly supported … (and if that's *not* going to be the\ncase, it should be explicitly documented in the `GIT(1)` manpage,\nalongside the other documentation of the environment-variables, that\nthe behaviour is undefined is `$GIT_DIR` isn't set first. =)\n\n\n⁓ ELLIOTTCABLE — fly safe.\n  http://ell.io/tt\n"}]}