{"thread":{"id":"52690","subject":"Inconsistent results from git rev-parse --show-toplevel","startedAt":"2020-01-24T18:08:00Z","lastAt":"2020-02-14T06:36:51Z","messageCount":8,"participants":["Crabtree, Andrew","Junio C Hamano","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"390372","messageId":"TU4PR8401MB111758B9513DD7D8B96CBFAAF90E0@TU4PR8401MB1117.NAMPRD84.PROD.OUTLOOK.COM","threadId":"52690","inReplyTo":null,"subject":"Inconsistent results from git rev-parse --show-toplevel","fromName":"Crabtree, Andrew","fromEmail":"andrew.crabtree@hpe.com","sentAt":"2020-01-24T18:07:55Z","receivedAt":"2020-01-24T18:08:00Z","isPatch":false,"sender":{"key":"andrew.crabtree@hpe.com","avatar":null},"body":"I ran into an issue where 'git rev-parse --show-toplevel' is reporting the current directory instead of the root of the repository.\nIn my case I'm running it from within the pre-commit hook, and it seems like it might be related to GIT_DIR being set. \nIt may also be related to using worktrees. Haven't had a chance to test thoroughly.\n\nRegards,\n-Andrew\n\nGit Version - 2.25.0\n\npre-commit hook\n\n#!/bin/bash\nenv\npwd\ngit rev-parse --show-toplevel\ncd subdir\npwd\ngit-rev-parse --show-toplevel\n\nCreate a repo with a subdirectory. Commit as normal. Output from the hook looks as expected. Create a worktree, and commit there and it appears as though it is returning incorrect results after changing directory.\n\nGit repo is in /tmp/revparse_test\nWorktree is in /tmp/revparse_test_worktree\n\nCommitting in /tmp/revparse_test yields the below, as expected \n/tmp/revparse_test\n/tmp/revparse_test\n/tmp/revparse_test/subdir\n/tmp/revparse_test\n\n\nCommitting in the worktree yields below, with the final line seeming incorrect.\n/tmp/revparse_test_worktree\n/tmp/revparse_test_worktree\n/tmp/revparse_test_worktree/subdir\n/tmp/revparse_test_worktree/subdir\n"},{"id":"390385","messageId":"xmqqftg4zkvo.fsf@gitster-ct.c.googlers.com","threadId":"52690","inReplyTo":"TU4PR8401MB111758B9513DD7D8B96CBFAAF90E0@TU4PR8401MB1117.NAMPRD84.PROD.OUTLOOK.COM","subject":"Re: Inconsistent results from git rev-parse --show-toplevel","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-01-24T19:59:07Z","receivedAt":"2020-01-24T19:59:12Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Crabtree, Andrew\" writes:\n\n> I ran into an issue where 'git rev-parse --show-toplevel' is reporting the current directory instead of the root of the repository.\n> In my case I'm running it from within the pre-commit hook, and it seems like it might be related to GIT_DIR being set. \n\nRunning \"git\" with GIT_DIR exported (or \"git --git-dir=\") tells\nGit that it should not perform the usual \"does this directory has a\nsubdirectory .git that is a repository? If not, go one level up and\nrepeat the check\" repository discovery. Because the repository\ndiscovery serves the purpose of finding both the repository and the\ntop of the working tree, disabling the discovery means you either\nneed to tell Git where the top of the working tree is, or you are\nalready at the top of the working tree by chdir()'ing to it.\n\nBecause you do not know where the top level is (otherwise you would\nnot be asking \"rev-parse --show-toplevel\" about it), either is\nan option for you, but you can \"unset GIT_DIR\" to stop telling Git\nthat it should not perform the repository discovery.\n"},{"id":"390464","messageId":"TU4PR8401MB1117B81EB9240905AA36B1E9F9090@TU4PR8401MB1117.NAMPRD84.PROD.OUTLOOK.COM","threadId":"52690","inReplyTo":"xmqqftg4zkvo.fsf@gitster-ct.c.googlers.com","subject":"RE: Inconsistent results from git rev-parse --show-toplevel","fromName":"Crabtree, Andrew","fromEmail":"andrew.crabtree@hpe.com","sentAt":"2020-01-25T19:31:20Z","receivedAt":"2020-01-25T19:31:27Z","isPatch":false,"sender":{"key":"andrew.crabtree@hpe.com","avatar":null},"body":"> From: Junio C Hamano [mailto:gitster@pobox.com] \n> Because you do not know where the top level is (otherwise you would not be asking \"rev-parse --show-toplevel\" about it), either is an option for you, \n> but you can \"unset GIT_DIR\" to stop telling Git that it should not perform the repository discovery.\n\nGotcha, thanks. Would it make sense to have 'git rev-parse --show-toplevel' error out if GIT_DIR is set? Or update the rev-parse documentation with a warning about GIT_DIR? \n\nRegards,\n-Andrew\n"},{"id":"390465","messageId":"20200125195319.GA5519@coredump.intra.peff.net","threadId":"52690","inReplyTo":"TU4PR8401MB1117B81EB9240905AA36B1E9F9090@TU4PR8401MB1117.NAMPRD84.PROD.OUTLOOK.COM","subject":"Re: Inconsistent results from git rev-parse --show-toplevel","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-01-25T19:53:19Z","receivedAt":"2020-01-25T19:53:22Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jan 25, 2020 at 07:31:20PM +0000, Crabtree, Andrew wrote:\n\n> > From: Junio C Hamano [mailto:gitster@pobox.com] \n> > Because you do not know where the top level is (otherwise you would not be asking \"rev-parse --show-toplevel\" about it), either is an option for you, \n> > but you can \"unset GIT_DIR\" to stop telling Git that it should not\n> > perform the repository discovery.\n> \n> Gotcha, thanks. Would it make sense to have 'git rev-parse\n> --show-toplevel' error out if GIT_DIR is set? Or update the rev-parse\n> documentation with a warning about GIT_DIR? \n\nNo, it shouldn't be an error; setting GIT_DIR means that your current\ndirectory is the worktree. That _is_ confusing in some situations, but\nat this point it's a historical thing that I don't think we want to\nchange. Discussing it in the documentation might be good (it's probably\nin there somewhere already, but a pointer from --show-toplevel doesn't\nseem unreasonable).\n\nBut the bigger thing, I think, is: who is setting GIT_DIR but not\nsetting GIT_WORK_TREE to match? Because IMHO that's the situation that\nis causing the confusion.\n\n-Peff\n"},{"id":"390470","messageId":"TU4PR8401MB1117E8EEB853728B0392D83CF9090@TU4PR8401MB1117.NAMPRD84.PROD.OUTLOOK.COM","threadId":"52690","inReplyTo":"20200125195319.GA5519@coredump.intra.peff.net","subject":"RE: Inconsistent results from git rev-parse --show-toplevel","fromName":"Crabtree, Andrew","fromEmail":"andrew.crabtree@hpe.com","sentAt":"2020-01-25T22:22:45Z","receivedAt":"2020-01-25T22:22:59Z","isPatch":false,"sender":{"key":"andrew.crabtree@hpe.com","avatar":null},"body":"> But the bigger thing, I think, is: who is setting GIT_DIR but not setting GIT_WORK_TREE to match? Because IMHO that's the situation that is causing the confusion.\nPre-commit hook when worktrees are used? \n\npre-commit\n#!/bin/bash\nenv | grep GIT\n\n/tmp/pre_commit_test_worktree (new_branch)$ git add frob\n/tmp/pre_commit_test_worktree (new_branch)$ git commit -m \"frob\"\nGIT_DIR=/tmp/pre_commit_test/.git/worktrees/pre_commit_test_worktree\nGIT_EDITOR=:\nGIT_INDEX_FILE=/tmp/pre_commit_test/.git/worktrees/pre_commit_test_worktree/index\nGIT_PREFIX=\nGIT_AUTHOR_DATE=@1579990789 -0800\nGIT_AUTHOR_NAME=Andrew Crabtree\nGIT_EXEC_PATH=/usr/local/libexec/git-core\nGIT_AUTHOR_EMAIL=andrew.crabtree@hpe.com\n[new_branch 7b1b747] frob\n 1 file changed, 0 insertions(+), 0 deletions(-)\n create mode 100644 frob\n"},{"id":"390781","messageId":"20200130102933.GE840531@coredump.intra.peff.net","threadId":"52690","inReplyTo":"TU4PR8401MB1117E8EEB853728B0392D83CF9090@TU4PR8401MB1117.NAMPRD84.PROD.OUTLOOK.COM","subject":"Re: Inconsistent results from git rev-parse --show-toplevel","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-01-30T10:29:33Z","receivedAt":"2020-01-30T10:29:36Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jan 25, 2020 at 10:22:45PM +0000, Crabtree, Andrew wrote:\n\n> > But the bigger thing, I think, is: who is setting GIT_DIR but not setting GIT_WORK_TREE to match? Because IMHO that's the situation that is causing the confusion.\n> Pre-commit hook when worktrees are used? \n> \n> pre-commit\n> #!/bin/bash\n> env | grep GIT\n> \n> /tmp/pre_commit_test_worktree (new_branch)$ git add frob\n> /tmp/pre_commit_test_worktree (new_branch)$ git commit -m \"frob\"\n> GIT_DIR=/tmp/pre_commit_test/.git/worktrees/pre_commit_test_worktree\n> GIT_EDITOR=:\n> GIT_INDEX_FILE=/tmp/pre_commit_test/.git/worktrees/pre_commit_test_worktree/index\n> GIT_PREFIX=\n> GIT_AUTHOR_DATE=@1579990789 -0800\n> GIT_AUTHOR_NAME=Andrew Crabtree\n> GIT_EXEC_PATH=/usr/local/libexec/git-core\n> GIT_AUTHOR_EMAIL=andrew.crabtree@hpe.com\n> [new_branch 7b1b747] frob\n> 1 file changed, 0 insertions(+), 0 deletions(-)\n> create mode 100644 frob\n\nI dug into this a little bit. I think the culprit is actually that our\ninternal set_git_dir() puts GIT_DIR into the environment, but the\nmatching setup_work_tree() doesn't touch GIT_WORK_TREE unless it's\nalready set.\n\nThis is mostly fine because we chdir() to the top-level of the working\ntree, meaning that any sub-processes would see the correct environment.\nBut if we execute some arbitrary script (like a hook) that does a chdir,\nthe results are surprising.\n\nSomething like this seems like it would be an improvement:\n\ndiff --git a/setup.c b/setup.c\nindex e2a479a64f..75e2d1393c 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -394,12 +394,7 @@ void setup_work_tree(void)\n \tif (!work_tree || chdir_notify(work_tree))\n \t\tdie(_(\"this operation must be run in a work tree\"));\n \n-\t/*\n-\t * Make sure subsequent git processes find correct worktree\n-\t * if $GIT_WORK_TREE is set relative\n-\t */\n-\tif (getenv(GIT_WORK_TREE_ENVIRONMENT))\n-\t\tsetenv(GIT_WORK_TREE_ENVIRONMENT, \".\", 1);\n+\tsetenv(GIT_WORK_TREE_ENVIRONMENT, work_tree, 1);\n \n \tinitialized = 1;\n }\n\nbut it fails a test in t5601 around git-clone. So there may be some\nweird subtle interaction here (or possibly just a bug in git-clone, if\nit isn't careful enough to clean its environment when moving into the\nnewly created repo).\n\n-Peff\n"},{"id":"390850","messageId":"TU4PR8401MB111777B793EA9C3D5336CE26F9040@TU4PR8401MB1117.NAMPRD84.PROD.OUTLOOK.COM","threadId":"52690","inReplyTo":"20200130102933.GE840531@coredump.intra.peff.net","subject":"RE: Inconsistent results from git rev-parse --show-toplevel","fromName":"Crabtree, Andrew","fromEmail":"andrew.crabtree@hpe.com","sentAt":"2020-01-30T21:59:31Z","receivedAt":"2020-01-30T22:00:19Z","isPatch":false,"sender":{"key":"andrew.crabtree@hpe.com","avatar":null},"body":"From: Jeff King [mailto:peff@peff.net] \nSent: Thursday, January 30, 2020 2:30 AM\nTo: Crabtree, Andrew \nCc: Junio C Hamano ; git@vger.kernel.org\nSubject: Re: Inconsistent results from git rev-parse --show-toplevel\n> > But the bigger thing, I think, is: who is setting GIT_DIR but not setting GIT_WORK_TREE to match? Because IMHO that's the situation that is causing the confusion.\n\n> but it fails a test in t5601 around git-clone. \n\nThanks jeff. It looks like this might have been tried previously and abandoned? I'm pretty far out of my league here in terms of how things are supposed to operate and any history around the previous attempts at making it work.\n\n-A \n\n\ncommit d95138e695d99d32dcad528a2a7974f434c51e79\nAuthor: Nguyễn Thái Ngọc Duy \nDate: Fri Jun 26 17:37:35 2015 +0700\n\nsetup: set env $GIT_WORK_TREE when work tree is set, like $GIT_DIR\n\nWhich was subsequently reverted \n\ncommit 86d26f240fcb4f287258ad459efc2b5e30e60cfd\nAuthor: Nguyễn Thái Ngọc Duy \nDate: Sun Dec 20 14:50:18 2015 +0700\n\n setup.c: re-fix d95138e (setup: set env $GIT_WORK_TREE when ..\n \n Commit d95138e [1] attempted to fix a .git file problem by\n setting GIT_WORK_TREE whenever GIT_DIR is set. It sounded harmless\n because we handle GIT_DIR and GIT_WORK_TREE side by side for most\n commands, with two exceptions: git-init and git-clone.\n \n \"git clone\" is not happy with d95138e.\n"},{"id":"391741","messageId":"20200214063649.GD605125@coredump.intra.peff.net","threadId":"52690","inReplyTo":"TU4PR8401MB111777B793EA9C3D5336CE26F9040@TU4PR8401MB1117.NAMPRD84.PROD.OUTLOOK.COM","subject":"Re: Inconsistent results from git rev-parse --show-toplevel","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-02-14T06:36:49Z","receivedAt":"2020-02-14T06:36:51Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jan 30, 2020 at 09:59:31PM +0000, Crabtree, Andrew wrote:\n\n> > > But the bigger thing, I think, is: who is setting GIT_DIR but not\n> > > setting GIT_WORK_TREE to match? Because IMHO that's the situation\n> > > that is causing the confusion.\n> \n> > but it fails a test in t5601 around git-clone. \n> \n> Thanks jeff. It looks like this might have been tried previously and\n> abandoned? I'm pretty far out of my league here in terms of how\n> things are supposed to operate and any history around the previous\n> attempts at making it work.\n> [...]\n> commit d95138e695d99d32dcad528a2a7974f434c51e79\n\nYeah, the commit you found was doing exactly the thing I suggested. IMHO\nthe right path forward is to actually fix the weirdness in git-clone. It\nwould be a backwards incompatibility, but a pretty obscure one. I think\nwe're likely to help more people than hurt by being able to handle\n$GIT_WORK_TREE consistently. At least that's my gut feeling.\n\nI guess one way to fix it without breaking compatibility would be for us\nto set $GIT_WORK_TREE alongside $GIT_DIR, but _also_ set a special\n$GIT_CLONE_NO_RESPECT_WORK_TREE variable that would instruct it that the\ncaller isn't trying to do trigger the special $GIT_WORK_TREE behavior.\n\nBut we'd have to carry that hack around forever, which doesn't excite\nme.\n\n-Peff\n"}]}