Merge pull request #1604 from arrbee/config-file-checks

Extend checking for config with no files

src/config.c

@@ -339,17 +339,6 @@ int git_config_foreach_match(
 	return ret;
 }
 
-int git_config_delete_entry(git_config *cfg, const char *name)
-{
-	git_config_backend *file;
-	file_internal *internal;
-
-	internal = git_vector_get(&cfg->files, 0);
-	file = internal->file;
-
-	return file->del(file, name);
-}
-
 /**************
  * Setters
  **************/
@@ -361,6 +350,19 @@ static int config_error_nofiles(const char *name)
 	return GIT_ENOTFOUND;
 }
 
+int git_config_delete_entry(git_config *cfg, const char *name)
+{
+	git_config_backend *file;
+	file_internal *internal;
+
+	internal = git_vector_get(&cfg->files, 0);
+	if (!internal || !internal->file)
+		return config_error_nofiles(name);
+	file = internal->file;
+
+	return file->del(file, name);
+}
+
 int git_config_set_int64(git_config *cfg, const char *name, int64_t value)
 {
 	char str_value[32]; /* All numbers should fit in here */
@@ -390,7 +392,7 @@ int git_config_set_string(git_config *cfg, const char *name, const char *value)
 	}
 
 	internal = git_vector_get(&cfg->files, 0);
-	if (!internal)
+	if (!internal || !internal->file)
 		return config_error_nofiles(name);
 	file = internal->file;
 
@@ -465,10 +467,13 @@ static int get_string(const char **out, const git_config *cfg, const char *name)
 {
 	file_internal *internal;
 	unsigned int i;
+	int res;
 
 	git_vector_foreach(&cfg->files, i, internal) {
-		int res = get_string_at_file(out, internal->file, name);
+		if (!internal || !internal->file)
+			continue;
 
+		res = get_string_at_file(out, internal->file, name);
 		if (res != GIT_ENOTFOUND)
 			return res;
 	}
@@ -503,12 +508,17 @@ int git_config_get_entry(const git_config_entry **out, const git_config *cfg, co
 {
 	file_internal *internal;
 	unsigned int i;
+	git_config_backend *file;
+	int ret;
 
 	*out = NULL;
 
 	git_vector_foreach(&cfg->files, i, internal) {
-		git_config_backend *file = internal->file;
-		int ret = file->get(file, name, out);
+		if (!internal || !internal->file)
+			continue;
+		file = internal->file;
+
+		ret = file->get(file, name, out);
 		if (ret != GIT_ENOTFOUND)
 			return ret;
 	}
@@ -516,8 +526,9 @@ int git_config_get_entry(const git_config_entry **out, const git_config *cfg, co
 	return config_error_notfound(name);
 }
 
-int git_config_get_multivar(const git_config *cfg, const char *name, const char *regexp,
-		git_config_foreach_cb cb, void *payload)
+int git_config_get_multivar(
+	const git_config *cfg, const char *name, const char *regexp,
+	git_config_foreach_cb cb, void *payload)
 {
 	file_internal *internal;
 	git_config_backend *file;
@@ -530,7 +541,10 @@ int git_config_get_multivar(const git_config *cfg, const char *name, const char
 	 */
 	for (i = cfg->files.length; i > 0; --i) {
 		internal = git_vector_get(&cfg->files, i - 1);
+		if (!internal || !internal->file)
+			continue;
 		file = internal->file;
+
 		ret = file->get_multivar(file, name, regexp, cb, payload);
 		if (ret < 0 && ret != GIT_ENOTFOUND)
 			return ret;
@@ -545,7 +559,7 @@ int git_config_set_multivar(git_config *cfg, const char *name, const char *regex
 	file_internal *internal;
 
 	internal = git_vector_get(&cfg->files, 0);
-	if (!internal)
+	if (!internal || !internal->file)
 		return config_error_nofiles(name);
 	file = internal->file;
 
@@ -640,13 +654,12 @@ int git_config_open_default(git_config **out)
 	git_config *cfg = NULL;
 	git_buf buf = GIT_BUF_INIT;
 
-	error = git_config_new(&cfg);
+	if ((error = git_config_new(&cfg)) < 0)
+		return error;
 
-	if (!error && (!git_config_find_global_r(&buf) ||
-		       !git_config__global_location(&buf))) {
+	if (!git_config_find_global_r(&buf) || !git_config__global_location(&buf)) {
 		error = git_config_add_file_ondisk(cfg, buf.ptr,
 			GIT_CONFIG_LEVEL_GLOBAL, 0);
-	} else {
 	}
 
 	if (!error && !git_config_find_xdg_r(&buf))
@@ -659,7 +672,7 @@ int git_config_open_default(git_config **out)
 
 	git_buf_free(&buf);
 
-	if (error && cfg) {
+	if (error) {
 		git_config_free(cfg);
 		cfg = NULL;
 	}
@@ -672,6 +685,7 @@ int git_config_open_default(git_config **out)
 /***********
  * Parsers
  ***********/
+
 int git_config_lookup_map_value(
 	int *out,
 	const git_cvar_map *maps,

tests-clar/repo/config.c

@@ -13,14 +13,15 @@ void test_repo_config__initialize(void)
 
 	cl_must_pass(p_mkdir("alternate", 0777));
 	cl_git_pass(git_path_prettify(&path, "alternate", NULL));
-
 }
 
 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));
-
 	git_buf_free(&path);
+	cl_assert(!git_path_isdir("alternate"));
+
 	cl_fixture_cleanup("empty_standard_repo");
 }
 
@@ -73,3 +74,127 @@ void test_repo_config__open_missing_global_with_separators(void)
 	git_config_free(config);
 	git_repository_free(repo);
 }
+
+#include "repository.h"
+
+void test_repo_config__read_no_configs(void)
+{
+	git_repository *repo;
+	int val;
+
+	cl_git_pass(git_libgit2_opts(
+		GIT_OPT_SET_SEARCH_PATH, GIT_CONFIG_LEVEL_GLOBAL, path.ptr));
+	cl_git_pass(git_libgit2_opts(
+		GIT_OPT_SET_SEARCH_PATH, GIT_CONFIG_LEVEL_SYSTEM, path.ptr));
+	cl_git_pass(git_libgit2_opts(
+		GIT_OPT_SET_SEARCH_PATH, GIT_CONFIG_LEVEL_XDG, path.ptr));
+
+	/* with none */
+
+	cl_must_pass(p_unlink("empty_standard_repo/.git/config"));
+	cl_assert(!git_path_isfile("empty_standard_repo/.git/config"));
+
+	cl_git_pass(git_repository_open(&repo, "empty_standard_repo"));
+	git_repository__cvar_cache_clear(repo);
+	val = -1;
+	cl_git_pass(git_repository__cvar(&val, repo, GIT_CVAR_ABBREV));
+	cl_assert_equal_i(GIT_ABBREV_DEFAULT, val);
+	git_repository_free(repo);
+
+	/* with just system */
+
+	cl_must_pass(p_mkdir("alternate/1", 0777));
+	cl_git_pass(git_buf_joinpath(&path, path.ptr, "1"));
+	cl_git_rewritefile("alternate/1/gitconfig", "[core]\n\tabbrev = 10\n");
+	cl_git_pass(git_libgit2_opts(
+		GIT_OPT_SET_SEARCH_PATH, GIT_CONFIG_LEVEL_SYSTEM, path.ptr));
+
+	cl_git_pass(git_repository_open(&repo, "empty_standard_repo"));
+	git_repository__cvar_cache_clear(repo);
+	val = -1;
+	cl_git_pass(git_repository__cvar(&val, repo, GIT_CVAR_ABBREV));
+	cl_assert_equal_i(10, val);
+	git_repository_free(repo);
+
+	/* with xdg + system */
+
+	cl_must_pass(p_mkdir("alternate/2", 0777));
+	path.ptr[path.size - 1] = '2';
+	cl_git_rewritefile("alternate/2/config", "[core]\n\tabbrev = 20\n");
+	cl_git_pass(git_libgit2_opts(
+		GIT_OPT_SET_SEARCH_PATH, GIT_CONFIG_LEVEL_XDG, path.ptr));
+
+	cl_git_pass(git_repository_open(&repo, "empty_standard_repo"));
+	git_repository__cvar_cache_clear(repo);
+	val = -1;
+	cl_git_pass(git_repository__cvar(&val, repo, GIT_CVAR_ABBREV));
+	cl_assert_equal_i(20, val);
+	git_repository_free(repo);
+
+	/* with global + xdg + system */
+
+	cl_must_pass(p_mkdir("alternate/3", 0777));
+	path.ptr[path.size - 1] = '3';
+	cl_git_rewritefile("alternate/3/.gitconfig", "[core]\n\tabbrev = 30\n");
+	cl_git_pass(git_libgit2_opts(
+		GIT_OPT_SET_SEARCH_PATH, GIT_CONFIG_LEVEL_GLOBAL, path.ptr));
+
+	cl_git_pass(git_repository_open(&repo, "empty_standard_repo"));
+	git_repository__cvar_cache_clear(repo);
+	val = -1;
+	cl_git_pass(git_repository__cvar(&val, repo, GIT_CVAR_ABBREV));
+	cl_assert_equal_i(30, val);
+	git_repository_free(repo);
+
+	/* with all configs */
+
+	cl_git_rewritefile("empty_standard_repo/.git/config", "[core]\n\tabbrev = 40\n");
+
+	cl_git_pass(git_repository_open(&repo, "empty_standard_repo"));
+	git_repository__cvar_cache_clear(repo);
+	val = -1;
+	cl_git_pass(git_repository__cvar(&val, repo, GIT_CVAR_ABBREV));
+	cl_assert_equal_i(40, val);
+	git_repository_free(repo);
+
+	/* with all configs but delete the files ? */
+
+	cl_git_pass(git_repository_open(&repo, "empty_standard_repo"));
+	git_repository__cvar_cache_clear(repo);
+	val = -1;
+	cl_git_pass(git_repository__cvar(&val, repo, GIT_CVAR_ABBREV));
+	cl_assert_equal_i(40, val);
+
+	cl_must_pass(p_unlink("empty_standard_repo/.git/config"));
+	cl_assert(!git_path_isfile("empty_standard_repo/.git/config"));
+
+	cl_must_pass(p_unlink("alternate/1/gitconfig"));
+	cl_assert(!git_path_isfile("alternate/1/gitconfig"));
+
+	cl_must_pass(p_unlink("alternate/2/config"));
+	cl_assert(!git_path_isfile("alternate/2/config"));
+
+	cl_must_pass(p_unlink("alternate/3/.gitconfig"));
+	cl_assert(!git_path_isfile("alternate/3/.gitconfig"));
+
+	git_repository__cvar_cache_clear(repo);
+	val = -1;
+	cl_git_pass(git_repository__cvar(&val, repo, GIT_CVAR_ABBREV));
+	cl_assert_equal_i(40, val);
+	git_repository_free(repo);
+
+	/* reopen */
+
+	cl_assert(!git_path_isfile("empty_standard_repo/.git/config"));
+	cl_assert(!git_path_isfile("alternate/3/.gitconfig"));
+
+	cl_git_pass(git_repository_open(&repo, "empty_standard_repo"));
+	git_repository__cvar_cache_clear(repo);
+	val = -1;
+	cl_git_pass(git_repository__cvar(&val, repo, GIT_CVAR_ABBREV));
+	cl_assert_equal_i(7, val);
+	git_repository_free(repo);
+
+	cl_assert(!git_path_exists("empty_standard_repo/.git/config"));
+	cl_assert(!git_path_exists("alternate/3/.gitconfig"));
+}