Re: [PATCH 1/2] clone: respect the settings in $HOME/.gitconfig and /etc/gitconfig

[Johannes Schindelin <Johannes.Schindelin@xxxxxx>]

Hi,

On Sun, 29 Jun 2008, Daniel Barkalow wrote:

> Now, clone writes to the config file before reading any configuration, so, 
> if it's going to write to ".git/config" instead of $GIT_CONFIG, it can't 
> read from $GIT_CONFIG either. So there's no way (outside of redesigning 
> config.c) to make GIT_CONFIG useful for "clone" in particular.

Except you could read the config _before_ writing.

Or you could enhance (not redesign, as you suggest) config.c thusly:

-- snip --
diff --git a/cache.h b/cache.h
index 871f6c1..f3ea997 100644
--- a/cache.h
+++ b/cache.h
@@ -742,6 +742,7 @@ extern int git_config_bool(const char *, const char *);
 extern int git_config_string(const char **, const char *, const char *);
 extern int git_config_set(const char *, const char *);
 extern int git_config_set_multivar(const char *, const char *, const char *, int);
+extern int git_config_set_multivar_in_file(const char *, const char *, const char *, int, const char *);
 extern int git_config_rename_section(const char *, const char *);
 extern const char *git_etc_gitconfig(void);
 extern int check_repository_format_version(const char *var, const char *value, void *cb);
diff --git a/config.c b/config.c
index 58749bf..41a35eb 100644
--- a/config.c
+++ b/config.c
@@ -863,23 +863,31 @@ int git_config_set(const char* key, const char* value)
  * - the config file is removed and the lock file rename()d to it.
  *
  */
-int git_config_set_multivar(const char* key, const char* value,
-	const char* value_regex, int multi_replace)
+int git_config_set_multivar(const char *key, const char *value,
+	const char *value_regex, int multi_replace)
+{
+	return git_config_set_multivar_in_file(key, value, value_regex,
+		multi_replace, NULL);
+}
+
+int git_config_set_multivar_in_file(const char *key, const char *value,
+	const char *value_regex, int multi_replace, const char *config_filename)
 {
 	int i, dot;
 	int fd = -1, in_fd;
 	int ret;
-	char* config_filename;
+	char *filename;
 	struct lock_file *lock = NULL;
 	const char* last_dot = strrchr(key, '.');
 
-	config_filename = getenv(CONFIG_ENVIRONMENT);
+	if (!config_filename)
+		config_filename = getenv(CONFIG_ENVIRONMENT);
 	if (!config_filename) {
 		config_filename = getenv(CONFIG_LOCAL_ENVIRONMENT);
 		if (!config_filename)
-			config_filename  = git_path("config");
+			config_filename =
+				filename = xstrdup(git_path("config"));
 	}
-	config_filename = xstrdup(config_filename);
 
 	/*
 	 * Since "key" actually contains the section name and the real
@@ -1091,7 +1099,8 @@ int git_config_set_multivar(const char* key, const char* value,
 out_free:
 	if (lock)
 		rollback_lock_file(lock);
-	free(config_filename);
+	if (filename)
+		free(filename);
 	return ret;
 
 write_err_out:
-- snap --

... and then have something like

        config_filename = xstrdup(mkpath("%s/config", git_dir));

        if (safe_create_leading_directories_const(git_dir) < 0)
                die("could not create leading directories of '%s'", git_dir);
        set_git_dir(make_absolute_path(git_dir));

	[...]

	if (option_bare) {
                strcpy(branch_top, "refs/heads/");

                git_config_set_multivar_in_file("core.bare", "true",
			NULL, 0, config_filename);

	[...]

Of course, you would also have to teach init_db() to use this filename.

But frankly, I do not see the use of your "narrow" special case.  And as I 
stated in another thread, I am pretty opposed to crossing bridges that are 
miles (or an eternity) away.

So unless you present me with a sensible scenario where your "respect 
GIT_CONFIG for _reading_ in GIT_CONFIG=... git clone" makes sense, there 
is nothing to see here, please move along.

Ciao,
Dscho


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html