Merge pull request #2297 from libgit2/rb/status-with-precomposed-changes

Improve test coverage of status with different core.precomposeunicode settings

include/git2/status.h

@@ -236,6 +236,11 @@ GIT_EXTERN(int) git_status_foreach(
  * in what order.  See the `git_status_options` structure for details
  * about the additional controls that this makes available.
  *
+ * Note that if a `pathspec` is given in the `git_status_options` to filter
+ * the status, then the results from rename detection (if you enable it) may
+ * not be accurate.  To do rename detection properly, this must be called
+ * with no `pathspec` so that all files can be considered.
+ *
  * @param repo Repository object
  * @param opts Status options structure
  * @param callback The function to call on each file
@@ -251,8 +256,20 @@ GIT_EXTERN(int) git_status_foreach_ext(
 /**
  * Get file status for a single file.
  *
- * This is not quite the same as calling `git_status_foreach_ext()` with
- * the pathspec set to the specified path.
+ * This tries to get status for the filename that you give.  If no files
+ * match that name (in either the HEAD, index, or working directory), this
+ * returns GIT_ENOTFOUND.
+ *
+ * If the name matches multiple files (for example, if the `path` names a
+ * directory or if running on a case- insensitive filesystem and yet the
+ * HEAD has two entries that both match the path), then this returns
+ * GIT_EAMBIGUOUS because it cannot give correct results.
+ *
+ * This does not do any sort of rename detection.  Renames require a set of
+ * targets and because of the path filtering, there is not enough
+ * information to check renames correctly.  To check file status with rename
+ * detection, there is no choice but to do a full `git_status_list_new` and
+ * scan through looking for the path that you are interested in.
  *
  * @param status_flags Output combination of git_status_t values for file
  * @param repo A repository object
@@ -269,6 +286,11 @@ GIT_EXTERN(int) git_status_file(
 /**
  * Gather file status information and populate the `git_status_list`.
  *
+ * Note that if a `pathspec` is given in the `git_status_options` to filter
+ * the status, then the results from rename detection (if you enable it) may
+ * not be accurate.  To do rename detection properly, this must be called
+ * with no `pathspec` so that all files can be considered.
+ *
  * @param out Pointer to store the status results in
  * @param repo Repository object
  * @param opts Status options structure
@@ -282,6 +304,9 @@ GIT_EXTERN(int) git_status_list_new(
 /**
  * Gets the count of status entries in this list.
  *
+ * If there are no changes in status (at least according the options given
+ * when the status list was created), this can return 0.
+ *
  * @param statuslist Existing status list object
  * @return the number of status entries
  */

tests/status/renames.c

@@ -20,40 +20,36 @@ void test_status_renames__cleanup(void)
 	cl_git_sandbox_cleanup();
 }
 
-static void rename_file(git_repository *repo, const char *oldname, const char *newname)
+static void _rename_helper(
+	git_repository *repo, const char *from, const char *to, const char *extra)
 {
 	git_buf oldpath = GIT_BUF_INIT, newpath = GIT_BUF_INIT;
 
-	git_buf_joinpath(&oldpath, git_repository_workdir(repo), oldname);
-	git_buf_joinpath(&newpath, git_repository_workdir(repo), newname);
+	cl_git_pass(git_buf_joinpath(
+		&oldpath, git_repository_workdir(repo), from));
+	cl_git_pass(git_buf_joinpath(
+		&newpath, git_repository_workdir(repo), to));
 
 	cl_git_pass(p_rename(oldpath.ptr, newpath.ptr));
 
-	git_buf_free(&oldpath);
-	git_buf_free(&newpath);
-}
-
-static void rename_and_edit_file(git_repository *repo, const char *oldname, const char *newname)
-{
-	git_buf oldpath = GIT_BUF_INIT, newpath = GIT_BUF_INIT;
-
-	git_buf_joinpath(&oldpath, git_repository_workdir(repo), oldname);
-	git_buf_joinpath(&newpath, git_repository_workdir(repo), newname);
-
-	cl_git_pass(p_rename(oldpath.ptr, newpath.ptr));
-	cl_git_append2file(newpath.ptr, "Added at the end to keep similarity!");
+	if (extra)
+		cl_git_append2file(newpath.ptr, extra);
 
 	git_buf_free(&oldpath);
 	git_buf_free(&newpath);
 }
 
+#define rename_file(R,O,N) _rename_helper((R), (O), (N), NULL)
+#define rename_and_edit_file(R,O,N) \
+	_rename_helper((R), (O), (N), "Added at the end to keep similarity!")
+
 struct status_entry {
 	git_status_t status;
 	const char *oldname;
 	const char *newname;
 };
 
-static void test_status(
+static void check_status(
 	git_status_list *status_list,
 	struct status_entry *expected_list,
 	size_t expected_len)
@@ -61,9 +57,9 @@ static void test_status(
 	const git_status_entry *actual;
 	const struct status_entry *expected;
 	const char *oldname, *newname;
-	size_t i;
+	size_t i, files_in_status = git_status_list_entrycount(status_list);
 
-	cl_assert_equal_sz(expected_len, git_status_list_entrycount(status_list));
+	cl_assert_equal_sz(expected_len, files_in_status);
 
 	for (i = 0; i < expected_len; i++) {
 		actual = git_status_byindex(status_list, i);
@@ -82,11 +78,13 @@ static void test_status(
 		else
 			cl_assert(expected->oldname == NULL);
 
+		if (actual->status & (GIT_STATUS_INDEX_RENAMED|GIT_STATUS_WT_RENAMED)) {
 			if (newname)
 				cl_assert(git__strcmp(newname, expected->newname) == 0);
 			else
 				cl_assert(expected->newname == NULL);
 		}
+	}
 }
 
 void test_status_renames__head2index_one(void)
@@ -109,7 +107,7 @@ void test_status_renames__head2index_one(void)
 	cl_git_pass(git_index_write(index));
 
 	cl_git_pass(git_status_list_new(&statuslist, g_repo, &opts));
-	test_status(statuslist, expected, 1);
+	check_status(statuslist, expected, 1);
 	git_status_list_free(statuslist);
 
 	git_index_free(index);
@@ -149,7 +147,7 @@ void test_status_renames__head2index_two(void)
 	cl_git_pass(git_index_write(index));
 
 	cl_git_pass(git_status_list_new(&statuslist, g_repo, &opts));
-	test_status(statuslist, expected, 4);
+	check_status(statuslist, expected, 4);
 	git_status_list_free(statuslist);
 
 	git_index_free(index);
@@ -178,7 +176,7 @@ void test_status_renames__head2index_no_rename_from_rewrite(void)
 	cl_git_pass(git_index_write(index));
 
 	cl_git_pass(git_status_list_new(&statuslist, g_repo, &opts));
-	test_status(statuslist, expected, 2);
+	check_status(statuslist, expected, 2);
 	git_status_list_free(statuslist);
 
 	git_index_free(index);
@@ -208,7 +206,7 @@ void test_status_renames__head2index_rename_from_rewrite(void)
 	cl_git_pass(git_index_write(index));
 
 	cl_git_pass(git_status_list_new(&statuslist, g_repo, &opts));
-	test_status(statuslist, expected, 2);
+	check_status(statuslist, expected, 2);
 	git_status_list_free(statuslist);
 
 	git_index_free(index);
@@ -228,7 +226,7 @@ void test_status_renames__index2workdir_one(void)
 	rename_file(g_repo, "ikeepsix.txt", "newname.txt");
 
 	cl_git_pass(git_status_list_new(&statuslist, g_repo, &opts));
-	test_status(statuslist, expected, 1);
+	check_status(statuslist, expected, 1);
 	git_status_list_free(statuslist);
 }
 
@@ -254,7 +252,7 @@ void test_status_renames__index2workdir_two(void)
 	rename_and_edit_file(g_repo, "untimely.txt", "bbb.txt");
 
 	cl_git_pass(git_status_list_new(&statuslist, g_repo, &opts));
-	test_status(statuslist, expected, 4);
+	check_status(statuslist, expected, 4);
 	git_status_list_free(statuslist);
 }
 
@@ -278,7 +276,7 @@ void test_status_renames__index2workdir_rename_from_rewrite(void)
 	rename_file(g_repo, "_temp_.txt", "sixserving.txt");
 
 	cl_git_pass(git_status_list_new(&statuslist, g_repo, &opts));
-	test_status(statuslist, expected, 2);
+	check_status(statuslist, expected, 2);
 	git_status_list_free(statuslist);
 
 	git_index_free(index);
@@ -309,7 +307,7 @@ void test_status_renames__both_one(void)
 	rename_file(g_repo, "newname-index.txt", "newname-workdir.txt");
 
 	cl_git_pass(git_status_list_new(&statuslist, g_repo, &opts));
-	test_status(statuslist, expected, 1);
+	check_status(statuslist, expected, 1);
 	git_status_list_free(statuslist);
 
 	git_index_free(index);
@@ -355,7 +353,7 @@ void test_status_renames__both_two(void)
 	rename_file(g_repo, "untimely-index.txt", "untimely-both.txt");
 
 	cl_git_pass(git_status_list_new(&statuslist, g_repo, &opts));
-	test_status(statuslist, expected, 4);
+	check_status(statuslist, expected, 4);
 	git_status_list_free(statuslist);
 
 	git_index_free(index);
@@ -399,7 +397,7 @@ void test_status_renames__both_rename_from_rewrite(void)
 	rename_file(g_repo, "_temp_.txt", "sixserving.txt");
 
 	cl_git_pass(git_status_list_new(&statuslist, g_repo, &opts));
-	test_status(statuslist, expected, 3);
+	check_status(statuslist, expected, 3);
 	git_status_list_free(statuslist);
 
 	git_index_free(index);
@@ -440,7 +438,7 @@ void test_status_renames__rewrites_only_for_renames(void)
 		"This is enough content for the file to be rewritten.\n");
 
 	cl_git_pass(git_status_list_new(&statuslist, g_repo, &opts));
-	test_status(statuslist, expected, 1);
+	check_status(statuslist, expected, 1);
 	git_status_list_free(statuslist);
 
 	git_index_free(index);
@@ -481,7 +479,7 @@ void test_status_renames__both_casechange_one(void)
 
 	cl_git_pass(git_status_list_new(&statuslist, g_repo, &opts));
 
-	test_status(statuslist, (index_caps & GIT_INDEXCAP_IGNORE_CASE) ?
+	check_status(statuslist, (index_caps & GIT_INDEXCAP_IGNORE_CASE) ?
 		expected_icase : expected_case, 1);
 
 	git_status_list_free(statuslist);
@@ -548,7 +546,7 @@ void test_status_renames__both_casechange_two(void)
 
 	cl_git_pass(git_status_list_new(&statuslist, g_repo, &opts));
 
-	test_status(statuslist, (index_caps & GIT_INDEXCAP_IGNORE_CASE) ?
+	check_status(statuslist, (index_caps & GIT_INDEXCAP_IGNORE_CASE) ?
 		expected_icase : expected_case, 4);
 
 	git_status_list_free(statuslist);
@@ -579,6 +577,135 @@ void test_status_renames__zero_byte_file_does_not_fail(void)
 	cl_git_mkfile("renames/zerobyte.txt", "");
 
 	cl_git_pass(git_status_list_new(&statuslist, g_repo, &opts));
-	test_status(statuslist, expected, 2);
+	check_status(statuslist, expected, 2);
+	git_status_list_free(statuslist);
+}
+
+#ifdef GIT_USE_ICONV
+static char *nfc = "\xC3\x85\x73\x74\x72\xC3\xB6\x6D";
+static char *nfd = "\x41\xCC\x8A\x73\x74\x72\x6F\xCC\x88\x6D";
+#endif
+
+void test_status_renames__precomposed_unicode_rename(void)
+{
+#ifdef GIT_USE_ICONV
+	git_status_list *statuslist;
+	git_status_options opts = GIT_STATUS_OPTIONS_INIT;
+	struct status_entry expected0[] = {
+		{ GIT_STATUS_WT_NEW, nfd, NULL },
+		{ GIT_STATUS_WT_DELETED, "sixserving.txt", NULL },
+	};
+	struct status_entry expected1[] = {
+		{ GIT_STATUS_WT_RENAMED, "sixserving.txt", nfd },
+	};
+	struct status_entry expected2[] = {
+		{ GIT_STATUS_WT_DELETED, "sixserving.txt", NULL },
+		{ GIT_STATUS_WT_NEW, nfc, NULL },
+	};
+	struct status_entry expected3[] = {
+		{ GIT_STATUS_WT_RENAMED, "sixserving.txt", nfc },
+	};
+
+	rename_file(g_repo, "sixserving.txt", nfc);
+
+	opts.flags |= GIT_STATUS_OPT_INCLUDE_UNTRACKED;
+
+	cl_repo_set_bool(g_repo, "core.precomposeunicode", false);
+
+	cl_git_pass(git_status_list_new(&statuslist, g_repo, &opts));
+	check_status(statuslist, expected0, ARRAY_SIZE(expected0));
+	git_status_list_free(statuslist);
+
+	opts.flags |= GIT_STATUS_OPT_RENAMES_INDEX_TO_WORKDIR;
+
+	cl_git_pass(git_status_list_new(&statuslist, g_repo, &opts));
+	check_status(statuslist, expected1, ARRAY_SIZE(expected1));
 	git_status_list_free(statuslist);
+
+	cl_repo_set_bool(g_repo, "core.precomposeunicode", true);
+
+	opts.flags &= ~GIT_STATUS_OPT_RENAMES_INDEX_TO_WORKDIR;
+
+	cl_git_pass(git_status_list_new(&statuslist, g_repo, &opts));
+	check_status(statuslist, expected2, ARRAY_SIZE(expected2));
+	git_status_list_free(statuslist);
+
+	opts.flags |= GIT_STATUS_OPT_RENAMES_INDEX_TO_WORKDIR;
+
+	cl_git_pass(git_status_list_new(&statuslist, g_repo, &opts));
+	check_status(statuslist, expected3, ARRAY_SIZE(expected3));
+	git_status_list_free(statuslist);
+#endif
 }
+
+void test_status_renames__precomposed_unicode_toggle_is_rename(void)
+{
+#ifdef GIT_USE_ICONV
+	git_status_list *statuslist;
+	git_status_options opts = GIT_STATUS_OPTIONS_INIT;
+	struct status_entry expected0[] = {
+		{ GIT_STATUS_INDEX_RENAMED, "ikeepsix.txt", nfd },
+	};
+	struct status_entry expected1[] = {
+		{ GIT_STATUS_WT_RENAMED, nfd, nfc },
+	};
+	struct status_entry expected2[] = {
+		{ GIT_STATUS_INDEX_RENAMED, nfd, nfc },
+	};
+	struct status_entry expected3[] = {
+		{ GIT_STATUS_INDEX_RENAMED | GIT_STATUS_WT_RENAMED, nfd, nfd },
+	};
+
+	cl_repo_set_bool(g_repo, "core.precomposeunicode", false);
+	rename_file(g_repo, "ikeepsix.txt", nfd);
+
+	{
+		git_index *index;
+		cl_git_pass(git_repository_index(&index, g_repo));
+		cl_git_pass(git_index_remove_bypath(index, "ikeepsix.txt"));
+		cl_git_pass(git_index_add_bypath(index, nfd));
+		cl_git_pass(git_index_write(index));
+		git_index_free(index);
+	}
+
+	opts.flags |= GIT_STATUS_OPT_INCLUDE_UNTRACKED |
+		GIT_STATUS_OPT_RENAMES_HEAD_TO_INDEX |
+		GIT_STATUS_OPT_RENAMES_INDEX_TO_WORKDIR;
+
+	cl_git_pass(git_status_list_new(&statuslist, g_repo, &opts));
+	check_status(statuslist, expected0, ARRAY_SIZE(expected0));
+	git_status_list_free(statuslist);
+
+	cl_repo_commit_from_index(NULL, g_repo, NULL, 0, "commit nfd");
+
+	cl_git_pass(git_status_list_new(&statuslist, g_repo, &opts));
+	cl_assert_equal_sz(0, git_status_list_entrycount(statuslist));
+	git_status_list_free(statuslist);
+
+	cl_repo_set_bool(g_repo, "core.precomposeunicode", true);
+
+	cl_git_pass(git_status_list_new(&statuslist, g_repo, &opts));
+	check_status(statuslist, expected1, ARRAY_SIZE(expected1));
+	git_status_list_free(statuslist);
+
+	{
+		git_index *index;
+		cl_git_pass(git_repository_index(&index, g_repo));
+		cl_git_pass(git_index_remove_bypath(index, nfd));
+		cl_git_pass(git_index_add_bypath(index, nfc));
+		cl_git_pass(git_index_write(index));
+		git_index_free(index);
+	}
+
+	cl_git_pass(git_status_list_new(&statuslist, g_repo, &opts));
+	check_status(statuslist, expected2, ARRAY_SIZE(expected2));
+	git_status_list_free(statuslist);
+
+	cl_repo_set_bool(g_repo, "core.precomposeunicode", false);
+
+	cl_git_pass(git_status_list_new(&statuslist, g_repo, &opts));
+	check_status(statuslist, expected3, ARRAY_SIZE(expected3));
+	git_status_list_free(statuslist);
+#endif
+}
+