Re: [jgit-dev] Implement cherry-pick by merge

[Johannes Schindelin <Johannes.Schindelin@xxxxxx>]

Hi,

On Tue, 21 Sep 2010, Shawn O. Pearce wrote:

> Christian Halstrick <christian.halstrick@xxxxxxxxx> wrote:
> > On Tue, Sep 21, 2010 at 16:14, Shawn Pearce (Code Review)
> > <codereview-daemon@xxxxxxxxxxx> wrote:
> > > cherry-pick can be implemented using merge.  :-)
> > >
> > > Lets say we want to cherry-pick commit C, whose parent is B, and we 
> > > want to apply it on top of Q:
> > >
> > >  o----- B ----- C
> > >   \
> > >    o--o--o-- Q
> > >
> > > * Set the "ours" side to be Q.
> > > * Set the "theirs" side to be C.
> > > * Fix the merge base to be C^1, that is B.
> > >
> > > Where there is a B-C difference, it shows up as an edit made by 
> > > "them" and is applied by the merge as a "theirs" side edit.  Where 
> > > there is a B-Q difference, it shows up as an edit made by "us" and 
> > > shows up as an "ours" side edit.  Providing that these edits aren't 
> > > conflicting, the merge will complete cleanly.

IIRC I explained that at the "Alles wird Git" already.

Knowing myself, I would probably have put it that way: a cherry-pick is 
really a 3-way merge, as we want to integrate the changes between a commit 
and its parent with the difference between that parent commit and the 
current HEAD.

> > Great news. I thought the core of a cherry-pick would be some kind of 
> > apply-patch.
> 
> Well, it could also be done that way.  But then you run into some issues 
> when it comes to rename detection.
> 
> > I was already planning some extra ours in finding out how native git 
> > does cherry-pick.
> 
> It does what I describe above via merge.  I know, because I'm the
> one that made native git do that.

Oh...!

I am not sure I agree with Shawn on that one. Attribution is something I 
take very seriously.

48313592 9509af68

> > But seems to be that is not needed, right? Or are there cases where we 
> > don't reuse merge for cherry-pick?
> 
> There is no good reason not to use merge.

There is. It is slow. The intelligent thing would be to try something 
simple first, and that failing, try something more complicated, and only 
then resort to the ultimate solution.

It is not done that way in C Git (which Shawn calls "native"). That is why 
git-rebase is still _way_ faster than git-rebase -m.

> You want to use the merge stuff because eventually merge will support 
> rename detection, and you want to take advantage of that during 
> cherry-pick too.

True, rename detection is something you want to do. But you can still do 
that with format-patch -M | apply, which should be faster than a 
full-blown cherry-pick in almost all cases. By an order of magnitude.

> To be fair you could do a diff with rename detection and then apply
> the patch.  But you need to consider renames on both sides, that is
> there could be a rename between B and Q that you need to take into
> account when you are applying the delta between B and C on top of Q.
> Or there could be a rename between B and C that you need to take
> into account when applying onto Q.  Merge already has to handle
> both cases.  :-)

People need to stop considering merges as something automatic. Merges fail 
as often without merge conflicts as with. You still have to look carefully 
whether the end result is correct (I know that many people do not do that, 
but that is just plain naive).

In that sense, format-patch -M | apply is just as bad as cherry-pick, but 
by design faster.

> Also keep in mind revert is just the opposite of cherry-pick.
> Given the above diagram, the merge base is C and theirs is B.

Historically, it is the other way round. A cherry-pick is an inverse 
cherry-pick. This fact might seem irrelevant, but you'll understand the C 
implementation much better if you keep that fact in mind.

Only with Junio's addition to be able to revert merges (by picking the 
merge parent to revert to), cherry-picks and reverts are truly different.

Ciao,
Johannes