For non-textual conflicts, provide additional information in the working
copy in the form of additional conflict markers and explanatory text
stating what type of non-textual conflict was involved.  In particular,
regular files involved in path-based conflicts would have something like
the following at the beginning of the file:

    <<<<<<<< HEAD
    Conflict hint: This block of text was not part of the original
    branch; it serves instead to hint about non-textual conflicts:
      MODIFY/DELETE: path foo modified in HEAD and deleted in BRANCH
    ========
    Conflict hint: This block of text was not part of the original
    branch; it serves instead to hint about non-textual conflicts:
      MODIFY/DELETE: path foo modified in HEAD and deleted in BRANCH
    >>>>>>>> BRANCH

When non regular files (binaries, symlinks, etc.) are involved in
conflicts, we instead put this information in a separate file that only
contains this conflict information.

The goals of providing this extra information are:
  * Make it clearer to users what conflicts they are dealing with and why
  * Enable new features like Thomas Rast' old remerge-diff proposal[1]

[1] https://public-inbox.org/git/cover.1409860234.git...@thomasrast.ch/

Signed-off-by: Elijah Newren <new...@gmail.com>
---
 merge-recursive.c                   | 133 +++++++++++++++++++++++++++-
 t/t3031-merge-criscross.sh          |   2 +
 t/t6022-merge-rename.sh             |  39 ++------
 t/t6043-merge-rename-directories.sh |   4 +-
 t/t7610-mergetool.sh                |   4 +
 5 files changed, 144 insertions(+), 38 deletions(-)

diff --git a/merge-recursive.c b/merge-recursive.c
index 4832234073..cdfe9824d2 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -313,6 +313,96 @@ static void output_commit_title(struct merge_options *o, 
struct commit *commit)
        flush_output(o);
 }
 
+static void write_conflict_notice(struct strbuf *buf,
+                                 struct strbuf *notice,
+                                 int use_crlf)
+{
+       int marker_size = 8;
+
+       strbuf_addchars(buf, '<', marker_size);
+       if (use_crlf)
+               strbuf_addch(buf, '\r');
+       strbuf_addch(buf, '\n');
+
+       strbuf_addbuf(buf, notice);
+       if (use_crlf)
+               strbuf_addch(buf, '\r');
+       strbuf_addch(buf, '\n');
+
+       strbuf_addchars(buf, '=', marker_size);
+       if (use_crlf)
+               strbuf_addch(buf, '\r');
+       strbuf_addch(buf, '\n');
+
+       strbuf_addbuf(buf, notice);
+       if (use_crlf)
+               strbuf_addch(buf, '\r');
+       strbuf_addch(buf, '\n');
+
+       strbuf_addchars(buf, '>', marker_size);
+       if (use_crlf)
+               strbuf_addch(buf, '\r');
+       strbuf_addch(buf, '\n');
+}
+
+static int create_non_textual_conflict_file(struct merge_options *o,
+                                           struct strbuf *notice,
+                                           const char *path,
+                                           struct object_id *oid)
+{
+       struct strbuf contents = STRBUF_INIT;
+       int ret = 0;
+       int use_crlf = 0; /* FIXME: Determine platform default?? */
+
+       write_conflict_notice(&contents, notice, use_crlf);
+
+       if (write_object_file(contents.buf, contents.len, "blob", oid))
+               ret = err(o, _("Unable to add %s to database"), path);
+
+       strbuf_release(&contents);
+       return ret;
+}
+
+static int insert_non_textual_conflict_header(struct merge_options *o,
+                                             struct strbuf *notice,
+                                             const char *path,
+                                             struct object_id *oid,
+                                             unsigned int mode)
+{
+       struct strbuf header = STRBUF_INIT;
+       struct strbuf dst_buf = STRBUF_INIT;
+       enum object_type type;
+       char *buf;
+       unsigned long size;
+       char *end;
+       int use_crlf;
+       int ret = 0;
+
+       if (!S_ISREG(mode))
+               BUG("insert_non_textual_conflict_header called on file with 
wrong mode: %0d", mode);
+
+       buf = read_object_file(oid, &type, &size);
+       if (!buf)
+               return err(o, _("cannot read object %s '%s'"), oid_to_hex(oid), 
path);
+       if (type != OBJ_BLOB) {
+               return err(o, _("blob expected for %s '%s'"), oid_to_hex(oid), 
path);
+       }
+
+       end = strchrnul(buf, '\n');
+       use_crlf = (end > buf && end[-1] == '\r');
+       write_conflict_notice(&header, notice, use_crlf);
+
+       strbuf_addbuf(&dst_buf, &header);
+       strbuf_add(&dst_buf, buf, size);
+
+       if (write_object_file(dst_buf.buf, dst_buf.len, type_name(type), oid))
+               ret = err(o, _("Unable to add %s to database"), path);
+
+       strbuf_release(&header);
+       strbuf_release(&dst_buf);
+       return ret;
+}
+
 static int add_cacheinfo(struct merge_options *o,
                         unsigned int mode, const struct object_id *oid,
                         const char *path, int stage, int refresh, int options)
@@ -1464,6 +1554,9 @@ static int handle_change_delete(struct merge_options *o,
 {
        char *alt_path = NULL;
        const char *update_path = path;
+       struct object_id new_oid;
+       struct strbuf sb = STRBUF_INIT;
+       int is_binary;
        int ret = 0;
 
        if (dir_in_way(path, !o->call_depth && !S_ISGITLINK(changed_mode), 0) ||
@@ -1527,8 +1620,44 @@ static int handle_change_delete(struct merge_options *o,
                 * path.  We could call update_file_flags() with update_cache=0
                 * and update_wd=0, but that's a no-op.
                 */
-               if (change_branch != o->branch1 || alt_path)
-                       ret = update_file(o, 0, changed_oid, changed_mode, 
update_path);
+               oidcpy(&new_oid, changed_oid);
+               strbuf_addf(&sb, _("CONFLICT (%s/delete): %s deleted in %s and 
%s in %s."),
+                           change, path, delete_branch, change_past, 
change_branch);
+                /*
+                 * FIXME: figure out if update_path's contents are binary;
+                 * buffer_is_binary() may help, though in the case of e.g.
+                 * add/add conflicts it'd be nice to avoid calling that
+                 * multiple times per buffer.
+                 */
+               is_binary = 0;
+               if (S_ISREG(changed_mode) && !is_binary) {
+                       insert_non_textual_conflict_header(
+                           o,
+                           &sb,
+                           update_path,
+                           &new_oid,
+                           changed_mode);
+                       ret = update_file_flags(o, &new_oid, changed_mode,
+                                               update_path,
+                                               /* update_cache */ 0,
+                                               /* update_wd */    1);
+               } else {
+                       struct diff_filespec conflict_file;
+                       char *conflict_path;
+                       conflict_path = unique_path(o, update_path, 
"conflicts");
+
+                       conflict_file.mode = S_IFREG | 0644;
+                       create_non_textual_conflict_file(o, &sb, conflict_path,
+                                                        &conflict_file.oid);
+                       ret = update_file_flags(o, &conflict_file.oid,
+                                               conflict_file.mode,
+                                               conflict_path,
+                                               /* update_cache */ 0,
+                                               /* update_wd */    1);
+                       ret |= update_stages(o, conflict_path, &conflict_file,
+                                            NULL, NULL);
+                       free(conflict_path);
+               }
        }
        free(alt_path);
 
diff --git a/t/t3031-merge-criscross.sh b/t/t3031-merge-criscross.sh
index e59b0a32d6..5c39339809 100755
--- a/t/t3031-merge-criscross.sh
+++ b/t/t3031-merge-criscross.sh
@@ -75,6 +75,7 @@ test_expect_success 'setup repo with criss-cross history' '
        git checkout B &&
        test_must_fail git merge E &&
        # force-resolve
+       echo 9 >data/new-9 &&
        git add data &&
        git commit -m F &&
        git branch F &&
@@ -83,6 +84,7 @@ test_expect_success 'setup repo with criss-cross history' '
        git checkout C &&
        test_must_fail git merge D &&
        # force-resolve
+       echo 9 >data/new-9 &&
        git add data &&
        git commit -m G &&
        git branch G
diff --git a/t/t6022-merge-rename.sh b/t/t6022-merge-rename.sh
index b760c223c6..a065d79049 100755
--- a/t/t6022-merge-rename.sh
+++ b/t/t6022-merge-rename.sh
@@ -467,7 +467,7 @@ test_expect_success 'both rename source and destination 
involved in D/F conflict
        test -f destdir/foo &&
        test -f one &&
        test -f destdir~HEAD &&
-       test "stuff" = "$(cat destdir~HEAD)"
+       grep "stuff" destdir~HEAD
 '
 
 test_expect_success 'setup pair rename to parent of other (D/F conflicts)' '
@@ -510,8 +510,8 @@ test_expect_success 'pair rename to parent of other (D/F 
conflicts) w/ untracked
        test -d one &&
        test -f one~rename-two &&
        test -f two &&
-       test "other" = $(cat one~rename-two) &&
-       test "stuff" = $(cat two)
+       grep "other" one~rename-two &&
+       grep "stuff" two
 '
 
 test_expect_success 'pair rename to parent of other (D/F conflicts) w/ clean 
start' '
@@ -529,8 +529,8 @@ test_expect_success 'pair rename to parent of other (D/F 
conflicts) w/ clean sta
 
        test -f one &&
        test -f two &&
-       test "other" = $(cat one) &&
-       test "stuff" = $(cat two)
+       grep "other" one &&
+       grep "stuff" two
 '
 
 test_expect_success 'setup rename of one file to two, with directories in the 
way' '
@@ -704,35 +704,6 @@ test_expect_success 'avoid unnecessary update, 
dir->(file,nothing)' '
        test_cmp expect actual # "df" should have stayed intact
 '
 
-test_expect_success 'setup avoid unnecessary update, modify/delete' '
-       git rm -rf . &&
-       git clean -fdqx &&
-       rm -rf .git &&
-       git init &&
-
-       >irrelevant &&
-       >file &&
-       git add -A &&
-       git commit -mA &&
-
-       git checkout -b side &&
-       git rm -f file &&
-       git commit -m "Delete file" &&
-
-       git checkout master &&
-       echo bla >file &&
-       git add -A &&
-       git commit -m "Modify file"
-'
-
-test_expect_success 'avoid unnecessary update, modify/delete' '
-       git checkout -q master^0 &&
-       test-tool chmtime --get =1000000000 file >expect &&
-       test_must_fail git merge side &&
-       test-tool chmtime --get file >actual &&
-       test_cmp expect actual # "file" should have stayed intact
-'
-
 test_expect_success 'setup avoid unnecessary update, rename/add-dest' '
        git rm -rf . &&
        git clean -fdqx &&
diff --git a/t/t6043-merge-rename-directories.sh 
b/t/t6043-merge-rename-directories.sh
index 4a71f17edd..c5e1874df1 100755
--- a/t/t6043-merge-rename-directories.sh
+++ b/t/t6043-merge-rename-directories.sh
@@ -1908,8 +1908,8 @@ test_expect_success '7e-check: transitive rename in 
rename/delete AND dirs in th
                         A:x/d/f  A:y/d/g  O:z/b  O:z/c  O:x/d &&
                test_cmp expect actual &&
 
-               git hash-object y/d~B^0 >actual &&
-               git rev-parse O:x/d >expect &&
+               tail -n +6 y/d~B^0 >actual &&
+               git cat-file -p O:x/d >expect &&
                test_cmp expect actual
        )
 '
diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index 047156e9d5..10c0c903c6 100755
--- a/t/t7610-mergetool.sh
+++ b/t/t7610-mergetool.sh
@@ -374,6 +374,7 @@ test_expect_success 'deleted vs modified submodule' '
        ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
        ( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
        ( yes "r" | git mergetool submod ) &&
+       ( yes "d" | git mergetool submod~conflicts ) &&
        rmdir submod && mv submod-movedaside submod &&
        test "$(cat submod/bar)" = "branch1 submodule" &&
        git submodule update -N &&
@@ -391,6 +392,7 @@ test_expect_success 'deleted vs modified submodule' '
        ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
        ( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
        ( yes "l" | git mergetool submod ) &&
+       ( yes "d" | git mergetool submod~conflicts ) &&
        test ! -e submod &&
        output="$(git mergetool --no-prompt)" &&
        test "$output" = "No files need merging" &&
@@ -405,6 +407,7 @@ test_expect_success 'deleted vs modified submodule' '
        ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
        ( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
        ( yes "r" | git mergetool submod ) &&
+       ( yes "d" | git mergetool submod~conflicts ) &&
        test ! -e submod &&
        test -d submod.orig &&
        git submodule update -N &&
@@ -421,6 +424,7 @@ test_expect_success 'deleted vs modified submodule' '
        ( yes "" | git mergetool both >/dev/null 2>&1 ) &&
        ( yes "d" | git mergetool file11 file12 >/dev/null 2>&1 ) &&
        ( yes "l" | git mergetool submod ) &&
+       ( yes "d" | git mergetool submod~conflicts ) &&
        test "$(cat submod/bar)" = "master submodule" &&
        git submodule update -N &&
        test "$(cat submod/bar)" = "master submodule" &&
-- 
2.18.0.550.g44d6daf40a.dirty

Reply via email to