Merge pull request #5893 from libgit2/ethomson/diff_fail

Ensure that completely ignored diff is empty

src/libgit2/diff_print.c

@@ -29,6 +29,7 @@ typedef struct {
 	const char *new_prefix;
 	uint32_t flags;
 	int id_strlen;
+	unsigned int sent_file_header;
 	git_oid_t oid_type;
 
 	int (*strcomp)(const char *, const char *);
@@ -579,6 +580,30 @@ static int diff_print_patch_file_binary(
 	return error;
 }
 
+GIT_INLINE(int) should_force_header(const git_diff_delta *delta)
+{
+	if (delta->old_file.mode != delta->new_file.mode)
+		return 1;
+
+	if (delta->status == GIT_DELTA_RENAMED || delta->status == GIT_DELTA_COPIED)
+		return 1;
+
+	return 0;
+}
+
+GIT_INLINE(int) flush_file_header(const git_diff_delta *delta, diff_print_info *pi)
+{
+	if (pi->sent_file_header)
+		return 0;
+
+	pi->line.origin      = GIT_DIFF_LINE_FILE_HDR;
+	pi->line.content     = git_str_cstr(pi->buf);
+	pi->line.content_len = git_str_len(pi->buf);
+	pi->sent_file_header = 1;
+
+	return pi->print_cb(delta, NULL, &pi->line, pi->payload);
+}
+
 static int diff_print_patch_file(
 	const git_diff_delta *delta, float progress, void *data)
 {
@@ -609,15 +634,22 @@ static int diff_print_patch_file(
 		 (pi->flags & GIT_DIFF_SHOW_UNTRACKED_CONTENT) == 0))
 		return 0;
 
+	pi->sent_file_header = 0;
+
 	if ((error = git_diff_delta__format_file_header(pi->buf, delta, oldpfx, newpfx,
 							id_strlen, print_index)) < 0)
 		return error;
 
-	pi->line.origin      = GIT_DIFF_LINE_FILE_HDR;
-	pi->line.content     = git_str_cstr(pi->buf);
-	pi->line.content_len = git_str_len(pi->buf);
+	/*
+	 * pi->buf now contains the file header data. Go ahead and send it
+	 * if there's useful data in there, like similarity. Otherwise, we
+	 * should queue it to send when we see the first hunk. This prevents
+	 * us from sending a header when all hunks were ignored.
+	 */
+	if (should_force_header(delta) && (error = flush_file_header(delta, pi)) < 0)
+		return error;
 
-	return pi->print_cb(delta, NULL, &pi->line, pi->payload);
+	return 0;
 }
 
 static int diff_print_patch_binary(
@@ -632,6 +664,9 @@ static int diff_print_patch_binary(
 		pi->new_prefix ? pi->new_prefix : DIFF_NEW_PREFIX_DEFAULT;
 	int error;
 
+	if ((error = flush_file_header(delta, pi)) < 0)
+		return error;
+
 	git_str_clear(pi->buf);
 
 	if ((error = diff_print_patch_file_binary(
@@ -651,10 +686,14 @@ static int diff_print_patch_hunk(
 	void *data)
 {
 	diff_print_info *pi = data;
+	int error;
 
 	if (S_ISDIR(d->new_file.mode))
 		return 0;
 
+	if ((error = flush_file_header(d, pi)) < 0)
+		return error;
+
 	pi->line.origin      = GIT_DIFF_LINE_HUNK_HDR;
 	pi->line.content     = h->header;
 	pi->line.content_len = h->header_len;
@@ -669,10 +708,14 @@ static int diff_print_patch_line(
 	void *data)
 {
 	diff_print_info *pi = data;
+	int error;
 
 	if (S_ISDIR(delta->new_file.mode))
 		return 0;
 
+	if ((error = flush_file_header(delta, pi)) < 0)
+		return error;
+
 	return pi->print_cb(delta, hunk, line, pi->payload);
 }
 

tests/libgit2/diff/workdir.c

@@ -2286,42 +2286,81 @@ void test_diff_workdir__to_index_reversed_content_loads(void)
 	diff_expects exp;
 	int use_iterator;
 	char *pathspec = "new_file";
-	
+
 	g_repo = cl_git_sandbox_init("status");
-	
+
 	opts.context_lines = 3;
 	opts.interhunk_lines = 1;
 	opts.flags |= GIT_DIFF_INCLUDE_IGNORED | GIT_DIFF_INCLUDE_UNTRACKED |
 		GIT_DIFF_SHOW_UNTRACKED_CONTENT | GIT_DIFF_REVERSE;
 	opts.pathspec.strings = &pathspec;
 	opts.pathspec.count   = 1;
-	
+
 	cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts));
-	
+
 	for (use_iterator = 0; use_iterator <= 1; use_iterator++) {
 		memset(&exp, 0, sizeof(exp));
-		
+
 		if (use_iterator)
 			cl_git_pass(diff_foreach_via_iterator(
 				diff, diff_file_cb, diff_binary_cb, diff_hunk_cb, diff_line_cb, &exp));
 		else
 			cl_git_pass(git_diff_foreach(
 				diff, diff_file_cb, diff_binary_cb, diff_hunk_cb, diff_line_cb, &exp));
-		
+
 		cl_assert_equal_i(1, exp.files);
 		cl_assert_equal_i(0, exp.file_status[GIT_DELTA_ADDED]);
 		cl_assert_equal_i(0, exp.file_status[GIT_DELTA_DELETED]);
 		cl_assert_equal_i(0, exp.file_status[GIT_DELTA_MODIFIED]);
 		cl_assert_equal_i(0, exp.file_status[GIT_DELTA_IGNORED]);
 		cl_assert_equal_i(1, exp.file_status[GIT_DELTA_UNTRACKED]);
-		
+
 		cl_assert_equal_i(1, exp.hunks);
-		
+
 		cl_assert_equal_i(1, exp.lines);
 		cl_assert_equal_i(0, exp.line_ctxt);
 		cl_assert_equal_i(0, exp.line_adds);
 		cl_assert_equal_i(1, exp.line_dels);
 	}
-	
+
+	git_diff_free(diff);
+}
+
+void test_diff_workdir__completely_ignored_shows_empty_diff(void)
+{
+	git_diff_options opts = GIT_DIFF_OPTIONS_INIT;
+	git_diff *diff;
+	git_patch *patch;
+	git_buf buf = GIT_BUF_INIT;
+	char *pathspec = "subdir.txt";
+
+	opts.pathspec.strings = &pathspec;
+	opts.pathspec.count = 1;
+
+	g_repo = cl_git_sandbox_init("status");
+	cl_git_rewritefile("status/subdir.txt", "Is it a bird?\n\nIs it a plane?\n");
+
+	/* Perform the diff normally */
+	cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts));
+	cl_git_pass(git_patch_from_diff(&patch, diff, 0));
+	cl_git_pass(git_patch_to_buf(&buf, patch));
+
+	cl_assert_equal_s("diff --git a/subdir.txt b/subdir.txt\nindex e8ee89e..53c8db5 100644\n--- a/subdir.txt\n+++ b/subdir.txt\n@@ -1,2 +1,3 @@\n Is it a bird?\n+\n Is it a plane?\n", buf.ptr);
+
+	git_buf_dispose(&buf);
+	git_patch_free(patch);
+	git_diff_free(diff);
+
+	/* Perform the diff ignoring blank lines */
+	opts.flags |= GIT_DIFF_IGNORE_BLANK_LINES;
+
+	cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts));
+	cl_git_pass(git_patch_from_diff(&patch, diff, 0));
+	cl_git_pass(git_patch_to_buf(&buf, patch));
+
+	cl_assert_equal_s("", buf.ptr);
+
+	git_buf_dispose(&buf);
+	git_patch_free(patch);
 	git_diff_free(diff);
 }