Re: [RFC] Rebasing merges: a jorney to the ultimate solution (Road Clear)

Hi Buga,

thank you for making this a lot more understandable to this thick developer.

On Tue, 27 Feb 2018, Igor Djordjevic wrote:

Good. Let's assume that I want to do this interactively (because let's face it, rebase is boring unless we shake up things a little). And let's assume that A1 is my only change to the README, and that I realized that it was incorrect and I do not want the world to see it, so I drop A1'.

Let's see how things go from here:

Okay, everything would still be the same except that I still have dropped A1'.

I still want to drop A1 in this rebase, so A1' is still missing.

And now it starts to get interesting.

The diff between A3 and U1 does not touch the README, of course, as I said that only A1 changed the README. But the diff between B3 and U2 does change the README, thanks to M containing A1 change.

Therefore, the diff between B3' and U2' will also have this change to the README. That change that I wanted to drop.

And here, thanks to B3'..U2' changing the README, M' will also have that change that I wanted to see dropped.

Note that A1' is still dropped in my example.

Now, thanks to U2' being dropped (and A1' *still* being dropped), the change in the README that is still in M' is really only in M'. No other rebased commit has it. That makes it look as if M' introduced this change in addition to the changes that were merged between the merge parents.

This is what an "evil merge" is: it does more than just combine the previously diverging branches. It introduces another change that was not in any non-merge commits before it.

Sometimes, such an "evil merge" is necessary. For example, when master converted a couple more `hash` references to `oid` including, say, in a function signature, and the merged branch contains a new caller using that old function signature: in this case, the merge commit must convert those `hash` references to `oid` references, or the code won't compile.

In my example, where I dropped A1' specifically so that that embarrasingly incorrect change to the README would not be seen by the world, though, the evil merge would be truly evil: it would show said change to the world. The exact opposite of what I wanted.

Sadly, it would also introduce evil merges in certain circumstances, as I demonstrated above.

It would have been nice to have such a simple solution ;-)

So the most obvious way to try to fix this design would be to recreate the original merge first, even with merge conflicts, and then trying to use the diff between that and the actual original merge commit. In your example, this would look like that:

---X1---o---o---o---o---o---X2
   |\                       |\
   | A1---A2---A3--         | A1'--A2'--A3'
   |             \ \        |
   |              M M²      |
   |             / /        |
   \-B1---B2---B3--         \-B1'--B2'--B3'

Note that M² would be generated somewhat like this: `git checkout A3; git merge -s recursive B3; git add -u; git commit`. If there are merge conflicts, then the result would include the conflict markers.

Now we would generate something similar to those U1/U2 commits: a single-parent commit R that reflects the diff between M and M²:

---X1---o---o---o---o---o---X2
   |\                       |\
   | A1---A2---A3           | A1'--A2'--A3'
   |             \          |
   |              M---R     |
   |             /          |
   \-B1---B2---B3           \-B1'--B2'--B3'

Note that the tree of R is identical to the tree of M². We can now proceed to generate the merge between A3' and B3' (possibly with merge conflicts) and then reverting R on top:

---X1---o---o---o---o---o---X2
   |\                       |\
   | A1---A2---A3           | A1'--A2'--A3'
   |             \          |              \
   |              M---R     |               M'---M³
   |             /          |              /
   \-B1---B2---B3           \-B1'--B2'--B3'

Of course, as before, the idea would be to squash the reverted changes into the final merge commit.

Now, would this work?

I doubt it, for at least two reasons:

- if there are merge conflicts between A3/B3 and between A3'/B3', those
  merge conflicts will very likely look very different, and the conflicts
  when reverting R will contain those nested conflicts: utterly confusing.
  And those conflicts will look even more confusing if a patch (such as
  A1') was dropped during an interactive rebase.

- One of the promises was that the new way would also handle merge
  strategies other than recursive. What would happen, for example, if M
  was generated using `-s ours` (read: dropping the B* patches' changes)
  and if B1 had been cherry-picked into the history between X1..X2?

  Reverting R would obviously revert those B1 changes, even if B1' would
  obviously not even be part of the rebased history!

Yes, I agree that this `-s ours` example is quite concocted, but the point of this example is not how plausible it is, but how easy it is to come up with a scenario where this design to "rebase merge commits" results in very, very unwanted behavior.

But maybe I missed something obvious, and the design can still be fixed somehow?

Ciao, Johannes