config: Cleanup external API

Do not mess with environment variables anymore. The new external API has
more helper methods, and everything is explicit.

include/git2/config.h

@@ -76,34 +76,54 @@ GIT_EXTERN(int) git_config_file__ondisk(struct git_config_file **out, const char
 GIT_EXTERN(int) git_config_new(git_config **out);
 
 /**
- * Open a configuration file
+ * Add a generic config file instance to an existing config
  *
- * This creates a new configuration object and adds the specified file
- * to it.
+ * Note that the configuration object will free the file
+ * automatically.
  *
- * @param cfg_out pointer to the configuration data
- * @param path where to load the confiration from
- */
-GIT_EXTERN(int) git_config_open_file(git_config **cfg_out, const char *path);
-
-/**
- * Open the global configuration file at $HOME/.gitconfig
+ * Further queries on this config object will access each
+ * of the config file instances in order (instances with
+ * a higher priority will be accessed first).
  *
- * @param cfg pointer to the configuration
+ * @param cfg the configuration to add the file to
+ * @param file the configuration file (backend) to add
+ * @param priority the priority the backend should have
  */
-GIT_EXTERN(int) git_config_open_global(git_config **cfg);
+GIT_EXTERN(int) git_config_add_file(git_config *cfg, git_config_file *file, int priority);
 
 /**
- * Add a config backend to an existing instance
+ * Add an on-disk config file instance to an existing config
+ *
+ * The on-disk file pointed at by `path` will be opened and
+ * parsed; it's expected to be a native Git config file following
+ * the default Git config syntax (see man git-config).
  *
  * Note that the configuration object will free the file
  * automatically.
  *
+ * Further queries on this config object will access each
+ * of the config file instances in order (instances with
+ * a higher priority will be accessed first).
+ *
  * @param cfg the configuration to add the file to
- * @param file the configuration file (backend) to add
+ * @param file path to the configuration file (backend) to add
  * @param priority the priority the backend should have
  */
-GIT_EXTERN(int) git_config_add_file(git_config *cfg, git_config_file *file, int priority);
+GIT_EXTERN(int) git_config_add_file_ondisk(git_config *cfg, const char *path, int priority);
+
+
+/**
+ * Create a new config instance containing a single on-disk file
+ *
+ * This method is a simple utility wrapper for the following sequence
+ * of calls:
+ *	- git_config_new
+ *	- git_config_add_file_ondisk
+ *
+ * @param cfg The configuration instance to create
+ * @param path Path to the on-disk file to open
+ */
+GIT_EXTERN(int) git_config_open_ondisk(git_config **cfg, const char *path);
 
 /**
  * Free the configuration and its associated memory and files

include/git2/repository.h

@@ -268,12 +268,40 @@ GIT_EXTERN(int) git_repository_is_bare(git_repository *repo);
 /**
  * Retrieve the relevant configuration for a repository
  *
- * Puts together the configuration from the global and local files.
+ * By default he returned `git_config` instance contains a single
+ * configuration file, the `.gitconfig' file that may be found
+ * inside the repository.
+ *
+ * If the `user_config_path` variable is not NULL, the given config
+ * file will be also included in the configuration set. On most UNIX
+ * systems, this file may be found on `$HOME/.gitconfig`.
+ *
+ * If the `system_config_path` variable is not NULL, the given config
+ * file will be also included in the configuration set. On most UNIX
+ * systems, this file may be found on `$PREFIX/etc/gitconfig`.
+ *
+ * The resulting `git_config` instance will query the files in the following
+ * order:
+ *
+ *	- Repository configuration file
+ *	- User configuration file
+ *	- System configuration file
+ *
+ * The method will fail if any of the passed config files cannot be
+ * found or accessed.
+ *
+ * The returned `git_config` instance is owned by the caller and must
+ * be manually free'd once it's no longer on use.
  *
  * @param out the repository's configuration
  * @param repo the repository for which to get the config
+ * @param user_config_path Path to the user config file
+ * @param system_config_path Path to the system-wide config file
  */
-GIT_EXTERN(int) git_repository_config(git_config **out, git_repository *repo);
+GIT_EXTERN(int) git_repository_config(git_config **out,
+	git_repository *repo,
+	const char *user_config_path,
+	const char *system_config_path);
 
 /** @} */
 GIT_END_DECL

src/config.c

@@ -37,54 +37,6 @@ typedef struct {
 	int priority;
 } file_internal;
 
-int git_config_open_file(git_config **out, const char *path)
-{
-	git_config_file *file = NULL;
-	git_config *cfg = NULL;
-	int error = GIT_SUCCESS;
-
-	error = git_config_new(&cfg);
-	if (error < GIT_SUCCESS)
-		return error;
-
-	error = git_config_file__ondisk(&file, path);
-	if (error < GIT_SUCCESS) {
-		git_config_free(cfg);
-		return error;
-	}
-
-	error = git_config_add_file(cfg, file, 1);
-	if (error < GIT_SUCCESS) {
-		file->free(file);
-		git_config_free(cfg);
-		return error;
-	}
-
-	error = file->open(file);
-	if (error < GIT_SUCCESS) {
-		git_config_free(cfg);
-		return git__rethrow(error, "Failed to open config file");
-	}
-
-	*out = cfg;
-
-	return GIT_SUCCESS;
-}
-
-int git_config_open_global(git_config **out)
-{
-	char full_path[GIT_PATH_MAX];
-	const char *home;
-
-	home = getenv("HOME");
-	if (home == NULL)
-		return git__throw(GIT_EOSERR, "Failed to open global config file. Cannot find $HOME variable");
-
-	git__joinpath(full_path, home, GIT_CONFIG_FILENAME);
-
-	return git_config_open_file(out, full_path);
-}
-
 void git_config_free(git_config *cfg)
 {
 	unsigned int i;
@@ -130,12 +82,49 @@ int git_config_new(git_config **out)
 	return GIT_SUCCESS;
 }
 
+int git_config_add_file_ondisk(git_config *cfg, const char *path, int priority)
+{
+	git_config_file *file = NULL;
+	int error;
+
+	error = git_config_file__ondisk(&file, path);
+	if (error < GIT_SUCCESS)
+		return error;
+
+	error = git_config_add_file(cfg, file, priority);
+	if (error < GIT_SUCCESS) {
+		file->free(file); /* free manually; the file is not owned by the ODB yet */
+		return error;
+	}
+
+	return GIT_SUCCESS;
+}
+
+int git_config_open_ondisk(git_config **cfg, const char *path)
+{
+	int error;
+
+	error = git_config_new(cfg);
+	if (error < GIT_SUCCESS)
+		return error;
+
+	error = git_config_add_file_ondisk(*cfg, path, 1);
+	if (error < GIT_SUCCESS)
+		git_config_free(*cfg);
+
+	return error;
+}
+
 int git_config_add_file(git_config *cfg, git_config_file *file, int priority)
 {
 	file_internal *internal;
+	int error;
 
 	assert(cfg && file);
 
+	if ((error = file->open(file)) < GIT_SUCCESS)
+		return git__throw(error, "Failed to open config file");
+
 	internal = git__malloc(sizeof(file_internal));
 	if (internal == NULL)
 		return GIT_ENOMEM;

src/repository.c

@@ -271,45 +271,42 @@ cleanup:
 	return git__rethrow(error, "Failed to open repository");
 }
 
-int git_repository_config(git_config **out, git_repository *repo)
+int git_repository_config(
+		git_config **out,
+		git_repository *repo,
+		const char *user_config_path,
+		const char *system_config_path)
 {
-	git_config *cfg = NULL;
-	git_config_file *local = NULL;
-	char gitconfig[GIT_PATH_MAX];
-	int error = GIT_SUCCESS;
+	char config_path[GIT_PATH_MAX];
+	int error;
+
+	assert(out && repo);
 
-	error = git_config_open_global(&cfg);
+	error = git_config_new(out);
 	if (error < GIT_SUCCESS)
-		return git__rethrow(error, "Failed to open global config");
+		return error;
 
-	git__joinpath(gitconfig, repo->path_repository, GIT_CONFIG_FILENAME_INREPO);
-	error = git_config_file__ondisk(&local, gitconfig);
-	if (error < GIT_SUCCESS) {
-		error = git__rethrow(error, "Failed to open local config");
+	git__joinpath(config_path, repo->path_repository, GIT_CONFIG_FILENAME_INREPO);
+	error = git_config_add_file_ondisk(*out, config_path, 1);
+	if (error < GIT_SUCCESS)
 		goto cleanup;
-	}
 
-	error = git_config_add_file(cfg, local, 2);
-	if (error < GIT_SUCCESS) {
-		error = git__rethrow(error, "Failed to add the local config");
+	if (user_config_path != NULL) {
+		error = git_config_add_file_ondisk(*out, user_config_path, 2);
+		if (error < GIT_SUCCESS)
 			goto cleanup;
 	}
 
-	error = local->open(local);
-	if (error < GIT_SUCCESS) {
-		error = git__rethrow(error, "Failed to open config file");
+	if (system_config_path != NULL) {
+		error = git_config_add_file_ondisk(*out, system_config_path, 3);
+		if (error < GIT_SUCCESS)
 			goto cleanup;
 	}
 
-	*out = cfg;
+	return GIT_SUCCESS;
 
 cleanup:
-	if (error < GIT_SUCCESS) {
-		git_config_free(cfg);
-		if (local)
-			local->free(local);
-	}
-
+	git_config_free(*out);
 	return error;
 }
 

tests/t15-config.c

@@ -36,7 +36,7 @@ BEGIN_TEST(config0, "read a simple configuration")
 	git_config *cfg;
 	int i;
 
-	must_pass(git_config_open_file(&cfg, CONFIG_BASE "/config0"));
+	must_pass(git_config_open_ondisk(&cfg, CONFIG_BASE "/config0"));
 	must_pass(git_config_get_int(cfg, "core.repositoryformatversion", &i));
 	must_be_true(i == 0);
 	must_pass(git_config_get_bool(cfg, "core.filemode", &i));
@@ -58,7 +58,7 @@ BEGIN_TEST(config1, "case sensitivity")
 	int i;
 	const char *str;
 
-	must_pass(git_config_open_file(&cfg, CONFIG_BASE "/config1"));
+	must_pass(git_config_open_ondisk(&cfg, CONFIG_BASE "/config1"));
 
 	must_pass(git_config_get_string(cfg, "this.that.other", &str));
 	must_be_true(!strcmp(str, "true"));
@@ -84,7 +84,7 @@ BEGIN_TEST(config2, "parse a multiline value")
 	git_config *cfg;
 	const char *str;
 
-	must_pass(git_config_open_file(&cfg, CONFIG_BASE "/config2"));
+	must_pass(git_config_open_ondisk(&cfg, CONFIG_BASE "/config2"));
 
 	must_pass(git_config_get_string(cfg, "this.That.and", &str));
 	must_be_true(!strcmp(str, "one one one two two three three"));
@@ -99,7 +99,7 @@ BEGIN_TEST(config3, "parse a [section.subsection] header")
 	git_config *cfg;
 	const char *str;
 
-	must_pass(git_config_open_file(&cfg, CONFIG_BASE "/config3"));
+	must_pass(git_config_open_ondisk(&cfg, CONFIG_BASE "/config3"));
 
 	must_pass(git_config_get_string(cfg, "section.subsection.var", &str));
 	must_be_true(!strcmp(str, "hello"));
@@ -117,7 +117,7 @@ BEGIN_TEST(config4, "a variable name on its own is valid")
 const char *str;
 int i;
 
-	must_pass(git_config_open_file(&cfg, CONFIG_BASE "/config4"));
+	must_pass(git_config_open_ondisk(&cfg, CONFIG_BASE "/config4"));
 
 	must_pass(git_config_get_string(cfg, "some.section.variable", &str));
 	must_be_true(str == NULL);
@@ -133,7 +133,7 @@ BEGIN_TEST(config5, "test number suffixes")
 	git_config *cfg;
 	long int i;
 
-	must_pass(git_config_open_file(&cfg, CONFIG_BASE "/config5"));
+	must_pass(git_config_open_ondisk(&cfg, CONFIG_BASE "/config5"));
 
 	must_pass(git_config_get_long(cfg, "number.simple", &i));
 	must_be_true(i == 1);
@@ -163,7 +163,7 @@ BEGIN_TEST(config6, "test blank lines")
 	git_config *cfg;
 	int i;
 
-	must_pass(git_config_open_file(&cfg, CONFIG_BASE "/config6"));
+	must_pass(git_config_open_ondisk(&cfg, CONFIG_BASE "/config6"));
 
 	must_pass(git_config_get_bool(cfg, "valid.subsection.something", &i));
 	must_be_true(i == 1);
@@ -177,14 +177,14 @@ END_TEST
 BEGIN_TEST(config7, "test for invalid ext headers")
 	git_config *cfg;
 
-	must_fail(git_config_open_file(&cfg, CONFIG_BASE "/config7"));
+	must_fail(git_config_open_ondisk(&cfg, CONFIG_BASE "/config7"));
 
 END_TEST
 
 BEGIN_TEST(config8, "don't fail on empty files")
 	git_config *cfg;
 
-	must_pass(git_config_open_file(&cfg, CONFIG_BASE "/config8"));
+	must_pass(git_config_open_ondisk(&cfg, CONFIG_BASE "/config8"));
 
 	git_config_free(cfg);
 END_TEST
@@ -195,16 +195,16 @@ BEGIN_TEST
 	int i;
 
 	/* By freeing the config, we make sure we flush the values  */
-	must_pass(git_config_open_file(&cfg, CONFIG_BASE "/config9"));
+	must_pass(git_config_open_ondisk(&cfg, CONFIG_BASE "/config9"));
 	must_pass(git_config_set_int(cfg, "core.dummy", 5));
 	git_config_free(cfg);
 
-	must_pass(git_config_open_file(&cfg, CONFIG_BASE "/config9"));
+	must_pass(git_config_open_ondisk(&cfg, CONFIG_BASE "/config9"));
 	must_pass(git_config_get_int(cfg, "core.dummy", &i));
 	must_be_true(i == 5);
 	git_config_free(cfg);
 
-	must_pass(git_config_open_file(&cfg, CONFIG_BASE "/config9"));
+	must_pass(git_config_open_ondisk(&cfg, CONFIG_BASE "/config9"));
 	must_pass(git_config_set_int(cfg, "core.dummy", 1));
 	git_config_free(cfg);
 
@@ -222,7 +222,7 @@ BEGIN_TEST(config10, "a repo's config overrides the global config")
 	setenv("HOME", CONFIG_BASE, 1);
 
 	must_pass(git_repository_open(&repo, REPOSITORY_FOLDER));
-	must_pass(git_repository_config(&cfg, repo));
+	must_pass(git_repository_config(&cfg, repo, NULL, NULL));
 	setenv("HOME", home_orig, 1);
 	must_pass(git_config_get_int(cfg, "core.repositoryformatversion", &version));
 	must_be_true(version == 0);
@@ -232,18 +232,11 @@ END_TEST
 
 BEGIN_TEST(config11, "fall back to the global config")
 	git_repository *repo;
-	char home_orig[GIT_PATH_MAX];
-	char *home;
 	git_config *cfg;
 	int num;
 
-	home = getenv("HOME");
-	strcpy(home_orig, home);
-	setenv("HOME", CONFIG_BASE, 1);
-
 	must_pass(git_repository_open(&repo, REPOSITORY_FOLDER));
-	must_pass(git_repository_config(&cfg, repo));
-	setenv("HOME", home_orig, 1);
+	must_pass(git_repository_config(&cfg, repo, CONFIG_BASE "/.gitconfig", NULL));
 	must_pass(git_config_get_int(cfg, "core.something", &num));
 	must_be_true(num == 2);
 	git_config_free(cfg);