git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[RFC PATCH 5/5] config API: add and use a repo_config_kvi()

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Mar 17, 2023, 05:01 UTC
Message-ID
<RFC-patch-5.5-2b80d293c83-20230317T042408Z-avarab@gmail.com>
In-Reply-To
<RFC-cover-0.5-00000000000-20230317T042408Z-avarab@gmail.com>

Introduce a repo_config_kvi(), which is a repo_config() which calls a "config_kvi_fn_t", rather than the "config_fn_t" that repo_config() uses.

This allows us to pass along the "struct key_value_info *" directly, rather than having the callback grab it from the global "current_config_kvi" that we've been maintaining in "configset_iter()".

This change is an alternate direction to the topic at [1], this expands on the vague suggestions I made in [2] to go in this direction.

As this shows we can split apart the "config_fn_t", and thus avoid having to change the hundreds of existing "config_fn_t" callers. By doing this we can already get rid of the current_config_kvi() function, as "builtin/remote.c" and "t/helper/test-config.c" were the only users of it.

The change to "t/t5505-remote.sh" ensures that the change here to config_read_push_default() isn't breaking things. It's the only test that would go through that codepath, but nothing asserted that we'd get the correct line number. Let's sanity check that, as well as the other callback data.

This leaves the other current_config_*() functions. Subsequent commits will need to deal with those.

1. https://lore.kernel.org/git/pull.1463.v2.git.git.1678925506.gitgitgadget@gmail.com/
2. https://lore.kernel.org/git/230307.86wn3szrzu.gmgdl@evledraar.gmail.com/
3. https://lore.kernel.org/git/230308.867cvrziac.gmgdl@evledraar.gmail.com/
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/remote.c       | 11 ++++++-----
 config.c               | 32 ++++++++++++++++++--------------
 config.h               |  9 ++++++++-
 t/helper/test-config.c | 13 +++++++------
 t/t5505-remote.sh      |  7 +++++--
 5 files changed, 44 insertions(+), 28 deletions(-)
diff --git a/builtin/remote.c b/builtin/remote.c
index 729f6f3643a..c65bce05034 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -644,17 +644,18 @@ struct push_default_info
 };
 
 static int config_read_push_default(const char *key, const char *value,
-	void *cb)
+				    struct key_value_info *kvi, void *cb)
 {
 	struct push_default_info* info = cb;
 	if (strcmp(key, "remote.pushdefault") ||
 	    !value || strcmp(value, info->old_name))
 		return 0;
 
-	info->scope = current_config_scope();
+	info->scope = kvi->scope;
 	strbuf_reset(&info->origin);
-	strbuf_addstr(&info->origin, current_config_name());
-	info->linenr = current_config_line();
+	if (kvi->filename)
+		strbuf_addstr(&info->origin, kvi->filename);
+	info->linenr = kvi->linenr;
 
 	return 0;
 }
@@ -663,7 +664,7 @@ static void handle_push_default(const char* old_name, const char* new_name)
 {
 	struct push_default_info push_default = {
 		old_name, CONFIG_SCOPE_UNKNOWN, STRBUF_INIT, -1 };
-	git_config(config_read_push_default, &push_default);
+	repo_config_kvi(the_repository, config_read_push_default, &push_default);
 	if (push_default.scope >= CONFIG_SCOPE_COMMAND)
 		; /* pass */
 	else if (push_default.scope >= CONFIG_SCOPE_LOCAL) {
diff --git a/config.c b/config.c
index 230a98b0631..1b3f534757c 100644
--- a/config.c
+++ b/config.c
@@ -2219,7 +2219,8 @@ int config_with_options(config_fn_t fn, void *data,
 	return ret;
 }
 
-static void configset_iter(struct config_set *cs, config_fn_t fn, void *data)
+static void configset_iter(struct config_set *cs, config_fn_t fn,
+			   config_kvi_fn_t fn_kvi, void *data)
 {
 	int i, value_index;
 	struct string_list *values;
@@ -2230,6 +2231,7 @@ static void configset_iter(struct config_set *cs, config_fn_t fn, void *data)
 		const char *key;
 		const char *val;
 		struct key_value_info *kvi;
+		int ret;
 
 		entry = list->items[i].e;
 		value_index = list->items[i].value_index;
@@ -2239,11 +2241,15 @@ static void configset_iter(struct config_set *cs, config_fn_t fn, void *data)
 		val = values->items[value_index].string;
 		kvi = values->items[value_index].util;
 
-		current_config_kvi = kvi;
-		if (fn(key, val, data) < 0)
+		if (!fn_kvi)
+			current_config_kvi = kvi;
+		ret = fn_kvi ? fn_kvi(key, val, kvi, data) :
+			fn(key, val, data);
+		current_config_kvi = NULL;
+
+		if (ret < 0)
 			git_die_config_linenr(entry->key, kvi->filename,
 					      kvi->linenr);
-		current_config_kvi = NULL;
 	}
 }
 
@@ -2567,7 +2573,13 @@ static void repo_config_clear(struct repository *repo)
 void repo_config(struct repository *repo, config_fn_t fn, void *data)
 {
 	git_config_check_init(repo);
-	configset_iter(repo->config, fn, data);
+	configset_iter(repo->config, fn, NULL, data);
+}
+
+void repo_config_kvi(struct repository *repo, config_kvi_fn_t fn, void *data)
+{
+	git_config_check_init(repo);
+	configset_iter(repo->config, NULL, fn, data);
 }
 
 int repo_config_get_value(struct repository *repo,
@@ -2670,7 +2682,7 @@ void git_protected_config(config_fn_t fn, void *data)
 {
 	if (!protected_config.hash_initialized)
 		read_protected_config();
-	configset_iter(&protected_config, fn, data);
+	configset_iter(&protected_config, fn, NULL, data);
 }
 
 /* Functions used historically to read configuration from 'the_repository' */
@@ -3843,14 +3855,6 @@ enum config_scope current_config_scope(void)
 		return current_parsing_scope;
 }
 
-int current_config_line(void)
-{
-	if (current_config_kvi)
-		return current_config_kvi->linenr;
-	else
-		return cf->linenr;
-}
-
 int lookup_config(const char **mapping, int nr_mapping, const char *var)
 {
 	int i;
diff --git a/config.h b/config.h
index a9cb01e9405..de5350dbee5 100644
--- a/config.h
+++ b/config.h
@@ -143,6 +143,13 @@ const char *config_origin_type_name(enum config_origin_type type);
  */
 typedef int (*config_fn_t)(const char *, const char *, void *);
 
+/**
+ * Like config_fn_t, but before the callback-specific data we'll get a
+ * "struct key_value_info" indicating the origin of the config.
+ */
+typedef int (*config_kvi_fn_t)(const char *key, const char *var,
+			       struct key_value_info *kvi, void *data);
+
 int git_default_config(const char *, const char *, void *);
 
 /**
@@ -371,7 +378,6 @@ int git_config_parse_parameter(const char *, config_fn_t fn, void *data);
 enum config_scope current_config_scope(void);
 const char *current_config_origin_type(void);
 const char *current_config_name(void);
-int current_config_line(void);
 
 /*
  * Match and parse a config key of the form:
@@ -498,6 +504,7 @@ int git_configset_get_pathname(struct config_set *cs, const char *key, const cha
 /* Functions for reading a repository's config */
 struct repository;
 void repo_config(struct repository *repo, config_fn_t fn, void *data);
+void repo_config_kvi(struct repository *repo, config_kvi_fn_t fn, void *data);
 int repo_config_get_value(struct repository *repo,
 			  const char *key, const char **value);
 const struct string_list *repo_config_get_value_multi(struct repository *repo,
diff --git a/t/helper/test-config.c b/t/helper/test-config.c
index 4ba9eb65606..2ef67b18a0b 100644
--- a/t/helper/test-config.c
+++ b/t/helper/test-config.c
@@ -37,7 +37,8 @@
  *
  */
 
-static int iterate_cb(const char *var, const char *value, void *data UNUSED)
+static int iterate_cb(const char *var, const char *value,
+		      struct key_value_info *kvi, void *data UNUSED)
 {
 	static int nr;
 
@@ -46,10 +47,10 @@ static int iterate_cb(const char *var, const char *value, void *data UNUSED)
 
 	printf("key=%s\n", var);
 	printf("value=%s\n", value ? value : "(null)");
-	printf("origin=%s\n", current_config_origin_type());
-	printf("name=%s\n", current_config_name());
-	printf("lno=%d\n", current_config_line());
-	printf("scope=%s\n", config_scope_name(current_config_scope()));
+	printf("origin=%s\n", config_origin_type_name(kvi->origin_type));
+	printf("name=%s\n", kvi->filename ? kvi->filename : "");
+	printf("lno=%d\n", kvi->linenr);
+	printf("scope=%s\n", config_scope_name(kvi->scope));
 
 	return 0;
 }
@@ -174,7 +175,7 @@ int cmd__config(int argc, const char **argv)
 			goto exit1;
 		}
 	} else if (!strcmp(argv[1], "iterate")) {
-		git_config(iterate_cb, NULL);
+		repo_config_kvi(the_repository, iterate_cb, NULL);
 		goto exit0;
 	}
 
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index 43b7bcd7159..d9659b9c65e 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -853,8 +853,11 @@ test_expect_success 'rename a remote' '
 	(
 		cd four &&
 		git config branch.main.pushRemote origin &&
-		GIT_TRACE2_EVENT=$(pwd)/trace \
-			git remote rename --progress origin upstream &&
+		GIT_TRACE2_EVENT=$PWD/trace \
+			git remote rename --progress origin upstream 2>warn &&
+		grep -F "The global configuration remote.pushDefault" warn &&
+		grep "/\.gitconfig:2$" warn &&
+		grep "remote '\''origin'\''" warn &&
 		test_region progress "Renaming remote references" trace &&
 		grep "pushRemote" .git/config &&
 		test -z "$(git for-each-ref refs/remotes/origin)" &&

-- 
2.40.0.rc1.1034.g5867a1b10c5
Previous: Ævar Arnfjörð BjarmasonNext: Junio C Hamano
Message 49 of 72 in “[RFC] config.c: use struct for config reading state”
  1. 0/6 [RFC] config.c: use struct for config reading stateGlen Choo via GitGitGadget, Mar 1, 2023
  2. 2/6 config.c: don't assign to "cf" directlyGlen Choo via GitGitGadget, Mar 1, 2023
  3. 1/6 config.c: plumb config_source through static fnsGlen Choo via GitGitGadget, Mar 1, 2023
  4. Junio C HamanoMar 3, 2023
  5. 3/6 config.c: create config_reader and the_readerGlen Choo via GitGitGadget, Mar 1, 2023
  6. Junio C HamanoMar 3, 2023
  7. 5/6 config.c: remove current_config_kviGlen Choo via GitGitGadget, Mar 1, 2023
  8. Calvin WanMar 6, 2023
  9. 4/6 config.c: plumb the_reader through callbacksGlen Choo via GitGitGadget, Mar 1, 2023
  10. Ævar Arnfjörð BjarmasonMar 8, 2023
  11. Glen ChooMar 8, 2023
  12. Junio C HamanoMar 8, 2023
  13. 6/6 config.c: remove current_parsing_scopeGlen Choo via GitGitGadget, Mar 1, 2023
  14. Jonathan TanMar 6, 2023
  15. Glen ChooMar 6, 2023
  16. Jonathan TanMar 6, 2023
  17. Ævar Arnfjörð BjarmasonMar 8, 2023
  18. Glen ChooMar 8, 2023
  19. Ævar Arnfjörð BjarmasonMar 7, 2023
  20. Glen ChooMar 7, 2023
  21. Ævar Arnfjörð BjarmasonMar 7, 2023
  22. Junio C HamanoMar 7, 2023
  23. Glen ChooMar 7, 2023
  24. Ævar Arnfjörð BjarmasonMar 8, 2023
  25. Glen ChooMar 8, 2023
  26. 0/8 config.c: use struct for config reading stateGlen Choo via GitGitGadget, Mar 16, 2023
  27. 2/8 config.c: don't assign to "cf_global" directlyGlen Choo via GitGitGadget, Mar 16, 2023
  28. Jonathan TanMar 16, 2023
  29. Junio C HamanoMar 16, 2023
  30. Glen ChooMar 16, 2023
  31. 1/8 config.c: plumb config_source through static fnsGlen Choo via GitGitGadget, Mar 16, 2023
  32. Jonathan TanMar 16, 2023
  33. 3/8 config.c: create config_reader and the_readerGlen Choo via GitGitGadget, Mar 16, 2023
  34. Jonathan TanMar 16, 2023
  35. 4/8 config.c: plumb the_reader through callbacksGlen Choo via GitGitGadget, Mar 16, 2023
  36. 6/8 config.c: remove current_parsing_scopeGlen Choo via GitGitGadget, Mar 16, 2023
  37. 5/8 config.c: remove current_config_kviGlen Choo via GitGitGadget, Mar 16, 2023
  38. 7/8 config: report cached filenames in die_bad_number()Glen Choo via GitGitGadget, Mar 16, 2023
  39. Jonathan TanMar 16, 2023
  40. Glen ChooMar 16, 2023
  41. 8/8 config.c: rename "struct config_source cf"Glen Choo via GitGitGadget, Mar 16, 2023
  42. Glen ChooMar 16, 2023
  43. Jonathan TanMar 16, 2023
  44. 0/5 bypass config.c global state with configsetÆvar Arnfjörð Bjarmason, Mar 17, 2023
  45. 1/5 config.h: move up "struct key_value_info"Ævar Arnfjörð Bjarmason, Mar 17, 2023
  46. 2/5 config.c: use "enum config_origin_type", not "int"Ævar Arnfjörð Bjarmason, Mar 17, 2023
  47. 3/5 config API: add a config_origin_type_name() helperÆvar Arnfjörð Bjarmason, Mar 17, 2023
  48. 4/5 config.c: refactor configset_iter()Ævar Arnfjörð Bjarmason, Mar 17, 2023
  49. 5/5 config API: add and use a repo_config_kvi()Ævar Arnfjörð Bjarmason, Mar 17, 2023
  50. Junio C HamanoMar 17, 2023
  51. Jonathan TanMar 17, 2023
  52. Junio C HamanoMar 17, 2023
  53. Glen ChooMar 17, 2023
  54. Glen ChooMar 17, 2023
  55. Glen ChooMar 17, 2023
  56. Ævar Arnfjörð BjarmasonMar 29, 2023
  57. 0/8 config.c: use struct for config reading stateGlen Choo via GitGitGadget, Mar 28, 2023
  58. 1/8 config.c: plumb config_source through static fnsGlen Choo via GitGitGadget, Mar 28, 2023
  59. 2/8 config.c: don't assign to "cf_global" directlyGlen Choo via GitGitGadget, Mar 28, 2023
  60. 3/8 config.c: create config_reader and the_readerGlen Choo via GitGitGadget, Mar 28, 2023
  61. Ævar Arnfjörð BjarmasonMar 29, 2023
  62. Junio C HamanoMar 29, 2023
  63. Glen ChooMar 29, 2023
  64. Glen ChooMar 30, 2023
  65. 4/8 config.c: plumb the_reader through callbacksGlen Choo via GitGitGadget, Mar 28, 2023
  66. 5/8 config.c: remove current_config_kviGlen Choo via GitGitGadget, Mar 28, 2023
  67. 6/8 config.c: remove current_parsing_scopeGlen Choo via GitGitGadget, Mar 28, 2023
  68. 7/8 config: report cached filenames in die_bad_number()Glen Choo via GitGitGadget, Mar 28, 2023
  69. 8/8 config.c: rename "struct config_source cf"Glen Choo via GitGitGadget, Mar 28, 2023
  70. Glen ChooMar 28, 2023
  71. Junio C HamanoMar 28, 2023
  72. Glen ChooMar 28, 2023

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.