Re: [PATCH v9r2 1/2] add `config_set` API for caching config-like files

[Tanay Abhra <tanayabh@gmail.com>]

On 7/17/2014 3:31 PM, Matthieu Moy wrote:
> Tanay Abhra <tanayabh@gmail.com> writes:
> 
>> I have a question about renaming git_config() to git_config_raw().
>> I was working on a patch and I am getting stuck here.
>> It fails in t1001, t1020 & t4018. The reason for the last test failure
>> is unknown right now. For the first two, it boils down to this,
>>
>> 		git init --bare bare-ancestor-aliased.git &&
>> 		cd bare-ancestor-aliased.git &&
>> 		echo "[alias] aliasedinit = init" >>config &&
>> 		mkdir plain-nested &&
>> 		cd plain-nested &&
>> 		git aliasedinit
> 
> Git tries to read .git/config relative to the current directory, and
> tries to resolve aliases from it. The problem is: if one tries to do
> this from a subdirectory inside the repo, .git/config is not the right
> path, and the alias lookup fails.
> 
> I'll investigate more later.
>

Hmn, this does the trick,
-- 8< --
diff --git a/cache.h b/cache.h
index c67639d..66b52f1 100644
--- a/cache.h
+++ b/cache.h
@@ -1272,6 +1272,7 @@ extern int git_config_from_buf(config_fn_t fn, const char *name,
 			       const char *buf, size_t len, void *data);
 extern void git_config_push_parameter(const char *text);
 extern int git_config_from_parameters(config_fn_t fn, void *data);
+extern int git_config_raw(config_fn_t fn, void *);
 extern int git_config(config_fn_t fn, void *);
 extern int git_config_with_options(config_fn_t fn, void *,
 				   struct git_config_source *config_source,
diff --git a/config.c b/config.c
index d14f761..9e3f99a 100644
--- a/config.c
+++ b/config.c
@@ -1222,11 +1222,36 @@ int git_config_with_options(config_fn_t fn, void *data,
 	return ret;
 }

-int git_config(config_fn_t fn, void *data)
+extern int git_config_raw(config_fn_t fn, void *data)
 {
 	return git_config_with_options(fn, data, NULL, 1);
 }

+int git_configset_iter(struct config_set *cs, config_fn_t fn, void *data)
+{
+	int i;
+	struct string_list *strptr;
+	struct config_set_element *entry;
+	struct hashmap_iter iter;
+	hashmap_iter_init(&cs->config_hash, &iter);
+	while ((entry = hashmap_iter_next(&iter))) {
+		strptr = &entry->value_list;
+		for (i = 0; i < strptr->nr; i++) {
+			if (fn(entry->key, strptr->items[i].string, data) < 0)
+				die("bad config file");
+		}
+	}
+	return 0;
+}
+
+static void git_config_check_init(void);
+
+int git_config(config_fn_t fn, void *data)
+{
+	git_config_check_init();
+	return git_configset_iter(&the_config_set, fn, data);
+}
+
 static struct config_set_element *configset_find_element(struct config_set *cs, const char *key)
 {
 	struct config_set_element k;
@@ -1409,7 +1434,7 @@ static void git_config_check_init(void)
 	if (the_config_set.hash_initialized)
 		return;
 	git_configset_init(&the_config_set);
-	git_config(config_hash_callback, &the_config_set);
+	git_config_raw(config_hash_callback, &the_config_set);
 }

 void git_config_clear(void)
diff --git a/pager.c b/pager.c
index 8b5cbc5..b4237e6 100644
--- a/pager.c
+++ b/pager.c
@@ -177,7 +177,7 @@ int check_pager_config(const char *cmd)
 	c.cmd = cmd;
 	c.want = -1;
 	c.value = NULL;
-	git_config(pager_command_config, &c);
+	git_config_raw(pager_command_config, &c);
 	if (c.value)
 		pager_program = c.value;
 	return c.want;
-- 8< --

The offending part is in git.c, line number 540:

static void execv_dashed_external(const char **argv)
{
	struct strbuf cmd = STRBUF_INIT;
	const char *tmp;
	int status;

	if (use_pager == -1)
		use_pager = check_pager_config(argv[0]);
----------------------------^^^^^^^^^^^^^^^^^^^^^^^^^^^
	commit_pager_choice();


which cause git_config() to be called before handle_alias
can call it in git.c line no 587,

static int run_argv(int *argcp, const char ***argv)
{
	int done_alias = 0;

	while (1) {
		/* See if it's a builtin */
		handle_builtin(*argcp, *argv);

		/* .. then try the external ones */
		execv_dashed_external(*argv);
/* calls git_config() first, skips the .git/config file*/
		/* It could be an alias -- this works around the insanity
		 * of overriding "git log" with "git show" by having
		 * alias.log = show
		 */
		if (done_alias)
			break;
		save_env();
		if (!handle_alias(argcp, argv))
/* does setup_git_directory_gently() before calling git_config() */
			break;
		done_alias = 1;
	}

	return done_alias;
}

I am searching for a more elegant solution to this problem.
Thanks,
Tanay Abhra.