{"thread":{"id":"57855","subject":"Question about pre-merge and git merge octopus strategy","startedAt":"2022-05-06T08:15:28Z","lastAt":"2022-05-19T14:47:14Z","messageCount":15,"participants":["ZheNing Hu","Christian Couder","Elijah Newren","Junio C Hamano","Carlo Marcelo Arenas Belón"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"454902","messageId":"CAOLTT8S8rh+VYcuaqBeNtmphiRqw7HropLFpkxfnTJq6BngGXw@mail.gmail.com","threadId":"57855","inReplyTo":null,"subject":"Question about pre-merge and git merge octopus strategy","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2022-05-06T08:14:56Z","receivedAt":"2022-05-06T08:15:28Z","isPatch":false,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Hi,\n\nI am thinking about if git can \"pre-merge\" multiple branches, which\ncan check if merge\nwill have conflict, but not to merge them actually, like a option `--intend`.\n\nI find \"git merge-tree\" can output merge result in stdout, which meets\nmy needs, but it can only\nsupport two branches' merge.\n\nSo I find git merge with more than two branches can use octopus strategy.\nWhat about git merge --no-commit? Which will not commit automatically,\nso we can check if they have\nconfilct, and abort merge.\n\nI think it's not useful for git merge-octopus, because if we meet a\nmerge conflict, we can't find\nMERGE_HEAD at all! How can we abort this conflict merge?\n\n--\nZheNing Hu\n"},{"id":"454926","messageId":"CAP8UFD2p+Evqv_MBAgv23zooppsNWjOw6ZU2GLqAq_skZoJPOw@mail.gmail.com","threadId":"57855","inReplyTo":"CAOLTT8S8rh+VYcuaqBeNtmphiRqw7HropLFpkxfnTJq6BngGXw@mail.gmail.com","subject":"Re: Question about pre-merge and git merge octopus strategy","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2022-05-06T17:23:58Z","receivedAt":"2022-05-06T17:24:15Z","isPatch":false,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Hi,\n\nOn Fri, May 6, 2022 at 10:15 AM ZheNing Hu <adlternative@gmail.com> wrote:\n\n> I am thinking about if git can \"pre-merge\" multiple branches, which\n> can check if merge\n> will have conflict, but not to merge them actually, like a option `--intend`.\n>\n> I find \"git merge-tree\" can output merge result in stdout, which meets\n> my needs, but it can only\n> support two branches' merge.\n\nElijah (added in Cc) has been working on \"git merge-tree\" improvements\nbased on the new \"ort\" merge he developed. It supports merging 2\nbranches, but maybe there are ways to make it support more than 2.\n\n> So I find git merge with more than two branches can use octopus strategy.\n> What about git merge --no-commit? Which will not commit automatically,\n> so we can check if they have\n> confilct, and abort merge.\n\nYeah, I think that's what you want.\n\n> I think it's not useful for git merge-octopus, because if we meet a\n> merge conflict, we can't find\n> MERGE_HEAD at all! How can we abort this conflict merge?\n\nI don't know octopus merges much, but I think you should be able to\nabort using \"git reset\" (maybe with \"--hard\"). If the merge was\nperformed using --no-commit or if there was a conflict, then I think\nit should be expected that there is no MERGE_HEAD as no commit would\nbe created so MERGE_HEAD would have nothing to point to.\n"},{"id":"454963","messageId":"CABPp-BGD3AZvXwmSHfQQ_xh_UqevH23kdBYijAWUk8GHu1q0Qw@mail.gmail.com","threadId":"57855","inReplyTo":"CAP8UFD2p+Evqv_MBAgv23zooppsNWjOw6ZU2GLqAq_skZoJPOw@mail.gmail.com","subject":"Re: Question about pre-merge and git merge octopus strategy","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-05-07T04:09:45Z","receivedAt":"2022-05-07T04:10:19Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Fri, May 6, 2022 at 10:24 AM Christian Couder\n<christian.couder@gmail.com> wrote:\n>\n> Hi,\n>\n> On Fri, May 6, 2022 at 10:15 AM ZheNing Hu <adlternative@gmail.com> wrote:\n>\n> > I am thinking about if git can \"pre-merge\" multiple branches, which\n> > can check if merge\n> > will have conflict, but not to merge them actually, like a option `--intend`.\n> >\n> > I find \"git merge-tree\" can output merge result in stdout, which meets\n> > my needs, but it can only\n> > support two branches' merge.\n>\n> Elijah (added in Cc) has been working on \"git merge-tree\" improvements\n> based on the new \"ort\" merge he developed. It supports merging 2\n> branches, but maybe there are ways to make it support more than 2.\n\nThe primary issue with in-core octopus merges is that there are lots\nof questions about how to handle conflicts; possibly even more so than\ngit merge-tree --write-tree brings up, and that took us months of\ndiscussion.  In particular, with octopii, do iterate merging one\nbranch in at a time and stop with any conflicts (thus potentially\nstopping N-1 times when merging N branches), do you attempt to just\nrun all N-1 merges and have conflicts that can't readily be expressed\nin the index (especially for non-textual multi-way conflicts, but even\nnested conflict markers for text files could be painful), do you\nattempt to handle the multi-way textual conflicts a new way with code\nreplacing xdiff/ in order to avoid nested conflicts?  I wasn't sure\nwhat to do, so never implemented that in \"ort\".\n\nOf course, people could roll their own by just serially merging pairs\nof commits, and then rewriting the history to replace the string of\nmerges with an octopus.  Or perhaps just use \"git merge --no-commit\"\nserially, though that only works if the branches touch disjoint files\n(otherwise one of the merges will complain you have changes that could\nbe overwritten by the next merge).  And yes, if you don't want to mess\nwith the working tree/index, you could serially use the \"git\nmerge-tree --write-tree\" once I finish it, but it's not ready yet.\n(Sorry about that; I've got a bunch of nearly complete changes from a\nwhile ago but just didn't have much Git time.  I'll try to get to it\nsoon.)\n\nHowever, I think Junio said that octopus merge only handles trivial\ncases anyway, in which case an iterated \"git merge --write-tree\" would\nactually be a sufficient solution here and we could sidestep the more\nconvoluted cases.  But, sadly for ZheNing, that option doesn't exist\nyet -- It's still under development.\n\n> > So I find git merge with more than two branches can use octopus strategy.\n> > What about git merge --no-commit? Which will not commit automatically,\n> > so we can check if they have\n> > confilct, and abort merge.\n>\n> Yeah, I think that's what you want.\n>\n> > I think it's not useful for git merge-octopus, because if we meet a\n> > merge conflict, we can't find\n> > MERGE_HEAD at all! How can we abort this conflict merge?\n\nMERGE_HEAD doesn't have anything to do with aborting the conflict\nresolution step.  When you need to abort, the thing you want to go\nback to is HEAD (which represents the commit you had checked out and\nwere merging the other stuff into), not MERGE_HEAD (which represents\nthe branch or branches you were merging into HEAD).\n\n> I don't know octopus merges much, but I think you should be able to\n> abort using \"git reset\" (maybe with \"--hard\").  If the merge was\n> performed using --no-commit or if there was a conflict, then I think\n> it should be expected that there is no MERGE_HEAD as no commit would\n> be created so MERGE_HEAD would have nothing to point to.\n\nMERGE_HEAD isn't something created during a merge, it's something that\nexisted before it -- namely, the tip of the other branch we are\nmerging.  For an octopus merge, you'd thus expect it to have N commits\nrather than just 1.\n\nAUTO_MERGE, new to ort, is something that is created during a merge\nand when the merge interrupts due to conflicts to ask the user to\nresolve the conflicts, AUTO_MERGE represents the tree checked out in\nthe working copy (thus it is a tree that likely has files with\nconflict markers in them).\n"},{"id":"454980","messageId":"xmqq8rrdgpeu.fsf@gitster.g","threadId":"57855","inReplyTo":"CABPp-BGD3AZvXwmSHfQQ_xh_UqevH23kdBYijAWUk8GHu1q0Qw@mail.gmail.com","subject":"Re: Question about pre-merge and git merge octopus strategy","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-05-07T18:37:13Z","receivedAt":"2022-05-07T18:37:23Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren <newren@gmail.com> writes:\n\n> However, I think Junio said that octopus merge only handles trivial\n> cases anyway,...\n\n\"git merge -s octopus A B C\" deliberately made a design decision to\nstrongly discourage the users from creating an octopus that requires\nnon-trivial merges.  One of the reasons is because octopus merges\nmake the history less efficient to bisect, and they are meant as a\nway to bind totally unrelated changes, whose order of merging into\nthe resulting history does not matter.\n\nYes, it is an opinionated design choice, but \"git merge\" is a\nPorcelain that can afford to be opinionated.  It also means that\ntools other than the git-merge--octopus backend does not have to\nfollow the design philosophy.\n\n> ... in which case an iterated \"git merge --write-tree\" would\n> actually be a sufficient solution here and we could sidestep the more\n> convoluted cases.  But, sadly for ZheNing, that option doesn't exist\n> yet -- It's still under development.\n"},{"id":"454986","messageId":"CAOLTT8RscqL_05tg0cwBBQ50HmpsoqGYXpcWPTT9nzL2LueqxA@mail.gmail.com","threadId":"57855","inReplyTo":"CAP8UFD2p+Evqv_MBAgv23zooppsNWjOw6ZU2GLqAq_skZoJPOw@mail.gmail.com","subject":"Re: Question about pre-merge and git merge octopus strategy","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2022-05-08T14:13:20Z","receivedAt":"2022-05-08T14:14:50Z","isPatch":false,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Christian Couder <christian.couder@gmail.com> 于2022年5月7日周六 01:24写道：\n>\n> Hi,\n>\n> On Fri, May 6, 2022 at 10:15 AM ZheNing Hu <adlternative@gmail.com> wrote:\n>\n> > I am thinking about if git can \"pre-merge\" multiple branches, which\n> > can check if merge\n> > will have conflict, but not to merge them actually, like a option `--intend`.\n> >\n> > I find \"git merge-tree\" can output merge result in stdout, which meets\n> > my needs, but it can only\n> > support two branches' merge.\n>\n> Elijah (added in Cc) has been working on \"git merge-tree\" improvements\n> based on the new \"ort\" merge he developed. It supports merging 2\n> branches, but maybe there are ways to make it support more than 2.\n>\n\nOK. As he said, there is not such option :(\n\n> > So I find git merge with more than two branches can use octopus strategy.\n> > What about git merge --no-commit? Which will not commit automatically,\n> > so we can check if they have\n> > confilct, and abort merge.\n>\n> Yeah, I think that's what you want.\n>\n> > I think it's not useful for git merge-octopus, because if we meet a\n> > merge conflict, we can't find\n> > MERGE_HEAD at all! How can we abort this conflict merge?\n>\n> I don't know octopus merges much, but I think you should be able to\n> abort using \"git reset\" (maybe with \"--hard\"). If the merge was\n> performed using --no-commit or if there was a conflict, then I think\n> it should be expected that there is no MERGE_HEAD as no commit would\n> be created so MERGE_HEAD would have nothing to point to.\n\nYes, git reset --hard HEAD is the way get out of merging conflicts. But why\ngit merge --abort doesn't work? I think it may confuse users...\n"},{"id":"454987","messageId":"CAOLTT8STRfqq4bfobCK8Q5uvtXzgNYY0x0wNh4HNyAhv0Mc07A@mail.gmail.com","threadId":"57855","inReplyTo":"CABPp-BGD3AZvXwmSHfQQ_xh_UqevH23kdBYijAWUk8GHu1q0Qw@mail.gmail.com","subject":"Re: Question about pre-merge and git merge octopus strategy","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2022-05-08T14:44:43Z","receivedAt":"2022-05-08T14:44:57Z","isPatch":false,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Elijah Newren <newren@gmail.com> 于2022年5月7日周六 12:09写道：\n>\n> On Fri, May 6, 2022 at 10:24 AM Christian Couder\n> <christian.couder@gmail.com> wrote:\n> >\n> > Hi,\n> >\n> > On Fri, May 6, 2022 at 10:15 AM ZheNing Hu <adlternative@gmail.com> wrote:\n> >\n> > > I am thinking about if git can \"pre-merge\" multiple branches, which\n> > > can check if merge\n> > > will have conflict, but not to merge them actually, like a option `--intend`.\n> > >\n> > > I find \"git merge-tree\" can output merge result in stdout, which meets\n> > > my needs, but it can only\n> > > support two branches' merge.\n> >\n> > Elijah (added in Cc) has been working on \"git merge-tree\" improvements\n> > based on the new \"ort\" merge he developed. It supports merging 2\n> > branches, but maybe there are ways to make it support more than 2.\n>\n> The primary issue with in-core octopus merges is that there are lots\n> of questions about how to handle conflicts; possibly even more so than\n> git merge-tree --write-tree brings up, and that took us months of\n> discussion.  In particular, with octopii, do iterate merging one\n> branch in at a time and stop with any conflicts (thus potentially\n> stopping N-1 times when merging N branches), do you attempt to just\n> run all N-1 merges and have conflicts that can't readily be expressed\n> in the index (especially for non-textual multi-way conflicts, but even\n> nested conflict markers for text files could be painful), do you\n> attempt to handle the multi-way textual conflicts a new way with code\n> replacing xdiff/ in order to avoid nested conflicts?  I wasn't sure\n> what to do, so never implemented that in \"ort\".\n>\n\nI have see the code of git-merge-octopus.sh, it run merge-base,\nread-tree, write-tree every loop.\n\nNow I get you about it's hard to do multiple tree merge because it's hard to\nexpress in index.\n\n> Of course, people could roll their own by just serially merging pairs\n> of commits, and then rewriting the history to replace the string of\n> merges with an octopus.  Or perhaps just use \"git merge --no-commit\"\n> serially, though that only works if the branches touch disjoint files\n> (otherwise one of the merges will complain you have changes that could\n> be overwritten by the next merge).  And yes, if you don't want to mess\n> with the working tree/index, you could serially use the \"git\n> merge-tree --write-tree\" once I finish it, but it's not ready yet.\n> (Sorry about that; I've got a bunch of nearly complete changes from a\n> while ago but just didn't have much Git time.  I'll try to get to it\n> soon.)\n>\n\nI'll be looking forward to this feature!\n\n> However, I think Junio said that octopus merge only handles trivial\n> cases anyway, in which case an iterated \"git merge --write-tree\" would\n> actually be a sufficient solution here and we could sidestep the more\n> convoluted cases.  But, sadly for ZheNing, that option doesn't exist\n> yet -- It's still under development.\n>\n> > > So I find git merge with more than two branches can use octopus strategy.\n> > > What about git merge --no-commit? Which will not commit automatically,\n> > > so we can check if they have\n> > > confilct, and abort merge.\n> >\n> > Yeah, I think that's what you want.\n> >\n> > > I think it's not useful for git merge-octopus, because if we meet a\n> > > merge conflict, we can't find\n> > > MERGE_HEAD at all! How can we abort this conflict merge?\n>\n> MERGE_HEAD doesn't have anything to do with aborting the conflict\n> resolution step.  When you need to abort, the thing you want to go\n> back to is HEAD (which represents the commit you had checked out and\n> were merging the other stuff into), not MERGE_HEAD (which represents\n> the branch or branches you were merging into HEAD).\n>\n\nThanks for clarifying. As I reply to Christian, when I just use \"git\nmerge A B C\" happily,\nand there is a conflict, so I try \"git merge --abort\" as usual, but it\ncan not work... git tell me:\n\nfatal: There is no merge to abort (MERGE_HEAD missing).\n\n> > I don't know octopus merges much, but I think you should be able to\n> > abort using \"git reset\" (maybe with \"--hard\").  If the merge was\n> > performed using --no-commit or if there was a conflict, then I think\n> > it should be expected that there is no MERGE_HEAD as no commit would\n> > be created so MERGE_HEAD would have nothing to point to.\n>\n> MERGE_HEAD isn't something created during a merge, it's something that\n> existed before it -- namely, the tip of the other branch we are\n> merging.  For an octopus merge, you'd thus expect it to have N commits\n> rather than just 1.\n>\n> AUTO_MERGE, new to ort, is something that is created during a merge\n> and when the merge interrupts due to conflicts to ask the user to\n> resolve the conflicts, AUTO_MERGE represents the tree checked out in\n> the working copy (thus it is a tree that likely has files with\n> conflict markers in them).\n\nI think this AUTO_MERGE maybe a good thing for checking merge conflicts,\nmaybe it will help git merge-octopus.\n\nThanks.\n"},{"id":"454988","messageId":"20220508150153.74dvhl5cgi7wu5a4@carlos-mbp-2.lan","threadId":"57855","inReplyTo":"CAOLTT8S8rh+VYcuaqBeNtmphiRqw7HropLFpkxfnTJq6BngGXw@mail.gmail.com","subject":"Re: Question about pre-merge and git merge octopus strategy","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2022-05-08T15:01:53Z","receivedAt":"2022-05-08T15:02:01Z","isPatch":false,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Fri, May 06, 2022 at 04:14:56PM +0800, ZheNing Hu wrote:\n> \n> I am thinking about if git can \"pre-merge\" multiple branches, which\n> can check if merge\n> will have conflict, but not to merge them actually, like a option `--intend`.\n\nwhy not merge them and find that way if they have conflicts that are left\nunresolved, or are you also interested in conflicts that were solved by the\nmerge?\n\nif you only care about the unresolved conflicts then (unless this broke\nrecently with ort) the following (untested) should work:\n\n  $ git switch target\n  $ git merge topic && echo \"SHIP IT!\" || git reset --hard ORIG_HEAD\n\nobviously will need to be changed if you want to merge more than two branches\nbut the principle is tme same.\n\nCarlo\n"},{"id":"455075","messageId":"CABPp-BEFYjTvK4ZNSg+yiahRAzXW=KU11a-b+QRdcBDGb-movQ@mail.gmail.com","threadId":"57855","inReplyTo":"CAOLTT8STRfqq4bfobCK8Q5uvtXzgNYY0x0wNh4HNyAhv0Mc07A@mail.gmail.com","subject":"Re: Question about pre-merge and git merge octopus strategy","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-05-10T07:07:17Z","receivedAt":"2022-05-10T07:33:34Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Sun, May 8, 2022 at 7:44 AM ZheNing Hu <adlternative@gmail.com> wrote:\n>\n> Elijah Newren <newren@gmail.com> 于2022年5月7日周六 12:09写道：\n> >\n> > On Fri, May 6, 2022 at 10:24 AM Christian Couder\n> > <christian.couder@gmail.com> wrote:\n> > >\n> > > On Fri, May 6, 2022 at 10:15 AM ZheNing Hu <adlternative@gmail.com> wrote:\n[...]\n> > > > I think it's not useful for git merge-octopus, because if we meet a\n> > > > merge conflict, we can't find\n> > > > MERGE_HEAD at all! How can we abort this conflict merge?\n> >\n> > MERGE_HEAD doesn't have anything to do with aborting the conflict\n> > resolution step.  When you need to abort, the thing you want to go\n> > back to is HEAD (which represents the commit you had checked out and\n> > were merging the other stuff into), not MERGE_HEAD (which represents\n> > the branch or branches you were merging into HEAD).\n> >\n>\n> Thanks for clarifying. As I reply to Christian, when I just use \"git\n> merge A B C\" happily,\n> and there is a conflict, so I try \"git merge --abort\" as usual, but it\n> can not work... git tell me:\n>\n> fatal: There is no merge to abort (MERGE_HEAD missing).\n\nSounds like a bug to me; .git/MERGE_HEAD should be written.  That file\nis created for me when I set up a simple octopus merge that has\nconflicts.  Do you have a set of steps others can use to reproduce the\nproblem you are seeing?\n"},{"id":"455159","messageId":"CAOLTT8R7QmpvaFPTRs3xTpxr7eiuxF-ZWtvUUSC0-JOo9Y+SqA@mail.gmail.com","threadId":"57855","inReplyTo":"CABPp-BEFYjTvK4ZNSg+yiahRAzXW=KU11a-b+QRdcBDGb-movQ@mail.gmail.com","subject":"Re: Question about pre-merge and git merge octopus strategy","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2022-05-11T11:21:14Z","receivedAt":"2022-05-11T11:21:38Z","isPatch":false,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Elijah Newren <newren@gmail.com> 于2022年5月10日周二 15:07写道：\n>\n> On Sun, May 8, 2022 at 7:44 AM ZheNing Hu <adlternative@gmail.com> wrote:\n> >\n> > Elijah Newren <newren@gmail.com> 于2022年5月7日周六 12:09写道：\n> > >\n> > > On Fri, May 6, 2022 at 10:24 AM Christian Couder\n> > > <christian.couder@gmail.com> wrote:\n> > > >\n> > > > On Fri, May 6, 2022 at 10:15 AM ZheNing Hu <adlternative@gmail.com> wrote:\n> [...]\n> > > > > I think it's not useful for git merge-octopus, because if we meet a\n> > > > > merge conflict, we can't find\n> > > > > MERGE_HEAD at all! How can we abort this conflict merge?\n> > >\n> > > MERGE_HEAD doesn't have anything to do with aborting the conflict\n> > > resolution step.  When you need to abort, the thing you want to go\n> > > back to is HEAD (which represents the commit you had checked out and\n> > > were merging the other stuff into), not MERGE_HEAD (which represents\n> > > the branch or branches you were merging into HEAD).\n> > >\n> >\n> > Thanks for clarifying. As I reply to Christian, when I just use \"git\n> > merge A B C\" happily,\n> > and there is a conflict, so I try \"git merge --abort\" as usual, but it\n> > can not work... git tell me:\n> >\n> > fatal: There is no merge to abort (MERGE_HEAD missing).\n>\n> Sounds like a bug to me; .git/MERGE_HEAD should be written.  That file\n> is created for me when I set up a simple octopus merge that has\n> conflicts.  Do you have a set of steps others can use to reproduce the\n> problem you are seeing?\n\nLet me minimally reproduce this question (with git 2.33.0):\n\n#!/bin/bash\n\nrm -rf test-repo\ngit init test-repo\ncd test-repo\ngit branch -m main\necho base > base\ngit add .\ngit commit -m \"base\"\ngit branch -c dev-1\ngit branch -c dev-2\necho main > main\ngit add .\ngit commit -m \"main change\"\ngit checkout dev-1\necho dev-1 >> base\ngit add .\ngit commit -m \"dev-1 change\"\ngit checkout dev-2\necho dev-2 >> base\ngit add .\ngit commit -m \"dev-2 change\"\ngit checkout main\necho main >> base\ngit add .\ngit commit -m \"main change\"\ngit merge dev-1 dev-2\nfile .git/MERGE_HEAD\n\nwhich output:\n\n.git/MERGE_HEAD: cannot open `.git/MERGE_HEAD' (No such file or directory)\n"},{"id":"455201","messageId":"CABPp-BHmNBMypVDrE=gPMXoHT9uH-u4HJG1dAuY0tjSGrK0yPg@mail.gmail.com","threadId":"57855","inReplyTo":"CAOLTT8R7QmpvaFPTRs3xTpxr7eiuxF-ZWtvUUSC0-JOo9Y+SqA@mail.gmail.com","subject":"Re: Question about pre-merge and git merge octopus strategy","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-05-12T15:04:41Z","receivedAt":"2022-05-12T15:04:58Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Wed, May 11, 2022 at 4:21 AM ZheNing Hu <adlternative@gmail.com> wrote:\n>\n> Elijah Newren <newren@gmail.com> 于2022年5月10日周二 15:07写道：\n> >\n> > On Sun, May 8, 2022 at 7:44 AM ZheNing Hu <adlternative@gmail.com> wrote:\n[...]\n> > > Thanks for clarifying. As I reply to Christian, when I just use \"git\n> > > merge A B C\" happily,\n> > > and there is a conflict, so I try \"git merge --abort\" as usual, but it\n> > > can not work... git tell me:\n> > >\n> > > fatal: There is no merge to abort (MERGE_HEAD missing).\n> >\n> > Sounds like a bug to me; .git/MERGE_HEAD should be written.  That file\n> > is created for me when I set up a simple octopus merge that has\n> > conflicts.  Do you have a set of steps others can use to reproduce the\n> > problem you are seeing?\n>\n> Let me minimally reproduce this question (with git 2.33.0):\n>\n> #!/bin/bash\n>\n> rm -rf test-repo\n> git init test-repo\n> cd test-repo\n> git branch -m main\n> echo base > base\n> git add .\n> git commit -m \"base\"\n> git branch -c dev-1\n> git branch -c dev-2\n> echo main > main\n> git add .\n> git commit -m \"main change\"\n> git checkout dev-1\n> echo dev-1 >> base\n> git add .\n> git commit -m \"dev-1 change\"\n> git checkout dev-2\n> echo dev-2 >> base\n> git add .\n> git commit -m \"dev-2 change\"\n> git checkout main\n> echo main >> base\n> git add .\n> git commit -m \"main change\"\n> git merge dev-1 dev-2\n> file .git/MERGE_HEAD\n>\n> which output:\n>\n> .git/MERGE_HEAD: cannot open `.git/MERGE_HEAD' (No such file or directory)\n\nAh, thanks.  With this testcase, the output is:\n\n    $ git merge dev-1 dev-2\n    Trying simple merge with dev-1\n    Simple merge did not work, trying automatic merge.\n    Auto-merging base\n    ERROR: content conflict in base\n    fatal: merge program failed\n    Automated merge did not work.\n    Should not be doing an octopus.\n    Merge with strategy octopus failed.\n\nAlso, if we check `git status`:\n\n    $ git status\n    On branch main\n    Unmerged paths:\n      (use \"git restore --staged <file>...\" to unstage)\n      (use \"git add <file>...\" to mark resolution)\n    both modified:   base\n\n    no changes added to commit (use \"git add\" and/or \"git commit -a\")\n\nAnd in git-merge-octopus.sh we see:\n\n    case \"$OCTOPUS_FAILURE\" in\n    1)\n    # We allow only last one to have a hand-resolvable\n    # conflicts.  Last round failed and we still had\n    # a head to merge.\n    gettextln \"Automated merge did not work.\"\n    gettextln \"Should not be doing an octopus.\"\n    exit 2\n    esac\n\nand in builtin/merge.c, we see:\n\n    /*\n     * The backend exits with 1 when conflicts are\n     * left to be resolved, with 2 when it does not\n     * handle the given merge at all.\n     */\n\nWhich means git-merge-octopus.sh is claiming it can't handle this type\nof merge, and some other merge strategy should be tried, and\nimplicitly that it didn't leave any conflicts to be resolved because\nit can't handle this merge.  But it clearly decides to leave the\nmodifications it made to the index and working tree around, which just\nseems wrong to me.  If it doesn't handle the merge and other merge\nstrategies (if available) should be tried, it should leave things the\nway it found them.  So, perhaps a patch like the below would do that;\ncan you give it a try to see if it solves your cases (it works for\nyour simplified testcase, but I'm curious about your more involved\ncases and if it works for them too)?\n\n\n----- >8 -----\n\ndiff --git a/git-merge-octopus.sh b/git-merge-octopus.sh\nindex 7d19d37951..6ddbdc8f56 100755\n--- a/git-merge-octopus.sh\n+++ b/git-merge-octopus.sh\n@@ -47,6 +47,13 @@ then\n     git diff-index --cached --name-only HEAD -- | sed -e 's/^/    /'\n     exit 2\n fi\n+\n+# If octopus is inapplicable, make sure we undo any changes we made first\n+cannot_octopus() {\n+       git reset --merge\n+       exit 2\n+}\n+\n MRC=$(git rev-parse --verify -q $head)\n MRT=$(git write-tree)\n NON_FF_MERGE=0\n@@ -60,7 +67,7 @@ do\n                # a head to merge.\n                gettextln \"Automated merge did not work.\"\n                gettextln \"Should not be doing an octopus.\"\n-               exit 2\n+               cannot_octopus\n        esac\n\n        eval pretty_name=\\${GITHEAD_$SHA1:-$SHA1}\n@@ -95,7 +102,8 @@ do\n        NON_FF_MERGE=1\n\n        eval_gettextln \"Trying simple merge with \\$pretty_name\"\n-       git read-tree -u -m --aggressive  $common $MRT $SHA1 || exit 2\n+       git read-tree -u -m --aggressive  $common $MRT $SHA1 || cannot_octopus\n+\n        next=$(git write-tree 2>/dev/null)\n        if test $? -ne 0\n        then\n"},{"id":"455208","messageId":"xmqqk0aqhia1.fsf@gitster.g","threadId":"57855","inReplyTo":"CABPp-BHmNBMypVDrE=gPMXoHT9uH-u4HJG1dAuY0tjSGrK0yPg@mail.gmail.com","subject":"Re: Question about pre-merge and git merge octopus strategy","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-05-12T15:39:34Z","receivedAt":"2022-05-12T15:39:45Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren <newren@gmail.com> writes:\n\n>     Merge with strategy octopus failed.\n>\n> Also, if we check `git status`:\n>\n>     $ git status\n>     On branch main\n>     Unmerged paths:\n>       (use \"git restore --staged <file>...\" to unstage)\n>       (use \"git add <file>...\" to mark resolution)\n>     both modified:   base\n>\n>     no changes added to commit (use \"git add\" and/or \"git commit -a\")\n>\n> And in git-merge-octopus.sh we see:\n>\n>     case \"$OCTOPUS_FAILURE\" in\n>     1)\n>     # We allow only last one to have a hand-resolvable\n>     # conflicts.  Last round failed and we still had\n>     # a head to merge.\n>     gettextln \"Automated merge did not work.\"\n>     gettextln \"Should not be doing an octopus.\"\n>     exit 2\n>     esac\n>\n> and in builtin/merge.c, we see:\n>\n>     /*\n>      * The backend exits with 1 when conflicts are\n>      * left to be resolved, with 2 when it does not\n>      * handle the given merge at all.\n>      */\n>\n> Which means git-merge-octopus.sh is claiming it can't handle this type\n> of merge, and some other merge strategy should be tried, and\n> implicitly that it didn't leave any conflicts to be resolved because\n> it can't handle this merge.\n\nCorrect.  Near the beginning of the loop you found the above\ncomment, there is this code:\n\n\tif (use_strategies_nr == 1 ||\n\t    /*\n\t     * Stash away the local changes so that we can try more than one.\n\t     */\n\t    save_state(&stash))\n\t\toidclr(&stash);\n\n\tfor (i = 0; !merge_was_ok && i < use_strategies_nr; i++) {\n\t\tint ret, cnt;\n\t\tif (i) {\n\t\t\tprintf(_(\"Rewinding the tree to pristine...\\n\"));\n\t\t\trestore_state(&head_commit->object.oid, &stash);\n\t\t}\n\nbut that save-then-restore triggers ONLY when there are multiple\nstrategies to try.  Unfortunately, octopus has no friends to fall\nback on, so we do not do the save-restore dance on the calling side.\n\n> But it clearly decides to leave the\n> modifications it made to the index and working tree around, which just\n> seems wrong to me.\n\nIf merge-recursive or merge-resolve is asked to merge a single\ncommit to the current branch without any other strategies to use as\na fallback, they leave the working tree and index into a state where\nthe end-user can conclude the conflict resolution and commit the\nresult.  In spirit, we are in the same situation, aren't we?\n\nThe user, if they want to proceed against octopus's opinion, would\nresolve the current conflict, read-tree -m the next one, ..., to\nconclude and commit the result. \n\nSo I am not sure if it is a good idea to unconditionally \"reset --merge\"\nin this situation.\n\n> +# If octopus is inapplicable, make sure we undo any changes we made first\n> +cannot_octopus() {\n> +       git reset --merge\n> +       exit 2\n> +}\n"},{"id":"455253","messageId":"CABPp-BGOGLUPOn20yWzCrBYCykiet0=5UfbkuGC78f-QoWVvYg@mail.gmail.com","threadId":"57855","inReplyTo":"xmqqk0aqhia1.fsf@gitster.g","subject":"Re: Question about pre-merge and git merge octopus strategy","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-05-13T05:15:52Z","receivedAt":"2022-05-13T05:16:23Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Thu, May 12, 2022 at 8:39 AM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Elijah Newren <newren@gmail.com> writes:\n>\n> >     Merge with strategy octopus failed.\n> >\n> > Also, if we check `git status`:\n> >\n> >     $ git status\n> >     On branch main\n> >     Unmerged paths:\n> >       (use \"git restore --staged <file>...\" to unstage)\n> >       (use \"git add <file>...\" to mark resolution)\n> >     both modified:   base\n> >\n> >     no changes added to commit (use \"git add\" and/or \"git commit -a\")\n> >\n> > And in git-merge-octopus.sh we see:\n> >\n> >     case \"$OCTOPUS_FAILURE\" in\n> >     1)\n> >     # We allow only last one to have a hand-resolvable\n> >     # conflicts.  Last round failed and we still had\n> >     # a head to merge.\n> >     gettextln \"Automated merge did not work.\"\n> >     gettextln \"Should not be doing an octopus.\"\n> >     exit 2\n> >     esac\n> >\n> > and in builtin/merge.c, we see:\n> >\n> >     /*\n> >      * The backend exits with 1 when conflicts are\n> >      * left to be resolved, with 2 when it does not\n> >      * handle the given merge at all.\n> >      */\n> >\n> > Which means git-merge-octopus.sh is claiming it can't handle this type\n> > of merge, and some other merge strategy should be tried, and\n> > implicitly that it didn't leave any conflicts to be resolved because\n> > it can't handle this merge.\n>\n> Correct.  Near the beginning of the loop you found the above\n> comment, there is this code:\n>\n>         if (use_strategies_nr == 1 ||\n>             /*\n>              * Stash away the local changes so that we can try more than one.\n>              */\n>             save_state(&stash))\n>                 oidclr(&stash);\n>\n>         for (i = 0; !merge_was_ok && i < use_strategies_nr; i++) {\n>                 int ret, cnt;\n>                 if (i) {\n>                         printf(_(\"Rewinding the tree to pristine...\\n\"));\n>                         restore_state(&head_commit->object.oid, &stash);\n>                 }\n\nSide-comment, which becomes important below: The save/restore code in\nbuiltin/merge.c appears to be broken to me.  As noted in the code\nabove, stash will be set to null_oid() if save_state() returns\nnon-zero (which happens when \"stash create\" has no output, which\nhappens if there is _initially_ no state to save, i.e. if there are no\nlocal changes before the merge started).  restore_state() is a no-op\nwhenever stash is the null_oid, meaning in that case it won't actually\nrewind the tree to a pristine state to undo the changes of the\nprevious merge attempt.  So, if:\n\n* The user had no local changes before starting the merge\n* Multiple merge strategies are applicable\n* The first merge strategy makes index/working-tree changes, but\nreturns with exit status 2\n\nThen the restore_state() called before the second merge strategy will\ndo nothing, and the second merge strategy will be working on an index\nand working tree with garbage leftover from the first merge strategy.\nWhile this may have never been triggered (in what case do we have\nmultiple merge strategies that all return an exit status of 2?), I\nsuspect we want to fix this problem with something like this:\n\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex f178f5a3ee..7f3650fb09 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -378,11 +378,11 @@ static void restore_state(const struct object_id *head,\n        struct strbuf sb = STRBUF_INIT;\n        const char *args[] = { \"stash\", \"apply\", NULL, NULL };\n\n+       reset_hard(head, 1);\n+\n        if (is_null_oid(stash))\n                return;\n\n-       reset_hard(head, 1);\n-\n        args[2] = oid_to_hex(stash);\n\n        /*\n\n> but that save-then-restore triggers ONLY when there are multiple\n> strategies to try.  Unfortunately, octopus has no friends to fall\n> back on, so we do not do the save-restore dance on the calling side.\n\nIf octopus did have a friend which also failed in the same way, the\nrestore_state() that you highlighted would only trigger for the first\nsuch strategy and not the second.  However, restore_state() would\nstill be called after the second strategy, it would just come from a\nlater section of the code, highlighted here:\n\n    /*\n     * Pick the result from the best strategy and have the user fix\n     * it up.\n     */\n    if (!best_strategy) {\n        restore_state(&head_commit->object.oid, &stash);\n        if (use_strategies_nr > 1)\n            fprintf(stderr,\n                    _(\"No merge strategy handled the merge.\\n\"));\n        else\n            fprintf(stderr, _(\"Merge with strategy %s failed.\\n\"),\n                    use_strategies[0]->name);\n\nInterestingly, the restore_state() call here does trigger even when we\nonly have octopus, but in that case it's a no-op because stash will\nalways be the null_oid because of the special case \"use_strategies_nr\n== 1\" in the code you highlighted.\n\n> > But it clearly decides to leave the\n> > modifications it made to the index and working tree around, which just\n> > seems wrong to me.\n>\n> If merge-recursive or merge-resolve is asked to merge a single\n> commit to the current branch without any other strategies to use as\n> a fallback, they leave the working tree and index into a state where\n> the end-user can conclude the conflict resolution and commit the\n> result.  In spirit, we are in the same situation, aren't we?\n\nI don't think it's quite the same.  Those strategies return an exit\nstatus of 1 on conflicts; if they returned a 2 (that is, if they all\nreturned a 2), meaning \"I don't handle this\", then in spirit we'd be\nin the same situation.  If they all returned a 2, then best_strategy\nwould remain NULL and the code would do the restore_state() I\nhighlighted above in the (!best_stratgegy) block.  I believe the\nintent of that restore_state() call was meant to make the working tree\nand index be clean afterward (even if that's not its effect as I\nmentioned in my side-note about its brokenness), and thus there would\nbe no conflict resolution for a user to perform.\n\nHowever, some alternatives...\n\nPerhaps you are arguing that git-merge-octopus.sh should return a 1\nhere instead of a 2, i.e. octopus handled the merge as far as it\ncould, but there are conflicts for the user to address?\n\nOr are you saying that if all merge strategies return a 2, we just\ntreat the last one as good enough and consider the merge to be in\nprogress?  If that's your intent, we should probably remove the\nrestore_state() call in the \"!best_strategy\" block, and add a call to\nwrite_merge_state(remoteheads) so that .git/MERGE_HEAD and friends get\nwritten; something like this:\n\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex f178f5a3ee..397eb9c228 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -1721,14 +1721,13 @@ int cmd_merge(int argc, const char **argv,\nconst char *prefix)\n         * it up.\n         */\n        if (!best_strategy) {\n-               restore_state(&head_commit->object.oid, &stash);\n+               write_merge_state(remoteheads);\n                if (use_strategies_nr > 1)\n                        fprintf(stderr,\n                                _(\"No merge strategy handled the merge.\\n\"));\n                else\n                        fprintf(stderr, _(\"Merge with strategy %s failed.\\n\"),\n                                use_strategies[0]->name);\n-               apply_autostash(git_path_merge_autostash(the_repository));\n                ret = 2;\n                goto done;\n        } else if (best_strategy == wt_strategy)\n\n\n> The user, if they want to proceed against octopus's opinion, would\n> resolve the current conflict, read-tree -m the next one, ..., to\n> conclude and commit the result.\n\nThat may be reasonable, but are there a few usability questions here?\nIn particular, would users know which trees have been merged and which\nremain that they need to call \"read-tree -m\" on?  Would they even know\nthat repeated invocations of \"read-tree -m\" are in order?  Also, do\nthey just memorize \"read-tree -m\", or would they perhaps expect \"git\nmerge --continue\" to invoke it for them?\n\n> So I am not sure if it is a good idea to unconditionally \"reset --merge\"\n> in this situation.\n\nYeah, I think it should be handled by builtin/merge.c given how it's\napparently meant to be handling saving and restoring state.\n"},{"id":"455262","messageId":"xmqq35hdd205.fsf@gitster.g","threadId":"57855","inReplyTo":"CABPp-BGOGLUPOn20yWzCrBYCykiet0=5UfbkuGC78f-QoWVvYg@mail.gmail.com","subject":"Re: Question about pre-merge and git merge octopus strategy","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-05-13T12:56:58Z","receivedAt":"2022-05-13T12:57:09Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren <newren@gmail.com> writes:\n\n>> If merge-recursive or merge-resolve is asked to merge a single\n>> commit to the current branch without any other strategies to use as\n>> a fallback, they leave the working tree and index into a state where\n>> the end-user can conclude the conflict resolution and commit the\n>> result.  In spirit, we are in the same situation, aren't we?\n>\n> I don't think it's quite the same....\n> ...\n> Or are you saying that if all merge strategies return a 2, we just\n> treat the last one as good enough and consider the merge to be in\n> progress?\n\nNo, I was just confusing the difference between the special return\nvalue 2 and a normal failure value 1.\n\nI have a feeling that it would be nice if we can restore to pristine\non the calling \"git merge\" side, rather than forcing individual\nstrategy backends to implement the restore-to-pristine correctly,\nbut in any case, as you said, we should behave as if a merge\nstrategy backend never run after it failed with exit value 2 by\nrestoring to pristine state.\n"},{"id":"455481","messageId":"CAOLTT8TDomk9zUg49yrhpu1T776FKrnu7RGJ_bx9iaYKUEvvTg@mail.gmail.com","threadId":"57855","inReplyTo":"CABPp-BGOGLUPOn20yWzCrBYCykiet0=5UfbkuGC78f-QoWVvYg@mail.gmail.com","subject":"Re: Question about pre-merge and git merge octopus strategy","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2022-05-19T13:15:19Z","receivedAt":"2022-05-19T13:15:37Z","isPatch":false,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Elijah Newren <newren@gmail.com> 于2022年5月13日周五 13:16写道：\n>\n> On Thu, May 12, 2022 at 8:39 AM Junio C Hamano <gitster@pobox.com> wrote:\n> >\n> > Elijah Newren <newren@gmail.com> writes:\n> >\n> > >     Merge with strategy octopus failed.\n> > >\n> > > Also, if we check `git status`:\n> > >\n> > >     $ git status\n> > >     On branch main\n> > >     Unmerged paths:\n> > >       (use \"git restore --staged <file>...\" to unstage)\n> > >       (use \"git add <file>...\" to mark resolution)\n> > >     both modified:   base\n> > >\n> > >     no changes added to commit (use \"git add\" and/or \"git commit -a\")\n> > >\n> > > And in git-merge-octopus.sh we see:\n> > >\n> > >     case \"$OCTOPUS_FAILURE\" in\n> > >     1)\n> > >     # We allow only last one to have a hand-resolvable\n> > >     # conflicts.  Last round failed and we still had\n> > >     # a head to merge.\n> > >     gettextln \"Automated merge did not work.\"\n> > >     gettextln \"Should not be doing an octopus.\"\n> > >     exit 2\n> > >     esac\n> > >\n> > > and in builtin/merge.c, we see:\n> > >\n> > >     /*\n> > >      * The backend exits with 1 when conflicts are\n> > >      * left to be resolved, with 2 when it does not\n> > >      * handle the given merge at all.\n> > >      */\n> > >\n> > > Which means git-merge-octopus.sh is claiming it can't handle this type\n> > > of merge, and some other merge strategy should be tried, and\n> > > implicitly that it didn't leave any conflicts to be resolved because\n> > > it can't handle this merge.\n> >\n> > Correct.  Near the beginning of the loop you found the above\n> > comment, there is this code:\n> >\n> >         if (use_strategies_nr == 1 ||\n> >             /*\n> >              * Stash away the local changes so that we can try more than one.\n> >              */\n> >             save_state(&stash))\n> >                 oidclr(&stash);\n> >\n> >         for (i = 0; !merge_was_ok && i < use_strategies_nr; i++) {\n> >                 int ret, cnt;\n> >                 if (i) {\n> >                         printf(_(\"Rewinding the tree to pristine...\\n\"));\n> >                         restore_state(&head_commit->object.oid, &stash);\n> >                 }\n>\n> Side-comment, which becomes important below: The save/restore code in\n> builtin/merge.c appears to be broken to me.  As noted in the code\n> above, stash will be set to null_oid() if save_state() returns\n> non-zero (which happens when \"stash create\" has no output, which\n> happens if there is _initially_ no state to save, i.e. if there are no\n> local changes before the merge started).  restore_state() is a no-op\n> whenever stash is the null_oid, meaning in that case it won't actually\n> rewind the tree to a pristine state to undo the changes of the\n> previous merge attempt.  So, if:\n>\n> * The user had no local changes before starting the merge\n> * Multiple merge strategies are applicable\n> * The first merge strategy makes index/working-tree changes, but\n> returns with exit status 2\n>\n> Then the restore_state() called before the second merge strategy will\n> do nothing, and the second merge strategy will be working on an index\n> and working tree with garbage leftover from the first merge strategy.\n> While this may have never been triggered (in what case do we have\n> multiple merge strategies that all return an exit status of 2?), I\n> suspect we want to fix this problem with something like this:\n>\n> diff --git a/builtin/merge.c b/builtin/merge.c\n> index f178f5a3ee..7f3650fb09 100644\n> --- a/builtin/merge.c\n> +++ b/builtin/merge.c\n> @@ -378,11 +378,11 @@ static void restore_state(const struct object_id *head,\n>         struct strbuf sb = STRBUF_INIT;\n>         const char *args[] = { \"stash\", \"apply\", NULL, NULL };\n>\n> +       reset_hard(head, 1);\n> +\n>         if (is_null_oid(stash))\n>                 return;\n>\n> -       reset_hard(head, 1);\n> -\n>         args[2] = oid_to_hex(stash);\n>\n>         /*\n>\n\nIt looks like this code can go back to the old state now?\n\nI think if we can handle it as special case when:\n\n1. Use octopus merge without other strategies (no friend).\n2. Merge failed, so we don't have the best strategy.\n\nThen we force restore the original state?\n"},{"id":"455487","messageId":"CABPp-BFrifAymOVNniXiKWv2tZpgxQwxmZAhhGE8eaeJVODNkA@mail.gmail.com","threadId":"57855","inReplyTo":"CAOLTT8TDomk9zUg49yrhpu1T776FKrnu7RGJ_bx9iaYKUEvvTg@mail.gmail.com","subject":"Re: Question about pre-merge and git merge octopus strategy","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-05-19T14:46:56Z","receivedAt":"2022-05-19T14:47:14Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Thu, May 19, 2022 at 6:15 AM ZheNing Hu <adlternative@gmail.com> wrote:\n>\n> Elijah Newren <newren@gmail.com> 于2022年5月13日周五 13:16写道：\n> >\n> > On Thu, May 12, 2022 at 8:39 AM Junio C Hamano <gitster@pobox.com> wrote:\n> > >\n> > > Elijah Newren <newren@gmail.com> writes:\n> > >\n> > > >     Merge with strategy octopus failed.\n> > > >\n> > > > Also, if we check `git status`:\n> > > >\n> > > >     $ git status\n> > > >     On branch main\n> > > >     Unmerged paths:\n> > > >       (use \"git restore --staged <file>...\" to unstage)\n> > > >       (use \"git add <file>...\" to mark resolution)\n> > > >     both modified:   base\n> > > >\n> > > >     no changes added to commit (use \"git add\" and/or \"git commit -a\")\n> > > >\n> > > > And in git-merge-octopus.sh we see:\n> > > >\n> > > >     case \"$OCTOPUS_FAILURE\" in\n> > > >     1)\n> > > >     # We allow only last one to have a hand-resolvable\n> > > >     # conflicts.  Last round failed and we still had\n> > > >     # a head to merge.\n> > > >     gettextln \"Automated merge did not work.\"\n> > > >     gettextln \"Should not be doing an octopus.\"\n> > > >     exit 2\n> > > >     esac\n> > > >\n> > > > and in builtin/merge.c, we see:\n> > > >\n> > > >     /*\n> > > >      * The backend exits with 1 when conflicts are\n> > > >      * left to be resolved, with 2 when it does not\n> > > >      * handle the given merge at all.\n> > > >      */\n> > > >\n> > > > Which means git-merge-octopus.sh is claiming it can't handle this type\n> > > > of merge, and some other merge strategy should be tried, and\n> > > > implicitly that it didn't leave any conflicts to be resolved because\n> > > > it can't handle this merge.\n> > >\n> > > Correct.  Near the beginning of the loop you found the above\n> > > comment, there is this code:\n> > >\n> > >         if (use_strategies_nr == 1 ||\n> > >             /*\n> > >              * Stash away the local changes so that we can try more than one.\n> > >              */\n> > >             save_state(&stash))\n> > >                 oidclr(&stash);\n> > >\n> > >         for (i = 0; !merge_was_ok && i < use_strategies_nr; i++) {\n> > >                 int ret, cnt;\n> > >                 if (i) {\n> > >                         printf(_(\"Rewinding the tree to pristine...\\n\"));\n> > >                         restore_state(&head_commit->object.oid, &stash);\n> > >                 }\n> >\n> > Side-comment, which becomes important below: The save/restore code in\n> > builtin/merge.c appears to be broken to me.  As noted in the code\n> > above, stash will be set to null_oid() if save_state() returns\n> > non-zero (which happens when \"stash create\" has no output, which\n> > happens if there is _initially_ no state to save, i.e. if there are no\n> > local changes before the merge started).  restore_state() is a no-op\n> > whenever stash is the null_oid, meaning in that case it won't actually\n> > rewind the tree to a pristine state to undo the changes of the\n> > previous merge attempt.  So, if:\n> >\n> > * The user had no local changes before starting the merge\n> > * Multiple merge strategies are applicable\n> > * The first merge strategy makes index/working-tree changes, but\n> > returns with exit status 2\n> >\n> > Then the restore_state() called before the second merge strategy will\n> > do nothing, and the second merge strategy will be working on an index\n> > and working tree with garbage leftover from the first merge strategy.\n> > While this may have never been triggered (in what case do we have\n> > multiple merge strategies that all return an exit status of 2?), I\n> > suspect we want to fix this problem with something like this:\n> >\n> > diff --git a/builtin/merge.c b/builtin/merge.c\n> > index f178f5a3ee..7f3650fb09 100644\n> > --- a/builtin/merge.c\n> > +++ b/builtin/merge.c\n> > @@ -378,11 +378,11 @@ static void restore_state(const struct object_id *head,\n> >         struct strbuf sb = STRBUF_INIT;\n> >         const char *args[] = { \"stash\", \"apply\", NULL, NULL };\n> >\n> > +       reset_hard(head, 1);\n> > +\n> >         if (is_null_oid(stash))\n> >                 return;\n> >\n> > -       reset_hard(head, 1);\n> > -\n> >         args[2] = oid_to_hex(stash);\n> >\n> >         /*\n> >\n>\n> It looks like this code can go back to the old state now?\n>\n> I think if we can handle it as special case when:\n>\n> 1. Use octopus merge without other strategies (no friend).\n> 2. Merge failed, so we don't have the best strategy.\n>\n> Then we force restore the original state?\n\nYeah, I've got some patches ready; just didn't submit yet as I was\nhoping to get time to update the in-core merge series.\nhttps://github.com/gitgitgadget/git/pull/1231\n"}]}