Re: [PATCH 09/10] Introduce get_octopus_merge_bases() in commit.c

[Junio C Hamano <gitster@pobox.com>]

Miklos Vajna <vmiklos@frugalware.org> writes:

> This is like get_merge_bases() but it works for multiple heads, like
> show-branch --merge-base.

In what sense is this "like show-branch --merge-base"?

The only similarlity I can spot is that it can take more than two heads,
but what it computes and the way it computes it seem to be different.  It
certainly looks much less efficient as it does not walk the ancestry chain
in one-go like show-branch does.

I haven't looked at [10/10] so I do not know yet how you intend to use
this value, but I am not sure if it makes sense to use the computed value
as the merge base when running an octopus.

When you want to merge A, B and C, because internally we always do
pairwise, you would first compute:

	T(A,B) == merge_3way(mb(A,B), A, B)

and then you would want to pick some merge base to merge that result with
C:

	T(A,B,C) == merge_3way(???, T(A,B), C)

I do not think using mb(mb(A,B),C) as ??? is necessary nor optimal.

          o---o---A
         /         .
    ----1---2---B...T(A,B)
             \       .
              o---C...T(A,B,C)

"1" above is mb(A,B) (i.e. the merge base between A and B) that you would
use when you merge A and B to compute an internal merge result T(A,B).
And "1" also is mb(mb(A,B),C).

But if you are doing 3-way merge to arrive at T(A,B,C) by merging T(A,B)
and C, don't you want to be using "2" as the merge base, not "1"?

The big comment on $MRC at the end of git-merge-octopus is about this
issue.  In earlier implementation, we used to use "1" to create T(A,B) and
then also used it to update the variable $MRC, which is used to compute
the merge base to use when the tentative tree T(A,B) is merged with the
other remote in the next round.  It was a mistake.

Ideally we should pick a better base between mb(A,C) and mb(B,C) (and if
we used mb(B,C) that's "2" and is a much better base than "1" when merging
T(A,B) and C).  Using mb(mb(A,B),C) means we are guaranteed to pick the
worst base for that last merge.