{"thread":{"id":"33023","subject":"two-way merge corner case bug","startedAt":"2013-02-26T20:06:42Z","lastAt":"2013-03-02T00:54:41Z","messageCount":10,"participants":["Junio C Hamano","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"210360","messageId":"7v7glun8kt.fsf@alter.siamese.dyndns.org","threadId":"33023","inReplyTo":null,"subject":"two-way merge corner case bug","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-26T20:06:42Z","receivedAt":"2013-02-26T20:06:42Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"It seems that we have a corner case bug in two-way merge to reset\naway the conflicts with \"read-tree -u --reset HEAD ORIG_HEAD\".\n\nIn a freshly created repository:\n\n    rm -f A Z\n    for i in $(seq 100)\n    do\n            echo hello\n    done >Z\n    git add Z\n    git commit -m initial\n    git branch side\n    echo goodbye >>Z\n    git commit -a -m changed\n    git checkout side\n    git mv Z A\n    echo kitty >>A\n    git commit -a -m side\n\nif you do one of these:\n\n\tgit checkout master && git merge side\n\tgit checkout side && git merge master\n\nyou will get three stages for path A, and nothing for path Z.\n\nWith the former, you are starting from a tree with Z (and not A),\nand \"resetting\" should give you A that is the same as 'master' in\nthe working tree.  With the latter, you are starting from a tree\nwith A, and \"resetting\" should give you Z and remove conflicted A\nfrom the working tree.\n\n\tgit read-tree -u --reset ORIG_HEAD\n\ndoes the right thing in either case, but\n\n\tgit read-tree -u --reset HEAD ORIG_HEAD\n\ndies with \"cache entry has null sha1: A\" for both cases.\n\nThe direct symptom comes from 4337b5856f88 (do not write null sha1s\nto on-disk index, 2012-07-28), but I think the root cause is\nsomewhere else.  When we start from 'side', we should notice that\nour HEAD has Z, and conflicted A we see in the index came from a\nconflicted merge that we are resetting away and should disappear\nfrom the working tree.  When we start from 'master', we should\nnotice that we do not want to keep the cache entry that is\n\"modified\" in the index, and write the one taken from the \"going to\"\ntree (ORIG_HEAD).\n"},{"id":"210364","messageId":"20130226201820.GD13830@sigill.intra.peff.net","threadId":"33023","inReplyTo":"7v7glun8kt.fsf@alter.siamese.dyndns.org","subject":"Re: two-way merge corner case bug","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-02-26T20:18:20Z","receivedAt":"2013-02-26T20:18:20Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Feb 26, 2013 at 12:06:42PM -0800, Junio C Hamano wrote:\n\n> It seems that we have a corner case bug in two-way merge to reset\n> away the conflicts with \"read-tree -u --reset HEAD ORIG_HEAD\".\n\nIsn't this a repeat of:\n\n  http://thread.gmane.org/gmane.comp.version-control.git/202440/focus=212316\n\nwhose fix never got finalized? It's on my todo list, but it seems that\nitems keep sinking further down on that list, rather than getting\ncompleted. :-/\n\n-Peff\n"},{"id":"210371","messageId":"7vwqtulplp.fsf@alter.siamese.dyndns.org","threadId":"33023","inReplyTo":"20130226201820.GD13830@sigill.intra.peff.net","subject":"Re: two-way merge corner case bug","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-26T21:41:54Z","receivedAt":"2013-02-26T21:41:54Z","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, Feb 26, 2013 at 12:06:42PM -0800, Junio C Hamano wrote:\n>\n>> It seems that we have a corner case bug in two-way merge to reset\n>> away the conflicts with \"read-tree -u --reset HEAD ORIG_HEAD\".\n>\n> Isn't this a repeat of:\n>\n>   http://thread.gmane.org/gmane.comp.version-control.git/202440/focus=212316\n>\n> whose fix never got finalized? It's on my todo list, but it seems that\n> items keep sinking further down on that list, rather than getting\n> completed. :-/\n\nYeah, I think the patch in your message is a good starting point to\nsolve a half of the issue (i.e. unmerged current one should be\nreplaced with the version from the \"going to\" tree).  Attached is\nwith a slight update.\n\nThe other half that is not solved by this patch is that we will lose\nZ because merge-recursive removed it when it renamed it and left\nthree stages for A in the index, and to \"read-tree --reset -u HEAD\nORIG_HEAD\", it looks as if you removed Z yourself while on HEAD and\nwant to carry your local change forward to ORIG_HEAD.\n\nI think this too needs to be fixed.  We are saying \"-u --reset\" to\nmean \"I want to go there no matter what\", not \"-u -m\" that means \"I\nam carrying my changes forward while checking out another branch\".\n\n unpack-trees.c | 25 +++++++++++++++++--------\n 1 file changed, 17 insertions(+), 8 deletions(-)\n\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 09e53df..3d17108 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -1748,14 +1748,23 @@ int twoway_merge(struct cache_entry **src, struct unpack_trees_options *o)\n \t\tnewtree = NULL;\n \n \tif (current) {\n-\t\tif ((!oldtree && !newtree) || /* 4 and 5 */\n-\t\t    (!oldtree && newtree &&\n-\t\t     same(current, newtree)) || /* 6 and 7 */\n-\t\t    (oldtree && newtree &&\n-\t\t     same(oldtree, newtree)) || /* 14 and 15 */\n-\t\t    (oldtree && newtree &&\n-\t\t     !same(oldtree, newtree) && /* 18 and 19 */\n-\t\t     same(current, newtree))) {\n+\t\tif (current->ce_flags & CE_CONFLICTED) {\n+\t\t\tif (same(oldtree, newtree) || o->reset) {\n+\t\t\t\tif (!newtree)\n+\t\t\t\t\treturn deleted_entry(current, current, o);\n+\t\t\t\telse\n+\t\t\t\t\treturn merged_entry(newtree, current, o);\n+\t\t\t}\n+\t\t\treturn o->gently ? -1 : reject_merge(current, o);\n+\t\t}\n+\t\telse if ((!oldtree && !newtree) || /* 4 and 5 */\n+\t\t\t (!oldtree && newtree &&\n+\t\t\t  same(current, newtree)) || /* 6 and 7 */\n+\t\t\t (oldtree && newtree &&\n+\t\t\t  same(oldtree, newtree)) || /* 14 and 15 */\n+\t\t\t (oldtree && newtree &&\n+\t\t\t  !same(oldtree, newtree) && /* 18 and 19 */\n+\t\t\t  same(current, newtree))) {\n \t\t\treturn keep_entry(current, o);\n \t\t}\n \t\telse if (oldtree && !newtree && same(current, oldtree)) {\n"},{"id":"210451","messageId":"20130301092201.GA17254@sigill.intra.peff.net","threadId":"33023","inReplyTo":"7vwqtulplp.fsf@alter.siamese.dyndns.org","subject":"Re: two-way merge corner case bug","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-03-01T09:22:01Z","receivedAt":"2013-03-01T09:22:01Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Feb 26, 2013 at 01:41:54PM -0800, Junio C Hamano wrote:\n\n> > Isn't this a repeat of:\n> >\n> >   http://thread.gmane.org/gmane.comp.version-control.git/202440/focus=212316\n> [...]\n> \n> Yeah, I think the patch in your message is a good starting point to\n> solve a half of the issue (i.e. unmerged current one should be\n> replaced with the version from the \"going to\" tree).  Attached is\n> with a slight update.\n\nThanks for picking this up. I'm not sure, though, about the trigger\nhere:\n\n> +\t\tif (current->ce_flags & CE_CONFLICTED) {\n> +\t\t\tif (same(oldtree, newtree) || o->reset) {\n> +\t\t\t\tif (!newtree)\n> +\t\t\t\t\treturn deleted_entry(current, current, o);\n> +\t\t\t\telse\n> +\t\t\t\t\treturn merged_entry(newtree, current, o);\n> +\t\t\t}\n> +\t\t\treturn o->gently ? -1 : reject_merge(current, o);\n\nWe have a conflicted state in the index for a path. If o->reset is true,\nthen we want to throw it out in favor of where we are moving to. So that\npart makes sense.\n\nBut if we are not in \"--reset\", and we see that oldtree and newtree are\nthe same, do we still want to move to newtree? It seems like we are\nthrowing away the conflicted state that the user has. Usually this isn't\na big deal, but with \"-u\" it could mean losing any in-progress\nresolution in the working tree.\n\nI think this generally shouldn't happen, as \"-m\" does not operate when\nthe index contains unmerged entries. So it would really just be a safety\nvalve for somebody calling into unpack_trees without having done the\nindex check. IOW, should the same(newtree, oldtree) call be cut there,\nand we trigger this just on reset? Any other case would be rejected\n(which _shouldn't_ happen, but this keeps us conservative, and if we\nfind a case where it does happen, we can figure out the semantics then).\n\n> The other half that is not solved by this patch is that we will lose\n> Z because merge-recursive removed it when it renamed it and left\n> three stages for A in the index, and to \"read-tree --reset -u HEAD\n> ORIG_HEAD\", it looks as if you removed Z yourself while on HEAD and\n> want to carry your local change forward to ORIG_HEAD.\n>\n> I think this too needs to be fixed.  We are saying \"-u --reset\" to\n> mean \"I want to go there no matter what\", not \"-u -m\" that means \"I\n> am carrying my changes forward while checking out another branch\".\n\nYeah, hmm. Since we cannot distinguish between such a staged removal and\nwhat merge-recursive deposits in the index, we have to err on on side\n(either losing a staged removal, or failing to move to the new state).\nWhat we really want is to detect that Z's removal is related to the\nconflicted path A, but we can't do so; that information was never put\ninto the index in the first place.\n\nWe could do this:\n\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 3d17108..1e84131 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -1788,7 +1788,7 @@ int twoway_merge(struct cache_entry **src, struct unpack_trees_options *o)\n \t\t}\n \t}\n \telse if (newtree) {\n-\t\tif (oldtree && !o->initial_checkout) {\n+\t\tif (oldtree && !o->initial_checkout && !o->reset) {\n \t\t\t/*\n \t\t\t * deletion of the path was staged;\n \t\t\t */\n\nwhich solves the case you presented. My worry would be that somebody is\nusing \"--reset\" but expecting the removal to be carried through\n(technically, \"--reset\" is documented as \"-m but discard unmerged\nentries\", but we are not really treating it that way here). The test\nsuite passes, but we may not be covering some oddball case.\n\n-Peff\n\nPS I wonder if the \"initial_checkout\" hack can just go away under this\n   new rule. Although we don't seem to use \"o->reset\" for the initial\n   checkout. Which seems kind of weird. I would think the initial\n   checkout would actually just be a oneway merge. Is the point to try\n   to leave any existing working tree contents untouched or something?\n"},{"id":"210465","messageId":"7va9qngisg.fsf@alter.siamese.dyndns.org","threadId":"33023","inReplyTo":"20130301092201.GA17254@sigill.intra.peff.net","subject":"Re: two-way merge corner case bug","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-01T16:57:03Z","receivedAt":"2013-03-01T16:57:03Z","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> PS I wonder if the \"initial_checkout\" hack can just go away under this\n>    new rule. Although we don't seem to use \"o->reset\" for the initial\n>    checkout. Which seems kind of weird. I would think the initial\n>    checkout would actually just be a oneway merge. Is the point to try\n>    to leave any existing working tree contents untouched or something?\n\nAn initial checkout is *supposed* to happen in an empty working\ntree, so if we code it not to overwrite an existing path in the\nworking tree, the user cannot lose possibly precious contents with\nan mistaken initial checkout (they will instead appear as modified\nrelative to the index), while in the normal case we will write out\nthe contents from the HEAD through the index.  We could attempt \"we\ndo not have to if the user behaves, but with this we could help\nmisbehaving users\" if we used twoway merge for an initial checkout.\n\nHaving said that, I notice that in the normal codepath (e.g. \"git\nclone\" without the \"--no-checkout\" option) we no longer use twoway\nmerge for the initial checkout.  Back when \"git clone\" was a\nscripted Porcelain, I think we used to do a twoway read-tree.  It\nmay be that we broke it when \"clone\" was rewritten in C, but the\nbreakage is to the \"we do not have to...\" thing, so it may not be a\nbig deal.\n\nThe only case that matters in today's code is \"git checkout\"\n(without any option or argument) immediately after \"git clone -n\", I\nthink.  The special casing for this initial checkout in twoway merge\nis needed because we go from HEAD to HEAD in that case, and we do\nnot want to keep the artificial local removals from the index; we\nstart from not even having the $GIT_INDEX_FILE, so without the\nspecial case all paths appear to have been \"rm --cached\", which is\nusually not what the user would want to see ;-)\n\n> ... My worry would be that somebody is\n> using \"--reset\" but expecting the removal to be carried through\n> (technically, \"--reset\" is documented as \"-m but discard unmerged\n> entries\", but we are not really treating it that way here).\n\nI've checked all in-tree uses of \"read-tree --reset -u\".\n\nNobody seems to use that combination, either from scripts or from C\n(i.e. when opts.update==1 and opts.merge==1, opts.reset is not set)\nwith a twoway merge, other than \"git am --abort/--skip\".\n"},{"id":"210483","messageId":"20130301223612.GA862@sigill.intra.peff.net","threadId":"33023","inReplyTo":"7va9qngisg.fsf@alter.siamese.dyndns.org","subject":"Re: two-way merge corner case bug","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-03-01T22:36:12Z","receivedAt":"2013-03-01T22:36:12Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Mar 01, 2013 at 08:57:03AM -0800, Junio C Hamano wrote:\n\n> An initial checkout is *supposed* to happen in an empty working\n> tree, so if we code it not to overwrite an existing path in the\n> working tree, the user cannot lose possibly precious contents with\n> an mistaken initial checkout (they will instead appear as modified\n> relative to the index), while in the normal case we will write out\n> the contents from the HEAD through the index.  We could attempt \"we\n> do not have to if the user behaves, but with this we could help\n> misbehaving users\" if we used twoway merge for an initial checkout.\n\nThat matches my thinking. It is probably not worth touching, though,\nsince it is not causing any problems. I just found it curious that the\nexact same (and only, as far as I can see) exception we make for\ninitial_checkout is the same thing we have to tweak here.\n\n> Having said that, I notice that in the normal codepath (e.g. \"git\n> clone\" without the \"--no-checkout\" option) we no longer use twoway\n> merge for the initial checkout.  Back when \"git clone\" was a\n> scripted Porcelain, I think we used to do a twoway read-tree.  It\n> may be that we broke it when \"clone\" was rewritten in C, but the\n> breakage is to the \"we do not have to...\" thing, so it may not be a\n> big deal.\n\nThe one-way merge that we use now in clone makes a lot of sense to me.\nWe do not have a \"previous state\" we were based on.\n\n> The only case that matters in today's code is \"git checkout\"\n> (without any option or argument) immediately after \"git clone -n\", I\n> think.  The special casing for this initial checkout in twoway merge\n> is needed because we go from HEAD to HEAD in that case, and we do\n> not want to keep the artificial local removals from the index; we\n> start from not even having the $GIT_INDEX_FILE, so without the\n> special case all paths appear to have been \"rm --cached\", which is\n> usually not what the user would want to see ;-)\n\nRight. I just wondered if such a checkout should instead be a \"reset\",\nin which case it would fall under the proposed patch. But \"git checkout\"\nnever does a twoway_merge with o->reset; instead, it uses a one-way\nmerge.\n\nAnyway, that is all tangential to the bug at hand.\n\n> > ... My worry would be that somebody is\n> > using \"--reset\" but expecting the removal to be carried through\n> > (technically, \"--reset\" is documented as \"-m but discard unmerged\n> > entries\", but we are not really treating it that way here).\n> \n> I've checked all in-tree uses of \"read-tree --reset -u\".\n> \n> Nobody seems to use that combination, either from scripts or from C\n> (i.e. when opts.update==1 and opts.merge==1, opts.reset is not set)\n> with a twoway merge, other than \"git am --abort/--skip\".\n\nI can believe it. So do we want to do that fix, then? Did you want to\nroll up the two halves of it with a test and write a commit message? I\nfeel like you could write a much more coherent one than I could on this\nsubject.\n\n-Peff\n"},{"id":"210486","messageId":"7vppzien3i.fsf@alter.siamese.dyndns.org","threadId":"33023","inReplyTo":"20130301223612.GA862@sigill.intra.peff.net","subject":"Re: two-way merge corner case bug","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-01T23:06:57Z","receivedAt":"2013-03-01T23:06:57Z","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 Fri, Mar 01, 2013 at 08:57:03AM -0800, Junio C Hamano wrote:\n>> \n>> Nobody seems to use that combination, either from scripts or from C\n>> (i.e. when opts.update==1 and opts.merge==1, opts.reset is not set)\n>> with a twoway merge, other than \"git am --abort/--skip\".\n>\n> I can believe it. So do we want to do that fix, then? Did you want to\n> roll up the two halves of it with a test and write a commit message? I\n> feel like you could write a much more coherent one than I could on this\n> subject.\n\nI actually was wondering if we can remove that sole uses of two-way\nmerge with --reset -u from \"git am\" and replace them with something\nelse.  But we do want to keep local change that existed before \"am\"\nstarted, so we cannot avoid use of two-way merge, I guess...\n"},{"id":"210487","messageId":"20130301230845.GA7317@sigill.intra.peff.net","threadId":"33023","inReplyTo":"7vppzien3i.fsf@alter.siamese.dyndns.org","subject":"Re: two-way merge corner case bug","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-03-01T23:08:45Z","receivedAt":"2013-03-01T23:08:45Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Mar 01, 2013 at 03:06:57PM -0800, Junio C Hamano wrote:\n\n> > I can believe it. So do we want to do that fix, then? Did you want to\n> > roll up the two halves of it with a test and write a commit message? I\n> > feel like you could write a much more coherent one than I could on this\n> > subject.\n> \n> I actually was wondering if we can remove that sole uses of two-way\n> merge with --reset -u from \"git am\" and replace them with something\n> else.  But we do want to keep local change that existed before \"am\"\n> started, so we cannot avoid use of two-way merge, I guess...\n\nYeah, I think that is a case we definitely want to keep, as it means any\nintermediate work done by the user in applying the patch is not lost.\n\n-Peff\n"},{"id":"210492","messageId":"7va9qmel4s.fsf@alter.siamese.dyndns.org","threadId":"33023","inReplyTo":"20130301230845.GA7317@sigill.intra.peff.net","subject":"Re: two-way merge corner case bug","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-01T23:49:23Z","receivedAt":"2013-03-01T23:49:23Z","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>> I actually was wondering if we can remove that sole uses of two-way\n>> merge with --reset -u from \"git am\" and replace them with something\n>> else.  But we do want to keep local change that existed before \"am\"\n>> started, so we cannot avoid use of two-way merge, I guess...\n>\n> Yeah, I think that is a case we definitely want to keep, as it means any\n> intermediate work done by the user in applying the patch is not lost.\n\nI am not sure what you mean by \"intermediate\" here.  When the user\nattempts to resolve conflict in a path manually and gives up, we do\nwant to revert changes to such a path to that of HEAD.\n\nClarifying the semantics of \"--reset\" needs to be done carefully.\n\nI think any difference \"git diff --cached\" shows are fair game to\nrevert to HEAD.  In the earlier example, path \"Z\" that was created\nby recursive merge in an attempt to rename \"A\" should be removed,\nand path \"A\" should be recreated to that of HEAD, as we know at the\npoint of \"am --skip/--abort\" that these two paths were involved in\nthe recursive merge invoked by the patch application process (that\nis the only possible reason why these are different in the index\nfrom HEAD).  Also any conflicting entries can only come from\nthree-way merge and they should be reverted to that of HEAD.\n\nOn the other hand, the paths that appear in \"git diff\" (except for\nthose that appear in \"git diff --cached\", which we will revert to\nHEAD following the logic of the previous paragraph) must be kept.\nThey are changes that were already present in the working tree\nbefore the user decided to accept a trivial patch via \"am\" that does\nnot overlap with what the user was doing.  We allow a dirty working\ntree but disallow a dirty index when \"git am\" starts, exactly\nbecause we want to ensure this property.\n\nBy doing both of the above, we should be able to satisfy the user\nwho uses \"am --abort/--skip\", in order to restore paths that were\ninvolved in the failed attempt to three-way merge to revert back to\nthat of HEAD, while keeping unrelated changes that were present\nbefore \"am\" started.\n"},{"id":"210498","messageId":"20130302005441.GA15836@sigill.intra.peff.net","threadId":"33023","inReplyTo":"7va9qmel4s.fsf@alter.siamese.dyndns.org","subject":"Re: two-way merge corner case bug","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-03-02T00:54:41Z","receivedAt":"2013-03-02T00:54:41Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Mar 01, 2013 at 03:49:23PM -0800, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> >> I actually was wondering if we can remove that sole uses of two-way\n> >> merge with --reset -u from \"git am\" and replace them with something\n> >> else.  But we do want to keep local change that existed before \"am\"\n> >> started, so we cannot avoid use of two-way merge, I guess...\n> >\n> > Yeah, I think that is a case we definitely want to keep, as it means any\n> > intermediate work done by the user in applying the patch is not lost.\n> \n> I am not sure what you mean by \"intermediate\" here.  When the user\n> attempts to resolve conflict in a path manually and gives up, we do\n> want to revert changes to such a path to that of HEAD.\n\nI thought we left it unchanged intentionally, but I cannot seem to find\nthe conversation I am thinking of. I did find this:\n\n  http://thread.gmane.org/gmane.comp.version-control.git/164002\n\nBut that is not about \"save the intermediate work done on applying the\nseries\", but rather to help with this case:\n\n  1. You \"git am\" a series. It doesn't apply.\n\n  2. You get distracted and work on something else, forgetting that the\n     dotest directory is sitting there.\n\n  3. Time passes.\n\n  4. You try to \"git am\" a new series. An \"am\" is already in progress,\n     so you are advised to \"git am --abort\".\n\nAt that point you do not want to reset the commit to the original head\n(which was fixed by 7b3b7e3), but you do not necessarily want to reset\nthe working tree contents either. We leave them in the name of safety\nand letting the user deal with it.\n\n> Clarifying the semantics of \"--reset\" needs to be done carefully.\n> \n> I think any difference \"git diff --cached\" shows are fair game to\n> revert to HEAD.  In the earlier example, path \"Z\" that was created\n> by recursive merge in an attempt to rename \"A\" should be removed,\n> and path \"A\" should be recreated to that of HEAD, as we know at the\n> point of \"am --skip/--abort\" that these two paths were involved in\n> the recursive merge invoked by the patch application process (that\n> is the only possible reason why these are different in the index\n> from HEAD).  Also any conflicting entries can only come from\n> three-way merge and they should be reverted to that of HEAD.\n> \n> On the other hand, the paths that appear in \"git diff\" (except for\n> those that appear in \"git diff --cached\", which we will revert to\n> HEAD following the logic of the previous paragraph) must be kept.\n> They are changes that were already present in the working tree\n> before the user decided to accept a trivial patch via \"am\" that does\n> not overlap with what the user was doing.  We allow a dirty working\n> tree but disallow a dirty index when \"git am\" starts, exactly\n> because we want to ensure this property.\n> \n> By doing both of the above, we should be able to satisfy the user\n> who uses \"am --abort/--skip\", in order to restore paths that were\n> involved in the failed attempt to three-way merge to revert back to\n> that of HEAD, while keeping unrelated changes that were present\n> before \"am\" started.\n\nI think that makes sense, but I don't think we have a read-tree mode to\ndo that in one pass, do we? Not that we can't add one if we need.\n\n-Peff\n"}]}