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.tr@xxxxxxxxxxxxx/ Signed-off-by: Elijah Newren <newren@xxxxxxxxx> --- 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