Re: [PATCH] submodule: Demonstrate known breakage during recursive merge

From: Heiko Voigt <hvoigt@hvoigt.net>
Date: Aug 24, 2011, 21:32 UTC
Message-ID: <20110824213205.GI45292@book.hvoigt.net>
In-Reply-To: <4E5561D2.2080206@kitware.com>

Hi,

On Wed, Aug 24, 2011 at 04:40:50PM -0400, Brad King wrote:

Show 17 quoted lines

> On 8/24/2011 4:27 PM, Heiko Voigt wrote:
>>>>      b---bc
>>>>     / \ /
>>>>    o   X
>>>>     \ / \
>>>>      c---cb
> [snip]
>> Supposing you merge bc into cb:
>> If I understand the situation correctly, the above is done first with
>> a := cb:sub, b := bc:sub, base := b:sub and then another time with
>> base := c:sub.
>
> When merging bc and cb there are two merge bases: b and c.  The recursive
> merge strategy first performs a "virtual" merge between b and c and uses
> the result as a fictional merge base between bc and cb.  Currently the
> submodule merge search runs during the "virtual" merge and gives advice.
> Then it later dies while trying to search during the "real" merge.

But what happens if this "virtual" merge of b and c does not succeed like in our case?

Show 21 quoted lines

> After applying my patch, try this:
>
>  $ cd t && ./t7405-submodule-merge.sh --verbose
>  ...
>  Merging:
>  8cbd0fb cb
>  virtual top-bc
>  found 2 common ancestor(s):
>  f6b4d5a b
>  4d9cfab c
>    Merging:
>    f6b4d5a b
>    4d9cfab c
>    found 1 common ancestor(s):
>    a2ff72f a
>  warning: Failed to merge submodule sub (multiple merges found)
>   806049692f8921101f2e7223852e3bd74f7187c8: > Merge branch 'sub-c' into sub-bc
>   db70dfacda48ce55365256a58eaf89b7da87cbe7: > Merge branch 'sub-b' into sub-cb
>    Auto-merging sub
>    CONFLICT (submodule): Merge conflict in sub
>  fatal: --ancestry-path given but there are no bottom commits

Yeah I have seen this and it looked to me as if those two commits are bc:sub and cb:sub since the commit message talks about c:sub merged into bc:sub and vice versa.

> One can see that the advice given talks about merging "b:sub" and "c:sub"
> and the suggested commits are actually "bc:sub" and "cb:sub".  This advice
> is not useful to someone mergeing bc and cb.

In this case we should only output the advice of the last merge of course. It will have the same result in this case though since the merge of bc:sub and cb:sub still has just these two candidates.

Cheers Heiko