Commit 0d422ec9c282added2aaa2a6bc0ea660e33f8f3a

Russell Belfer 2012-10-19T15:40:43

Fix env variable tests with new Win32 path rules The new Win32 global path search was not working with the environment variable tests. But when I fixed the test, the new codes use of getenv() was causing more failures (presumably because of caching on Windows ???). This fixes the global file lookup to always go directly to the Win32 API in a predictable way.

diff --git a/src/fileops.c b/src/fileops.c
index 3f9e987..6342b16 100644
--- a/src/fileops.c
+++ b/src/fileops.c
@@ -404,44 +404,39 @@ int git_futils_find_system_file(git_buf *path, const char *filename)
 
 int git_futils_find_global_file(git_buf *path, const char *filename)
 {
-	const char *home = getenv("HOME");
-
 #ifdef GIT_WIN32
 	struct win32_path root;
-
-	if (home != NULL) {
-		if (git_buf_joinpath(path, home, filename) < 0)
-			return -1;
-
-		if (git_path_exists(path->ptr)) {
+	static const wchar_t *tmpls[4] = {
+		L"%HOME%\\",
+		L"%HOMEDRIVE%%HOMEPATH%\\",
+		L"%USERPROFILE%\\",
+		NULL,
+	};
+	const wchar_t **tmpl;
+
+	for (tmpl = tmpls; *tmpl != NULL; tmpl++) {
+		/* try to expand environment variable, skipping if not set */
+		if (win32_expand_path(&root, *tmpl) != 0 || root.path[0] == L'%')
+			continue;
+
+		/* try to look up file under path */
+		if (!win32_find_file(path, &root, filename))
 			return 0;
-		}
-	}
 
-	if (getenv("HOMEPATH") != NULL) {
-		if (win32_expand_path(&root, L"%HOMEDRIVE%%HOMEPATH%\\") < 0 ||
-			root.path[0] == L'%') /* i.e. no expansion happened */
-		{
-			giterr_set(GITERR_OS, "Cannot locate the user's profile directory");
-			return GIT_ENOTFOUND;
-		}
-	} else {
-		if (win32_expand_path(&root, L"%USERPROFILE%\\") < 0 ||
-			root.path[0] == L'%') /* i.e. no expansion happened */
-		{
-			giterr_set(GITERR_OS, "Cannot locate the user's profile directory");
-			return GIT_ENOTFOUND;
-		}
+		/* No error if file not found under %HOME%, b/c we don't trust it,
+		 * but do error if another var is set and yet file is not found.
+		 */
+		if (tmpl != tmpls)
+			break;
 	}
 
-	if (win32_find_file(path, &root, filename) < 0) {
-		giterr_set(GITERR_OS, "The global file '%s' doesn't exist", filename);
-		git_buf_clear(path);
-		return GIT_ENOTFOUND;
-	}
+	giterr_set(GITERR_OS, "The global file '%s' doesn't exist", filename);
+	git_buf_clear(path);
 
-	return 0;
+	return GIT_ENOTFOUND;
 #else
+	const char *home = getenv("HOME");
+
 	if (home == NULL) {
 		giterr_set(GITERR_OS, "Global file lookup failed. "
 			"Cannot locate the user's home directory");
diff --git a/tests-clar/core/env.c b/tests-clar/core/env.c
index 0e0ddf3..288222d 100644
--- a/tests-clar/core/env.c
+++ b/tests-clar/core/env.c
@@ -3,31 +3,43 @@
 #include "path.h"
 
 #ifdef GIT_WIN32
-static char *env_userprofile = NULL;
-static char *env_programfiles = NULL;
+#define NUM_VARS 5
+static const char *env_vars[NUM_VARS] = {
+	"HOME", "HOMEDRIVE", "HOMEPATH", "USERPROFILE", "PROGRAMFILES"
+};
 #else
-static char *env_home = NULL;
+#define NUM_VARS 1
+static const char *env_vars[NUM_VARS] = { "HOME" };
 #endif
 
+static char *env_save[NUM_VARS];
+
 void test_core_env__initialize(void)
 {
+	int i;
+	for (i = 0; i < NUM_VARS; ++i)
+		env_save[i] = cl_getenv(env_vars[i]);
+}
+
+void test_core_env__cleanup(void)
+{
+	int i;
+	for (i = 0; i < NUM_VARS; ++i) {
+		cl_setenv(env_vars[i], env_save[i]);
 #ifdef GIT_WIN32
-	env_userprofile = cl_getenv("USERPROFILE");
-	env_programfiles = cl_getenv("PROGRAMFILES");
-#else
-	env_home = cl_getenv("HOME");
+		git__free(env_save[i]);
 #endif
+	}
 }
 
-void test_core_env__cleanup(void)
+static void setenv_and_check(const char *name, const char *value)
 {
+	char *check;
+	cl_git_pass(cl_setenv(name, value));
+	check = cl_getenv(name);
+	cl_assert_equal_s(value, check);
 #ifdef GIT_WIN32
-	cl_setenv("USERPROFILE", env_userprofile);
-	git__free(env_userprofile);
-	cl_setenv("PROGRAMFILES", env_programfiles);
-	git__free(env_programfiles);
-#else
-	cl_setenv("HOME", env_home);
+	git__free(check);
 #endif
 }
 
@@ -45,38 +57,67 @@ void test_core_env__0(void)
 		NULL
 	};
 	git_buf path = GIT_BUF_INIT, found = GIT_BUF_INIT;
+	char testfile[16], tidx = '0';
 	char **val;
-	char *check;
+
+	memset(testfile, 0, sizeof(testfile));
+	memcpy(testfile, "testfile", 8);
+	cl_assert_equal_s("testfile", testfile);
 
 	for (val = home_values; *val != NULL; val++) {
 
-		if (p_mkdir(*val, 0777) == 0) {
-			/* if we can't make the directory, let's just assume
-			 * we are on a filesystem that doesn't support the
-			 * characters in question and skip this test...
-			 */
-			cl_git_pass(git_path_prettify(&path, *val, NULL));
+		/* if we can't make the directory, let's just assume
+		 * we are on a filesystem that doesn't support the
+		 * characters in question and skip this test...
+		 */
+		if (p_mkdir(*val, 0777) != 0)
+			continue;
+
+		cl_git_pass(git_path_prettify(&path, *val, NULL));
+
+		/* vary testfile name in each directory so accidentally leaving
+		 * an environment variable set from a previous iteration won't
+		 * accidentally make this test pass...
+		 */
+		testfile[8] = tidx++;
+		cl_git_pass(git_buf_joinpath(&path, path.ptr, testfile));
+		cl_git_mkfile(path.ptr, "find me");
+		git_buf_rtruncate_at_char(&path, '/');
+
+		cl_git_fail(git_futils_find_global_file(&found, testfile));
+
+		setenv_and_check("HOME", path.ptr);
+		cl_git_pass(git_futils_find_global_file(&found, testfile));
+
+		cl_setenv("HOME", env_save[0]);
+		cl_git_fail(git_futils_find_global_file(&found, testfile));
 
 #ifdef GIT_WIN32
-			cl_git_pass(cl_setenv("USERPROFILE", path.ptr));
+		setenv_and_check("HOMEDRIVE", NULL);
+		setenv_and_check("HOMEPATH", NULL);
+		setenv_and_check("USERPROFILE", path.ptr);
 
-			/* do a quick check that it was set correctly */
-			check = cl_getenv("USERPROFILE");
-			cl_assert_equal_s(path.ptr, check);
-			git__free(check);
-#else
-			cl_git_pass(cl_setenv("HOME", path.ptr));
+		cl_git_pass(git_futils_find_global_file(&found, testfile));
 
-			/* do a quick check that it was set correctly */
-			check = cl_getenv("HOME");
-			cl_assert_equal_s(path.ptr, check);
-#endif
+		{
+			int root = git_path_root(path.ptr);
+			char old;
 
-			cl_git_pass(git_buf_puts(&path, "/testfile"));
-			cl_git_mkfile(path.ptr, "find me");
+			if (root >= 0) {
+				setenv_and_check("USERPROFILE", NULL);
 
-			cl_git_pass(git_futils_find_global_file(&found, "testfile"));
+				cl_git_fail(git_futils_find_global_file(&found, testfile));
+
+				old = path.ptr[root];
+				path.ptr[root] = '\0';
+				setenv_and_check("HOMEDRIVE", path.ptr);
+				path.ptr[root] = old;
+				setenv_and_check("HOMEPATH", &path.ptr[root]);
+
+				cl_git_pass(git_futils_find_global_file(&found, testfile));
+			}
 		}
+#endif
 	}
 
 	git_buf_free(&path);
@@ -89,21 +130,22 @@ void test_core_env__1(void)
 
 	cl_must_fail(git_futils_find_global_file(&path, "nonexistentfile"));
 
+	cl_git_pass(cl_setenv("HOME", "doesnotexist"));
 #ifdef GIT_WIN32
+	cl_git_pass(cl_setenv("HOMEPATH", "doesnotexist"));
 	cl_git_pass(cl_setenv("USERPROFILE", "doesnotexist"));
-#else
-	cl_git_pass(cl_setenv("HOME", "doesnotexist"));
 #endif
 
 	cl_must_fail(git_futils_find_global_file(&path, "nonexistentfile"));
 
+	cl_git_pass(cl_setenv("HOME", NULL));
 #ifdef GIT_WIN32
+	cl_git_pass(cl_setenv("HOMEPATH", NULL));
 	cl_git_pass(cl_setenv("USERPROFILE", NULL));
-#else
-	cl_git_pass(cl_setenv("HOME", NULL));
 #endif
 
 	cl_must_fail(git_futils_find_global_file(&path, "nonexistentfile"));
+
 	cl_must_fail(git_futils_find_system_file(&path, "nonexistentfile"));
 
 #ifdef GIT_WIN32