{"thread":{"id":"28663","subject":"[Bug] git pull doesn't recognize --work-tree parameter","startedAt":"2011-10-13T08:38:53Z","lastAt":"2011-10-18T06:34:22Z","messageCount":10,"participants":["Kirill Likhodedov","Nguyen Thai Ngoc Duy","Jeff King","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"177533","messageId":"E95C75ED-99F2-463C-A1AB-0F8152696739@jetbrains.com","threadId":"28663","inReplyTo":null,"subject":"[Bug] git pull doesn't recognize --work-tree parameter","fromName":"Kirill Likhodedov","fromEmail":"kirill.likhodedov@jetbrains.com","sentAt":"2011-10-13T08:38:53Z","receivedAt":"2011-10-13T08:38:53Z","isPatch":false,"sender":{"key":"kirill.likhodedov@jetbrains.com","avatar":"https://gravatar.com/avatar/9fd55d2a110e8e96deb3ed7b503d7f3b9b5b1ed5ca9ef6683c15525b11b94e63?d=mp&s=160"},"body":"\n'git pull' doesn't work from outside the working tree even if '--work-tree' is specified:\n\n# git version\ngit version 1.7.6\n# git --git-dir=/Users/loki/sandbox/git/child/.git --work-tree=/Users/loki/sandbox/git/child pull\nfatal: /opt/local/libexec/git-core/git-pull cannot be used without a working tree.\n\nNote that 'git fetch' and 'git merge origin/master' work fine, so 'git pull' should be easy to fix :)\n\n# git --git-dir=/Users/loki/sandbox/git/child/.git --work-tree=/Users/loki/sandbox/git/child merge origin/master\nAlready up-to-date.\n\n\n----------------------------------\nKirill Likhodedov\nJetBrains, Inc\nhttp://www.jetbrains.com\n\"Develop with pleasure!\"\n"},{"id":"177538","messageId":"CACsJy8AgW8fg3iOKQw=vv2s20hF0PTGD9Py-eMOTBmAv+6CYWg@mail.gmail.com","threadId":"28663","inReplyTo":"E95C75ED-99F2-463C-A1AB-0F8152696739@jetbrains.com","subject":"Re: [Bug] git pull doesn't recognize --work-tree parameter","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-10-13T10:55:46Z","receivedAt":"2011-10-13T10:55:46Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Oct 13, 2011 at 7:38 PM, Kirill Likhodedov\n wrote:\n>\n> 'git pull' doesn't work from outside the working tree even if '--work-tree' is specified:\n>\n> # git version\n> git version 1.7.6\n> # git --git-dir=/Users/loki/sandbox/git/child/.git --work-tree=/Users/loki/sandbox/git/child pull\n> fatal: /opt/local/libexec/git-core/git-pull cannot be used without a working tree.\n>\n> Note that  'git fetch' and 'git merge origin/master' work fine, so 'git pull' should be easy to fix :)\n\nNot exactly. git-pull is a shell script and may have problems that a\nbuiltin command like git-fetch never has. An \"easy\" way is to just\nbuild git-pull in. I have such a (stalled) WIP since May. Thanks for\nreminding me to finish my work :)\n-- \nDuy\n"},{"id":"177562","messageId":"20111013155923.GA13134@sigill.intra.peff.net","threadId":"28663","inReplyTo":"E95C75ED-99F2-463C-A1AB-0F8152696739@jetbrains.com","subject":"Re: [Bug] git pull doesn't recognize --work-tree parameter","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-10-13T15:59:24Z","receivedAt":"2011-10-13T15:59:24Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 13, 2011 at 12:38:53PM +0400, Kirill Likhodedov wrote:\n\n> 'git pull' doesn't work from outside the working tree even if '--work-tree' is specified:\n> \n> # git version\n> git version 1.7.6\n> # git --git-dir=/Users/loki/sandbox/git/child/.git --work-tree=/Users/loki/sandbox/git/child pull\n> fatal: /opt/local/libexec/git-core/git-pull cannot be used without a working tree.\n> \n> Note that 'git fetch' and 'git merge origin/master' work fine, so 'git pull' should be easy to fix :)\n> \n> # git --git-dir=/Users/loki/sandbox/git/child/.git --work-tree=/Users/loki/sandbox/git/child merge origin/master\n> Already up-to-date.\n\nThis is a known issue, and the fix is a one-liner, but it needed\nsomebody to look through the pull script to make sure it wasn't\nintroducing any new bugs. I just did this; the patch below should fix\nyour problem.\n\n-- >8 --\nSubject: [PATCH] pull,rebase: handle GIT_WORK_TREE better\n\nYou can't currently run git-pull or git-rebase from outside\nof the work tree, even with GIT_WORK_TREE set, due to an\novereager require_work_tree function. Commit e2eb527\ndocuments this problem and provides the infrastructure for a\nfix, but left it to later commits to audit and update\nindividual scripts.\n\nChanging these scripts to use require_work_tree_exists is\neasy to verify. We immediately call cd_to_toplevel, anyway.\nTherefore no matter which function we use, the state\nafterwards is one of:\n\n 1. We have a work tree, and we are at the top level.\n\n 2. We don't have a work tree, and we have died.\n\nThe only catch is that we must also make sure no code that\nran before the cd_to_toplevel assumed that we were already\nin the working tree.\n\nIn this case, we will only have included shell libraries and\ncalled set_reflog_action, neither of which care about the\ncurrent working directory at all.\n\nSigned-off-by: Jeff King \n---\nThis is the low-hanging, obviously correct fruit. git-am and\ngit-stash also immediately cd_to_toplevel, but they look at\n$PWD or `git rev-parse --show-prefix` beforehand, which\nmeans those uses have to be audited separately.\n\n git-pull.sh | 2 +-\n git-rebase.sh | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/git-pull.sh b/git-pull.sh\nindex 63da37b..902fc4a 100755\n--- a/git-pull.sh\n+++ b/git-pull.sh\n@@ -11,7 +11,7 @@ OPTIONS_SPEC=\n . git-sh-setup\n . git-sh-i18n\n set_reflog_action \"pull${1+ $*}\"\n-require_work_tree\n+require_work_tree_exists\n cd_to_toplevel\n \n \ndiff --git a/git-rebase.sh b/git-rebase.sh\nindex 6759702..00ca7b9 100755\n--- a/git-rebase.sh\n+++ b/git-rebase.sh\n@@ -63,7 +63,7 @@ skip! skip current patch and continue\n \"\n . git-sh-setup\n set_reflog_action rebase\n-require_work_tree\n+require_work_tree_exists\n cd_to_toplevel\n \n LF='\n-- \n1.7.6.4.37.g43b58b\n"},{"id":"177572","messageId":"7vbotk6aae.fsf@alter.siamese.dyndns.org","threadId":"28663","inReplyTo":"20111013155923.GA13134@sigill.intra.peff.net","subject":"Re: [Bug] git pull doesn't recognize --work-tree parameter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-13T18:01:13Z","receivedAt":"2011-10-13T18:01:13Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Changing these scripts to use require_work_tree_exists is\n> easy to verify. We immediately call cd_to_toplevel, anyway.\n> Therefore no matter which function we use, the state\n> afterwards is one of:\n>\n> 1. We have a work tree, and we are at the top level.\n>\n> 2. We don't have a work tree, and we have died.\n>\n> The only catch is that we must also make sure no code that\n> ran before the cd_to_toplevel assumed that we were already\n> in the working tree.\n>\n> In this case, we will only have included shell libraries and\n> called set_reflog_action, neither of which care about the\n> current working directory at all.\n>\n> Signed-off-by: Jeff King \n> ---\n> This is the low-hanging, obviously correct fruit.\n\nI am not absolutely sure about \"obviously correct\", given that you assume\nthat cd_to_toplevel does what its name makes you think it does. I've been\nwondering if we want to do give a bit more sanity to \"cd_to_toplevel\" and\n\"rev-parse --show-toplevel\".\n\n $ pwd\n /srv/project/git/git.git\n $ (cd Documentation/howto && git rev-parse --show-toplevel); echo $?\n /srv/project/git/git.git\n 0\n\nSo far so good, however:\n\n $ (cd .git/refs/heads && git rev-parse --show-toplevel); echo $?\n 0\n\nI do not think this is quite right.\n\nWe would probably want to add \"rev-parse --show-work-tree\", but we would\nneed to audit the users of cd_to_toplevel before starting to use it. I\nwouldn't be surprised if there is a script that creates a temporary work\ntree in .git/some/where and runs the scripted Porcelains without setting\nGIT_WORK_TREE, relying on the historical behaviour of cd_to_toplevel that\ndoes not really go to the top level.\n"},{"id":"177577","messageId":"20111013183709.GB17573@sigill.intra.peff.net","threadId":"28663","inReplyTo":"7vbotk6aae.fsf@alter.siamese.dyndns.org","subject":"Re: [Bug] git pull doesn't recognize --work-tree parameter","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-10-13T18:37:09Z","receivedAt":"2011-10-13T18:37:09Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 13, 2011 at 11:01:13AM -0700, Junio C Hamano wrote:\n\n> I am not absolutely sure about \"obviously correct\", given that you assume\n> that cd_to_toplevel does what its name makes you think it does. I've been\n> wondering if we want to do give a bit more sanity to \"cd_to_toplevel\" and\n> \"rev-parse --show-toplevel\".\n> \n> $ pwd\n> /srv/project/git/git.git\n> $ (cd Documentation/howto && git rev-parse --show-toplevel); echo $?\n> /srv/project/git/git.git\n> 0\n> \n> So far so good, however:\n> \n> $ (cd .git/refs/heads && git rev-parse --show-toplevel); echo $?\n> 0\n> \n> I do not think this is quite right.\n\nUgh. You are right. I for some reason assumed that cd_to_toplevel would,\nof all things, cd to the toplevel. I think the right solution is to\nintroduce a \"cd_to_work_tree_toplevel\" (or similarly named) command that\nalways moves to the root of the work tree.\n\nAnd then convert the two scripts in my patch to use it (along with the\nchange to require_work_tree_exists). That would make my prior analysis\nhold, then, as the annoying do-nothing behavior of \"cd_to_toplevel\" only\nkicks in when we are outside the work tree (i.e., it could not have\nhappened before in those scripts, because the existing require_work_tree\ncall would cause us to die).\n\n> We would probably want to add \"rev-parse --show-work-tree\", but we would\n> need to audit the users of cd_to_toplevel before starting to use it. I\n> wouldn't be surprised if there is a script that creates a temporary work\n> tree in .git/some/where and runs the scripted Porcelains without setting\n> GIT_WORK_TREE, relying on the historical behaviour of cd_to_toplevel that\n> does not really go to the top level.\n\nRight. I suspect the proposed behavior for cd_to_toplevel is what they\nall would want eventually, but some scripts may need minor tweaks. I\nthink we should follow the same path as require_work_tree_exists, and\nintroduce the new function, use it where we know it's safe, and then\neventually get rid of the old one.\n\nThe real trick is coming up with a good name, because cd_to_toplevel is\nalready taken. :)\n\n-Peff\n"},{"id":"177587","messageId":"7v62js4sop.fsf@alter.siamese.dyndns.org","threadId":"28663","inReplyTo":"20111013183709.GB17573@sigill.intra.peff.net","subject":"Re: [Bug] git pull doesn't recognize --work-tree parameter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-13T19:06:46Z","receivedAt":"2011-10-13T19:06:46Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> And then convert the two scripts in my patch to use it (along with the\n> change to require_work_tree_exists). That would make my prior analysis\n> hold, then, as the annoying do-nothing behavior of \"cd_to_toplevel\" only\n> kicks in when we are outside the work tree (i.e., it could not have\n> happened before in those scripts, because the existing require_work_tree\n> call would cause us to die).\n> ...\n> Right. I suspect the proposed behavior for cd_to_toplevel is what they\n> all would want eventually, but some scripts may need minor tweaks. I\n> think we should follow the same path as require_work_tree_exists, and\n> introduce the new function, use it where we know it's safe, and then\n> eventually get rid of the old one.\n>\n> The real trick is coming up with a good name, because cd_to_toplevel is\n> already taken. :)\n\nIt is not as simple as that I am afraid. We could introduce cd_to_top with\nthe new semantics and use it in pull and rebase, but a case that would\nbreak is for a script (let's call that hypothetical operation \"git svn\ndcommit\", even though I do not know if dcommit uses the real working tree\nor a temporary one) that prepares a temporary working tree inside .git/svn/\nand run \"git rebase\" there without setting GIT_WORKING_TREE to point at\nthe temporary directory.\n\nWith cd_to_toplevel, such a \"rebase\" would work and \"git svn dcommit\" can\ntake that result and do whatever it wants to the real working tree after\nit finishes. When we start using cd_to_top in the updated \"rebase\", such a\nscript suddenly breaks, as we would start touching the real working tree.\n\nSo I do not think it makes much sense to add cd_to_top with updated\nsemantics while keeping cd_to_toplevel.\n\nWhat we could do is to update cd_to_toplevel so that it would notice and\nwarn when the results between the historical incorrect behaviour and the\nupdated behaviour would be different. The warning can first read \"You are\nrunning 'rebase' somewhere in $GIT_DIR without setting $GIT_WORK_TREE; we\nhistorically used the directory you started 'rebase' as the top level of\nthe working tree, and this version continues to do so, but it will change\nto work on the real working tree associated with your $GIT_DIR in future\nversions of git. Update your script to correctly set $GIT_WORK_TREE\", and\nthen we transition to start using the new semantics while rewording the\nwarning message, and then later remove the warning altogether.\n"},{"id":"177588","messageId":"20111013191457.GA18460@sigill.intra.peff.net","threadId":"28663","inReplyTo":"7v62js4sop.fsf@alter.siamese.dyndns.org","subject":"Re: [Bug] git pull doesn't recognize --work-tree parameter","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-10-13T19:14:57Z","receivedAt":"2011-10-13T19:14:57Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 13, 2011 at 12:06:46PM -0700, Junio C Hamano wrote:\n\n> It is not as simple as that I am afraid. We could introduce cd_to_top with\n> the new semantics and use it in pull and rebase, but a case that would\n> break is for a script (let's call that hypothetical operation \"git svn\n> dcommit\", even though I do not know if dcommit uses the real working tree\n> or a temporary one) that prepares a temporary working tree inside .git/svn/\n> and run \"git rebase\" there without setting GIT_WORKING_TREE to point at\n> the temporary directory.\n\nI didn't think that could happen now, because you would not be in the\nworking tree, and therefore require_work_tree would fail. E.g., with\ncurrent git I get:\n\n $ mkdir .git/tmp\n $ cd .git/tmp\n $ git rebase\n fatal: fatal: /home/peff/local/git/private/libexec/git-core/git-rebase\n cannot be used without a working tree.\n\nSo that case is already broken. The only change this would make is that\nwhat used to fail would not actually take them to the top-level of the\nworking tree[1].\n\n-Peff\n\n[1] Actually, I am not sure it would do that. If we are in $GIT_DIR, do\nwe necessarily know where the working tree is? I guess in a non-bare\nrepo, we assume it is $GIT_DIR/..?\n"},{"id":"177589","messageId":"20111013194432.GA20082@sigill.intra.peff.net","threadId":"28663","inReplyTo":"20111013191457.GA18460@sigill.intra.peff.net","subject":"Re: [Bug] git pull doesn't recognize --work-tree parameter","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-10-13T19:44:32Z","receivedAt":"2011-10-13T19:44:32Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 13, 2011 at 03:14:57PM -0400, Jeff King wrote:\n\n> On Thu, Oct 13, 2011 at 12:06:46PM -0700, Junio C Hamano wrote:\n> \n> > It is not as simple as that I am afraid. We could introduce cd_to_top with\n> > the new semantics and use it in pull and rebase, but a case that would\n> > break is for a script (let's call that hypothetical operation \"git svn\n> > dcommit\", even though I do not know if dcommit uses the real working tree\n> > or a temporary one) that prepares a temporary working tree inside .git/svn/\n> > and run \"git rebase\" there without setting GIT_WORKING_TREE to point at\n> > the temporary directory.\n> \n> I didn't think that could happen now, because you would not be in the\n> working tree, and therefore require_work_tree would fail. E.g., with\n> current git I get:\n> \n> $ mkdir .git/tmp\n> $ cd .git/tmp\n> $ git rebase\n> fatal: fatal: /home/peff/local/git/private/libexec/git-core/git-rebase\n> cannot be used without a working tree.\n> \n> So that case is already broken. The only change this would make is that\n> what used to fail would not actually take them to the top-level of the\n> working tree[1].\n\nUgh. It does work if you do:\n\n mkdir .git/tmp\n cd .git/tmp\n GIT_DIR=$PWD/.. git rebase\n\nWhat a god-awful mess our initialization rules are.\n\n-Peff\n"},{"id":"177595","messageId":"7vwrc839ec.fsf@alter.siamese.dyndns.org","threadId":"28663","inReplyTo":"20111013191457.GA18460@sigill.intra.peff.net","subject":"Re: [Bug] git pull doesn't recognize --work-tree parameter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-13T20:48:43Z","receivedAt":"2011-10-13T20:48:43Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I didn't think that could happen now, because you would not be in the\n> working tree, and therefore require_work_tree would fail.\n\nOk, then I was worried about a non-existing problem, which is good.\n\nThen we can introduce cd_to_top that is more sensible than cd_to_toplevel\nand perhaps add some warning to the latter about deprecation and\nmigration.\n\nThanks.\n"},{"id":"177927","messageId":"7vhb36iz9t.fsf@alter.siamese.dyndns.org","threadId":"28663","inReplyTo":"20111013194432.GA20082@sigill.intra.peff.net","subject":"Re: [Bug] git pull doesn't recognize --work-tree parameter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-18T06:34:22Z","receivedAt":"2011-10-18T06:34:22Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n>> $ mkdir .git/tmp\n>> $ cd .git/tmp\n>> $ git rebase\n>> fatal: fatal: /home/peff/local/git/private/libexec/git-core/git-rebase\n>> cannot be used without a working tree.\n>> \n>> So that case is already broken. The only change this would make is that\n>> what used to fail would not actually take them to the top-level of the\n>> working tree[1].\n>\n> Ugh. It does work if you do:\n>\n> mkdir .git/tmp\n> cd .git/tmp\n> GIT_DIR=$PWD/.. git rebase\n\nActually, this one is OK. Presense of GIT_DIR combined with the lack of\nGIT_WORK_TREE means that .git/tmp must be the top of the working tree, so\nit is the script's responsibility to populate the directory with what\nmatches to $GIT_DIR/index between \"mkdir\" and \"rebase\" in the above\nsequence.\n\nSo I suspect we won't have to worry about this case either.\n"}]}