Re: [PATCH 3/4] Allow specifying specialized merge-backend per path.
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:43:04
Junio C Hamano [off-list ref] writes: I hate to nitpick my own patches, but...
There is one caveat, though. ll_merge() is called for both internal ancestor merge and the outer "final" merge. I think an interactive custom per-path merge backend should refrain from going interactive when performing an internal merge (you can tell it by checking call_depth) and instead just call either ll_xdl_merge() if the content is text, or call ll_ours_merge() otherwise.
After having thought about this a bit more, I think a conflict
in *internal* merge for a binary file should probably take the
ancestor's, instead of ours.
A conflict during the internal merge happens when there are
multiple common ancestors, and the development lines continuing
from these ancestores may have disagreed on the result of this
conflicting merge earlier.
o---A---C---o---Y
/ \ / \
GP . M
\ / \ /
o---B---D---o---Z
When we are trying to do the merge M between Y and Z, we find
two closest common ancestors, A and B. An internal merge to
merge them, using the grandparent ancestor, GP, is attempted,
and we will use the result as the virtual common ancestor to
create a merge between Y and Z.
The fact the internal merge between A and B conflicted means
that the development lines leading to Y and Z already saw that
same conflict before reaching Y and Z (in the above picture, at
C and D, respectively), and they might have resolved it
differently.
When it is a text file, Fredrik's clever "recursive merge"
leaves conflicted merge as the common ancestor, which is
different from either C or D's resolution (perhaps we can think
of it being somewhere areound sign X in the above picture), and
merge between Y and Z using that as the common ancestor cancels
the conflict out if C and D resolved it the same way.
However, when the file is a binary guck, the result of such a
half-merge with conflict markers is simply a corrupt binary
data, and is unusable even for inspection. That was the reason
why I initially suggested to use 'ours'. But that means, when
we are on the branch that leads to Y, we essentially favor C's
resolution, defeating the "accidental revert avoidance" the
recursive merge strategy offers. If instead we take the version
from GP as the common ancestor, we have a usable binary guck as
an ancestor as well, and also we are being neutral between the
branches, giving the end user a chance to make an unbiased
decision on his own.
quoted hunk
diff --git a/merge-recursive.c b/merge-recursive.c index 4eb62cf..a6c08a1 100644 --- a/merge-recursive.c +++ b/merge-recursive.c@@ -659,6 +660,124 @@ static void fill_mm(const unsigned char *sha1, mmfile_t *mm) mm->size = size; } +/* Low-level merge functions */ +typedef int (*ll_merge_fn)(mmfile_t *orig, + mmfile_t *src1, const char *name1, + mmfile_t *src2, const char *name2, + mmbuffer_t *result); + +static int ll_xdl_merge(mmfile_t *orig, + mmfile_t *src1, const char *name1, + mmfile_t *src2, const char *name2, + mmbuffer_t *result) +{ + xpparam_t xpp; + memset(&xpp, 0, sizeof(xpp)); + memset(&xpp, 0, sizeof(xpp));
I caught this after sending this message, and already fixed it in the version I pushed out on 'pu'.