From a944af1d86e6171d68ed2a3aa67b1d68f00e1fe8 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sat, 8 Sep 2012 21:27:19 -0700 Subject: [PATCH 1/3] merge: teach -Xours/-Xtheirs to binary ll-merge driver The (discouraged) -Xours/-Xtheirs modes of merge are supposed to give a quick and dirty way to come up with a random mixture of cleanly merged parts and punted conflict resolution to take contents from one side in conflicting parts. These options however were only passed down to the low level merge driver for text. Teach the built-in binary merge driver to notice them as well. Signed-off-by: Junio C Hamano --- Documentation/merge-strategies.txt | 3 ++- ll-merge.c | 25 ++++++++++++++++++++----- t/t6037-merge-ours-theirs.sh | 14 +++++++++++++- 3 files changed, 35 insertions(+), 7 deletions(-) diff --git a/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt index 595a3cf1a7..66db80296f 100644 --- a/Documentation/merge-strategies.txt +++ b/Documentation/merge-strategies.txt @@ -32,13 +32,14 @@ ours;; This option forces conflicting hunks to be auto-resolved cleanly by favoring 'our' version. Changes from the other tree that do not conflict with our side are reflected to the merge result. + For a binary file, the entire contents are taken from our side. + This should not be confused with the 'ours' merge strategy, which does not even look at what the other tree contains at all. It discards everything the other tree did, declaring 'our' history contains all that happened in it. theirs;; - This is opposite of 'ours'. + This is the opposite of 'ours'. patience;; With this option, 'merge-recursive' spends a little extra time diff --git a/ll-merge.c b/ll-merge.c index da59738c9b..8535e2d4a5 100644 --- a/ll-merge.c +++ b/ll-merge.c @@ -46,16 +46,31 @@ static int ll_binary_merge(const struct ll_merge_driver *drv_unused, assert(opts); /* - * The tentative merge result is "ours" for the final round, - * or common ancestor for an internal merge. Still return - * "conflicted merge" status. + * The tentative merge result is the or common ancestor for an internal merge. */ - stolen = opts->virtual_ancestor ? orig : src1; + if (opts->virtual_ancestor) { + stolen = orig; + } else { + switch (opts->variant) { + default: + case XDL_MERGE_FAVOR_OURS: + stolen = src1; + break; + case XDL_MERGE_FAVOR_THEIRS: + stolen = src2; + break; + } + } result->ptr = stolen->ptr; result->size = stolen->size; stolen->ptr = NULL; - return 1; + + /* + * With -Xtheirs or -Xours, we have cleanly merged; + * otherwise we got a conflict. + */ + return (opts->variant ? 0 : 1); } static int ll_xdl_merge(const struct ll_merge_driver *drv_unused, diff --git a/t/t6037-merge-ours-theirs.sh b/t/t6037-merge-ours-theirs.sh index 2cf42c73f1..8d05671414 100755 --- a/t/t6037-merge-ours-theirs.sh +++ b/t/t6037-merge-ours-theirs.sh @@ -53,7 +53,19 @@ test_expect_success 'recursive favouring ours' ' ! grep 1 file ' -test_expect_success 'pull with -X' ' +test_expect_success 'binary file with -Xours/-Xtheirs' ' + echo "file -merge" >.gitattributes && + + git reset --hard master && + git merge -s recursive -X theirs side && + git diff --exit-code side HEAD -- file && + + git reset --hard master && + git merge -s recursive -X ours side && + git diff --exit-code master HEAD -- file +' + +test_expect_success 'pull passes -X to underlying merge' ' git reset --hard master && git pull -s recursive -Xours . side && git reset --hard master && git pull -s recursive -X ours . side && git reset --hard master && git pull -s recursive -Xtheirs . side && From 155a4b712efd3d917c228d155ec57ec2c09d7ac0 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sat, 8 Sep 2012 21:28:55 -0700 Subject: [PATCH 2/3] attr: "binary" attribute should choose built-in "binary" merge driver The built-in "binary" attribute macro expands to "-diff -text", so that textual diff is not produced, and the contents will not go through any CR/LF conversion ever. During a merge, it should also choose the "binary" low-level merge driver, but it didn't. Make it expand to "-diff -merge -text". Signed-off-by: Junio C Hamano --- Documentation/gitattributes.txt | 2 +- attr.c | 2 +- t/t6037-merge-ours-theirs.sh | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt index a85b187e04..ead7254922 100644 --- a/Documentation/gitattributes.txt +++ b/Documentation/gitattributes.txt @@ -904,7 +904,7 @@ file at the toplevel (i.e. not in any subdirectory). The built-in macro attribute "binary" is equivalent to: ------------ -[attr]binary -diff -text +[attr]binary -diff -merge -text ------------ diff --git a/attr.c b/attr.c index 303751f6c2..3f581b3cec 100644 --- a/attr.c +++ b/attr.c @@ -306,7 +306,7 @@ static void free_attr_elem(struct attr_stack *e) } static const char *builtin_attr[] = { - "[attr]binary -diff -text", + "[attr]binary -diff -merge -text", NULL, }; diff --git a/t/t6037-merge-ours-theirs.sh b/t/t6037-merge-ours-theirs.sh index 8d05671414..3889eca4ae 100755 --- a/t/t6037-merge-ours-theirs.sh +++ b/t/t6037-merge-ours-theirs.sh @@ -54,7 +54,7 @@ test_expect_success 'recursive favouring ours' ' ' test_expect_success 'binary file with -Xours/-Xtheirs' ' - echo "file -merge" >.gitattributes && + echo file binary >.gitattributes && git reset --hard master && git merge -s recursive -X theirs side && From e0e2065f74500119d5e12524992273de362acd30 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 12 Sep 2012 02:01:52 -0700 Subject: [PATCH 3/3] ll-merge: warn about inability to merge binary files only when we can't When a path being merged is auto detected to be a binary file, we warned "Cannot merge binary files" before switching to activate the binary ll-merge driver. When we are merging with the -Xours/theirs option, however, we know what the "clean" merge result is, and the warning is inappropriate. In addition, when the path is explicitly marked as a binary file, this warning was not issued, even though without -Xours/theirs, we cannot cleanly automerge such a path, which was inconsistent. Move the warning code from ll_xdl_merge() to ll_binary_merge(), and issue the message only when we cannot cleanly automerge. Signed-off-by: Junio C Hamano --- ll-merge.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/ll-merge.c b/ll-merge.c index 8535e2d4a5..307315b788 100644 --- a/ll-merge.c +++ b/ll-merge.c @@ -35,7 +35,7 @@ struct ll_merge_driver { */ static int ll_binary_merge(const struct ll_merge_driver *drv_unused, mmbuffer_t *result, - const char *path_unused, + const char *path, mmfile_t *orig, const char *orig_name, mmfile_t *src1, const char *name1, mmfile_t *src2, const char *name2, @@ -53,6 +53,9 @@ static int ll_binary_merge(const struct ll_merge_driver *drv_unused, } else { switch (opts->variant) { default: + warning("Cannot merge binary files: %s (%s vs. %s)\n", + path, name1, name2); + /* fallthru */ case XDL_MERGE_FAVOR_OURS: stolen = src1; break; @@ -88,8 +91,6 @@ static int ll_xdl_merge(const struct ll_merge_driver *drv_unused, if (buffer_is_binary(orig->ptr, orig->size) || buffer_is_binary(src1->ptr, src1->size) || buffer_is_binary(src2->ptr, src2->size)) { - warning("Cannot merge binary files: %s (%s vs. %s)\n", - path, name1, name2); return ll_binary_merge(drv_unused, result, path, orig, orig_name,