{"thread":{"id":"22047","subject":"Possible bug in 1.6.6 with reset --hard and $GIT_WORK_TREE","startedAt":"2009-12-29T06:47:24Z","lastAt":"2009-12-30T17:54:54Z","messageCount":9,"participants":["Fyn Fynn","Tay Ray Chuan","Nguyen Thai Ngoc Duy","Nanako Shiraishi","Jeff King","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"130436","messageId":"1a04eebf0912282247p6975a96el8076cb39378bd104@mail.gmail.com","threadId":"22047","inReplyTo":null,"subject":"Possible bug in 1.6.6 with reset --hard and $GIT_WORK_TREE","fromName":"Fyn Fynn","fromEmail":"fynfynn@gmail.com","sentAt":"2009-12-29T06:47:24Z","receivedAt":"2009-12-29T06:47:24Z","isPatch":false,"sender":{"key":"fynfynn@gmail.com","avatar":null},"body":"The exact same git reset command that works in 1.6.4, fails to work\nunder 1.6.6:\n\n$ GIT_WORK_TREE=$HOME/rawdata/ GIT_DIR=$HOME/rawdata/.git\n/usr/local/git-1.6.6/bin/git reset --hard\nfatal: hard reset requires a work tree\n$ GIT_WORK_TREE=$HOME/rawdata/ GIT_DIR=$HOME/rawdata/.git\n/usr/local/git-1.6.4/bin/git reset --hard\nHEAD is now at 77ec73f...\n\nWhat gives?\n"},{"id":"130439","messageId":"be6fef0d0912290012m3d64669fw4315960274baecb9@mail.gmail.com","threadId":"22047","inReplyTo":"1a04eebf0912282247p6975a96el8076cb39378bd104@mail.gmail.com","subject":"Re: Possible bug in 1.6.6 with reset --hard and $GIT_WORK_TREE","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2009-12-29T08:12:41Z","receivedAt":"2009-12-29T08:12:41Z","isPatch":false,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Tue, Dec 29, 2009 at 2:47 PM, Fyn Fynn <fynfynn@gmail.com> wrote:\n> The exact same git reset command that works in 1.6.4, fails to work\n> under 1.6.6:\n>\n> $ GIT_WORK_TREE=$HOME/rawdata/ GIT_DIR=$HOME/rawdata/.git\n> /usr/local/git-1.6.6/bin/git reset --hard\n> fatal: hard reset requires a work tree\n> $ GIT_WORK_TREE=$HOME/rawdata/ GIT_DIR=$HOME/rawdata/.git\n> /usr/local/git-1.6.4/bin/git reset --hard\n> HEAD is now at 77ec73f...\n>\n> What gives?\n\nperhaps you should also specify the --exec-path option or\nGIT_EXEC_PATH environment variable.\n\nAssuming a full installation in /usr/local/git-1.6.6/ and thus that\nthe git commands are installed at\n/usr/local/git-1.6.6/libexec/git-core/, the reset for 1.6.6 would look\nlike this:\n\n$ GIT_WORK_TREE=$HOME/rawdata/ \\\nGIT_DIR=$HOME/rawdata/.git \\\nGIT_EXEC_PATH=/usr/local/git-1.6.6/libexec/git-core/ \\\n/usr/local/git-1.6.6/bin/git reset --hard\n\n-- \nCheers,\nRay Chuan\n"},{"id":"130441","messageId":"fcaeb9bf0912290038u77a5e83cqe2f3cbfab05af263@mail.gmail.com","threadId":"22047","inReplyTo":"1a04eebf0912282247p6975a96el8076cb39378bd104@mail.gmail.com","subject":"Re: Possible bug in 1.6.6 with reset --hard and $GIT_WORK_TREE","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2009-12-29T08:38:28Z","receivedAt":"2009-12-29T08:38:28Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On 12/29/09, Fyn Fynn <fynfynn@gmail.com> wrote:\n> The exact same git reset command that works in 1.6.4, fails to work\n>  under 1.6.6:\n>\n>  $ GIT_WORK_TREE=$HOME/rawdata/ GIT_DIR=$HOME/rawdata/.git\n>  /usr/local/git-1.6.6/bin/git reset --hard\n>  fatal: hard reset requires a work tree\n>  $ GIT_WORK_TREE=$HOME/rawdata/ GIT_DIR=$HOME/rawdata/.git\n>  /usr/local/git-1.6.4/bin/git reset --hard\n>  HEAD is now at 77ec73f...\n>\n>  What gives?\n\nA recent patch by Jeff (952dfc6 (reset: improve worktree safety valves\n- 2009-12-04)) makes sure that \"git reset --hard\" will not work\noutside worktree (which is right). Where was your current directory,\ninside $GIT_WORK_TREE or outside?\n-- \nDuy\n"},{"id":"130445","messageId":"20091229200426.6117@nanako3.lavabit.com","threadId":"22047","inReplyTo":"fcaeb9bf0912290038u77a5e83cqe2f3cbfab05af263@mail.gmail.com","subject":"Re: Possible bug in 1.6.6 with reset --hard and $GIT_WORK_TREE","fromName":"Nanako Shiraishi","fromEmail":"nanako3@lavabit.com","sentAt":"2009-12-29T11:04:26Z","receivedAt":"2009-12-29T11:04:26Z","isPatch":false,"sender":{"key":"nanako3@lavabit.com","avatar":"https://gravatar.com/avatar/3777b9e201c5883a62b1a6fdf7c53f2d712d1d80989146063ea861e33aad72a8?d=mp&s=160"},"body":"Quoting Nguyen Thai Ngoc Duy <pclouds@gmail.com>:\n\n> On 12/29/09, Fyn Fynn <fynfynn@gmail.com> wrote:\n>> The exact same git reset command that works in 1.6.4, fails to work\n>>  under 1.6.6:\n>>\n>>  $ GIT_WORK_TREE=$HOME/rawdata/ GIT_DIR=$HOME/rawdata/.git\n>>  /usr/local/git-1.6.6/bin/git reset --hard\n>>  fatal: hard reset requires a work tree\n>>  $ GIT_WORK_TREE=$HOME/rawdata/ GIT_DIR=$HOME/rawdata/.git\n>>  /usr/local/git-1.6.4/bin/git reset --hard\n>>  HEAD is now at 77ec73f...\n>>\n>>  What gives?\n>\n> A recent patch by Jeff (952dfc6 (reset: improve worktree safety valves\n> - 2009-12-04)) makes sure that \"git reset --hard\" will not work\n> outside worktree (which is right).\n\nSorry, but I don't understand why it is *right*. Isn't 'git reset --hard' supposed to make all the files in the working tree match the HEAD, no matter where you start from?\n\nJeff's commit message says:\n\n        make sure we are in a worktree. Otherwise, we can end up\n\tmunging files inside of '.git'\n\nBut if you have ~/myproject/.git project, whose working tree is ~/myproject, and if you run\n\n        % cd ~/myproject/.git\n\t% git reset --hard\n\nthe code mistakenly overwrote files in ~/myproject/.git directory before Jeff's patch, and I agree it was a bug.\n\nBut shouldn't the correct fix be to go to ~/myproject, the obvious root level of the working tree, and check out the files in that directory?\n\n-- \nNanako Shiraishi\nhttp://ivory.ap.teacup.com/nanako3/\n"},{"id":"130448","messageId":"fcaeb9bf0912290336m69303c3es8138c4e6497b801a@mail.gmail.com","threadId":"22047","inReplyTo":"20091229200426.6117@nanako3.lavabit.com","subject":"Re: Possible bug in 1.6.6 with reset --hard and $GIT_WORK_TREE","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2009-12-29T11:36:09Z","receivedAt":"2009-12-29T11:36:09Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On 12/29/09, Nanako Shiraishi <nanako3@lavabit.com> wrote:\n> Quoting Nguyen Thai Ngoc Duy <pclouds@gmail.com>:\n>\n>  > On 12/29/09, Fyn Fynn <fynfynn@gmail.com> wrote:\n>  >> The exact same git reset command that works in 1.6.4, fails to work\n>  >>  under 1.6.6:\n>  >>\n>  >>  $ GIT_WORK_TREE=$HOME/rawdata/ GIT_DIR=$HOME/rawdata/.git\n>  >>  /usr/local/git-1.6.6/bin/git reset --hard\n>  >>  fatal: hard reset requires a work tree\n>  >>  $ GIT_WORK_TREE=$HOME/rawdata/ GIT_DIR=$HOME/rawdata/.git\n>  >>  /usr/local/git-1.6.4/bin/git reset --hard\n>  >>  HEAD is now at 77ec73f...\n>  >>\n>  >>  What gives?\n>  >\n>  > A recent patch by Jeff (952dfc6 (reset: improve worktree safety valves\n>  > - 2009-12-04)) makes sure that \"git reset --hard\" will not work\n>  > outside worktree (which is right).\n>\n>\n> Sorry, but I don't understand why it is *right*. Isn't 'git reset --hard' supposed to make all the files in the working tree match the HEAD, no matter where you start from?\n\nIt is generally \"right\" to work from inside worktree, the way Git\nworked before GIT_WORK_TREE came. In case of \"git reset --hard\", yes\nit'd be best if Git could just go to worktree and reset it. I forgot\nthat \"git reset --hard\" does not take pathspec. The situation may be a\nbit more complicated with \"git status\" (which also handles worktree as\na whole) because you may need to represent the filename output to be\nrelative to current working directory, not the GIT_WORK_TREE. Using\nGIT_WORK_TREE from outside worktree is imo stretching git to its\nlimits.\n-- \nDuy\n"},{"id":"130478","messageId":"1a04eebf0912291309u7a222d9ch7e0926d30a5899b7@mail.gmail.com","threadId":"22047","inReplyTo":"fcaeb9bf0912290336m69303c3es8138c4e6497b801a@mail.gmail.com","subject":"Re: Possible bug in 1.6.6 with reset --hard and $GIT_WORK_TREE","fromName":"Fyn Fynn","fromEmail":"fynfynn@gmail.com","sentAt":"2009-12-29T21:09:20Z","receivedAt":"2009-12-29T21:09:20Z","isPatch":false,"sender":{"key":"fynfynn@gmail.com","avatar":null},"body":"I went over the 952dfc6 patch, and it seems like it was designed to\nfix a specific bug - the one demonstrated by Nanako Shiraishi above.\n\nNowhere there do I see an intention to prevent the action described in\nthe original post. And even if there was such intention, it should\nhave been made explicit by providing a proper error message: the\ncurrent one simply ignores the fact that we provided GIT_WORK_TREE,\nand is thus incorrect since the work tree can be found.\n\nMoreover, if there is an intention to prevent resetting from outside\nthe worktree (why?), then probably \"checkout -f\" should be limited as\nwell.\n\nBut it is more likely that the breaking of the original action between\n1.6.4 and 1.6.6 came about as an untended consequence of 952dfc6,\nwhich oversimplified by assuming that the worktree can only be found\nif we're inside it, ignoring the possibility that GIT_WORK_TREE was\nprovided.\n\nThus, the current state should be considered a bug.\n\n--\n\nFynn\n\nOn Tue, Dec 29, 2009 at 3:36 AM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:\n> On 12/29/09, Nanako Shiraishi <nanako3@lavabit.com> wrote:\n>> Quoting Nguyen Thai Ngoc Duy <pclouds@gmail.com>:\n>>\n>>  > On 12/29/09, Fyn Fynn <fynfynn@gmail.com> wrote:\n>>  >> The exact same git reset command that works in 1.6.4, fails to work\n>>  >>  under 1.6.6:\n>>  >>\n>>  >>  $ GIT_WORK_TREE=$HOME/rawdata/ GIT_DIR=$HOME/rawdata/.git\n>>  >>  /usr/local/git-1.6.6/bin/git reset --hard\n>>  >>  fatal: hard reset requires a work tree\n>>  >>  $ GIT_WORK_TREE=$HOME/rawdata/ GIT_DIR=$HOME/rawdata/.git\n>>  >>  /usr/local/git-1.6.4/bin/git reset --hard\n>>  >>  HEAD is now at 77ec73f...\n>>  >>\n>>  >>  What gives?\n>>  >\n>>  > A recent patch by Jeff (952dfc6 (reset: improve worktree safety valves\n>>  > - 2009-12-04)) makes sure that \"git reset --hard\" will not work\n>>  > outside worktree (which is right).\n>>\n>>\n>> Sorry, but I don't understand why it is *right*. Isn't 'git reset --hard' supposed to make all the files in the working tree match the HEAD, no matter where you start from?\n>\n> It is generally \"right\" to work from inside worktree, the way Git\n> worked before GIT_WORK_TREE came. In case of \"git reset --hard\", yes\n> it'd be best if Git could just go to worktree and reset it. I forgot\n> that \"git reset --hard\" does not take pathspec. The situation may be a\n> bit more complicated with \"git status\" (which also handles worktree as\n> a whole) because you may need to represent the filename output to be\n> relative to current working directory, not the GIT_WORK_TREE. Using\n> GIT_WORK_TREE from outside worktree is imo stretching git to its\n> limits.\n> --\n> Duy\n>\n"},{"id":"130492","messageId":"20091229215015.GA1529@coredump.intra.peff.net","threadId":"22047","inReplyTo":"1a04eebf0912291309u7a222d9ch7e0926d30a5899b7@mail.gmail.com","subject":"Re: Possible bug in 1.6.6 with reset --hard and $GIT_WORK_TREE","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-12-29T21:50:15Z","receivedAt":"2009-12-29T21:50:15Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Dec 29, 2009 at 01:09:20PM -0800, Fyn Fynn wrote:\n\n> But it is more likely that the breaking of the original action between\n> 1.6.4 and 1.6.6 came about as an untended consequence of 952dfc6,\n> which oversimplified by assuming that the worktree can only be found\n> if we're inside it, ignoring the possibility that GIT_WORK_TREE was\n> provided.\n\nYes, it is an unintended breakage. We discussed the possibility of\nautomatically moving to the work tree when inside the repo, but decided\nit could be done later, as other builtins which use NEEDS_WORK_TREE\n(e.g., clean) already fail in that instance.\n\nHowever, I did not take into account that those commands fail only with\nautomatic repo detection; they _do_ work with GIT_WORK_TREE. So instead\nof just dying, we need to be emulating the code in git.c which sets up\nthe work tree (and which will die itself if we run into an error).\n\nSo I think what we really want is this:\n\ndiff --git a/builtin-reset.c b/builtin-reset.c\nindex 11d1c6e..e4418bc 100644\n--- a/builtin-reset.c\n+++ b/builtin-reset.c\n@@ -286,10 +286,8 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \tif (reset_type == NONE)\n \t\treset_type = MIXED; /* by default */\n \n-\tif ((reset_type == HARD || reset_type == MERGE)\n-\t    && !is_inside_work_tree())\n-\t\tdie(\"%s reset requires a work tree\",\n-\t\t    reset_type_names[reset_type]);\n+\tif (reset_type == HARD || reset_type == MERGE)\n+\t\tsetup_work_tree();\n \n \t/* Soft reset does not touch the index file nor the working tree\n \t * at all, but requires them in a good order.  Other resets reset\n\nI'll try to write a few tests and make sure that's sane.\n\n-Peff\n"},{"id":"130530","messageId":"20091230084702.GA19649@coredump.intra.peff.net","threadId":"22047","inReplyTo":"20091229215015.GA1529@coredump.intra.peff.net","subject":"[PATCH] reset: unbreak hard resets with GIT_WORK_TREE","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-12-30T08:47:03Z","receivedAt":"2009-12-30T08:47:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Commit 952dfc6 tried to tighten the safety valves for doing\na \"reset --hard\" in a bare repository or outside the work\ntree, but accidentally broke the case for GIT_WORK_TREE.\nThis patch unbreaks it.\n\nMost git commands which need a work tree simply use\nNEED_WORK_TREE in git.c to die before they get to their\ncmd_* function. Reset, however, only needs a work tree in\nsome cases, and so must handle the work tree itself. The\nerror that 952dfc6 made was to simply forbid certain\noperations if the work tree was not set up; instead, we need\nto do the same thing that NEED_WORK_TREE does, which is to\ncall setup_work_tree(). We no longer have to worry about dying\nin the non-worktree case, as setup_work_tree handles that\nfor us.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nJunio, this should probably go onto maint. I based it directly on\n952dfc6, which is in v1.6.6 and v1.6.5.5.\n\n builtin-reset.c       |    6 ++----\n t/t7103-reset-bare.sh |    6 ++++++\n 2 files changed, 8 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin-reset.c b/builtin-reset.c\nindex 11d1c6e..e4418bc 100644\n--- a/builtin-reset.c\n+++ b/builtin-reset.c\n@@ -286,10 +286,8 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \tif (reset_type == NONE)\n \t\treset_type = MIXED; /* by default */\n \n-\tif ((reset_type == HARD || reset_type == MERGE)\n-\t    && !is_inside_work_tree())\n-\t\tdie(\"%s reset requires a work tree\",\n-\t\t    reset_type_names[reset_type]);\n+\tif (reset_type == HARD || reset_type == MERGE)\n+\t\tsetup_work_tree();\n \n \t/* Soft reset does not touch the index file nor the working tree\n \t * at all, but requires them in a good order.  Other resets reset\ndiff --git a/t/t7103-reset-bare.sh b/t/t7103-reset-bare.sh\nindex 68041df..afb55b3 100755\n--- a/t/t7103-reset-bare.sh\n+++ b/t/t7103-reset-bare.sh\n@@ -29,6 +29,12 @@ test_expect_success 'soft reset is ok' '\n \t(cd .git && git reset --soft)\n '\n \n+test_expect_success 'hard reset works with GIT_WORK_TREE' '\n+\tmkdir worktree &&\n+\tGIT_WORK_TREE=$PWD/worktree GIT_DIR=$PWD/.git git reset --hard &&\n+\ttest_cmp file worktree/file\n+'\n+\n test_expect_success 'setup bare' '\n \tgit clone --bare . bare.git &&\n \tcd bare.git\n-- \n1.6.6.320.g7c9b3\n"},{"id":"130567","messageId":"7v8wck2vpt.fsf@alter.siamese.dyndns.org","threadId":"22047","inReplyTo":"20091230084702.GA19649@coredump.intra.peff.net","subject":"Re: [PATCH] reset: unbreak hard resets with GIT_WORK_TREE","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-12-30T17:54:54Z","receivedAt":"2009-12-30T17:54:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Commit 952dfc6 tried to tighten the safety valves for doing\n> a \"reset --hard\" in a bare repository or outside the work\n> tree, but accidentally broke the case for GIT_WORK_TREE.\n> This patch unbreaks it.\n>\n> Most git commands which need a work tree simply use\n> NEED_WORK_TREE in git.c to die before they get to their\n> cmd_* function. Reset, however, only needs a work tree in\n> some cases, and so must handle the work tree itself. The\n> error that 952dfc6 made was to simply forbid certain\n> operations if the work tree was not set up; instead, we need\n> to do the same thing that NEED_WORK_TREE does, which is to\n> call setup_work_tree(). We no longer have to worry about dying\n> in the non-worktree case, as setup_work_tree handles that\n> for us.\n\nSounds very sane to me.  Thanks.\n"}]}