{"thread":{"id":"52581","subject":"Broken branch after git commit - tracked files in staging area can't be removed with restore --staged, or commit or stash","startedAt":"2020-01-07T12:55:56Z","lastAt":"2020-02-05T20:24:09Z","messageCount":13,"participants":["Torsten Krah","Jeff King","Junio C Hamano","Dennis Kaarsemaker"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"389365","messageId":"07c84224bb0b093ab3770be9b5ab2ec23ce2d31a.camel@gmail.com","threadId":"52581","inReplyTo":null,"subject":"Broken branch after git commit - tracked files in staging area can't be removed with restore --staged, or commit or stash","fromName":"Torsten Krah","fromEmail":"krah.tm@gmail.com","sentAt":"2020-01-07T12:55:50Z","receivedAt":"2020-01-07T12:55:56Z","isPatch":false,"sender":{"key":"krah.tm@gmail.com","avatar":null},"body":"Hi,\n\nas written here https://git-scm.com/community I should sent reports to\nthis list - if I am mistaken please tell me where to direct this\nquestion / problem I've encountered.\n\nI've got a problem with a git branch and as this is the second time now\n(I never hit that problem before I've started using the \"new\" git\nrestore --staged command - but since I am using it its the second time\nnow - maybe by accident, maybe related - I don't know).\n\nMaybe someone can help me with this matter.\n\nI am using 2.24.1 git and my starting point was a local branch and I\nwanted to removed some files from the last commit I made.\nSo I did this:\n\n1. git reset --soft HEAD~1\n\nNow I had all my files from the last commit in the staging area and to\nremove some of them I did:\n\n2. git restore --staged $my-files\n\nThis worked too - now those files were moved to the working tree in git\nstatus and were untracked and the rest looked still fine in the staging\narea.\n\n3. git commit\n\nNow I made my commit - had a look on that with git show and it looks\nfine.\n\nBut now I've hit the problem - looking with \"git status\" I was left now\nwith 6 files in the staging area all marked as deleted - still they are\nin the working tree.\nBut I did never deleted them and the most annoying part now - I can't\nfix that.\n\nI tried:\n\n1. git restore --staged\n\nDoes not change anything, still deleted in the staging area.\n\n2. git stash\n\nDoes create a stash entry (did not look at the content though yet) but\nstaging area still unchanged.\n\n3. git commit\n\nDoes create a commit but the whole commit is empty - nothing from the\ntracked staging area is there and \"git status\" is unchanged - still 6\ntracked deleted files.\n\nI don't know what went wrong and I am unable to fix it - help\nappreciated.\nI can't share the whole git repository but I can try whatever git\ncommand or other tool output may help here to shed some light on this -\nas written I don't have a clear reproducer, I just have my branch with\na borked staging area.\n\nthx and kind regards\n\nTorsten\n\nPS: Of cause I can delete the whole repo and clone it again or remove\nthe branch and start from a fresh new one ;) but I want to analyse\nthis.\n\n\n"},{"id":"389369","messageId":"234df85965f8a685be5e563fe795ed477f359d7c.camel@gmail.com","threadId":"52581","inReplyTo":"07c84224bb0b093ab3770be9b5ab2ec23ce2d31a.camel@gmail.com","subject":"Re: Broken branch after git commit - tracked files in staging area can't be removed with restore --staged, or commit or stash","fromName":"Torsten Krah","fromEmail":"krah.tm@gmail.com","sentAt":"2020-01-07T13:43:27Z","receivedAt":"2020-01-07T13:43:32Z","isPatch":false,"sender":{"key":"krah.tm@gmail.com","avatar":null},"body":"Am Dienstag, den 07.01.2020, 13:55 +0100 schrieb Torsten Krah:\n> 3. git commit\n> \n> Now I made my commit - had a look on that with git show and it looks\n> fine.\n\nI had a second look on this step and the result it wrong.\n\nAlthough restore --staged moved my unwanted files away from the staging\narea and \"git status\" told me that they are not \"in\" the commit the\ncommit itself did still include them.\n\nSo they are listed as unversioned now but are in the commit although\ngit status told me otherwise - weird.\n\nkind regards\n\nTorsten\n\n"},{"id":"389383","messageId":"f0638fc0d09c213b661d2b244d3457f362daebe0.camel@gmail.com","threadId":"52581","inReplyTo":"234df85965f8a685be5e563fe795ed477f359d7c.camel@gmail.com","subject":"Re: Broken branch after git commit - tracked files in staging area can't be removed with restore --staged, or commit or stash","fromName":"Torsten Krah","fromEmail":"krah.tm@gmail.com","sentAt":"2020-01-07T15:28:24Z","receivedAt":"2020-01-07T15:28:29Z","isPatch":false,"sender":{"key":"krah.tm@gmail.com","avatar":null},"body":"Am Dienstag, den 07.01.2020, 14:43 +0100 schrieb Torsten Krah:\n> Although restore --staged moved my unwanted files away from the\n> staging\n> area and \"git status\" told me that they are not \"in\" the commit the\n> commit itself did still include them.\n\nI can reproduce that (locally) at least:\n\nWhat does *not* work for me:\n\n   git clone XX main\n   cd main\n   git fetch XX && git checkout FETCH_HEAD\n   git checkout -b TEST\n   git reset --soft HEAD~1\n   git restore --staged $FILES\n\ngit status now lists $FILES as unstaged and they are not included in\nthe staging area.\n\n   git commit\n\n-> now $FILES are included in the commit (I would expect them not to be\nincluded - right?) and git status does list those still in the working\narea.\n\nWhat does work:\n\n   git clone XX main\n   cd main\n   git fetch XX && git checkout FETCH_HEAD\n   git checkout -b TEST\n   git reset --soft HEAD~1\n   git reset HEAD $FILES\n\ngit status now lists $FILES as unstaged and they are not included in\nthe staging area.\n\n   git commit\n\nproduces a commit where $FILES are not included and they are still in\nthe working area, unstaged - like expected.\n\n\n\ngit status tells me this in the staging area part:\n\nChanges to be committed:\n  (use \"git restore --staged <file>...\" to unstage)\n\nI did that and its not working (for me) - looks at least like a bug or\nI am doing something wrong and I am just too dumb at the moment to see\nmy failure.\n\nCheers\n\nTorsten\n\nPS: $FILES are files which are all \"new\" and first time added in the\ncommit I want to modify with restore.\n\nPPS: The second problem with those staged deleted, unstageable,\nuncomittable files still persists in my copy of those branch (I can't\nreproduce that - still I have the repository in that state).\n\n\n"},{"id":"389457","messageId":"20200108091119.GB87523@coredump.intra.peff.net","threadId":"52581","inReplyTo":"f0638fc0d09c213b661d2b244d3457f362daebe0.camel@gmail.com","subject":"Re: Broken branch after git commit - tracked files in staging area can't be removed with restore --staged, or commit or stash","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-01-08T09:11:19Z","receivedAt":"2020-01-08T09:11:22Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jan 07, 2020 at 04:28:24PM +0100, Torsten Krah wrote:\n\n> I can reproduce that (locally) at least:\n> \n> What does *not* work for me:\n> \n>    git clone XX main\n>    cd main\n>    git fetch XX && git checkout FETCH_HEAD\n>    git checkout -b TEST\n>    git reset --soft HEAD~1\n>    git restore --staged $FILES\n> \n> git status now lists $FILES as unstaged and they are not included in\n> the staging area.\n> \n>    git commit\n> \n> -> now $FILES are included in the commit (I would expect them not to be\n> included - right?) and git status does list those still in the working\n> area.\n\nThat step seems wrong, and I can't reproduce it here. If \"git status\"\nlists the files as unstaged, then \"git commit\" should not be committing\nthem. Can you show us a more complete example that we can run ourselves\n(i.e., that does not rely on whatever is in \"main\", and what is in\n$FILES)? Barring that, can you show us the output of the commands, as\nwell as \"git show FETCH_HEAD FETCH_HEAD~1\"?\n\n-Peff\n"},{"id":"389461","messageId":"2423f8c0b91578c0faf7527b7d97b0e1e9666261.camel@gmail.com","threadId":"52581","inReplyTo":"20200108091119.GB87523@coredump.intra.peff.net","subject":"Re: Broken branch after git commit - tracked files in staging area can't be removed with restore --staged, or commit or stash","fromName":"Torsten Krah","fromEmail":"krah.tm@gmail.com","sentAt":"2020-01-08T10:02:41Z","receivedAt":"2020-01-08T10:02:47Z","isPatch":false,"sender":{"key":"krah.tm@gmail.com","avatar":null},"body":"Am Mittwoch, den 08.01.2020, 04:11 -0500 schrieb Jeff King:\n> That step seems wrong, and I can't reproduce it here. If \"git status\"\n> lists the files as unstaged, then \"git commit\" should not be\n> committing\n> them. Can you show us a more complete example that we can run\n> ourselves\n> (i.e., that does not rely on whatever is in \"main\", and what is in\n> $FILES)? Barring that, can you show us the output of the commands, as\n> well as \"git show FETCH_HEAD FETCH_HEAD~1\"?\n> \n> -Peff\n\nHi Jeff, I have a poc you can try:\n\ncd /tmp\nmkdir testrepo\ncd testrepo\ntouch TEST1 TEST2\ngit add -A\ngit commit -m First\ntouch TEST3 TEST4\ngit add -A\ngit commit -m Second\ngit reset --soft HEAD~1\n\ngit status\n\n   Auf Branch master\n   Zum Commit vorgemerkte Änderungen:\n     (benutzen Sie \"git restore --staged <Datei>...\" zum Entfernen aus\n   der Staging-Area)\n   \tneue Datei:     TEST3\n   \tneue Datei:     TEST4\n\ngit restore --staged TEST3\n\n   [10:57:26][tkrah@torstenknbl:/tmp/testrepo]  (master) $ LC_ALL=C git\n   status\n   On branch master\n   Changes to be committed:\n     (use \"git restore --staged <file>...\" to unstage)\n   \tnew file:   TEST4\n\n   Untracked files:\n     (use \"git add <file>...\" to include in what will be committed)\n   \tTEST3\n\ngit commit -m Second\n\n   [master 5b62331] Second\n    2 files changed, 0 insertions(+), 0\n   deletions(-)\n    create mode 100644 TEST3\n    create mode 100644 TEST4\n\nAnd now TEST3 is in the commit and what is even more \"interesting\" is\nthe next one:\n\n   [10:59:16][tkrah@torstenknbl:/tmp/testrepo]  (master) $ LC_ALL=C git\nstatus\n   On branch master\n   Changes to be committed:\n     (use \"git restore --staged <file>...\" to unstage)\n   \tdeleted:    TEST3\n\n   Untracked files:\n     (use \"git add <file>...\" to include in what will be committed)\n   \tTEST3\n\nTEST3 is unstaged and deleted now.\n\nThis seems wrong - or did I something wrong?\n\nCheers\n\nTorsten\n\n\n-- \nMit freundlichen Grüßen / Best regards\n\nTorsten Krah\n\nmgm technology partners GmbH\nNeumarkt 2\n04109 Leipzig\n\nTel. +49 (341) 339 893-539\nE-Mail Torsten.Krah@mgm-tp.com\n\nInnovation Implemented.\n\nGeschäftsführer / CEO: Hamarz Mehmanesh\nSitz der Gesellschaft / Registered office: München\nHandelsregister/ Commercial register: AG München HRB 161298\nUSt-IdNr. / VAT ID: DE815309575\n\n"},{"id":"389464","messageId":"63f412b2b88c5b26fe6c7e515c18f9de802603bf.camel@gmail.com","threadId":"52581","inReplyTo":"2423f8c0b91578c0faf7527b7d97b0e1e9666261.camel@gmail.com","subject":"Re: Broken branch after git commit - tracked files in staging area can't be removed with restore --staged, or commit or stash","fromName":"Torsten Krah","fromEmail":"krah.tm@gmail.com","sentAt":"2020-01-08T10:31:37Z","receivedAt":"2020-01-08T10:31:42Z","isPatch":false,"sender":{"key":"krah.tm@gmail.com","avatar":null},"body":"Am Mittwoch, den 08.01.2020, 11:02 +0100 schrieb Torsten Krah:\n> cd testrepo\n\ngit init\n\nof cause after this one ;)\n\n\n\n"},{"id":"389465","messageId":"20200108104008.GA2207365@coredump.intra.peff.net","threadId":"52581","inReplyTo":"2423f8c0b91578c0faf7527b7d97b0e1e9666261.camel@gmail.com","subject":"Re: Broken branch after git commit - tracked files in staging area can't be removed with restore --staged, or commit or stash","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-01-08T10:40:08Z","receivedAt":"2020-01-08T10:40:11Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jan 08, 2020 at 11:02:41AM +0100, Torsten Krah wrote:\n\n> Hi Jeff, I have a poc you can try:\n\nThanks, I can see the problem now.\n\n> cd /tmp\n> mkdir testrepo\n> cd testrepo\n> touch TEST1 TEST2\n> git add -A\n> git commit -m First\n> touch TEST3 TEST4\n> git add -A\n> git commit -m Second\n> git reset --soft HEAD~1\n\nOK, so we'd expect to still see TEST3 and TEST4 in the index. And we do:\n\n> git status\n> \n>    Auf Branch master\n>    Zum Commit vorgemerkte Änderungen:\n>      (benutzen Sie \"git restore --staged <Datei>...\" zum Entfernen aus\n>    der Staging-Area)\n>    \tneue Datei:     TEST3\n>    \tneue Datei:     TEST4\n\nAnd then if we try to unstage one of them, we should see that:\n\n> git restore --staged TEST3\n> \n>    [10:57:26][tkrah@torstenknbl:/tmp/testrepo]  (master) $ LC_ALL=C git\n>    status\n>    On branch master\n>    Changes to be committed:\n>      (use \"git restore --staged <file>...\" to unstage)\n>    \tnew file:   TEST4\n> \n>    Untracked files:\n>      (use \"git add <file>...\" to include in what will be committed)\n>    \tTEST3\n\nSo far so good. But I think there is a lurking problem in the index,\nbecause...\n\n> git commit -m Second\n> \n>    [master 5b62331] Second\n>     2 files changed, 0 insertions(+), 0\n>    deletions(-)\n>     create mode 100644 TEST3\n>     create mode 100644 TEST4\n\nThis is very wrong (and is a bug, not a problem with what you're doing).\nWe wrote a commit with TEST3 in it, even though it wasn't in the index.\nWhy would we do that? Almost certainly it's because of a cache-tree\nextension in the index. Presumably \"git restore --staged\" is not\nproperly invalidating the cache-tree entry. And that would explain what\nyou see next:\n\n> And now TEST3 is in the commit and what is even more \"interesting\" is\n> the next one:\n> \n>    [10:59:16][tkrah@torstenknbl:/tmp/testrepo]  (master) $ LC_ALL=C git\n> status\n>    On branch master\n>    Changes to be committed:\n>      (use \"git restore --staged <file>...\" to unstage)\n>    \tdeleted:    TEST3\n> \n>    Untracked files:\n>      (use \"git add <file>...\" to include in what will be committed)\n>    \tTEST3\n> \n> TEST3 is unstaged and deleted now.\n\nThat happens because we wrote out a commit with TEST3, but it's still\nnot in the index! So it appears as if a deletion was staged (remember\nthat what is \"staged\" is really just the diff between the index and\nHEAD).\n\nWe can repeat the same process, substituting \"git reset -- TEST3\" for\ngit-restore, and it works fine. So the problem is in git-restore itself.\n\nHere's another observation. If we do:\n\n  git restore --staged TEST3 TEST4\n\nthen running \"git commit\" won't do anything, since there's nothing\nstaged (and this is why I had trouble reproducing the problem earlier).\nIf we add a new file, like:\n\n  echo whatever >new\n  git add new\n\nthen the problem won't exhibit, because that will also invalidate the\ncache-tree (because it has to account for \"new\"). But we could put those\nfiles in their own subdirectory and do:\n\n  $ git restore --staged subdir/TEST3\n  $ git status\n  On branch master\n  Changes to be committed:\n  \tnew file:   subdir/TEST4\n  \n  Untracked files:\n  \tsubdir/TEST3\n\nand if I commit that, I see the bug again:\n\n  $ git commit -m second\n  [master 5c3b8c1] second\n   2 files changed, 0 insertions(+), 0 deletions(-)\n   create mode 100644 subdir/TEST3\n   create mode 100644 subdir/TEST4\n\nNow here's even more fun. I wanted to do another test, so I tried to\nreset back to the original \"second\" commit, but it segfaults! Yikes.\n\n  $ git reset --hard HEAD@{2}\n  Segmentation fault\n\nRunning it under valgrind shows we're indeed seeing a bogus cache-tree:\n\n  ==2214443== Invalid read of size 4\n  ==2214443==    at 0x362DFF: traverse_by_cache_tree (unpack-trees.c:734)\n  ==2214443==    by 0x3630A3: traverse_trees_recursive (unpack-trees.c:799)\n  ==2214443==    by 0x364209: unpack_callback (unpack-trees.c:1258)\n  ==2214443==    by 0x35F925: traverse_trees (tree-walk.c:497)\n  ==2214443==    by 0x364DDD: unpack_trees (unpack-trees.c:1589)\n  ==2214443==    by 0x1BE6CF: reset_index (reset.c:95)\n  ==2214443==    by 0x1BF894: cmd_reset (reset.c:421)\n  ==2214443==    by 0x124868: run_builtin (git.c:444)\n  ==2214443==    by 0x124BAE: handle_builtin (git.c:674)\n  ==2214443==    by 0x124E24: run_argv (git.c:741)\n  ==2214443==    by 0x1252AB: cmd_main (git.c:872)\n  ==2214443==    by 0x1E811A: main (common-main.c:52)\n  ==2214443==  Address 0x40 is not stack'd, malloc'd or (recently) free'd\n  ==2214443== \n  ==2214443== Process terminating with default action of signal 11 (SIGSEGV)\n\nThis isn't the same spot that Emily fixed earlier today in [1], but it\nseems like a very similar problem. Weird.\n\nSo instead I blew away the index and then did hard reset:\n\n  $ rm .git/index\n  $ git reset --hard\n  HEAD is now at 5c3b8c1 second\n\nAnd now we can see what happens if both files are unstaged, and then we\nadd a new file, and then commit:\n\n  $ git reset --soft HEAD^\n  $ git restore --staged subdir/TEST3 subdir/TEST4\n  $ echo whatever >new\n  $ git add new\n  $ git commit -m again\n  [master 119d9e4] again\n   1 file changed, 1 insertion(+)\n   create mode 100644 new\n\nSo that _doesn't_ exhibit the problem. I guess because adding the new\nfile causes git-add to reexamine the whole cache-tree and clean it up.\n\nSo there seem to be at least two bugs:\n\n - git-restore doesn't properly invalidate the cache-tree\n\n - the index-reading code is not careful enough about bogus cache-trees,\n   and may segfault\n\n-Peff\n\n[1] https://lore.kernel.org/git/20200108023127.219429-1-emilyshaffer@google.com/\n"},{"id":"389467","messageId":"20200108114344.GA3380580@coredump.intra.peff.net","threadId":"52581","inReplyTo":"20200108104008.GA2207365@coredump.intra.peff.net","subject":"[PATCH] restore: invalidate cache-tree when removing entries with --staged","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-01-08T11:43:44Z","receivedAt":"2020-01-08T11:43:49Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jan 08, 2020 at 05:40:08AM -0500, Jeff King wrote:\n\n> So there seem to be at least two bugs:\n> \n>  - git-restore doesn't properly invalidate the cache-tree\n> \n>  - the index-reading code is not careful enough about bogus cache-trees,\n>    and may segfault\n\nHere's a fix for the first one. I'm adding Junio to the cc as an expert\nin index and cache-tree issues. I'm pretty sure this is the correct fix,\nbut I have some lingering questions below.\n\nI'm not planning on working on the second one immediately. Between this\nand Emily's patch from yesterday, I have a feeling that the index code\ncould use an audit to be a bit more careful about handling bogus on-disk\ndata.\n\n-- >8 --\nSubject: restore: invalidate cache-tree when removing entries with --staged\n\nWhen \"git restore --staged <path>\" removes a path that's in the index,\nit marks the entry with CE_REMOVE, but we don't do anything to\ninvalidate the cache-tree. In the non-staged case, we end up in\ncheckout_worktree(), which calls remove_marked_cache_entries(). That\nactually drops the entries from the index, as well as invalidating the\ncache-tree and untracked-cache.\n\nBut with --staged, we never call checkout_worktree(), and the CE_REMOVE\nentries remain. Interestingly, they are dropped when we write out the\nindex, but that means the resulting index is inconsistent: its\ncache-tree will not match the actual entries, and running \"git commit\"\nimmediately after will create the wrong tree.\n\nWe can solve this by calling remove_marked_cache_entries() ourselves\nbefore writing out the index. Note that we can't just hoist it out of\ncheckout_worktree(); that function needs to iterate over the CE_REMOVE\nentries (to drop their matching worktree files) before removing them.\n\nOne curiosity about the test: without this patch, it actually triggers a\nBUG() when running git-restore:\n\n  BUG: cache-tree.c:810: new1 with flags 0x4420000 should not be in cache-tree\n\nBut in the original problem report, which used a similar recipe,\ngit-restore actually creates the bogus index (and the commit is created\nwith the wrong tree). I'm not sure why the test here behaves differently\nthan my out-of-suite reproduction, but what's here should catch either\nsymptom (and the fix corrects both cases).\n\nReported-by: Torsten Krah <krah.tm@gmail.com>\nSigned-off-by: Jeff King <peff@peff.net>\n---\nSo I'm mildly puzzled about the BUG thing above. But also: it seems\nreally weird that do_write_index() will drop CE_REMOVE entries as it\nwrites, but not invalidate the cache-tree (or the untracked-cache, which\nAFAICT is probably similarly affected). Should it be doing that\ninvalidation? Or should it be a BUG() to write out an index without\nhaving done remove_marked_cache_entries()?\n\n builtin/checkout.c |  2 ++\n t/t2070-restore.sh | 18 ++++++++++++++++++\n 2 files changed, 20 insertions(+)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex b52c490c8f..18ef5fb975 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -524,6 +524,8 @@ static int checkout_paths(const struct checkout_opts *opts,\n \t/* Now we are committed to check them out */\n \tif (opts->checkout_worktree)\n \t\terrs |= checkout_worktree(opts);\n+\telse\n+\t\tremove_marked_cache_entries(&the_index, 1);\n \n \t/*\n \t * Allow updating the index when checking out from the index.\ndiff --git a/t/t2070-restore.sh b/t/t2070-restore.sh\nindex 21c3f84459..06a5976143 100755\n--- a/t/t2070-restore.sh\n+++ b/t/t2070-restore.sh\n@@ -96,6 +96,7 @@ test_expect_success 'restore --ignore-unmerged ignores unmerged entries' '\n '\n \n test_expect_success 'restore --staged adds deleted intent-to-add file back to index' '\n+\ttest_when_finished git reset --hard &&\n \techo \"nonempty\" >nonempty &&\n \t>empty &&\n \tgit add nonempty empty &&\n@@ -106,4 +107,21 @@ test_expect_success 'restore --staged adds deleted intent-to-add file back to in\n \tgit diff --cached --exit-code\n '\n \n+test_expect_success 'restore --staged invalidates cache tree for deletions' '\n+\ttest_when_finished git reset --hard &&\n+\t>new1 &&\n+\t>new2 &&\n+\tgit add new1 new2 &&\n+\n+\t# It is important to commit and then reset here, so that the index\n+\t# contains a valid cache-tree for the \"both\" tree.\n+\tgit commit -m both &&\n+\tgit reset --soft HEAD^ &&\n+\n+\tgit restore --staged new1 &&\n+\tgit commit -m \"just new2\" &&\n+\tgit rev-parse HEAD:new2 &&\n+\ttest_must_fail git rev-parse HEAD:new1\n+'\n+\n test_done\n-- \n2.25.0.rc1.622.g8cfac75bdd\n\n"},{"id":"389469","messageId":"654cc7a58cf6947f91478411dd6a3f7f3473bc67.camel@gmail.com","threadId":"52581","inReplyTo":"20200108104008.GA2207365@coredump.intra.peff.net","subject":"Re: Broken branch after git commit - tracked files in staging area can't be removed with restore --staged, or commit or stash","fromName":"Torsten Krah","fromEmail":"krah.tm@gmail.com","sentAt":"2020-01-08T12:42:14Z","receivedAt":"2020-01-08T12:42:19Z","isPatch":false,"sender":{"key":"krah.tm@gmail.com","avatar":null},"body":"Am Mittwoch, den 08.01.2020, 05:40 -0500 schrieb Jeff King:\n> So there seem to be at least two bugs:\n> \n>  - git-restore doesn't properly invalidate the cache-tree\n> \n>  - the index-reading code is not careful enough about bogus cache-\n> trees,\n>    and may segfault\n> \n> -Peff\n-- \n\nHi Peff,\n\nthanks for confirming those bugs and taking a look at my report itself\neven before I've put the small reproducer recipe.\nI don't need to take any other actions or create tickets somewhere else\nnow to get those fixed, you're driving this from here, right?\n\nAnother question - how can I fix my broken original branch where this\nbug did hit me which is in that \"bogus\" state now to get back to the\n\"original\" commit made so I can remove my unwanted files with \"git\nreset HEAD <files>\" instead of git-restore in the meantime (tried your\nrm .git/index variant but after that I had files which were already in\nthe branch shown as unversioned after that, so that did not work very\nwell for me)?\n\nkind regards\n\nTorsten\n\n\n"},{"id":"389481","messageId":"xmqq4kx63qh8.fsf@gitster-ct.c.googlers.com","threadId":"52581","inReplyTo":"20200108114344.GA3380580@coredump.intra.peff.net","subject":"Re: [PATCH] restore: invalidate cache-tree when removing entries with --staged","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-01-08T15:41:39Z","receivedAt":"2020-01-08T15:41:48Z","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> diff --git a/builtin/checkout.c b/builtin/checkout.c\n> index b52c490c8f..18ef5fb975 100644\n> --- a/builtin/checkout.c\n> +++ b/builtin/checkout.c\n> @@ -524,6 +524,8 @@ static int checkout_paths(const struct checkout_opts *opts,\n>  \t/* Now we are committed to check them out */\n>  \tif (opts->checkout_worktree)\n>  \t\terrs |= checkout_worktree(opts);\n> +\telse\n> +\t\tremove_marked_cache_entries(&the_index, 1);\n\nAh, I was wondering why we were seeing breakages on cache-tree,\nwhich is fairly old and stable part of the system---even though it\nhad caused us quite a lot of pain while it was growing---all of a\nsudden.  No wonder---this codepath is a fairly new one, introduced\nwhen \"restore\" was added X-<.\n\nAnd the fix looks right, of course.  Thanks for extracting a\nreproducible recipe out of the original reporter and quickly\ndiagnosing it.  Very much appreciated.\n"},{"id":"389523","messageId":"20200109093931.GA299671@coredump.intra.peff.net","threadId":"52581","inReplyTo":"654cc7a58cf6947f91478411dd6a3f7f3473bc67.camel@gmail.com","subject":"Re: Broken branch after git commit - tracked files in staging area can't be removed with restore --staged, or commit or stash","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-01-09T09:39:31Z","receivedAt":"2020-01-09T09:39:35Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jan 08, 2020 at 01:42:14PM +0100, Torsten Krah wrote:\n\n> thanks for confirming those bugs and taking a look at my report itself\n> even before I've put the small reproducer recipe.\n> I don't need to take any other actions or create tickets somewhere else\n> now to get those fixed, you're driving this from here, right?\n\nCorrect.\n\n> Another question - how can I fix my broken original branch where this\n> bug did hit me which is in that \"bogus\" state now to get back to the\n> \"original\" commit made so I can remove my unwanted files with \"git\n> reset HEAD <files>\" instead of git-restore in the meantime (tried your\n> rm .git/index variant but after that I had files which were already in\n> the branch shown as unversioned after that, so that did not work very\n> well for me)?\n\nYou should be able to do:\n\n  git checkout your-branch\n  git reset --hard <original>\n\nto go back to the state you were at (this is assuming you haven't built\nmore commits on top, of course), and then repeat your steps. That should\nalso clear out any breakage in the index, since \"reset --hard\" will\ninvalidate the cache-tree as necessary.\n\nIf you need to find the commit id of that original commit, try looking\nin the reflog for your branch: git log -g your-branch\n\n-Peff\n"},{"id":"389530","messageId":"df169ee421429f8f0aa18a897a67677a2adf2b6b.camel@gmail.com","threadId":"52581","inReplyTo":"20200109093931.GA299671@coredump.intra.peff.net","subject":"Re: Broken branch after git commit - tracked files in staging area can't be removed with restore --staged, or commit or stash","fromName":"Torsten Krah","fromEmail":"krah.tm@gmail.com","sentAt":"2020-01-09T10:49:02Z","receivedAt":"2020-01-09T10:49:08Z","isPatch":false,"sender":{"key":"krah.tm@gmail.com","avatar":null},"body":"Am Donnerstag, den 09.01.2020, 04:39 -0500 schrieb Jeff King:\n> You should be able to do:\n> \n>   git checkout your-branch\n>   git reset --hard <original>\n> \n> to go back to the state you were at (this is assuming you haven't\n> built\n> more commits on top, of course), and then repeat your steps. That\n> should\n> also clear out any breakage in the index, since \"reset --hard\" will\n> invalidate the cache-tree as necessary.\n> \n> If you need to find the commit id of that original commit, try\n> looking\n> in the reflog for your branch: git log -g your-branch\n\nHi Peff,\n\nthanks that did work - but I had to \"rm .git/index\" before the hard\nreset to get those \"deleted\" ones removed from the staging area, after\nthat I was able to use \"git reset -- $files\" and that was working like\nexpected.\nWithout deleting that index file I could run the hard reset but running\n\"git status\" afterwards insisted every time on this:\n\nOn branch feature-2182\nChanges to be committed:\n  (use \"git restore --staged <file>...\" to unstage)\n\tdeleted:    docker-compose.gerrit.yml\n\tdeleted:    docker-compose.reverseproxy.yml\n\tdeleted:    docker-compose.yml\n\tdeleted:    docker-seccomp.json\n\tdeleted:    gradle.properties\n\tdeleted:    settings.gradle\n\nThose only vanished after deleting the index file.\n\nThe \"settings.gradle\" is part of my changeset and listed in the\n\"commit\".\nBut those other 5 files listed I had never touched in my commit - I\ndon't know why they are in the staging area here - maybe some other\nedge case of the git restore bug - but just a \"git hard --reset $hash\"\ndid not clear those.\n\nTorsten\n-- \n\n\n"},{"id":"391193","messageId":"8575e5067f7506992cc61ceb3b4185ab9c6f3d57.camel@kaarsemaker.net","threadId":"52581","inReplyTo":"20200108114344.GA3380580@coredump.intra.peff.net","subject":"Re: [PATCH] restore: invalidate cache-tree when removing entries with --staged","fromName":"Dennis Kaarsemaker","fromEmail":"dennis@kaarsemaker.net","sentAt":"2020-02-05T20:24:04Z","receivedAt":"2020-02-05T20:24:09Z","isPatch":true,"sender":{"key":"dennis@kaarsemaker.net","avatar":"https://avatars.githubusercontent.com/u/200649?v=4"},"body":"On Wed, 2020-01-08 at 06:43 -0500, Jeff King wrote:\n> On Wed, Jan 08, 2020 at 05:40:08AM -0500, Jeff King wrote:\n> \n> > So there seem to be at least two bugs:\n> > \n> >  - git-restore doesn't properly invalidate the cache-tree\n> > \n> >  - the index-reading code is not careful enough about bogus cache-trees,\n> >    and may segfault\n> \n> Here's a fix for the first one. I'm adding Junio to the cc as an expert\n> in index and cache-tree issues. I'm pretty sure this is the correct fix,\n> but I have some lingering questions below.\n> \n> I'm not planning on working on the second one immediately. Between this\n> and Emily's patch from yesterday, I have a feeling that the index code\n> could use an audit to be a bit more careful about handling bogus on-disk\n> data.\n\nWe just ran into something similar where git would create really bogus\ncommits when mixing squash merges and restore. As it's a private repo,\nI don't have an exact recipe for reproducing it, but it roughly goes\nlike:\n\ngit checkout master\ngit merge --squash branch-for-which-we-want-to-redo-commits\ngit restore --staged .\ngit add file1 file2 file3\ngit commit -m \"commit\"\n\nThis commit would remove a file4 that wasn't even touched in the branch\n(further commits would do even more broken things, eventually leading\nto broken commit objects with duplicate file contents). Changing `git\nrestore --staged .` to `git reset HEAD` made this behaviour go away. A\nquick search in the list brought this patch and I'm happy to say it\nfixes our issue as well.\n\nThanks Peff!\n\nD.\n\n"}]}