{"thread":{"id":"47978","subject":"[RFC PATCH 2/5] merge-recursive: new function for better colliding conflict resolutions","startedAt":"2018-03-05T17:12:02Z","lastAt":"2018-03-12T18:19:39Z","messageCount":9,"participants":["Elijah Newren","SZEDER Gábor"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"341029","messageId":"20180305171125.22331-3-newren@gmail.com","threadId":"47978","inReplyTo":"20180305171125.22331-1-newren@gmail.com","subject":"[RFC PATCH 2/5] merge-recursive: new function for better colliding conflict resolutions","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-03-05T17:11:22Z","receivedAt":"2018-03-05T17:12:02Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"There are three conflict types that represent two (possibly entirely\nunrelated) files colliding at the same location:\n  * add/add\n  * rename/add\n  * rename/rename(2to1)\n\nThese three conflict types already share more similarity than might be\nimmediately apparent from their description: (1) the handling of the\nrename variants already involves removing any entries from the index\ncorresponding to the original file names[*], thus only leaving entries\nin the index for the colliding path; (2) likewise, any trace of the\noriginal file name in the working tree is also removed.  So, in all\nthree cases we're left with how to represent two colliding files in both\nthe index and the working copy.\n\n[*] Technically, this isn't quite true because rename/rename(2to1)\nconflicts in the recursive (o->call_depth > 0) case do an \"unrename\"\nsince about seven years ago.  But even in that case, Junio felt\ncompelled to explain that my decision to \"unrename\" wasn't necessarily\nthe only or right answer -- search for \"Comment from Junio\" in t6036 for\ndetails.\n\nMy initial motivation for looking at these three conflict types was that\nif the handling of these three conflict types is the same, at least in\nthe limited set of cases where a renamed file is unmodified on the side\nof history where the file is not renamed, then a significant performance\nimprovement for rename detection during merges is possible.  However,\nwhile that served as motivation to look at these three types of\nconflicts, the actual goal of this new function is to try to improve the\nhandling for all three cases, not to merely make them the same as each\nother in that special circumstance.\n\n=== Handling the working tree ===\n\nThe previous behavior for these conflict types in regards to the\nworking tree (assuming the file collision occurs at 'foo') was:\n  * add/add does a two-way merge of the two files and records it as 'foo'.\n  * rename/rename(2to1) records the two different files into two new\n    uniquely named files (foo~HEAD and foo~$MERGE), while removing 'foo'\n    from the working tree.\n  * rename/add records the two different files into two different\n    locations, recording the add at foo~$SIDE and, oddly, recording\n    the rename at foo (why is the rename more important than the add?)\n\nSo, the question for what to write to the working tree boils down to\nwhether the two colliding files should be two-way merged and recorded in\nplace, or recorded into separate files.  If the files are similar enough,\nthe two-way merge is probably preferable, but if they're not similar,\nrecording as separate files is probably preferable.  (The same logic that\napplies for the working directory here would also apply to the recursive\ncase, i.e. the o->call_depth > 0 case, as well.)  The code handling the\ndifferent types of conflicts appear to have been written with different\nassumptions about whether the colliding files would be similar.\n\nBut, rather than make an assumption about whether the two files will be\nsimilar, why not just check?  If we simply call estimate_similarity(),\nwe can two-way merge the files if they are similar, and otherwise record\nthe two files at different locations.\n\n=== Handling of the index ===\n\nFor a typical rename, unpack_trees() would set up the index in the\nfollowing fashion:\n           old_path  new_path\n   stage1: 5ca1ab1e  00000000\n   stage2: f005ba11  00000000\n   stage3: 00000000  b0a710ad\nAnd merge-recursive would rewrite this to\n           new_path\n   stage1: 5ca1ab1e\n   stage2: f005ba11\n   stage3: b0a710ad\nRemoving old_path from the index means the user won't have to `git rm\nold_path` manually every time a renamed path has a content conflict.\nIt also means they can use `git checkout [--ours|--theirs|--conflict|-m]\nnew_path`, `git diff [--ours|--theirs]` and various other commands that\nwould be difficult otherwise.\n\nThis strategy becomes a problem when we have a rename/add or\nrename/rename(2to1) conflict, however, because then we have only three\nslots to store blob sha1s and we need either four or six.  Previously,\nthis was handled by continuing to delete old_path from the index, and\njust outright ignoring any blob shas from old_path.  That had the\ndownside of deleting any trace of changes made to old_path on the other\nside of history.  This function instead does a three-way content merge of\nthe renamed file, and stores the blob sha1 for that at either stage2 or\nstage3 for new_path (depending on which side the rename came from).  That\nhas the advantage of bringing information about changes on both sides and\nstill allows for easy resolution (no need to git rm old_path, etc.), but\ndoes have the downside that if the content merge had conflict markers,\nthen what we store in the index is the sha1 of a blob with conflict\nmarkers.  While that is a downside, it seems less problematic than the\ndownsides of any obvious alternatives, and certainly makes more sense\nthan the previous handling.\n\nSigned-off-by: Elijah Newren <newren@gmail.com>\n---\n diff.h            |   4 ++\n diffcore-rename.c |   6 +--\n merge-recursive.c | 124 ++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 131 insertions(+), 3 deletions(-)\n\ndiff --git a/diff.h b/diff.h\nindex 7cf276f077..810944247c 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -442,4 +442,8 @@ extern void print_stat_summary(FILE *fp, int files,\n \t\t\t       int insertions, int deletions);\n extern void setup_diff_pager(struct diff_options *);\n \n+extern int estimate_similarity(struct diff_filespec *src,\n+\t\t\t       struct diff_filespec *dst,\n+\t\t\t       int minimum_score);\n+\n #endif /* DIFF_H */\ndiff --git a/diffcore-rename.c b/diffcore-rename.c\nindex 245e999fe5..0f4388a9e0 100644\n--- a/diffcore-rename.c\n+++ b/diffcore-rename.c\n@@ -127,9 +127,9 @@ struct diff_score {\n \tshort name_score;\n };\n \n-static int estimate_similarity(struct diff_filespec *src,\n-\t\t\t       struct diff_filespec *dst,\n-\t\t\t       int minimum_score)\n+int estimate_similarity(struct diff_filespec *src,\n+\t\t\tstruct diff_filespec *dst,\n+\t\t\tint minimum_score)\n {\n \t/* src points at a file that existed in the original tree (or\n \t * optionally a file in the destination tree) and dst points\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 5f42c677d5..c54b918dc8 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -1290,6 +1290,130 @@ static struct diff_filespec *filespec_from_entry(struct diff_filespec *target,\n \treturn target;\n }\n \n+#if 0 // #if-0-ing avoids unused function warning; will make live in next commit\n+static int handle_file_collision(struct merge_options *o,\n+\t\t\t\t const char *collide_path,\n+\t\t\t\t const char *prev_path1,\n+\t\t\t\t const char *prev_path2,\n+\t\t\t\t const char *branch1, const char *branch2,\n+\t\t\t\t const struct object_id *a_oid,\n+\t\t\t\t unsigned int a_mode,\n+\t\t\t\t const struct object_id *b_oid,\n+\t\t\t\t unsigned int b_mode,\n+\t\t\t\t unsigned int conflict_markers_already_present)\n+{\n+\tstruct merge_file_info mfi;\n+\tstruct diff_filespec null, a, b;\n+\tint minimum_score;\n+\tchar *new_path1, *new_path2;\n+\n+\t/* Remove rename sources if rename/add or rename/rename(2to1) */\n+\tif (prev_path1)\n+\t\tremove_file(o, 1, prev_path1,\n+\t\t\t    o->call_depth || would_lose_untracked(prev_path1));\n+\tif (prev_path2)\n+\t\tremove_file(o, 1, prev_path2,\n+\t\t\t    o->call_depth || would_lose_untracked(prev_path2));\n+\n+\t/*\n+\t * Remove the collision path, if it wouldn't cause dirty contents\n+\t * or an untracked file to get lost.  We'll either overwrite with\n+\t * merged contents, or just write out to differently named files.\n+\t */\n+\tif (was_dirty(o, collide_path))\n+\t\toutput(o, 1, _(\"Refusing to lose dirty file at %s\"),\n+\t\t       collide_path);\n+\telse if (would_lose_untracked(collide_path))\n+\t\t/*\n+\t\t * Only way we get here is if both renames were from\n+\t\t * a directory rename AND user had an untracked file\n+\t\t * at the location where both files end up after the\n+\t\t * two directory renames.  See testcase 10d of t6043.\n+\t\t */\n+\t\toutput(o, 1, _(\"Refusing to lose untracked file at \"\n+\t\t\t       \"%s, even though it's in the way.\"),\n+\t\t       collide_path);\n+\telse\n+\t\t/*\n+\t\t * FIXME: It's possible that neither of the two files have\n+\t\t * conflict markers already present, and that they're\n+\t\t * identical, and that the current working copy happens to\n+\t\t * match, in which case we are unnecessarily touching the\n+\t\t * working tree file.  It's not a likely enough scenario\n+\t\t * that I want to code up the checks for it and a better\n+\t\t * fix is available if we restructure how unpack_trees()\n+\t\t * and merge-recursive interoperate anyway, so punting for\n+\t\t * now...\n+\t\t */\n+\t\tremove_file(o, 0, collide_path, 0);\n+\n+\t/* Store things in diff_filespecs for functions that need it */\n+\tmemset(&a, 0, sizeof(struct diff_filespec));\n+\tmemset(&b, 0, sizeof(struct diff_filespec));\n+\tnull.path = a.path = b.path = (char *)collide_path;\n+\toidcpy(&null.oid, &null_oid);\n+\tnull.mode = 0;\n+\toidcpy(&a.oid, a_oid);\n+\ta.mode = a_mode;\n+\ta.oid_valid = 1;\n+\toidcpy(&b.oid, b_oid);\n+\tb.mode = b_mode;\n+\tb.oid_valid = 1;\n+\n+\t/*\n+\t * If the colliding files are similar enough, we can simply merge\n+\t * them.  But we don't want to merge files that have conflict\n+\t * markers in them already, because nested conflict markers are\n+\t * too confusing.\n+\t */\n+\tminimum_score = o->rename_score ? o->rename_score\n+\t\t\t\t\t: DEFAULT_RENAME_SCORE;\n+\tif (!conflict_markers_already_present && minimum_score <=\n+\t    estimate_similarity(&a, &b, o->rename_score)) {\n+\t\tif (merge_file_1(o, &null, &a, &b, branch1, branch2, &mfi))\n+\t\t\treturn -1;\n+\t\tif (update_file(o, mfi.clean, &mfi.oid, mfi.mode, collide_path))\n+\t\t\treturn -1;\n+\t\tif (!mfi.clean && !o->call_depth &&\n+\t\t    update_stages(o, collide_path, NULL, &a, &b))\n+\t\t\treturn -1;\n+\t\treturn mfi.clean;\n+\t}\n+\n+\t/*\n+\t * Put the colliding files into different paths, and record the\n+\t * updated sha1sums in the index\n+\t */\n+\tnew_path1 = (o->call_depth && prev_path1) ? strdup(prev_path1) :\n+\t\t    unique_path(o, collide_path, branch1);\n+\tnew_path2 = (o->call_depth && prev_path2) ? strdup(prev_path2) :\n+\t\t    unique_path(o, collide_path, branch2);\n+\toutput(o, 1, _(\"Renaming collisions at %s to %s and %s instead\"),\n+\t       collide_path, new_path1, new_path2);\n+\n+\tif (update_file(o, 0, a_oid, a_mode, new_path1))\n+\t\treturn -1;\n+\tif (update_file(o, 0, b_oid, b_mode, new_path2))\n+\t\treturn -1;\n+\n+\t/* Update index too, making sure to get stage order correct. */\n+\tif (!o->call_depth) {\n+\t\tif (o->branch1 == branch1) {\n+\t\t\tif (update_stages(o, collide_path, NULL, &a, &b))\n+\t\t\t\treturn -1;\n+\t\t} else {\n+\t\t\tif (update_stages(o, collide_path, NULL, &b, &a))\n+\t\t\t\treturn -1;\n+\t\t}\n+\t}\n+\n+\tfree(new_path2);\n+\tfree(new_path1);\n+\n+\treturn 0; /* not clean */\n+}\n+#endif\n+\n static int handle_file(struct merge_options *o,\n \t\t\tstruct diff_filespec *rename,\n \t\t\tint stage,\n-- \n2.16.0.41.g6a66043158\n\n"},{"id":"341030","messageId":"20180305171125.22331-2-newren@gmail.com","threadId":"47978","inReplyTo":"20180305171125.22331-1-newren@gmail.com","subject":"[RFC PATCH 1/5] Add testcases for improved file collision conflict handling","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-03-05T17:11:21Z","receivedAt":"2018-03-05T17:12:05Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Adds testcases dealing with file collisions for the following types of\nconflicts:\n  * add/add\n  * rename/add\n  * rename/rename(2to1)\nThese tests include expectations for proposed smarter behavior which has\nnot yet been implemented and thus are currently expected to fail.\nSubsequent commits will correct that and explain the new behavior.\n\nSigned-off-by: Elijah Newren <newren@gmail.com>\n---\n t/t6042-merge-rename-corner-cases.sh | 220 +++++++++++++++++++++++++++++++++++\n 1 file changed, 220 insertions(+)\n\ndiff --git a/t/t6042-merge-rename-corner-cases.sh b/t/t6042-merge-rename-corner-cases.sh\nindex 411550d2b6..a6c151ef95 100755\n--- a/t/t6042-merge-rename-corner-cases.sh\n+++ b/t/t6042-merge-rename-corner-cases.sh\n@@ -575,4 +575,224 @@ test_expect_success 'rename/rename/add-dest merge still knows about conflicting\n \ttest ! -f c\n '\n \n+test_conflicts_with_adds_and_renames() {\n+\ttest $1 != 0 && side1=rename || side1=add\n+\ttest $2 != 0 && side2=rename || side2=add\n+\n+\t# Setup:\n+\t#          L\n+\t#         / \\\n+\t#   master   ?\n+\t#         \\ /\n+\t#          R\n+\t#\n+\t# Where:\n+\t#   Both L and R have files named 'three-unrelated' and\n+\t#   'three-related' which collide (i.e. 4 files colliding at two\n+\t#   pathnames).  Each of the colliding files could have been\n+\t#   involved in a rename, in which case there was a file named\n+\t#   'one-[un]related' or 'two-[un]related' that was modified on the\n+\t#   opposite side of history and renamed into the collision on this\n+\t#   side of history.\n+\t#\n+\t# Questions for both sets of collisions:\n+\t#   1) The index should contain both a stage 2 and stage 3 entry\n+\t#      for the colliding file.  Does it?\n+\t#   2) When renames are involved, the content merges are clean, so\n+\t#      the index should reflect the content merges, not merely the\n+\t#      version of the colliding file from the prior commit.  Does\n+\t#      it?\n+\t#\n+\t# Questions for three-unrelated:\n+\t#   3) There should be files in the worktree named\n+\t#      'three-unrelated~HEAD' and 'three-unrelated~R^0' with the\n+\t#      (content-merged) version of 'three-unrelated' from the\n+\t#      appropriate side of the merge.  Are they present?\n+\t#   4) There should be no file named 'three-unrelated' in the\n+\t#      working tree.  That'd make it too likely that users would\n+\t#      use it instead of carefully looking at both\n+\t#      three-unrelated~HEAD and three-unrelated~R^0.  Is it\n+\t#      correctly missing?\n+\t#\n+\t# Questions for three-related:\n+\t#   3) There should be a file in the worktree named three-related\n+\t#      containing the two-way merged contents of the content-merged\n+\t#      versions of three-related from each of the two colliding\n+\t#      files.  Is it present?\n+\t#   4) There should not be any three-related~* files in the working\n+\t#      tree.\n+\ttest_expect_success \"setup simple $side1/$side2 conflict\" '\n+\t\ttest_create_repo simple_${side1}_${side2} &&\n+\t\t(\n+\t\t\tcd simple_${side1}_${side2} &&\n+\n+\t\t\t# Create a simple file with 10 lines\n+\t\t\tten=\"0 1 2 3 4 5 6 7 8 9\" &&\n+\t\t\tfor i in $ten\n+\t\t\tdo\n+\t\t\t\techo line $i in a sample file\n+\t\t\tdone >unrelated1_v1 &&\n+\t\t\t# Create a 2nd version of same file with one more line\n+\t\t\tcat unrelated1_v1 >unrelated1_v2 &&\n+\t\t\techo another line >>unrelated1_v2 &&\n+\n+\t\t\t# Create an unrelated simple file with 10 lines\n+\t\t\tfor i in $ten\n+\t\t\tdo\n+\t\t\t\techo line $i in another sample file\n+\t\t\tdone >unrelated2_v1 &&\n+\t\t\t# Create a 2nd version of same file with one more line\n+\t\t\tcat unrelated2_v1 >unrelated2_v2 &&\n+\t\t\techo another line >>unrelated2_v2 &&\n+\n+\t\t\t# Create some related files now\n+\t\t\tfor i in $ten\n+\t\t\tdo\n+\t\t\t\techo Random base content line $i\n+\t\t\tdone >related1_v1 &&\n+\t\t\tcp -a related1_v1 related1_v2 &&\n+\t\t\techo modification >>related1_v2 &&\n+\n+\t\t\tcp -a related1_v1 related2_v1 &&\n+\t\t\techo more stuff >>related2_v1 &&\n+\t\t\tcp -a related2_v1 related2_v2 &&\n+\t\t\techo yet more stuff >>related2_v2 &&\n+\n+\t\t\t# Use a tag to record both these files for simple\n+\t\t\t# access, and clean out these untracked files\n+\t\t\tgit tag unrelated1_v1 `git hash-object -w unrelated1_v1` &&\n+\t\t\tgit tag unrelated1_v2 `git hash-object -w unrelated1_v2` &&\n+\t\t\tgit tag unrelated2_v1 `git hash-object -w unrelated2_v1` &&\n+\t\t\tgit tag unrelated2_v2 `git hash-object -w unrelated2_v2` &&\n+\t\t\tgit tag related1_v1 `git hash-object -w related1_v1` &&\n+\t\t\tgit tag related1_v2 `git hash-object -w related1_v2` &&\n+\t\t\tgit tag related2_v1 `git hash-object -w related2_v1` &&\n+\t\t\tgit tag related2_v2 `git hash-object -w related2_v2` &&\n+\t\t\tgit clean -f &&\n+\n+\t\t\t# Setup merge-base, consisting of files named \"one-*\"\n+\t\t\t# and \"two-*\" if renames were involved.\n+\t\t\ttouch irrelevant_file &&\n+\t\t\tgit add irrelevant_file &&\n+\t\t\tif [ $side1 == \"rename\" ]; then\n+\t\t\t\tgit show unrelated1_v1 >one-unrelated &&\n+\t\t\t\tgit add one-unrelated\n+\t\t\t\tgit show related1_v1 >one-related &&\n+\t\t\t\tgit add one-related\n+\t\t\tfi &&\n+\t\t\tif [ $side2 == \"rename\" ]; then\n+\t\t\t\tgit show unrelated2_v1 >two-unrelated &&\n+\t\t\t\tgit add two-unrelated\n+\t\t\t\tgit show related2_v1 >two-related &&\n+\t\t\t\tgit add two-related\n+\t\t\tfi &&\n+\t\t\ttest_tick && git commit -m initial &&\n+\n+\t\t\tgit branch L &&\n+\t\t\tgit branch R &&\n+\n+\t\t\t# Handle the left side\n+\t\t\tgit checkout L &&\n+\t\t\tif [ $side1 == \"rename\" ]; then\n+\t\t\t\tgit mv one-unrelated three-unrelated\n+\t\t\t\tgit mv one-related   three-related\n+\t\t\telse\n+\t\t\t\tgit show unrelated1_v2 >three-unrelated &&\n+\t\t\t\tgit add three-unrelated\n+\t\t\t\tgit show related1_v2 >three-related &&\n+\t\t\t\tgit add three-related\n+\t\t\tfi &&\n+\t\t\tif [ $side2 == \"rename\" ]; then\n+\t\t\t\tgit show unrelated2_v2 >two-unrelated &&\n+\t\t\t\tgit add two-unrelated\n+\t\t\t\tgit show related2_v2 >two-related &&\n+\t\t\t\tgit add two-related\n+\t\t\tfi &&\n+\t\t\ttest_tick && git commit -m L &&\n+\n+\t\t\t# Handle the right side\n+\t\t\tgit checkout R &&\n+\t\t\tif [ $side1 == \"rename\" ]; then\n+\t\t\t\tgit show unrelated1_v2 >one-unrelated &&\n+\t\t\t\tgit add one-unrelated\n+\t\t\t\tgit show related1_v2 >one-related &&\n+\t\t\t\tgit add one-related\n+\t\t\tfi &&\n+\t\t\tif [ $side2 == \"rename\" ]; then\n+\t\t\t\tgit mv two-unrelated three-unrelated\n+\t\t\t\tgit mv two-related three-related\n+\t\t\telse\n+\t\t\t\tgit show unrelated2_v2 >three-unrelated &&\n+\t\t\t\tgit add three-unrelated\n+\t\t\t\tgit show related2_v2 >three-related &&\n+\t\t\t\tgit add three-related\n+\t\t\tfi &&\n+\t\t\ttest_tick && git commit -m R\n+\t\t)\n+\t'\n+\n+\ttest_expect_failure \"check simple $side1/$side2 conflict\" '\n+\t\t(\n+\t\t\tcd simple_${side1}_${side2} &&\n+\n+\t\t\tgit checkout L^0 &&\n+\n+\t\t\t# Merge must fail; there is a conflict\n+\t\t\ttest_must_fail git merge -s recursive R^0 &&\n+\n+\t\t\t# Make sure the index has the right number of entries\n+\t\t\tgit ls-files -s >out &&\n+\t\t\ttest_line_count = 5 out &&\n+\t\t\tgit ls-files -u >out &&\n+\t\t\ttest_line_count = 4 out &&\n+\n+\t\t\t# Nothing should have touched irrelevant_file\n+\t\t\tgit rev-parse >actual \\\n+\t\t\t\t:0:irrelevant_file \\\n+\t\t\t\t:2:three-unrelated :3:three-unrelated \\\n+\t\t\t\t:2:three-related   :3:three-related   &&\n+\t\t\tgit rev-parse >expected \\\n+\t\t\t\tmaster:irrelevant_file \\\n+\t\t\t\tunrelated1_v2      unrelated2_v2 \\\n+\t\t\t\trelated1_v2        related2_v2   &&\n+\n+\t\t\t# Ensure we have the correct number of untracked files\n+\t\t\tgit ls-files -o >out &&\n+\t\t\ttest_line_count = 5 out &&\n+\n+\t\t\t# Make sure each file (with merging if rename\n+\t\t\t# involved) is present in the working tree for the\n+\t\t\t# user to work with.\n+\t\t\tgit hash-object >actual \\\n+\t\t\t\tthree-unrelated~HEAD three-unrelated~R^0 &&\n+\t\t\tgit rev-parse >expected \\\n+\t\t\t\tunrelated1_v2        unrelated2_v2 &&\n+\n+\t\t\t# \"three-unrelated\" should not exist because there is\n+\t\t\t# no reason to give preference to either\n+\t\t\t# three-unrelated~HEAD or three-unrelated~R^0\n+\t\t\ttest_path_is_missing three-unrelated &&\n+\n+\t\t\t# Make sure we have the correct merged contents for\n+\t\t\t# three-related\n+\t\t\tgit show related1_v1 >expected &&\n+\t\t\tcat <<EOF >>expected &&\n+<<<<<<< HEAD\n+modification\n+=======\n+more stuff\n+yet more stuff\n+>>>>>>> R^0\n+EOF\n+\n+\t\t\ttest_cmp expected three-related\n+\t\t)\n+\t'\n+}\n+\n+test_conflicts_with_adds_and_renames 1 1\n+test_conflicts_with_adds_and_renames 1 0\n+test_conflicts_with_adds_and_renames 0 1\n+test_conflicts_with_adds_and_renames 0 0\n+\n test_done\n-- \n2.16.0.41.g6a66043158\n\n"},{"id":"341031","messageId":"20180305171125.22331-1-newren@gmail.com","threadId":"47978","inReplyTo":null,"subject":"[RFC PATCH 0/5] Improve path collision conflict resolutions","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-03-05T17:11:20Z","receivedAt":"2018-03-05T17:12:07Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"This series improves conflict resolutions for conflict types that involve\ntwo (possibly entirely unrelated) files colliding at the same path.  These\nconflict types are:\n  * add/add\n  * rename/add\n  * rename/rename(2to1)\n\nImproving these conflict types has some subtle (though significant)\nperformance ramifications, but for now, I am just concentrating on\nproviding the user with better information for their conflict resolution.\nPerformance considerations will be addressed in a future patch series.\n\nBefore mentioning the improvements, it may be worth noting that these three\ntypes are actually more similar than might at first be apparent: for the\ncases involving renames, any trace of the rename-source path is deleted\nfrom both the index and the working copy (modulo a small technicality\nmentioned in patch 2), leaving just the question of how to represent two\ncolliding files in both the index and the working copy for all three cases.\n\nThere are three important changes this patch series introduces:\n\n  1) Consolidating the code for these three types of conflict resolutions\n     into a single function that all three code paths can call.\n\n  2) Doing content merges for a rename before looking at the path collision\n     between a rename and some other file.  In particular (in what I most\n     suspect others might have an objection to from this series), record\n     that content-merged file -- which may have conflict markers -- in the\n     index at the appropriate higher stage.\n\n  3) Smarter behavior for recording the conflict in the working tree: first\n     checking whether the two colliding files are similar, and then based\n     on that, deciding whether to handle the path collision via a two-way\n     merge of the different files or to instead record the two different\n     files at separate temporary paths.\n\nIn more detail:\n\n1)\n\nThe consolidation seems fairly self-explanatory, but it had a bigger effect\nthan you'd expect: it made it clear that the rename/add conflict resolution\nis broken in multiple ways, and it also made it easier to reason about what\n_should_ be done for rename/add conflicts (something I had struggled with\nwhen I looked at that particular conflict type in the past).  See patch 3\nfor more details.\n\nSidenote: I was kind of surprised that rename/add could have been broken\nfor this long, unnoticed.  Does no one ever hit that conflict in real life?\nIt looks like we did not have very good test coverage for rename/add\nconflicts; a brief search seems to show that we only had a few testcases\ntriggering that conflict type, and all of them were added by me.  Patch 1\ntries to address the testcase problem by adding some tests that try to\ncheck the index and working copy more strictly for all three conflict\ntypes.\n\n2)\n\nPreviously, rename/rename(2to1) conflict resolution for the colliding path\nwould just accept the index changes made by unpack_trees(), meaning that\neach of the higher order stages in the index for the path collision would\nimplicitly ignore any changes to each renamed file from the other side of\nhistory.  Since, as noted above, all traces of the rename-source path were\nremoved from both the index and the working tree, this meant that the index\nwas missing information about changes to such files.  If the user tried to\nresolve the conflict using the index rather than the working copy, they\nwould end up with a silent loss of changes.\n\nI \"fixed\" this by doing the three-way content merge for each renamed-file,\nand then recorded THAT in the index at either stage 2 or 3 as appropriate.\nSince that merge might have conflict markers, that could mean recording in\nthe index a file with conflict markers as though it were a given side.\n(See patch 2 for a more detailed explanation.)  I figure this might be the\nmost controversial change I made.  I can think of a few alternatives, but I\nliked all of them less.  Opinions?\n\nThis change did not require any significant changes to the testsuite; the\ndifference between the old and new behavior was essentially untested.\n\n(rename/add was even worse: not recording _any_ higher order stages in the\nindex, and thus partially hiding the fact that the path was involved in a\nconflict at all.)\n\n3)\n\nGiven the similarity between the conflict types, the big question for\nhandling the conflict in the working tree was whether the two colliding\nfiles should be two-way merged and recorded in place (as add/add does,\nwhich seems to be preferable if the two files are similar), or whether the\ntwo files should be recorded into separate files (as rename/add and\nrename/rename(2to1) do, which seems to be preferable if the two files are\ndissimilar).  The code handling the different types of conflicts appear to\nhave been written with different assumptions about whether the colliding\nfiles would be similar.\n\nBut, rather than make an assumption about whether the two files will be\nsimilar, why not just check and then make the best choice based on that?\nThus, this code makes use of estimate_similarity(), and uses that to decide\nwhether to do a two-way content merge or writing unrelated files out to\ndifferently named temporary files.\n\nThis logical change did require changing one to two dozen testcases in the\ntestsuite; I think this is more logical behavior and that the testcases\nwere toy examples utilized to test other things, but maybe someone else has\nan argument for why add/add conflicts should always be two-way merged\nregardless of file dissimilarity?\n\n\nOther notes:\n\nThis series builds on en/rename-directory-detection; there are too many\nconflicts to resolve if I tried to just base on master.\n\n\nElijah Newren (5):\n  Add testcases for improved file collision conflict handling\n  merge-recursive: new function for better colliding conflict\n    resolutions\n  merge-recursive: fix rename/add conflict handling\n  merge-recursive: improve handling for rename/rename(2to1) conflicts\n  merge-recursive: improve handling for add/add conflicts\n\n diff.h                               |   4 +\n diffcore-rename.c                    |   6 +-\n merge-recursive.c                    | 383 +++++++++++++++++++++++------------\n t/t2023-checkout-m.sh                |   2 +-\n t/t3418-rebase-continue.sh           |  27 ++-\n t/t3504-cherry-pick-rerere.sh        |  19 +-\n t/t4200-rerere.sh                    |  12 +-\n t/t6020-merge-df.sh                  |   4 +-\n t/t6024-recursive-merge.sh           |  35 ++--\n t/t6025-merge-symlinks.sh            |   9 +-\n t/t6031-merge-filemode.sh            |   4 +-\n t/t6036-recursive-corner-cases.sh    |  19 +-\n t/t6042-merge-rename-corner-cases.sh | 212 ++++++++++++++++++-\n t/t6043-merge-rename-directories.sh  |  15 +-\n t/t7060-wtstatus.sh                  |   1 +\n t/t7064-wtstatus-pv2.sh              |   4 +-\n t/t7506-status-submodule.sh          |  11 +-\n t/t7610-mergetool.sh                 |  28 +--\n 18 files changed, 588 insertions(+), 207 deletions(-)\n\n-- \n2.16.0.41.g7fdc8a0834\n\n"},{"id":"341032","messageId":"20180305171125.22331-5-newren@gmail.com","threadId":"47978","inReplyTo":"20180305171125.22331-1-newren@gmail.com","subject":"[RFC PATCH 4/5] merge-recursive: improve handling for rename/rename(2to1) conflicts","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-03-05T17:11:24Z","receivedAt":"2018-03-05T17:12:09Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"This makes the rename/rename(2to1) conflicts use the new\nhandle_file_collision() function.  Since that function was based\noriginally on the rename/rename(2to1) handling code, the main\ndifferences here are in what was added.  In particular:\n\n  * If the two colliding files are similar, instead of being stored\n    at collide_path~HEAD and collide_path~MERGE, the files are two-way\n    merged and recorded at collide_path.\n  * Instead of recording the version of the renamed file that existed\n    on the renamed side in the index (thus ignoring any changes that\n    were made to the file on the side of history without the rename),\n    we do a three-way content merge on the renamed path, then store\n    that at either stage 2 or stage 3.\n  * Note that if either of the three-way content merges done for each\n    rename have conflicts, we do NOT try to estimate the similarity of\n    the resulting two files and just automatically consider them to be\n    dissimilar.  This is done to avoid foisting conflicts-of-conflicts\n    on the user.\n\nSigned-off-by: Elijah Newren <newren@gmail.com>\n---\n merge-recursive.c                    | 101 +++++------------------------------\n t/t6042-merge-rename-corner-cases.sh |   2 +-\n 2 files changed, 14 insertions(+), 89 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 403c0006dc..96f0e9cee2 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -657,27 +657,6 @@ static int update_stages(struct merge_options *opt, const char *path,\n \treturn 0;\n }\n \n-static int update_stages_for_stage_data(struct merge_options *opt,\n-\t\t\t\t\tconst char *path,\n-\t\t\t\t\tconst struct stage_data *stage_data)\n-{\n-\tstruct diff_filespec o, a, b;\n-\n-\to.mode = stage_data->stages[1].mode;\n-\toidcpy(&o.oid, &stage_data->stages[1].oid);\n-\n-\ta.mode = stage_data->stages[2].mode;\n-\toidcpy(&a.oid, &stage_data->stages[2].oid);\n-\n-\tb.mode = stage_data->stages[3].mode;\n-\toidcpy(&b.oid, &stage_data->stages[3].oid);\n-\n-\treturn update_stages(opt, path,\n-\t\t\t     is_null_oid(&o.oid) ? NULL : &o,\n-\t\t\t     is_null_oid(&a.oid) ? NULL : &a,\n-\t\t\t     is_null_oid(&b.oid) ? NULL : &b);\n-}\n-\n static void update_entry(struct stage_data *entry,\n \t\t\t struct diff_filespec *o,\n \t\t\t struct diff_filespec *a,\n@@ -1615,7 +1594,6 @@ static int conflict_rename_rename_2to1(struct merge_options *o,\n \tchar *path = c1->path; /* == c2->path */\n \tstruct merge_file_info mfi_c1;\n \tstruct merge_file_info mfi_c2;\n-\tint ret;\n \n \toutput(o, 1, _(\"CONFLICT (rename/rename): \"\n \t       \"Rename %s->%s in %s. \"\n@@ -1623,9 +1601,6 @@ static int conflict_rename_rename_2to1(struct merge_options *o,\n \t       a->path, c1->path, ci->branch1,\n \t       b->path, c2->path, ci->branch2);\n \n-\tremove_file(o, 1, a->path, o->call_depth || would_lose_untracked(a->path));\n-\tremove_file(o, 1, b->path, o->call_depth || would_lose_untracked(b->path));\n-\n \tif (merge_file_special_markers(o, a, c1, &ci->ren1_other,\n \t\t\t\t       o->branch1, c1->path,\n \t\t\t\t       o->branch2, ci->ren1_other.path, &mfi_c1) ||\n@@ -1634,66 +1609,11 @@ static int conflict_rename_rename_2to1(struct merge_options *o,\n \t\t\t\t       o->branch2, c2->path, &mfi_c2))\n \t\treturn -1;\n \n-\tif (o->call_depth) {\n-\t\t/*\n-\t\t * If mfi_c1.clean && mfi_c2.clean, then it might make\n-\t\t * sense to do a two-way merge of those results.  But, I\n-\t\t * think in all cases, it makes sense to have the virtual\n-\t\t * merge base just undo the renames; they can be detected\n-\t\t * again later for the non-recursive merge.\n-\t\t */\n-\t\tremove_file(o, 0, path, 0);\n-\t\tret = update_file(o, 0, &mfi_c1.oid, mfi_c1.mode, a->path);\n-\t\tif (!ret)\n-\t\t\tret = update_file(o, 0, &mfi_c2.oid, mfi_c2.mode,\n-\t\t\t\t\t  b->path);\n-\t} else {\n-\t\tchar *new_path1 = unique_path(o, path, ci->branch1);\n-\t\tchar *new_path2 = unique_path(o, path, ci->branch2);\n-\t\toutput(o, 1, _(\"Renaming %s to %s and %s to %s instead\"),\n-\t\t       a->path, new_path1, b->path, new_path2);\n-\t\tif (was_dirty(o, path))\n-\t\t\toutput(o, 1, _(\"Refusing to lose dirty file at %s\"),\n-\t\t\t       path);\n-\t\telse if (would_lose_untracked(path))\n-\t\t\t/*\n-\t\t\t * Only way we get here is if both renames were from\n-\t\t\t * a directory rename AND user had an untracked file\n-\t\t\t * at the location where both files end up after the\n-\t\t\t * two directory renames.  See testcase 10d of t6043.\n-\t\t\t */\n-\t\t\toutput(o, 1, _(\"Refusing to lose untracked file at \"\n-\t\t\t\t       \"%s, even though it's in the way.\"),\n-\t\t\t       path);\n-\t\telse\n-\t\t\tremove_file(o, 0, path, 0);\n-\t\tret = update_file(o, 0, &mfi_c1.oid, mfi_c1.mode, new_path1);\n-\t\tif (!ret)\n-\t\t\tret = update_file(o, 0, &mfi_c2.oid, mfi_c2.mode,\n-\t\t\t\t\t  new_path2);\n-\t\t/*\n-\t\t * unpack_trees() actually populates the index for us for\n-\t\t * \"normal\" rename/rename(2to1) situtations so that the\n-\t\t * correct entries are at the higher stages, which would\n-\t\t * make the call below to update_stages_for_stage_data\n-\t\t * unnecessary.  However, if either of the renames came\n-\t\t * from a directory rename, then unpack_trees() will not\n-\t\t * have gotten the right data loaded into the index, so we\n-\t\t * need to do so now.  (While it'd be tempting to move this\n-\t\t * call to update_stages_for_stage_data() to\n-\t\t * apply_directory_rename_modifications(), that would break\n-\t\t * our intermediate calls to would_lose_untracked() since\n-\t\t * those rely on the current in-memory index.  See also the\n-\t\t * big \"NOTE\" in update_stages()).\n-\t\t */\n-\t\tif (update_stages_for_stage_data(o, path, ci->dst_entry1))\n-\t\t\tret = -1;\n-\n-\t\tfree(new_path2);\n-\t\tfree(new_path1);\n-\t}\n-\n-\treturn ret;\n+\treturn handle_file_collision(o, path, a->path, b->path,\n+\t\t\t\t     ci->branch1, ci->branch2,\n+\t\t\t\t     &mfi_c1.oid, mfi_c1.mode,\n+\t\t\t\t     &mfi_c2.oid, mfi_c2.mode,\n+\t\t\t\t     !mfi_c1.clean || !mfi_c2.clean);\n }\n \n /*\n@@ -3075,9 +2995,14 @@ static int process_entry(struct merge_options *o,\n \t\t\t\tclean_merge = -1;\n \t\t\tbreak;\n \t\tcase RENAME_TWO_FILES_TO_ONE:\n-\t\t\tclean_merge = 0;\n-\t\t\tif (conflict_rename_rename_2to1(o, conflict_info))\n-\t\t\t\tclean_merge = -1;\n+\t\t\t/*\n+\t\t\t * Probably unclean merge, but if the two renamed\n+\t\t\t * files merge cleanly and the two resulting files\n+\t\t\t * can then be two-way merged cleanly, I guess it's\n+\t\t\t * a clean merge?\n+\t\t\t */\n+\t\t\tclean_merge = conflict_rename_rename_2to1(o,\n+\t\t\t\t\t\t\t\t  conflict_info);\n \t\t\tbreak;\n \t\tdefault:\n \t\t\tentry->processed = 0;\ndiff --git a/t/t6042-merge-rename-corner-cases.sh b/t/t6042-merge-rename-corner-cases.sh\nindex a6c151ef95..cf5ea5a0f9 100755\n--- a/t/t6042-merge-rename-corner-cases.sh\n+++ b/t/t6042-merge-rename-corner-cases.sh\n@@ -358,7 +358,7 @@ test_expect_success 'setup rename/rename (2to1) + modify/modify' '\n \tgit init &&\n \n \tprintf \"1\\n2\\n3\\n4\\n5\\n\" >a &&\n-\tprintf \"5\\n4\\n3\\n2\\n1\\n\" >b &&\n+\tprintf \"9\\n8\\n7\\n6\\n5\\n\" >b &&\n \tgit add a b &&\n \tgit commit -m A &&\n \tgit tag A &&\n-- \n2.16.0.41.g6a66043158\n\n"},{"id":"341033","messageId":"20180305171125.22331-4-newren@gmail.com","threadId":"47978","inReplyTo":"20180305171125.22331-1-newren@gmail.com","subject":"[RFC PATCH 3/5] merge-recursive: fix rename/add conflict handling","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-03-05T17:11:23Z","receivedAt":"2018-03-05T17:12:45Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"This makes the rename/add conflict handling make use of the new\nhandle_file_collision() function, which fixes several bugs and improves\nthings for the rename/add case significantly.  Previously, rename/add\nwould:\n\n  * Not leave any higher order stage entries in the index, making it\n    appear as if there were no conflict.\n  * Would place the rename file at the colliding path, and move the\n    added file elsewhere, which combined with the lack of higher order\n    stage entries felt really odd.  It's not clear to me why the\n    rename should take precedence over the add; if one should be moved\n    out of the way, they both probably should.\n  * In the recursive case, it would do a two way merge of the added\n    file and the version of the renamed file on the renamed side,\n    completely excluding modifications to the renamed file on the\n    unrenamed side of history.\n\nUsing the new handle_file_collision() fixes all of these issues, and\nadds smarts to allow two-way merge OR move colliding files to separate\npaths depending on the similarity of the colliding files.\n\nSigned-off-by: Elijah Newren <newren@gmail.com>\n---\n merge-recursive.c                 | 137 +++++++++++++++++++++++++++-----------\n t/t6036-recursive-corner-cases.sh |  19 +++---\n 2 files changed, 108 insertions(+), 48 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex c54b918dc8..403c0006dc 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -181,6 +181,7 @@ static int oid_eq(const struct object_id *a, const struct object_id *b)\n enum rename_type {\n \tRENAME_NORMAL = 0,\n \tRENAME_DIR,\n+\tRENAME_ADD,\n \tRENAME_DELETE,\n \tRENAME_ONE_FILE_TO_ONE,\n \tRENAME_ONE_FILE_TO_TWO,\n@@ -223,6 +224,7 @@ static inline void setup_rename_conflict_info(enum rename_type rename_type,\n \t\t\t\t\t      struct stage_data *src_entry1,\n \t\t\t\t\t      struct stage_data *src_entry2)\n {\n+\tint ostage1, ostage2;\n \tstruct rename_conflict_info *ci = xcalloc(1, sizeof(struct rename_conflict_info));\n \tci->rename_type = rename_type;\n \tci->pair1 = pair1;\n@@ -240,18 +242,22 @@ static inline void setup_rename_conflict_info(enum rename_type rename_type,\n \t\tdst_entry2->rename_conflict_info = ci;\n \t}\n \n-\tif (rename_type == RENAME_TWO_FILES_TO_ONE) {\n-\t\t/*\n-\t\t * For each rename, there could have been\n-\t\t * modifications on the side of history where that\n-\t\t * file was not renamed.\n-\t\t */\n-\t\tint ostage1 = o->branch1 == branch1 ? 3 : 2;\n-\t\tint ostage2 = ostage1 ^ 1;\n+\t/*\n+\t * For each rename, there could have been\n+\t * modifications on the side of history where that\n+\t * file was not renamed.\n+\t */\n+\tif (rename_type == RENAME_ADD ||\n+\t    rename_type == RENAME_TWO_FILES_TO_ONE) {\n+\t\tostage1 = o->branch1 == branch1 ? 3 : 2;\n \n \t\tci->ren1_other.path = pair1->one->path;\n \t\toidcpy(&ci->ren1_other.oid, &src_entry1->stages[ostage1].oid);\n \t\tci->ren1_other.mode = src_entry1->stages[ostage1].mode;\n+\t}\n+\n+\tif (rename_type == RENAME_TWO_FILES_TO_ONE) {\n+\t\tostage2 = ostage1 ^ 1;\n \n \t\tci->ren2_other.path = pair2->one->path;\n \t\toidcpy(&ci->ren2_other.oid, &src_entry2->stages[ostage2].oid);\n@@ -1119,6 +1125,18 @@ static int merge_file_special_markers(struct merge_options *o,\n \tchar *side2 = NULL;\n \tint ret;\n \n+\tif (o->branch1 != branch1) {\n+\t\t/*\n+\t\t * It's weird getting a reverse merge with HEAD on the bottom\n+\t\t * and the other branch on the top.  Fix that.\n+\t\t */\n+\t\treturn merge_file_special_markers(o,\n+\t\t\t\t\t\t  one, b, a,\n+\t\t\t\t\t\t  branch2, filename2,\n+\t\t\t\t\t\t  branch1, filename1,\n+\t\t\t\t\t\t  mfi);\n+\t}\n+\n \tif (filename1)\n \t\tside1 = xstrfmt(\"%s:%s\", branch1, filename1);\n \tif (filename2)\n@@ -1290,7 +1308,6 @@ static struct diff_filespec *filespec_from_entry(struct diff_filespec *target,\n \treturn target;\n }\n \n-#if 0 // #if-0-ing avoids unused function warning; will make live in next commit\n static int handle_file_collision(struct merge_options *o,\n \t\t\t\t const char *collide_path,\n \t\t\t\t const char *prev_path1,\n@@ -1307,6 +1324,21 @@ static int handle_file_collision(struct merge_options *o,\n \tint minimum_score;\n \tchar *new_path1, *new_path2;\n \n+\t/*\n+\t * It's easiest to get the correct things into stage 2 and 3, and\n+\t * to make sure that the content merge puts HEAD before the other\n+\t * branch if we just ensure that branch1 == o->branch1.  So, simply\n+\t * flip arguments around if we don't have that.\n+\t */\n+\tif (branch1 != o->branch1) {\n+\t\treturn handle_file_collision(o, collide_path,\n+\t\t\t\t\t     prev_path2, prev_path1,\n+\t\t\t\t\t     branch2, branch1,\n+\t\t\t\t\t     b_oid, b_mode,\n+\t\t\t\t\t     a_oid, a_mode,\n+\t\t\t\t\t     conflict_markers_already_present);\n+\t}\n+\n \t/* Remove rename sources if rename/add or rename/rename(2to1) */\n \tif (prev_path1)\n \t\tremove_file(o, 1, prev_path1,\n@@ -1412,7 +1444,36 @@ static int handle_file_collision(struct merge_options *o,\n \n \treturn 0; /* not clean */\n }\n-#endif\n+\n+static int conflict_rename_add(struct merge_options *o,\n+\t\t\t       struct rename_conflict_info *ci)\n+{\n+\t/* a was renamed to c, and a separate c was added. */\n+\tstruct diff_filespec *a = ci->pair1->one;\n+\tstruct diff_filespec *c = ci->pair1->two;\n+\tchar *path = c->path;\n+\tstruct merge_file_info mfi;\n+\n+\tint other_stage = (ci->branch1 == o->branch1 ? 3 : 2);\n+\n+\toutput(o, 1, _(\"CONFLICT (rename/add): \"\n+\t       \"Rename %s->%s in %s.  Added %s in %s\"),\n+\t       a->path, c->path, ci->branch1,\n+\t       c->path, ci->branch2);\n+\n+\tif (merge_file_special_markers(o, a, c, &ci->ren1_other,\n+\t\t\t\t       o->branch1, path,\n+\t\t\t\t       o->branch2, ci->ren1_other.path, &mfi))\n+\t\treturn -1;\n+\n+\treturn handle_file_collision(o,\n+\t\t\t\t     c->path, a->path, NULL,\n+\t\t\t\t     ci->branch1, ci->branch2,\n+\t\t\t\t     &mfi.oid, mfi.mode,\n+\t\t\t\t     &ci->dst_entry1->stages[other_stage].oid,\n+\t\t\t\t     ci->dst_entry1->stages[other_stage].mode,\n+\t\t\t\t     !mfi.clean);\n+}\n \n static int handle_file(struct merge_options *o,\n \t\t\tstruct diff_filespec *rename,\n@@ -2582,36 +2643,23 @@ static int process_renames(struct merge_options *o,\n \t\t\t\t\t\t      0  /* update_wd    */))\n \t\t\t\t\tclean_merge = -1;\n \t\t\t} else if (!oid_eq(&dst_other.oid, &null_oid)) {\n-\t\t\t\tclean_merge = 0;\n-\t\t\t\ttry_merge = 1;\n-\t\t\t\toutput(o, 1, _(\"CONFLICT (rename/add): Rename %s->%s in %s. \"\n-\t\t\t\t       \"%s added in %s\"),\n-\t\t\t\t       ren1_src, ren1_dst, branch1,\n-\t\t\t\t       ren1_dst, branch2);\n-\t\t\t\tif (o->call_depth) {\n-\t\t\t\t\tstruct merge_file_info mfi;\n-\t\t\t\t\tif (merge_file_one(o, ren1_dst, &null_oid, 0,\n-\t\t\t\t\t\t\t   &ren1->pair->two->oid,\n-\t\t\t\t\t\t\t   ren1->pair->two->mode,\n-\t\t\t\t\t\t\t   &dst_other.oid,\n-\t\t\t\t\t\t\t   dst_other.mode,\n-\t\t\t\t\t\t\t   branch1, branch2, &mfi)) {\n-\t\t\t\t\t\tclean_merge = -1;\n-\t\t\t\t\t\tgoto cleanup_and_return;\n-\t\t\t\t\t}\n-\t\t\t\t\toutput(o, 1, _(\"Adding merged %s\"), ren1_dst);\n-\t\t\t\t\tif (update_file(o, 0, &mfi.oid,\n-\t\t\t\t\t\t\tmfi.mode, ren1_dst))\n-\t\t\t\t\t\tclean_merge = -1;\n-\t\t\t\t\ttry_merge = 0;\n-\t\t\t\t} else {\n-\t\t\t\t\tchar *new_path = unique_path(o, ren1_dst, branch2);\n-\t\t\t\t\toutput(o, 1, _(\"Adding as %s instead\"), new_path);\n-\t\t\t\t\tif (update_file(o, 0, &dst_other.oid,\n-\t\t\t\t\t\t\tdst_other.mode, new_path))\n-\t\t\t\t\t\tclean_merge = -1;\n-\t\t\t\t\tfree(new_path);\n-\t\t\t\t}\n+\t\t\t\t/*\n+\t\t\t\t * Probably not a clean merge, but it's\n+\t\t\t\t * premature to set clean_merge to 0 here,\n+\t\t\t\t * because if the rename merges cleanly and\n+\t\t\t\t * the merge exactly matches the newly added\n+\t\t\t\t * file, then the merge will be clean.\n+\t\t\t\t */\n+\t\t\t\tsetup_rename_conflict_info(RENAME_ADD,\n+\t\t\t\t\t\t\t   ren1->pair,\n+\t\t\t\t\t\t\t   NULL,\n+\t\t\t\t\t\t\t   branch1,\n+\t\t\t\t\t\t\t   branch2,\n+\t\t\t\t\t\t\t   ren1->dst_entry,\n+\t\t\t\t\t\t\t   NULL,\n+\t\t\t\t\t\t\t   o,\n+\t\t\t\t\t\t\t   ren1->src_entry,\n+\t\t\t\t\t\t\t   NULL);\n \t\t\t} else\n \t\t\t\ttry_merge = 1;\n \n@@ -3004,6 +3052,15 @@ static int process_entry(struct merge_options *o,\n \t\t\t\t\t\tconflict_info->branch2))\n \t\t\t\tclean_merge = -1;\n \t\t\tbreak;\n+\t\tcase RENAME_ADD:\n+\t\t\t/*\n+\t\t\t * Probably unclean merge, but if the renamed file\n+\t\t\t * merges cleanly and the result can then be\n+\t\t\t * two-way merged cleanly with the added file, I\n+\t\t\t * guess it's a clean merge?\n+\t\t\t */\n+\t\t\tclean_merge = conflict_rename_add(o, conflict_info);\n+\t\t\tbreak;\n \t\tcase RENAME_DELETE:\n \t\t\tclean_merge = 0;\n \t\t\tif (conflict_rename_delete(o,\ndiff --git a/t/t6036-recursive-corner-cases.sh b/t/t6036-recursive-corner-cases.sh\nindex 18aa88b5c0..09accbc62a 100755\n--- a/t/t6036-recursive-corner-cases.sh\n+++ b/t/t6036-recursive-corner-cases.sh\n@@ -168,7 +168,7 @@ test_expect_success 'setup differently handled merges of rename/add conflict' '\n \tgit branch B &&\n \tgit checkout -b C &&\n \techo 10 >>a &&\n-\techo \"other content\" >>new_a &&\n+\tprintf \"0\\n1\\n2\\n3\\n4\\n5\\n6\\n7\\nfoobar\" >new_a &&\n \tgit add a new_a &&\n \ttest_tick && git commit -m C &&\n \n@@ -179,13 +179,13 @@ test_expect_success 'setup differently handled merges of rename/add conflict' '\n \tgit checkout B^0 &&\n \ttest_must_fail git merge C &&\n \tgit clean -f &&\n+\tgit add new_a &&\n \ttest_tick && git commit -m D &&\n \tgit tag D &&\n \n \tgit checkout C^0 &&\n \ttest_must_fail git merge B &&\n-\trm new_a~HEAD new_a &&\n-\tprintf \"Incorrectly merged content\" >>new_a &&\n+\tprintf \"0\\n1\\n2\\n3\\n4\\n5\\n6\\n7\\nbad merge\" >new_a &&\n \tgit add -u &&\n \ttest_tick && git commit -m E &&\n \tgit tag E\n@@ -204,15 +204,18 @@ test_expect_success 'git detects differently handled merges conflict' '\n \ttest $(git rev-parse :2:new_a) = $(git rev-parse D:new_a) &&\n \ttest $(git rev-parse :3:new_a) = $(git rev-parse E:new_a) &&\n \n-\tgit cat-file -p B:new_a >>merged &&\n+\t# Since A:a == B:new_a, the three-way merge of A:a, B:new_a, and\n+\t# C:a is just C:a.  Then we do a two-way merge of that with\n+\t# C:new_a.\n+\tgit cat-file -p C:a >>merged &&\n \tgit cat-file -p C:new_a >>merge-me &&\n \t>empty &&\n \ttest_must_fail git merge-file \\\n-\t\t-L \"Temporary merge branch 2\" \\\n-\t\t-L \"\" \\\n \t\t-L \"Temporary merge branch 1\" \\\n-\t\tmerged empty merge-me &&\n-\tsed -e \"s/^\\([<=>]\\)/\\1\\1\\1/\" merged >merged-internal &&\n+\t\t-L \"\" \\\n+\t\t-L \"Temporary merge branch 2\" \\\n+\t\tmerge-me empty merged &&\n+\tsed -e \"s/^\\([<=>]\\)/\\1\\1\\1/\" merge-me >merged-internal &&\n \ttest $(git rev-parse :1:new_a) = $(git hash-object merged-internal)\n '\n \n-- \n2.16.0.41.g6a66043158\n\n"},{"id":"341034","messageId":"20180305171125.22331-6-newren@gmail.com","threadId":"47978","inReplyTo":"20180305171125.22331-1-newren@gmail.com","subject":"[RFC PATCH 5/5] merge-recursive: improve handling for add/add conflicts","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-03-05T17:11:25Z","receivedAt":"2018-03-05T17:13:50Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"This makes add/add conflicts use the new handle_file_collision()\nfunction.  This leaves the handling of the index the same, but\nmodifies how the working tree is handled: instead of always doing\na two-way merge of the file contents and recording them at the\ncollision path name, we instead first estimate the similarity of the\ntwo files involved.  If they are not similar, we instead record the\nfile contents into two separate files for the user to inspect.\n\nSeveral testcases had to be modified to either expect files to be\nwritten to different locations, or for the two test colliding files\nto be modified so that they were similar.\n\nSigned-off-by: Elijah Newren <newren@gmail.com>\n---\n merge-recursive.c                    | 25 ++++++++++++++++++++-----\n t/t2023-checkout-m.sh                |  2 +-\n t/t3418-rebase-continue.sh           | 27 +++++++++++++++++++++++----\n t/t3504-cherry-pick-rerere.sh        | 19 ++++++++++++++-----\n t/t4200-rerere.sh                    | 12 ++++++------\n t/t6020-merge-df.sh                  |  4 ++--\n t/t6024-recursive-merge.sh           | 35 +++++++++++++++++++++--------------\n t/t6025-merge-symlinks.sh            |  9 ++++++---\n t/t6031-merge-filemode.sh            |  4 ++--\n t/t6042-merge-rename-corner-cases.sh |  2 +-\n t/t6043-merge-rename-directories.sh  | 15 ++++++++-------\n t/t7060-wtstatus.sh                  |  1 +\n t/t7064-wtstatus-pv2.sh              |  4 ++--\n t/t7506-status-submodule.sh          | 11 +++++++----\n t/t7610-mergetool.sh                 | 28 ++++++++++++++--------------\n 15 files changed, 128 insertions(+), 70 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 96f0e9cee2..edba4fb11c 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -3070,11 +3070,26 @@ static int process_entry(struct merge_options *o,\n \t\t\t\tclean_merge = -1;\n \t\t}\n \t} else if (a_oid && b_oid) {\n-\t\t/* Case C: Added in both (check for same permissions) and */\n-\t\t/* case D: Modified in both, but differently. */\n-\t\tclean_merge = merge_content(o, path, 0 /* file_in_way */,\n-\t\t\t\t\t    o_oid, o_mode, a_oid, a_mode, b_oid, b_mode,\n-\t\t\t\t\t    NULL);\n+\t\tif (!o_oid) {\n+\t\t\t/* Case C: Added in both (check for same permissions) */\n+\t\t\toutput(o, 1,\n+\t\t\t       _(\"CONFLICT (add/add): Merge conflict in %s\"),\n+\t\t\t       path);\n+\t\t\tclean_merge = handle_file_collision(o,\n+\t\t\t\t\t\t\t    path, NULL, NULL,\n+\t\t\t\t\t\t\t    o->branch1,\n+\t\t\t\t\t\t\t    o->branch2,\n+\t\t\t\t\t\t\t    a_oid, a_mode,\n+\t\t\t\t\t\t\t    b_oid, b_mode,\n+\t\t\t\t\t\t\t    0);\n+\t\t} else\n+\t\t\t/* case D: Modified in both, but differently. */\n+\t\t\tclean_merge = merge_content(o, path,\n+\t\t\t\t\t\t    0 /* file_in_way */,\n+\t\t\t\t\t\t    o_oid, o_mode,\n+\t\t\t\t\t\t    a_oid, a_mode,\n+\t\t\t\t\t\t    b_oid, b_mode,\n+\t\t\t\t\t\t    NULL);\n \t} else if (!o_oid && !a_oid && !b_oid) {\n \t\t/*\n \t\t * this entry was deleted altogether. a_mode == 0 means\ndiff --git a/t/t2023-checkout-m.sh b/t/t2023-checkout-m.sh\nindex 7e18985134..2f8ea52318 100755\n--- a/t/t2023-checkout-m.sh\n+++ b/t/t2023-checkout-m.sh\n@@ -27,7 +27,7 @@ clean_branchnames () {\n }\n \n test_expect_success '-m restores 2-way conflicted+resolved file' '\n-\tcp each.txt each.txt.conflicted &&\n+\ttest_must_fail git merge-file -p each.txt~HEAD /dev/null each.txt~master >each.txt.conflicted &&\n \techo resolved >each.txt &&\n \tgit add each.txt &&\n \tgit checkout -m -- each.txt &&\ndiff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh\nindex 7c91a85f43..cf3a82d512 100755\n--- a/t/t3418-rebase-continue.sh\n+++ b/t/t3418-rebase-continue.sh\n@@ -10,10 +10,17 @@ set_fake_editor\n \n test_expect_success 'setup' '\n \ttest_commit \"commit-new-file-F1\" F1 1 &&\n-\ttest_commit \"commit-new-file-F2\" F2 2 &&\n+\tprintf \"1\\n2\\n2\\n\" >F2 &&\n+\tgit add F2 &&\n+\ttest_tick &&\n+\tgit commit -m \"commit-new-file-F2\" &&\n \n \tgit checkout -b topic HEAD^ &&\n-\ttest_commit \"commit-new-file-F2-on-topic-branch\" F2 22 &&\n+\tprintf \"1\\n2\\n22\\n\" >F2 &&\n+\tgit add F2 &&\n+\ttest_tick &&\n+\tgit commit -m \"commit-new-file-F2-on-topic-branch\" &&\n+\tgit tag \"commit-new-file-F2-on-topic-branch\" &&\n \n \tgit checkout master\n '\n@@ -48,7 +55,13 @@ test_expect_success 'rebase --continue can not be used with other options' '\n test_expect_success 'rebase --continue remembers merge strategy and options' '\n \trm -fr .git/rebase-* &&\n \tgit reset --hard commit-new-file-F2-on-topic-branch &&\n-\ttest_commit \"commit-new-file-F3-on-topic-branch\" F3 32 &&\n+\n+\tprintf \"1\\n2\\n32\\n\" >F3 &&\n+\tgit add F3 &&\n+\ttest_tick &&\n+\tgit commit -m \"commit-new-file-F3-on-topic-branch\" &&\n+\tgit tag \"commit-new-file-F3-on-topic-branch\" &&\n+\n \ttest_when_finished \"rm -fr test-bin funny.was.run\" &&\n \tmkdir test-bin &&\n \tcat >test-bin/git-merge-funny <<-EOF &&\n@@ -92,7 +105,13 @@ test_expect_success 'setup rerere database' '\n \trm -fr .git/rebase-* &&\n \tgit reset --hard commit-new-file-F3-on-topic-branch &&\n \tgit checkout master &&\n-\ttest_commit \"commit-new-file-F3\" F3 3 &&\n+\n+\tprintf \"1\\n2\\n3\\n\" >F3 &&\n+\tgit add F3 &&\n+\ttest_tick &&\n+\tgit commit -m \"commit-new-file-F3\" &&\n+\tgit tag \"commit-new-file-F3\" &&\n+\n \ttest_config rerere.enabled true &&\n \ttest_must_fail git rebase -m master topic &&\n \techo \"Resolved\" >F2 &&\ndiff --git a/t/t3504-cherry-pick-rerere.sh b/t/t3504-cherry-pick-rerere.sh\nindex a267b2d144..444ab06ad1 100755\n--- a/t/t3504-cherry-pick-rerere.sh\n+++ b/t/t3504-cherry-pick-rerere.sh\n@@ -4,14 +4,23 @@ test_description='cherry-pick should rerere for conflicts'\n \n . ./test-lib.sh\n \n+test_basic_commit () {\n+\tfile=$2 &&\n+\tprintf \"1\\n2\\n3\\n4\\n5\\n6\\n7\\n8\\n9\\n10\\n$1\" >\"$2\" &&\n+\tgit add \"$file\" &&\n+\ttest_tick\n+\tgit commit -m \"$1\" &&\n+\tgit tag \"$1\"\n+}\n+\n test_expect_success setup '\n-\ttest_commit foo &&\n-\ttest_commit foo-master foo &&\n-\ttest_commit bar-master bar &&\n+\ttest_basic_commit foo foo &&\n+\ttest_basic_commit foo-master foo &&\n+\ttest_basic_commit bar-master bar &&\n \n \tgit checkout -b dev foo &&\n-\ttest_commit foo-dev foo &&\n-\ttest_commit bar-dev bar &&\n+\ttest_basic_commit foo-dev foo &&\n+\ttest_basic_commit bar-dev bar &&\n \tgit config rerere.enabled true\n '\n \ndiff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh\nindex d97d2bebc9..80451907ce 100755\n--- a/t/t4200-rerere.sh\n+++ b/t/t4200-rerere.sh\n@@ -280,36 +280,36 @@ test_expect_success 'setup: file2 added differently in two branches' '\n \tgit reset --hard &&\n \n \tgit checkout -b fourth &&\n-\techo Hallo >file2 &&\n+\tprintf \"1\\n2\\n3\\n4\\n5\\n6\\n7\\n8\\n9\\nHallo\" >file2 &&\n \tgit add file2 &&\n \ttest_tick &&\n \tgit commit -m version1 &&\n \n \tgit checkout third &&\n-\techo Bello >file2 &&\n+\tprintf \"1\\n2\\n3\\n4\\n5\\n6\\n7\\n8\\n9\\nBello\" >file2 &&\n \tgit add file2 &&\n \ttest_tick &&\n \tgit commit -m version2 &&\n \n \ttest_must_fail git merge fourth &&\n-\techo Cello >file2 &&\n+\tprintf \"1\\n2\\n3\\n4\\n5\\n6\\n7\\n8\\n9\\nCello\" >file2 &&\n \tgit add file2 &&\n \tgit commit -m resolution\n '\n \n test_expect_success 'resolution was recorded properly' '\n-\techo Cello >expected &&\n+\tprintf \"1\\n2\\n3\\n4\\n5\\n6\\n7\\n8\\n9\\nCello\" >expected &&\n \n \tgit reset --hard HEAD~2 &&\n \tgit checkout -b fifth &&\n \n-\techo Hallo >file3 &&\n+\tprintf \"1\\n2\\n3\\n4\\n5\\n6\\n7\\n8\\n9\\nHallo\" >file3 &&\n \tgit add file3 &&\n \ttest_tick &&\n \tgit commit -m version1 &&\n \n \tgit checkout third &&\n-\techo Bello >file3 &&\n+\tprintf \"1\\n2\\n3\\n4\\n5\\n6\\n7\\n8\\n9\\nBello\" >file3 &&\n \tgit add file3 &&\n \ttest_tick &&\n \tgit commit -m version2 &&\ndiff --git a/t/t6020-merge-df.sh b/t/t6020-merge-df.sh\nindex 2af1beec5f..4ee27d2e20 100755\n--- a/t/t6020-merge-df.sh\n+++ b/t/t6020-merge-df.sh\n@@ -62,7 +62,7 @@ test_expect_success 'setup modify/delete + directory/file conflict' '\n \t# Throw in letters.txt for sorting order fun\n \t# (\"letters.txt\" sorts between \"letters\" and \"letters/file\")\n \techo i >>letters &&\n-\techo \"version 2\" >letters.txt &&\n+\tprintf \"1\\n2\\n3\\n4\\n5\\nversion 2\" >letters.txt &&\n \tgit add letters letters.txt &&\n \tgit commit -m modified &&\n \n@@ -70,7 +70,7 @@ test_expect_success 'setup modify/delete + directory/file conflict' '\n \tgit rm letters &&\n \tmkdir letters &&\n \t>letters/file &&\n-\techo \"version 1\" >letters.txt &&\n+\tprintf \"1\\n2\\n3\\n4\\n5\\nversion 1\" >letters.txt &&\n \tgit add letters letters.txt &&\n \tgit commit -m deleted\n '\ndiff --git a/t/t6024-recursive-merge.sh b/t/t6024-recursive-merge.sh\nindex 3f59e58dfb..354f70a66f 100755\n--- a/t/t6024-recursive-merge.sh\n+++ b/t/t6024-recursive-merge.sh\n@@ -15,47 +15,47 @@ GIT_COMMITTER_DATE=\"2006-12-12 23:28:00 +0100\"\n export GIT_COMMITTER_DATE\n \n test_expect_success \"setup tests\" '\n-echo 1 > a1 &&\n+printf \"1\\n2\\n3\\n4\\n1\\n\" > a1 &&\n git add a1 &&\n GIT_AUTHOR_DATE=\"2006-12-12 23:00:00\" git commit -m 1 a1 &&\n \n git checkout -b A master &&\n-echo A > a1 &&\n+printf \"1\\n2\\n3\\n4\\nA\\n\" > a1 &&\n GIT_AUTHOR_DATE=\"2006-12-12 23:00:01\" git commit -m A a1 &&\n \n git checkout -b B master &&\n-echo B > a1 &&\n+printf \"1\\n2\\n3\\n4\\nB\\n\" > a1 &&\n GIT_AUTHOR_DATE=\"2006-12-12 23:00:02\" git commit -m B a1 &&\n \n git checkout -b D A &&\n git rev-parse B > .git/MERGE_HEAD &&\n-echo D > a1 &&\n+printf \"1\\n2\\n3\\n4\\nD\\n\" > a1 &&\n git update-index a1 &&\n GIT_AUTHOR_DATE=\"2006-12-12 23:00:03\" git commit -m D &&\n \n git symbolic-ref HEAD refs/heads/other &&\n-echo 2 > a1 &&\n+printf \"1\\n2\\n3\\n4\\n2\\n\" > a1 &&\n GIT_AUTHOR_DATE=\"2006-12-12 23:00:04\" git commit -m 2 a1 &&\n \n git checkout -b C &&\n-echo C > a1 &&\n+printf \"1\\n2\\n3\\n4\\nC\\n\" > a1 &&\n GIT_AUTHOR_DATE=\"2006-12-12 23:00:05\" git commit -m C a1 &&\n \n git checkout -b E C &&\n git rev-parse B > .git/MERGE_HEAD &&\n-echo E > a1 &&\n+printf \"1\\n2\\n3\\n4\\nE\\n\" > a1 &&\n git update-index a1 &&\n GIT_AUTHOR_DATE=\"2006-12-12 23:00:06\" git commit -m E &&\n \n git checkout -b G E &&\n git rev-parse A > .git/MERGE_HEAD &&\n-echo G > a1 &&\n+printf \"1\\n2\\n3\\n4\\nG\\n\" > a1 &&\n git update-index a1 &&\n GIT_AUTHOR_DATE=\"2006-12-12 23:00:07\" git commit -m G &&\n \n git checkout -b F D &&\n git rev-parse C > .git/MERGE_HEAD &&\n-echo F > a1 &&\n+printf \"1\\n2\\n3\\n4\\nF\\n\" > a1 &&\n git update-index a1 &&\n GIT_AUTHOR_DATE=\"2006-12-12 23:00:08\" git commit -m F\n '\n@@ -65,6 +65,10 @@ test_expect_success \"combined merge conflicts\" \"\n \"\n \n cat > expect << EOF\n+1\n+2\n+3\n+4\n <<<<<<< HEAD\n F\n =======\n@@ -76,9 +80,9 @@ test_expect_success \"result contains a conflict\" \"test_cmp expect a1\"\n \n git ls-files --stage > out\n cat > expect << EOF\n-100644 ec3fe2a791706733f2d8fa7ad45d9a9672031f5e 1\ta1\n-100644 cf84443e49e1b366fac938711ddf4be2d4d1d9e9 2\ta1\n-100644 fd7923529855d0b274795ae3349c5e0438333979 3\ta1\n+100644 a2849e152d46797b83870a479775e980d9a138b1 1\ta1\n+100644 b1a886a67c3f471cec4d4112a91dc6caa6e3b709 2\ta1\n+100644 e9d143c719e446df2bafb4b6aba8ed511ecd63b1 3\ta1\n EOF\n \n test_expect_success \"virtual trees were processed\" \"test_cmp expect out\"\n@@ -87,10 +91,13 @@ test_expect_success 'refuse to merge binary files' '\n \tgit reset --hard &&\n \tprintf \"\\0\" > binary-file &&\n \tgit add binary-file &&\n-\tgit commit -m binary &&\n-\tgit checkout G &&\n+\tgit commit -m initial-binary &&\n \tprintf \"\\0\\0\" > binary-file &&\n \tgit add binary-file &&\n+\tgit commit -m binary1 &&\n+\tgit checkout -b H HEAD~1 &&\n+\tprintf \"\\0\\0\\0\" > binary-file &&\n+\tgit add binary-file &&\n \tgit commit -m binary2 &&\n \ttest_must_fail git merge F > merge.out 2> merge.err &&\n \tgrep \"Cannot merge binary files: binary-file (HEAD vs. F)\" merge.err\ndiff --git a/t/t6025-merge-symlinks.sh b/t/t6025-merge-symlinks.sh\nindex 433c4de08f..08eb85e703 100755\n--- a/t/t6025-merge-symlinks.sh\n+++ b/t/t6025-merge-symlinks.sh\n@@ -37,7 +37,8 @@ test_must_fail git merge master'\n \n test_expect_success \\\n 'the merge result must be a file' '\n-test -f symlink'\n+test -f symlink~HEAD &&\n+test -f symlink~master'\n \n test_expect_success \\\n 'merge master into b-file, which has a file instead of a symbolic link' '\n@@ -46,7 +47,8 @@ test_must_fail git merge master'\n \n test_expect_success \\\n 'the merge result must be a file' '\n-test -f symlink'\n+test -f symlink~HEAD &&\n+test -f symlink~master'\n \n test_expect_success \\\n 'merge b-file, which has a file instead of a symbolic link, into master' '\n@@ -56,6 +58,7 @@ test_must_fail git merge b-file'\n \n test_expect_success \\\n 'the merge result must be a file' '\n-test -f symlink'\n+test -f symlink~HEAD &&\n+test -f symlink~master'\n \n test_done\ndiff --git a/t/t6031-merge-filemode.sh b/t/t6031-merge-filemode.sh\nindex 7d06461f13..79e77f391f 100755\n--- a/t/t6031-merge-filemode.sh\n+++ b/t/t6031-merge-filemode.sh\n@@ -40,12 +40,12 @@ do_one_mode resolve b1 a1\n test_expect_success 'set up mode change in both branches' '\n \tgit reset --hard HEAD &&\n \tgit checkout -b a2 master &&\n-\t: >file2 &&\n+\techo true >file2 &&\n \tH=$(git hash-object file2) &&\n \ttest_chmod +x file2 &&\n \tgit commit -m a2 &&\n \tgit checkout -b b2 master &&\n-\t: >file2 &&\n+\techo true >file2 &&\n \tgit add file2 &&\n \tgit commit -m b2 &&\n \t{\ndiff --git a/t/t6042-merge-rename-corner-cases.sh b/t/t6042-merge-rename-corner-cases.sh\nindex cf5ea5a0f9..7bd6d64671 100755\n--- a/t/t6042-merge-rename-corner-cases.sh\n+++ b/t/t6042-merge-rename-corner-cases.sh\n@@ -731,7 +731,7 @@ test_conflicts_with_adds_and_renames() {\n \t\t)\n \t'\n \n-\ttest_expect_failure \"check simple $side1/$side2 conflict\" '\n+\ttest_expect_success \"check simple $side1/$side2 conflict\" '\n \t\t(\n \t\t\tcd simple_${side1}_${side2} &&\n \ndiff --git a/t/t6043-merge-rename-directories.sh b/t/t6043-merge-rename-directories.sh\nindex 2e28f2908d..a5560f7d94 100755\n--- a/t/t6043-merge-rename-directories.sh\n+++ b/t/t6043-merge-rename-directories.sh\n@@ -996,7 +996,7 @@ test_expect_success '5b-check: Rename/delete in order to get add/add/add conflic\n \t\tgit ls-files -u >out &&\n \t\ttest_line_count = 2 out &&\n \t\tgit ls-files -o >out &&\n-\t\ttest_line_count = 1 out &&\n+\t\ttest_line_count = 3 out &&\n \n \t\tgit rev-parse >actual \\\n \t\t\t:0:y/b :0:y/c :0:y/e :2:y/d :3:y/d &&\n@@ -1005,7 +1005,9 @@ test_expect_success '5b-check: Rename/delete in order to get add/add/add conflic\n \t\ttest_cmp expect actual &&\n \n \t\ttest_must_fail git rev-parse :1:y/d &&\n-\t\ttest_path_is_file y/d\n+\t\ttest_path_is_file y/d~HEAD &&\n+\t\ttest_path_is_file y/d~B^0 &&\n+\t\ttest_path_is_missing y/d\n \t)\n '\n \n@@ -1077,7 +1079,7 @@ test_expect_success '5c-check: Transitive rename would cause rename/rename/renam\n \t\tgit ls-files -u >out &&\n \t\ttest_line_count = 6 out &&\n \t\tgit ls-files -o >out &&\n-\t\ttest_line_count = 3 out &&\n+\t\ttest_line_count = 5 out &&\n \n \t\tgit rev-parse >actual \\\n \t\t\t:0:y/b :0:y/c :0:y/e &&\n@@ -1093,13 +1095,12 @@ test_expect_success '5c-check: Transitive rename would cause rename/rename/renam\n \t\ttest_cmp expect actual &&\n \n \t\tgit hash-object >actual \\\n-\t\t\tw/d~HEAD w/d~B^0 z/d &&\n+\t\t\tw/d~HEAD w/d~B^0 y/d~HEAD y/d~B^0 z/d &&\n \t\tgit rev-parse >expect \\\n-\t\t\tO:x/d    B:w/d   O:x/d &&\n+\t\t\tO:x/d    B:w/d   A:y/d    B:y/d   O:x/d &&\n \t\ttest_cmp expect actual &&\n \t\ttest_path_is_missing x/d &&\n-\t\ttest_path_is_file y/d &&\n-\t\tgrep -q \"<<<<\" y/d  # conflict markers should be present\n+\t\ttest_path_is_missing y/d\n \t)\n '\n \ndiff --git a/t/t7060-wtstatus.sh b/t/t7060-wtstatus.sh\nindex 53cf42fac1..a7888f9fc3 100755\n--- a/t/t7060-wtstatus.sh\n+++ b/t/t7060-wtstatus.sh\n@@ -214,6 +214,7 @@ EOF\n \tgit status --untracked-files=no >actual &&\n \ttest_i18ncmp expected actual &&\n \tgit reset --hard &&\n+\tgit clean -f *~* &&\n \tgit checkout master\n '\n \ndiff --git a/t/t7064-wtstatus-pv2.sh b/t/t7064-wtstatus-pv2.sh\nindex e319fa2e84..c4d3b1d7e4 100755\n--- a/t/t7064-wtstatus-pv2.sh\n+++ b/t/t7064-wtstatus-pv2.sh\n@@ -259,14 +259,14 @@ test_expect_success 'verify AA (add-add) conflict' '\n \n \tgit branch AA_A master &&\n \tgit checkout AA_A &&\n-\techo \"Branch AA_A\" >conflict.txt &&\n+\tprintf \"1\\n2\\n3\\n4\\n5\\n6\\nBranch AA_A\" >conflict.txt &&\n \tOID_AA_A=$(git hash-object -t blob -- conflict.txt) &&\n \tgit add conflict.txt &&\n \tgit commit -m \"branch aa_a\" &&\n \n \tgit branch AA_B master &&\n \tgit checkout AA_B &&\n-\techo \"Branch AA_B\" >conflict.txt &&\n+\tprintf \"1\\n2\\n3\\n4\\n5\\n6\\nBranch AA_B\" >conflict.txt &&\n \tOID_AA_B=$(git hash-object -t blob -- conflict.txt) &&\n \tgit add conflict.txt &&\n \tgit commit -m \"branch aa_b\" &&\ndiff --git a/t/t7506-status-submodule.sh b/t/t7506-status-submodule.sh\nindex 9edf6572ed..0514a019be 100755\n--- a/t/t7506-status-submodule.sh\n+++ b/t/t7506-status-submodule.sh\n@@ -240,6 +240,8 @@ test_expect_success 'status -a clean (empty submodule dir)' '\n cat >status_expect <<\\EOF\n AA .gitmodules\n A  sub1\n+?? .gitmodules~HEAD\n+?? .gitmodules~add_sub1\n EOF\n \n test_expect_success 'status with merge conflict in .gitmodules' '\n@@ -273,7 +275,7 @@ index badaa4c,44f999a..0000000\n --- a/.gitmodules\n +++ b/.gitmodules\n @@@ -1,3 -1,3 +1,9 @@@\n-++<<<<<<< HEAD\n+++<<<<<<< ours\n  +[submodule \"sub2\"]\n  +\tpath = sub2\n  +\turl = ../sub2\n@@ -281,7 +283,7 @@ index badaa4c,44f999a..0000000\n + [submodule \"sub1\"]\n + \tpath = sub1\n + \turl = ../sub1\n-++>>>>>>> add_sub1\n+++>>>>>>> theirs\n EOF\n \n cat >diff_submodule_expect <<\\EOF\n@@ -290,7 +292,7 @@ index badaa4c,44f999a..0000000\n --- a/.gitmodules\n +++ b/.gitmodules\n @@@ -1,3 -1,3 +1,9 @@@\n-++<<<<<<< HEAD\n+++<<<<<<< ours\n  +[submodule \"sub2\"]\n  +\tpath = sub2\n  +\turl = ../sub2\n@@ -298,12 +300,13 @@ index badaa4c,44f999a..0000000\n + [submodule \"sub1\"]\n + \tpath = sub1\n + \turl = ../sub1\n-++>>>>>>> add_sub1\n+++>>>>>>> theirs\n EOF\n \n test_expect_success 'diff with merge conflict in .gitmodules' '\n \t(\n \t\tcd super &&\n+\t\tgit checkout -m .gitmodules &&\n \t\tgit diff >../diff_actual 2>&1\n \t) &&\n \ttest_cmp diff_expect diff_actual\ndiff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh\nindex 1a430b9c40..71f41fc9a8 100755\n--- a/t/t7610-mergetool.sh\n+++ b/t/t7610-mergetool.sh\n@@ -37,9 +37,9 @@ test_expect_success 'setup' '\n \tgit checkout -b branch1 master &&\n \tgit submodule update -N &&\n \techo branch1 change >file1 &&\n-\techo branch1 newfile >file2 &&\n+\tprintf \"1\\n2\\n3\\n4\\n5\\n6\\n7\\n8\\n9\\nbranch1 newfile\\n\" >file2 &&\n \techo branch1 spaced >\"spaced name\" &&\n-\techo branch1 both added >both &&\n+\tprintf \"1\\n2\\n3\\n4\\n5\\n6\\n7\\n8\\n9\\n10\\nbranch1 both added\\n\" >both &&\n \techo branch1 change file11 >file11 &&\n \techo branch1 change file13 >file13 &&\n \techo branch1 sub >subdir/file3 &&\n@@ -84,9 +84,9 @@ test_expect_success 'setup' '\n \tgit checkout master &&\n \tgit submodule update -N &&\n \techo master updated >file1 &&\n-\techo master new >file2 &&\n+\tprintf \"1\\n2\\n3\\n4\\n5\\n6\\n7\\n8\\n9\\nmaster new\\n\" >file2 &&\n \techo master updated spaced >\"spaced name\" &&\n-\techo master both added >both &&\n+\tprintf \"1\\n2\\n3\\n4\\n5\\n6\\n7\\n8\\n9\\n10\\nmaster both added\\n\" >both &&\n \techo master updated file12 >file12 &&\n \techo master updated file14 >file14 &&\n \techo master new sub >subdir/file3 &&\n@@ -139,7 +139,7 @@ test_expect_success 'custom mergetool' '\n \t( yes \"d\" | git mergetool file12 >/dev/null 2>&1 ) &&\n \t( yes \"l\" | git mergetool submod >/dev/null 2>&1 ) &&\n \ttest \"$(cat file1)\" = \"master updated\" &&\n-\ttest \"$(cat file2)\" = \"master new\" &&\n+\ttest \"$(git hash-object file2)\" = \"$(git rev-parse master:file2)\" &&\n \ttest \"$(cat subdir/file3)\" = \"master new sub\" &&\n \ttest \"$(cat submod/bar)\" = \"branch1 submodule\" &&\n \tgit commit -m \"branch1 resolved with mergetool\"\n@@ -163,7 +163,7 @@ test_expect_success 'mergetool crlf' '\n \t( yes \"d\" | git mergetool file12 >/dev/null 2>&1 ) &&\n \t( yes \"r\" | git mergetool submod >/dev/null 2>&1 ) &&\n \ttest \"$(printf x | cat file1 -)\" = \"$(printf \"master updated\\r\\nx\")\" &&\n-\ttest \"$(printf x | cat file2 -)\" = \"$(printf \"master new\\r\\nx\")\" &&\n+\ttest \"$(printf x | cat file2 -)\" = \"$(printf \"1\\r\\n2\\r\\n3\\r\\n4\\r\\n5\\r\\n6\\r\\n7\\r\\n8\\r\\n9\\r\\nmaster new\\r\\nx\")\" &&\n \ttest \"$(printf x | cat subdir/file3 -)\" = \"$(printf \"master new sub\\r\\nx\")\" &&\n \tgit submodule update -N &&\n \ttest \"$(cat submod/bar)\" = \"master submodule\" &&\n@@ -171,7 +171,7 @@ test_expect_success 'mergetool crlf' '\n '\n \n test_expect_success 'mergetool in subdir' '\n-\ttest_when_finished \"git reset --hard\" &&\n+\ttest_when_finished \"git reset --hard && git clean -f\" &&\n \tgit checkout -b test$test_count branch1 &&\n \tgit submodule update -N &&\n \t(\n@@ -183,7 +183,7 @@ test_expect_success 'mergetool in subdir' '\n '\n \n test_expect_success 'mergetool on file in parent dir' '\n-\ttest_when_finished \"git reset --hard\" &&\n+\ttest_when_finished \"git reset --hard && git clean -f\" &&\n \tgit checkout -b test$test_count branch1 &&\n \tgit submodule update -N &&\n \t(\n@@ -197,14 +197,14 @@ test_expect_success 'mergetool on file in parent dir' '\n \t\t( yes \"d\" | git mergetool ../file12 >/dev/null 2>&1 ) &&\n \t\t( yes \"l\" | git mergetool ../submod >/dev/null 2>&1 ) &&\n \t\ttest \"$(cat ../file1)\" = \"master updated\" &&\n-\t\ttest \"$(cat ../file2)\" = \"master new\" &&\n+\t\ttest \"$(git hash-object ../file2)\" = \"$(git rev-parse master:file2)\" &&\n \t\ttest \"$(cat ../submod/bar)\" = \"branch1 submodule\" &&\n \t\tgit commit -m \"branch1 resolved with mergetool - subdir\"\n \t)\n '\n \n test_expect_success 'mergetool skips autoresolved' '\n-\ttest_when_finished \"git reset --hard\" &&\n+\ttest_when_finished \"git reset --hard && git clean -f\" &&\n \tgit checkout -b test$test_count branch1 &&\n \tgit submodule update -N &&\n \ttest_must_fail git merge master &&\n@@ -217,7 +217,7 @@ test_expect_success 'mergetool skips autoresolved' '\n '\n \n test_expect_success 'mergetool merges all from subdir (rerere disabled)' '\n-\ttest_when_finished \"git reset --hard\" &&\n+\ttest_when_finished \"git reset --hard && git clean -f\" &&\n \tgit checkout -b test$test_count branch1 &&\n \ttest_config rerere.enabled false &&\n \t(\n@@ -226,7 +226,7 @@ test_expect_success 'mergetool merges all from subdir (rerere disabled)' '\n \t\t( yes \"r\" | git mergetool ../submod ) &&\n \t\t( yes \"d\" \"d\" | git mergetool --no-prompt ) &&\n \t\ttest \"$(cat ../file1)\" = \"master updated\" &&\n-\t\ttest \"$(cat ../file2)\" = \"master new\" &&\n+\t\ttest \"$(git hash-object ../file2)\" = \"$(git rev-parse master:file2)\" &&\n \t\ttest \"$(cat file3)\" = \"master new sub\" &&\n \t\t( cd .. && git submodule update -N ) &&\n \t\ttest \"$(cat ../submod/bar)\" = \"master submodule\" &&\n@@ -245,7 +245,7 @@ test_expect_success 'mergetool merges all from subdir (rerere enabled)' '\n \t\t( yes \"r\" | git mergetool ../submod ) &&\n \t\t( yes \"d\" \"d\" | git mergetool --no-prompt ) &&\n \t\ttest \"$(cat ../file1)\" = \"master updated\" &&\n-\t\ttest \"$(cat ../file2)\" = \"master new\" &&\n+\t\ttest \"$(git hash-object ../file2)\" = \"$(git rev-parse master:file2)\" &&\n \t\ttest \"$(cat file3)\" = \"master new sub\" &&\n \t\t( cd .. && git submodule update -N ) &&\n \t\ttest \"$(cat ../submod/bar)\" = \"master submodule\" &&\n@@ -631,7 +631,7 @@ test_expect_success 'custom commands override built-ins' '\n \ttest_config mergetool.defaults.trustExitCode true &&\n \ttest_must_fail git merge master &&\n \tgit mergetool --no-prompt --tool defaults -- both &&\n-\techo master both added >expected &&\n+\tgit cat-file -p master:both >expected &&\n \ttest_cmp expected both\n '\n \n-- \n2.16.0.41.g6a66043158\n\n"},{"id":"341284","messageId":"20180308122523.14434-1-szeder.dev@gmail.com","threadId":"47978","inReplyTo":"20180305171125.22331-2-newren@gmail.com","subject":"Re: [RFC PATCH 1/5] Add testcases for improved file collision conflict handling","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-03-08T12:25:23Z","receivedAt":"2018-03-08T12:25:39Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"> Adds testcases dealing with file collisions for the following types of\n> conflicts:\n>   * add/add\n>   * rename/add\n>   * rename/rename(2to1)\n> These tests include expectations for proposed smarter behavior which has\n> not yet been implemented and thus are currently expected to fail.\n> Subsequent commits will correct that and explain the new behavior.\n> \n> Signed-off-by: Elijah Newren <newren@gmail.com>\n> ---\n>  t/t6042-merge-rename-corner-cases.sh | 220 +++++++++++++++++++++++++++++++++++\n>  1 file changed, 220 insertions(+)\n> \n> diff --git a/t/t6042-merge-rename-corner-cases.sh b/t/t6042-merge-rename-corner-cases.sh\n> index 411550d2b6..a6c151ef95 100755\n> --- a/t/t6042-merge-rename-corner-cases.sh\n> +++ b/t/t6042-merge-rename-corner-cases.sh\n> @@ -575,4 +575,224 @@ test_expect_success 'rename/rename/add-dest merge still knows about conflicting\n>  \ttest ! -f c\n>  '\n>\n> +test_conflicts_with_adds_and_renames() {\n> +\ttest $1 != 0 && side1=rename || side1=add\n> +\ttest $2 != 0 && side2=rename || side2=add\n\nFor additonal context I'm going to quote the callsites of this\nfunction from the end of the test script:\n\n> +test_conflicts_with_adds_and_renames 1 1\n> +test_conflicts_with_adds_and_renames 1 0\n> +test_conflicts_with_adds_and_renames 0 1\n> +test_conflicts_with_adds_and_renames 0 0\n\nInstead of the two conditions at the beginning of the function and the\n1 and 0 sort-of magic numbers at the callsites, you could just pass\nthe words \"rename\" and \"add\" as parameters to the function.  The\ncallsites would be clearer and the function could start with two\nsimple variable assignments side1=$1 ; side2=$2.\n\nPlease feel free to dismiss this as bikeshedding: since the branches\nare called 'L' and 'R', maybe calling the variables $sideL and $sideR\nwould match better the rest of the test?  Dunno.\n\n> +\t# Setup:\n> +\t#          L\n> +\t#         / \\\n> +\t#   master   ?\n> +\t#         \\ /\n> +\t#          R\n> +\t#\n> +\t# Where:\n> +\t#   Both L and R have files named 'three-unrelated' and\n> +\t#   'three-related' which collide (i.e. 4 files colliding at two\n> +\t#   pathnames).  Each of the colliding files could have been\n> +\t#   involved in a rename, in which case there was a file named\n> +\t#   'one-[un]related' or 'two-[un]related' that was modified on the\n> +\t#   opposite side of history and renamed into the collision on this\n> +\t#   side of history.\n> +\t#\n> +\t# Questions for both sets of collisions:\n> +\t#   1) The index should contain both a stage 2 and stage 3 entry\n> +\t#      for the colliding file.  Does it?\n> +\t#   2) When renames are involved, the content merges are clean, so\n> +\t#      the index should reflect the content merges, not merely the\n> +\t#      version of the colliding file from the prior commit.  Does\n> +\t#      it?\n> +\t#\n> +\t# Questions for three-unrelated:\n> +\t#   3) There should be files in the worktree named\n> +\t#      'three-unrelated~HEAD' and 'three-unrelated~R^0' with the\n> +\t#      (content-merged) version of 'three-unrelated' from the\n> +\t#      appropriate side of the merge.  Are they present?\n> +\t#   4) There should be no file named 'three-unrelated' in the\n> +\t#      working tree.  That'd make it too likely that users would\n> +\t#      use it instead of carefully looking at both\n> +\t#      three-unrelated~HEAD and three-unrelated~R^0.  Is it\n> +\t#      correctly missing?\n> +\t#\n> +\t# Questions for three-related:\n> +\t#   3) There should be a file in the worktree named three-related\n> +\t#      containing the two-way merged contents of the content-merged\n> +\t#      versions of three-related from each of the two colliding\n> +\t#      files.  Is it present?\n> +\t#   4) There should not be any three-related~* files in the working\n> +\t#      tree.\n> +\ttest_expect_success \"setup simple $side1/$side2 conflict\" '\n> +\t\ttest_create_repo simple_${side1}_${side2} &&\n> +\t\t(\n> +\t\t\tcd simple_${side1}_${side2} &&\n> +\n> +\t\t\t# Create a simple file with 10 lines\n> +\t\t\tten=\"0 1 2 3 4 5 6 7 8 9\" &&\n> +\t\t\tfor i in $ten\n> +\t\t\tdo\n> +\t\t\t\techo line $i in a sample file\n> +\t\t\tdone >unrelated1_v1 &&\n> +\t\t\t# Create a 2nd version of same file with one more line\n> +\t\t\tcat unrelated1_v1 >unrelated1_v2 &&\n\n'cp unrelated1_v1 unrelated1_v2', perhaps?\n\n> +\t\t\techo another line >>unrelated1_v2 &&\n> +\n> +\t\t\t# Create an unrelated simple file with 10 lines\n> +\t\t\tfor i in $ten\n> +\t\t\tdo\n> +\t\t\t\techo line $i in another sample file\n> +\t\t\tdone >unrelated2_v1 &&\n> +\t\t\t# Create a 2nd version of same file with one more line\n> +\t\t\tcat unrelated2_v1 >unrelated2_v2 &&\n\nLikewise.\n\n> +\t\t\techo another line >>unrelated2_v2 &&\n> +\n> +\t\t\t# Create some related files now\n> +\t\t\tfor i in $ten\n> +\t\t\tdo\n> +\t\t\t\techo Random base content line $i\n> +\t\t\tdone >related1_v1 &&\n> +\t\t\tcp -a related1_v1 related1_v2 &&\n\nWouldn't a \"plain\" 'cp', i.e. without '-a', be sufficient?\n\n> +\t\t\techo modification >>related1_v2 &&\n> +\n> +\t\t\tcp -a related1_v1 related2_v1 &&\n> +\t\t\techo more stuff >>related2_v1 &&\n> +\t\t\tcp -a related2_v1 related2_v2 &&\n> +\t\t\techo yet more stuff >>related2_v2 &&\n> +\n> +\t\t\t# Use a tag to record both these files for simple\n> +\t\t\t# access, and clean out these untracked files\n> +\t\t\tgit tag unrelated1_v1 `git hash-object -w unrelated1_v1` &&\n> +\t\t\tgit tag unrelated1_v2 `git hash-object -w unrelated1_v2` &&\n> +\t\t\tgit tag unrelated2_v1 `git hash-object -w unrelated2_v1` &&\n> +\t\t\tgit tag unrelated2_v2 `git hash-object -w unrelated2_v2` &&\n> +\t\t\tgit tag related1_v1 `git hash-object -w related1_v1` &&\n> +\t\t\tgit tag related1_v2 `git hash-object -w related1_v2` &&\n> +\t\t\tgit tag related2_v1 `git hash-object -w related2_v1` &&\n> +\t\t\tgit tag related2_v2 `git hash-object -w related2_v2` &&\n\nStyle nit: please use $() for command substitutions instead of\nbackticks.\n\n> +\t\t\tgit clean -f &&\n> +\n> +\t\t\t# Setup merge-base, consisting of files named \"one-*\"\n> +\t\t\t# and \"two-*\" if renames were involved.\n> +\t\t\ttouch irrelevant_file &&\n> +\t\t\tgit add irrelevant_file &&\n> +\t\t\tif [ $side1 == \"rename\" ]; then\n\nAnother style nit:\n\n        if test $side1 = \"rename\"\n        then\n                ...\n\n> +\t\t\t\tgit show unrelated1_v1 >one-unrelated &&\n> +\t\t\t\tgit add one-unrelated\n\nBroken && chain.\nPlease check the subsequent if statements as well, there are more\nplaces where the && is missing.\n\nAlso note that you can run 'git add one-unrelated one-related', i.e.\nadd more than one file at once, sparing a couple of lines and git\nexecutions.\n\n> +\t\t\t\tgit show related1_v1 >one-related &&\n> +\t\t\t\tgit add one-related\n> +\t\t\tfi &&\n> +\t\t\tif [ $side2 == \"rename\" ]; then\n> +\t\t\t\tgit show unrelated2_v1 >two-unrelated &&\n> +\t\t\t\tgit add two-unrelated\n> +\t\t\t\tgit show related2_v1 >two-related &&\n> +\t\t\t\tgit add two-related\n> +\t\t\tfi &&\n> +\t\t\ttest_tick && git commit -m initial &&\n> +\n> +\t\t\tgit branch L &&\n> +\t\t\tgit branch R &&\n> +\n> +\t\t\t# Handle the left side\n> +\t\t\tgit checkout L &&\n> +\t\t\tif [ $side1 == \"rename\" ]; then\n> +\t\t\t\tgit mv one-unrelated three-unrelated\n> +\t\t\t\tgit mv one-related   three-related\n> +\t\t\telse\n> +\t\t\t\tgit show unrelated1_v2 >three-unrelated &&\n> +\t\t\t\tgit add three-unrelated\n> +\t\t\t\tgit show related1_v2 >three-related &&\n> +\t\t\t\tgit add three-related\n> +\t\t\tfi &&\n> +\t\t\tif [ $side2 == \"rename\" ]; then\n> +\t\t\t\tgit show unrelated2_v2 >two-unrelated &&\n> +\t\t\t\tgit add two-unrelated\n> +\t\t\t\tgit show related2_v2 >two-related &&\n> +\t\t\t\tgit add two-related\n> +\t\t\tfi &&\n> +\t\t\ttest_tick && git commit -m L &&\n> +\n> +\t\t\t# Handle the right side\n> +\t\t\tgit checkout R &&\n> +\t\t\tif [ $side1 == \"rename\" ]; then\n> +\t\t\t\tgit show unrelated1_v2 >one-unrelated &&\n> +\t\t\t\tgit add one-unrelated\n> +\t\t\t\tgit show related1_v2 >one-related &&\n> +\t\t\t\tgit add one-related\n> +\t\t\tfi &&\n> +\t\t\tif [ $side2 == \"rename\" ]; then\n> +\t\t\t\tgit mv two-unrelated three-unrelated\n> +\t\t\t\tgit mv two-related three-related\n> +\t\t\telse\n> +\t\t\t\tgit show unrelated2_v2 >three-unrelated &&\n> +\t\t\t\tgit add three-unrelated\n> +\t\t\t\tgit show related2_v2 >three-related &&\n> +\t\t\t\tgit add three-related\n> +\t\t\tfi &&\n> +\t\t\ttest_tick && git commit -m R\n> +\t\t)\n> +\t'\n\nThis setup test is enormous, and the conditions for the combination of\nthe two sides and the add/rename conflicts are somewhat distracting.\nI don't know how it could be structured better/shorter/clearer...  I\ncouldn't come up with anything useful during lunch.\n\n> +\n> +\ttest_expect_failure \"check simple $side1/$side2 conflict\" '\n> +\t\t(\n> +\t\t\tcd simple_${side1}_${side2} &&\n> +\n> +\t\t\tgit checkout L^0 &&\n> +\n> +\t\t\t# Merge must fail; there is a conflict\n> +\t\t\ttest_must_fail git merge -s recursive R^0 &&\n> +\n> +\t\t\t# Make sure the index has the right number of entries\n> +\t\t\tgit ls-files -s >out &&\n> +\t\t\ttest_line_count = 5 out &&\n> +\t\t\tgit ls-files -u >out &&\n> +\t\t\ttest_line_count = 4 out &&\n> +\n> +\t\t\t# Nothing should have touched irrelevant_file\n> +\t\t\tgit rev-parse >actual \\\n> +\t\t\t\t:0:irrelevant_file \\\n> +\t\t\t\t:2:three-unrelated :3:three-unrelated \\\n> +\t\t\t\t:2:three-related   :3:three-related   &&\n> +\t\t\tgit rev-parse >expected \\\n> +\t\t\t\tmaster:irrelevant_file \\\n> +\t\t\t\tunrelated1_v2      unrelated2_v2 \\\n> +\t\t\t\trelated1_v2        related2_v2   &&\n\nMissing 'test_cmp'?  The above lines write the files 'actual' and\n'expected', but they are never looked at.\n\n> +\t\t\t# Ensure we have the correct number of untracked files\n> +\t\t\tgit ls-files -o >out &&\n> +\t\t\ttest_line_count = 5 out &&\n> +\n> +\t\t\t# Make sure each file (with merging if rename\n> +\t\t\t# involved) is present in the working tree for the\n> +\t\t\t# user to work with.\n> +\t\t\tgit hash-object >actual \\\n> +\t\t\t\tthree-unrelated~HEAD three-unrelated~R^0 &&\n> +\t\t\tgit rev-parse >expected \\\n> +\t\t\t\tunrelated1_v2        unrelated2_v2 &&\n\nAgain, missing 'test_cmp'?\n\n> +\t\t\t# \"three-unrelated\" should not exist because there is\n> +\t\t\t# no reason to give preference to either\n> +\t\t\t# three-unrelated~HEAD or three-unrelated~R^0\n> +\t\t\ttest_path_is_missing three-unrelated &&\n> +\n> +\t\t\t# Make sure we have the correct merged contents for\n> +\t\t\t# three-related\n> +\t\t\tgit show related1_v1 >expected &&\n> +\t\t\tcat <<EOF >>expected &&\n> +<<<<<<< HEAD\n> +modification\n> +=======\n> +more stuff\n> +yet more stuff\n> +>>>>>>> R^0\n> +EOF\n\nYou could use 'cat <<-EOF ....' and indent the here doc with tabs, so\nit won't look so out-of-place.  Or even '<<-\\EOF' to indicate that\nthere is nothing to be expanded in the here doc. \n\n> +\n> +\t\t\ttest_cmp expected three-related\n> +\t\t)\n> +\t'\n> +}\n> +\n> +test_conflicts_with_adds_and_renames 1 1\n> +test_conflicts_with_adds_and_renames 1 0\n> +test_conflicts_with_adds_and_renames 0 1\n> +test_conflicts_with_adds_and_renames 0 0\n> +\n>  test_done\n> --\n> 2.16.0.41.g6a66043158\n> \n> \n"},{"id":"341303","messageId":"CABPp-BGO1m=OAbUS8RUKdqP+8p-DokzbjGa3TcigmTSSa_r3-Q@mail.gmail.com","threadId":"47978","inReplyTo":"20180308122523.14434-1-szeder.dev@gmail.com","subject":"Re: [RFC PATCH 1/5] Add testcases for improved file collision conflict handling","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-03-08T17:51:56Z","receivedAt":"2018-03-08T17:52:06Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Sweet!  Thanks for taking a look, and for spotting lots of\nimprovements (and some really embarrassing errors).  I'll keep the\nfixes queued up while waiting for other feedback.  A few comments...\n\nOn Thu, Mar 8, 2018 at 4:25 AM, SZEDER Gábor <szeder.dev@gmail.com> wrote:\n\n> This setup test is enormous, and the conditions for the combination of\n> the two sides and the add/rename conflicts are somewhat distracting.\n> I don't know how it could be structured better/shorter/clearer...  I\n> couldn't come up with anything useful during lunch.\n\nYeah.  :-(  It's part attempt to test these conflict types much more\nthoroughly than other tests in the testsuite do, and part attempt to\nkeep the test setup consistent between the types to reflect the fact\nthat I'm consolidating the conflict resolution into a single function\nas well.\n\nTwo possible ideas:\n\n  * Split the tests for \"*_unrelated\" files and \"*_related\" files into\nseparate tests (doubling the number of tests, but making each only\ndeal with half the files.  That would make each setup function about\nhalf the size, though both check functions would be about as big as\nthe original.\n\n  * Instead of programatically generated tests, just manually write\nout the tests for each of the four combinations (rename/rename,\nrename/add, add/rename, add/add).  That means four \"copies\" of fairly\nsimilar functions (and possible greater difficulty keeping things\nconsistent if changes are requested), but does allow removal of the\nthree different if-statements and thus makes each one easier to\nunderstand in isolation.\n\nDoing both would be possible as well.\n\nPersonally, I'm much more in favor of the first idea than the second.\nI'm still kind of borderline about whether to make the change\nmentioned in the first idea, but if others feel that splitting would\nhelp a lot, I'm happy to look into either or both ideas.\n\n>> +                     cat <<EOF >>expected &&\n>> +<<<<<<< HEAD\n>> +modification\n>> +=======\n>> +more stuff\n>> +yet more stuff\n>> +>>>>>>> R^0\n>> +EOF\n>\n> You could use 'cat <<-EOF ....' and indent the here doc with tabs, so\n> it won't look so out-of-place.  Or even '<<-\\EOF' to indicate that\n> there is nothing to be expanded in the here doc.\n\nI just learned two new things about heredocs; I've wanted both of\nthose things in the past, but didn't even think to check if they were\npossible.  Thanks for enlightening me.\n"},{"id":"341485","messageId":"CABPp-BHh9byDtdr=HcUTY17dqnjd4V=My_xTH0jzLUv4bfWf5w@mail.gmail.com","threadId":"47978","inReplyTo":"20180305171125.22331-1-newren@gmail.com","subject":"Re: [RFC PATCH 0/5] Improve path collision conflict resolutions","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-03-12T18:19:32Z","receivedAt":"2018-03-12T18:19:39Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Mon, Mar 5, 2018 at 9:11 AM, Elijah Newren <newren@gmail.com> wrote:\n\n>   2) Doing content merges for a rename before looking at the path collision\n>      between a rename and some other file.  In particular (in what I most\n>      suspect others might have an objection to from this series), record\n>      that content-merged file -- which may have conflict markers -- in the\n>      index at the appropriate higher stage.\n>\n> 2)\n>\n> Previously, rename/rename(2to1) conflict resolution for the colliding path\n> would just accept the index changes made by unpack_trees(), meaning that\n> each of the higher order stages in the index for the path collision would\n> implicitly ignore any changes to each renamed file from the other side of\n> history.  Since, as noted above, all traces of the rename-source path were\n> removed from both the index and the working tree, this meant that the index\n> was missing information about changes to such files.  If the user tried to\n> resolve the conflict using the index rather than the working copy, they\n> would end up with a silent loss of changes.\n>\n> I \"fixed\" this by doing the three-way content merge for each renamed-file,\n> and then recorded THAT in the index at either stage 2 or 3 as appropriate.\n> Since that merge might have conflict markers, that could mean recording in\n> the index a file with conflict markers as though it were a given side.\n> (See patch 2 for a more detailed explanation.)  I figure this might be the\n> most controversial change I made.  I can think of a few alternatives, but I\n> liked all of them less.  Opinions?\n\nActually, I realized this last weekend that this wasn't unprecedented\nafter all, and thus likely not at all controversial.  In particular,\nthere is already a case where git stores the sha1 of a file with\nconflict markers from a \"provisional content merge\" at a non-zero\nstage in the index: If a recursive merge is needed and there is a file\nwith content conflicts when creating the virtual merge base, then the\nfile with all the conflict markers will be accepted as part of the\nvirtual merge base and, depending on how the outer merge goes, the\nsha1 of this file with conflict markers can appear in the index at\nstage 1.\n\nSo, that really only leaves the changes to the working tree files.\nAnd based on a few factors including things mentioned in the cover\nletter, I suspect most would only have an opinion about how this patch\nseries affects add/add conflicts.  I'll send a separate email to ask\nabout that, to make it clear I want folks opinions on that issue even\nif they don't have enough time to review the patch series or even read\nmy really long cover letter.\n"}]}