Merge branch 'jc/ll-merge-binary-ours'

"git merge -Xtheirs" did not help content-level merge of binary
files; it should just take their version.  Also "*.jpg binary" in
the attributes did not imply they should use the binary ll-merge
driver.

* jc/ll-merge-binary-ours:
ll-merge: warn about inability to merge binary files only when we can't
attr: "binary" attribute should choose built-in "binary" merge driver
merge: teach -Xours/-Xtheirs to binary ll-merge driver

diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index e16f3e175bd8915d139961c649be209b0afc535f..462b79c120ddb132bd125104240a488cc31f877b 100644 (file)
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -927,7 +927,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/Documentation/merge-strategies.txt b/Documentation/merge-strategies.txt
index 595a3cf1a7118ba29a1d57d7fc17d233d89cd3d0..66db80296f505a9bd830c746f82bb54e3f49f313 100644 (file)
--- 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/attr.c b/attr.c
index f12c83f80a0f03b2112c212e46c571ec4074abf3..3430faf2ccc5e11a9c04432b7a4f13160130e141 100644 (file)
--- 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/ll-merge.c b/ll-merge.c
index f3f7692158666ffd2ab6f65f4040462e4a7d2d00..acea33bf1babfe541c319081f14625ac779bb582 100644 (file)
--- 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,
@@ -46,16 +46,34 @@ 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:
+                       warning("Cannot merge binary files: %s (%s vs. %s)",
+                               path, name1, name2);
+                       /* fallthru */
+               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,
@@ -73,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)",
-                       path, name1, name2);
                return ll_binary_merge(drv_unused, result,
                                       path,
                                       orig, orig_name,

diff --git a/t/t6037-merge-ours-theirs.sh b/t/t6037-merge-ours-theirs.sh
index 2cf42c73f14ef5069d096fb29e67dac571227cfe..3889eca4ae8524cd62eaf45b4d7a2394e53da429 100755 (executable)
--- 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 binary >.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 &&