Merge pull request #6642 from yori/yori/fix-remove-unmodified

git_diff_find_similar doesn't always remove unmodified deltas

src/libgit2/diff_tform.c

@@ -653,6 +653,23 @@ static int calc_self_similarity(
 	return 0;
 }
 
+static void handle_non_blob(
+	git_diff *diff,
+	const git_diff_find_options *opts,
+	size_t delta_idx)
+{
+	git_diff_delta *delta = GIT_VECTOR_GET(&diff->deltas, delta_idx);
+
+	/* skip things that are blobs */
+	if (GIT_MODE_ISBLOB(delta->old_file.mode))
+		return;
+
+	/* honor "remove unmodified" flag for non-blobs (eg submodules) */
+	if (delta->status == GIT_DELTA_UNMODIFIED &&
+	    FLAG_SET(opts, GIT_DIFF_FIND_REMOVE_UNMODIFIED))
+		delta->flags |= GIT_DIFF_FLAG__TO_DELETE;
+}
+
 static bool is_rename_target(
 	git_diff *diff,
 	const git_diff_find_options *opts,
@@ -810,7 +827,8 @@ int git_diff_find_similar(
 	git_diff_find_options opts = GIT_DIFF_FIND_OPTIONS_INIT;
 	size_t num_deltas, num_srcs = 0, num_tgts = 0;
 	size_t tried_srcs = 0, tried_tgts = 0;
-	size_t num_rewrites = 0, num_updates = 0, num_bumped = 0;
+	size_t num_rewrites = 0, num_updates = 0, num_bumped = 0,
+	       num_to_delete = 0;
 	size_t sigcache_size;
 	void **sigcache = NULL; /* cache of similarity metric file signatures */
 	diff_find_match *tgt2src = NULL;
@@ -844,6 +862,8 @@ int git_diff_find_similar(
 	 * mark them for splitting if break-rewrites is enabled
 	 */
 	git_vector_foreach(&diff->deltas, t, tgt) {
+		handle_non_blob(diff, &opts, t);
+
 		if (is_rename_source(diff, &opts, t, sigcache))
 			++num_srcs;
 
@@ -852,11 +872,14 @@ int git_diff_find_similar(
 
 		if ((tgt->flags & GIT_DIFF_FLAG__TO_SPLIT) != 0)
 			num_rewrites++;
+
+		if ((tgt->flags & GIT_DIFF_FLAG__TO_DELETE) != 0)
+			num_to_delete++;
 	}
 
-	/* if there are no candidate srcs or tgts, we're done */
+	/* If there are no candidate srcs or tgts, no need to find matches */
 	if (!num_srcs || !num_tgts)
-		goto cleanup;
+		goto split_and_delete;
 
 	src2tgt = git__calloc(num_deltas, sizeof(diff_find_match));
 	GIT_ERROR_CHECK_ALLOC(src2tgt);
@@ -1093,15 +1116,20 @@ find_best_matches:
 		}
 	}
 
+split_and_delete:
 	/*
 	 * Actually split and delete entries as needed
 	 */
 
-	if (num_rewrites > 0 || num_updates > 0)
+	if (num_rewrites > 0 || num_updates > 0 || num_to_delete > 0) {
+		size_t apply_len = diff->deltas.length -
+		                   num_rewrites - num_to_delete;
+
 		error = apply_splits_and_deletes(
-			diff, diff->deltas.length - num_rewrites,
+			diff, apply_len,
 			FLAG_SET(&opts, GIT_DIFF_BREAK_REWRITES) &&
 			!FLAG_SET(&opts, GIT_DIFF_BREAK_REWRITES_FOR_RENAMES_ONLY));
+	}
 
 cleanup:
 	git__free(tgt2src);

tests/libgit2/diff/rename.c

@@ -1441,6 +1441,52 @@ void test_diff_rename__can_delete_unmodified_deltas(void)
 	git_str_dispose(&c1);
 }
 
+void test_diff_rename__can_delete_unmodified_deltas_including_submodule(void)
+{
+	git_repository *repo; /* "submodules" */
+	git_index *index;
+	git_tree *tree;
+	git_diff *diff;
+	git_diff_options diffopts = GIT_DIFF_OPTIONS_INIT;
+	git_diff_find_options opts = GIT_DIFF_FIND_OPTIONS_INIT;
+	diff_expects exp;
+
+	cl_git_sandbox_cleanup(); /* Don't use "renames" for this test */
+	repo = cl_git_sandbox_init("submodules");
+
+	cl_repo_set_bool(repo, "core.autocrlf", false);
+
+	cl_git_pass(
+		git_revparse_single((git_object **)&tree, repo, "HEAD^{tree}"));
+
+	cl_git_pass(git_repository_index(&index, repo));
+	cl_git_pass(git_index_read_tree(index, tree));
+
+	diffopts.flags = GIT_DIFF_INCLUDE_UNMODIFIED;
+
+	cl_git_pass(git_diff_tree_to_index(&diff, repo, tree, index, &diffopts));
+
+	memset(&exp, 0, sizeof(exp));
+	cl_git_pass(git_diff_foreach(
+		diff, diff_file_cb, diff_binary_cb, diff_hunk_cb, diff_line_cb, &exp));
+	cl_assert_equal_i(5, exp.files);
+	cl_assert_equal_i(5, exp.file_status[GIT_DELTA_UNMODIFIED]);
+
+	opts.flags = GIT_DIFF_FIND_ALL | GIT_DIFF_FIND_REMOVE_UNMODIFIED;
+	cl_git_pass(git_diff_find_similar(diff, &opts));
+
+	memset(&exp, 0, sizeof(exp));
+	cl_git_pass(git_diff_foreach(
+		diff, diff_file_cb, diff_binary_cb, diff_hunk_cb, diff_line_cb, &exp));
+	cl_assert_equal_i(0, exp.files);
+
+	git_diff_free(diff);
+	git_tree_free(tree);
+	git_index_free(index);
+
+	cl_git_sandbox_cleanup();
+}
+
 void test_diff_rename__matches_config_behavior(void)
 {
 	const char *sha0 = INITIAL_COMMIT;