Improve case handling in git_diff__paired_foreach

This commit reinstates some changes to git_diff__paired_foreach
that were discarded during the rebase (because the diff_output.c
file had gone away), and also adjusts the case insensitively
logic slightly to hopefully deal with either mismatched icase
diffs and other case insensitivity scenarios.

src/diff.c

@@ -254,6 +254,13 @@ int git_diff_delta__cmp(const void *a, const void *b)
 	return val ? val : ((int)da->status - (int)db->status);
 }
 
+int git_diff_delta__casecmp(const void *a, const void *b)
+{
+	const git_diff_delta *da = a, *db = b;
+	int val = strcasecmp(diff_delta__path(da), diff_delta__path(db));
+	return val ? val : ((int)da->status - (int)db->status);
+}
+
 bool git_diff_delta__should_skip(
 	const git_diff_options *opts, const git_diff_delta *delta)
 {
@@ -1197,51 +1204,78 @@ size_t git_diff_num_deltas_of_type(git_diff_list *diff, git_delta_t type)
 }
 
 int git_diff__paired_foreach(
-	git_diff_list *idx2head,
-	git_diff_list *wd2idx,
-	int (*cb)(git_diff_delta *i2h, git_diff_delta *w2i, void *payload),
+	git_diff_list *head2idx,
+	git_diff_list *idx2wd,
+	int (*cb)(git_diff_delta *h2i, git_diff_delta *i2w, void *payload),
 	void *payload)
 {
 	int cmp;
-	git_diff_delta *i2h, *w2i;
+	git_diff_delta *h2i, *i2w;
 	size_t i, j, i_max, j_max;
-	int (*strcomp)(const char *, const char *);
-
-	i_max = idx2head ? idx2head->deltas.length : 0;
-	j_max = wd2idx   ? wd2idx->deltas.length   : 0;
+	int (*strcomp)(const char *, const char *) = git__strcmp;
+	bool icase_mismatch;
 
-	/* Get appropriate strcmp function */
-	strcomp = idx2head ? idx2head->strcomp : wd2idx ? wd2idx->strcomp : NULL;
+	i_max = head2idx ? head2idx->deltas.length : 0;
+	j_max = idx2wd ? idx2wd->deltas.length : 0;
 
-	/* Assert both iterators use matching ignore-case. If this function ever
-	* supports merging diffs that are not sorted by the same function, then
-	* it will need to spool and sort on one of the results before merging
-	*/
-	if (idx2head && wd2idx) {
-		assert(idx2head->strcomp == wd2idx->strcomp);
+	/* At some point, tree-to-index diffs will probably never ignore case,
+	 * even if that isn't true now.  Index-to-workdir diffs may or may not
+	 * ignore case, but the index filename for the idx2wd diff should
+	 * still be using the canonical case-preserving name.
+	 *
+	 * Therefore the main thing we need to do here is make sure the diffs
+	 * are traversed in a compatible order.  To do this, we temporarily
+	 * resort a mismatched diff to get the order correct.
+	 */
+	icase_mismatch =
+		(head2idx != NULL && idx2wd != NULL &&
+		 ((head2idx->opts.flags ^ idx2wd->opts.flags) & GIT_DIFF_DELTAS_ARE_ICASE));
+
+	/* force case-sensitive delta sort */
+	if (icase_mismatch) {
+		if (head2idx->opts.flags & GIT_DIFF_DELTAS_ARE_ICASE) {
+			head2idx->deltas._cmp = git_diff_delta__cmp;
+			git_vector_sort(&head2idx->deltas);
+		} else {
+			idx2wd->deltas._cmp = git_diff_delta__cmp;
+			git_vector_sort(&idx2wd->deltas);
+		}
 	}
+	else if (head2idx->opts.flags & GIT_DIFF_DELTAS_ARE_ICASE)
+		strcomp = git__strcasecmp;
 
 	for (i = 0, j = 0; i < i_max || j < j_max; ) {
-		i2h = idx2head ? GIT_VECTOR_GET(&idx2head->deltas,i) : NULL;
-		w2i = wd2idx   ? GIT_VECTOR_GET(&wd2idx->deltas,j)   : NULL;
+		h2i = head2idx ? GIT_VECTOR_GET(&head2idx->deltas, i) : NULL;
+		i2w = idx2wd ? GIT_VECTOR_GET(&idx2wd->deltas, j) : NULL;
 
-		cmp = !w2i ? -1 : !i2h ? 1 :
-			strcomp(i2h->old_file.path, w2i->old_file.path);
+		cmp = !i2w ? -1 : !h2i ? 1 :
+			strcomp(h2i->new_file.path, i2w->old_file.path);
 
 		if (cmp < 0) {
-			if (cb(i2h, NULL, payload))
+			if (cb(h2i, NULL, payload))
 				return GIT_EUSER;
 			i++;
 		} else if (cmp > 0) {
-			if (cb(NULL, w2i, payload))
+			if (cb(NULL, i2w, payload))
 				return GIT_EUSER;
 			j++;
 		} else {
-			if (cb(i2h, w2i, payload))
+			if (cb(h2i, i2w, payload))
 				return GIT_EUSER;
 			i++; j++;
 		}
 	}
 
+	/* restore case-insensitive delta sort */
+	if (icase_mismatch) {
+		if (head2idx->opts.flags & GIT_DIFF_DELTAS_ARE_ICASE) {
+			head2idx->deltas._cmp = git_diff_delta__casecmp;
+			git_vector_sort(&head2idx->deltas);
+		} else {
+			idx2wd->deltas._cmp = git_diff_delta__casecmp;
+			git_vector_sort(&idx2wd->deltas);
+		}
+	}
+
 	return 0;
 }

src/diff.h

@@ -74,6 +74,7 @@ extern void git_diff__cleanup_modes(
 extern void git_diff_list_addref(git_diff_list *diff);
 
 extern int git_diff_delta__cmp(const void *a, const void *b);
+extern int git_diff_delta__casecmp(const void *a, const void *b);
 
 extern bool git_diff_delta__should_skip(
 	const git_diff_options *opts, const git_diff_delta *delta);

src/status.c

@@ -130,12 +130,12 @@ static int status_collect(
 	git_diff_delta *idx2wd,
 	void *payload)
 {
-	git_status_list *statuslist = payload;
+	git_status_list *status = payload;
 	git_status_entry *status_entry;
-	
-	if (!status_is_included(statuslist, head2idx, idx2wd))
+
+	if (!status_is_included(status, head2idx, idx2wd))
 		return 0;
-	
+
 	status_entry = git__malloc(sizeof(git_status_entry));
 	GITERR_CHECK_ALLOC(status_entry);
 
@@ -143,9 +143,7 @@ static int status_collect(
 	status_entry->head_to_index = head2idx;
 	status_entry->index_to_workdir = idx2wd;
 
-	git_vector_insert(&statuslist->paired, status_entry);
-
-	return 0;
+	return git_vector_insert(&status->paired, status_entry);
 }
 
 GIT_INLINE(int) status_entry_cmp_base(
@@ -184,18 +182,23 @@ static int status_entry_cmp(const void *a, const void *b)
 
 static git_status_list *git_status_list_alloc(git_index *index)
 {
-	git_status_list *statuslist = NULL;
+	git_status_list *status = NULL;
 	int (*entrycmp)(const void *a, const void *b);
 
+	if (!(status = git__calloc(1, sizeof(git_status_list))))
+		return NULL;
+
 	entrycmp = index->ignore_case ? status_entry_icmp : status_entry_cmp;
 
-	if ((statuslist = git__calloc(1, sizeof(git_status_list))) == NULL ||
-		git_vector_init(&statuslist->paired, 0, entrycmp) < 0)
+	if (git_vector_init(&status->paired, 0, entrycmp) < 0) {
+		git__free(status);
 		return NULL;
+	}
 
-	return statuslist;
+	return status;
 }
 
+/*
 static int newfile_cmp(const void *a, const void *b)
 {
 	const git_diff_delta *delta_a = a;
@@ -211,6 +214,7 @@ static int newfile_casecmp(const void *a, const void *b)
 
 	return git__strcasecmp(delta_a->new_file.path, delta_b->new_file.path);
 }
+*/
 
 int git_status_list_new(
 	git_status_list **out,
@@ -218,13 +222,14 @@ int git_status_list_new(
 	const git_status_options *opts)
 {
 	git_index *index = NULL;
-	git_status_list *statuslist = NULL;
+	git_status_list *status = NULL;
 	git_diff_options diffopt = GIT_DIFF_OPTIONS_INIT;
 	git_diff_find_options findopts_i2w = GIT_DIFF_FIND_OPTIONS_INIT;
 	git_tree *head = NULL;
 	git_status_show_t show =
 		opts ? opts->show : GIT_STATUS_SHOW_INDEX_AND_WORKDIR;
 	int error = 0;
+	unsigned int flags = opts ? opts->flags : GIT_STATUS_OPT_DEFAULTS;
 
 	assert(show <= GIT_STATUS_SHOW_INDEX_THEN_WORKDIR);
 
@@ -238,123 +243,121 @@ int git_status_list_new(
 
 	/* 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_EORPHANEDHEAD))
+		error != GIT_ENOTFOUND && error != GIT_EORPHANEDHEAD) {
+		git_index_free(index); /* release index */
 		return error;
+	}
 
-	statuslist = git_status_list_alloc(index);
-	GITERR_CHECK_ALLOC(statuslist);
-
-	memcpy(&statuslist->opts, opts, sizeof(git_status_options));
+	status = git_status_list_alloc(index);
+	GITERR_CHECK_ALLOC(status);
 
-	memcpy(&diffopt.pathspec, &opts->pathspec, sizeof(diffopt.pathspec));
+	if (opts) {
+		memcpy(&status->opts, opts, sizeof(git_status_options));
+		memcpy(&diffopt.pathspec, &opts->pathspec, sizeof(diffopt.pathspec));
+	}
 
 	diffopt.flags = GIT_DIFF_INCLUDE_TYPECHANGE;
 
-	if ((opts->flags & GIT_STATUS_OPT_INCLUDE_UNTRACKED) != 0)
+	if ((flags & GIT_STATUS_OPT_INCLUDE_UNTRACKED) != 0)
 		diffopt.flags = diffopt.flags | GIT_DIFF_INCLUDE_UNTRACKED;
-	if ((opts->flags & GIT_STATUS_OPT_INCLUDE_IGNORED) != 0)
+	if ((flags & GIT_STATUS_OPT_INCLUDE_IGNORED) != 0)
 		diffopt.flags = diffopt.flags | GIT_DIFF_INCLUDE_IGNORED;
-	if ((opts->flags & GIT_STATUS_OPT_INCLUDE_UNMODIFIED) != 0)
+	if ((flags & GIT_STATUS_OPT_INCLUDE_UNMODIFIED) != 0)
 		diffopt.flags = diffopt.flags | GIT_DIFF_INCLUDE_UNMODIFIED;
-	if ((opts->flags & GIT_STATUS_OPT_RECURSE_UNTRACKED_DIRS) != 0)
+	if ((flags & GIT_STATUS_OPT_RECURSE_UNTRACKED_DIRS) != 0)
 		diffopt.flags = diffopt.flags | GIT_DIFF_RECURSE_UNTRACKED_DIRS;
-	if ((opts->flags & GIT_STATUS_OPT_DISABLE_PATHSPEC_MATCH) != 0)
+	if ((flags & GIT_STATUS_OPT_DISABLE_PATHSPEC_MATCH) != 0)
 		diffopt.flags = diffopt.flags | GIT_DIFF_DISABLE_PATHSPEC_MATCH;
-	if ((opts->flags & GIT_STATUS_OPT_RECURSE_IGNORED_DIRS) != 0)
+	if ((flags & GIT_STATUS_OPT_RECURSE_IGNORED_DIRS) != 0)
 		diffopt.flags = diffopt.flags | GIT_DIFF_RECURSE_IGNORED_DIRS;
-	if ((opts->flags & GIT_STATUS_OPT_EXCLUDE_SUBMODULES) != 0)
+	if ((flags & GIT_STATUS_OPT_EXCLUDE_SUBMODULES) != 0)
 		diffopt.flags = diffopt.flags | GIT_DIFF_IGNORE_SUBMODULES;
 
 	findopts_i2w.flags |= GIT_DIFF_FIND_FOR_UNTRACKED;
 
 	if (show != GIT_STATUS_SHOW_WORKDIR_ONLY) {
-		if ((error = git_diff_tree_to_index(&statuslist->head2idx, repo, head, NULL, &diffopt)) < 0)
-			goto on_error;
+		if ((error = git_diff_tree_to_index(
+				&status->head2idx, repo, head, NULL, &diffopt)) < 0)
+			goto done;
 
-		if ((opts->flags & GIT_STATUS_OPT_RENAMES_HEAD_TO_INDEX) != 0 &&
-			(error = git_diff_find_similar(statuslist->head2idx, NULL)) < 0)
-			goto on_error;
+		if ((flags & GIT_STATUS_OPT_RENAMES_HEAD_TO_INDEX) != 0 &&
+			(error = git_diff_find_similar(status->head2idx, NULL)) < 0)
+			goto done;
 	}
 
 	if (show != GIT_STATUS_SHOW_INDEX_ONLY) {
-		if ((error = git_diff_index_to_workdir(&statuslist->idx2wd, repo, NULL, &diffopt)) < 0)
-			goto on_error;
+		if ((error = git_diff_index_to_workdir(
+				&status->idx2wd, repo, NULL, &diffopt)) < 0)
+			goto done;
 
-		if ((opts->flags & GIT_STATUS_OPT_RENAMES_INDEX_TO_WORKDIR) != 0 &&
-			(error = git_diff_find_similar(statuslist->idx2wd, &findopts_i2w)) < 0)
-			goto on_error;
+		if ((flags & GIT_STATUS_OPT_RENAMES_INDEX_TO_WORKDIR) != 0 &&
+			(error = git_diff_find_similar(status->idx2wd, &findopts_i2w)) < 0)
+			goto done;
 	}
 
 	if (show == GIT_STATUS_SHOW_INDEX_THEN_WORKDIR) {
-		if ((error = git_diff__paired_foreach(statuslist->head2idx, NULL, status_collect, statuslist)) < 0)
-			goto on_error;
-
-		git_diff_list_free(statuslist->head2idx);
-		statuslist->head2idx = NULL;
-	}
-
-	if ((opts->flags & GIT_STATUS_OPT_RENAMES_HEAD_TO_INDEX) != 0) {
-		statuslist->head2idx->deltas._cmp =
-			(statuslist->head2idx->opts.flags & GIT_DIFF_DELTAS_ARE_ICASE) != 0 ?
-			newfile_casecmp : newfile_cmp;
+		if ((error = git_diff__paired_foreach(
+				status->head2idx, NULL, status_collect, status)) < 0)
+			goto done;
 
-		git_vector_sort(&statuslist->head2idx->deltas);
+		git_diff_list_free(status->head2idx);
+		status->head2idx = NULL;
 	}
 
-	if ((error = git_diff__paired_foreach(statuslist->head2idx, statuslist->idx2wd,
-		status_collect, statuslist)) < 0)
-		goto on_error;
+	if ((error = git_diff__paired_foreach(
+			status->head2idx, status->idx2wd, status_collect, status)) < 0)
+		goto done;
 
-	if ((opts->flags & GIT_STATUS_OPT_RENAMES_HEAD_TO_INDEX) != 0 ||
-		(opts->flags & GIT_STATUS_OPT_RENAMES_INDEX_TO_WORKDIR) != 0)
-		git_vector_sort(&statuslist->paired);
+	if ((flags & GIT_STATUS_OPT_RENAMES_HEAD_TO_INDEX) != 0 ||
+		(flags & GIT_STATUS_OPT_RENAMES_INDEX_TO_WORKDIR) != 0)
+		git_vector_sort(&status->paired);
 
-	*out = statuslist;
-	goto done;
+done:
+	if (error < 0) {
+		git_status_list_free(status);
+		status = NULL;
+	}
 
-on_error:
-	git_status_list_free(statuslist);
+	*out = status;
 
-done:
 	git_tree_free(head);
 	git_index_free(index);
 
 	return error;
 }
 
-size_t git_status_list_entrycount(git_status_list *statuslist)
+size_t git_status_list_entrycount(git_status_list *status)
 {
-	assert(statuslist);
+	assert(status);
 
-	return statuslist->paired.length;
+	return status->paired.length;
 }
 
-const git_status_entry *git_status_byindex(
-	git_status_list *statuslist, 
-	size_t i)
+const git_status_entry *git_status_byindex(git_status_list *status, size_t i)
 {
-	assert(statuslist);
+	assert(status);
 
-	return git_vector_get(&statuslist->paired, i);
+	return git_vector_get(&status->paired, i);
 }
 
-void git_status_list_free(git_status_list *statuslist)
+void git_status_list_free(git_status_list *status)
 {
 	git_status_entry *status_entry;
 	size_t i;
 
-	if (statuslist == NULL)
+	if (status == NULL)
 		return;
 
-	git_diff_list_free(statuslist->head2idx);
-	git_diff_list_free(statuslist->idx2wd);
+	git_diff_list_free(status->head2idx);
+	git_diff_list_free(status->idx2wd);
 
-	git_vector_foreach(&statuslist->paired, i, status_entry)
+	git_vector_foreach(&status->paired, i, status_entry)
 		git__free(status_entry);
 
-	git_vector_free(&statuslist->paired);
+	git_vector_free(&status->paired);
 
-	git__free(statuslist);
+	git__memzero(status, sizeof(*status));
+	git__free(status);
 }
 
 int git_status_foreach_ext(
@@ -363,15 +366,15 @@ int git_status_foreach_ext(
 	git_status_cb cb,
 	void *payload)
 {
-	git_status_list *statuslist;
+	git_status_list *status;
 	const git_status_entry *status_entry;
 	size_t i;
 	int error = 0;
 
-	if ((error = git_status_list_new(&statuslist, repo, opts)) < 0)
+	if ((error = git_status_list_new(&status, repo, opts)) < 0)
 		return error;
 
-	git_vector_foreach(&statuslist->paired, i, status_entry) {
+	git_vector_foreach(&status->paired, i, status_entry) {
 		const char *path = status_entry->head_to_index ?
 			status_entry->head_to_index->old_file.path :
 			status_entry->index_to_workdir->old_file.path;
@@ -383,24 +386,14 @@ int git_status_foreach_ext(
 		}
 	}
 
-	git_status_list_free(statuslist);
+	git_status_list_free(status);
 
 	return error;
 }
 
-int git_status_foreach(
-	git_repository *repo,
-	git_status_cb callback,
-	void *payload)
+int git_status_foreach(git_repository *repo, git_status_cb cb, void *payload)
 {
-	git_status_options opts = GIT_STATUS_OPTIONS_INIT;
-
-	opts.show = GIT_STATUS_SHOW_INDEX_AND_WORKDIR;
-	opts.flags = GIT_STATUS_OPT_INCLUDE_IGNORED |
-		GIT_STATUS_OPT_INCLUDE_UNTRACKED |
-		GIT_STATUS_OPT_RECURSE_UNTRACKED_DIRS;
-
-	return git_status_foreach_ext(repo, &opts, callback, payload);
+	return git_status_foreach_ext(repo, NULL, cb, payload);
 }
 
 struct status_file_info {

tests-clar/status/worktree.c

@@ -695,3 +695,51 @@ void test_status_worktree__file_status_honors_case_ignorecase_regarding_untracke
     /* Actually returns GIT_STATUS_IGNORED on Windows */
     cl_git_fail_with(git_status_file(&status, repo, "NEW_FILE"), GIT_ENOTFOUND);
 }
+
+void test_status_worktree__simple_delete(void)
+{
+    git_repository *repo = cl_git_sandbox_init("renames");
+	git_status_options opts = GIT_STATUS_OPTIONS_INIT;
+	int count;
+
+	opts.flags = GIT_STATUS_OPT_INCLUDE_UNTRACKED |
+		GIT_STATUS_OPT_DISABLE_PATHSPEC_MATCH |
+		GIT_STATUS_OPT_EXCLUDE_SUBMODULES |
+		GIT_STATUS_OPT_RECURSE_UNTRACKED_DIRS;
+
+	count = 0;
+	cl_git_pass(
+		git_status_foreach_ext(repo, &opts, cb_status__count, &count) );
+	cl_assert_equal_i(0, count);
+
+	cl_must_pass(p_unlink("renames/untimely.txt"));
+
+	count = 0;
+	cl_git_pass(
+		git_status_foreach_ext(repo, &opts, cb_status__count, &count) );
+	cl_assert_equal_i(1, count);
+}
+
+void test_status_worktree__simple_delete_indexed(void)
+{
+	git_repository *repo = cl_git_sandbox_init("renames");
+	git_status_options opts = GIT_STATUS_OPTIONS_INIT;
+	git_status_list *status;
+
+	opts.flags = GIT_STATUS_OPT_INCLUDE_UNTRACKED |
+		GIT_STATUS_OPT_DISABLE_PATHSPEC_MATCH |
+		GIT_STATUS_OPT_EXCLUDE_SUBMODULES |
+		GIT_STATUS_OPT_RECURSE_UNTRACKED_DIRS;
+
+	cl_git_pass(git_status_list_new(&status, repo, &opts));
+	cl_assert_equal_sz(0, git_status_list_entrycount(status));
+	git_status_list_free(status);
+
+	cl_must_pass(p_unlink("renames/untimely.txt"));
+
+	cl_git_pass(git_status_list_new(&status, repo, &opts));
+	cl_assert_equal_sz(1, git_status_list_entrycount(status));
+	cl_assert_equal_i(
+		GIT_STATUS_WT_DELETED, git_status_byindex(status, 0)->status);
+	git_status_list_free(status);
+}

tests-clar/submodule/status.c

@@ -376,9 +376,12 @@ void test_submodule_status__iterator(void)
 
 	git_iterator_free(iter);
 
-	opts.flags = GIT_STATUS_OPT_INCLUDE_UNTRACKED | GIT_STATUS_OPT_INCLUDE_UNMODIFIED | GIT_STATUS_OPT_RECURSE_UNTRACKED_DIRS;
+	opts.flags = GIT_STATUS_OPT_INCLUDE_UNTRACKED |
+		GIT_STATUS_OPT_INCLUDE_UNMODIFIED |
+		GIT_STATUS_OPT_RECURSE_UNTRACKED_DIRS;
 
-	cl_git_pass(git_status_foreach_ext(g_repo, &opts, confirm_submodule_status, &exp));
+	cl_git_pass(git_status_foreach_ext(
+		g_repo, &opts, confirm_submodule_status, &exp));
 }
 
 void test_submodule_status__untracked_dirs_containing_ignored_files(void)