Add label for common ancestor to conflictstyle=diff3

PATCH In review Jonathan Nieder 3 participants 23 messages Jun 15, 2016

lore

Share & tools

lore

Jonathan Nieder

author

2016-06-15 22:48 UTC #0 lore raw

v2 cover Add label for common ancestor to conflictstyle=diff3

Hi all,

This is a more polished version of a series I sent a couple of days ago [1]. Thanks to Junio for some helpful advice.

The purpose: When outputting diff3-style conflict hunks, this patch series adds a label for the common ancestor (like diff3 -m would):

<<<<<<< ours
Text from the current branch
||||||| base
Text from the merge base
=======
Text from the remote branch

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
theirs

Apparently, without this label, smerge-mode.el found in emacs 22 and later has trouble parsing the conflict hunks created by git with merge.conflictstyle=diff3. More importantly, humans can sometimes use a reminder of what the ancestor represents, especially in cases like cherry-pick and revert.

As before, the bad news is that sometimes the labels for the ancestor are kind of pointless, as in “merged common ancestors” for merge-recursive. I am not planning to improve this. I’d be thrilled if someone changes it to something like “merge of ac76d, 8d7ca9, and 81873”.

The good news is a couple of new tests thrown in. All the other changes I can remember are detailed in the comments after each patch (probably missing some, since I haven’t started using git notes for this).

I think the series is ready to receive wider exposure. Please let me know if you run into any problems when trying it out. I would also be glad to hear thoughts about the details of its behavior and implementation.

The series is based against master. There are a few small conflicts with next, but nothing major; I’ll point out the few that I remember as I go.

Enjoy,
Jonathan Nieder (16):
xdl_merge(): add optional ancestor label to diff3-style output xdl_merge(): move file1 and file2 labels to xmparam structure merge-file --diff3: add a label for ancestor ll_merge(): add ancestor label parameter for diff3-style output checkout --conflict=diff3: add a label for ancestor merge_file(): add comment explaining behavior wrt conflict style merge_trees(): add ancestor label parameter for diff3-style output
tests: document format of conflicts from checkout -m checkout -m --conflict=diff3: add a label for ancestor
tests: document cherry-pick behavior in face of conflicts revert_or_cherry_pick(): get oneline_body from get_oneline()
compat: add mempcpy()
revert: simplify get_oneline() using mempcpy
revert: clarify label on conflict hunks
cherry-pick: add a label for ancestor
merge-recursive: add a label for ancestor

 builtin/checkout.c              |    3 +-
 builtin/merge-file.c            |    7 +-
 builtin/revert.c                |   33 ++++---
 git-compat-util.h               |    9 ++
 ll-merge.c                      |   28 +++---
 ll-merge.h                      |    2 +-
 merge-file.c                    |    8 ++-
 merge-recursive.c               |   12 ++-
 merge-recursive.h               |    1 +
 rerere.c                        |    4 +-
 t/t3507-cherry-pick-conflict.sh |  198 +++++++++++++++++++++++++++++++++++++++
 t/t6023-merge-file.sh           |    2 +-
 t/t7201-co.sh                   |   69 ++++++++++++-
 xdiff/xdiff.h                   |    6 +-
 xdiff/xmerge.c                  |   29 ++++--
 15 files changed, 358 insertions(+), 53 deletions(-)
 create mode 100644 t/t3507-cherry-pick-conflict.sh

[1] http://thread.gmane.org/gmane.linux.debian.devel.bugs.general/680778/focus=142176

Jonathan Nieder

author

2016-06-15 22:48 UTC #1 lore raw

PATCH v2 1/16 xdl_merge(): add optional ancestor label to diff3-style output

The ‘git checkout --conflict=diff3’ command can be used to present conflicts hunks including text from the common ancestor:

	<<<<<<< ours
	ourside
	|||||||
	original
	=======
	theirside
	>>>>>>> theirs

The added information is helpful for resolving merges by hand, and merge tools can usually grok it because it is very similar to the output from diff3 -m.

A subtle change can help more tools to understand the output. ‘diff3’ includes the name of the merge base on the ||||||| line of the output, and some tools misparse the conflict hunks without it. Add a new xmp->ancestor parameter to xdl_merge() for use with conflict style XDL_MERGE_DIFF3 as a label on the ||||||| line for any conflict hunks.

If xmp->ancestor is NULL, the output format is unchanged. ‘git checkout’ and other users within git all use NULL for this parameter, so this change only provides unexposed plumbing for a fix:
it does not affect the outward behavior of git in any way.

Requested-by Stefan Monnier <[email protected]>

Signed-off-by Jonathan Nieder <[email protected]>

---
Aside from the commit message, the only change from v1 is

- const char *ancestor;
+ const char *ancestor; /* label for orig */

 xdiff/xdiff.h  |    1 +
 xdiff/xmerge.c |   16 ++++++++++++++--
 2 files changed, 15 insertions(+), 2 deletions(-)

xdiff/xdiff.h +1 -0

index 3f6229e..fd89b9a 100644
@@ -117,6 +117,7 @@ int xdl_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp,
 typedef struct s_xmparam {
 	xpparam_t xpp;
 	int marker_size;
+	const char *ancestor;	/* label for orig */
 } xmparam_t;
 
 #define DEFAULT_CONFLICT_MARKER_SIZE 7

xdiff/xmerge.c +14 -2

index 8cbe45e..3a0ae14 100644
@@ -144,11 +144,13 @@ static int xdl_orig_copy(xdfenv_t *xe, int i, int count, int add_nl, char *dest)
 
 static int fill_conflict_hunk(xdfenv_t *xe1, const char *name1,
 			      xdfenv_t *xe2, const char *name2,
+			      const char *name3,
 			      int size, int i, int style,
 			      xdmerge_t *m, char *dest, int marker_size)
 {
 	int marker1_size = (name1 ? strlen(name1) + 1 : 0);
 	int marker2_size = (name2 ? strlen(name2) + 1 : 0);
+	int marker3_size = (name3 ? strlen(name3) + 1 : 0);
 	int j;
 
 	if (marker_size <= 0)

@@ -178,10 +180,15 @@ static int fill_conflict_hunk(xdfenv_t *xe1, const char *name1,
 	if (style == XDL_MERGE_DIFF3) {
 		/* Shared preimage */
 		if (!dest) {
-			size += marker_size + 1;
+			size += marker_size + 1 + marker3_size;
 		} else {
 			for (j = 0; j < marker_size; j++)
 				dest[size++] = '|';
+			if (marker3_size) {
+				dest[size] = ' ';
+				memcpy(dest + size + 1, name3, marker3_size - 1);
+				size += marker3_size;
+			}
 			dest[size++] = '\n';
 		}
 		size += xdl_orig_copy(xe1, m->i0, m->chg0, 1,

@@ -216,6 +223,7 @@ 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,
+				 const char *ancestor_name,
 				 int favor,
 				 xdmerge_t *m, char *dest, int style,
 				 int marker_size)

@@ -228,6 +236,7 @@ static int xdl_fill_merge_buffer(xdfenv_t *xe1, const char *name1,
 
 		if (m->mode == 0)
 			size = fill_conflict_hunk(xe1, name1, xe2, name2,
+						  ancestor_name,
 						  size, i, style, m, dest,
 						  marker_size);
 		else if (m->mode == 1)

@@ -397,6 +406,7 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1,
 		int flags, xmparam_t const *xmp, mmbuffer_t *result) {
 	xdmerge_t *changes, *c;
 	xpparam_t const *xpp = &xmp->xpp;
+	const char * const ancestor_name = xmp->ancestor;
 	int i0, i1, i2, chg0, chg1, chg2;
 	int level = flags & XDL_MERGE_LEVEL_MASK;
 	int style = flags & XDL_MERGE_STYLE_MASK;

@@ -534,6 +544,7 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1,
 	if (result) {
 		int marker_size = xmp->marker_size;
 		int size = xdl_fill_merge_buffer(xe1, name1, xe2, name2,
+						 ancestor_name,
 						 favor, changes, NULL, style,
 						 marker_size);
 		result->ptr = xdl_malloc(size);

@@ -542,7 +553,8 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1,
 			return -1;
 		}
 		result->size = size;
-		xdl_fill_merge_buffer(xe1, name1, xe2, name2, favor, changes,
+		xdl_fill_merge_buffer(xe1, name1, xe2, name2,
+				      ancestor_name, favor, changes,
 				      result->ptr, style, marker_size);
 	}
 	return xdl_cleanup_merge(changes);

Signature · 1 line

1.7.0

Bert Wesarg

Acked-by on v2 1/16

2016-06-15 22:48 UTC #2 lore raw

Re: [PATCH 01/16] xdl_merge(): add optional ancestor label to diff3-style output

xdiff/xmerge.c static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1, · line 409

Quoted 9 lines · Jonathan Nieder

xdiff/xmerge.c +1 -0

index 8cbe45e..3a0ae14 100644
@@ -397,6 +406,7 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1,
                int flags, xmparam_t const *xmp, mmbuffer_t *result) {
        xdmerge_t *changes, *c;
        xpparam_t const *xpp = &xmp->xpp;
+       const char * const ancestor_name = xmp->ancestor;

Style. * should be aligned to the variable name.

xdiff/xmerge.c static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1, · line 412

Quoted 3 lines

xdiff/xmerge.c +0 -0

        int i0, i1, i2, chg0, chg1, chg2;
        int level = flags & XDL_MERGE_LEVEL_MASK;
        int style = flags & XDL_MERGE_STYLE_MASK;

FWIW:

Acked-by Bert Wesarg <[email protected]>

Jonathan Nieder

author

2016-06-15 22:48 UTC #3 lore raw

Re: [PATCH 01/16] xdl_merge(): add optional ancestor label to diff3-style output

Quoted 11 lines · Bert Wesarg

xdiff/xmerge.c static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1, · line 409
Quoted 7 lines · Jonathan Nieder
xdiff/xmerge.c +1 -0
@@ -397,6 +406,7 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1,
   	int flags, xmparam_t const *xmp, mmbuffer_t *result) {
 	xdmerge_t *changes, *c;
 	xpparam_t const *xpp = &xmp->xpp;
+	const char * const ancestor_name = xmp->ancestor;
Style. * should be aligned to the variable name.

Not so clear to me:

 $ git grep -e '\* const' origin/master | wc -l
 90
 $ git grep -e '\*const' origin/master | wc -l
 23

I do prefer the style you suggest, so I’ve fixed it.

Quoted 3 lines

FWIW:
Acked-by Bert Wesarg <[email protected]>

Thanks for looking it over. I’ll be sending a rebased version in a moment including your ack; I hope that’s okay.

Jonathan

Jonathan Nieder

author

2016-06-15 22:48 UTC #4 lore raw

PATCH v2 2/16 xdl_merge(): move file1 and file2 labels to xmparam structure

In reply to Jonathan Nieder

The labels for the three participants in a potential conflict are all optional arguments for the xdiff merge routine; if they are NULL, then xdl_merge() can cope by omitting the labels from its output. Move them to the xmparam structure to allow new callers to save some keystrokes where they are not needed.

This also has the virtue of making the xdiff merge interface more similar to merge_trees, which might make it easier to learn.

Signed-off-by Jonathan Nieder <[email protected]>

---
Thanks to Junio for the idea. I was worried for a moment that this might be a step in the wrong direction: aren’t all existing xmparam_t members the sort of thing that would be the same over multiple merges? That is true but not really relevant.

This has interactions of the “nonconflicting changes on adjacent lines” kind with commit 560119b (refactor merge flags into xmparam_t, 2010-03-01) in next.

 builtin/merge-file.c |    6 ++++--
 ll-merge.c           |    8 ++++----
 xdiff/xdiff.h        |    5 +++--
 xdiff/xmerge.c       |   13 +++++++------
 4 files changed, 18 insertions(+), 14 deletions(-)

builtin/merge-file.c +4 -2

index 1e70073..ade1e8b 100644
@@ -72,8 +72,10 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
 					argv[i]);
 	}
 
-	ret = xdl_merge(mmfs + 1, mmfs + 0, names[0], mmfs + 2, names[2],
-			&xmp, XDL_MERGE_FLAGS(level, style, favor), &result);
+	xmp.file1 = names[0];
+	xmp.file2 = names[2];
+	ret = xdl_merge(mmfs + 1, mmfs + 0, mmfs + 2, &xmp,
+			XDL_MERGE_FLAGS(level, style, favor), &result);
 
 	for (i = 0; i < 3; i++)
 		free(mmfs[i].ptr);

ll-merge.c +4 -4

index 4c7f11b..b8e8971 100644
@@ -83,10 +83,10 @@ static int ll_xdl_merge(const struct ll_merge_driver *drv_unused,
 		style = git_xmerge_style;
 	if (marker_size > 0)
 		xmp.marker_size = marker_size;
-	return xdl_merge(orig,
-			 src1, name1,
-			 src2, name2,
-			 &xmp, XDL_MERGE_FLAGS(XDL_MERGE_ZEALOUS, style, favor),
+	xmp.file1 = name1;
+	xmp.file2 = name2;
+	return xdl_merge(orig, src1, src2, &xmp,
+			 XDL_MERGE_FLAGS(XDL_MERGE_ZEALOUS, style, favor),
 			 result);
 }

xdiff/xdiff.h +3 -2

index fd89b9a..82d7fc7 100644
@@ -118,12 +118,13 @@ typedef struct s_xmparam {
 	xpparam_t xpp;
 	int marker_size;
 	const char *ancestor;	/* label for orig */
+	const char *file1;	/* label for mf1 */
+	const char *file2;	/* label for mf2 */
 } xmparam_t;
 
 #define DEFAULT_CONFLICT_MARKER_SIZE 7
 
-int xdl_merge(mmfile_t *orig, mmfile_t *mf1, const char *name1,
-		mmfile_t *mf2, const char *name2,
+int xdl_merge(mmfile_t *orig, mmfile_t *mf1, mmfile_t *mf2,
 		xmparam_t const *xmp, int flags, mmbuffer_t *result);
 
 #ifdef __cplusplus

xdiff/xmerge.c +7 -6

index 3a0ae14..241f7ba 100644
@@ -401,12 +401,14 @@ static int xdl_simplify_non_conflicts(xdfenv_t *xe1, xdmerge_t *m,
  *
  * returns < 0 on error, == 0 for no conflicts, else number of conflicts
  */
-static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1,
-		xdfenv_t *xe2, xdchange_t *xscr2, const char *name2,
+static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1,
+		xdfenv_t *xe2, xdchange_t *xscr2,
 		int flags, xmparam_t const *xmp, mmbuffer_t *result) {
 	xdmerge_t *changes, *c;
 	xpparam_t const *xpp = &xmp->xpp;
 	const char * const ancestor_name = xmp->ancestor;
+	const char * const name1 = xmp->file1;
+	const char * const name2 = xmp->file2;
 	int i0, i1, i2, chg0, chg1, chg2;
 	int level = flags & XDL_MERGE_LEVEL_MASK;
 	int style = flags & XDL_MERGE_STYLE_MASK;

@@ -560,8 +562,7 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1,
 	return xdl_cleanup_merge(changes);
 }
 
-int xdl_merge(mmfile_t *orig, mmfile_t *mf1, const char *name1,
-		mmfile_t *mf2, const char *name2,
+int xdl_merge(mmfile_t *orig, mmfile_t *mf1, mmfile_t *mf2,
 		xmparam_t const *xmp, int flags, mmbuffer_t *result) {
 	xdchange_t *xscr1, *xscr2;
 	xdfenv_t xe1, xe2;

@@ -597,8 +598,8 @@ int xdl_merge(mmfile_t *orig, mmfile_t *mf1, const char *name1,
 		memcpy(result->ptr, mf1->ptr, mf1->size);
 		result->size = mf1->size;
 	} else {
-		status = xdl_do_merge(&xe1, xscr1, name1,
-				      &xe2, xscr2, name2,
+		status = xdl_do_merge(&xe1, xscr1,
+				      &xe2, xscr2,
 				      flags, xmp, result);
 	}
 	xdl_free_script(xscr1);

Signature · 1 line

1.7.0

Jonathan Nieder

author

2016-06-15 22:48 UTC #5 lore raw

PATCH v2 3/16 merge-file --diff3: add a label for ancestor

In reply to Jonathan Nieder

git merge-file --diff3 can be used to present conflicts hunks including text from the common ancestor.

The added information is helpful for resolving a merge by hand, and merge tools can usually grok it because it looks like output from diff3 -m. However, ‘diff3’ includes a label for the merge base on the

||||||| line and some tools cannot parse conflict hunks without such a
label.  Write the base-name as passed in a -L option (or the name of

the ancestor file by default) on that line.

git rerere will not have trouble parsing this output, since instead of looking for a newline, it looks for whitespace after the ||||||| marker. Since rerere includes its own code for recreating conflict hunks, conflict identifiers are unaffected. No other code in git tries to parse conflict hunks.

Requested-by Stefan Monnier <[email protected]>

Signed-off-by Jonathan Nieder <[email protected]>

---
t/t6023-merge-files.sh needs another change when merging to next.

@@ -225,7 +225,7 @@ et nihil mihi deerit;

In loco pascuae ibi me collocavit;
super aquam refectionis educavit me.

-||||||||||
+|||||||||| new5.txt

et nihil mihi deerit.
In loco pascuae ibi me collocavit,
super aquam refectionis educavit me;

 builtin/merge-file.c  |    1 +
 t/t6023-merge-file.sh |    2 +-
 2 files changed, 2 insertions(+), 1 deletions(-)

builtin/merge-file.c +1 -0

index ade1e8b..04cb67a 100644
@@ -72,6 +72,7 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
 					argv[i]);
 	}
 
+	xmp.ancestor = names[1];
 	xmp.file1 = names[0];
 	xmp.file2 = names[2];
 	ret = xdl_merge(mmfs + 1, mmfs + 0, mmfs + 2, &xmp,

t/t6023-merge-file.sh +1 -1

index d605024..16c3c69 100755
@@ -181,7 +181,7 @@ et nihil mihi deerit;
 
 In loco pascuae ibi me collocavit;
 super aquam refectionis educavit me.
-|||||||
+||||||| new5.txt
 et nihil mihi deerit.
 In loco pascuae ibi me collocavit,
 super aquam refectionis educavit me;

Signature · 1 line

1.7.0

Jonathan Nieder

author

2016-06-15 22:48 UTC #6 lore raw

PATCH v2 4/16 ll_merge(): add ancestor label parameter for diff3-style output

In reply to Jonathan Nieder

Various commands using the ll_merge() function present conflict hunks in output something like what ‘diff3 -m’ produces if the merge.conflictstyle configuration option is set to diff3. The output leaves out the name of the merge base on the ||||||| line of the output, and some tools misparse the conflict hunks without that.

Add a new ancestor_label parameter to ll_merge() to give callers the power to rectify this situation. All callers pass NULL for now to avoid distracting changes in outward behavior when testing this interface change.

Requested-by Stefan Monnier <[email protected]>

Signed-off-by Jonathan Nieder <[email protected]>

---
change from v1:

--- b/ll-merge.c
+++ b/ll-merge.c
@@ -108,7 +108,7 @@ static int ll_union_merge(const struct ll_merge_dr

saved_style = git_xmerge_style; git_xmerge_style = 0;
status = ll_xdl_merge(drv_unused, result, path_unused,
- orig, orig_name, src1, NULL, src2, NULL,
+ orig, NULL, src1, NULL, src2, NULL,

                              flag, marker_size);
        git_xmerge_style = saved_style;
        if (status <= 0)

There is no possibility of a diff3-style merge in that codepath.

 builtin/checkout.c |    2 +-
 ll-merge.c         |   20 +++++++++++---------
 ll-merge.h         |    2 +-
 merge-file.c       |    2 +-
 merge-recursive.c  |    2 +-
 rerere.c           |    4 ++--
 6 files changed, 17 insertions(+), 15 deletions(-)

builtin/checkout.c +1 -1

index acefaaf..d67f809 100644
@@ -149,7 +149,7 @@ static int checkout_merged(int pos, struct checkout *state)
 	read_mmblob(&ours, active_cache[pos+1]->sha1);
 	read_mmblob(&theirs, active_cache[pos+2]->sha1);
 
-	status = ll_merge(&result_buf, path, &ancestor,
+	status = ll_merge(&result_buf, path, &ancestor, NULL,
 			  &ours, "ours", &theirs, "theirs", 0);
 	free(ancestor.ptr);
 	free(ours.ptr);

ll-merge.c +11 -9

index b8e8971..92670af 100644
@@ -15,7 +15,7 @@ struct ll_merge_driver;
 typedef int (*ll_merge_fn)(const struct ll_merge_driver *,
 			   mmbuffer_t *result,
 			   const char *path,
-			   mmfile_t *orig,
+			   mmfile_t *orig, const char *orig_name,
 			   mmfile_t *src1, const char *name1,
 			   mmfile_t *src2, const char *name2,
 			   int flag,

@@ -36,7 +36,7 @@ struct ll_merge_driver {
 static int ll_binary_merge(const struct ll_merge_driver *drv_unused,
 			   mmbuffer_t *result,
 			   const char *path_unused,
-			   mmfile_t *orig,
+			   mmfile_t *orig, const char *orig_name,
 			   mmfile_t *src1, const char *name1,
 			   mmfile_t *src2, const char *name2,
 			   int flag, int marker_size)

@@ -57,7 +57,7 @@ static int ll_binary_merge(const struct ll_merge_driver *drv_unused,
 static int ll_xdl_merge(const struct ll_merge_driver *drv_unused,
 			mmbuffer_t *result,
 			const char *path,
-			mmfile_t *orig,
+			mmfile_t *orig, const char *orig_name,
 			mmfile_t *src1, const char *name1,
 			mmfile_t *src2, const char *name2,
 			int flag, int marker_size)

@@ -73,7 +73,8 @@ static int ll_xdl_merge(const struct ll_merge_driver *drv_unused,
 			path, name1, name2);
 		return ll_binary_merge(drv_unused, result,
 				       path,
-				       orig, src1, name1,
+				       orig, orig_name,
+				       src1, name1,
 				       src2, name2,
 				       flag, marker_size);
 	}

@@ -83,6 +84,7 @@ static int ll_xdl_merge(const struct ll_merge_driver *drv_unused,
 		style = git_xmerge_style;
 	if (marker_size > 0)
 		xmp.marker_size = marker_size;
+	xmp.ancestor = orig_name;
 	xmp.file1 = name1;
 	xmp.file2 = name2;
 	return xdl_merge(orig, src1, src2, &xmp,

@@ -93,7 +95,7 @@ static int ll_xdl_merge(const struct ll_merge_driver *drv_unused,
 static int ll_union_merge(const struct ll_merge_driver *drv_unused,
 			  mmbuffer_t *result,
 			  const char *path_unused,
-			  mmfile_t *orig,
+			  mmfile_t *orig, const char *orig_name,
 			  mmfile_t *src1, const char *name1,
 			  mmfile_t *src2, const char *name2,
 			  int flag, int marker_size)

@@ -106,7 +108,7 @@ static int ll_union_merge(const struct ll_merge_driver *drv_unused,
 	saved_style = git_xmerge_style;
 	git_xmerge_style = 0;
 	status = ll_xdl_merge(drv_unused, result, path_unused,
-			      orig, src1, NULL, src2, NULL,
+			      orig, NULL, src1, NULL, src2, NULL,
 			      flag, marker_size);
 	git_xmerge_style = saved_style;
 	if (status <= 0)

@@ -165,7 +167,7 @@ static void create_temp(mmfile_t *src, char *path)
 static int ll_ext_merge(const struct ll_merge_driver *fn,
 			mmbuffer_t *result,
 			const char *path,
-			mmfile_t *orig,
+			mmfile_t *orig, const char *orig_name,
 			mmfile_t *src1, const char *name1,
 			mmfile_t *src2, const char *name2,
 			int flag, int marker_size)

@@ -356,7 +358,7 @@ static int git_path_check_merge(const char *path, struct git_attr_check check[2]
 
 int ll_merge(mmbuffer_t *result_buf,
 	     const char *path,
-	     mmfile_t *ancestor,
+	     mmfile_t *ancestor, const char *ancestor_label,
 	     mmfile_t *ours, const char *our_label,
 	     mmfile_t *theirs, const char *their_label,
 	     int flag)

@@ -378,7 +380,7 @@ int ll_merge(mmbuffer_t *result_buf,
 	driver = find_ll_merge_driver(ll_driver_name);
 	if (virtual_ancestor && driver->recursive)
 		driver = find_ll_merge_driver(driver->recursive);
-	return driver->fn(driver, result_buf, path, ancestor,
+	return driver->fn(driver, result_buf, path, ancestor, ancestor_label,
 			  ours, our_label, theirs, their_label,
 			  flag, marker_size);
 }

ll-merge.h +1 -1

index 5788922..57754cc 100644
@@ -7,7 +7,7 @@
 
 int ll_merge(mmbuffer_t *result_buf,
 	     const char *path,
-	     mmfile_t *ancestor,
+	     mmfile_t *ancestor, const char *ancestor_label,
 	     mmfile_t *ours, const char *our_label,
 	     mmfile_t *theirs, const char *their_label,
 	     int flag);

merge-file.c +1 -1

index fd34d76..07cc0c9 100644
@@ -30,7 +30,7 @@ static void *three_way_filemerge(const char *path, mmfile_t *base, mmfile_t *our
 	int merge_status;
 	mmbuffer_t res;
 
-	merge_status = ll_merge(&res, path, base,
+	merge_status = ll_merge(&res, path, base, NULL,
 				our, ".our", their, ".their", 0);
 	if (merge_status < 0)
 		return NULL;

merge-recursive.c +1 -1

index 195ebf9..3b2cc9d 100644
@@ -640,7 +640,7 @@ static int merge_3way(struct merge_options *o,
 	read_mmblob(&src1, a->sha1);
 	read_mmblob(&src2, b->sha1);
 
-	merge_status = ll_merge(result_buf, a->path, &orig,
+	merge_status = ll_merge(result_buf, a->path, &orig, NULL,
 				&src1, name1, &src2, name2,
 				(!!o->call_depth) | (favor << 1));

rerere.c +2 -2

index a59f74f..f221bed 100644
@@ -319,7 +319,7 @@ static int handle_cache(const char *path, unsigned char *sha1, const char *outpu
 		if (!mmfile[i].ptr && !mmfile[i].size)
 			mmfile[i].ptr = xstrdup("");
 	}
-	ll_merge(&result, path, &mmfile[0],
+	ll_merge(&result, path, &mmfile[0], NULL,
 		 &mmfile[1], "ours",
 		 &mmfile[2], "theirs", 0);
 	for (i = 0; i < 3; i++)

@@ -376,7 +376,7 @@ static int merge(const char *name, const char *path)
 		ret = 1;
 		goto out;
 	}
-	ret = ll_merge(&result, path, &base, &cur, "", &other, "", 0);
+	ret = ll_merge(&result, path, &base, NULL, &cur, "", &other, "", 0);
 	if (!ret) {
 		FILE *f = fopen(path, "w");
 		if (!f)

Signature · 1 line

1.7.0

Jonathan Nieder

author

2016-06-15 22:48 UTC #7 lore raw

PATCH v2 5/16 checkout --conflict=diff3: add a label for ancestor

In reply to Jonathan Nieder

git checkout --conflict=diff3 can be used to present conflicts hunks including text from the common ancestor:

	<<<<<<< ours
	ourside
	|||||||
	original
	=======
	theirside
	>>>>>>> theirs

The added information is helpful for resolving a merge by hand, and merge tools can usually understand it without trouble because it looks like output from ‘diff3 -m’.

Unfortunately, not all can: diff3 includes a label for the merge base on the ||||||| line and it seems some tools cannot parse conflict hunks without such a label. Humans could use help in interpreting the output, too. So mark the start of the text from the common ancestor with the label “||||||| base”.

git rerere’s conflict identifiers are not affected: to parse conflict hunks, rerere looks for whitespace after the ||||||| marker rather than a newline, and to compute preimage ids, rerere has its own code for creating conflict hunks. No other code in git tries to parse conflict hunks.

Requested-by Stefan Monnier <[email protected]>

Signed-off-by Jonathan Nieder <[email protected]>

---
Label changed. Thanks for the feedback.

 builtin/checkout.c |    2 +-
 t/t7201-co.sh      |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

builtin/checkout.c +1 -1

index d67f809..d652b4c 100644
@@ -149,7 +149,7 @@ static int checkout_merged(int pos, struct checkout *state)
 	read_mmblob(&ours, active_cache[pos+1]->sha1);
 	read_mmblob(&theirs, active_cache[pos+2]->sha1);
 
-	status = ll_merge(&result_buf, path, &ancestor, NULL,
+	status = ll_merge(&result_buf, path, &ancestor, "base",
 			  &ours, "ours", &theirs, "theirs", 0);
 	free(ancestor.ptr);
 	free(ours.ptr);

t/t7201-co.sh +2 -2

index d20ed61..981ae8f 100755
@@ -481,7 +481,7 @@ test_expect_success 'checkout with --merge, in diff3 -m style' '
 	(
 		echo "<<<<<<< ours"
 		echo ourside
-		echo "|||||||"
+		echo "||||||| base"
 		echo original
 		echo "======="
 		echo theirside

@@ -525,7 +525,7 @@ test_expect_success 'checkout --conflict=diff3' '
 	(
 		echo "<<<<<<< ours"
 		echo ourside
-		echo "|||||||"
+		echo "||||||| base"
 		echo original
 		echo "======="
 		echo theirside

Signature · 1 line

1.7.0

Jonathan Nieder

author

2016-06-15 22:48 UTC #8 lore raw

PATCH v2 6/16 merge_file(): add comment explaining behavior wrt conflict style

In reply to Jonathan Nieder

The merge_file() function is a helper for ‘git read-tree’, which does not respect the merge.conflictstyle option, so there is no need to worry about what ancestor_name it should pass to ll_merge(). Add a comment to this effect.

Signed-off-by Jonathan Nieder <[email protected]>

---

 merge-file.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

merge-file.c +6 -0

index 07cc0c9..c336c93 100644
@@ -30,6 +30,12 @@ static void *three_way_filemerge(const char *path, mmfile_t *base, mmfile_t *our
 	int merge_status;
 	mmbuffer_t res;
 
+	/*
+	 * This function is only used by cmd_merge_tree, which
+	 * does not respect the merge.conflictstyle option.
+	 * There is no need to worry about a label for the
+	 * common ancestor.
+	 */
 	merge_status = ll_merge(&res, path, base, NULL,
 				our, ".our", their, ".their", 0);
 	if (merge_status < 0)

Signature · 1 line

1.7.0

Jonathan Nieder

author

2016-06-15 22:48 UTC #9 lore raw

PATCH v2 7/16 merge_trees(): add ancestor label parameter for diff3-style output

In reply to Jonathan Nieder

Commands using the merge_trees() machinery will present conflict hunks in output something like what ‘diff3 -m’ produces if the merge.conflictstyle configuration option is set to diff3. The output lacks the name of the merge base on the ||||||| line of the output, and tools can misparse the conflict hunks without it. Add a new o->ancestor parameter to merge_trees() for use as a label for the ancestor in conflict hunks.

If o->ancestor is NULL, the output format is as before. All callers pass NULL, so this patch does not affect the outward behavior of git.

If o->ancestor is non-NULL and both branches renamed the base file to the same name, that name is included in the conflict hunk labels. Even if o->ancestor is NULL I think this would be a good change, but this patch only does it in the non-NULL case to ensure the output format does not change where it might matter.

Requested-by Stefan Monnier <[email protected]>

Signed-off-by Jonathan Nieder <[email protected]>

---
Only the commit message was changed.

 merge-recursive.c |   11 ++++++++---
 merge-recursive.h |    1 +
 2 files changed, 9 insertions(+), 3 deletions(-)

merge-recursive.c +8 -3

index 3b2cc9d..017cafd 100644
@@ -608,7 +608,7 @@ static int merge_3way(struct merge_options *o,
 		      const char *branch2)
 {
 	mmfile_t orig, src1, src2;
-	char *name1, *name2;
+	char *base_name, *name1, *name2;
 	int merge_status;
 	int favor;

@@ -628,10 +628,15 @@ static int merge_3way(struct merge_options *o,
 		}
 	}
 
-	if (strcmp(a->path, b->path)) {
+	if (strcmp(a->path, b->path) ||
+	    (o->ancestor != NULL && strcmp(a->path, one->path) != 0)) {
+		base_name = o->ancestor == NULL ? NULL :
+			xstrdup(mkpath("%s:%s", o->ancestor, one->path));
 		name1 = xstrdup(mkpath("%s:%s", branch1, a->path));
 		name2 = xstrdup(mkpath("%s:%s", branch2, b->path));
 	} else {
+		base_name = o->ancestor == NULL ? NULL :
+			xstrdup(mkpath("%s", o->ancestor));
 		name1 = xstrdup(mkpath("%s", branch1));
 		name2 = xstrdup(mkpath("%s", branch2));
 	}

@@ -640,7 +645,7 @@ static int merge_3way(struct merge_options *o,
 	read_mmblob(&src1, a->sha1);
 	read_mmblob(&src2, b->sha1);
 
-	merge_status = ll_merge(result_buf, a->path, &orig, NULL,
+	merge_status = ll_merge(result_buf, a->path, &orig, base_name,
 				&src1, name1, &src2, name2,
 				(!!o->call_depth) | (favor << 1));

merge-recursive.h +1 -0

index be8410a..d1192f5 100644
@@ -4,6 +4,7 @@
 #include "string-list.h"
 
 struct merge_options {
+	const char *ancestor;
 	const char *branch1;
 	const char *branch2;
 	enum {

Signature · 1 line

1.7.0

Jonathan Nieder

author

2016-06-15 22:48 UTC #10 lore raw

PATCH v2 8/16 tests: document format of conflicts from checkout -m

In reply to Jonathan Nieder

We are about to change the format of the conflict hunks that ‘checkout --merge’ writes. Add tests checking the current behavior first.

Signed-off-by Jonathan Nieder <[email protected]>

---
Unchanged.

 t/t7201-co.sh |   65 +++++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 61 insertions(+), 4 deletions(-)

t/t7201-co.sh +61 -4

index 981ae8f..f3f0c4c 100755
@@ -11,10 +11,12 @@ Test switching across them.
   ! [master] Initial A one, A two
    * [renamer] Renamer R one->uno, M two
     ! [side] Side M one, D two, A three
-  ---
-    + [side] Side M one, D two, A three
-   *  [renamer] Renamer R one->uno, M two
-  +*+ [master] Initial A one, A two
+     ! [simple] Simple D one, M two
+  ----
+     + [simple] Simple D one, M two
+    +  [side] Side M one, D two, A three
+   *   [renamer] Renamer R one->uno, M two
+  +*++ [master] Initial A one, A two
 
 '

@@ -52,6 +54,11 @@ test_expect_success setup '
 	git update-index --add --remove one two three &&
 	git commit -m "Side M one, D two, A three" &&
 
+	git checkout -b simple master &&
+	rm -f one &&
+	fill a c e > two &&
+	git commit -a -m "Simple D one, M two" &&
+
 	git checkout master
 '

@@ -166,6 +173,56 @@ test_expect_success 'checkout -m with merge conflict' '
 	! test -s current
 '
 
+test_expect_success 'format of merge conflict from checkout -m' '
+
+	git checkout -f master && git clean -f &&
+
+	fill b d > two &&
+	git checkout -m simple &&
+
+	git ls-files >current &&
+	fill same two two two >expect &&
+	test_cmp current expect &&
+
+	cat <<-EOF >expect &&
+	<<<<<<< simple
+	a
+	c
+	e
+	=======
+	b
+	d
+	>>>>>>> local
+	EOF
+	test_cmp two expect
+'
+
+test_expect_success 'checkout --merge --conflict=diff3 <branch>' '
+
+	git checkout -f master && git reset --hard && git clean -f &&
+
+	fill b d > two &&
+	git checkout --merge --conflict=diff3 simple &&
+
+	cat <<-EOF >expect &&
+	<<<<<<< simple
+	a
+	c
+	e
+	|||||||
+	a
+	b
+	c
+	d
+	e
+	=======
+	b
+	d
+	>>>>>>> local
+	EOF
+	test_cmp two expect
+'
+
 test_expect_success 'checkout to detach HEAD (with advice declined)' '
 
 	git config advice.detachedHead false &&

Signature · 1 line

1.7.0

Jonathan Nieder

author

2016-06-15 22:48 UTC #11 lore raw

PATCH v2 9/16 checkout -m --conflict=diff3: add a label for ancestor

In reply to Jonathan Nieder

git checkout --merge --conflict=diff3 can be used to present conflict hunks including text from the common ancestor. The added information can be helpful for resolving a merge by hand, and merge tools tend to understand it because it is very similar to what ‘diff3 -m’ produces.

Unlike current git, diff3 -m includes a label for the merge base on the ||||||| line, and unfortunately, some tools cannot parse the conflict hunks without it. Humans can benefit from a cue when learning to interpreting the format, too. Mark the start of the text from the old branch with a label based on the branch’s name.

git rerere does not have trouble parsing this output and its preimage ids are unchanged since it includes its own code for recreating conflict hunks. No other code in git tries to parse conflict hunks.

Requested-by Stefan Monnier <[email protected]>

Signed-off-by Jonathan Nieder <[email protected]>

---
Unchanged.

 builtin/checkout.c |    1 +
 t/t7201-co.sh      |    2 +-
 2 files changed, 2 insertions(+), 1 deletions(-)

builtin/checkout.c +1 -0

index d652b4c..88b1f43 100644
@@ -439,6 +439,7 @@ static int merge_working_tree(struct checkout_opts *opts,
 			ret = reset_tree(new->commit->tree, opts, 1);
 			if (ret)
 				return ret;
+			o.ancestor = old->name;
 			o.branch1 = new->name;
 			o.branch2 = "local";
 			merge_trees(&o, new->commit->tree, work,

t/t7201-co.sh +1 -1

index f3f0c4c..1337fa5 100755
@@ -209,7 +209,7 @@ test_expect_success 'checkout --merge --conflict=diff3 <branch>' '
 	a
 	c
 	e
-	|||||||
+	||||||| master
 	a
 	b
 	c

Signature · 1 line

1.7.0

Jonathan Nieder

author

2016-06-15 22:48 UTC #12 lore raw

PATCH v2 10/16 tests: document cherry-pick behavior in face of conflicts

In reply to Jonathan Nieder

We are about to change the format of the conflict hunks that cherry-pick and revert write. Add tests checking the current behavior first.

Signed-off-by Jonathan Nieder <[email protected]>

---
 t/t3507-cherry-pick-conflict.sh |  198 +++++++++++++++++++++++++++++++++++++++
 1 files changed, 198 insertions(+), 0 deletions(-)
 create mode 100644 t/t3507-cherry-pick-conflict.sh

t/t3507-cherry-pick-conflict.sh new file +198 -0

new file mode 100644
index 0000000..e856356
@@ -0,0 +1,198 @@
+#!/bin/sh
+
+test_description='test cherry-pick and revert with conflicts
+
+  -
+  + picked: rewrites foo to c
+  + base: rewrites foo to b
+  + initial: writes foo as a, unrelated as unrelated
+
+'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+
+	echo unrelated >unrelated &&
+	git add unrelated &&
+	test_commit initial foo a &&
+	test_commit base foo b &&
+	test_commit picked foo c &&
+	git config advice.detachedhead false
+
+'
+
+test_expect_success 'failed cherry-pick does not advance HEAD' '
+
+	git checkout -f initial^0 &&
+	git read-tree -u --reset HEAD &&
+	git clean -d -f -f -q -x &&
+
+	git update-index --refresh &&
+	git diff-index --exit-code HEAD &&
+
+	head=$(git rev-parse HEAD) &&
+	test_must_fail git cherry-pick picked &&
+	newhead=$(git rev-parse HEAD) &&
+
+	test "$head" = "$newhead"
+'
+
+test_expect_success 'failed cherry-pick produces dirty index' '
+
+	git checkout -f initial^0 &&
+	git read-tree -u --reset HEAD &&
+	git clean -d -f -f -q -x &&
+
+	git update-index --refresh &&
+	git diff-index --exit-code HEAD &&
+
+	test_must_fail git cherry-pick picked &&
+
+	test_must_fail git update-index --refresh -q &&
+	test_must_fail git diff-index --exit-code HEAD
+'
+
+test_expect_success 'failed cherry-pick registers participants in index' '
+
+	git read-tree -u --reset HEAD &&
+	git clean -d -f -f -q -x &&
+	{
+		git checkout base -- foo &&
+		git ls-files --stage foo &&
+		git checkout initial -- foo &&
+		git ls-files --stage foo &&
+		git checkout picked -- foo &&
+		git ls-files --stage foo
+	} > stages &&
+	sed "
+		1 s/ 0	/ 1	/
+		2 s/ 0	/ 2	/
+		3 s/ 0	/ 3	/
+	" < stages > expected &&
+	git checkout -f initial^0 &&
+
+	git update-index --refresh &&
+	git diff-index --exit-code HEAD &&
+
+	test_must_fail git cherry-pick picked &&
+	git ls-files --stage --unmerged > actual &&
+
+	test_cmp expected actual
+'
+
+test_expect_success 'failed cherry-pick describes conflict in work tree' '
+
+	git checkout -f initial^0 &&
+	git read-tree -u --reset HEAD &&
+	git clean -d -f -f -q -x &&
+	cat <<-EOF > expected &&
+	<<<<<<< HEAD
+	a
+	=======
+	c
+	>>>>>>> objid picked
+	EOF
+
+	git update-index --refresh &&
+	git diff-index --exit-code HEAD &&
+
+	test_must_fail git cherry-pick picked &&
+
+	sed "s/[a-f0-9]*\.\.\./objid/" foo > actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'diff3 -m style' '
+
+	git config merge.conflictstyle diff3 &&
+	git checkout -f initial^0 &&
+	git read-tree -u --reset HEAD &&
+	git clean -d -f -f -q -x &&
+	cat <<-EOF > expected &&
+	<<<<<<< HEAD
+	a
+	|||||||
+	b
+	=======
+	c
+	>>>>>>> objid picked
+	EOF
+
+	git update-index --refresh &&
+	git diff-index --exit-code HEAD &&
+
+	test_must_fail git cherry-pick picked &&
+
+	sed "s/[a-f0-9]*\.\.\./objid/" foo > actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'revert also handles conflicts sanely' '
+
+	git config --unset merge.conflictstyle &&
+	git read-tree -u --reset HEAD &&
+	git clean -d -f -f -q -x &&
+	cat <<-EOF > expected &&
+	<<<<<<< HEAD
+	a
+	=======
+	b
+	>>>>>>> objid picked
+	EOF
+	{
+		git checkout picked -- foo &&
+		git ls-files --stage foo &&
+		git checkout initial -- foo &&
+		git ls-files --stage foo &&
+		git checkout base -- foo &&
+		git ls-files --stage foo
+	} > stages &&
+	sed "
+		1 s/ 0	/ 1	/
+		2 s/ 0	/ 2	/
+		3 s/ 0	/ 3	/
+	" < stages > expected-stages &&
+	git checkout -f initial^0 &&
+
+	git update-index --refresh &&
+	git diff-index --exit-code HEAD &&
+
+	head=$(git rev-parse HEAD) &&
+	test_must_fail git revert picked &&
+	newhead=$(git rev-parse HEAD) &&
+	git ls-files --stage --unmerged > actual-stages &&
+
+	test "$head" = "$newhead" &&
+	test_must_fail git update-index --refresh -q &&
+	test_must_fail git diff-index --exit-code HEAD &&
+	test_cmp expected-stages actual-stages &&
+	sed "s/[a-f0-9]*\.\.\./objid/" foo > actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'revert conflict, diff3 -m style' '
+	git config merge.conflictstyle diff3 &&
+	git checkout -f initial^0 &&
+	git read-tree -u --reset HEAD &&
+	git clean -d -f -f -q -x &&
+	cat <<-EOF > expected &&
+	<<<<<<< HEAD
+	a
+	|||||||
+	c
+	=======
+	b
+	>>>>>>> objid picked
+	EOF
+
+	git update-index --refresh &&
+	git diff-index --exit-code HEAD &&
+
+	test_must_fail git revert picked &&
+
+	sed "s/[a-f0-9]*\.\.\./objid/" foo > actual &&
+	test_cmp expected actual
+'
+
+test_done

Signature · 1 line

1.7.0

Jonathan Nieder

author

2016-06-15 22:48 UTC #13 lore raw

PATCH v2 11/16 revert_or_cherry_pick(): get oneline_body from get_oneline()

In reply to Jonathan Nieder

Currently the one-line commit message for the cherry-picked commit is found with strchr(get_oneline(), ' '). That wastes a few CPU cycles, and more importantly, it makes it difficult to tweak the output from get_oneline() later. Teach get_oneline() to store the one-line commit message using a caller-supplied pointer to avoid these problems.

Signed-off-by Jonathan Nieder <[email protected]>

---
Maybe the next few patches should be squashed. They are separated one commit per idea, but they affect the same code.

 builtin/revert.c |   11 +++++------
 1 files changed, 5 insertions(+), 6 deletions(-)

builtin/revert.c +5 -6

index eff5268..4b2042f 100644
@@ -73,7 +73,7 @@ static void parse_args(int argc, const char **argv)
 		exit(1);
 }
 
-static char *get_oneline(const char *message)
+static char *get_oneline(const char *message, char **body)
 {
 	char *result;
 	const char *p = message, *abbrev, *eol;

@@ -99,6 +99,7 @@ static char *get_oneline(const char *message)
 	memcpy(result + abbrev_len, "... ", 4);
 	memcpy(result + abbrev_len + 4, p, oneline_len);
 	result[abbrev_len + 4 + oneline_len] = '\0';
+	*body = result + abbrev_len + 4;
 	return result;
 }

@@ -249,7 +250,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 	unsigned char head[20];
 	struct commit *base, *next, *parent;
 	int i, index_fd, clean;
-	char *oneline, *reencoded_message = NULL;
+	char *oneline, *oneline_body, *reencoded_message = NULL;
 	const char *message, *encoding;
 	char *defmsg = git_pathdup("MERGE_MSG");
 	struct merge_options o;

@@ -341,15 +342,13 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 					git_commit_encoding, encoding)))
 		message = reencoded_message;
 
-	oneline = get_oneline(message);
+	oneline = get_oneline(message, &oneline_body);
 
 	if (action == REVERT) {
-		char *oneline_body = strchr(oneline, ' ');
-
 		base = commit;
 		next = parent;
 		add_to_msg("Revert \"");
-		add_to_msg(oneline_body + 1);
+		add_to_msg(oneline_body);
 		add_to_msg("\"\n\nThis reverts commit ");
 		add_to_msg(sha1_to_hex(commit->object.sha1));

Signature · 1 line

1.7.0

Stephen Boyd

2016-06-15 22:48 UTC #14 lore raw

Re: [PATCH 11/16] revert_or_cherry_pick(): get oneline_body from get_oneline()

Quoted 4 lines · Jonathan Nieder

Maybe the next few patches should be squashed. They are separated one commit per idea, but they affect the same code.

Could all this code just use the custom format printing routines in pretty.c? It might not be super efficient but at least it wouldn't duplicate a bunch of code.

Jonathan Nieder

author

2016-06-15 22:48 UTC #15 lore raw

Re: [PATCH 11/16] revert_or_cherry_pick(): get oneline_body from get_oneline()

Quoted 3 lines · Stephen Boyd

Could all this code just use the custom format printing routines in pretty.c? It might not be super efficient but at least it wouldn't duplicate a bunch of code.

I like this idea, but I wasn’t able to pull it off. You see, cmd_revert() uses a few substrings from a single string it generates:

parent of ca87ef9... Do something very imporant

 ^         ^          ^
 1         2          3

These are put together like so:

For the commit message:

Subject: Revert "[3]Do something very important"

This reverts commit ca87ef90000000000000000000000000000000000.

For the conflict markers:

<<<<<<< HEAD
Version from the tip of the current branch. ||||||| [2]ca87ef9... Do something very important Version from the commit to be reverted.
=======
Version from the parent of the commit to be reverted.

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
[1]parent of ca87ef9... Do something very important

Nevertheless, investigating it did spur me to simplify this part of the series a little. I think the following two patches could replace patches 11, 13, 14, and 15 from this thread. I do not want to do any drastic refactoring without libifying this code, though, and that should wait for another series, I think.

Thanks for the feedback.

Jonathan Nieder (2):
revert: clarify label on conflict hunks
cherry-pick, revert: add a label for ancestor

 builtin/revert.c                |  129 +++++++++++++++++++++++----------------
 t/t3507-cherry-pick-conflict.sh |    8 +-
 2 files changed, 80 insertions(+), 57 deletions(-)

Jonathan Nieder

author

2016-06-15 22:48 UTC #16 lore raw

PATCH v2 1/1 revert: clarify label on conflict hunks

When reverting a commit, the commit being merged is not the commit to revert itself but its parent. Clarify the text in conflict hunks to explain this.

Example:

	<<<<<<< HEAD
	Something old.
	Something new.
	Something blue.
	=======
	Something old.
	Something rotten.
	Something blue.
	>>>>>>> parent of ac789a9... Remove rotten line

Also free a temporary buffer that was not freed before. This is not important because it is a small one-time allocation. It would become a memory leak if unnoticed when libifying cherry-pick.

Signed-off-by Jonathan Nieder <[email protected]>

---
This rearranges the code a little to make it easier to read and manipulate.

 builtin/revert.c                |  100 ++++++++++++++++++++++++---------------
 t/t3507-cherry-pick-conflict.sh |    4 +-
 2 files changed, 63 insertions(+), 41 deletions(-)

builtin/revert.c +61 ~~-39~~

index eff5268..b314629 100644
@@ -45,6 +45,8 @@ static const char *me;
 
 #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
 
+static char *get_encoding(const char *message);
+
 static void parse_args(int argc, const char **argv)
 {
 	const char * const * usage_str =

@@ -73,33 +75,63 @@ static void parse_args(int argc, const char **argv)
 		exit(1);
 }
 
-static char *get_oneline(const char *message)
+struct commit_message {
+	char *parent_label;
+	const char *label;
+	const char *subject;
+	char *reencoded_message;
+	const char *message;
+};
+
+static int get_message(const char *raw_message, struct commit_message *out)
 {
-	char *result;
-	const char *p = message, *abbrev, *eol;
+	const char *encoding;
+	const char *p, *abbrev, *eol;
+	char *q;
 	int abbrev_len, oneline_len;
 
-	if (!p)
-		die ("Could not read commit message of %s",
-				sha1_to_hex(commit->object.sha1));
+	if (!raw_message)
+		return -1;
+	encoding = get_encoding(raw_message);
+	if (!encoding)
+		encoding = "UTF-8";
+	if (!git_commit_encoding)
+		git_commit_encoding = "UTF-8";
+	if ((out->reencoded_message = reencode_string(raw_message,
+					git_commit_encoding, encoding)))
+		out->message = out->reencoded_message;
+
+	abbrev = find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV);
+	abbrev_len = strlen(abbrev);
+
+	/* Find beginning and end of commit subject. */
+	p = out->message;
 	while (*p && (*p != '\n' || p[1] != '\n'))
 		p++;
-
 	if (*p) {
 		p += 2;
 		for (eol = p + 1; *eol && *eol != '\n'; eol++)
 			; /* do nothing */
 	} else
 		eol = p;
-	abbrev = find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV);
-	abbrev_len = strlen(abbrev);
 	oneline_len = eol - p;
-	result = xmalloc(abbrev_len + 5 + oneline_len);
-	memcpy(result, abbrev, abbrev_len);
-	memcpy(result + abbrev_len, "... ", 4);
-	memcpy(result + abbrev_len + 4, p, oneline_len);
-	result[abbrev_len + 4 + oneline_len] = '\0';
-	return result;
+
+	out->parent_label = xmalloc(strlen("parent of ") + abbrev_len +
+			      strlen("... ") + oneline_len + 1);
+	q = out->parent_label;
+	q = mempcpy(q, "parent of ", strlen("parent of "));
+	out->label = q;
+	q = mempcpy(q, abbrev, abbrev_len);
+	q = mempcpy(q, "... ", strlen("... "));
+	out->subject = q;
+	q = mempcpy(q, p, oneline_len);
+	*q = '\0';
+	return 0;
+}
+
+static void free_message(struct commit_message *msg) {
+	free(msg->parent_label);
+	free(msg->reencoded_message);
 }
 
 static char *get_encoding(const char *message)

@@ -248,9 +280,10 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 {
 	unsigned char head[20];
 	struct commit *base, *next, *parent;
+	const char *next_label;
 	int i, index_fd, clean;
-	char *oneline, *reencoded_message = NULL;
-	const char *message, *encoding;
+	struct commit_message msg = { NULL, NULL, NULL, NULL, NULL };
+
 	char *defmsg = git_pathdup("MERGE_MSG");
 	struct merge_options o;
 	struct tree *result, *next_tree, *base_tree, *head_tree;

@@ -314,14 +347,14 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 	else
 		parent = commit->parents->item;
 
-	if (!(message = commit->buffer))
-		die ("Cannot get commit message for %s",
-				sha1_to_hex(commit->object.sha1));
-
 	if (parent && parse_commit(parent) < 0)
 		die("%s: cannot parse parent commit %s",
 		    me, sha1_to_hex(parent->object.sha1));
 
+	if (get_message(commit->buffer, &msg) != 0)
+		die ("Cannot get commit message for %s",
+				sha1_to_hex(commit->object.sha1));
+
 	/*
 	 * "commit" is an existing commit.  We would want to apply
 	 * the difference it introduces since its first parent "prev"

@@ -332,24 +365,12 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 	msg_fd = hold_lock_file_for_update(&msg_file, defmsg,
 					   LOCK_DIE_ON_ERROR);
 
-	encoding = get_encoding(message);
-	if (!encoding)
-		encoding = "UTF-8";
-	if (!git_commit_encoding)
-		git_commit_encoding = "UTF-8";
-	if ((reencoded_message = reencode_string(message,
-					git_commit_encoding, encoding)))
-		message = reencoded_message;
-
-	oneline = get_oneline(message);
-
 	if (action == REVERT) {
-		char *oneline_body = strchr(oneline, ' ');
-
 		base = commit;
 		next = parent;
+		next_label = msg.parent_label;
 		add_to_msg("Revert \"");
-		add_to_msg(oneline_body + 1);
+		add_to_msg(msg.subject);
 		add_to_msg("\"\n\nThis reverts commit ");
 		add_to_msg(sha1_to_hex(commit->object.sha1));

@@ -361,8 +382,9 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 	} else {
 		base = parent;
 		next = commit;
-		set_author_ident_env(message);
-		add_message_to_msg(message);
+		next_label = msg.label;
+		set_author_ident_env(msg.message);
+		add_message_to_msg(msg.message);
 		if (no_replay) {
 			add_to_msg("(cherry picked from commit ");
 			add_to_msg(sha1_to_hex(commit->object.sha1));

@@ -373,7 +395,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 	read_cache();
 	init_merge_options(&o);
 	o.branch1 = "HEAD";
-	o.branch2 = oneline;
+	o.branch2 = next ? next_label : "(empty tree)";
 
 	head_tree = parse_tree_indirect(head);
 	next_tree = next ? next->tree : empty_tree();

@@ -437,7 +459,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 		args[i] = NULL;
 		return execv_git_cmd(args);
 	}
-	free(reencoded_message);
+	free_message(&msg);
 	free(defmsg);
 
 	return 0;

t/t3507-cherry-pick-conflict.sh +2 -2

index e856356..6a20817 100644
@@ -138,7 +138,7 @@ test_expect_success 'revert also handles conflicts sanely' '
 	a
 	=======
 	b
-	>>>>>>> objid picked
+	>>>>>>> parent of objid picked
 	EOF
 	{
 		git checkout picked -- foo &&

@@ -183,7 +183,7 @@ test_expect_success 'revert conflict, diff3 -m style' '
 	c
 	=======
 	b
-	>>>>>>> objid picked
+	>>>>>>> parent of objid picked
 	EOF
 
 	git update-index --refresh &&

Signature · 1 line

1.7.0

Jonathan Nieder

author

2016-06-15 22:48 UTC #17 lore raw

PATCH v2 15/16 cherry-pick, revert: add a label for ancestor

In reply to Jonathan Nieder

When writing conflict hunks in ‘diff3 -m’ format, also add a label to the common ancestor. Especially in a cherry-pick, it is not immediately obvious without such a label what the common ancestor represents.

git rerere does not have trouble parsing the new output and its preimage ids are unchanged since it includes its own code for recreating conflict hunks. No other code in git parses conflict hunks.

Requested-by Stefan Monnier <[email protected]>

Signed-off-by Jonathan Nieder <[email protected]>

---
 builtin/revert.c                |    5 ++++-
 t/t3507-cherry-pick-conflict.sh |    4 ++--
 2 files changed, 6 insertions(+), 3 deletions(-)

builtin/revert.c +4 -1

index b314629..625281c 100644
@@ -280,7 +280,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 {
 	unsigned char head[20];
 	struct commit *base, *next, *parent;
-	const char *next_label;
+	const char *base_label, *next_label;
 	int i, index_fd, clean;
 	struct commit_message msg = { NULL, NULL, NULL, NULL, NULL };

@@ -367,6 +367,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 
 	if (action == REVERT) {
 		base = commit;
+		base_label = msg.label;
 		next = parent;
 		next_label = msg.parent_label;
 		add_to_msg("Revert \"");

@@ -381,6 +382,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 		add_to_msg(".\n");
 	} else {
 		base = parent;
+		base_label = msg.parent_label;
 		next = commit;
 		next_label = msg.label;
 		set_author_ident_env(msg.message);

@@ -394,6 +396,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 
 	read_cache();
 	init_merge_options(&o);
+	o.ancestor = base ? base_label : "(empty tree)";
 	o.branch1 = "HEAD";
 	o.branch2 = next ? next_label : "(empty tree)";

t/t3507-cherry-pick-conflict.sh +2 -2

index 6a20817..e25cf80 100644
@@ -112,7 +112,7 @@ test_expect_success 'diff3 -m style' '
 	cat <<-EOF > expected &&
 	<<<<<<< HEAD
 	a
-	|||||||
+	||||||| parent of objid picked
 	b
 	=======
 	c

@@ -179,7 +179,7 @@ test_expect_success 'revert conflict, diff3 -m style' '
 	cat <<-EOF > expected &&
 	<<<<<<< HEAD
 	a
-	|||||||
+	||||||| objid picked
 	c
 	=======
 	b

Signature · 1 line

1.7.0

Jonathan Nieder

author

2016-06-15 22:48 UTC #18 lore raw

PATCH v2 12/16 compat: add mempcpy()

In reply to Jonathan Nieder

The mempcpy() function was added in glibc 2.1. It is quite handy, so add an implementation for cross-platform use.

Signed-off-by Jonathan Nieder <[email protected]>

---
 git-compat-util.h |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

git-compat-util.h +9 -0

index a3c4537..9bed5a0 100644
@@ -331,6 +331,7 @@ extern int git_vsnprintf(char *str, size_t maxsize,
 #ifdef __GLIBC_PREREQ
 #if __GLIBC_PREREQ(2, 1)
 #define HAVE_STRCHRNUL
+#define HAVE_MEMPCPY
 #endif
 #endif

@@ -344,6 +345,14 @@ static inline char *gitstrchrnul(const char *s, int c)
 }
 #endif
 
+#ifndef HAVE_MEMPCPY
+#define mempcpy gitmempcpy
+static inline void *gitmempcpy(void *dest, const void *src, size_t n)
+{
+	return (char *)memcpy(dest, src, n) + n;
+}
+#endif
+
 extern void release_pack_memory(size_t, int);
 
 extern char *xstrdup(const char *str);

Signature · 1 line

1.7.0

Jonathan Nieder

author

2016-06-15 22:48 UTC #19 lore raw

PATCH v2 13/16 revert: simplify get_oneline() using mempcpy

In reply to Jonathan Nieder

Avoid some pointer arithmetic and make future modifications easier by using mempcpy to advance a pointer as the result is written.

While at it, make the code more self-explanatory by using strlen() on constant strings. GCC will compute the length at compile time; I am not sure about other compilers, but this is not performance-critical anyway. It should be simple to add a conststrlen() macro later if needed.

Signed-off-by Jonathan Nieder <[email protected]>

---
 builtin/revert.c |   13 ++++++-------
 1 files changed, 6 insertions(+), 7 deletions(-)

builtin/revert.c +6 -7

index 4b2042f..6a47655 100644
@@ -75,7 +75,7 @@ static void parse_args(int argc, const char **argv)
 
 static char *get_oneline(const char *message, char **body)
 {
-	char *result;
+	char *result, *q;
 	const char *p = message, *abbrev, *eol;
 	int abbrev_len, oneline_len;

@@ -94,12 +94,11 @@ static char *get_oneline(const char *message, char **body)
 	abbrev = find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV);
 	abbrev_len = strlen(abbrev);
 	oneline_len = eol - p;
-	result = xmalloc(abbrev_len + 5 + oneline_len);
-	memcpy(result, abbrev, abbrev_len);
-	memcpy(result + abbrev_len, "... ", 4);
-	memcpy(result + abbrev_len + 4, p, oneline_len);
-	result[abbrev_len + 4 + oneline_len] = '\0';
-	*body = result + abbrev_len + 4;
+	q = result = xmalloc(abbrev_len + strlen("... ") + oneline_len + 1);
+	q = mempcpy(q, abbrev, abbrev_len);
+	*body = q = mempcpy(q, "... ", strlen("... "));
+	q = mempcpy(q, p, oneline_len);
+	*q = '\0';
 	return result;
 }

Signature · 1 line

1.7.0

Jonathan Nieder

author

2016-06-15 22:48 UTC #20 lore raw

PATCH v2 14/16 revert: clarify label on conflict hunks

In reply to Jonathan Nieder

When reverting a commit, the commit being merged is not the commit to revert itself but its parent. Clarify the text in conflict hunks to explain this.

Example:

	<<<<<<< HEAD
	Something old.
	Something new.
	Something blue.
	=======
	Something old.
	Something rotten.
	Something blue.
	>>>>>>> parent of ac789a9... Remove rotten line

Signed-off-by Jonathan Nieder <[email protected]>

---
New patch: a small fix.

 builtin/revert.c                |   14 +++++++++-----
 t/t3507-cherry-pick-conflict.sh |    4 ++--
 2 files changed, 11 insertions(+), 7 deletions(-)

builtin/revert.c +9 -5

index 6a47655..56f2947 100644
@@ -73,11 +73,11 @@ static void parse_args(int argc, const char **argv)
 		exit(1);
 }
 
-static char *get_oneline(const char *message, char **body)
+static char *get_oneline(const char *message, int revert, char **body)
 {
 	char *result, *q;
-	const char *p = message, *abbrev, *eol;
-	int abbrev_len, oneline_len;
+	const char *p = message, *prefix, *abbrev, *eol;
+	int prefix_len, abbrev_len, oneline_len;
 
 	if (!p)
 		die ("Could not read commit message of %s",

@@ -91,10 +91,14 @@ static char *get_oneline(const char *message, char **body)
 			; /* do nothing */
 	} else
 		eol = p;
+	prefix = revert ? "parent of " : "";
+	prefix_len = revert ? strlen("parent of ") : 0;
 	abbrev = find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV);
 	abbrev_len = strlen(abbrev);
 	oneline_len = eol - p;
-	q = result = xmalloc(abbrev_len + strlen("... ") + oneline_len + 1);
+	q = result = xmalloc(prefix_len + abbrev_len +
+			     strlen("... ") + oneline_len + 1);
+	q = mempcpy(q, prefix, prefix_len);
 	q = mempcpy(q, abbrev, abbrev_len);
 	*body = q = mempcpy(q, "... ", strlen("... "));
 	q = mempcpy(q, p, oneline_len);

@@ -341,7 +345,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 					git_commit_encoding, encoding)))
 		message = reencoded_message;
 
-	oneline = get_oneline(message, &oneline_body);
+	oneline = get_oneline(message, action == REVERT, &oneline_body);
 
 	if (action == REVERT) {
 		base = commit;

t/t3507-cherry-pick-conflict.sh +2 -2

index e856356..6a20817 100644
@@ -138,7 +138,7 @@ test_expect_success 'revert also handles conflicts sanely' '
 	a
 	=======
 	b
-	>>>>>>> objid picked
+	>>>>>>> parent of objid picked
 	EOF
 	{
 		git checkout picked -- foo &&

@@ -183,7 +183,7 @@ test_expect_success 'revert conflict, diff3 -m style' '
 	c
 	=======
 	b
-	>>>>>>> objid picked
+	>>>>>>> parent of objid picked
 	EOF
 
 	git update-index --refresh &&

Signature · 1 line

1.7.0

Jonathan Nieder

author

2016-06-15 22:48 UTC #21 lore raw

PATCH v2 15/16 cherry-pick: add a label for ancestor

In reply to Jonathan Nieder

git cherry-pick and git revert will write conflict hunks something like what ‘diff3 -m’ produces if the merge.conflictstyle configuration option is set to diff3. Unlike ‘diff3 -m’, though, the output lacks a label for the merge base on the ||||||| line of the output.

Some tools can misparse the conflict hunks without such a label. Especially in a cherry-pick, humans parsing the conflict hunks by hand might want to know what the common ancestor represents, too. Add a label.

Example:

	<<<<<<< HEAD
	Something old.
	Something new.
	Something rotten.
	Something blue.
	||||||| parent of 387924c... Remove something rotten
	Something old.
	Something rotten.
	Something blue.
	=======
	Something old.
	something blue.
	>>>>>>> 387924c... Remove something rotten

git rerere does not have trouble parsing the new output, and its preimage ids are unchanged since it includes its own code for recreating conflict hunks. No other code in git parses conflict hunks.

Requested-by Stefan Monnier <[email protected]>

Signed-off-by Jonathan Nieder <[email protected]>

---
Hopefully the new labels clearer, especially when reverting a commit. Thoughts?

 builtin/revert.c                |   27 ++++++++++++++++-----------
 t/t3507-cherry-pick-conflict.sh |    4 ++--
 2 files changed, 18 insertions(+), 13 deletions(-)

builtin/revert.c +16 ~~-11~~

index 56f2947..7a39b52 100644
@@ -73,11 +73,11 @@ static void parse_args(int argc, const char **argv)
 		exit(1);
 }
 
-static char *get_oneline(const char *message, int revert, char **body)
+static char *get_oneline(const char *message, char **body, char **parent)
 {
 	char *result, *q;
-	const char *p = message, *prefix, *abbrev, *eol;
-	int prefix_len, abbrev_len, oneline_len;
+	const char *p = message, *abbrev, *eol;
+	int abbrev_len, oneline_len;
 
 	if (!p)
 		die ("Could not read commit message of %s",

@@ -91,14 +91,12 @@ static char *get_oneline(const char *message, int revert, char **body)
 			; /* do nothing */
 	} else
 		eol = p;
-	prefix = revert ? "parent of " : "";
-	prefix_len = revert ? strlen("parent of ") : 0;
 	abbrev = find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV);
 	abbrev_len = strlen(abbrev);
 	oneline_len = eol - p;
-	q = result = xmalloc(prefix_len + abbrev_len +
-			     strlen("... ") + oneline_len + 1);
-	q = mempcpy(q, prefix, prefix_len);
+	*parent = q = xmalloc(strlen("parent of ") + abbrev_len +
+			      strlen("... ") + oneline_len + 1);
+	result = q = mempcpy(q, "parent of ", strlen("parent of "));
 	q = mempcpy(q, abbrev, abbrev_len);
 	*body = q = mempcpy(q, "... ", strlen("... "));
 	q = mempcpy(q, p, oneline_len);

@@ -252,8 +250,10 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 {
 	unsigned char head[20];
 	struct commit *base, *next, *parent;
+	const char *base_label, *next_label;
 	int i, index_fd, clean;
-	char *oneline, *oneline_body, *reencoded_message = NULL;
+	char *oneline, *oneline_body, *parent_label;
+	char *reencoded_message = NULL;
 	const char *message, *encoding;
 	char *defmsg = git_pathdup("MERGE_MSG");
 	struct merge_options o;

@@ -345,11 +345,13 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 					git_commit_encoding, encoding)))
 		message = reencoded_message;
 
-	oneline = get_oneline(message, action == REVERT, &oneline_body);
+	oneline = get_oneline(message, &oneline_body, &parent_label);
 
 	if (action == REVERT) {
 		base = commit;
+		base_label = oneline;
 		next = parent;
+		next_label = parent_label;
 		add_to_msg("Revert \"");
 		add_to_msg(oneline_body);
 		add_to_msg("\"\n\nThis reverts commit ");

@@ -362,7 +364,9 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 		add_to_msg(".\n");
 	} else {
 		base = parent;
+		base_label = parent_label;
 		next = commit;
+		next_label = oneline;
 		set_author_ident_env(message);
 		add_message_to_msg(message);
 		if (no_replay) {

@@ -374,8 +378,9 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 
 	read_cache();
 	init_merge_options(&o);
+	o.ancestor = base ? base_label : "(empty tree)";
 	o.branch1 = "HEAD";
-	o.branch2 = oneline;
+	o.branch2 = next ? next_label : "(empty tree)";
 
 	head_tree = parse_tree_indirect(head);
 	next_tree = next ? next->tree : empty_tree();

t/t3507-cherry-pick-conflict.sh +2 -2

index 6a20817..e25cf80 100644
@@ -112,7 +112,7 @@ test_expect_success 'diff3 -m style' '
 	cat <<-EOF > expected &&
 	<<<<<<< HEAD
 	a
-	|||||||
+	||||||| parent of objid picked
 	b
 	=======
 	c

@@ -179,7 +179,7 @@ test_expect_success 'revert conflict, diff3 -m style' '
 	cat <<-EOF > expected &&
 	<<<<<<< HEAD
 	a
-	|||||||
+	||||||| objid picked
 	c
 	=======
 	b

Signature · 1 line

1.7.0

Jonathan Nieder

author

2016-06-15 22:48 UTC #22 lore raw

PATCH v2 16/16 merge-recursive: add a label for ancestor

In reply to Jonathan Nieder

git merge-recursive (and hence git merge) will present conflict hunks in output something like what ‘diff3 -m’ produces if the merge.conflictstyle configuration option is set to diff3. There is a small difference from diff3: diff3 -m includes a label for the merge base on the ||||||| line.

Tools familiar with the format and humans unfamiliar with the format do a better job with a label. Mark the start of the text from the merge bases with the heading "||||||| merged common ancestors".

It would be nicer to use a more informative label. Perhaps someone will provide one some day.

git rerere does not have trouble parsing the new output, and its preimage ids are unchanged since it has its own code for re-creating conflict hunks. No other code in git parses conflict hunks.

Requested-by Stefan Monnier <[email protected]>

Signed-off-by Jonathan Nieder <[email protected]>

---
As before.

That’s all. I hope you enjoyed the patches and that the added complication for the new cherry-pick messages was not too unpleasant to look at.

Good night,
Jonathan

 merge-recursive.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

merge-recursive.c +1 -0

index 017cafd..917397c 100644
@@ -1347,6 +1347,7 @@ int merge_recursive(struct merge_options *o,
 	if (!o->call_depth)
 		read_cache();
 
+	o->ancestor = "merged common ancestors";
 	clean = merge_trees(o, h1->tree, h2->tree, merged_common_ancestors->tree,
 			    &mrtree);

Signature · 1 line

1.7.0