Commit 840fb4fc4346580a6398856b3fa2f1226b3a365d

Russell Belfer 2013-10-03T14:42:37

Update repo init with fewer platform assumptions The repo init code was assuming Windows == no filemode, and Mac or Windows == no case sensitivity. Those assumptions are not consistently true depending on the mounted file system. This is a first step to removing those assumptions. It focuses on the repo init code and the tests of that code. There are still many other tests that are broken when those assumptions don't hold true, but this clears up one area of the code. Also, this moves the core.precomposeunicode logic to be closer to the current logic in core Git where it will be set to true on any filesystem where composed unicode is decomposed when read back.

diff --git a/src/repository.c b/src/repository.c
index 51cc76d..f609e57 100644
--- a/src/repository.c
+++ b/src/repository.c
@@ -895,18 +895,20 @@ static bool are_symlinks_supported(const char *wd_path)
 static const char *nfc_file = "\xC3\x85\x73\x74\x72\xC3\xB6\x6D.XXXXXX";
 static const char *nfd_file = "\x41\xCC\x8A\x73\x74\x72\x6F\xCC\x88\x6D.XXXXXX";
 
-/* On Mac, HDFS always stores files using decomposed unicode, but when
- * writing to VFAT or SAMBA file systems, filenames may be kept as
- * precomposed unicode, but will be converted to decomposed form when
- * reading the directory entries.  This can cause file name mismatches.
- * The solution is to convert directory entries to precomposed form if we
- * cannot look up the file from the decomposed path.
+/* Check if the platform is decomposing unicode data for us.  We will
+ * emulate core Git and prefer to use precomposed unicode data internally
+ * on these platforms, composing the decomposed unicode on the fly.
+ *
+ * This mainly happens on the Mac where HDFS stores filenames as
+ * decomposed unicode.  Even on VFAT and SAMBA file systems, the Mac will
+ * return decomposed unicode from readdir() even when the actual
+ * filesystem is storing precomposed unicode.
  */
-static bool should_precompose_unicode_paths(const char *wd_path)
+static bool does_fs_decompose_unicode_paths(const char *wd_path)
 {
 	git_buf path = GIT_BUF_INIT;
 	int fd;
-	bool need_precompose = false;
+	bool found_decomposed = false;
 	char tmp[6];
 
 	/* Create a file using a precomposed path and then try to find it
@@ -915,7 +917,7 @@ static bool should_precompose_unicode_paths(const char *wd_path)
 	 */
 	if (git_buf_joinpath(&path, wd_path, nfc_file) < 0 ||
 		(fd = p_mkstemp(path.ptr)) < 0)
-		goto fail;
+		goto done;
 	p_close(fd);
 
 	/* record trailing digits generated by mkstemp */
@@ -923,21 +925,21 @@ static bool should_precompose_unicode_paths(const char *wd_path)
 
 	/* try to look up as NFD path */
 	if (git_buf_joinpath(&path, wd_path, nfd_file) < 0)
-		goto fail;
+		goto done;
 	memcpy(path.ptr + path.size - sizeof(tmp), tmp, sizeof(tmp));
 
-	need_precompose = !git_path_exists(path.ptr);
+	found_decomposed = git_path_exists(path.ptr);
 
-	/* remove temporary file */
+	/* remove temporary file (using original precomposed path) */
 	if (git_buf_joinpath(&path, wd_path, nfc_file) < 0)
-		goto fail;
+		goto done;
 	memcpy(path.ptr + path.size - sizeof(tmp), tmp, sizeof(tmp));
 
 	(void)p_unlink(path.ptr);
 
-fail:
+done:
 	git_buf_free(&path);
-	return need_precompose;
+	return found_decomposed;
 }
 
 #endif
@@ -1001,7 +1003,7 @@ static int repo_init_config(
 #ifdef GIT_USE_ICONV
 	SET_REPO_CONFIG(
 		bool, "core.precomposeunicode",
-		should_precompose_unicode_paths(is_bare ? repo_dir : work_dir));
+		does_fs_decompose_unicode_paths(is_bare ? repo_dir : work_dir));
 #endif
 
 	if (!are_symlinks_supported(is_bare ? repo_dir : work_dir))
diff --git a/tests-clar/repo/init.c b/tests-clar/repo/init.c
index 6f3c841..061e444 100644
--- a/tests-clar/repo/init.c
+++ b/tests-clar/repo/init.c
@@ -179,41 +179,32 @@ void test_repo_init__additional_templates(void)
 	git_buf_free(&path);
 }
 
-static void assert_config_entry_on_init_bytype(const char *config_key, int expected_value, bool is_bare)
+static void assert_config_entry_on_init_bytype(
+	const char *config_key, int expected_value, bool is_bare)
 {
 	git_config *config;
-	int current_value;
-	git_buf repo_path = GIT_BUF_INIT;
+	int error, current_value;
+	const char *repo_path = is_bare ?
+		"config_entry/test.bare.git" : "config_entry/test.non.bare.git";
 
 	cl_set_cleanup(&cleanup_repository, "config_entry");
 
-	cl_git_pass(git_buf_puts(&repo_path, "config_entry/test."));
-
-	if (!is_bare)
-		cl_git_pass(git_buf_puts(&repo_path, "non."));
-
-	cl_git_pass(git_buf_puts(&repo_path, "bare.git"));
-
-	cl_git_pass(git_repository_init(&_repo, git_buf_cstr(&repo_path), is_bare));
-
-	git_buf_free(&repo_path);
+	cl_git_pass(git_repository_init(&_repo, repo_path, is_bare));
 
-	git_repository_config(&config, _repo);
+	cl_git_pass(git_repository_config(&config, _repo));
+	error = git_config_get_bool(&current_value, config, config_key);
+	git_config_free(config);
 
 	if (expected_value >= 0) {
-		cl_git_pass(git_config_get_bool(&current_value, config, config_key));
-
+		cl_assert_equal_i(0, error);
 		cl_assert_equal_i(expected_value, current_value);
 	} else {
-		int error = git_config_get_bool(&current_value, config, config_key);
-
 		cl_assert_equal_i(expected_value, error);
 	}
-
-	git_config_free(config);
 }
 
-static void assert_config_entry_on_init(const char *config_key, int expected_value)
+static void assert_config_entry_on_init(
+	const char *config_key, int expected_value)
 {
 	assert_config_entry_on_init_bytype(config_key, expected_value, true);
 	git_repository_free(_repo);
@@ -221,31 +212,47 @@ static void assert_config_entry_on_init(const char *config_key, int expected_val
 	assert_config_entry_on_init_bytype(config_key, expected_value, false);
 }
 
-void test_repo_init__detect_filemode(void)
+static int expect_filemode_support(void)
 {
-#ifdef GIT_WIN32
-	assert_config_entry_on_init("core.filemode", false);
-#else
-	assert_config_entry_on_init("core.filemode", true);
-#endif
+	struct stat st;
+
+	cl_git_write2file("testmode", "whatever\n", 0, O_CREAT | O_WRONLY, 0767);
+	cl_must_pass(p_stat("testmode", &st));
+	cl_must_pass(p_unlink("testmode"));
+
+	return (st.st_mode & 0111) == 0101;
 }
 
-#define CASE_INSENSITIVE_FILESYSTEM (defined GIT_WIN32 || defined __APPLE__)
+void test_repo_init__detect_filemode(void)
+{
+	assert_config_entry_on_init("core.filemode", expect_filemode_support());
+}
 
 void test_repo_init__detect_ignorecase(void)
 {
-#if CASE_INSENSITIVE_FILESYSTEM
-	assert_config_entry_on_init("core.ignorecase", true);
-#else
-	assert_config_entry_on_init("core.ignorecase", GIT_ENOTFOUND);
-#endif
+	struct stat st;
+	bool found_without_match;
+
+	cl_git_write2file("testCAPS", "whatever\n", 0, O_CREAT | O_WRONLY, 0666);
+	found_without_match = (p_stat("Testcaps", &st) == 0);
+	cl_must_pass(p_unlink("testCAPS"));
+
+	assert_config_entry_on_init(
+		"core.ignorecase", found_without_match ? true : GIT_ENOTFOUND);
 }
 
 void test_repo_init__detect_precompose_unicode_required(void)
 {
-#ifdef __APPLE__
-	/* hard to test "true" case without SAMBA or VFAT file system available */
-	assert_config_entry_on_init("core.precomposeunicode", false);
+	char *composed = "ḱṷṓn", *decomposed = "ḱṷṓn";
+	struct stat st;
+	bool found_with_nfd;
+
+	cl_git_write2file(composed, "whatever\n", 0, O_CREAT | O_WRONLY, 0666);
+	found_with_nfd = (p_stat(decomposed, &st) == 0);
+	cl_must_pass(p_unlink(composed));
+
+#ifdef GIT_USE_ICONV
+	assert_config_entry_on_init("core.precomposeunicode", found_with_nfd);
 #else
 	assert_config_entry_on_init("core.precomposeunicode", GIT_ENOTFOUND);
 #endif
@@ -280,13 +287,7 @@ void test_repo_init__reinit_doesnot_overwrite_ignorecase(void)
 
 void test_repo_init__reinit_overwrites_filemode(void)
 {
-	int expected, current_value;
-
-#ifdef GIT_WIN32
-	expected = false;
-#else
-	expected = true;
-#endif
+	int expected = expect_filemode_support(), current_value;
 
 	/* Init a new repo */
 	cl_set_cleanup(&cleanup_repository, "overwrite.git");
@@ -358,7 +359,10 @@ void test_repo_init__extended_1(void)
 
 	cl_git_pass(git_path_lstat(git_repository_path(_repo), &st));
 	cl_assert(S_ISDIR(st.st_mode));
-	cl_assert((S_ISGID & st.st_mode) == S_ISGID);
+	if (expect_filemode_support())
+		cl_assert((S_ISGID & st.st_mode) == S_ISGID);
+	else
+		cl_assert((S_ISGID & st.st_mode) == 0);
 
 	cl_git_pass(git_reference_lookup(&ref, _repo, "HEAD"));
 	cl_assert(git_reference_type(ref) == GIT_REF_SYMBOLIC);