diff: fix the diffing of two identical blobs

carlosmn · May 7, 2012 · 9a29f8d · 9a29f8d
1 parent 28ef7f9
commit 9a29f8d
Show file tree

Hide file tree

Showing 5 changed files with 50 additions and 9 deletions.
diff --git a/src/diff_output.c b/src/diff_output.c
@@ -747,6 +747,10 @@ int git_diff_blobs(
 	delta.status = new ?
 		(old ? GIT_DELTA_MODIFIED : GIT_DELTA_ADDED) :
 		(old ? GIT_DELTA_DELETED : GIT_DELTA_UNTRACKED);
+
+	if (git_oid_cmp(&delta.new_file.oid, &delta.old_file.oid) == 0)
+		delta.status = GIT_DELTA_UNMODIFIED;
+
 	delta.old_file.size = old_data.size;
 	delta.new_file.size = new_data.size;
 
@@ -762,6 +766,10 @@ int git_diff_blobs(
 			return error;
 	}
 
+	/* don't do hunk and line diffs if the two blobs are identical */
+	if (delta.status == GIT_DELTA_UNMODIFIED)
+		return 0;
+
 	/* don't do hunk and line diffs if file is binary */
 	if (delta.binary == 1)
 		return 0;

diff --git a/tests-clar/diff/blob.c b/tests-clar/diff/blob.c
@@ -173,6 +173,37 @@ void test_diff_blob__can_compare_against_null_blobs(void)
 	cl_assert(exp.lines == 0);
 }
 
+void assert_identical_blobs_comparison(diff_expects exp)
+{
+	cl_assert(exp.files == 1);
+	cl_assert(exp.file_unmodified == 1);
+	cl_assert(exp.hunks == 0);
+	cl_assert(exp.lines == 0);
+}
+
+void test_diff_blob__can_compare_identical_blobs(void)
+{
+	cl_git_pass(git_diff_blobs(
+		d, d, &opts, &exp, diff_file_fn, diff_hunk_fn, diff_line_fn));
+
+	cl_assert(exp.at_least_one_of_them_is_binary == false);
+	assert_identical_blobs_comparison(exp);
+
+	memset(&exp, 0, sizeof(exp));
+	cl_git_pass(git_diff_blobs(
+		NULL, NULL, &opts, &exp, diff_file_fn, diff_hunk_fn, diff_line_fn));
+
+	cl_assert(exp.at_least_one_of_them_is_binary == false);
+	assert_identical_blobs_comparison(exp);
+
+	memset(&exp, 0, sizeof(exp));
+	cl_git_pass(git_diff_blobs(
+		alien, alien, &opts, &exp, diff_file_fn, diff_hunk_fn, diff_line_fn));
+
+	cl_assert(exp.at_least_one_of_them_is_binary == true);
+	assert_identical_blobs_comparison(exp);
+}
+
 void assert_binary_blobs_comparison(diff_expects exp)
 {
 	cl_assert(exp.at_least_one_of_them_is_binary == true);

diff --git a/tests-clar/diff/diff_helpers.c b/tests-clar/diff/diff_helpers.c
@@ -39,6 +39,7 @@ int diff_file_fn(
 	case GIT_DELTA_MODIFIED: e->file_mods++; break;
 	case GIT_DELTA_IGNORED: e->file_ignored++; break;
 	case GIT_DELTA_UNTRACKED: e->file_untracked++; break;
+	case GIT_DELTA_UNMODIFIED: e->file_unmodified++; break;
 	default: break;
 	}
 	return 0;

diff --git a/tests-clar/diff/diff_helpers.h b/tests-clar/diff/diff_helpers.h
@@ -11,6 +11,7 @@ typedef struct {
 	int file_mods;
 	int file_ignored;
 	int file_untracked;
+	int file_unmodified;
 
 	int hunks;
 	int hunk_new_lines;

diff --git a/tests-clar/diff/tree.c b/tests-clar/diff/tree.c
@@ -113,16 +113,16 @@ void test_diff_tree__options(void)
 	 */
 	diff_expects test_expects[] = {
 		/* a vs b tests */
-		{ 5, 3, 0, 2, 0, 0, 4, 0, 0, 51, 2, 46, 3 },
-		{ 5, 3, 0, 2, 0, 0, 4, 0, 0, 53, 4, 46, 3 },
-		{ 5, 0, 3, 2, 0, 0, 4, 0, 0, 52, 3, 3, 46 },
-		{ 5, 3, 0, 2, 0, 0, 5, 0, 0, 54, 3, 48, 3 },
+		{ 5, 3, 0, 2, 0, 0, 0, 4, 0, 0, 51, 2, 46, 3 },
+		{ 5, 3, 0, 2, 0, 0, 0, 4, 0, 0, 53, 4, 46, 3 },
+		{ 5, 0, 3, 2, 0, 0, 0, 4, 0, 0, 52, 3, 3, 46 },
+		{ 5, 3, 0, 2, 0, 0, 0, 5, 0, 0, 54, 3, 48, 3 },
 		/* c vs d tests */
-		{ 1, 0, 0, 1, 0, 0, 1, 0, 0, 22, 9, 10, 3 },
-		{ 1, 0, 0, 1, 0, 0, 1, 0, 0, 19, 12, 7, 0 },
-		{ 1, 0, 0, 1, 0, 0, 1, 0, 0, 20, 11, 8, 1 },
-		{ 1, 0, 0, 1, 0, 0, 1, 0, 0, 20, 11, 8, 1 },
-		{ 1, 0, 0, 1, 0, 0, 1, 0, 0, 18, 11, 0, 7 },
+		{ 1, 0, 0, 1, 0, 0, 0, 1, 0, 0, 22, 9, 10, 3 },
+		{ 1, 0, 0, 1, 0, 0, 0, 1, 0, 0, 19, 12, 7, 0 },
+		{ 1, 0, 0, 1, 0, 0, 0, 1, 0, 0, 20, 11, 8, 1 },
+		{ 1, 0, 0, 1, 0, 0, 0, 1, 0, 0, 20, 11, 8, 1 },
+		{ 1, 0, 0, 1, 0, 0, 0, 1, 0, 0, 18, 11, 0, 7 },
 		{ 0 },
 	};
 	diff_expects *expected;