From 1a92e53ba3614105b7a58a4820f4d33f3cf33a3a Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 23 Feb 2016 01:04:41 -0500 Subject: merge-one-file: use empty blob for add/add base When we see an add/add conflict on a file, we generate the conflicted content by doing a 3-way merge with a "virtual" base consisting of the common lines of the two sides. This strategy dates back to cb93c19 (merge-one-file: use common as base, instead of emptiness., 2005-11-09). Back then, the next step was to call rcs merge to generate the 3-way conflicts. Using the virtual base produced much better results, as rcs merge does not attempt to minimize the hunks. As a result, you'd get a conflict with the entirety of the files on either side. Since then, though, we've switched to using git-merge-file, which uses xdiff's "zealous" merge. This will find the minimal hunks even with just the simple, empty base. Let's switch to using that empty base. It's simpler, more efficient, and reduces our dependencies (we no longer need a working diff binary). It's also how the merge-recursive strategy handles this same case. We can almost get rid of git-sh-setup's create_virtual_base, but we don't here, for two reasons: 1. The functions in git-sh-setup are part of our public interface, so it's possible somebody is depending on it. We'd at least need to deprecate it first. 2. It's also used by mergetool's p4merge driver. It's unknown whether its 3-way merge is as capable as git's; if not, then it is benefiting from the function. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/git-merge-one-file.sh b/git-merge-one-file.sh index 07dfeb8..b8bab75 100755 --- a/git-merge-one-file.sh +++ b/git-merge-one-file.sh @@ -112,8 +112,7 @@ case "${1:-.}${2:-.}${3:-.}" in case "$1" in '') echo "Added $4 in both, but differently." - orig=$(git-unpack-file $2) - create_virtual_base "$orig" "$src2" + orig=$(git-unpack-file e69de29bb2d1d6434b8b29ae775ad8c2e48c5391) ;; *) echo "Auto-merging $4" -- cgit v0.10.2-6-g49f6 From b779b3a199d9d11e60e522288c84ece5d20dcc2c Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 23 Feb 2016 01:06:55 -0500 Subject: merge-tree: drop generate_common strategy When merge_blobs sees an add/add conflict, it tries to create a virtual base object for the 3-way merge that consists of the common lines of each file. It inherited this strategy from merge-one-file in 0c79938 (Improved three-way blob merging code, 2006-06-28), and the point is to minimize the size of the conflict hunks. That commit talks about "if libxdiff were to ever grow a compatible three-way merge, it could probably be directly plugged in". That has long since happened. So as with merge-one-file in the previous commit, this extra step is no longer necessary. Our 3-way merge code is smart enough to do the minimizing itself if we simply feed it an empty base, which is what the more modern merge-recursive strategy already does. Not only does this let us drop some code, but it removes an overflow bug in generate_common_file(). We allocate a buffer as large as the smallest of the two blobs, under the assumption that there cannot be more common content than what is in the smaller blob. However, xdiff may feed us more: if neither file ends in a newline, it feeds us the "\nNo newline at end of file" marker as common content, and we write it into the output. If the differences between the files are small than that string, we overflow the output buffer. This patch solves it by simply dropping the buggy code entirely. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/merge-blobs.c b/merge-blobs.c index 7abb894..4f68537 100644 --- a/merge-blobs.c +++ b/merge-blobs.c @@ -48,40 +48,6 @@ static void *three_way_filemerge(const char *path, mmfile_t *base, mmfile_t *our return res.ptr; } -static int common_outf(void *priv_, mmbuffer_t *mb, int nbuf) -{ - int i; - mmfile_t *dst = priv_; - - for (i = 0; i < nbuf; i++) { - memcpy(dst->ptr + dst->size, mb[i].ptr, mb[i].size); - dst->size += mb[i].size; - } - return 0; -} - -static int generate_common_file(mmfile_t *res, mmfile_t *f1, mmfile_t *f2) -{ - unsigned long size = f1->size < f2->size ? f1->size : f2->size; - void *ptr = xmalloc(size); - xpparam_t xpp; - xdemitconf_t xecfg; - xdemitcb_t ecb; - - memset(&xpp, 0, sizeof(xpp)); - xpp.flags = 0; - memset(&xecfg, 0, sizeof(xecfg)); - xecfg.ctxlen = 3; - xecfg.flags = XDL_EMIT_COMMON; - ecb.outf = common_outf; - - res->ptr = ptr; - res->size = 0; - - ecb.priv = res; - return xdi_diff(f1, f2, &xpp, &xecfg, &ecb); -} - void *merge_blobs(const char *path, struct blob *base, struct blob *our, struct blob *their, unsigned long *size) { void *res = NULL; @@ -112,8 +78,8 @@ void *merge_blobs(const char *path, struct blob *base, struct blob *our, struct if (fill_mmfile_blob(&common, base) < 0) goto out_free_f2_f1; } else { - if (generate_common_file(&common, &f1, &f2) < 0) - goto out_free_f2_f1; + common.ptr = xstrdup(""); + common.size = 0; } res = three_way_filemerge(path, &common, &f1, &f2, size); free_mmfile(&common); -- cgit v0.10.2-6-g49f6 From 907681e940d095a0ab96756a8cddfd4f63cd71e1 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 23 Feb 2016 01:07:25 -0500 Subject: xdiff: drop XDL_EMIT_COMMON There are no more callers that use this mode, and none likely to be added (as our xdl_merge() eliminates the common use of it for generating 3-way merge bases). This is effectively a revert of a9ed376 (xdiff: generate "anti-diffs" aka what is common to two files, 2006-06-28), though of course trying to revert that ancient commit directly produces many textual conflicts. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/xdiff/xdiff.h b/xdiff/xdiff.h index c033991..4fb7e79 100644 --- a/xdiff/xdiff.h +++ b/xdiff/xdiff.h @@ -42,7 +42,6 @@ extern "C" { #define XDF_IGNORE_BLANK_LINES (1 << 7) #define XDL_EMIT_FUNCNAMES (1 << 0) -#define XDL_EMIT_COMMON (1 << 1) #define XDL_EMIT_FUNCCONTEXT (1 << 2) #define XDL_MMB_READONLY (1 << 0) diff --git a/xdiff/xemit.c b/xdiff/xemit.c index 4266ada..993724b 100644 --- a/xdiff/xemit.c +++ b/xdiff/xemit.c @@ -120,21 +120,6 @@ static long def_ff(const char *rec, long len, char *buf, long sz, void *priv) return -1; } -static int xdl_emit_common(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb, - xdemitconf_t const *xecfg) { - xdfile_t *xdf = &xe->xdf2; - const char *rchg = xdf->rchg; - long ix; - - for (ix = 0; ix < xdf->nrec; ix++) { - if (rchg[ix]) - continue; - if (xdl_emit_record(xdf, ix, "", ecb)) - return -1; - } - return 0; -} - struct func_line { long len; char buf[80]; @@ -170,9 +155,6 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb, long funclineprev = -1; struct func_line func_line = { 0 }; - if (xecfg->flags & XDL_EMIT_COMMON) - return xdl_emit_common(xe, xscr, ecb, xecfg); - for (xch = xscr; xch; xch = xche->next) { xche = xdl_get_hunk(&xch, xecfg); if (!xch) -- cgit v0.10.2-6-g49f6