From: Junio C Hamano Date: Tue, 17 May 2016 21:38:32 +0000 (-0700) Subject: Merge branch 'jc/ll-merge-internal' X-Git-Tag: v2.9.0-rc0~26 X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/243a7f05571f62e3635c90000c07693c1b78b977?hp=-c Merge branch 'jc/ll-merge-internal' "git rerere" can get confused by conflict markers deliberately left by the inner merge step, because they are indistinguishable from the real conflict markers left by the outermost merge which are what the end user and "rerere" need to look at. This was fixed by making the conflict markers left by the inner merges a bit longer. * jc/ll-merge-internal: t6036: remove pointless test that expects failure ll-merge: use a longer conflict marker for internal merge ll-merge: fix typo in comment --- 243a7f05571f62e3635c90000c07693c1b78b977 diff --combined ll-merge.c index ff4a43a982,e5ff7f6cb9..ad8be42f91 --- a/ll-merge.c +++ b/ll-merge.c @@@ -9,7 -9,6 +9,7 @@@ #include "xdiff-interface.h" #include "run-command.h" #include "ll-merge.h" +#include "quote.h" struct ll_merge_driver; @@@ -47,7 -46,9 +47,9 @@@ static int ll_binary_merge(const struc assert(opts); /* - * The tentative merge result is the or common ancestor for an internal merge. + * The tentative merge result is the common ancestor for an + * internal merge. For the final merge, it is "ours" by + * default but -Xours/-Xtheirs can tweak the choice. */ if (opts->virtual_ancestor) { stolen = orig; @@@ -89,10 -90,7 +91,10 @@@ static int ll_xdl_merge(const struct ll xmparam_t xmp; assert(opts); - if (buffer_is_binary(orig->ptr, orig->size) || + if (orig->size > MAX_XDIFF_SIZE || + src1->size > MAX_XDIFF_SIZE || + src2->size > MAX_XDIFF_SIZE || + buffer_is_binary(orig->ptr, orig->size) || buffer_is_binary(src1->ptr, src1->size) || buffer_is_binary(src2->ptr, src2->size)) { return ll_binary_merge(drv_unused, result, @@@ -145,11 -143,11 +147,11 @@@ static struct ll_merge_driver ll_merge_ { "union", "built-in union merge", ll_union_merge }, }; -static void create_temp(mmfile_t *src, char *path) +static void create_temp(mmfile_t *src, char *path, size_t len) { int fd; - strcpy(path, ".merge_file_XXXXXX"); + xsnprintf(path, len, ".merge_file_XXXXXX"); fd = xmkstemp(path); if (write_in_full(fd, src->ptr, src->size) != src->size) die_errno("unable to write temp-file"); @@@ -170,30 -168,27 +172,30 @@@ static int ll_ext_merge(const struct ll { char temp[4][50]; struct strbuf cmd = STRBUF_INIT; - struct strbuf_expand_dict_entry dict[5]; + struct strbuf_expand_dict_entry dict[6]; + struct strbuf path_sq = STRBUF_INIT; const char *args[] = { NULL, NULL }; int status, fd, i; struct stat st; assert(opts); + sq_quote_buf(&path_sq, path); dict[0].placeholder = "O"; dict[0].value = temp[0]; dict[1].placeholder = "A"; dict[1].value = temp[1]; dict[2].placeholder = "B"; dict[2].value = temp[2]; dict[3].placeholder = "L"; dict[3].value = temp[3]; - dict[4].placeholder = NULL; dict[4].value = NULL; + dict[4].placeholder = "P"; dict[4].value = path_sq.buf; + dict[5].placeholder = NULL; dict[5].value = NULL; if (fn->cmdline == NULL) die("custom merge driver %s lacks command line.", fn->name); result->ptr = NULL; result->size = 0; - create_temp(orig, temp[0]); - create_temp(src1, temp[1]); - create_temp(src2, temp[2]); - sprintf(temp[3], "%d", marker_size); + create_temp(orig, temp[0], sizeof(temp[0])); + create_temp(src1, temp[1], sizeof(temp[1])); + create_temp(src2, temp[2], sizeof(temp[2])); + xsnprintf(temp[3], sizeof(temp[3]), "%d", marker_size); strbuf_expand(&cmd, fn->cmdline, strbuf_expand_dict_cb, &dict); @@@ -205,7 -200,7 +207,7 @@@ if (fstat(fd, &st)) goto close_bad; result->size = st.st_size; - result->ptr = xmalloc(result->size + 1); + result->ptr = xmallocz(result->size); if (read_in_full(fd, result->ptr, result->size) != result->size) { free(result->ptr); result->ptr = NULL; @@@ -217,7 -212,6 +219,7 @@@ for (i = 0; i < 3; i++) unlink_or_warn(temp[i]); strbuf_release(&cmd); + strbuf_release(&path_sq); return status; } @@@ -277,7 -271,6 +279,7 @@@ static int read_merge_config(const cha * %A - temporary file name for our version. * %B - temporary file name for the other branches' version. * %L - conflict marker length + * %P - the original path (safely quoted for the shell) * * The external merge driver should write the results in the * file named by %A, and signal that it has done with zero exit @@@ -383,8 -376,12 +385,12 @@@ int ll_merge(mmbuffer_t *result_buf } } driver = find_ll_merge_driver(ll_driver_name); - if (opts->virtual_ancestor && driver->recursive) - driver = find_ll_merge_driver(driver->recursive); + + if (opts->virtual_ancestor) { + if (driver->recursive) + driver = find_ll_merge_driver(driver->recursive); + marker_size += 2; + } return driver->fn(driver, result_buf, path, ancestor, ancestor_label, ours, our_label, theirs, their_label, opts, marker_size); diff --combined t/t6036-recursive-corner-cases.sh index 9d6621c056,c7b0ae6379..18aa88b5c0 --- a/t/t6036-recursive-corner-cases.sh +++ b/t/t6036-recursive-corner-cases.sh @@@ -86,7 -86,7 +86,7 @@@ test_expect_success 'setup criss-cross rm -rf .git && git init && - ten="0 1 2 3 4 5 6 7 8 9" + ten="0 1 2 3 4 5 6 7 8 9" && for i in $ten do echo line $i in a sample file @@@ -195,7 -195,12 +195,7 @@@ test_expect_success 'git detects differ git reset --hard && git checkout D^0 && - git merge -s recursive E^0 && { - echo "BAD: should have conflicted" - test "Incorrectly merged content" = "$(cat new_a)" && - echo "BAD: Silently accepted wrong content" - return 1 - } + test_must_fail git merge -s recursive E^0 && test 3 = $(git ls-files -s | wc -l) && test 3 = $(git ls-files -u | wc -l) && @@@ -212,7 -217,8 +212,8 @@@ -L "" \ -L "Temporary merge branch 1" \ merged empty merge-me && - test $(git rev-parse :1:new_a) = $(git hash-object merged) + sed -e "s/^\([<=>]\)/\1\1\1/" merged >merged-internal && + test $(git rev-parse :1:new_a) = $(git hash-object merged-internal) ' # @@@ -298,89 -304,6 +299,6 @@@ test_expect_success 'git detects confli test $(git rev-parse :3:file) = $(git rev-parse B:file) ' - # - # criss-cross + modify/modify with very contrived file contents: - # - # B D - # o---o - # / \ / \ - # A o X ? F - # \ / \ / - # o---o - # C E - # - # Commit A: file with contents 'A\n' - # Commit B: file with contents 'B\n' - # Commit C: file with contents 'C\n' - # Commit D: file with contents 'D\n' - # Commit E: file with contents: - # <<<<<<< Temporary merge branch 1 - # C - # ======= - # B - # >>>>>>> Temporary merge branch 2 - # - # Now, when we merge commits D & E, does git detect the conflict? - - test_expect_success 'setup differently handled merges of content conflict' ' - git clean -fdqx && - rm -rf .git && - git init && - - echo A >file && - git add file && - test_tick && - git commit -m A && - - git branch B && - git checkout -b C && - echo C >file && - git add file && - test_tick && - git commit -m C && - - git checkout B && - echo B >file && - git add file && - test_tick && - git commit -m B && - - git checkout B^0 && - test_must_fail git merge C && - echo D >file && - git add file && - test_tick && - git commit -m D && - git tag D && - - git checkout C^0 && - test_must_fail git merge B && - cat <file && - <<<<<<< Temporary merge branch 1 - C - ======= - B - >>>>>>> Temporary merge branch 2 - EOF - git add file && - test_tick && - git commit -m E && - git tag E - ' - - test_expect_failure 'git detects conflict w/ criss-cross+contrived resolution' ' - git checkout D^0 && - - test_must_fail git merge -s recursive E^0 && - - test 3 -eq $(git ls-files -s | wc -l) && - test 3 -eq $(git ls-files -u | wc -l) && - test 0 -eq $(git ls-files -o | wc -l) && - - test $(git rev-parse :2:file) = $(git rev-parse D:file) && - test $(git rev-parse :3:file) = $(git rev-parse E:file) - ' - # # criss-cross + d/f conflict via add/add: # Commit A: Neither file 'a' nor directory 'a/' exists. @@@ -528,7 -451,7 +446,7 @@@ test_expect_success 'merge of E2 & D fa test $(git rev-parse :3:a) = $(git rev-parse B:a) && test $(git rev-parse :2:a/file) = $(git rev-parse E2:a/file) && - test $(git rev-parse :1:a/file) = $(git rev-parse C:a/file) + test $(git rev-parse :1:a/file) = $(git rev-parse C:a/file) && test $(git rev-parse :0:ignore-me) = $(git rev-parse A:ignore-me) && test -f a~D^0