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

Fix some observed problems with incorrect diffs

7 files changed, 131 insertions, 46 deletions

diff --git a/src/checkout.c b/src/checkout.c
index 76edc6a72..dce9afdf4 100644
--- a/src/checkout.c
+++ b/src/checkout.c
@@ -1837,7 +1837,7 @@ static int checkout_data_init(
 		} else {
 			/* otherwise, grab and reload the index */
 			if ((error = git_repository_index(&data->index, data->repo)) < 0 ||
-				(error = git_index_read(data->index)) < 0)
+				(error = git_index_read(data->index, true)) < 0)
 				goto cleanup;
 
 			/* cannot checkout if unresolved conflicts exist */
diff --git a/src/diff.c b/src/diff.c
index 37bc737d6..4c33a0213 100644
--- a/src/diff.c
+++ b/src/diff.c
@@ -63,13 +63,16 @@ static int diff_notify(
 
 static int diff_delta__from_one(
 	git_diff *diff,
-	git_delta_t   status,
+	git_delta_t status,
 	const git_index_entry *entry)
 {
 	git_diff_delta *delta;
 	const char *matched_pathspec;
 	int notify_res;
 
+	if ((entry->flags & GIT_IDXENTRY_VALID) != 0)
+		return 0;
+
 	if (status == GIT_DELTA_IGNORED &&
 		DIFF_FLAG_ISNT_SET(diff, GIT_DIFF_INCLUDE_IGNORED))
 		return 0;
@@ -426,7 +429,7 @@ static int diff_list_apply_options(
 		diff->diffcaps = diff->diffcaps | GIT_DIFFCAPS_HAS_SYMLINKS;
 
 	if (!git_repository__cvar(&val, repo, GIT_CVAR_IGNORESTAT) && val)
-		diff->diffcaps = diff->diffcaps | GIT_DIFFCAPS_ASSUME_UNCHANGED;
+		diff->diffcaps = diff->diffcaps | GIT_DIFFCAPS_IGNORE_STAT;
 
 	if ((diff->opts.flags & GIT_DIFF_IGNORE_FILEMODE) == 0 &&
 		!git_repository__cvar(&val, repo, GIT_CVAR_FILEMODE) && val)
@@ -448,6 +451,13 @@ static int diff_list_apply_options(
 		/* add other defaults here */
 	}
 
+	/* Reverse src info if diff is reversed */
+	if (DIFF_FLAG_IS_SET(diff, GIT_DIFF_REVERSE)) {
+		git_iterator_type_t tmp_src = diff->old_src;
+		diff->old_src = diff->new_src;
+		diff->new_src = tmp_src;
+	}
+
 	/* if ignore_submodules not explicitly set, check diff config */
 	if (diff->opts.ignore_submodules <= 0) {
 		const char *str;
@@ -484,9 +494,9 @@ static int diff_list_apply_options(
 		return -1;
 
 	if (DIFF_FLAG_IS_SET(diff, GIT_DIFF_REVERSE)) {
-		const char *swap = diff->opts.old_prefix;
-		diff->opts.old_prefix = diff->opts.new_prefix;
-		diff->opts.new_prefix = swap;
+		const char *tmp_prefix = diff->opts.old_prefix;
+		diff->opts.old_prefix  = diff->opts.new_prefix;
+		diff->opts.new_prefix  = tmp_prefix;
 	}
 
 	return 0;
@@ -694,9 +704,8 @@ static int maybe_modified(
 		nmode = (nmode & ~MODE_BITS_MASK) | (omode & MODE_BITS_MASK);
 
 	/* support "assume unchanged" (poorly, b/c we still stat everything) */
-	if ((diff->diffcaps & GIT_DIFFCAPS_ASSUME_UNCHANGED) != 0)
-		status = (oitem->flags_extended & GIT_IDXENTRY_INTENT_TO_ADD) ?
-			GIT_DELTA_MODIFIED : GIT_DELTA_UNMODIFIED;
+	if ((oitem->flags & GIT_IDXENTRY_VALID) != 0)
+		status = GIT_DELTA_UNMODIFIED;
 
 	/* support "skip worktree" index bit */
 	else if ((oitem->flags_extended & GIT_IDXENTRY_SKIP_WORKTREE) != 0)
@@ -1177,6 +1186,17 @@ int git_diff_tree_to_tree(
 	return error;
 }
 
+static int diff_load_index(git_index **index, git_repository *repo)
+{
+	int error = git_repository_index__weakptr(index, repo);
+
+	/* reload the repository index when user did not pass one in */
+	if (!error && git_index_read(*index, false) < 0)
+		giterr_clear();
+
+	return error;
+}
+
 int git_diff_tree_to_index(
 	git_diff **diff,
 	git_repository *repo,
@@ -1189,7 +1209,7 @@ int git_diff_tree_to_index(
 
 	assert(diff && repo);
 
-	if (!index && (error = git_repository_index__weakptr(&index, repo)) < 0)
+	if (!index && (error = diff_load_index(&index, repo)) < 0)
 		return error;
 
 	if (index->ignore_case) {
@@ -1232,7 +1252,7 @@ int git_diff_index_to_workdir(
 
 	assert(diff && repo);
 
-	if (!index && (error = git_repository_index__weakptr(&index, repo)) < 0)
+	if (!index && (error = diff_load_index(&index, repo)) < 0)
 		return error;
 
 	DIFF_FROM_ITERATORS(
@@ -1271,11 +1291,15 @@ int git_diff_tree_to_workdir_with_index(
 {
 	int error = 0;
 	git_diff *d1 = NULL, *d2 = NULL;
+	git_index *index = NULL;
 
 	assert(diff && repo);
 
-	if (!(error = git_diff_tree_to_index(&d1, repo, old_tree, NULL, opts)) &&
-		!(error = git_diff_index_to_workdir(&d2, repo, NULL, opts)))
+	if ((error = diff_load_index(&index, repo)) < 0)
+		return error;
+
+	if (!(error = git_diff_tree_to_index(&d1, repo, old_tree, index, opts)) &&
+		!(error = git_diff_index_to_workdir(&d2, repo, index, opts)))
 		error = git_diff_merge(d1, d2);
 
 	git_diff_free(d2);
@@ -1289,6 +1313,19 @@ int git_diff_tree_to_workdir_with_index(
 	return error;
 }
 
+int git_diff_options_init(git_diff_options *options, unsigned int version)
+{
+	git_diff_options template = GIT_DIFF_OPTIONS_INIT;
+
+	if (version != template.version) {
+		giterr_set(GITERR_INVALID,
+			"Invalid version %d for git_diff_options", (int)version);
+		return -1;
+	}
+
+	memcpy(options, &template, sizeof(*options));
+	return 0;
+}
 
 size_t git_diff_num_deltas(const git_diff *diff)
 {
diff --git a/src/diff.h b/src/diff.h
index 270bea071..2c9298a5f 100644
--- a/src/diff.h
+++ b/src/diff.h
@@ -23,7 +23,7 @@
 
 enum {
 	GIT_DIFFCAPS_HAS_SYMLINKS     = (1 << 0), /* symlinks on platform? */
-	GIT_DIFFCAPS_ASSUME_UNCHANGED = (1 << 1), /* use stat? */
+	GIT_DIFFCAPS_IGNORE_STAT      = (1 << 1), /* use stat? */
 	GIT_DIFFCAPS_TRUST_MODE_BITS  = (1 << 2), /* use st_mode? */
 	GIT_DIFFCAPS_TRUST_CTIME      = (1 << 3), /* use st_ctime? */
 	GIT_DIFFCAPS_USE_DEV          = (1 << 4), /* use st_dev? */
diff --git a/src/diff_tform.c b/src/diff_tform.c
index 9461ca2c8..28a9cc70d 100644
--- a/src/diff_tform.c
+++ b/src/diff_tform.c
@@ -46,7 +46,9 @@ fail:
 }
 
 static git_diff_delta *diff_delta__merge_like_cgit(
-	const git_diff_delta *a, const git_diff_delta *b, git_pool *pool)
+	const git_diff_delta *a,
+	const git_diff_delta *b,
+	git_pool *pool)
 {
 	git_diff_delta *dup;
 
@@ -96,15 +98,46 @@ static git_diff_delta *diff_delta__merge_like_cgit(
 	return dup;
 }
 
-int git_diff_merge(
-	git_diff *onto,
-	const git_diff *from)
+static git_diff_delta *diff_delta__merge_like_cgit_reversed(
+	const git_diff_delta *a,
+	const git_diff_delta *b,
+	git_pool *pool)
+{
+	git_diff_delta *dup;
+
+	/* reversed version of above logic */
+
+	if (a->status == GIT_DELTA_UNMODIFIED)
+		return diff_delta__dup(b, pool);
+
+	if ((dup = diff_delta__dup(a, pool)) == NULL)
+		return NULL;
+
+	if (b->status == GIT_DELTA_UNMODIFIED || b->status == GIT_DELTA_UNTRACKED)
+		return dup;
+
+	if (dup->status == GIT_DELTA_DELETED) {
+		if (b->status == GIT_DELTA_ADDED)
+			dup->status = GIT_DELTA_UNMODIFIED;
+	} else {
+		dup->status = b->status;
+	}
+
+	git_oid_cpy(&dup->old_file.oid, &b->old_file.oid);
+	dup->old_file.mode  = b->old_file.mode;
+	dup->old_file.size  = b->old_file.size;
+	dup->old_file.flags = b->old_file.flags;
+
+	return dup;
+}
+
+int git_diff_merge(git_diff *onto, const git_diff *from)
 {
 	int error = 0;
 	git_pool onto_pool;
 	git_vector onto_new;
 	git_diff_delta *delta;
-	bool ignore_case = false;
+	bool ignore_case, reversed;
 	unsigned int i, j;
 
 	assert(onto && from);
@@ -112,26 +145,26 @@ int git_diff_merge(
 	if (!from->deltas.length)
 		return 0;
 
+	ignore_case = ((onto->opts.flags & GIT_DIFF_IGNORE_CASE) != 0);
+	reversed    = ((onto->opts.flags & GIT_DIFF_REVERSE) != 0);
+
+	if (ignore_case != ((from->opts.flags & GIT_DIFF_IGNORE_CASE) != 0) ||
+		reversed    != ((from->opts.flags & GIT_DIFF_REVERSE) != 0)) {
+		giterr_set(GITERR_INVALID,
+			"Attempt to merge diffs created with conflicting options");
+		return -1;
+	}
+
 	if (git_vector_init(
 			&onto_new, onto->deltas.length, git_diff_delta__cmp) < 0 ||
 		git_pool_init(&onto_pool, 1, 0) < 0)
 		return -1;
 
-	if ((onto->opts.flags & GIT_DIFF_IGNORE_CASE) != 0 ||
-		(from->opts.flags & GIT_DIFF_IGNORE_CASE) != 0)
-	{
-		ignore_case = true;
-
-		/* This function currently only supports merging diff lists that
-		 * are sorted identically. */
-		assert((onto->opts.flags & GIT_DIFF_IGNORE_CASE) != 0 &&
-				(from->opts.flags & GIT_DIFF_IGNORE_CASE) != 0);
-	}
-
 	for (i = 0, j = 0; i < onto->deltas.length || j < from->deltas.length; ) {
 		git_diff_delta *o = GIT_VECTOR_GET(&onto->deltas, i);
 		const git_diff_delta *f = GIT_VECTOR_GET(&from->deltas, j);
-		int cmp = !f ? -1 : !o ? 1 : STRCMP_CASESELECT(ignore_case, o->old_file.path, f->old_file.path);
+		int cmp = !f ? -1 : !o ? 1 :
+			STRCMP_CASESELECT(ignore_case, o->old_file.path, f->old_file.path);
 
 		if (cmp < 0) {
 			delta = diff_delta__dup(o, &onto_pool);
@@ -140,7 +173,9 @@ int git_diff_merge(
 			delta = diff_delta__dup(f, &onto_pool);
 			j++;
 		} else {
-			delta = diff_delta__merge_like_cgit(o, f, &onto_pool);
+			delta = reversed ?
+				diff_delta__merge_like_cgit_reversed(o, f, &onto_pool) :
+				diff_delta__merge_like_cgit(o, f, &onto_pool);
 			i++;
 			j++;
 		}
@@ -160,7 +195,11 @@ int git_diff_merge(
 	if (!error) {
 		git_vector_swap(&onto->deltas, &onto_new);
 		git_pool_swap(&onto->pool, &onto_pool);
-		onto->new_src = from->new_src;
+
+		if ((onto->opts.flags & GIT_DIFF_REVERSE) != 0)
+			onto->old_src = from->old_src;
+		else
+			onto->new_src = from->new_src;
 
 		/* prefix strings also come from old pool, so recreate those.*/
 		onto->opts.old_prefix =
diff --git a/src/index.c b/src/index.c
index 5cdd40aaa..dbf1ab529 100644
--- a/src/index.c
+++ b/src/index.c
@@ -349,7 +349,7 @@ int git_index_open(git_index **index_out, const char *index_path)
 	*index_out = index;
 	GIT_REFCOUNT_INC(index);
 
-	return (index_path != NULL) ? git_index_read(index) : 0;
+	return (index_path != NULL) ? git_index_read(index, true) : 0;
 }
 
 int git_index_new(git_index **out)
@@ -451,11 +451,11 @@ unsigned int git_index_caps(const git_index *index)
 			(index->no_symlinks ? GIT_INDEXCAP_NO_SYMLINKS : 0));
 }
 
-int git_index_read(git_index *index)
+int git_index_read(git_index *index, int force)
 {
 	int error = 0, updated;
 	git_buf buffer = GIT_BUF_INIT;
-	git_futils_filestamp stamp = {0};
+	git_futils_filestamp stamp = index->stamp;
 
 	if (!index->index_file_path)
 		return create_index_error(-1,
@@ -464,12 +464,13 @@ int git_index_read(git_index *index)
 	index->on_disk = git_path_exists(index->index_file_path);
 
 	if (!index->on_disk) {
-		git_index_clear(index);
+		if (force)
+			git_index_clear(index);
 		return 0;
 	}
 
 	updated = git_futils_filestamp_check(&stamp, index->index_file_path);
-	if (updated <= 0)
+	if (updated < 0 || (!updated && !force))
 		return updated;
 
 	error = git_futils_readbuffer(&buffer, index->index_file_path);
diff --git a/src/status.c b/src/status.c
index 2b84794b5..07fdcb5c3 100644
--- a/src/status.c
+++ b/src/status.c
@@ -251,12 +251,17 @@ int git_status_list_new(
 		return error;
 
 	/* if there is no HEAD, that's okay - we'll make an empty iterator */
-	if (((error = git_repository_head_tree(&head, repo)) < 0) &&
-		error != GIT_ENOTFOUND && error != GIT_EUNBORNBRANCH) {
-		git_index_free(index); /* release index */
-		return error;
+	if ((error = git_repository_head_tree(&head, repo)) < 0) {
+		if (error != GIT_ENOTFOUND && error != GIT_EUNBORNBRANCH)
+			goto done;
+		giterr_clear();
 	}
 
+	/* refresh index from disk unless prevented */
+	if ((flags & GIT_STATUS_OPT_NO_REFRESH) == 0 &&
+		git_index_read(index, false) < 0)
+		giterr_clear();
+
 	status = git_status_list_alloc(index);
 	GITERR_CHECK_ALLOC(status);
 
@@ -291,7 +296,7 @@ int git_status_list_new(
 
 	if (show != GIT_STATUS_SHOW_WORKDIR_ONLY) {
 		if ((error = git_diff_tree_to_index(
-				&status->head2idx, repo, head, NULL, &diffopt)) < 0)
+				&status->head2idx, repo, head, index, &diffopt)) < 0)
 			goto done;
 
 		if ((flags & GIT_STATUS_OPT_RENAMES_HEAD_TO_INDEX) != 0 &&
@@ -301,7 +306,7 @@ int git_status_list_new(
 
 	if (show != GIT_STATUS_SHOW_INDEX_ONLY) {
 		if ((error = git_diff_index_to_workdir(
-				&status->idx2wd, repo, NULL, &diffopt)) < 0)
+				&status->idx2wd, repo, index, &diffopt)) < 0)
 			goto done;
 
 		if ((flags & GIT_STATUS_OPT_RENAMES_INDEX_TO_WORKDIR) != 0 &&
diff --git a/src/submodule.c b/src/submodule.c
index 18d80f0a9..586494fed 100644
--- a/src/submodule.c
+++ b/src/submodule.c
@@ -1527,6 +1527,7 @@ static void submodule_get_wd_status(
 	const git_oid *wd_oid =
 		(sm->flags & GIT_SUBMODULE_STATUS__WD_OID_VALID) ? &sm->wd_oid : NULL;
 	git_tree *sm_head = NULL;
+	git_index *index = NULL;
 	git_diff_options opt = GIT_DIFF_OPTIONS_INIT;
 	git_diff *diff;
 
@@ -1558,12 +1559,14 @@ static void submodule_get_wd_status(
 	if (ign == GIT_SUBMODULE_IGNORE_NONE)
 		opt.flags |= GIT_DIFF_INCLUDE_UNTRACKED;
 
+	(void)git_repository_index__weakptr(&index, sm_repo);
+
 	/* if we don't have an unborn head, check diff with index */
 	if (git_repository_head_tree(&sm_head, sm_repo) < 0)
 		giterr_clear();
 	else {
 		/* perform head to index diff on submodule */
-		if (git_diff_tree_to_index(&diff, sm_repo, sm_head, NULL, &opt) < 0)
+		if (git_diff_tree_to_index(&diff, sm_repo, sm_head, index, &opt) < 0)
 			giterr_clear();
 		else {
 			if (git_diff_num_deltas(diff) > 0)
@@ -1576,7 +1579,7 @@ static void submodule_get_wd_status(
 	}
 
 	/* perform index-to-workdir diff on submodule */
-	if (git_diff_index_to_workdir(&diff, sm_repo, NULL, &opt) < 0)
+	if (git_diff_index_to_workdir(&diff, sm_repo, index, &opt) < 0)
 		giterr_clear();
 	else {
 		size_t untracked =