Merge pull request #1929 from libgit2/rb/misc-diff-fixes

Fix some observed problems with incorrect diffs

10 files changed, 210 insertions, 44 deletions

diff --git a/tests-clar/checkout/head.c b/tests-clar/checkout/head.c
index 74c6fb87a..a7a7e9071 100644
--- a/tests-clar/checkout/head.c
+++ b/tests-clar/checkout/head.c
@@ -54,7 +54,6 @@ void test_checkout_head__with_index_only_tree(void)
 	cl_git_pass(git_checkout_head(g_repo, &opts));
 
 	cl_git_pass(git_repository_index(&index, g_repo));
-	cl_git_pass(git_index_read(index)); /* reload if needed */
 
 	cl_assert(!git_path_isfile("testrepo/newdir/newfile.txt"));
 	cl_assert(git_index_get_bypath(index, "newdir/newfile.txt", 0) == NULL);
diff --git a/tests-clar/diff/blob.c b/tests-clar/diff/blob.c
index b51bc0f38..93f20711c 100644
--- a/tests-clar/diff/blob.c
+++ b/tests-clar/diff/blob.c
@@ -26,9 +26,8 @@ void test_diff_blob__initialize(void)
 
 	g_repo = cl_git_sandbox_init("attr");
 
-	GIT_INIT_STRUCTURE(&opts, GIT_DIFF_OPTIONS_VERSION);
+	cl_git_pass(git_diff_options_init(&opts, GIT_DIFF_OPTIONS_VERSION));
 	opts.context_lines = 1;
-	opts.interhunk_lines = 0;
 
 	memset(&expected, 0, sizeof(expected));
 
diff --git a/tests-clar/diff/diff_helpers.c b/tests-clar/diff/diff_helpers.c
index 466d0ef54..33bb561f6 100644
--- a/tests-clar/diff/diff_helpers.c
+++ b/tests-clar/diff/diff_helpers.c
@@ -221,11 +221,15 @@ static int diff_print_cb(
 	const git_diff_line *line,
 	void *payload)
 {
-	GIT_UNUSED(payload);
-	GIT_UNUSED(delta);
-	GIT_UNUSED(hunk);
-	fprintf((FILE *)payload, "%c%.*s",
-		line->origin, (int)line->content_len, line->content);
+	FILE *fp = payload;
+
+	GIT_UNUSED(delta); GIT_UNUSED(hunk);
+
+	if (line->origin == GIT_DIFF_LINE_CONTEXT ||
+		line->origin == GIT_DIFF_LINE_ADDITION ||
+		line->origin == GIT_DIFF_LINE_DELETION)
+		fputc(line->origin, fp);
+	fwrite(line->content, 1, line->content_len, fp);
 	return 0;
 }
 
diff --git a/tests-clar/diff/tree.c b/tests-clar/diff/tree.c
index ca2daf5fb..582174b8b 100644
--- a/tests-clar/diff/tree.c
+++ b/tests-clar/diff/tree.c
@@ -9,9 +9,7 @@ static diff_expects expect;
 
 void test_diff_tree__initialize(void)
 {
-	GIT_INIT_STRUCTURE(&opts, GIT_DIFF_OPTIONS_VERSION);
-	/* The default context lines is set by _INIT which we can't use here */
-	opts.context_lines = 3;
+	cl_git_pass(git_diff_options_init(&opts, GIT_DIFF_OPTIONS_VERSION));
 
 	memset(&expect, 0, sizeof(expect));
 
@@ -91,7 +89,8 @@ void test_diff_tree__0(void)
 }
 
 #define DIFF_OPTS(FLAGS, CTXT) \
-	{GIT_DIFF_OPTIONS_VERSION, (FLAGS), 0, {NULL,0}, NULL, NULL, (CTXT), 1}
+	{GIT_DIFF_OPTIONS_VERSION, (FLAGS), GIT_SUBMODULE_IGNORE_DEFAULT, \
+	{NULL,0}, NULL, NULL, (CTXT), 1}
 
 void test_diff_tree__options(void)
 {
diff --git a/tests-clar/diff/workdir.c b/tests-clar/diff/workdir.c
index 60acd365d..df31e7322 100644
--- a/tests-clar/diff/workdir.c
+++ b/tests-clar/diff/workdir.c
@@ -63,6 +63,60 @@ void test_diff_workdir__to_index(void)
 	git_diff_free(diff);
 }
 
+void test_diff_workdir__to_index_with_assume_unchanged(void)
+{
+	git_diff_options opts = GIT_DIFF_OPTIONS_INIT;
+	git_diff *diff = NULL;
+	git_index *idx = NULL;
+	diff_expects exp;
+	const git_index_entry *iep;
+	git_index_entry ie;
+
+	g_repo = cl_git_sandbox_init("status");
+
+	/* do initial diff */
+
+	cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts));
+	memset(&exp, 0, sizeof(exp));
+	cl_git_pass(git_diff_foreach(
+		diff, diff_file_cb, diff_hunk_cb, diff_line_cb, &exp));
+	cl_assert_equal_i(8, exp.files);
+	cl_assert_equal_i(0, exp.file_status[GIT_DELTA_ADDED]);
+	cl_assert_equal_i(4, exp.file_status[GIT_DELTA_DELETED]);
+	cl_assert_equal_i(4, exp.file_status[GIT_DELTA_MODIFIED]);
+	git_diff_free(diff);
+
+	/* mark a couple of entries with ASSUME_UNCHANGED */
+
+	cl_git_pass(git_repository_index(&idx, g_repo));
+
+	cl_assert((iep = git_index_get_bypath(idx, "modified_file", 0)) != NULL);
+	memcpy(&ie, iep, sizeof(ie));
+	ie.flags |= GIT_IDXENTRY_VALID;
+	cl_git_pass(git_index_add(idx, &ie));
+
+	cl_assert((iep = git_index_get_bypath(idx, "file_deleted", 0)) != NULL);
+	memcpy(&ie, iep, sizeof(ie));
+	ie.flags |= GIT_IDXENTRY_VALID;
+	cl_git_pass(git_index_add(idx, &ie));
+
+	cl_git_pass(git_index_write(idx));
+	git_index_free(idx);
+
+	/* redo diff and see that entries are skipped */
+
+	cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts));
+	memset(&exp, 0, sizeof(exp));
+	cl_git_pass(git_diff_foreach(
+		diff, diff_file_cb, diff_hunk_cb, diff_line_cb, &exp));
+	cl_assert_equal_i(6, exp.files);
+	cl_assert_equal_i(0, exp.file_status[GIT_DELTA_ADDED]);
+	cl_assert_equal_i(3, exp.file_status[GIT_DELTA_DELETED]);
+	cl_assert_equal_i(3, exp.file_status[GIT_DELTA_MODIFIED]);
+	git_diff_free(diff);
+
+}
+
 void test_diff_workdir__to_tree(void)
 {
 	/* grabbed a couple of commit oids from the history of the attr repo */
@@ -196,6 +250,38 @@ void test_diff_workdir__to_tree(void)
 
 	git_diff_free(diff);
 
+	/* Let's try that once more with a reversed diff */
+
+	opts.flags |= GIT_DIFF_REVERSE;
+
+	cl_git_pass(git_diff_tree_to_index(&diff, g_repo, b, NULL, &opts));
+	cl_git_pass(git_diff_index_to_workdir(&diff2, g_repo, NULL, &opts));
+	cl_git_pass(git_diff_merge(diff, diff2));
+	git_diff_free(diff2);
+
+	memset(&exp, 0, sizeof(exp));
+
+	cl_git_pass(git_diff_foreach(
+		diff, diff_file_cb, diff_hunk_cb, diff_line_cb, &exp));
+
+	cl_assert_equal_i(16, exp.files);
+	cl_assert_equal_i(5, exp.file_status[GIT_DELTA_DELETED]);
+	cl_assert_equal_i(4, exp.file_status[GIT_DELTA_ADDED]);
+	cl_assert_equal_i(3, exp.file_status[GIT_DELTA_MODIFIED]);
+	cl_assert_equal_i(1, exp.file_status[GIT_DELTA_IGNORED]);
+	cl_assert_equal_i(3, exp.file_status[GIT_DELTA_UNTRACKED]);
+
+	cl_assert_equal_i(12, exp.hunks);
+
+	cl_assert_equal_i(19, exp.lines);
+	cl_assert_equal_i(3, exp.line_ctxt);
+	cl_assert_equal_i(12, exp.line_dels);
+	cl_assert_equal_i(4, exp.line_adds);
+
+	git_diff_free(diff);
+
+	/* all done now */
+
 	git_tree_free(a);
 	git_tree_free(b);
 }
@@ -1330,3 +1416,73 @@ void test_diff_workdir__patience_diff(void)
 	git_patch_free(patch);
 	git_diff_free(diff);
 }
+
+void test_diff_workdir__with_stale_index(void)
+{
+	git_diff_options opts = GIT_DIFF_OPTIONS_INIT;
+	git_diff *diff = NULL;
+	git_index *idx = NULL;
+	diff_expects exp;
+
+	g_repo = cl_git_sandbox_init("status");
+	cl_git_pass(git_repository_index(&idx, g_repo));
+
+	/* make the in-memory index invalid */
+	{
+		git_repository *r2;
+		git_index *idx2;
+		cl_git_pass(git_repository_open(&r2, "status"));
+		cl_git_pass(git_repository_index(&idx2, r2));
+		cl_git_pass(git_index_add_bypath(idx2, "new_file"));
+		cl_git_pass(git_index_add_bypath(idx2, "subdir/new_file"));
+		cl_git_pass(git_index_remove_bypath(idx2, "staged_new_file"));
+		cl_git_pass(git_index_remove_bypath(idx2, "staged_changes_file_deleted"));
+		cl_git_pass(git_index_write(idx2));
+		git_index_free(idx2);
+		git_repository_free(r2);
+	}
+
+	opts.context_lines = 3;
+	opts.interhunk_lines = 1;
+	opts.flags |= GIT_DIFF_INCLUDE_UNTRACKED | GIT_DIFF_INCLUDE_UNMODIFIED;
+
+	/* first try with index pointer which should prevent reload */
+
+	cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, idx, &opts));
+
+	memset(&exp, 0, sizeof(exp));
+
+	cl_git_pass(git_diff_foreach(
+		diff, diff_file_cb, diff_hunk_cb, diff_line_cb, &exp));
+
+	cl_assert_equal_i(17, exp.files);
+	cl_assert_equal_i(0, exp.file_status[GIT_DELTA_ADDED]);
+	cl_assert_equal_i(4, exp.file_status[GIT_DELTA_DELETED]);
+	cl_assert_equal_i(4, exp.file_status[GIT_DELTA_MODIFIED]);
+	cl_assert_equal_i(4, exp.file_status[GIT_DELTA_UNTRACKED]);
+	cl_assert_equal_i(5, exp.file_status[GIT_DELTA_UNMODIFIED]);
+
+	git_diff_free(diff);
+
+	/* now let's try without the index pointer which should trigger reload */
+
+	/* two files that were UNTRACKED should have become UNMODIFIED */
+	/* one file that was UNMODIFIED should now have become UNTRACKED */
+	/* one file that was DELETED should now be gone completely */
+
+	cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts));
+
+	memset(&exp, 0, sizeof(exp));
+
+	cl_git_pass(git_diff_foreach(
+		diff, diff_file_cb, diff_hunk_cb, diff_line_cb, &exp));
+
+	cl_assert_equal_i(16, exp.files);
+	cl_assert_equal_i(0, exp.file_status[GIT_DELTA_ADDED]);
+	cl_assert_equal_i(3, exp.file_status[GIT_DELTA_DELETED]);
+	cl_assert_equal_i(4, exp.file_status[GIT_DELTA_MODIFIED]);
+	cl_assert_equal_i(3, exp.file_status[GIT_DELTA_UNTRACKED]);
+	cl_assert_equal_i(6, exp.file_status[GIT_DELTA_UNMODIFIED]);
+
+	git_index_free(idx);
+}
diff --git a/tests-clar/index/addall.c b/tests-clar/index/addall.c
index 3b5f5f22d..44c51279d 100644
--- a/tests-clar/index/addall.c
+++ b/tests-clar/index/addall.c
@@ -68,10 +68,11 @@ static int index_status_cb(
 	return 0;
 }
 
-static void check_status(
+static void check_status_at_line(
 	git_repository *repo,
 	size_t index_adds, size_t index_dels, size_t index_mods,
-	size_t wt_adds, size_t wt_dels, size_t wt_mods, size_t ignores)
+	size_t wt_adds, size_t wt_dels, size_t wt_mods, size_t ignores,
+	const char *file, int line)
 {
 	index_status_counts vals;
 
@@ -79,15 +80,25 @@ static void check_status(
 
 	cl_git_pass(git_status_foreach(repo, index_status_cb, &vals));
 
-	cl_assert_equal_sz(index_adds, vals.index_adds);
-	cl_assert_equal_sz(index_dels, vals.index_dels);
-	cl_assert_equal_sz(index_mods, vals.index_mods);
-	cl_assert_equal_sz(wt_adds, vals.wt_adds);
-	cl_assert_equal_sz(wt_dels, vals.wt_dels);
-	cl_assert_equal_sz(wt_mods, vals.wt_mods);
-	cl_assert_equal_sz(ignores, vals.ignores);
+	clar__assert_equal(
+		file,line,"wrong index adds", 1, "%"PRIuZ, index_adds, vals.index_adds);
+	clar__assert_equal(
+		file,line,"wrong index dels", 1, "%"PRIuZ, index_dels, vals.index_dels);
+	clar__assert_equal(
+		file,line,"wrong index mods", 1, "%"PRIuZ, index_mods, vals.index_mods);
+	clar__assert_equal(
+		file,line,"wrong workdir adds", 1, "%"PRIuZ, wt_adds, vals.wt_adds);
+	clar__assert_equal(
+		file,line,"wrong workdir dels", 1, "%"PRIuZ, wt_dels, vals.wt_dels);
+	clar__assert_equal(
+		file,line,"wrong workdir mods", 1, "%"PRIuZ, wt_mods, vals.wt_mods);
+	clar__assert_equal(
+		file,line,"wrong ignores", 1, "%"PRIuZ, ignores, vals.ignores);
 }
 
+#define check_status(R,IA,ID,IM,WA,WD,WM,IG) \
+	check_status_at_line(R,IA,ID,IM,WA,WD,WM,IG,__FILE__,__LINE__)
+
 static void check_stat_data(git_index *index, const char *path, bool match)
 {
 	const git_index_entry *entry;
diff --git a/tests-clar/index/names.c b/tests-clar/index/names.c
index 87453ecbf..9007b1b15 100644
--- a/tests-clar/index/names.c
+++ b/tests-clar/index/names.c
@@ -63,7 +63,7 @@ void test_index_names__roundtrip(void)
 	git_index_clear(repo_index);
 	cl_assert(git_index_name_entrycount(repo_index) == 0);
 
-	cl_git_pass(git_index_read(repo_index));
+	cl_git_pass(git_index_read(repo_index, true));
 	cl_assert(git_index_name_entrycount(repo_index) == 3);
 
 	conflict_name = git_index_name_get_byindex(repo_index, 0);
@@ -120,7 +120,7 @@ void test_index_names__cleaned_on_checkout_tree(void)
 	git_reference_name_to_id(&oid, repo, "refs/heads/master");
 	git_object_lookup(&obj, repo, &oid, GIT_OBJ_ANY);
 	git_checkout_tree(repo, obj, &opts);
-	cl_assert(git_index_name_entrycount(repo_index) == 0);
+	cl_assert_equal_sz(0, git_index_name_entrycount(repo_index));
 
 	git_object_free(obj);
 }
@@ -133,7 +133,7 @@ void test_index_names__cleaned_on_checkout_head(void)
 
 	test_index_names__add();
 	git_checkout_head(repo, &opts);
-	cl_assert(git_index_name_entrycount(repo_index) == 0);
+	cl_assert_equal_sz(0, git_index_name_entrycount(repo_index));
 }
 
 void test_index_names__retained_on_checkout_index(void)
diff --git a/tests-clar/index/reuc.c b/tests-clar/index/reuc.c
index 69ed4a933..a18d5602e 100644
--- a/tests-clar/index/reuc.c
+++ b/tests-clar/index/reuc.c
@@ -276,8 +276,6 @@ void test_index_reuc__write(void)
 		0100644, &their_oid));
 
 	cl_git_pass(git_index_write(repo_index));
-
-	cl_git_pass(git_index_read(repo_index));
 	cl_assert_equal_i(2, git_index_reuc_entrycount(repo_index));
 
 	/* ensure sort order was round-tripped correct */
diff --git a/tests-clar/index/tests.c b/tests-clar/index/tests.c
index 009d393d7..09b05bf6e 100644
--- a/tests-clar/index/tests.c
+++ b/tests-clar/index/tests.c
@@ -8,7 +8,7 @@ static const size_t index_entry_count_2 = 1437;
 #define TEST_INDEXBIG_PATH cl_fixture("big.index")
 
 
-// Suite data
+/* Suite data */
 struct test_entry {
    size_t index;
    char path[128];
@@ -24,7 +24,7 @@ static struct test_entry test_entries[] = {
    {48, "src/revobject.h", 1448, 0x4C3F7FE2}
 };
 
-// Helpers
+/* Helpers */
 static void copy_file(const char *src, const char *dst)
 {
 	git_buf source_buf = GIT_BUF_INIT;
@@ -32,7 +32,7 @@ static void copy_file(const char *src, const char *dst)
 
 	cl_git_pass(git_futils_readbuffer(&source_buf, src));
 
-	dst_fd = git_futils_creat_withpath(dst, 0777, 0666); //-V536
+	dst_fd = git_futils_creat_withpath(dst, 0777, 0666); /* -V536 */
 	if (dst_fd < 0)
 		goto cleanup;
 
@@ -66,7 +66,7 @@ static void files_are_equal(const char *a, const char *b)
 }
 
 
-// Fixture setup and teardown
+/* Fixture setup and teardown */
 void test_index_tests__initialize(void)
 {
 }
@@ -173,7 +173,8 @@ void test_index_tests__write(void)
 
 void test_index_tests__sort0(void)
 {
-   // sort the entires in an index
+	/* sort the entires in an index */
+
    /*
    * TODO: This no longer applies:
    * index sorting in Git uses some specific changes to the way
@@ -187,7 +188,7 @@ void test_index_tests__sort0(void)
 
 void test_index_tests__sort1(void)
 {
-   // sort the entires in an empty index
+   /* sort the entires in an empty index */
    git_index *index;
 
    cl_git_pass(git_index_open(&index, "fake-index"));
@@ -349,14 +350,14 @@ void test_index_tests__remove_entry(void)
 	cl_git_pass(git_index_add_bypath(index, "hello"));
 	cl_git_pass(git_index_write(index));
 
-	cl_git_pass(git_index_read(index)); /* reload */
+	cl_git_pass(git_index_read(index, true)); /* reload */
 	cl_assert(git_index_entrycount(index) == 1);
 	cl_assert(git_index_get_bypath(index, "hello", 0) != NULL);
 
 	cl_git_pass(git_index_remove(index, "hello", 0));
 	cl_git_pass(git_index_write(index));
 
-	cl_git_pass(git_index_read(index)); /* reload */
+	cl_git_pass(git_index_read(index, true)); /* reload */
 	cl_assert(git_index_entrycount(index) == 0);
 	cl_assert(git_index_get_bypath(index, "hello", 0) == NULL);
 
@@ -388,7 +389,7 @@ void test_index_tests__remove_directory(void)
 	cl_git_pass(git_index_add_bypath(index, "b.txt"));
 	cl_git_pass(git_index_write(index));
 
-	cl_git_pass(git_index_read(index)); /* reload */
+	cl_git_pass(git_index_read(index, true)); /* reload */
 	cl_assert_equal_i(4, (int)git_index_entrycount(index));
 	cl_assert(git_index_get_bypath(index, "a/1.txt", 0) != NULL);
 	cl_assert(git_index_get_bypath(index, "a/2.txt", 0) != NULL);
@@ -397,7 +398,7 @@ void test_index_tests__remove_directory(void)
 	cl_git_pass(git_index_remove(index, "a/1.txt", 0));
 	cl_git_pass(git_index_write(index));
 
-	cl_git_pass(git_index_read(index)); /* reload */
+	cl_git_pass(git_index_read(index, true)); /* reload */
 	cl_assert_equal_i(3, (int)git_index_entrycount(index));
 	cl_assert(git_index_get_bypath(index, "a/1.txt", 0) == NULL);
 	cl_assert(git_index_get_bypath(index, "a/2.txt", 0) != NULL);
@@ -406,7 +407,7 @@ void test_index_tests__remove_directory(void)
 	cl_git_pass(git_index_remove_directory(index, "a", 0));
 	cl_git_pass(git_index_write(index));
 
-	cl_git_pass(git_index_read(index)); /* reload */
+	cl_git_pass(git_index_read(index, true)); /* reload */
 	cl_assert_equal_i(1, (int)git_index_entrycount(index));
 	cl_assert(git_index_get_bypath(index, "a/1.txt", 0) == NULL);
 	cl_assert(git_index_get_bypath(index, "a/2.txt", 0) == NULL);
@@ -517,7 +518,7 @@ void test_index_tests__reload_from_disk(void)
 	/* Sync the changes back into the read_index */
 	cl_assert_equal_sz(0, git_index_entrycount(read_index));
 
-	cl_git_pass(git_index_read(read_index));
+	cl_git_pass(git_index_read(read_index, true));
 	cl_assert_equal_i(true, read_index->on_disk);
 
 	cl_assert_equal_sz(2, git_index_entrycount(read_index));
@@ -526,7 +527,7 @@ void test_index_tests__reload_from_disk(void)
 	cl_git_pass(p_unlink(write_index->index_file_path));
 
 	/* Sync the changes back into the read_index */
-	cl_git_pass(git_index_read(read_index));
+	cl_git_pass(git_index_read(read_index, true));
 	cl_assert_equal_i(false, read_index->on_disk);
 	cl_assert_equal_sz(0, git_index_entrycount(read_index));
 
diff --git a/tests-clar/status/worktree.c b/tests-clar/status/worktree.c
index 3b569c7ba..34be6d34c 100644
--- a/tests-clar/status/worktree.c
+++ b/tests-clar/status/worktree.c
@@ -470,16 +470,15 @@ void test_status_worktree__conflict_with_diff3(void)
 	cl_assert_equal_i(GIT_STATUS_WT_MODIFIED, status);
 
 	cl_git_pass(git_repository_index(&index, repo));
-
 	cl_git_pass(git_index_remove(index, "modified_file", 0));
-	cl_git_pass(git_index_conflict_add(index, &ancestor_entry,
-		&our_entry, &their_entry));
+	cl_git_pass(git_index_conflict_add(
+		index, &ancestor_entry, &our_entry, &their_entry));
+	cl_git_pass(git_index_write(index));
+	git_index_free(index);
 
 	cl_git_pass(git_status_file(&status, repo, "modified_file"));
 
 	cl_assert_equal_i(GIT_STATUS_INDEX_DELETED | GIT_STATUS_WT_NEW, status);
-
-	git_index_free(index);
 }
 
 static const char *filemode_paths[] = {