Merge pull request #2356 from libgit2/rb/restore-search-paths

Better global search path sandboxing in tests

tests/clar_libgit2.c

@@ -518,3 +518,16 @@ void cl_fake_home(void)
 		GIT_OPT_SET_SEARCH_PATH, GIT_CONFIG_LEVEL_GLOBAL, path.ptr));
 	git_buf_free(&path);
 }
+
+void cl_sandbox_set_search_path_defaults(void)
+{
+	const char *sandbox_path = clar_sandbox_path();
+
+	git_libgit2_opts(
+		GIT_OPT_SET_SEARCH_PATH, GIT_CONFIG_LEVEL_GLOBAL, sandbox_path);
+	git_libgit2_opts(
+		GIT_OPT_SET_SEARCH_PATH, GIT_CONFIG_LEVEL_XDG, sandbox_path);
+	git_libgit2_opts(
+		GIT_OPT_SET_SEARCH_PATH, GIT_CONFIG_LEVEL_SYSTEM, sandbox_path);
+}
+

tests/clar_libgit2.h

@@ -139,4 +139,6 @@ void cl_repo_set_string(git_repository *repo, const char *cfg, const char *value
 void cl_fake_home(void);
 void cl_fake_home_cleanup(void *);
 
+void cl_sandbox_set_search_path_defaults(void);
+
 #endif

tests/config/global.c

@@ -2,25 +2,10 @@
 #include "buffer.h"
 #include "fileops.h"
 
-static git_config_level_t setting[3] = {
-	GIT_CONFIG_LEVEL_GLOBAL,
-	GIT_CONFIG_LEVEL_XDG,
-	GIT_CONFIG_LEVEL_SYSTEM
-};
-static char *restore[3];
-
 void test_config_global__initialize(void)
 {
-	int i;
 	git_buf path = GIT_BUF_INIT;
 
-	/* snapshot old settings to restore later */
-	for (i = 0; i < 3; ++i) {
-		cl_git_pass(
-			git_libgit2_opts(GIT_OPT_GET_SEARCH_PATH, setting[i], &path));
-		restore[i] = git_buf_detach(&path);
-	}
-
 	cl_git_pass(git_futils_mkdir_r("home", NULL, 0777));
 	cl_git_pass(git_path_prettify(&path, "home", NULL));
 	cl_git_pass(git_libgit2_opts(
@@ -41,18 +26,7 @@ void test_config_global__initialize(void)
 
 void test_config_global__cleanup(void)
 {
-	int i;
-
-	for (i = 0; i < 3; ++i) {
-		cl_git_pass(
-			git_libgit2_opts(GIT_OPT_SET_SEARCH_PATH, setting[i], restore[i]));
-		git__free(restore[i]);
-		restore[i] = NULL;
-	}
-
-	cl_git_pass(git_futils_rmdir_r("home", NULL, GIT_RMDIR_REMOVE_FILES));
-	cl_git_pass(git_futils_rmdir_r("xdg", NULL, GIT_RMDIR_REMOVE_FILES));
-	cl_git_pass(git_futils_rmdir_r("etc", NULL, GIT_RMDIR_REMOVE_FILES));
+	cl_sandbox_set_search_path_defaults();
 }
 
 void test_config_global__open_global(void)

tests/config/include.c

@@ -47,6 +47,8 @@ void test_config_include__homedir(void)
 	cl_assert_equal_s(str, "huzzah");
 
 	git_config_free(cfg);
+
+	cl_sandbox_set_search_path_defaults();
 }
 
 void test_config_include__refresh(void)

tests/core/env.c

@@ -40,12 +40,12 @@ void test_core_env__initialize(void)
 	}
 }
 
-static void reset_global_search_path(void)
+static void set_global_search_path_from_env(void)
 {
 	cl_git_pass(git_sysdir_set(GIT_SYSDIR_GLOBAL, NULL));
 }
 
-static void reset_system_search_path(void)
+static void set_system_search_path_from_env(void)
 {
 	cl_git_pass(git_sysdir_set(GIT_SYSDIR_SYSTEM, NULL));
 }
@@ -69,9 +69,7 @@ void test_core_env__cleanup(void)
 			(void)p_rmdir(*val);
 	}
 
-	/* reset search paths to default */
-	reset_global_search_path();
-	reset_system_search_path();
+	cl_sandbox_set_search_path_defaults();
 }
 
 static void setenv_and_check(const char *name, const char *value)
@@ -124,12 +122,12 @@ void test_core_env__0(void)
 			GIT_ENOTFOUND, git_sysdir_find_global_file(&found, testfile));
 
 		setenv_and_check("HOME", path.ptr);
-		reset_global_search_path();
+		set_global_search_path_from_env();
 
 		cl_git_pass(git_sysdir_find_global_file(&found, testfile));
 
 		cl_setenv("HOME", env_save[0]);
-		reset_global_search_path();
+		set_global_search_path_from_env();
 
 		cl_assert_equal_i(
 			GIT_ENOTFOUND, git_sysdir_find_global_file(&found, testfile));
@@ -138,7 +136,7 @@ void test_core_env__0(void)
 		setenv_and_check("HOMEDRIVE", NULL);
 		setenv_and_check("HOMEPATH", NULL);
 		setenv_and_check("USERPROFILE", path.ptr);
-		reset_global_search_path();
+		set_global_search_path_from_env();
 
 		cl_git_pass(git_sysdir_find_global_file(&found, testfile));
 
@@ -148,7 +146,7 @@ void test_core_env__0(void)
 
 			if (root >= 0) {
 				setenv_and_check("USERPROFILE", NULL);
-				reset_global_search_path();
+				set_global_search_path_from_env();
 
 				cl_assert_equal_i(
 					GIT_ENOTFOUND, git_sysdir_find_global_file(&found, testfile));
@@ -158,7 +156,7 @@ void test_core_env__0(void)
 				setenv_and_check("HOMEDRIVE", path.ptr);
 				path.ptr[root] = old;
 				setenv_and_check("HOMEPATH", &path.ptr[root]);
-				reset_global_search_path();
+				set_global_search_path_from_env();
 
 				cl_git_pass(git_sysdir_find_global_file(&found, testfile));
 			}
@@ -185,7 +183,7 @@ void test_core_env__1(void)
 	cl_git_pass(cl_setenv("HOMEPATH", "doesnotexist"));
 	cl_git_pass(cl_setenv("USERPROFILE", "doesnotexist"));
 #endif
-	reset_global_search_path();
+	set_global_search_path_from_env();
 
 	cl_assert_equal_i(
 		GIT_ENOTFOUND, git_sysdir_find_global_file(&path, "nonexistentfile"));
@@ -195,8 +193,8 @@ void test_core_env__1(void)
 	cl_git_pass(cl_setenv("HOMEPATH", NULL));
 	cl_git_pass(cl_setenv("USERPROFILE", NULL));
 #endif
-	reset_global_search_path();
-	reset_system_search_path();
+	set_global_search_path_from_env();
+	set_system_search_path_from_env();
 
 	cl_assert_equal_i(
 		GIT_ENOTFOUND, git_sysdir_find_global_file(&path, "nonexistentfile"));
@@ -206,7 +204,7 @@ void test_core_env__1(void)
 
 #ifdef GIT_WIN32
 	cl_git_pass(cl_setenv("PROGRAMFILES", NULL));
-	reset_system_search_path();
+	set_system_search_path_from_env();
 
 	cl_assert_equal_i(
 		GIT_ENOTFOUND, git_sysdir_find_system_file(&path, "nonexistentfile"));

tests/main.c

@@ -6,16 +6,12 @@ int __cdecl main(int argc, char *argv[])
 int main(int argc, char *argv[])
 #endif
 {
-	const char *sandbox_path;
 	int res;
 
 	clar_test_init(argc, argv);
 
 	git_threads_init();
-
-	sandbox_path = clar_sandbox_path();
-	git_libgit2_opts(GIT_OPT_SET_SEARCH_PATH, GIT_CONFIG_LEVEL_GLOBAL, sandbox_path);
-	git_libgit2_opts(GIT_OPT_SET_SEARCH_PATH, GIT_CONFIG_LEVEL_XDG, sandbox_path);
+	cl_sandbox_set_search_path_defaults();
 
 	/* Run the test suite */
 	res = clar_test_run();

tests/repo/config.c

@@ -8,7 +8,8 @@ static git_buf path = GIT_BUF_INIT;
 void test_repo_config__initialize(void)
 {
 	cl_fixture_sandbox("empty_standard_repo");
-	cl_git_pass(cl_rename("empty_standard_repo/.gitted", "empty_standard_repo/.git"));
+	cl_git_pass(cl_rename(
+		"empty_standard_repo/.gitted", "empty_standard_repo/.git"));
 
 	git_buf_clear(&path);
 
@@ -18,15 +19,19 @@ void test_repo_config__initialize(void)
 
 void test_repo_config__cleanup(void)
 {
-	cl_git_pass(git_path_prettify(&path, "alternate", NULL));
-	cl_git_pass(git_futils_rmdir_r(path.ptr, NULL, GIT_RMDIR_REMOVE_FILES));
+	cl_sandbox_set_search_path_defaults();
+
 	git_buf_free(&path);
+
+	cl_git_pass(
+		git_futils_rmdir_r("alternate", NULL, GIT_RMDIR_REMOVE_FILES));
 	cl_assert(!git_path_isdir("alternate"));
 
 	cl_fixture_cleanup("empty_standard_repo");
+
 }
 
-void test_repo_config__open_missing_global(void)
+void test_repo_config__can_open_global_when_there_is_no_file(void)
 {
 	git_repository *repo;
 	git_config *config, *global;
@@ -40,23 +45,23 @@ void test_repo_config__open_missing_global(void)
 
 	cl_git_pass(git_repository_open(&repo, "empty_standard_repo"));
 	cl_git_pass(git_repository_config(&config, repo));
-	cl_git_pass(git_config_open_level(&global, config, GIT_CONFIG_LEVEL_GLOBAL));
+	cl_git_pass(git_config_open_level(
+		&global, config, GIT_CONFIG_LEVEL_GLOBAL));
 
 	cl_git_pass(git_config_set_string(global, "test.set", "42"));
 
 	git_config_free(global);
 	git_config_free(config);
 	git_repository_free(repo);
-
-	git_sysdir_global_shutdown();
 }
 
-void test_repo_config__open_missing_global_with_separators(void)
+void test_repo_config__can_open_missing_global_with_separators(void)
 {
 	git_repository *repo;
 	git_config *config, *global;
 
-	cl_git_pass(git_buf_printf(&path, "%c%s", GIT_PATH_LIST_SEPARATOR, "dummy"));
+	cl_git_pass(git_buf_printf(
+		&path, "%c%s", GIT_PATH_LIST_SEPARATOR, "dummy"));
 
 	cl_git_pass(git_libgit2_opts(
 		GIT_OPT_SET_SEARCH_PATH, GIT_CONFIG_LEVEL_GLOBAL, path.ptr));
@@ -69,20 +74,19 @@ void test_repo_config__open_missing_global_with_separators(void)
 
 	cl_git_pass(git_repository_open(&repo, "empty_standard_repo"));
 	cl_git_pass(git_repository_config(&config, repo));
-	cl_git_pass(git_config_open_level(&global, config, GIT_CONFIG_LEVEL_GLOBAL));
+	cl_git_pass(git_config_open_level(
+		&global, config, GIT_CONFIG_LEVEL_GLOBAL));
 
 	cl_git_pass(git_config_set_string(global, "test.set", "42"));
 
 	git_config_free(global);
 	git_config_free(config);
 	git_repository_free(repo);
-
-	git_sysdir_global_shutdown();
 }
 
 #include "repository.h"
 
-void test_repo_config__read_no_configs(void)
+void test_repo_config__read_with_no_configs_at_all(void)
 {
 	git_repository *repo;
 	int val;
@@ -106,9 +110,9 @@ void test_repo_config__read_no_configs(void)
 	cl_assert_equal_i(GIT_ABBREV_DEFAULT, val);
 	git_repository_free(repo);
 
-	git_sysdir_global_shutdown();
+	/* with no local config, just system */
 
-	/* with just system */
+	cl_sandbox_set_search_path_defaults();
 
 	cl_must_pass(p_mkdir("alternate/1", 0777));
 	cl_git_pass(git_buf_joinpath(&path, path.ptr, "1"));
@@ -123,7 +127,7 @@ void test_repo_config__read_no_configs(void)
 	cl_assert_equal_i(10, val);
 	git_repository_free(repo);
 
-	/* with xdg + system */
+	/* with just xdg + system */
 
 	cl_must_pass(p_mkdir("alternate/2", 0777));
 	path.ptr[path.size - 1] = '2';
@@ -204,6 +208,4 @@ void test_repo_config__read_no_configs(void)
 
 	cl_assert(!git_path_exists("empty_standard_repo/.git/config"));
 	cl_assert(!git_path_exists("alternate/3/.gitconfig"));
-
-	git_sysdir_global_shutdown();
 }

tests/repo/open.c

@@ -298,7 +298,8 @@ void test_repo_open__no_config(void)
 	git_config *config;
 
 	cl_fixture_sandbox("empty_standard_repo");
-	cl_git_pass(cl_rename("empty_standard_repo/.gitted", "empty_standard_repo/.git"));
+	cl_git_pass(cl_rename(
+		"empty_standard_repo/.gitted", "empty_standard_repo/.git"));
 
 	/* remove local config */
 	cl_git_pass(git_futils_rmdir_r(
@@ -325,7 +326,7 @@ void test_repo_open__no_config(void)
 	git_repository_free(repo);
 	cl_fixture_cleanup("empty_standard_repo");
 
-	git_sysdir_global_shutdown();
+	cl_sandbox_set_search_path_defaults();
 }
 
 void test_repo_open__force_bare(void)