threads / patch / 15280

patch, 12 partsTowards a better merge resolution support

Subject: [PATCH 00/12] Towards a better merge resolution support

## tl;dr

26 messages between Aug 30, 2008 and Sep 1, 2008. Diffs are folded; open one to read it.

replies: 25people: 5as markdown or json

Junio C Hamano· Aug 30, 2008, 00:42 UTC · lore

This consists of two loosely related topics on improving conflicted merge resolution support.

The early part of the series is what you already saw. In addition to recording a conflicted merge in the RCS merge style we have traditionally used, this allows you to optionally use "diff3 -m" style. The difference is that the latter format shows the part from the common ancestor that corresponds to the parts both sides modified to cause the conflict, in addition to the changes done on each side. This can be chosen by setting a configuration variable. Rerere mechanism is updated to understand this new format as well, and conflicts from either formats interoperate well, because rerere mechanism only records and uses the changes made on each side, not what was in the common ancestor.

The last four patches are to "git checkout" that checks things out of the index. When resolving conflicts, sometimes you would screw up the state of the working tree so badly that you would wish to redo the merge from the beginning for one file, without having to redo the whole merge. Some other times, you already know changes made on one side already solves everything the other side attempted to do, and would want to take the change from that side as a whole. New options supported by "git checkout" when checking out from the index for these purposes are:

 * git checkout -m -- path
   This recreates the merge using information staged in stages 1/2/3;
 
 * git checkout --ours -- path
   This writes 'our' version (stage #2) out for the path to the working
   tree;
 * git checkout --theirs -- path
   This writes 'their' version (stage #3) out for the path to the working
   tree;

None of these operations mark the path resolved. They are to help you prepare the working tree into a shape suitable as the resolution, and you will still conclude it with "git add path".

Junio C Hamano (12):
  xdl_fill_merge_buffer(): separate out a too deeply nested function
  xdiff-merge: optionally show conflicts in "diff3 -m" style
  xmerge.c: minimum readability fixups
  xmerge.c: "diff3 -m" style clips merge reduction level to EAGER or
    less
  rerere.c: use symbolic constants to keep track of parsing states
  rerere: understand "diff3 -m" style conflicts with the original
  merge.conflictstyle: choose between "merge" and "diff3 -m" styles
  git-merge-recursive: learn to honor merge.conflictstyle
  checkout: do not check out unmerged higher stages randomly
  checkout: allow ignoring unmerged paths when checking out of the
    index
  checkout --ours/--theirs
  checkout -m: recreate merge when checking out of unmerged index
 Documentation/config.txt  |    8 ++
 builtin-checkout.c        |  206 +++++++++++++++++++++++++++++++++++----
 builtin-merge-file.c      |   17 +++-
 builtin-merge-recursive.c |    2 +-
 ll-merge.c                |   16 +++-
 rerere.c                  |   29 ++++--
 t/t6023-merge-file.sh     |   44 +++++++++
 t/t7201-co.sh             |  133 +++++++++++++++++++++++++
 xdiff-interface.c         |   20 ++++
 xdiff-interface.h         |    2 +
 xdiff/xdiff.h             |    6 +
 xdiff/xmerge.c            |  237 +++++++++++++++++++++++++++++++--------------
 12 files changed, 612 insertions(+), 108 deletions(-)
Junio C Hamano· Aug 30, 2008, 00:42 UTC · re: Junio C Hamano · lore

[PATCH 01/12] xdl_fill_merge_buffer(): separate out a too deeply nested function

This simply moves code around to make a separate function that prepares a single conflicted hunk with markers into the buffer.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 xdiff/xmerge.c |  121 ++++++++++++++++++++++++++++++++-----------------------
 1 files changed, 70 insertions(+), 51 deletions(-)
Show changes to xdiff/xmerge.c +70 −51
diff --git a/xdiff/xmerge.c b/xdiff/xmerge.c
index 82b3573..6ffaa4f 100644
--- a/xdiff/xmerge.c
+++ b/xdiff/xmerge.c
@@ -113,65 +113,84 @@ static int xdl_recs_copy(xdfenv_t *xe, int i, int count, int add_nl, char *dest)
 	return size;
 }
 
-static int xdl_fill_merge_buffer(xdfenv_t *xe1, const char *name1,
-		xdfenv_t *xe2, const char *name2, xdmerge_t *m, char *dest)
+static int fill_conflict_hunk(xdfenv_t *xe1, const char *name1,
+			      xdfenv_t *xe2, const char *name2,
+			      int size, int i,
+			      xdmerge_t *m, char *dest)
 {
 	const int marker_size = 7;
 	int marker1_size = (name1 ? strlen(name1) + 1 : 0);
 	int marker2_size = (name2 ? strlen(name2) + 1 : 0);
-	int conflict_marker_size = 3 * (marker_size + 1)
-		+ marker1_size + marker2_size;
-	int size, i1, j;
-
-	for (size = i1 = 0; m; m = m->next) {
-		if (m->mode == 0) {
-			size += xdl_recs_copy(xe1, i1, m->i1 - i1, 0,
-					dest ? dest + size : NULL);
-			if (dest) {
-				for (j = 0; j < marker_size; j++)
-					dest[size++] = '<';
-				if (marker1_size) {
-					dest[size] = ' ';
-					memcpy(dest + size + 1, name1,
-							marker1_size - 1);
-					size += marker1_size;
-				}
-				dest[size++] = '\n';
-			} else
-				size += conflict_marker_size;
-			size += xdl_recs_copy(xe1, m->i1, m->chg1, 1,
-					dest ? dest + size : NULL);
-			if (dest) {
-				for (j = 0; j < marker_size; j++)
-					dest[size++] = '=';
-				dest[size++] = '\n';
-			}
-			size += xdl_recs_copy(xe2, m->i2, m->chg2, 1,
-					dest ? dest + size : NULL);
-			if (dest) {
-				for (j = 0; j < marker_size; j++)
-					dest[size++] = '>';
-				if (marker2_size) {
-					dest[size] = ' ';
-					memcpy(dest + size + 1, name2,
-							marker2_size - 1);
-					size += marker2_size;
-				}
-				dest[size++] = '\n';
-			}
-		} else if (m->mode == 1)
-			size += xdl_recs_copy(xe1, i1, m->i1 + m->chg1 - i1, 0,
-					dest ? dest + size : NULL);
+	int j;
+
+	/* Before conflicting part */
+	size += xdl_recs_copy(xe1, i, m->i1 - i, 0,
+			      dest ? dest + size : NULL);
+
+	if (!dest) {
+		size += marker_size + 1 + marker1_size;
+	} else {
+		for (j = 0; j < marker_size; j++)
+			dest[size++] = '<';
+		if (marker1_size) {
+			dest[size] = ' ';
+			memcpy(dest + size + 1, name1, marker1_size - 1);
+			size += marker1_size;
+		}
+		dest[size++] = '\n';
+	}
+
+	/* Postimage from side #1 */
+	size += xdl_recs_copy(xe1, m->i1, m->chg1, 1,
+			      dest ? dest + size : NULL);
+	if (!dest) {
+		size += marker_size + 1;
+	} else {
+		for (j = 0; j < marker_size; j++)
+			dest[size++] = '=';
+		dest[size++] = '\n';
+	}
+
+	/* Postimage from side #2 */
+	size += xdl_recs_copy(xe2, m->i2, m->chg2, 1,
+			      dest ? dest + size : NULL);
+	if (!dest) {
+		size += marker_size + 1 + marker2_size;
+	} else {
+		for (j = 0; j < marker_size; j++)
+			dest[size++] = '>';
+		if (marker2_size) {
+			dest[size] = ' ';
+			memcpy(dest + size + 1, name2, marker2_size - 1);
+			size += marker2_size;
+		}
+		dest[size++] = '\n';
+	}
+	return size;
+}
+
+static int xdl_fill_merge_buffer(xdfenv_t *xe1, const char *name1,
+		xdfenv_t *xe2, const char *name2, xdmerge_t *m, char *dest)
+{
+	int size, i;
+
+	for (size = i = 0; m; m = m->next) {
+		if (m->mode == 0)
+			size = fill_conflict_hunk(xe1, name1, xe2, name2,
+						  size, i, m, dest);
+		else if (m->mode == 1)
+			size += xdl_recs_copy(xe1, i, m->i1 + m->chg1 - i, 0,
+					      dest ? dest + size : NULL);
 		else if (m->mode == 2)
-			size += xdl_recs_copy(xe2, m->i2 - m->i1 + i1,
-					m->i1 + m->chg2 - i1, 0,
-					dest ? dest + size : NULL);
+			size += xdl_recs_copy(xe2, m->i2 - m->i1 + i,
+					      m->i1 + m->chg2 - i, 0,
+					      dest ? dest + size : NULL);
 		else
 			continue;
-		i1 = m->i1 + m->chg1;
+		i = m->i1 + m->chg1;
 	}
-	size += xdl_recs_copy(xe1, i1, xe1->xdf2.nrec - i1, 0,
-			dest ? dest + size : NULL);
+	size += xdl_recs_copy(xe1, i, xe1->xdf2.nrec - i, 0,
+			      dest ? dest + size : NULL);
 	return size;
 }
 
-- 
1.6.0.1.149.ga4c44
Junio C Hamano· Aug 30, 2008, 00:42 UTC · re: Junio C Hamano · lore

[PATCH 02/12] xdiff-merge: optionally show conflicts in "diff3 -m" style

When showing conflicting merges, we traditionally followed RCS's merge output format. The output shows:

 <<<<<<<
 postimage from one side;
 =======
 postimage of the other side; and
 >>>>>>>

Some poeple find it easier to be able to understand what is going on when they can view the common ancestor's version, which is used by "diff3 -m", which shows:

 <<<<<<<
 postimage from one side;
 |||||||
 shared preimage;
 =======
 postimage of the other side; and
 >>>>>>>

This is an initial step to bring that as an optional feature to git. Only "git merge-file" has been converted, with "--diff3" option.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-merge-file.c  |   10 ++++-
 t/t6023-merge-file.sh |   37 +++++++++++++++++
 xdiff/xdiff.h         |    6 +++
 xdiff/xmerge.c        |  103 ++++++++++++++++++++++++++++++++++++++++---------
 4 files changed, 135 insertions(+), 21 deletions(-)
Show changes to 4 files +135 −21

builtin-merge-file.c, t/t6023-merge-file.sh, xdiff/xdiff.h, xdiff/xmerge.c

diff --git a/builtin-merge-file.c b/builtin-merge-file.c
index 3605960..5b4f020 100644
--- a/builtin-merge-file.c
+++ b/builtin-merge-file.c
@@ -4,7 +4,7 @@
 #include "xdiff-interface.h"
 
 static const char merge_file_usage[] =
-"git merge-file [-p | --stdout] [-q | --quiet] [-L name1 [-L orig [-L name2]]] file1 orig_file file2";
+"git merge-file [-p | --stdout] [--diff3] [-q | --quiet] [-L name1 [-L orig [-L name2]]] file1 orig_file file2";
 
 int cmd_merge_file(int argc, const char **argv, const char *prefix)
 {
@@ -13,6 +13,8 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
 	mmbuffer_t result = {NULL, 0};
 	xpparam_t xpp = {XDF_NEED_MINIMAL};
 	int ret = 0, i = 0, to_stdout = 0;
+	int merge_level = XDL_MERGE_ZEALOUS_ALNUM;
+	int merge_style = 0;
 
 	while (argc > 4) {
 		if (!strcmp(argv[1], "-L") && i < 3) {
@@ -25,6 +27,10 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
 		else if (!strcmp(argv[1], "-q") ||
 				!strcmp(argv[1], "--quiet"))
 			freopen("/dev/null", "w", stderr);
+		else if (!strcmp(argv[1], "--diff3")) {
+			merge_style = XDL_MERGE_DIFF3;
+			merge_level = XDL_MERGE_EAGER;
+		}
 		else
 			usage(merge_file_usage);
 		argc--;
@@ -46,7 +52,7 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
 	}
 
 	ret = xdl_merge(mmfs + 1, mmfs + 0, names[0], mmfs + 2, names[2],
-			&xpp, XDL_MERGE_ZEALOUS_ALNUM, &result);
+			&xpp, merge_level | merge_style, &result);
 
 	for (i = 0; i < 3; i++)
 		free(mmfs[i].ptr);
diff --git a/t/t6023-merge-file.sh b/t/t6023-merge-file.sh
index 42620e0..f3484e3 100755
--- a/t/t6023-merge-file.sh
+++ b/t/t6023-merge-file.sh
@@ -161,4 +161,41 @@ test_expect_success 'ZEALOUS_ALNUM' '
 
 '
 
+cat >expect <<\EOF
+Dominus regit me,
+<<<<<<< new8.txt
+et nihil mihi deerit;
+
+
+
+
+In loco pascuae ibi me collocavit;
+super aquam refectionis educavit me.
+|||||||
+et nihil mihi deerit.
+In loco pascuae ibi me collocavit,
+super aquam refectionis educavit me;
+=======
+et nihil mihi deerit,
+
+
+
+
+In loco pascuae ibi me collocavit --
+super aquam refectionis educavit me,
+>>>>>>> new9.txt
+animam meam convertit,
+deduxit me super semitas jusitiae,
+propter nomen suum.
+Nam et si ambulavero in medio umbrae mortis,
+non timebo mala, quoniam TU mecum es:
+virga tua et baculus tuus ipsa me consolata sunt.
+EOF
+
+test_expect_success '"diff3 -m" style output' '
+	test_must_fail git merge-file -p --diff3 \
+		new8.txt new5.txt new9.txt >actual &&
+	test_cmp expect actual
+'
+
 test_done
diff --git a/xdiff/xdiff.h b/xdiff/xdiff.h
index 413082e..deebe02 100644
--- a/xdiff/xdiff.h
+++ b/xdiff/xdiff.h
@@ -50,10 +50,16 @@ extern "C" {
 #define XDL_BDOP_CPY 2
 #define XDL_BDOP_INSB 3
 
+/* merge simplification levels */
 #define XDL_MERGE_MINIMAL 0
 #define XDL_MERGE_EAGER 1
 #define XDL_MERGE_ZEALOUS 2
 #define XDL_MERGE_ZEALOUS_ALNUM 3
+#define XDL_MERGE_LEVEL_MASK 0x0f
+
+/* merge output styles */
+#define XDL_MERGE_DIFF3 0x8000
+#define XDL_MERGE_STYLE_MASK 0x8000
 
 typedef struct s_mmfile {
 	char *ptr;
diff --git a/xdiff/xmerge.c b/xdiff/xmerge.c
index 6ffaa4f..29cdbea 100644
--- a/xdiff/xmerge.c
+++ b/xdiff/xmerge.c
@@ -30,17 +30,32 @@ typedef struct s_xdmerge {
 	 * 2 = no conflict, take second.
 	 */
 	int mode;
+	/*
+	 * These point at the respective postimages.  E.g. <i1,chg1> is
+	 * how side #1 wants to change the common ancestor; if there is no
+	 * overlap, lines before i1 in the postimage of side #1 appear
+	 * in the merge result as a region touched by neither side.
+	 */
 	long i1, i2;
 	long chg1, chg2;
+	/*
+	 * These point at the preimage; of course there is just one
+	 * preimage, that is from the shared common ancestor.
+	 */
+	long i0;
+	long chg0;
 } xdmerge_t;
 
 static int xdl_append_merge(xdmerge_t **merge, int mode,
-		long i1, long chg1, long i2, long chg2)
+			    long i0, long chg0,
+			    long i1, long chg1,
+			    long i2, long chg2)
 {
 	xdmerge_t *m = *merge;
 	if (m && (i1 <= m->i1 + m->chg1 || i2 <= m->i2 + m->chg2)) {
 		if (mode != m->mode)
 			m->mode = 0;
+		m->chg0 = i0 + chg0 - m->i0;
 		m->chg1 = i1 + chg1 - m->i1;
 		m->chg2 = i2 + chg2 - m->i2;
 	} else {
@@ -49,6 +64,8 @@ static int xdl_append_merge(xdmerge_t **merge, int mode,
 			return -1;
 		m->next = NULL;
 		m->mode = mode;
+		m->i0 = i0;
+		m->chg0 = chg0;
 		m->i1 = i1;
 		m->chg1 = chg1;
 		m->i2 = i2;
@@ -91,11 +108,13 @@ static int xdl_merge_cmp_lines(xdfenv_t *xe1, int i1, xdfenv_t *xe2, int i2,
 	return 0;
 }
 
-static int xdl_recs_copy(xdfenv_t *xe, int i, int count, int add_nl, char *dest)
+static int xdl_recs_copy_0(int use_orig, xdfenv_t *xe, int i, int count, int add_nl, char *dest)
 {
-	xrecord_t **recs = xe->xdf2.recs + i;
+	xrecord_t **recs;
 	int size = 0;
 
+	recs = (use_orig ? xe->xdf1.recs : xe->xdf2.recs) + i;
+
 	if (count < 1)
 		return 0;
 
@@ -113,9 +132,19 @@ static int xdl_recs_copy(xdfenv_t *xe, int i, int count, int add_nl, char *dest)
 	return size;
 }
 
+static int xdl_recs_copy(xdfenv_t *xe, int i, int count, int add_nl, char *dest)
+{
+	return xdl_recs_copy_0(0, xe, i, count, add_nl, dest);
+}
+
+static int xdl_orig_copy(xdfenv_t *xe, int i, int count, int add_nl, char *dest)
+{
+	return xdl_recs_copy_0(1, xe, i, count, add_nl, dest);
+}
+
 static int fill_conflict_hunk(xdfenv_t *xe1, const char *name1,
 			      xdfenv_t *xe2, const char *name2,
-			      int size, int i,
+			      int size, int i, int style,
 			      xdmerge_t *m, char *dest)
 {
 	const int marker_size = 7;
@@ -143,6 +172,20 @@ static int fill_conflict_hunk(xdfenv_t *xe1, const char *name1,
 	/* Postimage from side #1 */
 	size += xdl_recs_copy(xe1, m->i1, m->chg1, 1,
 			      dest ? dest + size : NULL);
+
+	if (style == XDL_MERGE_DIFF3) {
+		/* Shared preimage */
+		if (!dest) {
+			size += marker_size + 1;
+		} else {
+			for (j = 0; j < marker_size; j++)
+				dest[size++] = '|';
+			dest[size++] = '\n';
+		}
+		size += xdl_orig_copy(xe1, m->i0, m->chg0, 1,
+				      dest ? dest + size : NULL);
+	}
+
 	if (!dest) {
 		size += marker_size + 1;
 	} else {
@@ -170,14 +213,15 @@ static int fill_conflict_hunk(xdfenv_t *xe1, const char *name1,
 }
 
 static int xdl_fill_merge_buffer(xdfenv_t *xe1, const char *name1,
-		xdfenv_t *xe2, const char *name2, xdmerge_t *m, char *dest)
+				 xdfenv_t *xe2, const char *name2,
+				 xdmerge_t *m, char *dest, int style)
 {
 	int size, i;
 
 	for (size = i = 0; m; m = m->next) {
 		if (m->mode == 0)
 			size = fill_conflict_hunk(xe1, name1, xe2, name2,
-						  size, i, m, dest);
+						  size, i, style, m, dest);
 		else if (m->mode == 1)
 			size += xdl_recs_copy(xe1, i, m->i1 + m->chg1 - i, 0,
 					      dest ? dest + size : NULL);
@@ -342,9 +386,11 @@ static int xdl_simplify_non_conflicts(xdfenv_t *xe1, xdmerge_t *m,
  */
 static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1,
 		xdfenv_t *xe2, xdchange_t *xscr2, const char *name2,
-		int level, xpparam_t const *xpp, mmbuffer_t *result) {
+		int flags, xpparam_t const *xpp, mmbuffer_t *result) {
 	xdmerge_t *changes, *c;
-	int i1, i2, chg1, chg2;
+	int i0, i1, i2, chg0, chg1, chg2;
+	int level = flags & XDL_MERGE_LEVEL_MASK;
+	int style = flags & XDL_MERGE_STYLE_MASK;
 
 	c = changes = NULL;
 
@@ -352,11 +398,14 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1,
 		if (!changes)
 			changes = c;
 		if (xscr1->i1 + xscr1->chg1 < xscr2->i1) {
+			i0 = xscr1->i1;
 			i1 = xscr1->i2;
 			i2 = xscr2->i2 - xscr2->i1 + xscr1->i1;
+			chg0 = xscr1->chg1;
 			chg1 = xscr1->chg2;
 			chg2 = xscr1->chg1;
-			if (xdl_append_merge(&c, 1, i1, chg1, i2, chg2)) {
+			if (xdl_append_merge(&c, 1,
+					     i0, chg0, i1, chg1, i2, chg2)) {
 				xdl_cleanup_merge(changes);
 				return -1;
 			}
@@ -364,11 +413,14 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1,
 			continue;
 		}
 		if (xscr2->i1 + xscr2->chg1 < xscr1->i1) {
+			i0 = xscr2->i1;
 			i1 = xscr1->i2 - xscr1->i1 + xscr2->i1;
 			i2 = xscr2->i2;
+			chg0 = xscr2->chg1;
 			chg1 = xscr2->chg1;
 			chg2 = xscr2->chg2;
-			if (xdl_append_merge(&c, 2, i1, chg1, i2, chg2)) {
+			if (xdl_append_merge(&c, 2,
+					     i0, chg0, i1, chg1, i2, chg2)) {
 				xdl_cleanup_merge(changes);
 				return -1;
 			}
@@ -385,19 +437,26 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1,
 			int off = xscr1->i1 - xscr2->i1;
 			int ffo = off + xscr1->chg1 - xscr2->chg1;
 
+			i0 = xscr1->i1;
 			i1 = xscr1->i2;
 			i2 = xscr2->i2;
-			if (off > 0)
+			if (off > 0) {
+				i0 -= off;
 				i1 -= off;
+			}
 			else
 				i2 += off;
+			chg0 = xscr1->i1 + xscr1->chg1 - i0;
 			chg1 = xscr1->i2 + xscr1->chg2 - i1;
 			chg2 = xscr2->i2 + xscr2->chg2 - i2;
 			if (ffo > 0)
 				chg2 += ffo;
-			else
+			else {
+				chg0 -= ffo;
 				chg1 -= ffo;
-			if (xdl_append_merge(&c, 0, i1, chg1, i2, chg2)) {
+			}
+			if (xdl_append_merge(&c, 0,
+					     i0, chg0, i1, chg1, i2, chg2)) {
 				xdl_cleanup_merge(changes);
 				return -1;
 			}
@@ -414,11 +473,14 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1,
 	while (xscr1) {
 		if (!changes)
 			changes = c;
+		i0 = xscr1->i1;
 		i1 = xscr1->i2;
 		i2 = xscr1->i1 + xe2->xdf2.nrec - xe2->xdf1.nrec;
+		chg0 = xscr1->chg1;
 		chg1 = xscr1->chg2;
 		chg2 = xscr1->chg1;
-		if (xdl_append_merge(&c, 1, i1, chg1, i2, chg2)) {
+		if (xdl_append_merge(&c, 1,
+				     i0, chg0, i1, chg1, i2, chg2)) {
 			xdl_cleanup_merge(changes);
 			return -1;
 		}
@@ -427,11 +489,14 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1,
 	while (xscr2) {
 		if (!changes)
 			changes = c;
+		i0 = xscr2->i1;
 		i1 = xscr2->i1 + xe1->xdf2.nrec - xe1->xdf1.nrec;
 		i2 = xscr2->i2;
+		chg0 = xscr2->chg1;
 		chg1 = xscr2->chg1;
 		chg2 = xscr2->chg2;
-		if (xdl_append_merge(&c, 2, i1, chg1, i2, chg2)) {
+		if (xdl_append_merge(&c, 2,
+				     i0, chg0, i1, chg1, i2, chg2)) {
 			xdl_cleanup_merge(changes);
 			return -1;
 		}
@@ -449,7 +514,7 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1,
 	/* output */
 	if (result) {
 		int size = xdl_fill_merge_buffer(xe1, name1, xe2, name2,
-			changes, NULL);
+			changes, NULL, style);
 		result->ptr = xdl_malloc(size);
 		if (!result->ptr) {
 			xdl_cleanup_merge(changes);
@@ -457,14 +522,14 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1,
 		}
 		result->size = size;
 		xdl_fill_merge_buffer(xe1, name1, xe2, name2, changes,
-				result->ptr);
+				      result->ptr, style);
 	}
 	return xdl_cleanup_merge(changes);
 }
 
 int xdl_merge(mmfile_t *orig, mmfile_t *mf1, const char *name1,
 		mmfile_t *mf2, const char *name2,
-		xpparam_t const *xpp, int level, mmbuffer_t *result) {
+		xpparam_t const *xpp, int flags, mmbuffer_t *result) {
 	xdchange_t *xscr1, *xscr2;
 	xdfenv_t xe1, xe2;
 	int status;
@@ -501,7 +566,7 @@ int xdl_merge(mmfile_t *orig, mmfile_t *mf1, const char *name1,
 		} else {
 			status = xdl_do_merge(&xe1, xscr1, name1,
 					      &xe2, xscr2, name2,
-					      level, xpp, result);
+					      flags, xpp, result);
 		}
 		xdl_free_script(xscr1);
 		xdl_free_script(xscr2);
-- 
1.6.0.1.149.ga4c44
Junio C Hamano· Aug 30, 2008, 00:42 UTC · re: Junio C Hamano · lore

[PATCH 03/12] xmerge.c: minimum readability fixups

This replaces hardcoded magic constants with symbolic ones for readability, and swaps one if/else blocks to better match the order in which 0/1/2 variables are handled to nearby codepath.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 xdiff/xmerge.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)
Show changes to xdiff/xmerge.c +7 −7
diff --git a/xdiff/xmerge.c b/xdiff/xmerge.c
index 29cdbea..7dcd405 100644
--- a/xdiff/xmerge.c
+++ b/xdiff/xmerge.c
@@ -427,7 +427,7 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1,
 			xscr2 = xscr2->next;
 			continue;
 		}
-		if (level < 1 || xscr1->i1 != xscr2->i1 ||
+		if (level == XDL_MERGE_MINIMAL || xscr1->i1 != xscr2->i1 ||
 				xscr1->chg1 != xscr2->chg1 ||
 				xscr1->chg2 != xscr2->chg2 ||
 				xdl_merge_cmp_lines(xe1, xscr1->i2,
@@ -449,12 +449,11 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1,
 			chg0 = xscr1->i1 + xscr1->chg1 - i0;
 			chg1 = xscr1->i2 + xscr1->chg2 - i1;
 			chg2 = xscr2->i2 + xscr2->chg2 - i2;
-			if (ffo > 0)
-				chg2 += ffo;
-			else {
+			if (ffo < 0) {
 				chg0 -= ffo;
 				chg1 -= ffo;
-			}
+			} else
+				chg2 += ffo;
 			if (xdl_append_merge(&c, 0,
 					     i0, chg0, i1, chg1, i2, chg2)) {
 				xdl_cleanup_merge(changes);
@@ -505,9 +504,10 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1,
 	if (!changes)
 		changes = c;
 	/* refine conflicts */
-	if (level > 1 &&
+	if (XDL_MERGE_ZEALOUS <= level &&
 	    (xdl_refine_conflicts(xe1, xe2, changes, xpp) < 0 ||
-	     xdl_simplify_non_conflicts(xe1, changes, level > 2) < 0)) {
+	     xdl_simplify_non_conflicts(xe1, changes,
+					XDL_MERGE_ZEALOUS < level) < 0)) {
 		xdl_cleanup_merge(changes);
 		return -1;
 	}
-- 
1.6.0.1.149.ga4c44
Junio C Hamano· Aug 30, 2008, 00:42 UTC · re: Junio C Hamano · lore

[PATCH 04/12] xmerge.c: "diff3 -m" style clips merge reduction level to EAGER or less

When showing a conflicting merge result, and "--diff3 -m" style is asked for, this patch makes sure that the merge reduction level does not exceed XDL_MERGE_EAGER. This is because "diff3 -m" style output would not make sense for anything more aggressive than XDL_MERGE_EAGER, because of the way how the merge reduction works.

"git merge-file" no longer has to force MERGE_EAGER when "--diff3" is asked for because of this change.

Suppose a common ancestor (shared preimage) is modified to postimage #1 and #2 (each letter represents one line):

                     #####
    postimage#1: 1234ABCDE789
                    |    /
                    |   /
    preimage:    123456789
                    |   \
    postimage#2: 1234AXYE789
                     ####
XDL_MERGE_MINIMAL and XDL_MERGE_EAGER would:
 (1) find the s/56/ABCDE/ done on one side and s/56/AXYE/ done on the
     other side,
 (2) notice that they touch an overlapping area, and
 (3) mark it as a conflict, "ABCDE vs AXYE".

The difference between the two algorithms is that EAGER drops the hunk altogether if the postimages match (i.e. both sides modified the same way), while MINIMAL keeps it. There is no other operation performed to the hunk. As the result, lines marked with "#" in the above picure will be in the RCS merge style output like this (letters <, = and > represent conflict marker lines):

    output:      1234<ABCDE=AXYE>789    ; with MINIMAL/EAGER

The part from the preimage that corresponds to these conflicting changes is "56", which is what "diff3 -m" style output adds to it:

    output:      1234<ABCDE|56=AXYE>789 ; in "diff3 -m" style

Now, XDL_MERGE_ZEALOUS looks at the differences between the changes two postimages made in order to reduce the number of lines in the conflicting regions. It notices that both sides start their new contents with "A", and excludes it from the output (it also excludes "E" for the same reason). The conflict that used to be "ABCDE vs AXYE" is now "BCD vs XY":

    output:      1234A<BCD=XY>E789      ; with ZEALOUS

There could even be matching parts between two postimages in the middle. Instead of one side rewriting the shared "56" to "ABCDE" and the other side to "AXYE", imagine the case where the postimages are "ABCDE" and "AXCYE", in which case instead of having one conflicted hunk "BCD vs XY", you would have two conflicting hunks "B vs X" and "D vs Y".

In either case, once you reduce "ABCDE vs AXYE" to "BCD vs XY" (or "ABCDE vs AXCYE" to "B vs X" and "D vs Y"), there is no part from the preimage that corresponds to the conflicting change made in both postimages anymore. In other words, conflict reduced by ZEALOUS algorithm cannot be expressed in "diff3 -m" style. Representing the last illustration like this is misleading to say the least:

    output:      1234A<BCD|56=XY>E789   ; broken "diff3 -m" style

because the preimage was not ...4A56E... to begin with. "A" and "E" are common only between the postimages.

Even worse, once a single conflicting hunk is split into multiple ones (recall the example of breaking "ABCDE vs AXCYE" to "B vs X" and "D vs Y"), there is no sane way to distribute the preimage text across split conflicting hunks.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-merge-file.c |    4 +---
 xdiff/xmerge.c       |    9 +++++++++
 2 files changed, 10 insertions(+), 3 deletions(-)
Show changes to 2 files +10 −3

builtin-merge-file.c, xdiff/xmerge.c

diff --git a/builtin-merge-file.c b/builtin-merge-file.c
index 5b4f020..1e92510 100644
--- a/builtin-merge-file.c
+++ b/builtin-merge-file.c
@@ -27,10 +27,8 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
 		else if (!strcmp(argv[1], "-q") ||
 				!strcmp(argv[1], "--quiet"))
 			freopen("/dev/null", "w", stderr);
-		else if (!strcmp(argv[1], "--diff3")) {
+		else if (!strcmp(argv[1], "--diff3"))
 			merge_style = XDL_MERGE_DIFF3;
-			merge_level = XDL_MERGE_EAGER;
-		}
 		else
 			usage(merge_file_usage);
 		argc--;
diff --git a/xdiff/xmerge.c b/xdiff/xmerge.c
index 7dcd405..d9737f0 100644
--- a/xdiff/xmerge.c
+++ b/xdiff/xmerge.c
@@ -392,6 +392,15 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1,
 	int level = flags & XDL_MERGE_LEVEL_MASK;
 	int style = flags & XDL_MERGE_STYLE_MASK;
 
+	if (style == XDL_MERGE_DIFF3) {
+		/*
+		 * "diff3 -m" output does not make sense for anything
+		 * more aggressive than XDL_MERGE_EAGER.
+		 */
+		if (XDL_MERGE_EAGER < level)
+			level = XDL_MERGE_EAGER;
+	}
+
 	c = changes = NULL;
 
 	while (xscr1 && xscr2) {
-- 
1.6.0.1.149.ga4c44
Junio C Hamano· Aug 30, 2008, 00:42 UTC · re: Junio C Hamano · lore

[PATCH 05/12] rerere.c: use symbolic constants to keep track of parsing states

These hardcoded integers make the code harder to follow than necessary; replace them with enums to make it easier to read, before adding support for optionally parsing "diff3 -m" style conflict markers.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 rerere.c |   23 +++++++++++++----------
 1 files changed, 13 insertions(+), 10 deletions(-)
Show changes to rerere.c +13 −10
diff --git a/rerere.c b/rerere.c
index 323e493..bf74b26 100644
--- a/rerere.c
+++ b/rerere.c
@@ -75,7 +75,10 @@ static int handle_file(const char *path,
 {
 	SHA_CTX ctx;
 	char buf[1024];
-	int hunk = 0, hunk_no = 0;
+	int hunk_no = 0;
+	enum {
+		RR_CONTEXT = 0, RR_SIDE_1, RR_SIDE_2,
+	} hunk = RR_CONTEXT;
 	struct strbuf one, two;
 	FILE *f = fopen(path, "r");
 	FILE *out = NULL;
@@ -98,20 +101,20 @@ static int handle_file(const char *path,
 	strbuf_init(&two,  0);
 	while (fgets(buf, sizeof(buf), f)) {
 		if (!prefixcmp(buf, "<<<<<<< ")) {
-			if (hunk)
+			if (hunk != RR_CONTEXT)
 				goto bad;
-			hunk = 1;
+			hunk = RR_SIDE_1;
 		} else if (!prefixcmp(buf, "=======") && isspace(buf[7])) {
-			if (hunk != 1)
+			if (hunk != RR_SIDE_1)
 				goto bad;
-			hunk = 2;
+			hunk = RR_SIDE_2;
 		} else if (!prefixcmp(buf, ">>>>>>> ")) {
-			if (hunk != 2)
+			if (hunk != RR_SIDE_2)
 				goto bad;
 			if (strbuf_cmp(&one, &two) > 0)
 				strbuf_swap(&one, &two);
 			hunk_no++;
-			hunk = 0;
+			hunk = RR_CONTEXT;
 			if (out) {
 				fputs("<<<<<<<\n", out);
 				fwrite(one.buf, one.len, 1, out);
@@ -127,9 +130,9 @@ static int handle_file(const char *path,
 			}
 			strbuf_reset(&one);
 			strbuf_reset(&two);
-		} else if (hunk == 1)
+		} else if (hunk == RR_SIDE_1)
 			strbuf_addstr(&one, buf);
-		else if (hunk == 2)
+		else if (hunk == RR_SIDE_2)
 			strbuf_addstr(&two, buf);
 		else if (out)
 			fputs(buf, out);
@@ -146,7 +149,7 @@ static int handle_file(const char *path,
 		fclose(out);
 	if (sha1)
 		SHA1_Final(sha1, &ctx);
-	if (hunk) {
+	if (hunk != RR_CONTEXT) {
 		if (output)
 			unlink(output);
 		return error("Could not parse conflict hunks in %s", path);
-- 
1.6.0.1.149.ga4c44
Junio C Hamano· Aug 30, 2008, 00:42 UTC · re: Junio C Hamano · lore

[PATCH 06/12] rerere: understand "diff3 -m" style conflicts with the original

This teaches rerere to grok conflicts expressed in "diff3 -m" style output, where the version from the common ancestor is output after the first side, preceded by a "|||||||" line.

The rerere database needs to keep only the versions from two sides, so the code parses the original copy and discards it.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 rerere.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)
Show changes to rerere.c +8 −2
diff --git a/rerere.c b/rerere.c
index bf74b26..4e2c9dd 100644
--- a/rerere.c
+++ b/rerere.c
@@ -77,7 +77,7 @@ static int handle_file(const char *path,
 	char buf[1024];
 	int hunk_no = 0;
 	enum {
-		RR_CONTEXT = 0, RR_SIDE_1, RR_SIDE_2,
+		RR_CONTEXT = 0, RR_SIDE_1, RR_SIDE_2, RR_ORIGINAL,
 	} hunk = RR_CONTEXT;
 	struct strbuf one, two;
 	FILE *f = fopen(path, "r");
@@ -104,9 +104,13 @@ static int handle_file(const char *path,
 			if (hunk != RR_CONTEXT)
 				goto bad;
 			hunk = RR_SIDE_1;
-		} else if (!prefixcmp(buf, "=======") && isspace(buf[7])) {
+		} else if (!prefixcmp(buf, "|||||||") && isspace(buf[7])) {
 			if (hunk != RR_SIDE_1)
 				goto bad;
+			hunk = RR_ORIGINAL;
+		} else if (!prefixcmp(buf, "=======") && isspace(buf[7])) {
+			if (hunk != RR_SIDE_1 && hunk != RR_ORIGINAL)
+				goto bad;
 			hunk = RR_SIDE_2;
 		} else if (!prefixcmp(buf, ">>>>>>> ")) {
 			if (hunk != RR_SIDE_2)
@@ -132,6 +136,8 @@ static int handle_file(const char *path,
 			strbuf_reset(&two);
 		} else if (hunk == RR_SIDE_1)
 			strbuf_addstr(&one, buf);
+		else if (hunk == RR_ORIGINAL)
+			; /* discard */
 		else if (hunk == RR_SIDE_2)
 			strbuf_addstr(&two, buf);
 		else if (out)
-- 
1.6.0.1.149.ga4c44
Junio C Hamano· Aug 30, 2008, 00:42 UTC · re: Junio C Hamano · lore

[PATCH 07/12] merge.conflictstyle: choose between "merge" and "diff3 -m" styles

This teaches "git merge-file" to honor merge.conflictstyle configuration variable, whose value can be "merge" (default) or "diff3".

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/config.txt |    8 ++++++++
 builtin-merge-file.c     |    9 +++++++++
 t/t6023-merge-file.sh    |    9 ++++++++-
 xdiff-interface.c        |   20 ++++++++++++++++++++
 xdiff-interface.h        |    2 ++
 5 files changed, 47 insertions(+), 1 deletions(-)
Show changes to 5 files +47 −1

Documentation/config.txt, builtin-merge-file.c, t/t6023-merge-file.sh, xdiff-interface.c, xdiff-interface.h

diff --git a/Documentation/config.txt b/Documentation/config.txt
index af57d94..cb4c4ca 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -893,6 +893,14 @@ man.<tool>.path::
 	Override the path for the given tool that may be used to
 	display help in the 'man' format. See linkgit:git-help[1].
 
+merge.conflictstyle::
+	Specify the style in which conflicted hunks are written out to
+	working tree files upon merge.  The default is "merge", which
+	shows `<<<<<<<` conflict marker, change made by one side,
+	`=======` marker, change made by the other side, and then
+	`>>>>>>>` marker.  An alternate style, "diff3", adds `|||||||`
+	marker and the original text before `=======` marker.
+
 mergetool.<tool>.path::
 	Override the path for the given tool.  This is useful in case
 	your tool is not in the PATH.
diff --git a/builtin-merge-file.c b/builtin-merge-file.c
index 1e92510..f009e73 100644
--- a/builtin-merge-file.c
+++ b/builtin-merge-file.c
@@ -15,6 +15,15 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
 	int ret = 0, i = 0, to_stdout = 0;
 	int merge_level = XDL_MERGE_ZEALOUS_ALNUM;
 	int merge_style = 0;
+	int nongit;
+
+	prefix = setup_git_directory_gently(&nongit);
+	if (!nongit) {
+		/* Read the configuration file */
+		git_config(git_xmerge_config, NULL);
+		if (git_xmerge_style > 0)
+			merge_style = git_xmerge_style;
+	}
 
 	while (argc > 4) {
 		if (!strcmp(argv[1], "-L") && i < 3) {
diff --git a/t/t6023-merge-file.sh b/t/t6023-merge-file.sh
index f3484e3..93ec517 100755
--- a/t/t6023-merge-file.sh
+++ b/t/t6023-merge-file.sh
@@ -192,10 +192,17 @@ non timebo mala, quoniam TU mecum es:
 virga tua et baculus tuus ipsa me consolata sunt.
 EOF
 
-test_expect_success '"diff3 -m" style output' '
+test_expect_success '"diff3 -m" style output (1)' '
 	test_must_fail git merge-file -p --diff3 \
 		new8.txt new5.txt new9.txt >actual &&
 	test_cmp expect actual
 '
 
+test_expect_success '"diff3 -m" style output (2)' '
+	git config merge.conflictstyle diff3 &&
+	test_must_fail git merge-file -p \
+		new8.txt new5.txt new9.txt >actual &&
+	test_cmp expect actual
+'
+
 test_done
diff --git a/xdiff-interface.c b/xdiff-interface.c
index 944ad98..8457601 100644
--- a/xdiff-interface.c
+++ b/xdiff-interface.c
@@ -249,3 +249,23 @@ void xdiff_set_find_func(xdemitconf_t *xecfg, const char *value)
 		value = ep + 1;
 	}
 }
+
+int git_xmerge_style = -1;
+
+int git_xmerge_config(const char *var, const char *value, void *cb)
+{
+	if (!strcasecmp(var, "merge.conflictstyle")) {
+		if (!value)
+			die("'%s' is not a boolean", var);
+		if (!strcmp(value, "diff3"))
+			git_xmerge_style = XDL_MERGE_DIFF3;
+		else if (!strcmp(value, "merge"))
+			git_xmerge_style = 0;
+		else
+			die("unknown style '%s' given for '%s'",
+			    value, var);
+		return 0;
+	}
+	return git_default_config(var, value, cb);
+}
+
diff --git a/xdiff-interface.h b/xdiff-interface.h
index 558492b..b3b5c93 100644
--- a/xdiff-interface.h
+++ b/xdiff-interface.h
@@ -17,5 +17,7 @@ int read_mmfile(mmfile_t *ptr, const char *filename);
 int buffer_is_binary(const char *ptr, unsigned long size);
 
 extern void xdiff_set_find_func(xdemitconf_t *xecfg, const char *line);
+extern int git_xmerge_config(const char *var, const char *value, void *cb);
+extern int git_xmerge_style;
 
 #endif
-- 
1.6.0.1.149.ga4c44
Junio C Hamano· Aug 30, 2008, 00:42 UTC · re: Junio C Hamano · lore

[PATCH 08/12] git-merge-recursive: learn to honor merge.conflictstyle

This teaches the low-level ll_xdl_merge() routine to honor merge.conflictstyle configuration variable, so that merge-recursive strategy can show the conflicts in the style of user's choice.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-merge-recursive.c |    2 +-
 ll-merge.c                |   16 ++++++++++++----
 2 files changed, 13 insertions(+), 5 deletions(-)
Show changes to 2 files +14 −6

builtin-merge-recursive.c, ll-merge.c

diff --git a/builtin-merge-recursive.c b/builtin-merge-recursive.c
index dfb363e..f3b6ede 100644
--- a/builtin-merge-recursive.c
+++ b/builtin-merge-recursive.c
@@ -1348,7 +1348,7 @@ static int merge_config(const char *var, const char *value, void *cb)
 		merge_rename_limit = git_config_int(var, value);
 		return 0;
 	}
-	return git_default_config(var, value, cb);
+	return git_xmerge_config(var, value, cb);
 }
 
 int cmd_merge_recursive(int argc, const char **argv, const char *prefix)
diff --git a/ll-merge.c b/ll-merge.c
index 9837c84..4a71614 100644
--- a/ll-merge.c
+++ b/ll-merge.c
@@ -63,6 +63,7 @@ static int ll_xdl_merge(const struct ll_merge_driver *drv_unused,
 			int virtual_ancestor)
 {
 	xpparam_t xpp;
+	int style = 0;
 
 	if (buffer_is_binary(orig->ptr, orig->size) ||
 	    buffer_is_binary(src1->ptr, src1->size) ||
@@ -77,10 +78,12 @@ static int ll_xdl_merge(const struct ll_merge_driver *drv_unused,
 	}
 
 	memset(&xpp, 0, sizeof(xpp));
+	if (git_xmerge_style >= 0)
+		style = git_xmerge_style;
 	return xdl_merge(orig,
 			 src1, name1,
 			 src2, name2,
-			 &xpp, XDL_MERGE_ZEALOUS,
+			 &xpp, XDL_MERGE_ZEALOUS | style,
 			 result);
 }
 
@@ -95,10 +98,15 @@ static int ll_union_merge(const struct ll_merge_driver *drv_unused,
 	char *src, *dst;
 	long size;
 	const int marker_size = 7;
-
-	int status = ll_xdl_merge(drv_unused, result, path_unused,
-				  orig, src1, NULL, src2, NULL,
-				  virtual_ancestor);
+	int status, saved_style;
+
+	/* We have to force the RCS "merge" style */
+	saved_style = git_xmerge_style;
+	git_xmerge_style = 0;
+	status = ll_xdl_merge(drv_unused, result, path_unused,
+			      orig, src1, NULL, src2, NULL,
+			      virtual_ancestor);
+	git_xmerge_style = saved_style;
 	if (status <= 0)
 		return status;
 	size = result->size;
-- 
1.6.0.1.149.ga4c44
Junio C Hamano· Aug 30, 2008, 00:42 UTC · re: Junio C Hamano · lore

[PATCH 09/12] checkout: do not check out unmerged higher stages randomly

During a conflicted merge when you have unmerged stages for a path F in the index, if you said:

    $ git checkout F

we rewrote F as many times as we have stages for it, and the last one (typically "theirs") was left in the work tree, without resolving the conflict.

This fixes it by noticing that a specified pathspec pattern matches an unmerged path, and by erroring out.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-checkout.c |   27 +++++++++++++++++++++++++++
 t/t7201-co.sh      |   23 +++++++++++++++++++++++
 2 files changed, 50 insertions(+), 0 deletions(-)
Show changes to 2 files +50 −0

builtin-checkout.c, t/t7201-co.sh

diff --git a/builtin-checkout.c b/builtin-checkout.c
index b380ad6..9b33f3a 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -76,6 +76,15 @@ static int read_tree_some(struct tree *tree, const char **pathspec)
 	return 0;
 }
 
+static int skip_same_name(struct cache_entry *ce, int pos)
+{
+	while (++pos < active_nr &&
+	       !strcmp(active_cache[pos]->name, ce->name))
+		; /* skip */
+	return pos;
+}
+
+
 static int checkout_paths(struct tree *source_tree, const char **pathspec)
 {
 	int pos;
@@ -107,6 +116,20 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec)
 	if (report_path_error(ps_matched, pathspec, 0))
 		return 1;
 
+	/* Any unmerged paths? */
+	for (pos = 0; pos < active_nr; pos++) {
+		struct cache_entry *ce = active_cache[pos];
+		if (pathspec_match(pathspec, NULL, ce->name, 0) &&
+		    ce_stage(ce)) {
+			errs = 1;
+			error("path '%s' is unmerged", ce->name);
+			pos = skip_same_name(ce, pos) - 1;
+			continue;
+		}
+	}
+	if (errs)
+		return 1;
+
 	/* Now we are committed to check them out */
 	memset(&state, 0, sizeof(state));
 	state.force = 1;
@@ -114,6 +137,10 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec)
 	for (pos = 0; pos < active_nr; pos++) {
 		struct cache_entry *ce = active_cache[pos];
 		if (pathspec_match(pathspec, NULL, ce->name, 0)) {
+			if (ce_stage(ce)) {
+				pos = skip_same_name(ce, pos) - 1;
+				continue;
+			}
 			errs |= checkout_entry(ce, &state, NULL);
 		}
 	}
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 1dff84d..303cf62 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -369,4 +369,27 @@ test_expect_success \
     'checkout with --track, but without -b, fails with too short tracked name' '
     test_must_fail git checkout --track renamer'
 
+test_expect_success 'checkout an unmerged path should fail' '
+	rm -f .git/index &&
+	O=$(echo original | git hash-object -w --stdin) &&
+	A=$(echo ourside | git hash-object -w --stdin) &&
+	B=$(echo theirside | git hash-object -w --stdin) &&
+	(
+		echo "100644 $A 0	fild" &&
+		echo "100644 $O 1	file" &&
+		echo "100644 $A 2	file" &&
+		echo "100644 $B 3	file" &&
+		echo "100644 $A 0	filf"
+	) | git update-index --index-info &&
+	echo "none of the above" >sample &&
+	cat sample >fild &&
+	cat sample >file &&
+	cat sample >filf &&
+	test_must_fail git checkout fild file filf &&
+	test_cmp sample fild &&
+	test_cmp sample filf &&
+	test_cmp sample file
+'
+
 test_done
+
-- 
1.6.0.1.149.ga4c44
Junio C Hamano· Aug 30, 2008, 00:42 UTC · re: Junio C Hamano · lore

[PATCH 10/12] checkout: allow ignoring unmerged paths when checking out of the index

Earlier we made "git checkout $pathspec" to atomically refuse the operation of $pathspec matched any path with unmerged stages. This patch allows:

    $ git checkout -f a b c

to ignore, instead of error out on, such unmerged paths. The fix to prevent checkout of an unmerged path from random stages is still there.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-checkout.c |   41 +++++++++++++++++++++++------------------
 t/t7201-co.sh      |   24 +++++++++++++++++++++++-
 2 files changed, 46 insertions(+), 19 deletions(-)
Show changes to 2 files +46 −19

builtin-checkout.c, t/t7201-co.sh

diff --git a/builtin-checkout.c b/builtin-checkout.c
index 9b33f3a..49c43d9 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -20,6 +20,17 @@ static const char * const checkout_usage[] = {
 	NULL,
 };
 
+struct checkout_opts {
+	int quiet;
+	int merge;
+	int force;
+	int writeout_error;
+
+	const char *new_branch;
+	int new_branch_log;
+	enum branch_track track;
+};
+
 static int post_checkout_hook(struct commit *old, struct commit *new,
 			      int changed)
 {
@@ -85,7 +96,8 @@ static int skip_same_name(struct cache_entry *ce, int pos)
 }
 
 
-static int checkout_paths(struct tree *source_tree, const char **pathspec)
+static int checkout_paths(struct tree *source_tree, const char **pathspec,
+			  struct checkout_opts *opts)
 {
 	int pos;
 	struct checkout state;
@@ -121,8 +133,12 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec)
 		struct cache_entry *ce = active_cache[pos];
 		if (pathspec_match(pathspec, NULL, ce->name, 0) &&
 		    ce_stage(ce)) {
-			errs = 1;
-			error("path '%s' is unmerged", ce->name);
+			if (!opts->force) {
+				errs = 1;
+				error("path '%s' is unmerged", ce->name);
+			} else {
+				warning("path '%s' is unmerged", ce->name);
+			}
 			pos = skip_same_name(ce, pos) - 1;
 			continue;
 		}
@@ -178,17 +194,6 @@ static void describe_detached_head(char *msg, struct commit *commit)
 	strbuf_release(&sb);
 }
 
-struct checkout_opts {
-	int quiet;
-	int merge;
-	int force;
-	int writeout_error;
-
-	const char *new_branch;
-	int new_branch_log;
-	enum branch_track track;
-};
-
 static int reset_tree(struct tree *tree, struct checkout_opts *o, int worktree)
 {
 	struct unpack_trees_options opts;
@@ -569,15 +574,15 @@ no_reference:
 			die("invalid path specification");
 
 		/* Checkout paths */
-		if (opts.new_branch || opts.force || opts.merge) {
+		if (opts.new_branch || opts.merge) {
 			if (argc == 1) {
-				die("git checkout: updating paths is incompatible with switching branches/forcing\nDid you intend to checkout '%s' which can not be resolved as commit?", argv[0]);
+				die("git checkout: updating paths is incompatible with switching branches.\nDid you intend to checkout '%s' which can not be resolved as commit?", argv[0]);
 			} else {
-				die("git checkout: updating paths is incompatible with switching branches/forcing");
+				die("git checkout: updating paths is incompatible with switching branches.");
 			}
 		}
 
-		return checkout_paths(source_tree, pathspec);
+		return checkout_paths(source_tree, pathspec, &opts);
 	}
 
 	if (new.name && !new.commit) {
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 303cf62..b7274eb 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -391,5 +391,27 @@ test_expect_success 'checkout an unmerged path should fail' '
 	test_cmp sample file
 '
 
-test_done
+test_expect_success 'checkout with an unmerged path can be ignored' '
+	rm -f .git/index &&
+	O=$(echo original | git hash-object -w --stdin) &&
+	A=$(echo ourside | git hash-object -w --stdin) &&
+	B=$(echo theirside | git hash-object -w --stdin) &&
+	(
+		echo "100644 $A 0	fild" &&
+		echo "100644 $O 1	file" &&
+		echo "100644 $A 2	file" &&
+		echo "100644 $B 3	file" &&
+		echo "100644 $A 0	filf"
+	) | git update-index --index-info &&
+	echo "none of the above" >sample &&
+	echo ourside >expect &&
+	cat sample >fild &&
+	cat sample >file &&
+	cat sample >filf &&
+	git checkout -f fild file filf &&
+	test_cmp expect fild &&
+	test_cmp expect filf &&
+	test_cmp sample file
+'
 
+test_done
-- 
1.6.0.1.149.ga4c44
Junio C Hamano· Aug 30, 2008, 00:42 UTC · re: Junio C Hamano · lore

[PATCH 11/12] checkout --ours/--theirs

This lets you to check out 'our' (or 'their') version of an unmerged path out of the index while resolving conflicts.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-checkout.c |   47 ++++++++++++++++++++++++++++++++++++++++++-----
 t/t7201-co.sh      |   25 +++++++++++++++++++++++++
 2 files changed, 67 insertions(+), 5 deletions(-)
Show changes to 2 files +67 −5

builtin-checkout.c, t/t7201-co.sh

diff --git a/builtin-checkout.c b/builtin-checkout.c
index 49c43d9..bdfdb65 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -24,6 +24,7 @@ struct checkout_opts {
 	int quiet;
 	int merge;
 	int force;
+	int writeout_stage;
 	int writeout_error;
 
 	const char *new_branch;
@@ -95,6 +96,32 @@ static int skip_same_name(struct cache_entry *ce, int pos)
 	return pos;
 }
 
+static int check_stage(int stage, struct cache_entry *ce, int pos)
+{
+	while (pos < active_nr &&
+	       !strcmp(active_cache[pos]->name, ce->name)) {
+		if (ce_stage(active_cache[pos]) == stage)
+			return 0;
+		pos++;
+	}
+	return error("path '%s' does not have %s version",
+		     ce->name,
+		     (stage == 2) ? "our" : "their");
+}
+
+static int checkout_stage(int stage, struct cache_entry *ce, int pos,
+			  struct checkout *state)
+{
+	while (pos < active_nr &&
+	       !strcmp(active_cache[pos]->name, ce->name)) {
+		if (ce_stage(active_cache[pos]) == stage)
+			return checkout_entry(active_cache[pos], state, NULL);
+		pos++;
+	}
+	return error("path '%s' does not have %s version",
+		     ce->name,
+		     (stage == 2) ? "our" : "their");
+}
 
 static int checkout_paths(struct tree *source_tree, const char **pathspec,
 			  struct checkout_opts *opts)
@@ -106,6 +133,7 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec,
 	int flag;
 	struct commit *head;
 	int errs = 0;
+	int stage = opts->writeout_stage;
 
 	int newfd;
 	struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));
@@ -131,13 +159,16 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec,
 	/* Any unmerged paths? */
 	for (pos = 0; pos < active_nr; pos++) {
 		struct cache_entry *ce = active_cache[pos];
-		if (pathspec_match(pathspec, NULL, ce->name, 0) &&
-		    ce_stage(ce)) {
-			if (!opts->force) {
+		if (pathspec_match(pathspec, NULL, ce->name, 0)) {
+			if (!ce_stage(ce))
+				continue;
+			if (stage) {
+				errs |= check_stage(stage, ce, pos);
+			} else if (opts->force) {
+				warning("path '%s' is unmerged", ce->name);
+			} else {
 				errs = 1;
 				error("path '%s' is unmerged", ce->name);
-			} else {
-				warning("path '%s' is unmerged", ce->name);
 			}
 			pos = skip_same_name(ce, pos) - 1;
 			continue;
@@ -154,6 +185,8 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec,
 		struct cache_entry *ce = active_cache[pos];
 		if (pathspec_match(pathspec, NULL, ce->name, 0)) {
 			if (ce_stage(ce)) {
+				if (stage)
+					errs |= checkout_stage(stage, ce, pos, &state);
 				pos = skip_same_name(ce, pos) - 1;
 				continue;
 			}
@@ -458,6 +491,10 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		OPT_BOOLEAN('l', NULL, &opts.new_branch_log, "log for new branch"),
 		OPT_SET_INT('t', "track",  &opts.track, "track",
 			BRANCH_TRACK_EXPLICIT),
+		OPT_SET_INT('2', "ours", &opts.writeout_stage, "stage",
+			    2),
+		OPT_SET_INT('3', "theirs", &opts.writeout_stage, "stage",
+			    3),
 		OPT_BOOLEAN('f', NULL, &opts.force, "force"),
 		OPT_BOOLEAN('m', NULL, &opts.merge, "merge"),
 		OPT_END(),
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index b7274eb..85c792c 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -414,4 +414,29 @@ test_expect_success 'checkout with an unmerged path can be ignored' '
 	test_cmp sample file
 '
 
+test_expect_success 'checkout unmerged stage' '
+	rm -f .git/index &&
+	O=$(echo original | git hash-object -w --stdin) &&
+	A=$(echo ourside | git hash-object -w --stdin) &&
+	B=$(echo theirside | git hash-object -w --stdin) &&
+	(
+		echo "100644 $A 0	fild" &&
+		echo "100644 $O 1	file" &&
+		echo "100644 $A 2	file" &&
+		echo "100644 $B 3	file" &&
+		echo "100644 $A 0	filf"
+	) | git update-index --index-info &&
+	echo "none of the above" >sample &&
+	echo ourside >expect &&
+	cat sample >fild &&
+	cat sample >file &&
+	cat sample >filf &&
+	git checkout --ours . &&
+	test_cmp expect fild &&
+	test_cmp expect filf &&
+	test_cmp expect file &&
+	git checkout --theirs file &&
+	test ztheirside = "z$(cat file)"
+'
+
 test_done
-- 
1.6.0.1.149.ga4c44
Junio C Hamano· Aug 30, 2008, 00:42 UTC · re: Junio C Hamano · lore

[PATCH 12/12] checkout -m: recreate merge when checking out of unmerged index

This teaches git-checkout to recreate a merge out of unmerged index entries while resolving conflicts.

With this patch, checking out an unmerged path from the index now have the following possibilities:

 * Without any option, an attempt to checkout an unmerged path
   will atomically fail (i.e. no other cleanly-merged paths are
   checked out either);
 * With "-f", other cleanly-merged paths are checked out, and
   unmerged paths are ignored;
 * With "--ours" or "--theirs, the contents from the specified
   stage is checked out;
 * With "-m" (we should add "--merge" as synonym), the 3-way merge
   is recreated from the staged object names and checked out.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-checkout.c |  115 ++++++++++++++++++++++++++++++++++++++++++++++++----
 t/t7201-co.sh      |   63 ++++++++++++++++++++++++++++
 2 files changed, 170 insertions(+), 8 deletions(-)
Show changes to 2 files +170 −8

builtin-checkout.c, t/t7201-co.sh

diff --git a/builtin-checkout.c b/builtin-checkout.c
index bdfdb65..8b5da7f 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -13,6 +13,9 @@
 #include "diff.h"
 #include "revision.h"
 #include "remote.h"
+#include "blob.h"
+#include "xdiff-interface.h"
+#include "ll-merge.h"
 
 static const char * const checkout_usage[] = {
 	"git checkout [options] <branch>",
@@ -109,6 +112,19 @@ static int check_stage(int stage, struct cache_entry *ce, int pos)
 		     (stage == 2) ? "our" : "their");
 }
 
+static int check_all_stage(struct cache_entry *ce, int pos)
+{
+	if (ce_stage(ce) != 1 ||
+	    active_nr <= pos + 2 ||
+	    strcmp(active_cache[pos+1]->name, ce->name) ||
+	    ce_stage(active_cache[pos+1]) != 2 ||
+	    strcmp(active_cache[pos+2]->name, ce->name) ||
+	    ce_stage(active_cache[pos+2]) != 3)
+		return error("path '%s' does not have all three versions",
+			     ce->name);
+	return 0;
+}
+
 static int checkout_stage(int stage, struct cache_entry *ce, int pos,
 			  struct checkout *state)
 {
@@ -123,6 +139,77 @@ static int checkout_stage(int stage, struct cache_entry *ce, int pos,
 		     (stage == 2) ? "our" : "their");
 }
 
+/* NEEDSWORK: share with merge-recursive */
+static void fill_mm(const unsigned char *sha1, mmfile_t *mm)
+{
+	unsigned long size;
+	enum object_type type;
+
+	if (!hashcmp(sha1, null_sha1)) {
+		mm->ptr = xstrdup("");
+		mm->size = 0;
+		return;
+	}
+
+	mm->ptr = read_sha1_file(sha1, &type, &size);
+	if (!mm->ptr || type != OBJ_BLOB)
+		die("unable to read blob object %s", sha1_to_hex(sha1));
+	mm->size = size;
+}
+
+static int checkout_merged(int pos, struct checkout *state)
+{
+	struct cache_entry *ce = active_cache[pos];
+	const char *path = ce->name;
+	mmfile_t ancestor, ours, theirs;
+	int status;
+	unsigned char sha1[20];
+	mmbuffer_t result_buf;
+
+	if (ce_stage(ce) != 1 ||
+	    active_nr <= pos + 2 ||
+	    strcmp(active_cache[pos+1]->name, path) ||
+	    ce_stage(active_cache[pos+1]) != 2 ||
+	    strcmp(active_cache[pos+2]->name, path) ||
+	    ce_stage(active_cache[pos+2]) != 3)
+		return error("path '%s' does not have all 3 versions", path);
+
+	fill_mm(active_cache[pos]->sha1, &ancestor);
+	fill_mm(active_cache[pos+1]->sha1, &ours);
+	fill_mm(active_cache[pos+2]->sha1, &theirs);
+
+	status = ll_merge(&result_buf, path, &ancestor,
+			  &ours, "ours", &theirs, "theirs", 1);
+	free(ancestor.ptr);
+	free(ours.ptr);
+	free(theirs.ptr);
+	if (status < 0 || !result_buf.ptr) {
+		free(result_buf.ptr);
+		return error("path '%s': cannot merge", path);
+	}
+
+	/*
+	 * NEEDSWORK:
+	 * There is absolutely no reason to write this as a blob object
+	 * and create a phoney cache entry just to leak.  This hack is
+	 * primarily to get to the write_entry() machinery that massages
+	 * the contents to work-tree format and writes out which only
+	 * allows it for a cache entry.  The code in write_entry() needs
+	 * to be refactored to allow us to feed a <buffer, size, mode>
+	 * instead of a cache entry.  Such a refactoring would help
+	 * merge_recursive as well (it also writes the merge result to the
+	 * object database even when it may contain conflicts).
+	 */
+	if (write_sha1_file(result_buf.ptr, result_buf.size,
+			    blob_type, sha1))
+		die("Unable to add merge result for '%s'", path);
+	ce = make_cache_entry(create_ce_mode(active_cache[pos+1]->ce_mode),
+			      sha1,
+			      path, 2, 0);
+	status = checkout_entry(ce, state, NULL);
+	return status;
+}
+
 static int checkout_paths(struct tree *source_tree, const char **pathspec,
 			  struct checkout_opts *opts)
 {
@@ -134,7 +221,7 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec,
 	struct commit *head;
 	int errs = 0;
 	int stage = opts->writeout_stage;
-
+	int merge = opts->merge;
 	int newfd;
 	struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));
 
@@ -166,6 +253,8 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec,
 				errs |= check_stage(stage, ce, pos);
 			} else if (opts->force) {
 				warning("path '%s' is unmerged", ce->name);
+			} else if (opts->merge) {
+				errs |= check_all_stage(ce, pos);
 			} else {
 				errs = 1;
 				error("path '%s' is unmerged", ce->name);
@@ -184,13 +273,15 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec,
 	for (pos = 0; pos < active_nr; pos++) {
 		struct cache_entry *ce = active_cache[pos];
 		if (pathspec_match(pathspec, NULL, ce->name, 0)) {
-			if (ce_stage(ce)) {
-				if (stage)
-					errs |= checkout_stage(stage, ce, pos, &state);
-				pos = skip_same_name(ce, pos) - 1;
+			if (!ce_stage(ce)) {
+				errs |= checkout_entry(ce, &state, NULL);
 				continue;
 			}
-			errs |= checkout_entry(ce, &state, NULL);
+			if (stage)
+				errs |= checkout_stage(stage, ce, pos, &state);
+			else if (merge)
+				errs |= checkout_merged(pos, &state);
+			pos = skip_same_name(ce, pos) - 1;
 		}
 	}
 
@@ -478,6 +569,11 @@ static int switch_branches(struct checkout_opts *opts, struct branch_info *new)
 	return ret || opts->writeout_error;
 }
 
+static int git_checkout_config(const char *var, const char *value, void *cb)
+{
+	return git_xmerge_config(var, value, cb);
+}
+
 int cmd_checkout(int argc, const char **argv, const char *prefix)
 {
 	struct checkout_opts opts;
@@ -504,7 +600,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 	memset(&opts, 0, sizeof(opts));
 	memset(&new, 0, sizeof(new));
 
-	git_config(git_default_config, NULL);
+	git_config(git_checkout_config, NULL);
 
 	opts.track = BRANCH_TRACK_UNSPECIFIED;
 
@@ -611,7 +707,7 @@ no_reference:
 			die("invalid path specification");
 
 		/* Checkout paths */
-		if (opts.new_branch || opts.merge) {
+		if (opts.new_branch) {
 			if (argc == 1) {
 				die("git checkout: updating paths is incompatible with switching branches.\nDid you intend to checkout '%s' which can not be resolved as commit?", argv[0]);
 			} else {
@@ -619,6 +715,9 @@ no_reference:
 			}
 		}
 
+		if (1 < !!opts.writeout_stage + !!opts.force + !!opts.merge)
+			die("git checkout: --ours/--theirs, --force and --merge are incompatible when\nchecking out of the index.");
+
 		return checkout_paths(source_tree, pathspec, &opts);
 	}
 
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 85c792c..6016915 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -439,4 +439,67 @@ test_expect_success 'checkout unmerged stage' '
 	test ztheirside = "z$(cat file)"
 '
 
+test_expect_success 'checkout with --merge' '
+	rm -f .git/index &&
+	O=$(echo original | git hash-object -w --stdin) &&
+	A=$(echo ourside | git hash-object -w --stdin) &&
+	B=$(echo theirside | git hash-object -w --stdin) &&
+	(
+		echo "100644 $A 0	fild" &&
+		echo "100644 $O 1	file" &&
+		echo "100644 $A 2	file" &&
+		echo "100644 $B 3	file" &&
+		echo "100644 $A 0	filf"
+	) | git update-index --index-info &&
+	echo "none of the above" >sample &&
+	echo ourside >expect &&
+	cat sample >fild &&
+	cat sample >file &&
+	cat sample >filf &&
+	git checkout -m -- fild file filf &&
+	(
+		echo "<<<<<<< ours"
+		echo ourside
+		echo "======="
+		echo theirside
+		echo ">>>>>>> theirs"
+	) >merged &&
+	test_cmp expect fild &&
+	test_cmp expect filf &&
+	test_cmp merged file
+'
+
+test_expect_success 'checkout with --merge, in diff3 -m style' '
+	git config merge.conflictstyle diff3 &&
+	rm -f .git/index &&
+	O=$(echo original | git hash-object -w --stdin) &&
+	A=$(echo ourside | git hash-object -w --stdin) &&
+	B=$(echo theirside | git hash-object -w --stdin) &&
+	(
+		echo "100644 $A 0	fild" &&
+		echo "100644 $O 1	file" &&
+		echo "100644 $A 2	file" &&
+		echo "100644 $B 3	file" &&
+		echo "100644 $A 0	filf"
+	) | git update-index --index-info &&
+	echo "none of the above" >sample &&
+	echo ourside >expect &&
+	cat sample >fild &&
+	cat sample >file &&
+	cat sample >filf &&
+	git checkout -m -- fild file filf &&
+	(
+		echo "<<<<<<< ours"
+		echo ourside
+		echo "|||||||"
+		echo original
+		echo "======="
+		echo theirside
+		echo ">>>>>>> theirs"
+	) >merged &&
+	test_cmp expect fild &&
+	test_cmp expect filf &&
+	test_cmp merged file
+'
+
 test_done
-- 
1.6.0.1.149.ga4c44
Johannes Schindelin· Aug 30, 2008, 09:42 UTC · re: Junio C Hamano · lore

Re: [PATCH 07/12] merge.conflictstyle: choose between "merge" and "diff3 -m" styles

Hi,
On Fri, 29 Aug 2008, Junio C Hamano wrote:
Show 16 quoted lines
> diff --git a/builtin-merge-file.c b/builtin-merge-file.c
> index 1e92510..f009e73 100644
> --- a/builtin-merge-file.c
> +++ b/builtin-merge-file.c
> @@ -15,6 +15,15 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
>  	int ret = 0, i = 0, to_stdout = 0;
>  	int merge_level = XDL_MERGE_ZEALOUS_ALNUM;
>  	int merge_style = 0;
> +	int nongit;
> +
> +	prefix = setup_git_directory_gently(&nongit);
> +	if (!nongit) {
> +		/* Read the configuration file */
> +		git_config(git_xmerge_config, NULL);
> +		if (git_xmerge_style > 0)
> +			merge_style = git_xmerge_style;

Did you not mean ">="? In the future, the default merge style could very well change...

Ciao, Dscho

Johannes Schindelin· Aug 30, 2008, 09:34 UTC · re: Junio C Hamano · lore

Re: [PATCH 04/12] xmerge.c: "diff3 -m" style clips merge reduction level to EAGER or less

Hi,
On Fri, 29 Aug 2008, Junio C Hamano wrote:
Show 7 quoted lines
> Representing the last illustration like this is misleading to say the 
> least:
> 
>     output:      1234A<BCD|56=XY>E789   ; broken "diff3 -m" style
> 
> because the preimage was not ...4A56E... to begin with.  "A" and "E" are
> common only between the postimages.
Okay, I understand now.

Thanks, Dscho

Johannes Schindelin· Aug 30, 2008, 09:31 UTC · re: Junio C Hamano · lore

Re: [PATCH 03/12] xmerge.c: minimum readability fixups

Hi,
On Fri, 29 Aug 2008, Junio C Hamano wrote:
Show 10 quoted lines
> diff --git a/xdiff/xmerge.c b/xdiff/xmerge.c
> index 29cdbea..7dcd405 100644
> --- a/xdiff/xmerge.c
> +++ b/xdiff/xmerge.c
> @@ -427,7 +427,7 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1,
>  			xscr2 = xscr2->next;
>  			continue;
>  		}
> -		if (level < 1 || xscr1->i1 != xscr2->i1 ||
> +		if (level == XDL_MERGE_MINIMAL || xscr1->i1 != xscr2->i1 ||
Yeah, okay, sorry.
Show 13 quoted lines
> @@ -449,12 +449,11 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1,
>  			chg0 = xscr1->i1 + xscr1->chg1 - i0;
>  			chg1 = xscr1->i2 + xscr1->chg2 - i1;
>  			chg2 = xscr2->i2 + xscr2->chg2 - i2;
> -			if (ffo > 0)
> -				chg2 += ffo;
> -			else {
> +			if (ffo < 0) {
>  				chg0 -= ffo;
>  				chg1 -= ffo;
> -			}
> +			} else
> +				chg2 += ffo;

I do not understand why the order was changed, but hey, I do not care that deeply.

Ciao, Dscho

Junio C Hamano· Aug 30, 2008, 15:42 UTC · re: Johannes Schindelin · lore

Re: [PATCH 03/12] xmerge.c: minimum readability fixups

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 16 quoted lines
>> @@ -449,12 +449,11 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1,
>>  			chg0 = xscr1->i1 + xscr1->chg1 - i0;
>>  			chg1 = xscr1->i2 + xscr1->chg2 - i1;
>>  			chg2 = xscr2->i2 + xscr2->chg2 - i2;
>> -			if (ffo > 0)
>> -				chg2 += ffo;
>> -			else {
>> +			if (ffo < 0) {
>>  				chg0 -= ffo;
>>  				chg1 -= ffo;
>> -			}
>> +			} else
>> +				chg2 += ffo;
>
> I do not understand why the order was changed, but hey, I do not care that 
> deeply.

Everywhere else the code deals with variables var0, var1 and var2 in this order, and that is because "if" blocks are consistently about changes made on side#1 while "else" blocks are about changes made on side#2. This statement alone was inconsistent, and now it all reads 0, 1 and then 2.

Johannes Schindelin· Aug 30, 2008, 09:29 UTC · re: Junio C Hamano · lore

Re: [PATCH 02/12] xdiff-merge: optionally show conflicts in "diff3 -m" style

Hi,
On Fri, 29 Aug 2008, Junio C Hamano wrote:
Show 12 quoted lines
> diff --git a/builtin-merge-file.c b/builtin-merge-file.c
> index 3605960..5b4f020 100644
> --- a/builtin-merge-file.c
> +++ b/builtin-merge-file.c
> @@ -25,6 +27,10 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
>  		else if (!strcmp(argv[1], "-q") ||
>  				!strcmp(argv[1], "--quiet"))
>  			freopen("/dev/null", "w", stderr);
> +		else if (!strcmp(argv[1], "--diff3")) {
> +			merge_style = XDL_MERGE_DIFF3;
> +			merge_level = XDL_MERGE_EAGER;
> +		}

FWIW I do not follow your reasoning why --diff3 does not make sense for anything more eager than MERGE_EAGER. All that ZEALOUS and ZEALOUS_ALNUM (the latter of which is useless at the moment, since it is not enabled for git-merge) do is change the way the conflicting regions are displayed, but they do not leave out conflicting regions.

So I actually suspect that ZEALOUS_ALNUM will be _especially_ useful with --diff3, since it is designed to skip the single curly brackets that would disrupt the reading pleasure otherwise.

Show 18 quoted lines
> diff --git a/xdiff/xdiff.h b/xdiff/xdiff.h
> index 413082e..deebe02 100644
> --- a/xdiff/xdiff.h
> +++ b/xdiff/xdiff.h
> @@ -50,10 +50,16 @@ extern "C" {
>  #define XDL_BDOP_CPY 2
>  #define XDL_BDOP_INSB 3
>  
> +/* merge simplification levels */
>  #define XDL_MERGE_MINIMAL 0
>  #define XDL_MERGE_EAGER 1
>  #define XDL_MERGE_ZEALOUS 2
>  #define XDL_MERGE_ZEALOUS_ALNUM 3
> +#define XDL_MERGE_LEVEL_MASK 0x0f
> +
> +/* merge output styles */
> +#define XDL_MERGE_DIFF3 0x8000
> +#define XDL_MERGE_STYLE_MASK 0x8000

Hmm. This is not the Linux kernel, I think we could safely pass around two integers instead of one.

Show 6 quoted lines
> @@ -91,11 +108,13 @@ static int xdl_merge_cmp_lines(xdfenv_t *xe1, int i1, xdfenv_t *xe2, int i2,
>  	return 0;
>  }
>  
> -static int xdl_recs_copy(xdfenv_t *xe, int i, int count, int add_nl, char *dest)
> +static int xdl_recs_copy_0(int use_orig, xdfenv_t *xe, int i, int count, int add_nl, char *dest)

You rewrapped many function headers already; I wonder why this one was left out.

The rest looks pretty much obviously correct to me; I was too lazy/ran out of time to apply the patch and look through the resulting code, though, but I guess that you searched for "i1" and "chg1" and added the code for i0 and chg0 where necessary.

So: ACK.

Ciao, Dscho

Johannes Schindelin· Aug 30, 2008, 09:14 UTC · re: Junio C Hamano · lore

Re: [PATCH 01/12] xdl_fill_merge_buffer(): separate out a too deeply nested function

Hi,
On Fri, 29 Aug 2008, Junio C Hamano wrote:
> This simply moves code around to make a separate function that prepares 
> a single conflicted hunk with markers into the buffer.

Apart from renaming "i1" to "i", inverting the order of the if clauses, and making it more obvious that the calculation of size without dest is correct, this is a straight forward refactoring.

Ciao, Dscho

Alex Riesen· Sep 1, 2008, 09:39 UTC · re: Junio C Hamano · lore

Re: [PATCH 00/12] Towards a better merge resolution support

Junio C Hamano, Sat, Aug 30, 2008 02:42:31 +0200:
Show 13 quoted lines
> The early part of the series is what you already saw.  In addition to
> recording a conflicted merge in the RCS merge style we have traditionally
> used, this allows you to optionally use "diff3 -m" style.  The difference
> is that the latter format shows the part from the common ancestor that
> corresponds to the parts both sides modified to cause the conflict, in
> addition to the changes done on each side.  This can be chosen by setting
> a configuration variable.  Rerere mechanism is updated to understand this
> new format as well, and conflicts from either formats interoperate well,
> because rerere mechanism only records and uses the changes made on each
> side, not what was in the common ancestor.
> 
> The last four patches are to "git checkout" that checks things out of the
> index.  ...

I like that and started using the patches. Do you have any specific area I should pay a special attention to?

Alex Riesen· Sep 1, 2008, 09:44 UTC · re: Junio C Hamano · lore

Re: [PATCH 00/12] Towards a better merge resolution support

Junio C Hamano, Sat, Aug 30, 2008 02:42:31 +0200:
Show 13 quoted lines
> This consists of two loosely related topics on improving conflicted merge
> resolution support.
> 
> The early part of the series is what you already saw.  In addition to
> recording a conflicted merge in the RCS merge style we have traditionally
> used, this allows you to optionally use "diff3 -m" style.  The difference
> is that the latter format shows the part from the common ancestor that
> corresponds to the parts both sides modified to cause the conflict, in
> addition to the changes done on each side.  This can be chosen by setting
> a configuration variable.  Rerere mechanism is updated to understand this
> new format as well, and conflicts from either formats interoperate well,
> because rerere mechanism only records and uses the changes made on each
> side, not what was in the common ancestor.

This reminds me: when resolving a conflict in a git repo (when trying something from next or pu), I often notice that I'd like to resolve it the same way it was done on next or pu. IOW, copy the commit resolution from some other merge commit. Maybe can be a way to use rerere mechanism with that?

Abhijit Menon-Sen· Sep 1, 2008, 09:50 UTC · re: Alex Riesen · lore

Re: [PATCH 00/12] Towards a better merge resolution support

At 2008-09-01 11:44:12 +0200, raa.lkml@gmail.com wrote:
>
> IOW, copy the commit resolution from some other merge commit. Maybe
> can be a way to use rerere mechanism with that?

That's what I'm trying to implement on Dscho's suggestion. I'm still just trying to understand the code, so any suggestions about how to do this are very welcome.

-- ams
Thomas Rast· Sep 1, 2008, 12:20 UTC · re: Abhijit Menon-Sen · lore

Re: [PATCH 00/12] Towards a better merge resolution support

Abhijit Menon-Sen wrote:
Show 8 quoted lines
> At 2008-09-01 11:44:12 +0200, raa.lkml@gmail.com wrote:
> >
> > IOW, copy the commit resolution from some other merge commit. Maybe
> > can be a way to use rerere mechanism with that?
> 
> That's what I'm trying to implement on Dscho's suggestion. I'm still
> just trying to understand the code, so any suggestions about how to
> do this are very welcome.

Random idea: you could use a script that just replays the merges present in history and lets rerere record them. Like so:

-- 8< -- #!/bin/sh

. "$(git --exec-path)/git-sh-setup"

require_work_tree git update-index --refresh || die "can't run with dirty index"

git rev-list --parents "$@" |
grep '.* .* .*' |
while read merge firstparent otherparents
do
	git checkout $firstparent >/dev/null 2>/dev/null
	git merge $otherparents >/dev/null
	if test -z "$(git ls-files -u)"; then
		echo -n 'no conflicts: '
		git --no-pager log -1 --pretty=oneline --abbrev-commit $merge
		continue
	fi
	git rerere
	git ls-files -t | grep ^M | cut -c 3- | xargs git checkout $merge --
	git rerere
	git reset --hard >/dev/null 2>/dev/null
	echo -n 'recorded:     '
	git --no-pager log -1 --pretty=oneline --abbrev-commit $merge
done
-- >8 --
The intended usage is like
  ./rerereimport.sh v1.6.0..origin/next

to import all resolutions in the specified range. Granted, it's probably not as good and definitely not as fast as an automatic feature integrated with the merge machinery.

- Thomas
-- 
Thomas Rast
trast@student.ethz.ch
Junio C Hamano· Sep 1, 2008, 10:38 UTC · re: Alex Riesen · lore

Re: [PATCH 00/12] Towards a better merge resolution support

Alex Riesen <raa.lkml@gmail.com> writes:
> ... IOW, copy the commit
> resolution from some other merge commit. Maybe can be a way to use
> rerere mechanism with that?

If you know which merge I did you want to steal from, you can prime your rerere database by pretending to be me, doing the merge. Something like:

	$ git checkout $merge^1 ;# detach to the parent of merge
        $ git merge $merge^2 ;# pretend you were me to redo it
        $ git diff -R $merge | git apply --index ;# and get what I did
	$ git rerere ;# have rerere record the resolution
Alex Riesen· Sep 1, 2008, 11:34 UTC · re: Junio C Hamano · lore

Re: [PATCH 00/12] Towards a better merge resolution support

Junio C Hamano, Mon, Sep 01, 2008 12:38:25 +0200:
Show 12 quoted lines
> Alex Riesen <raa.lkml@gmail.com> writes:
> 
> > ... IOW, copy the commit
> > resolution from some other merge commit. Maybe can be a way to use
> > rerere mechanism with that?
> 
> If you know which merge I did you want to steal from, you can prime your
> rerere database by pretending to be me, doing the merge.  Something like:
> 
> 	$ git checkout $merge^1 ;# detach to the parent of merge
>         $ git merge $merge^2 ;# pretend you were me to redo it
>         $ git diff -R $merge | git apply --index ;# and get what I did
I ended up using
    $ git checkout Merge^1
    $ git merge Merge^2
    $ git diff -R Merge | git apply
    $ git diff -R Merge --name-only -z | git update-index -z --stdin
    $ git rerere

Just git apply --index complained about the files missing from the index:

    $ git tag Merge c5e2ace70271b481632aaf987361027ca4592df6
    $ gco Merge^1
    Previous HEAD position was c5e2ace... Merge branch 'jc/better-conflict-resolution' into next
    HEAD is now at 2392877... Merge branch 'master' into next
    $ git merge Merge^2
    Auto-merging Documentation/config.txt
    Auto-merging Documentation/git-checkout.txt
    CONFLICT (content): Merge conflict in Documentation/git-checkout.txt
    Auto-merging builtin-checkout.c
    CONFLICT (content): Merge conflict in builtin-checkout.c
    Auto-merging builtin-merge-recursive.c
    Auto-merging t/t6023-merge-file.sh
    Auto-merging t/t7201-co.sh
    CONFLICT (content): Merge conflict in t/t7201-co.sh
    Auto-merging xdiff-interface.c
    Auto-merging xdiff-interface.h
    Recorded preimage for 'Documentation/git-checkout.txt'
    Recorded preimage for 'builtin-checkout.c'
    Recorded preimage for 't/t7201-co.sh'
    Automatic merge failed; fix conflicts and then commit the result.
    $ git diff -R Merge |git apply --index
    error: Documentation/git-checkout.txt: does not exist in index
    error: builtin-checkout.c: does not exist in index
    error: t/t7201-co.sh: does not exist in index
> 	$ git rerere ;# have rerere record the resolution

Well, it works, but it's a bit of work and hard to automate (needs a working tree). An option to merge:

    $ git merge <branch>
    conflict ... investigate ... find a resolution in <resolution>
    $ git reset --hard
    $ git merge --rerere <resolution> branch
    check... Ok.
    $ git commit
or rerere:
    $ git merge <branch>
    conflict ... investigate ... find a resolution in <resolution>
    $ git rerere <resolution>
    $ git reset --hard
    $ git merge <branch>
    check... Ok.
    $ git commit
These could be more convenient.
Junio C Hamano· Sep 1, 2008, 17:26 UTC · re: Alex Riesen · lore

Re: [PATCH 00/12] Towards a better merge resolution support

Alex Riesen <raa.lkml@gmail.com> writes:
Show 15 quoted lines
> Junio C Hamano, Mon, Sep 01, 2008 12:38:25 +0200:
>> Alex Riesen <raa.lkml@gmail.com> writes:
>> 
>> > ... IOW, copy the commit
>> > resolution from some other merge commit. Maybe can be a way to use
>> > rerere mechanism with that?
>> 
>> If you know which merge I did you want to steal from, you can prime your
>> rerere database by pretending to be me, doing the merge.  Something like:
>> 
>> 	$ git checkout $merge^1 ;# detach to the parent of merge
>>         $ git merge $merge^2 ;# pretend you were me to redo it
>>         $ git diff -R $merge | git apply --index ;# and get what I did
>
> I ended up using

I think the last step should be "git read-tree --reset -u $merge" if we really want the minimum sequence.

← back to recent threads