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

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Mar 12, 2018, 10:20 UTC
Message-ID: <nycvar.QRO.7.76.6.1803121056400.20700@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz>
In-Reply-To: <6362804d-e204-a9e0-9ff0-51d8497ce921@gmail.com>

Hi Buga,

On Sun, 11 Mar 2018, Igor Djordjevic wrote:

Show 14 quoted lines

> On 11/03/2018 16:47, Johannes Schindelin wrote:
> > 
> > > Having explained all this, I realized this is the same "essentially
> > > merging the new tips into the original pretending that the new tips
> > > were not rebased but merged into upstream" as Phillip`s one, just
> > > that we have additional temporary commits U1 and U2 (as per
> > > mentioned "patch theory") :)
> > 
> > But if the old tips had been merged into upstream (resulting in the
> > new tips), then the merge bases would be *the old tips*.
> 
> Exactly, and that is what part you`ve cut out of the quote was 
> showing :) By Phillip`s implementation, we would start with *old tips* 
> as merge bases, indeed (old tips being U1 and U2 in this case),

I really do not see how it would make sense to take the original merge commit as merge base in this scenario. It makes no *logical* sense: in which interpretation did you develop changes in two divergent directions from that merge commit?

Whereas if you use the old tips as merge bases, you can say very easily what those two directions were: one merged with other merge parents, the other direction rebased on top of upstream. There. Two divergent sets of changes that we want to reconcile ("merge"). Easy as apple pie.

Show 30 quoted lines

> where it further gets transformed as previously written:
> 
> > 	git merge-recursive U1 -- M U1'
> > 	tree="$(git write-tree)"
> > 	git merge-recursive U2 -- $tree U2'
> > 	tree="$(git write-tree)"
> > 
> > ..., where we know U1 = U2 = M (in regards to trees), so this is the 
> > same as:
> > 
> > 	git merge-recursive M -- M U1'
> > 	tree="$(git write-tree)"
> > 	git merge-recursive M -- $tree U2'
> > 	tree="$(git write-tree)"
> 
> Here, `git merge-recursive M -- M U1'` simply equals to U1' tree 
> (being a fast-forward merge), so we can write the two merges above as
> a single merge, too:
> 
> > 	git merge-recursive M -- U1' U2'
> > 	tree="$(git write-tree)"
> > 
> > ... which is exactly what Sergey`s (updated) approach suggests, 
> > merging U1' and U2' with M as merge-base (and shown inside that 
> > sample implementation script I provided[1]) :)
> 
> So from *old tips* being the rebased merge base (Phillip), we got to 
> *old merge commit* being the rebased merge base (Sergey), or vice 
> versa. Does this shed a bit more light on it now? Or you wanted to 
> point out something else in the first place...?

Okay, I'll trust you that these stunts show that the two strategies are equivalent as to what their results are.

The biggest difference is that it is easy for me to see the motivation behind Phillip's strategy, whereas I am still puzzled why one would come up with a complicated strategy that splits merge commits and re-merges them later, and why it should work in general (I still suspect that this is not the case).

Where "easy" meant that I had to spend 1h still to figure out why using the unrebased merge parents as merge bases. The same amount of time did not allow me to wrap my head around Sergey's verbose explanations.

But I'll take your word for it that the strategies are equivalent, and go with the one that has both a simpler explanation (in my mind, at least), and an more robust implementation.

Show 10 quoted lines

> > I am still not sure for what scenarios Phillip's strategy is the same as
> > Sergey's (updated) one, as the former strategy can do completely without
> > temporary commits [...]
> 
> I think the root of misunderstanding might be coming from the fact 
> that Sergey was mainly describing a general concept (without a 
> strictly defined implementation strategy, not being restricted to a 
> specific one), where Phillip came up with a solution that eventually 
> seems to use the same concept (as those transformations above should 
> show), but simplifying it further inside a concrete implementation.

Well, Sergey started off by suggesting the "rebase the patch relatively to the first parent always" strategy, then came up with a long-ish email describing a different approach (which I slowly realize is related to the first strategy, and it would have been *much* appreciated if it was not left to the reader to figure that one out), then incorporated what I called your hack (again, no clear and concise description what changed, just throwing a bunch of big bones to the dogs with the next long-ish document).

So I will not apologize for stopping to pay so much attention to that sub-thread at some point.

> By saying that Phillip "simplified it", even though transformations
> shown above might show different, I mean he managed to further decompose
> what Sergey was aiming for, abstracting temporary commits U1 and U2 out
> of the equation, thus making them optional, but not required.

That is not how I read Phillip's mail. It was more like "how about this instead". And it was simple enough, with clear example how to implement it, that I thought about that single mail for an hour, until I was satisfied that the motivation behind this strategy is sound.

Then you confirmed that it worked on your examples, and that's what settled the score here.

Show 6 quoted lines

> So, if easier to implement and reason about, I think what Phillip 
> described is a way to go to produce the rebased merged commit - but 
> in case we want to have that "clean rebased merge" check that U1' == 
> U2' comparison does provide, as they should really be the same (tree 
> wise) in simple (and the most used?) merge rebasing, we can do that 
> after the fact, even.

I do not know what that U1' == U2' check would buy us, as Phillip's strategy does not require rebasing the amendments *twice*.

> Might be this check could be the most useful in non-interactive 
> rebases, where rebased merge parents` trees could be expected to stay 
> "balanced" more often (U1' == U2'), without interactive fiddling to 
> "disbalance" them. I don`t know, just thinking out loud.

Again, Phillip's strategy does not leave things to be "disbalanced".

> > [...] and cannot introduce ambiguities when rebasing the
> > changes introduced by M (i.e. the "amendmendts" we talked about).
> 
> Hmm, not following here, which ambiguities are we talking about?

U1' vs U2' of course. Those are two things that can be different, even if they ideally would have identical trees.

Phillip's strategy does not leave that room for ambiguity.

Ciao, Dscho