Merge pull request #5839 from staktrace/find_similar

src/diff_tform.c

@@ -1039,7 +1039,7 @@ find_best_matches:
 				memcpy(&src->old_file, &swap, sizeof(src->old_file));
 
 				/* if we've just swapped the new element into the correct
-				 * place, clear the SPLIT flag
+				 * place, clear the SPLIT and RENAME_TARGET flags
 				 */
 				if (tgt2src[s].idx == t &&
 					tgt2src[s].similarity >
@@ -1047,7 +1047,7 @@ find_best_matches:
 					src->status     = GIT_DELTA_RENAMED;
 					src->similarity = tgt2src[s].similarity;
 					tgt2src[s].similarity = 0;
-					src->flags &= ~GIT_DIFF_FLAG__TO_SPLIT;
+					src->flags &= ~(GIT_DIFF_FLAG__TO_SPLIT | GIT_DIFF_FLAG__IS_RENAME_TARGET);
 					num_rewrites--;
 				}
 				/* otherwise, if we just overwrote a source, update mapping */

tests/diff/rename.c

@@ -21,6 +21,8 @@ void test_diff_rename__cleanup(void)
 #define RENAME_MODIFICATION_COMMIT "19dd32dfb1520a64e5bbaae8dce6ef423dfa2f13"
 #define REWRITE_DELETE_COMMIT "84d8efa38af7ace2b302de0adbda16b1f1cd2e1b"
 #define DELETE_RENAME_COMMIT "be053a189b0bbde545e0a3f59ce00b46ad29ce0d"
+#define BREAK_REWRITE_BASE_COMMIT "db98035f715427eef1f5e17f03e1801c05301e9e"
+#define BREAK_REWRITE_COMMIT "7e7bfb88ba9bc65fd700fee1819cf1c317aafa56"
 
 /*
  * Renames repo has:
@@ -1977,3 +1979,56 @@ void test_diff_rename__delete_and_rename(void)
 	git_tree_free(old_tree);
 	git_tree_free(new_tree);
 }
+
+/*
+ * The break_rewrite branch contains a testcase reduced from
+ * a real-world scenario, rather than being "constructed" like
+ * the above tests seem to be. There are two commits layered
+ * on top of the repo's initial commit; the base commit which
+ * clears out the files from the initial commit and installs
+ * four files. And then there's the modification commit which
+ * mutates the files in such a way as to trigger the bug in
+ * libgit2.
+ * commit db98035f715427eef1f5e17f03e1801c05301e9e
+ *   serving.txt     (deleted)
+ *   sevencities.txt (deleted)
+ *   AAA             (313 lines)
+ *   BBB             (314 lines)
+ *   CCC             (704 lines)
+ *   DDD             (314 lines, identical to BBB)
+ * commit 7e7bfb88ba9bc65fd700fee1819cf1c317aafa56
+ *   This deletes CCC and makes slight modifications
+ *   to AAA, BBB, and DDD. The find_best_matches loop
+ *   for git_diff_find_similar computes the following:
+ *   CCC    moved to    AAA     (similarity 91)
+ *   CCC    copied to   AAA     (similarity 91)
+ *   DDD    moved to    BBB     (similarity 52)
+ *   CCC    copied to   BBB     (similarity 90)
+ *   BBB    moved to    DDD     (similarity 52)
+ *   CCC    copied to   DDD     (similarity 90)
+ * The code to rewrite the diffs by resolving these
+ * copies/renames would resolve the BBB <-> DDD moves
+ * but then still leave BBB as a rename target for
+ * the deleted file CCC. Since the split flag on BBB
+ * was cleared, this would trigger an error.
+ */
+void test_diff_rename__break_rewrite(void)
+{
+	const char *old_sha = BREAK_REWRITE_BASE_COMMIT;
+	const char *new_sha = BREAK_REWRITE_COMMIT;
+	git_tree *old_tree, *new_tree;
+	git_diff *diff;
+	git_diff_find_options find_opts = GIT_DIFF_FIND_OPTIONS_INIT;
+
+	old_tree = resolve_commit_oid_to_tree(g_repo, old_sha);
+	new_tree = resolve_commit_oid_to_tree(g_repo, new_sha);
+
+	find_opts.flags = GIT_DIFF_FIND_RENAMES | GIT_DIFF_FIND_COPIES | GIT_DIFF_BREAK_REWRITES | GIT_DIFF_BREAK_REWRITES_FOR_RENAMES_ONLY;
+
+	cl_git_pass(git_diff_tree_to_tree(&diff, g_repo, old_tree, new_tree, NULL));
+	cl_git_pass(git_diff_find_similar(diff, &find_opts));
+
+	git_diff_free(diff);
+	git_tree_free(old_tree);
+	git_tree_free(new_tree);
+}

tests/resources/renames/.gitted/ORIG_HEAD

+19dd32dfb1520a64e5bbaae8dce6ef423dfa2f13

tests/resources/renames/.gitted/logs/HEAD

@@ -2,3 +2,8 @@
 31e47d8c1fa36d7f8d537b96158e3f024de0a9f2 2bc7f351d20b53f1c72c16c4b036e491c478c49a Russell Belfer <rb@github.com> 1351024817 -0700	commit: copy and rename with no change
 2bc7f351d20b53f1c72c16c4b036e491c478c49a 1c068dee5790ef1580cfc4cd670915b48d790084 Russell Belfer <rb@github.com> 1361485758 -0800	commit: rewrites, copies with changes, etc.
 1c068dee5790ef1580cfc4cd670915b48d790084 19dd32dfb1520a64e5bbaae8dce6ef423dfa2f13 Russell Belfer <rb@github.com> 1361486360 -0800	commit: more renames and smallish modifications
+19dd32dfb1520a64e5bbaae8dce6ef423dfa2f13 31e47d8c1fa36d7f8d537b96158e3f024de0a9f2 Kartikaya Gupta <kats@pancake.staktrace.com> 1618486966 -0400	checkout: moving from master to break_rewrite
+31e47d8c1fa36d7f8d537b96158e3f024de0a9f2 db98035f715427eef1f5e17f03e1801c05301e9e Kartikaya Gupta <kats@pancake.staktrace.com> 1618487039 -0400	commit: This test needs to start with a minimum of four files
+db98035f715427eef1f5e17f03e1801c05301e9e 7e7bfb88ba9bc65fd700fee1819cf1c317aafa56 Kartikaya Gupta <kats@pancake.staktrace.com> 1618487097 -0400	commit: Copy/modify files around
+7e7bfb88ba9bc65fd700fee1819cf1c317aafa56 19dd32dfb1520a64e5bbaae8dce6ef423dfa2f13 Kartikaya Gupta <kats@pancake.staktrace.com> 1618487105 -0400	checkout: moving from break_rewrite to master
+19dd32dfb1520a64e5bbaae8dce6ef423dfa2f13 19dd32dfb1520a64e5bbaae8dce6ef423dfa2f13 Kartikaya Gupta <kats@pancake.staktrace.com> 1618487271 -0400	reset: moving to HEAD

tests/resources/renames/.gitted/refs/heads/break_rewrite

+7e7bfb88ba9bc65fd700fee1819cf1c317aafa56