From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path:
X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on dcvr.yhbt.net
X-Spam-Level:
X-Spam-ASN: AS31976 209.132.180.0/23
X-Spam-Status: No, score=-2.6 required=3.0 tests=AWL,BAYES_00,
FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM,HEADER_FROM_DIFFERENT_DOMAINS,
RCVD_IN_DNSWL_HI,T_RP_MATCHES_RCVD shortcircuit=no autolearn=no
autolearn_force=no version=3.4.0
Received: from vger.kernel.org (vger.kernel.org [209.132.180.67])
by dcvr.yhbt.net (Postfix) with ESMTP id 278561F404
for ; Tue, 27 Feb 2018 23:27:30 +0000 (UTC)
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
id S1751876AbeB0X11 (ORCPT );
Tue, 27 Feb 2018 18:27:27 -0500
Received: from mout.gmx.net ([212.227.17.20]:59957 "EHLO mout.gmx.net"
rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
id S1751791AbeB0X10 (ORCPT );
Tue, 27 Feb 2018 18:27:26 -0500
Received: from [192.168.0.129] ([37.201.195.115]) by mail.gmx.com (mrgmx103
[212.227.17.168]) with ESMTPSA (Nemesis) id 0Lg1QN-1eOKax1yFn-00pZHG; Wed, 28
Feb 2018 00:27:20 +0100
Date: Wed, 28 Feb 2018 00:27:19 +0100 (STD)
From: Johannes Schindelin
X-X-Sender: virtualbox@MININT-6BKU6QN.europe.corp.microsoft.com
To: Igor Djordjevic
cc: Git mailing list ,
Jacob Keller ,
Sergey Organov ,
Johannes Sixt ,
Junio C Hamano
Subject: Re: [RFC] Rebasing merges: a jorney to the ultimate solution (Road
Clear)
In-Reply-To: <33da31e9-9101-475d-8901-4b6b3df2f29d@gmail.com>
Message-ID:
References: <87y3jtqdyg.fsf@javad.com>
<4d7f3406-b206-cc22-87df-85700d6a03d9@gmail.com> <33da31e9-9101-475d-8901-4b6b3df2f29d@gmail.com>
User-Agent: Alpine 2.21.1 (DEB 209 2017-03-23)
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="8323329-1047807922-1519774040=:56"
X-Provags-ID: V03:K0:+wyr9PuOLRBPD8spiopzUSF/+jaduUp68cnVZwbJJIK/dnGmgPy
537WfUb3Jn1FkCUfmonw7NrXXpVC0i5Vn2ztn6TF0ZME42X0wnlJIHM8w0lADywKCA+ZyNQ
YpWa/0OgNPqACcAK71opWEoUFUZ2KDSGZmeGhNlhcX6Bpxt4KS8b+GQ5ixXj3O+HTgYQoxJ
Xokif/gsP90N7Vggq9D2Q==
X-UI-Out-Filterresults: notjunk:1;V01:K0:FEvvq+n44L8=:DlvGCij+DPGsIenjrftz86
EafKW57jWfnEdQKmEVmVf5QKKUyCYK6qL0Ozg2hq4F4X2y98QBrM0eHESDgGjfE9AzPD647bv
MjRqIP0qxjWMhm9/V8Odf1Yw2K4WglkiBT6qnWamEFgJEjUq+pllOhvV3JR60oCmil+RRmjux
6l/gwIvONM4iUuZ/JdX29i09DG2NlDBofYHPoRKuVC9UUug2ZrQjUiQGWnolFG1bjj36t2QZV
EtvNtCXN9tE/tFZ1dr3znsOEaS2FmVPkAnqzMEoB1DzIHk72dV87cU0zE5pqPXjE5sHjwdQh0
7PgFcuhRhs9Cmzq44vyyG48LZpBbUe0RpP6ojNU03A/f6HwNY3ZkP10tda3gdDpAGN0DMtioW
8KbkIMiQgPWjSmGccFs232gqMggMOSdfHbezd5hyMXweeVIQ9XfW1ma921ijH7R5vCKGoduP6
ecU4EzHz7mAOoWGCHDYrWZwQa9gmEWAulkBvihlgw58H9Jp33B3WJ9cdmmMeqAxqHInOvqAFt
ESGJgNeK8o1Ntdm5j6LWkSrQxy18lCmYc2zw0CDrMl2OjwObD7DIU5NVC12gZRuf+4TS/x43c
3a8Tutto5oCYvxbAEnuIU8fn2z04LQBmpfgJTCYd3V0nI2LIFoQD2g349aVQXzccJBtaNoyr/
0uHOvvrO26htn2fDDGdgpdyET2pAy9W8jSG7pGLo5xyJczwxgiby/PXV2BtSJgQeUJA3W5OQy
PFFSnYpEolFsXwsa6j0nXdabfwralB/2MiAQAfjNGDC8luNjU8ZHJcfJ7fJ693DRCzJClLLbq
kK7CDlnH7pnNzLbGVLmJVE1YRphPzmVuyS79L5uPeWpy8pczbErp0Y2L3t6xxM0rjqjDgzr
Sender: git-owner@vger.kernel.org
Precedence: bulk
List-ID:
X-Mailing-List: git@vger.kernel.org
Archived-At:
List-Archive:
List-Post:
This message is in MIME format. The first part should be readable text,
while the remaining parts are likely unreadable without MIME-aware tools.
--8323329-1047807922-1519774040=:56
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: QUOTED-PRINTABLE
Hi Buga,
thank you for making this a lot more understandable to this thick
developer.
On Tue, 27 Feb 2018, Igor Djordjevic wrote:
> On 27/02/2018 19:55, Igor Djordjevic wrote:
> >=20
> > It would be more along the lines of "(1) rebase old merge commit parent=
s,=20
> > (2) generate separate diff between old merge commit and each of its=20
> > parents, (3) apply each diff to their corresponding newly rebased=20
> > parent respectively (as a temporary commit, one per rebased parent),=20
> > (4) merge these temporary commits to generate 'rebased' merge commit,=
=20
> > (5) drop temporary commits, recording their parents as parents of=20
> > 'rebased' merge commit (instead of dropped temporary commits)".
> >=20
> > Implementation wise, steps (2) and (3) could also be done by simply=20
> > copying old merge commit _snapshot_ on top of each of its parents as=20
> > a temporary, non-merge commit, then rebasing (cherry-picking) these=20
> > temporary commits on top of their rebased parent commits to produce=20
> > rebased temporary commits (to be merged for generating 'rebased'=20
> > merge commit in step (4)).
>=20
> For those still tagging along (and still confused), here are some=20
> diagrams (following what Sergey originally described). Note that=20
> actual implementation might be even simpler, but I believe it`s a bit=20
> easier to understand like this, using some "temporary" commits approach.
>=20
> Here`s our starting position:
>=20
> (0) ---X1---o---o---o---o---o---X2 (master)
> |\
> | A1---A2---A3
> | \
> | M (topic)
> | /
> \-B1---B2---B3
>=20
>=20
> Now, we want to rebase merge commit M from X1 onto X2. First, rebase
> merge commit parents as usual:
>=20
> (1) ---X1---o---o---o---o---o---X2
> |\ |\
> | A1---A2---A3 | A1'--A2'--A3'
> | \ |
> | M |
> | / |
> \-B1---B2---B3 \-B1'--B2'--B3'
>=20
>=20
> That was commonly understandable part.
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:
> Now, for "rebasing" the merge commit (keeping possible amendments), we
> do some extra work. First, we make two temporary commits on top of old
> merge parents, by using exact tree (snapshot) of commit M:
>=20
> (2) ---X1---o---o---o---o---o---X2
> |\ |\
> | A1---A2---A3---U1 | A1'--A2'--A3'
> | \ |
> | M |
> | / |
> \-B1---B2---B3---U2 \-B1'--B2'--B3'
Okay, everything would still be the same except that I still have dropped
A1'.
> So here, in terms of _snapshots_ (trees, not diffs), U1 =3D U2 =3D M.
>=20
> Now, we rebase these temporary commits, too:
>=20
> (3) ---X1---o---o---o---o---o---X2
> |\ |\
> | A1---A2---A3---U1 | A1'--A2'--A3'--U1'
> | \ |
> | M |
> | / |
> \-B1---B2---B3---U2 \-B1'--B2'--B3'--U2'
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.
> As a next step, we merge these temporary commits to produce our
> "rebased" merged commit M:
>=20
> (4) ---X1---o---o---o---o---o---X2
> |\ |\
> | A1---A2---A3---U1 | A1'--A2'--A3'--U1'
> | \ | \
> | M | M'
> | / | /
> \-B1---B2---B3---U2 \-B1'--B2'--B3'--U2'
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.
> Finally, we drop temporary commits, and record rebased commits A3'=20
> and B3' as our "rebased" merge commit parents instead (merge commit=20
> M' keeps its same tree/snapshot state, just gets parents replaced):
>=20
> (5) ---X1---o---o---o---o---o---X2
> |\ |\
> | A1---A2---A3---U1 | A1'--A2'--A3'
> | \ | \
> | M | M'
> | / | /
> \-B1---B2---B3---U2 \-B1'--B2'--B3'
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.
> And that`s it, our merge commit M has been "rebased" to M' :)
>=20
> (6) ---X1---o---o---o---o---o---X2 (master)
> |\
> | A1'--A2'--A3'
> | \
> | M' (topic)
> | /
> \-B1'--B2'--B3'
>=20
>=20
> Important thing to note here is that in our step (3) above, still in=20
> terms of trees/snapshots (not diffs), U1' could still be equal to=20
> U2', produced merge commit M' tree thus being equal to both of them=20
> as well (merge commit introducing no changes to either of its=20
> parents, originally described by Sergey as "angel merge").
>=20
> But it doesn`t have to be so - if any of the rebased commits A1 to A3=20
> or B1 to B3 was dropped or modified (or extra commits added, even),=20
> that would influence the trees (snapshots) produced after rebasing U1=20
> and U2 to U1' and U2', final merge M' reflecting all these changes as=20
> well, besides keeping original merge commit M amendments (preserving=20
> "evil merge").
Sadly, it would also introduce evil merges in certain circumstances, as I
demonstrated above.
> Well, that`s some theory, now to hopefully confirm/test/polish all=20
> this... or trash it, if flawed beyond correction :P
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=C2=B2 |
| / / |
\-B1---B2---B3-- \-B1'--B2'--B3'
Note that M=C2=B2 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=C2=B2:
---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=C2=B2. 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=C2=B3
| / | /
\-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
--8323329-1047807922-1519774040=:56--