Merge pull request #1643 from ethomson/rename_source

Keep data about source of similarity

src/diff_tform.c

@@ -688,7 +688,7 @@ int git_diff_find_similar(
 	git_diff_find_options opts;
 	size_t num_rewrites = 0, num_updates = 0;
 	void **cache; /* cache of similarity metric file signatures */
-	diff_find_match *matches; /* cache of best matches */
+	diff_find_match *match_sources, *match_targets; /* cache of best matches */
 
 	if ((error = normalize_find_opts(diff, &opts, given_opts)) < 0)
 		return error;
@@ -701,16 +701,18 @@ int git_diff_find_similar(
 	cache = git__calloc(cache_size, sizeof(void *));
 	GITERR_CHECK_ALLOC(cache);
 
-	matches = git__calloc(diff->deltas.length, sizeof(diff_find_match));
-	GITERR_CHECK_ALLOC(matches);
+	match_sources = git__calloc(diff->deltas.length, sizeof(diff_find_match));
+	match_targets = git__calloc(diff->deltas.length, sizeof(diff_find_match));
+	GITERR_CHECK_ALLOC(match_sources);
+	GITERR_CHECK_ALLOC(match_targets);
 
 	/* next find the most similar delta for each rename / copy candidate */
 
 	git_vector_foreach(&diff->deltas, i, to) {
 		size_t tried_sources = 0;
 
-		matches[i].idx = i;
-		matches[i].similarity = 0;
+		match_targets[i].idx = i;
+		match_targets[i].similarity = 0;
 
 		/* skip things that are not rename targets */
 		if (!is_rename_target(diff, &opts, i, cache))
@@ -738,9 +740,12 @@ int git_diff_find_similar(
 				continue;
 			}
 
-			if (matches[i].similarity < (uint32_t)similarity) {
-				matches[i].similarity = (uint32_t)similarity;
-				matches[i].idx = j;
+			if (match_targets[i].similarity < (uint32_t)similarity &&
+				match_sources[j].similarity < (uint32_t)similarity) {
+				match_targets[i].similarity = (uint32_t)similarity;
+				match_sources[j].similarity = (uint32_t)similarity;
+				match_targets[i].idx = j;
+				match_sources[j].idx = i;
 			}
 		}
 	}
@@ -748,13 +753,13 @@ int git_diff_find_similar(
 	/* next rewrite the diffs with renames / copies */
 
 	git_vector_foreach(&diff->deltas, i, to) {
-
-		/* check if this delta was matched to another one */
-		if ((similarity = (int)matches[i].similarity) <= 0)
+		/* check if this delta was the target of a similarity */
+		if ((similarity = (int)match_targets[i].similarity) <= 0)
 			continue;
+
 		assert(to && (to->flags & GIT_DIFF_FLAG__IS_RENAME_TARGET) != 0);
 
-		from = GIT_VECTOR_GET(&diff->deltas, matches[i].idx);
+		from = GIT_VECTOR_GET(&diff->deltas, match_targets[i].idx);
 		assert(from && (from->flags & GIT_DIFF_FLAG__IS_RENAME_SOURCE) != 0);
 
 		/* possible scenarios:
@@ -851,14 +856,14 @@ int git_diff_find_similar(
 				/* in the off chance that we've just swapped the new
 				 * element into the correct place, clear the SPLIT flag
 				 */
-				if (matches[matches[i].idx].idx == i &&
-					matches[matches[i].idx].similarity >
+				if (match_targets[match_targets[i].idx].idx == i &&
+					match_targets[match_targets[i].idx].similarity >
 					opts.rename_from_rewrite_threshold) {
 
 					from->status = GIT_DELTA_RENAMED;
 					from->similarity =
-						(uint32_t)matches[matches[i].idx].similarity;
-					matches[matches[i].idx].similarity = 0;
+						(uint32_t)match_targets[match_targets[i].idx].similarity;
+					match_targets[match_targets[i].idx].similarity = 0;
 					from->flags &= ~GIT_DIFF_FLAG__TO_SPLIT;
 					num_rewrites--;
 				}
@@ -886,7 +891,8 @@ int git_diff_find_similar(
 			FLAG_SET(&opts, GIT_DIFF_BREAK_REWRITES));
 
 cleanup:
-	git__free(matches);
+	git__free(match_sources);
+	git__free(match_targets);
 
 	for (i = 0; i < cache_size; ++i) {
 		if (cache[i] != NULL)

tests-clar/diff/rename.c

@@ -811,3 +811,96 @@ void test_diff_rename__from_deleted_to_split(void)
 
 	git_buf_free(&c1);
 }
+
+struct rename_expected
+{
+	size_t len;
+	const char **sources;
+	const char **targets;
+
+	size_t idx;
+};
+
+int test_names_expected(const git_diff_delta *delta, float progress, void *p)
+{
+	struct rename_expected *expected = p;
+
+	cl_assert(expected->idx < expected->len);
+
+	cl_assert_equal_i(delta->status, GIT_DELTA_RENAMED);
+
+	cl_assert(git__strcmp(expected->sources[expected->idx],
+		delta->old_file.path) == 0);
+	cl_assert(git__strcmp(expected->targets[expected->idx],
+		delta->new_file.path) == 0);
+
+	expected->idx++;
+
+	return 0;
+}
+
+void test_diff_rename__rejected_match_can_match_others(void)
+{
+	git_reference *head, *selfsimilar;
+	git_index *index;
+	git_tree *tree;
+	git_checkout_opts opts = GIT_CHECKOUT_OPTS_INIT;
+	git_diff_list *diff;
+	git_diff_options diffopts = GIT_DIFF_OPTIONS_INIT;
+	git_diff_find_options findopts = GIT_DIFF_FIND_OPTIONS_INIT;
+	git_buf one = GIT_BUF_INIT, two = GIT_BUF_INIT;
+	const char *sources[] = { "Class1.cs", "Class2.cs" };
+	const char *targets[] = { "ClassA.cs", "ClassB.cs" };
+	struct rename_expected expect = { 2, sources, targets };
+	char *ptr;
+
+	opts.checkout_strategy = GIT_CHECKOUT_FORCE;
+
+	cl_git_pass(git_reference_lookup(&head, g_repo, "HEAD"));
+	cl_git_pass(git_reference_symbolic_set_target(
+		&selfsimilar, head, "refs/heads/renames_similar"));
+	cl_git_pass(git_checkout_head(g_repo, &opts));
+	cl_git_pass(git_repository_index(&index, g_repo));
+
+	cl_git_pass(git_futils_readbuffer(&one, "renames/Class1.cs"));
+	cl_git_pass(git_futils_readbuffer(&two, "renames/Class2.cs"));
+
+	cl_git_pass(p_unlink("renames/Class1.cs"));
+	cl_git_pass(p_unlink("renames/Class2.cs"));
+
+	cl_git_pass(git_index_remove_bypath(index, "Class1.cs"));
+	cl_git_pass(git_index_remove_bypath(index, "Class2.cs"));
+
+	cl_assert(ptr = strstr(one.ptr, "Class1"));
+	ptr[5] = 'A';
+
+	cl_assert(ptr = strstr(two.ptr, "Class2"));
+	ptr[5] = 'B';
+
+	cl_git_pass(
+		git_futils_writebuffer(&one, "renames/ClassA.cs", O_RDWR|O_CREAT, 0777));
+	cl_git_pass(
+		git_futils_writebuffer(&two, "renames/ClassB.cs", O_RDWR|O_CREAT, 0777));
+
+	cl_git_pass(git_index_add_bypath(index, "ClassA.cs"));
+	cl_git_pass(git_index_add_bypath(index, "ClassB.cs"));
+
+	cl_git_pass(git_index_write(index));
+
+	cl_git_pass(
+		git_revparse_single((git_object **)&tree, g_repo, "HEAD^{tree}"));
+
+	cl_git_pass(
+		git_diff_tree_to_index(&diff, g_repo, tree, index, &diffopts));
+	cl_git_pass(git_diff_find_similar(diff, &findopts));
+
+	cl_git_pass(
+		git_diff_foreach(diff, test_names_expected, NULL, NULL, &expect));
+
+	git_tree_free(tree);
+	git_index_free(index);
+	git_reference_free(head);
+	git_reference_free(selfsimilar);
+	git_buf_free(&one);
+	git_buf_free(&two);
+}

tests-clar/resources/renames/.gitted/refs/heads/renames_similar

@@ -0,0 +1 @@
+444a76ed3e45b183753f49376af30da8c3fe276a