{"thread":{"id":"46660","subject":"Should rerere auto-update a merge resolution?","startedAt":"2017-08-23T19:39:38Z","lastAt":"2017-08-25T17:08:08Z","messageCount":8,"participants":["Martin Langhoff","Junio C Hamano","Johannes Schindelin"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"327072","messageId":"CACPiFCJH7RSb_rz6M6ADuGi0q+oeWYhE1fNMQC0EUcCn_kCJwg@mail.gmail.com","threadId":"46660","inReplyTo":null,"subject":"Should rerere auto-update a merge resolution?","fromName":"Martin Langhoff","fromEmail":"martin.langhoff@gmail.com","sentAt":"2017-08-23T19:39:04Z","receivedAt":"2017-08-23T19:39:38Z","isPatch":false,"sender":{"key":"martin.langhoff@gmail.com","avatar":"https://gravatar.com/avatar/1e3f311b6c4c15836501901ca58f8c0b0667246488084ba524d8bc9867e22fd9?d=mp&s=160"},"body":"Hi List!\n\nLet's say...\n - git v2.9.4\n - rerere is enabled.\n - I merge maint into master, resolve erroneously, commit\n - I publish my merge in a temp branch, a reviewer points out my mistake\n - I reset hard, retry the merge, using --no-commit, rerere applies\nwhat it knows\n - I fix things up, then commit\n\nSo far so good.\n\nOops! One of the branches has moved forward in the meantime, so\n\n - git fetch\n - git reset --hard master\n - git merge maint\n... rerere applies the first (incorrect) resolution...\n\nAm I doing it wrong? {C,Sh}ould rerere have done better?\n\ncheers,\n\n\nm\n-- \n martin.langhoff@gmail.com\n - ask interesting questions ~ http://linkedin.com/in/martinlanghoff\n - don't be distracted ~ http://github.com/martin-langhoff\n by shiny stuff\n"},{"id":"327082","messageId":"xmqqlgmaxbuc.fsf@gitster.mtv.corp.google.com","threadId":"46660","inReplyTo":"CACPiFCJH7RSb_rz6M6ADuGi0q+oeWYhE1fNMQC0EUcCn_kCJwg@mail.gmail.com","subject":"Re: Should rerere auto-update a merge resolution?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-23T20:34:19Z","receivedAt":"2017-08-23T20:34:27Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin Langhoff writes:\n\n> Hi List!\n>\n> Let's say...\n> - git v2.9.4\n> - rerere is enabled.\n> - I merge maint into master, resolve erroneously, commit\n> - I publish my merge in a temp branch, a reviewer points out my mistake\n> - I reset hard, retry the merge, using --no-commit, rerere applies\n> what it knows\n> - I fix things up, then commit\n>\n> So far so good.\n>\n> Oops! One of the branches has moved forward in the meantime, so\n>\n> - git fetch\n> - git reset --hard master\n> - git merge maint\n> ... rerere applies the first (incorrect) resolution...\n>\n> Am I doing it wrong? {C,Sh}ould rerere have done better?\n\nBetween these two steps:\n\n> - I reset hard, retry the merge, using --no-commit, rerere applies what it knows\n> - I fix things up, then commit\n\nYou'd tell rerere to forget what it knows because it is wrong. Then\nafter these two (eh, now \"three\" because there is the \"forget\"\nstep), \"rerere\" notices that an updated resolution needs to be\nrecorded, so it remembers it. Later re-resolution will replay the\ncorrected one, simply because the old incorrect one is forgotten and\nreplaced by the updated correct one.\n"},{"id":"327087","messageId":"CACPiFCJCgKtTbKX8jCSC3QgMKZ7Usu2ojqXe5w_QAHwk7T4M-A@mail.gmail.com","threadId":"46660","inReplyTo":"xmqqlgmaxbuc.fsf@gitster.mtv.corp.google.com","subject":"Re: Should rerere auto-update a merge resolution?","fromName":"Martin Langhoff","fromEmail":"martin.langhoff@gmail.com","sentAt":"2017-08-23T21:12:04Z","receivedAt":"2017-08-23T21:12:30Z","isPatch":false,"sender":{"key":"martin.langhoff@gmail.com","avatar":"https://gravatar.com/avatar/1e3f311b6c4c15836501901ca58f8c0b0667246488084ba524d8bc9867e22fd9?d=mp&s=160"},"body":"On Wed, Aug 23, 2017 at 4:34 PM, Junio C Hamano wrote:\n> Between these two steps:\n>\n>> - I reset hard, retry the merge, using --no-commit, rerere applies what it knows\n>> - I fix things up, then commit\n>\n> You'd tell rerere to forget what it knows because it is wrong.\n\nHi Junio!\n\nthanks for the quick response.\n\nQuestions\n\n - when I tell it to forget, won't it forget the pre-resolution state?\nmy read of the rerere docs imply that it gets called during the merge\nto record the conflicted state.\n\n - would it be a feature if it updated its resolution db\nautomagically? rerere is plenty automagic already...\n\ncheers,\n\n\n\nm\n-- \n martin.langhoff@gmail.com\n - ask interesting questions ~ http://linkedin.com/in/martinlanghoff\n - don't be distracted ~ http://github.com/martin-langhoff\n by shiny stuff\n"},{"id":"327179","messageId":"alpine.DEB.2.21.1.1708251218460.7424@virtualbox","threadId":"46660","inReplyTo":"CACPiFCJCgKtTbKX8jCSC3QgMKZ7Usu2ojqXe5w_QAHwk7T4M-A@mail.gmail.com","subject":"Re: Should rerere auto-update a merge resolution?","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-08-25T10:19:56Z","receivedAt":"2017-08-25T10:20:08Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Martin,\n\nOn Wed, 23 Aug 2017, Martin Langhoff wrote:\n\n> On Wed, Aug 23, 2017 at 4:34 PM, Junio C Hamano wrote:\n> > Between these two steps:\n> >\n> >> - I reset hard, retry the merge, using --no-commit, rerere applies what it knows\n> >> - I fix things up, then commit\n> >\n> > You'd tell rerere to forget what it knows because it is wrong.\n> \n> Questions\n> \n> - when I tell it to forget, won't it forget the pre-resolution state?\n> my read of the rerere docs imply that it gets called during the merge\n> to record the conflicted state.\n\nIn my hands, I need to tell rerere to forget, *and then recreate the merge\nconflict* before I can resolve it again and let rerere learn the new\nresolution.\n\n> - would it be a feature if it updated its resolution db\n> automagically? rerere is plenty automagic already...\n\nThat would most likely be a very welcome feature here.\n\nThanks,\nJohannes\n"},{"id":"327187","messageId":"xmqqwp5ru17n.fsf@gitster.mtv.corp.google.com","threadId":"46660","inReplyTo":"CACPiFCJCgKtTbKX8jCSC3QgMKZ7Usu2ojqXe5w_QAHwk7T4M-A@mail.gmail.com","subject":"Re: Should rerere auto-update a merge resolution?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-25T15:16:44Z","receivedAt":"2017-08-25T15:16:53Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin Langhoff writes:\n\n> - when I tell it to forget, won't it forget the pre-resolution state?\n\nI do not recall the details of what I did ;-) so I played around a\nbit. Here is what I did:\n\n\tgit checkout master^0\n\tgit merge --no-ff --no-edit pb/trailers-from-command-line\n\tgit merge --no-ff --no-edit jk/trailers-parse\n\nI know that the last one I know gets conflict and triggers rerere\nto replay the recorded resolution. Imagine that I earlier botched\nresolution and the working tree contents is wrong or something at\nthis point.\n\n\tmake test ;# fails, perhaps\n\nSo I can do:\n\n\tgit rerere forget \n\nAfter git rerere forget, I observe (check subdirectories in\n.git/rr-cache/ whose timestamps are recent) that postimage gets\nremoved but preimage and thisimage stay.\n\nI can then edit that file, and say \"git rerere\" again, which is\ngreeted by \"Recorded resolution for ''\".\n\nI do not recall if I designed it that way or not, but it even seems\nto work correctly if you had \"git add -u\" after the botched auto\napplication (i.e. between the \"make test\" step and \"rerere forget\"\nstep in the above sequence). I never do \"add -u\" myself before\ntesting so I didn't notice.\n\nIf you want to _see_ the conflicts again while fixing a botched\nresolution, then you'd need to do a bit more involved recovery.\nAfter seeing \"make test\" fail and realize your older resolution is\nbotched, you'd probably do:\n\n\tgit checkout -m \n\nto unmerge, \"rerere forget \", fix the botched resolution and\nthen finally \"rerere\" to record the correction.\n\n"},{"id":"327188","messageId":"xmqqshgftyua.fsf@gitster.mtv.corp.google.com","threadId":"46660","inReplyTo":"xmqqwp5ru17n.fsf@gitster.mtv.corp.google.com","subject":"Re: Should rerere auto-update a merge resolution?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-25T16:07:57Z","receivedAt":"2017-08-25T16:08:05Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Martin Langhoff writes:\n>\n>> - when I tell it to forget, won't it forget the pre-resolution state?\n>\n> I do not recall the details of what I did ;-) so I played around a\n> bit. Here is what I did:\n> ...\n> After git rerere forget, I observe (check subdirectories in\n> .git/rr-cache/ whose timestamps are recent) that postimage gets\n> removed but preimage and thisimage stay.\n\nHaving said that, I suspect that it may be a bug if this procedure\nkept the original preimage. It should either remove it, or update\nit to record the state before the ealier resolution was applied\n(i.e. make the updated preimage identical to thisimage, so that a\ncorrected resolution can be taken from the working tree to pair with\nit).\n\nAs you may be able to see, \"rerere forget\" is much less used, hence\nmuch less exposed to end-user needs to gain both feature and\nusability polish than other parts of \"rerere\". I'd expect there\nwould be many cases where an extra cleverness can be used without\nmaking the result less robust, and it would be a good thing to see\nif we can improve them. We must avoid the cleverness leading us to\na \"works most of the time\" machinery whose operation cannot be\ntrusted, though.\n\nThe way the current machinery works is\n\n * \"rerere\" sees an unmerged index entry. It computes the conflict id\n to look into its database, finds a image pair, and\n does a three-way merge to update the working tree file (with\n conflict markers) with pre->post change (i.e. previous resolution).\n\nand that's it. We may be able to make \"updating the resolution\"\neasier, perhaps like so:\n\n * The end user edits working tree file further to correct the\n mistake in the earlier resolution that was applied.\n\n * The end user says \"rerere update \", which internally does\n the equivalent of \"checkout -m \" to unmerge and then\n computes the preimage for this round by redoing the original\n merge, write out the preimage and take the working tree file as\n the postimage\n\nWe might even be able to do a bit more in the \"normal application\"\ncodepath to make it record the result of the three-way merge,\nperhaps in an index extension. Then \"rerere update \" may not\neven be needed---at strategic places in the workflow (e.g. when the\nuser adds the path to the index), we can check if the contents is\ndifferent from what rerere wrote out as the resolution and if they\ndiffer, do the \"rerere update \" automatically.\n\nBut we may be taking the extra cleverness a bit too close to a\ndanger zone by trying to automate too much without thinking the\nramification through, so I'd stop speculation at this point.\n\n"},{"id":"327189","messageId":"xmqqo9r3tyl2.fsf@gitster.mtv.corp.google.com","threadId":"46660","inReplyTo":"alpine.DEB.2.21.1.1708251218460.7424@virtualbox","subject":"Re: Should rerere auto-update a merge resolution?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-25T16:13:29Z","receivedAt":"2017-08-25T16:13:39Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> In my hands, I need to tell rerere to forget, *and then recreate the merge\n> conflict* before I can resolve it again and let rerere learn the new\n> resolution.\n\nI can believe that---that is how I originally desiged \"forget\" to\nbehave.\n\nEven though My quick tests to prepare a reply to Martin seemed to\nshow that unmerging was not strictly needed in today's Git in order\nto forget and learn new, I wouldn't be surprised if an explicit\nunmerging in addition to \"rerere forget\" was necessary, especially\nif the original preimage were very different from the state before\nthe botched resolution was applied.\n\n"},{"id":"327195","messageId":"xmqqfucftw26.fsf@gitster.mtv.corp.google.com","threadId":"46660","inReplyTo":"xmqqshgftyua.fsf@gitster.mtv.corp.google.com","subject":"Re: Should rerere auto-update a merge resolution?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-25T17:08:01Z","receivedAt":"2017-08-25T17:08:08Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Junio C Hamano writes:\n>\n>> Martin Langhoff writes:\n>>\n>>> - when I tell it to forget, won't it forget the pre-resolution state?\n>>\n>> I do not recall the details of what I did ;-) so I played around a\n>> bit. Here is what I did:\n>> ...\n>> After git rerere forget, I observe (check subdirectories in\n>> .git/rr-cache/ whose timestamps are recent) that postimage gets\n>> removed but preimage and thisimage stay.\n>\n> Having said that, I suspect that it may be a bug if this procedure\n> kept the original preimage. It should either remove it, or update\n> it to record the state before the ealier resolution was applied\n> (i.e. make the updated preimage identical to thisimage, so that a\n> corrected resolution can be taken from the working tree to pair with\n> it).\n\nI just made a cursory scan of rerere.c again, and it seems we are\ndoing the right thing. The details are in rerere_forget_one_path()\nwhere we unlink postimage, we recreate the conflicted state from the\nstages in the index and update preimage.\n\nIt seems that code gives up if you already declared that you'd take\nthe previous resolution by adding the result to the index. It may\nprobably be a good idea to unmerge such a merged index entry instead\nof giving up. #leftoverbits\n\nSo, yes, it will forget both preimage and postimage, and it should\nupdate the preimage with the conflict you got during _this_ merge,\nso that the resolution you make _this_ time will become usable as\nthe corresponding postimage for the next time.\n\n\n\n"}]}