* [PATCH 01/16] xdl_merge(): add optional ancestor label to diff3-style output
2010-03-17 11:36 [PATCH v2 0/16] Add label for common ancestor to conflictstyle=diff3 Jonathan Nieder
@ 2010-03-17 11:46 ` Jonathan Nieder
2010-03-17 14:22 ` Bert Wesarg
2010-03-17 11:53 ` [PATCH 02/16] xdl_merge(): move file1 and file2 labels to xmparam structure Jonathan Nieder
` (14 subsequent siblings)
15 siblings, 1 reply; 23+ messages in thread
From: Jonathan Nieder @ 2010-03-17 11:46 UTC (permalink / raw)
To: git; +Cc: Stefan Monnier
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 <monnier@iro.umontreal.ca>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
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(-)
diff --git a/xdiff/xdiff.h b/xdiff/xdiff.h
index 3f6229e..fd89b9a 100644
--- a/xdiff/xdiff.h
+++ b/xdiff/xdiff.h
@@ -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
diff --git a/xdiff/xmerge.c b/xdiff/xmerge.c
index 8cbe45e..3a0ae14 100644
--- a/xdiff/xmerge.c
+++ b/xdiff/xmerge.c
@@ -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);
--
1.7.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH 01/16] xdl_merge(): add optional ancestor label to diff3-style output
2010-03-17 11:46 ` [PATCH 01/16] xdl_merge(): add optional ancestor label to diff3-style output Jonathan Nieder
@ 2010-03-17 14:22 ` Bert Wesarg
2010-03-20 22:47 ` Jonathan Nieder
0 siblings, 1 reply; 23+ messages in thread
From: Bert Wesarg @ 2010-03-17 14:22 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, Stefan Monnier
On Wed, Mar 17, 2010 at 12:46, Jonathan Nieder <jrnieder@gmail.com> wrote:
> diff --git a/xdiff/xmerge.c b/xdiff/xmerge.c
> index 8cbe45e..3a0ae14 100644
> --- a/xdiff/xmerge.c
> +++ b/xdiff/xmerge.c
> @@ -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.
> 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 <Bert.Wesarg@googlemail.com>
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 01/16] xdl_merge(): add optional ancestor label to diff3-style output
2010-03-17 14:22 ` Bert Wesarg
@ 2010-03-20 22:47 ` Jonathan Nieder
0 siblings, 0 replies; 23+ messages in thread
From: Jonathan Nieder @ 2010-03-20 22:47 UTC (permalink / raw)
To: Bert Wesarg; +Cc: git, Stefan Monnier
Bert Wesarg wrote:
> On Wed, Mar 17, 2010 at 12:46, Jonathan Nieder <jrnieder@gmail.com> wrote:
>> --- a/xdiff/xmerge.c
>> +++ b/xdiff/xmerge.c
>> @@ -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.
> FWIW:
>
> Acked-by: Bert Wesarg <Bert.Wesarg@googlemail.com>
Thanks for looking it over. I’ll be sending a rebased version in a
moment including your ack; I hope that’s okay.
Jonathan
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 02/16] xdl_merge(): move file1 and file2 labels to xmparam structure
2010-03-17 11:36 [PATCH v2 0/16] Add label for common ancestor to conflictstyle=diff3 Jonathan Nieder
2010-03-17 11:46 ` [PATCH 01/16] xdl_merge(): add optional ancestor label to diff3-style output Jonathan Nieder
@ 2010-03-17 11:53 ` Jonathan Nieder
2010-03-17 11:57 ` [PATCH 03/16] merge-file --diff3: add a label for ancestor Jonathan Nieder
` (13 subsequent siblings)
15 siblings, 0 replies; 23+ messages in thread
From: Jonathan Nieder @ 2010-03-17 11:53 UTC (permalink / raw)
To: git; +Cc: Stefan Monnier
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 <jrnieder@gmail.com>
---
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(-)
diff --git a/builtin/merge-file.c b/builtin/merge-file.c
index 1e70073..ade1e8b 100644
--- a/builtin/merge-file.c
+++ b/builtin/merge-file.c
@@ -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);
diff --git a/ll-merge.c b/ll-merge.c
index 4c7f11b..b8e8971 100644
--- a/ll-merge.c
+++ b/ll-merge.c
@@ -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);
}
diff --git a/xdiff/xdiff.h b/xdiff/xdiff.h
index fd89b9a..82d7fc7 100644
--- a/xdiff/xdiff.h
+++ b/xdiff/xdiff.h
@@ -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
diff --git a/xdiff/xmerge.c b/xdiff/xmerge.c
index 3a0ae14..241f7ba 100644
--- a/xdiff/xmerge.c
+++ b/xdiff/xmerge.c
@@ -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);
--
1.7.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 03/16] merge-file --diff3: add a label for ancestor
2010-03-17 11:36 [PATCH v2 0/16] Add label for common ancestor to conflictstyle=diff3 Jonathan Nieder
2010-03-17 11:46 ` [PATCH 01/16] xdl_merge(): add optional ancestor label to diff3-style output Jonathan Nieder
2010-03-17 11:53 ` [PATCH 02/16] xdl_merge(): move file1 and file2 labels to xmparam structure Jonathan Nieder
@ 2010-03-17 11:57 ` Jonathan Nieder
2010-03-17 12:03 ` [PATCH 04/16] ll_merge(): add ancestor label parameter for diff3-style output Jonathan Nieder
` (12 subsequent siblings)
15 siblings, 0 replies; 23+ messages in thread
From: Jonathan Nieder @ 2010-03-17 11:57 UTC (permalink / raw)
To: git; +Cc: Stefan Monnier
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 <monnier@iro.umontreal.ca>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
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(-)
diff --git a/builtin/merge-file.c b/builtin/merge-file.c
index ade1e8b..04cb67a 100644
--- a/builtin/merge-file.c
+++ b/builtin/merge-file.c
@@ -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,
diff --git a/t/t6023-merge-file.sh b/t/t6023-merge-file.sh
index d605024..16c3c69 100755
--- a/t/t6023-merge-file.sh
+++ b/t/t6023-merge-file.sh
@@ -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;
--
1.7.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 04/16] ll_merge(): add ancestor label parameter for diff3-style output
2010-03-17 11:36 [PATCH v2 0/16] Add label for common ancestor to conflictstyle=diff3 Jonathan Nieder
` (2 preceding siblings ...)
2010-03-17 11:57 ` [PATCH 03/16] merge-file --diff3: add a label for ancestor Jonathan Nieder
@ 2010-03-17 12:03 ` Jonathan Nieder
2010-03-17 12:04 ` [PATCH 05/16] checkout --conflict=diff3: add a label for ancestor Jonathan Nieder
` (11 subsequent siblings)
15 siblings, 0 replies; 23+ messages in thread
From: Jonathan Nieder @ 2010-03-17 12:03 UTC (permalink / raw)
To: git; +Cc: Stefan Monnier
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 <monnier@iro.umontreal.ca>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
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(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index acefaaf..d67f809 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -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);
diff --git a/ll-merge.c b/ll-merge.c
index b8e8971..92670af 100644
--- a/ll-merge.c
+++ b/ll-merge.c
@@ -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);
}
diff --git a/ll-merge.h b/ll-merge.h
index 5788922..57754cc 100644
--- a/ll-merge.h
+++ b/ll-merge.h
@@ -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);
diff --git a/merge-file.c b/merge-file.c
index fd34d76..07cc0c9 100644
--- a/merge-file.c
+++ b/merge-file.c
@@ -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;
diff --git a/merge-recursive.c b/merge-recursive.c
index 195ebf9..3b2cc9d 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -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));
diff --git a/rerere.c b/rerere.c
index a59f74f..f221bed 100644
--- a/rerere.c
+++ b/rerere.c
@@ -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)
--
1.7.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 05/16] checkout --conflict=diff3: add a label for ancestor
2010-03-17 11:36 [PATCH v2 0/16] Add label for common ancestor to conflictstyle=diff3 Jonathan Nieder
` (3 preceding siblings ...)
2010-03-17 12:03 ` [PATCH 04/16] ll_merge(): add ancestor label parameter for diff3-style output Jonathan Nieder
@ 2010-03-17 12:04 ` Jonathan Nieder
2010-03-17 12:05 ` [PATCH 06/16] merge_file(): add comment explaining behavior wrt conflict style Jonathan Nieder
` (10 subsequent siblings)
15 siblings, 0 replies; 23+ messages in thread
From: Jonathan Nieder @ 2010-03-17 12:04 UTC (permalink / raw)
To: git; +Cc: Stefan Monnier
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 <monnier@iro.umontreal.ca>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Label changed. Thanks for the feedback.
builtin/checkout.c | 2 +-
t/t7201-co.sh | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index d67f809..d652b4c 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -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);
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index d20ed61..981ae8f 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -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
--
1.7.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 06/16] merge_file(): add comment explaining behavior wrt conflict style
2010-03-17 11:36 [PATCH v2 0/16] Add label for common ancestor to conflictstyle=diff3 Jonathan Nieder
` (4 preceding siblings ...)
2010-03-17 12:04 ` [PATCH 05/16] checkout --conflict=diff3: add a label for ancestor Jonathan Nieder
@ 2010-03-17 12:05 ` Jonathan Nieder
2010-03-17 12:08 ` [PATCH 07/16] merge_trees(): add ancestor label parameter for diff3-style output Jonathan Nieder
` (9 subsequent siblings)
15 siblings, 0 replies; 23+ messages in thread
From: Jonathan Nieder @ 2010-03-17 12:05 UTC (permalink / raw)
To: git; +Cc: Stefan Monnier
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 <jrnieder@mgila.com>
---
merge-file.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/merge-file.c b/merge-file.c
index 07cc0c9..c336c93 100644
--- a/merge-file.c
+++ b/merge-file.c
@@ -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)
--
1.7.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 07/16] merge_trees(): add ancestor label parameter for diff3-style output
2010-03-17 11:36 [PATCH v2 0/16] Add label for common ancestor to conflictstyle=diff3 Jonathan Nieder
` (5 preceding siblings ...)
2010-03-17 12:05 ` [PATCH 06/16] merge_file(): add comment explaining behavior wrt conflict style Jonathan Nieder
@ 2010-03-17 12:08 ` Jonathan Nieder
2010-03-17 12:09 ` [PATCH 08/16] tests: document format of conflicts from checkout -m Jonathan Nieder
` (8 subsequent siblings)
15 siblings, 0 replies; 23+ messages in thread
From: Jonathan Nieder @ 2010-03-17 12:08 UTC (permalink / raw)
To: git; +Cc: Stefan Monnier
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 <monnier@iro.umontreal.ca>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Only the commit message was changed.
merge-recursive.c | 11 ++++++++---
merge-recursive.h | 1 +
2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index 3b2cc9d..017cafd 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -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));
diff --git a/merge-recursive.h b/merge-recursive.h
index be8410a..d1192f5 100644
--- a/merge-recursive.h
+++ b/merge-recursive.h
@@ -4,6 +4,7 @@
#include "string-list.h"
struct merge_options {
+ const char *ancestor;
const char *branch1;
const char *branch2;
enum {
--
1.7.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 08/16] tests: document format of conflicts from checkout -m
2010-03-17 11:36 [PATCH v2 0/16] Add label for common ancestor to conflictstyle=diff3 Jonathan Nieder
` (6 preceding siblings ...)
2010-03-17 12:08 ` [PATCH 07/16] merge_trees(): add ancestor label parameter for diff3-style output Jonathan Nieder
@ 2010-03-17 12:09 ` Jonathan Nieder
2010-03-17 12:13 ` [PATCH 09/16] checkout -m --conflict=diff3: add a label for ancestor Jonathan Nieder
` (7 subsequent siblings)
15 siblings, 0 replies; 23+ messages in thread
From: Jonathan Nieder @ 2010-03-17 12:09 UTC (permalink / raw)
To: git; +Cc: Stefan Monnier
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 <jrnieder@gmail.com>
---
Unchanged.
t/t7201-co.sh | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++---
1 files changed, 61 insertions(+), 4 deletions(-)
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 981ae8f..f3f0c4c 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -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 &&
--
1.7.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 09/16] checkout -m --conflict=diff3: add a label for ancestor
2010-03-17 11:36 [PATCH v2 0/16] Add label for common ancestor to conflictstyle=diff3 Jonathan Nieder
` (7 preceding siblings ...)
2010-03-17 12:09 ` [PATCH 08/16] tests: document format of conflicts from checkout -m Jonathan Nieder
@ 2010-03-17 12:13 ` Jonathan Nieder
2010-03-17 12:13 ` [PATCH 10/16] tests: document cherry-pick behavior in face of conflicts Jonathan Nieder
` (6 subsequent siblings)
15 siblings, 0 replies; 23+ messages in thread
From: Jonathan Nieder @ 2010-03-17 12:13 UTC (permalink / raw)
To: git; +Cc: Stefan Monnier
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 <monnier@iro.umontreal.ca>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Unchanged.
builtin/checkout.c | 1 +
t/t7201-co.sh | 2 +-
2 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index d652b4c..88b1f43 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -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,
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index f3f0c4c..1337fa5 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -209,7 +209,7 @@ test_expect_success 'checkout --merge --conflict=diff3 <branch>' '
a
c
e
- |||||||
+ ||||||| master
a
b
c
--
1.7.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 10/16] tests: document cherry-pick behavior in face of conflicts
2010-03-17 11:36 [PATCH v2 0/16] Add label for common ancestor to conflictstyle=diff3 Jonathan Nieder
` (8 preceding siblings ...)
2010-03-17 12:13 ` [PATCH 09/16] checkout -m --conflict=diff3: add a label for ancestor Jonathan Nieder
@ 2010-03-17 12:13 ` Jonathan Nieder
2010-03-17 12:15 ` [PATCH 11/16] revert_or_cherry_pick(): get oneline_body from get_oneline() Jonathan Nieder
` (5 subsequent siblings)
15 siblings, 0 replies; 23+ messages in thread
From: Jonathan Nieder @ 2010-03-17 12:13 UTC (permalink / raw)
To: git; +Cc: Stefan Monnier
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 <jrnieder@gmail.com>
---
t/t3507-cherry-pick-conflict.sh | 198 +++++++++++++++++++++++++++++++++++++++
1 files changed, 198 insertions(+), 0 deletions(-)
create mode 100644 t/t3507-cherry-pick-conflict.sh
diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh
new file mode 100644
index 0000000..e856356
--- /dev/null
+++ b/t/t3507-cherry-pick-conflict.sh
@@ -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
--
1.7.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 11/16] revert_or_cherry_pick(): get oneline_body from get_oneline()
2010-03-17 11:36 [PATCH v2 0/16] Add label for common ancestor to conflictstyle=diff3 Jonathan Nieder
` (9 preceding siblings ...)
2010-03-17 12:13 ` [PATCH 10/16] tests: document cherry-pick behavior in face of conflicts Jonathan Nieder
@ 2010-03-17 12:15 ` Jonathan Nieder
2010-03-17 20:09 ` Stephen Boyd
2010-03-17 12:16 ` [PATCH 12/16] compat: add mempcpy() Jonathan Nieder
` (4 subsequent siblings)
15 siblings, 1 reply; 23+ messages in thread
From: Jonathan Nieder @ 2010-03-17 12:15 UTC (permalink / raw)
To: git; +Cc: Stefan Monnier
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 <jrnieder@gmail.com>
---
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(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index eff5268..4b2042f 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -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));
--
1.7.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH 11/16] revert_or_cherry_pick(): get oneline_body from get_oneline()
2010-03-17 12:15 ` [PATCH 11/16] revert_or_cherry_pick(): get oneline_body from get_oneline() Jonathan Nieder
@ 2010-03-17 20:09 ` Stephen Boyd
2010-03-18 3:16 ` Jonathan Nieder
0 siblings, 1 reply; 23+ messages in thread
From: Stephen Boyd @ 2010-03-17 20:09 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, Stefan Monnier
On Wed, Mar 17, 2010 at 5:15 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
>
> 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.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 11/16] revert_or_cherry_pick(): get oneline_body from get_oneline()
2010-03-17 20:09 ` Stephen Boyd
@ 2010-03-18 3:16 ` Jonathan Nieder
2010-03-18 3:33 ` [PATCH 11,13,14 v2] revert: clarify label on conflict hunks Jonathan Nieder
2010-03-18 3:34 ` [PATCH 15/16 v2] cherry-pick, revert: add a label for ancestor Jonathan Nieder
0 siblings, 2 replies; 23+ messages in thread
From: Jonathan Nieder @ 2010-03-18 3:16 UTC (permalink / raw)
To: Stephen Boyd; +Cc: git, Stefan Monnier
Stephen Boyd wrote:
> 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.
>>>>>>> [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(-)
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 11,13,14 v2] revert: clarify label on conflict hunks
2010-03-18 3:16 ` Jonathan Nieder
@ 2010-03-18 3:33 ` Jonathan Nieder
2010-03-18 3:34 ` [PATCH 15/16 v2] cherry-pick, revert: add a label for ancestor Jonathan Nieder
1 sibling, 0 replies; 23+ messages in thread
From: Jonathan Nieder @ 2010-03-18 3:33 UTC (permalink / raw)
To: Stephen Boyd; +Cc: git, Stefan Monnier
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 <jrnieder@gmail.com>
---
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(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index eff5268..b314629 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -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;
diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh
index e856356..6a20817 100644
--- a/t/t3507-cherry-pick-conflict.sh
+++ b/t/t3507-cherry-pick-conflict.sh
@@ -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 &&
--
1.7.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 15/16 v2] cherry-pick, revert: add a label for ancestor
2010-03-18 3:16 ` Jonathan Nieder
2010-03-18 3:33 ` [PATCH 11,13,14 v2] revert: clarify label on conflict hunks Jonathan Nieder
@ 2010-03-18 3:34 ` Jonathan Nieder
1 sibling, 0 replies; 23+ messages in thread
From: Jonathan Nieder @ 2010-03-18 3:34 UTC (permalink / raw)
To: Stephen Boyd; +Cc: git, Stefan Monnier
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 <monnier@iro.umontreal.ca>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
builtin/revert.c | 5 ++++-
t/t3507-cherry-pick-conflict.sh | 4 ++--
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index b314629..625281c 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -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)";
diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh
index 6a20817..e25cf80 100644
--- a/t/t3507-cherry-pick-conflict.sh
+++ b/t/t3507-cherry-pick-conflict.sh
@@ -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
--
1.7.0
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 12/16] compat: add mempcpy()
2010-03-17 11:36 [PATCH v2 0/16] Add label for common ancestor to conflictstyle=diff3 Jonathan Nieder
` (10 preceding siblings ...)
2010-03-17 12:15 ` [PATCH 11/16] revert_or_cherry_pick(): get oneline_body from get_oneline() Jonathan Nieder
@ 2010-03-17 12:16 ` Jonathan Nieder
2010-03-17 12:17 ` [PATCH 13/16] revert: simplify get_oneline() using mempcpy Jonathan Nieder
` (3 subsequent siblings)
15 siblings, 0 replies; 23+ messages in thread
From: Jonathan Nieder @ 2010-03-17 12:16 UTC (permalink / raw)
To: git; +Cc: Stefan Monnier
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 <jrnieder@gmail.com>
---
git-compat-util.h | 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/git-compat-util.h b/git-compat-util.h
index a3c4537..9bed5a0 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -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);
--
1.7.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 13/16] revert: simplify get_oneline() using mempcpy
2010-03-17 11:36 [PATCH v2 0/16] Add label for common ancestor to conflictstyle=diff3 Jonathan Nieder
` (11 preceding siblings ...)
2010-03-17 12:16 ` [PATCH 12/16] compat: add mempcpy() Jonathan Nieder
@ 2010-03-17 12:17 ` Jonathan Nieder
2010-03-17 12:19 ` [PATCH 14/16] revert: clarify label on conflict hunks Jonathan Nieder
` (2 subsequent siblings)
15 siblings, 0 replies; 23+ messages in thread
From: Jonathan Nieder @ 2010-03-17 12:17 UTC (permalink / raw)
To: git; +Cc: Stefan Monnier
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 <jrnieder@gmail.com>
---
builtin/revert.c | 13 ++++++-------
1 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 4b2042f..6a47655 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -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;
}
--
1.7.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 14/16] revert: clarify label on conflict hunks
2010-03-17 11:36 [PATCH v2 0/16] Add label for common ancestor to conflictstyle=diff3 Jonathan Nieder
` (12 preceding siblings ...)
2010-03-17 12:17 ` [PATCH 13/16] revert: simplify get_oneline() using mempcpy Jonathan Nieder
@ 2010-03-17 12:19 ` Jonathan Nieder
2010-03-17 12:23 ` [PATCH 15/16] cherry-pick: add a label for ancestor Jonathan Nieder
2010-03-17 12:26 ` [PATCH 16/16] merge-recursive: " Jonathan Nieder
15 siblings, 0 replies; 23+ messages in thread
From: Jonathan Nieder @ 2010-03-17 12:19 UTC (permalink / raw)
To: git; +Cc: Stefan Monnier
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 <jrnieder@gmail.com>
---
New patch: a small fix.
builtin/revert.c | 14 +++++++++-----
t/t3507-cherry-pick-conflict.sh | 4 ++--
2 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 6a47655..56f2947 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -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;
diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh
index e856356..6a20817 100644
--- a/t/t3507-cherry-pick-conflict.sh
+++ b/t/t3507-cherry-pick-conflict.sh
@@ -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 &&
--
1.7.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 15/16] cherry-pick: add a label for ancestor
2010-03-17 11:36 [PATCH v2 0/16] Add label for common ancestor to conflictstyle=diff3 Jonathan Nieder
` (13 preceding siblings ...)
2010-03-17 12:19 ` [PATCH 14/16] revert: clarify label on conflict hunks Jonathan Nieder
@ 2010-03-17 12:23 ` Jonathan Nieder
2010-03-17 12:26 ` [PATCH 16/16] merge-recursive: " Jonathan Nieder
15 siblings, 0 replies; 23+ messages in thread
From: Jonathan Nieder @ 2010-03-17 12:23 UTC (permalink / raw)
To: git; +Cc: Stefan Monnier
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 <monnier@iro.umontreal.ca>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
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(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 56f2947..7a39b52 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -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();
diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh
index 6a20817..e25cf80 100644
--- a/t/t3507-cherry-pick-conflict.sh
+++ b/t/t3507-cherry-pick-conflict.sh
@@ -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
--
1.7.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 16/16] merge-recursive: add a label for ancestor
2010-03-17 11:36 [PATCH v2 0/16] Add label for common ancestor to conflictstyle=diff3 Jonathan Nieder
` (14 preceding siblings ...)
2010-03-17 12:23 ` [PATCH 15/16] cherry-pick: add a label for ancestor Jonathan Nieder
@ 2010-03-17 12:26 ` Jonathan Nieder
15 siblings, 0 replies; 23+ messages in thread
From: Jonathan Nieder @ 2010-03-17 12:26 UTC (permalink / raw)
To: git; +Cc: Stefan Monnier
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 <monnier@iro.umontreal.ca>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
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(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index 017cafd..917397c 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -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);
--
1.7.0
^ permalink raw reply related [flat|nested] 23+ messages in thread