{"thread":{"id":"55431","subject":"git rebase --rebase-merges information loss (and other woes)","startedAt":"2021-04-03T10:42:31Z","lastAt":"2021-04-03T19:02:15Z","messageCount":4,"participants":["ydirson@free.fr","Sergey Organov"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"420928","messageId":"1054682599.520899173.1617446548600.JavaMail.root@zimbra39-e7","threadId":"55431","inReplyTo":"1874143044.520636715.1617442122946.JavaMail.root@zimbra39-e7","subject":"git rebase --rebase-merges information loss (and other woes)","fromName":"","fromEmail":"ydirson@free.fr","sentAt":"2021-04-03T10:42:28Z","receivedAt":"2021-04-03T10:42:31Z","isPatch":false,"sender":{"key":"ydirson@free.fr","avatar":null},"body":"I've been going through a couple of \"rebase -i -r\" lately, and would\nlike to share a couple of thoughts, starting with something looking\nlike a bug.\n\n1. when a merge has been done with \"-s ours\", rebase replays it without\nany special options, I proceed with the manual resolution, and if I just\n--continue, the rebase mechanism believes I want to drop the commit, which\ncould not be more wrong.  I can still be careful myself, and use \"git commit\n--allow-empty\" before --continue, but this feels awkward.\n\nIs there any compelling reason not record the merge here ?\n\n\n2. more generally, when a merge has been done with special options, it\nwould be a useful help in solving conflicts if rebase could use the same\noptions.  Maybe we could allow the rebase \"merge\" instruction to use more\nmerge options.  The user would still have to edit the instruction sheet\nmanually for those, however, and we could then want \"rebase -i\" to fill\nthem automatically, but that would seem to require recording the merge\noptions somewhere to start with - maybe in a note.\n\n\n3. while it's made clear that any conflict resolution and amendments\nhave to be redone, maybe we could provide some support for a common\nuse case, namely \"sink that commit/fixup down\".  The conflict\nresolution would then be like \"checkout $OLD && cherry-pick -n $FIXUP\".\n\nMaybe this could be activated by a merge option in rebase-interactive\ninstructions, like \"merge -C$OLD --fixup $F1 --fixup $F2\".\n\nWould that seem reasonable ?\n\n-- \nYann\n"},{"id":"420935","messageId":"87zgyfmpif.fsf@osv.gnss.ru","threadId":"55431","inReplyTo":"1054682599.520899173.1617446548600.JavaMail.root@zimbra39-e7","subject":"Re: git rebase --rebase-merges information loss (and other woes)","fromName":"Sergey Organov","fromEmail":"sorganov@gmail.com","sentAt":"2021-04-03T14:06:16Z","receivedAt":"2021-04-03T14:06:28Z","isPatch":false,"sender":{"key":"sorganov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8501568?v=4"},"body":"ydirson@free.fr writes:\n\n> I've been going through a couple of \"rebase -i -r\" lately, and would\n> like to share a couple of thoughts, starting with something looking\n> like a bug.\n\nI feel your pain and sympathize deeply.\n\n>\n> 1. when a merge has been done with \"-s ours\", rebase replays it without\n> any special options, I proceed with the manual resolution, and if I just\n> --continue, the rebase mechanism believes I want to drop the commit, which\n> could not be more wrong.  I can still be careful myself, and use \"git commit\n> --allow-empty\" before --continue, but this feels awkward.\n>\n> Is there any compelling reason not record the merge here ?\n\nThis looks like rather easy case to fix indeed. I mean empty commit\nissue, not the original cause of the problem.\n\n>\n> 2. more generally, when a merge has been done with special options, it\n> would be a useful help in solving conflicts if rebase could use the same\n> options.  Maybe we could allow the rebase \"merge\" instruction to use more\n> merge options.  The user would still have to edit the instruction sheet\n> manually for those, however, and we could then want \"rebase -i\" to fill\n> them automatically, but that would seem to require recording the merge\n> options somewhere to start with - maybe in a note.\n\nThat could help now an then, but doesn't solve the problem in general,\nas, first, the behavior of merge algorithms could change over time, and,\nsecond, the merge could have been performed with external merge\nalgorithm in the first place, including entirely manual merge, and after\nall, the person rebasing may have no idea at all how the original merge\nhas been achieved.\n\nRecording information about merges at merge time has similar problems to\nrecording information about renames, both being \"obvious\" solutions that\nin fact end-up being sub-optimal.\n\nFortunately, we still have the original merge handy, that Git simply\ndoesn't care to take into account, see below.\n\n>\n> 3. while it's made clear that any conflict resolution and amendments\n> have to be redone, maybe we could provide some support for a common\n> use case, namely \"sink that commit/fixup down\".  The conflict\n> resolution would then be like \"checkout $OLD && cherry-pick -n $FIXUP\".\n>\n> Maybe this could be activated by a merge option in rebase-interactive\n> instructions, like \"merge -C$OLD --fixup $F1 --fixup $F2\".\n>\n> Would that seem reasonable ?\n\nI still (as this has been already heavily discussed some time ago)\nbelieve that the most reasonable solution to all this is to rebase\nmerges rather than to throw them away. Redoing them, as Git does, is\nwrong choice in most cases as what it means is that Git, despite the\noption name --rebase-merges (and even better old name\n--preserve-merges), simply still throws away your precious merge\ncommits, only then it substitutes something potentially entirely\ndifferent for them, often silently.\n\nIn addition to the problems you've encountered, silent drop of user\ncontent is possible, and what's worse than that for a content preserving\ntool? As a result, to be on the safe side, with current approach to\nhandling merges during rebase, any non-trivial merge that is expected to\nbe rebased (and how would one be sure it never will?) is to be very\ncarefully performed in 2 commits: merge itself and fixups, otherwise\nchances are high fixups are silently lost during rebase.\n\nFurther, even this two-step approach doesn't solve all the problems. For\ninstance, issues with merges being originally performed with non-default\nalgorithm still remain (as in your case 1.) Moreover, if we notice that\ndefault (or any thereof) algorithm itself could change over time,\ninherent problems with the policy of recreating merge commits from\nscratch during rebase get even more obvious.\n\nOverall, to get this right, Git should finally refrain (at least by\ndefault) from generally hopeless attempts to re-create merges from\nscratch on rebase. Instead it should try to actually rebase existing\nmerges when user asks to preserve history shape. When and if automatic\nrebase fails, one of the options to resolve the issue, besides fixing\nrebase conflicts, is indeed to redo the merge, but then the user will be\nperfectly aware of particular re-merge, and will be responsible for the\nend result himself.\n\n-- Sergey\n"},{"id":"420940","messageId":"667025246.521774815.1617466172428.JavaMail.root@zimbra39-e7","threadId":"55431","inReplyTo":"87zgyfmpif.fsf@osv.gnss.ru","subject":"Re: git rebase --rebase-merges information loss (and other woes)","fromName":"","fromEmail":"ydirson@free.fr","sentAt":"2021-04-03T16:09:32Z","receivedAt":"2021-04-03T16:10:04Z","isPatch":false,"sender":{"key":"ydirson@free.fr","avatar":null},"body":"Hi Sergey,\n\n> > 1. when a merge has been done with \"-s ours\", rebase replays it\n> > without\n> > any special options, I proceed with the manual resolution, and if I\n> > just\n> > --continue, the rebase mechanism believes I want to drop the\n> > commit, which\n> > could not be more wrong.  I can still be careful myself, and use\n> > \"git commit\n> > --allow-empty\" before --continue, but this feels awkward.\n> >\n> > Is there any compelling reason not record the merge here ?\n> \n> This looks like rather easy case to fix indeed. I mean empty commit\n> issue, not the original cause of the problem.\n> \n> >\n> > 2. more generally, when a merge has been done with special options,\n> > it\n> > would be a useful help in solving conflicts if rebase could use the\n> > same\n> > options.  Maybe we could allow the rebase \"merge\" instruction to\n> > use more\n> > merge options.  The user would still have to edit the instruction\n> > sheet\n> > manually for those, however, and we could then want \"rebase -i\" to\n> > fill\n> > them automatically, but that would seem to require recording the\n> > merge\n> > options somewhere to start with - maybe in a note.\n> \n> That could help now an then, but doesn't solve the problem in\n> general,\n> as, first, the behavior of merge algorithms could change over time,\n> and,\n> second, the merge could have been performed with external merge\n> algorithm in the first place, including entirely manual merge, and\n> after\n> all, the person rebasing may have no idea at all how the original\n> merge\n> has been achieved.\n> \n> Recording information about merges at merge time has similar problems\n> to\n> recording information about renames, both being \"obvious\" solutions\n> that\n> in fact end-up being sub-optimal.\n> \n> Fortunately, we still have the original merge handy, that Git simply\n> doesn't care to take into account, see below.\n> \n> >\n> > 3. while it's made clear that any conflict resolution and\n> > amendments\n> > have to be redone, maybe we could provide some support for a common\n> > use case, namely \"sink that commit/fixup down\".  The conflict\n> > resolution would then be like \"checkout $OLD && cherry-pick -n\n> > $FIXUP\".\n> >\n> > Maybe this could be activated by a merge option in\n> > rebase-interactive\n> > instructions, like \"merge -C$OLD --fixup $F1 --fixup $F2\".\n> >\n> > Would that seem reasonable ?\n> \n> I still (as this has been already heavily discussed some time ago)\n> believe that the most reasonable solution to all this is to rebase\n> merges rather than to throw them away. Redoing them, as Git does, is\n> wrong choice in most cases as what it means is that Git, despite the\n> option name --rebase-merges (and even better old name\n> --preserve-merges), simply still throws away your precious merge\n> commits, only then it substitutes something potentially entirely\n> different for them, often silently.\n> \n> In addition to the problems you've encountered, silent drop of user\n> content is possible, and what's worse than that for a content\n> preserving\n> tool? As a result, to be on the safe side, with current approach to\n> handling merges during rebase, any non-trivial merge that is expected\n> to\n> be rebased (and how would one be sure it never will?) is to be very\n> carefully performed in 2 commits: merge itself and fixups, otherwise\n> chances are high fixups are silently lost during rebase.\n\nThis reminds me of the approach used in git-reintegrate: to get merges\nredone without loosing the fixup, it allows you to do exactly this, and\nthen is able to use rerere information to redo that often-incomplete part\nof conflict resolution.  Then it squashes the fixup commit in the merge,\nand is able to do that as long as the fixup commit is reachable.\n\nOne thing we could do given a merge commit, and provided that 1. we have\naccess to rerere cache, 2. a \"standard merge\" was done, and 3. the merge\nalgorithm did not change, we can pretty easily derive the two \"separate\ncommits\" (or arguably \"separate parts of the merge\").\n\nThat alone could maybe form the basis of the \"redo merge\" you're suggesting,\nand would already cover a good number of use-cases.\n\nFor the case where the rerere cache is not available any more, I saw\nwe have a contrib/rerere-train.sh script, although I never tried it, as I\nhad written mine at the time, though I felt it had left it had too many rough edges\nto share.  I'm attaching it for reference, as it also creates the separate fixup\ncommit (originally for use by git-reintegrate).\n\nIn fact, I wonder how much replaying merges created by other strategies would\nperform, if we simply try to apply this idea to them too.\n\n\nHowever, before I get too high on the idea, I have to say that in the rebase\nthat triggered this mail the rerere cache failed to get used in a couple of\nsituations: I did not care to check but I'd wager those were the commits in\nconflict with precisely the fixup I was bringing down below the merges.\nIn this case (quite lots of conflicts to re-resolve because of a one-line\nconflict, I felt so bad), the \"apply the one-line by hand and juste resolve\n*that* conflict\" approach was really effective - so maybe it makes sense to\nprovide the two options, which may be suitable for different situations.\n\n> \n> Further, even this two-step approach doesn't solve all the problems.\n> For\n> instance, issues with merges being originally performed with\n> non-default\n> algorithm still remain (as in your case 1.) Moreover, if we notice\n> that\n> default (or any thereof) algorithm itself could change over time,\n> inherent problems with the policy of recreating merge commits from\n> scratch during rebase get even more obvious.\n> \n> Overall, to get this right, Git should finally refrain (at least by\n> default) from generally hopeless attempts to re-create merges from\n> scratch on rebase. Instead it should try to actually rebase existing\n> merges when user asks to preserve history shape. When and if\n> automatic\n> rebase fails, one of the options to resolve the issue, besides fixing\n> rebase conflicts, is indeed to redo the merge, but then the user will\n> be\n> perfectly aware of particular re-merge, and will be responsible for\n> the\n> end result himself.\n> \n> -- Sergey\n> \n"},{"id":"420941","messageId":"87r1jrmbtk.fsf@osv.gnss.ru","threadId":"55431","inReplyTo":"667025246.521774815.1617466172428.JavaMail.root@zimbra39-e7","subject":"Re: git rebase --rebase-merges information loss (and other woes)","fromName":"Sergey Organov","fromEmail":"sorganov@gmail.com","sentAt":"2021-04-03T19:01:59Z","receivedAt":"2021-04-03T19:02:15Z","isPatch":false,"sender":{"key":"sorganov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8501568?v=4"},"body":"ydirson@free.fr writes:\n\n> Hi Sergey,\n\nHi Yann,\n\n\n[...]\n\n> This reminds me of the approach used in git-reintegrate: to get merges\n> redone without loosing the fixup, it allows you to do exactly this, and\n> then is able to use rerere information to redo that often-incomplete part\n> of conflict resolution.  Then it squashes the fixup commit in the merge,\n> and is able to do that as long as the fixup commit is reachable.\n>\n> One thing we could do given a merge commit, and provided that 1. we have\n> access to rerere cache, 2. a \"standard merge\" was done, and 3. the merge\n> algorithm did not change, we can pretty easily derive the two \"separate\n> commits\" (or arguably \"separate parts of the merge\").\n\nUnfortunately, rerere is unreliable as you may rebase in a different\nrepository in the first place. Fortunately, all the needed information\nis still there in the original merge commit though.\n\nI suggest you read the following. You will find that it exactly talks\nabout 2 separate parts of the merge, but does not need rerere to do the\njob:\n\nhttps://public-inbox.org/git/87r2oxe3o1.fsf@javad.com/\n\nTo give you even more background, here is a reference to \"Git Rev News\"\nthat discusses the issue:\n\nhttps://git.github.io/rev_news/2018/04/18/edition-38/#general\n\nUnfortunately I've turned to other issues and lost track of what current\nsituation is, but according to your question the cart remains there\nstill.\n\n>\n> That alone could maybe form the basis of the \"redo merge\" you're suggesting,\n> and would already cover a good number of use-cases.\n>\n> For the case where the rerere cache is not available any more, I saw\n> we have a contrib/rerere-train.sh script, although I never tried it,\n> as I had written mine at the time, though I felt it had left it had\n> too many rough edges to share. I'm attaching it for reference, as it\n> also creates the separate fixup commit (originally for use by\n> git-reintegrate).\n>\n> In fact, I wonder how much replaying merges created by other\n> strategies would perform, if we simply try to apply this idea to them\n> too.\n\nI deeply believe that Git should not care. You already have a merge\ncommit. What \"strategies\" or algorithm have been used to create that\ncommit should not matter for Git when it rebases *that commit*, the same\nway it doesn't care how exactly you've created a non-merge commit.\n\nI think that only after the basic rebasing is done right, some\nadditional niceties, such as guessing the strategy, could be\nimplemented, on top of fundamentally correct rebasing.\n\n>\n> However, before I get too high on the idea, I have to say that in the\n>rebase that triggered this mail the rerere cache failed to get used in\n>a couple of situations: I did not care to check but I'd wager those\n>were the commits in conflict with precisely the fixup I was bringing\n>down below the merges. In this case (quite lots of conflicts to\n>re-resolve because of a one-line conflict, I felt so bad), the \"apply\n>the one-line by hand and juste resolve *that* conflict\" approach was\n>really effective - so maybe it makes sense to provide the two options,\n>which may be suitable for different situations.\n\nI'd not rely on rerere for rebasing merges if at all possible, and it\n*is* possible, see the aforementioned reference. Options are definitely\ngood to have, but the default one must be the safest, that currently is\nnot the case at all.\n\nIf Git tried to actually rebase the merge, it could have happened there\nwould be no conflict, or else, but the worst situation currently is when\nGit silently replaces original merge with something rather different\nthat just happens to result in no textual conflicts.\n\n-- Sergey Organov\n"}]}