config.c: pass ctx with CLI config

Pass config_context when parsing CLI config. To provide the .kvi member, refactor out kvi_from_param() from the logic that caches CLI config in configsets. Now that config_context and config_context.kvi is always present when config machinery calls config callbacks, plumb "kvi" so that we can remove all calls of current_config_scope() except for trace2/*.c (which will be handled in a later commit), and remove all other current_config_*() (the functions themselves and their calls). Note that this results in .kvi containing a different, more complete set of information than the mocked up "struct config_source" in git_config_from_parameters(). Plumbing "kvi" reveals a few places where we've been doing the wrong thing: * git_config_parse_parameter() hasn't been setting config source information, so plumb "kvi" there too. * Several sites in builtin/config.c have been calling current_config_*() functions outside of config callbacks (indirectly, via the format_config() helper), which means they're reading state that isn't set correctly: * "git config --get-urlmatch --show-scope" iterates config to collect values, but then attempts to display the scope after config iteration, causing the "unknown" scope to be shown instead of the config file's scope. It's clear that this wasn't intended: we knew that "--get-urlmatch" couldn't show config source metadata, which is why "--show-origin" was marked incompatible with "--get-urlmatch" when it was introduced [1]. It was most likely a mistake that we allowed "--show-scope" to sneak through. Fix this by copying the "kvi" value in the collection phase so that it can be read back later. This means that we can now support "git config --get-urlmatch --show-origin", but that is left unchanged for now. * "git config --default" doesn't have config source metadata when displaying the default value, so "--show-scope" also results in "unknown", and "--show-origin" results in a BUG(). Fix this by treating the default value as if it came from the command line (e.g. like we do with "git -c" or "git config --file"), using kvi_from_param(). [1] https://lore.kernel.org/git/20160205112001.GA13397@sigill.intra.peff.net/ Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Glen Choo committed Jun 28, 2023 at 19:26 UTC 26b669324bb3ef8fd15387815bc020f08d0b4e7b
4 files changed +99 -58
builtin/config.c
+30 -17
@@ -194,38 +194,42 @@ static void check_argc(int argc, int min, int max)
194 usage_builtin_config();
195 }
196
197 -static void show_config_origin(struct strbuf *buf)
197 +static void show_config_origin(const struct key_value_info *kvi,
198 + struct strbuf *buf)
199 {
200 const char term = end_nul ? '\0' : '\t';
201
201 - strbuf_addstr(buf, current_config_origin_type());
202 + strbuf_addstr(buf, config_origin_type_name(kvi->origin_type));
203 strbuf_addch(buf, ':');
204 if (end_nul)
204 - strbuf_addstr(buf, current_config_name());
205 + strbuf_addstr(buf, kvi->filename ? kvi->filename : "");
206 else
206 - quote_c_style(current_config_name(), buf, NULL, 0);
207 + quote_c_style(kvi->filename ? kvi->filename : "", buf, NULL, 0);
208 strbuf_addch(buf, term);
209 }
210
210 -static void show_config_scope(struct strbuf *buf)
211 +static void show_config_scope(const struct key_value_info *kvi,
212 + struct strbuf *buf)
213 {
214 const char term = end_nul ? '\0' : '\t';
213 - const char *scope = config_scope_name(current_config_scope());
215 + const char *scope = config_scope_name(kvi->scope);
216
217 strbuf_addstr(buf, N_(scope));
218 strbuf_addch(buf, term);
219 }
220
221 static int show_all_config(const char *key_, const char *value_,
220 - const struct config_context *ctx UNUSED,
222 + const struct config_context *ctx,
223 void *cb UNUSED)
224 {
225 + const struct key_value_info *kvi = ctx->kvi;
226 +
227 if (show_origin || show_scope) {
228 struct strbuf buf = STRBUF_INIT;
229 if (show_scope)
226 - show_config_scope(&buf);
230 + show_config_scope(kvi, &buf);
231 if (show_origin)
228 - show_config_origin(&buf);
232 + show_config_origin(kvi, &buf);
233 /* Use fwrite as "buf" can contain \0's if "end_null" is set. */
234 fwrite(buf.buf, 1, buf.len, stdout);
235 strbuf_release(&buf);
@@ -243,12 +247,13 @@ struct strbuf_list {
247 int alloc;
248 };
249
246 -static int format_config(struct strbuf *buf, const char *key_, const char *value_)
250 +static int format_config(struct strbuf *buf, const char *key_,
251 + const char *value_, const struct key_value_info *kvi)
252 {
253 if (show_scope)
249 - show_config_scope(buf);
254 + show_config_scope(kvi, buf);
255 if (show_origin)
251 - show_config_origin(buf);
256 + show_config_origin(kvi, buf);
257 if (show_keys)
258 strbuf_addstr(buf, key_);
259 if (!omit_values) {
@@ -303,9 +308,10 @@ static int format_config(struct strbuf *buf, const char *key_, const char *value
308 }
309
310 static int collect_config(const char *key_, const char *value_,
306 - const struct config_context *ctx UNUSED, void *cb)
311 + const struct config_context *ctx, void *cb)
312 {
313 struct strbuf_list *values = cb;
314 + const struct key_value_info *kvi = ctx->kvi;
315
316 if (!use_key_regexp && strcmp(key_, key))
317 return 0;
@@ -320,7 +326,7 @@ static int collect_config(const char *key_, const char *value_,
326 ALLOC_GROW(values->items, values->nr + 1, values->alloc);
327 strbuf_init(&values->items[values->nr], 0);
328
323 - return format_config(&values->items[values->nr++], key_, value_);
329 + return format_config(&values->items[values->nr++], key_, value_, kvi);
330 }
331
332 static int get_value(const char *key_, const char *regex_, unsigned flags)
@@ -382,11 +388,14 @@ static int get_value(const char *key_, const char *regex_, unsigned flags)
388 &config_options);
389
390 if (!values.nr && default_value) {
391 + struct key_value_info kvi = KVI_INIT;
392 struct strbuf *item;
393 +
394 + kvi_from_param(&kvi);
395 ALLOC_GROW(values.items, values.nr + 1, values.alloc);
396 item = &values.items[values.nr++];
397 strbuf_init(item, 0);
389 - if (format_config(item, key_, default_value) < 0)
398 + if (format_config(item, key_, default_value, &kvi) < 0)
399 die(_("failed to format default config value: %s"),
400 default_value);
401 }
@@ -563,15 +572,17 @@ static void check_write(void)
572 struct urlmatch_current_candidate_value {
573 char value_is_null;
574 struct strbuf value;
575 + struct key_value_info kvi;
576 };
577
578 static int urlmatch_collect_fn(const char *var, const char *value,
569 - const struct config_context *ctx UNUSED,
579 + const struct config_context *ctx,
580 void *cb)
581 {
582 struct string_list *values = cb;
583 struct string_list_item *item = string_list_insert(values, var);
584 struct urlmatch_current_candidate_value *matched = item->util;
585 + const struct key_value_info *kvi = ctx->kvi;
586
587 if (!matched) {
588 matched = xmalloc(sizeof(*matched));
@@ -580,6 +591,7 @@ static int urlmatch_collect_fn(const char *var, const char *value,
591 } else {
592 strbuf_reset(&matched->value);
593 }
594 + matched->kvi = *kvi;
595
596 if (value) {
597 strbuf_addstr(&matched->value, value);
@@ -627,7 +639,8 @@ static int get_urlmatch(const char *var, const char *url)
639 struct strbuf buf = STRBUF_INIT;
640
641 format_config(&buf, item->string,
630 - matched->value_is_null ? NULL : matched->value.buf);
642 + matched->value_is_null ? NULL : matched->value.buf,
643 + &matched->kvi);
644 fwrite(buf.buf, 1, buf.len, stdout);
645 strbuf_release(&buf);
646
config.c
+41 -39
@@ -219,7 +219,9 @@ static const char include_depth_advice[] = N_(
219 "from\n"
220 " %s\n"
221 "This might be due to circular includes.");
222 -static int handle_path_include(struct config_source *cs, const char *path,
222 +static int handle_path_include(struct config_source *cs,
223 + const struct key_value_info *kvi,
224 + const char *path,
225 struct config_include_data *inc)
226 {
227 int ret = 0;
@@ -260,8 +262,7 @@ static int handle_path_include(struct config_source *cs, const char *path,
262 cs->name ? cs->name :
263 "the command line");
264 ret = git_config_from_file_with_options(git_config_include, path, inc,
263 - current_config_scope(),
264 - NULL);
265 + kvi->scope, NULL);
266 inc->depth--;
267 }
268 cleanup:
@@ -510,7 +511,7 @@ static int git_config_include(const char *var, const char *value,
511 return ret;
512
513 if (!strcmp(var, "include.path"))
513 - ret = handle_path_include(cs, value, inc);
514 + ret = handle_path_include(cs, ctx->kvi, value, inc);
515
516 if (!parse_config_key(var, "includeif", &cond, &cond_len, &key) &&
517 cond && include_condition_is_true(cs, inc, cond, cond_len) &&
@@ -519,7 +520,7 @@ static int git_config_include(const char *var, const char *value,
520
521 if (inc->opts->unconditional_remote_url)
522 inc->fn = forbid_remote_url;
522 - ret = handle_path_include(cs, value, inc);
523 + ret = handle_path_include(cs, ctx->kvi, value, inc);
524 inc->fn = old_fn;
525 }
526
@@ -677,27 +678,44 @@ out_free_ret_1:
678 }
679
680 static int config_parse_pair(const char *key, const char *value,
680 - config_fn_t fn, void *data)
681 + struct key_value_info *kvi,
682 + config_fn_t fn, void *data)
683 {
684 char *canonical_name;
685 int ret;
686 + struct config_context ctx = {
687 + .kvi = kvi,
688 + };
689
690 if (!strlen(key))
691 return error(_("empty config key"));
692 if (git_config_parse_key(key, &canonical_name, NULL))
693 return -1;
694
690 - ret = (fn(canonical_name, value, NULL, data) < 0) ? -1 : 0;
695 + ret = (fn(canonical_name, value, &ctx, data) < 0) ? -1 : 0;
696 free(canonical_name);
697 return ret;
698 }
699
700 +
701 +/* for values read from `git_config_from_parameters()` */
702 +void kvi_from_param(struct key_value_info *out)
703 +{
704 + out->filename = NULL;
705 + out->linenr = -1;
706 + out->origin_type = CONFIG_ORIGIN_CMDLINE;
707 + out->scope = CONFIG_SCOPE_COMMAND;
708 +}
709 +
710 int git_config_parse_parameter(const char *text,
711 config_fn_t fn, void *data)
712 {
713 const char *value;
714 struct strbuf **pair;
715 int ret;
716 + struct key_value_info kvi = KVI_INIT;
717 +
718 + kvi_from_param(&kvi);
719
720 pair = strbuf_split_str(text, '=', 2);
721 if (!pair[0])
@@ -716,12 +734,13 @@ int git_config_parse_parameter(const char *text,
734 return error(_("bogus config parameter: %s"), text);
735 }
736
719 - ret = config_parse_pair(pair[0]->buf, value, fn, data);
737 + ret = config_parse_pair(pair[0]->buf, value, &kvi, fn, data);
738 strbuf_list_free(pair);
739 return ret;
740 }
741
724 -static int parse_config_env_list(char *env, config_fn_t fn, void *data)
742 +static int parse_config_env_list(char *env, struct key_value_info *kvi,
743 + config_fn_t fn, void *data)
744 {
745 char *cur = env;
746 while (cur && *cur) {
@@ -755,7 +774,7 @@ static int parse_config_env_list(char *env, config_fn_t fn, void *data)
774 CONFIG_DATA_ENVIRONMENT);
775 }
776
758 - if (config_parse_pair(key, value, fn, data) < 0)
777 + if (config_parse_pair(key, value, kvi, fn, data) < 0)
778 return -1;
779 }
780 else {
@@ -780,10 +799,13 @@ int git_config_from_parameters(config_fn_t fn, void *data)
799 int ret = 0;
800 char *envw = NULL;
801 struct config_source source = CONFIG_SOURCE_INIT;
802 + struct key_value_info kvi = KVI_INIT;
803
804 source.origin_type = CONFIG_ORIGIN_CMDLINE;
805 config_reader_push_source(&the_reader, &source);
806
807 + kvi_from_param(&kvi);
808 +
809 env = getenv(CONFIG_COUNT_ENVIRONMENT);
810 if (env) {
811 unsigned long count;
@@ -819,7 +841,7 @@ int git_config_from_parameters(config_fn_t fn, void *data)
841 }
842 strbuf_reset(&envvar);
843
822 - if (config_parse_pair(key, value, fn, data) < 0) {
844 + if (config_parse_pair(key, value, &kvi, fn, data) < 0) {
845 ret = -1;
846 goto out;
847 }
@@ -830,7 +852,7 @@ int git_config_from_parameters(config_fn_t fn, void *data)
852 if (env) {
853 /* sq_dequote will write over it */
854 envw = xstrdup(env);
833 - if (parse_config_env_list(envw, fn, data) < 0) {
855 + if (parse_config_env_list(envw, &kvi, fn, data) < 0) {
856 ret = -1;
857 goto out;
858 }
@@ -2442,7 +2464,8 @@ static int configset_find_element(struct config_set *set, const char *key,
2464 return 0;
2465 }
2466
2445 -static int configset_add_value(struct config_reader *reader,
2467 +static int configset_add_value(const struct key_value_info *kvi_p,
2468 + struct config_reader *reader,
2469 struct config_set *set, const char *key,
2470 const char *value)
2471 {
@@ -2476,13 +2499,9 @@ static int configset_add_value(struct config_reader *reader,
2499 if (!reader->source)
2500 BUG("configset_add_value has no source");
2501 if (reader->source->name) {
2479 - kvi_from_source(reader->source, current_config_scope(), kv_info);
2502 + kvi_from_source(reader->source, kvi_p->scope, kv_info);
2503 } else {
2481 - /* for values read from `git_config_from_parameters()` */
2482 - kv_info->filename = NULL;
2483 - kv_info->linenr = -1;
2484 - kv_info->origin_type = CONFIG_ORIGIN_CMDLINE;
2485 - kv_info->scope = reader->parsing_scope;
2504 + kvi_from_param(kv_info);
2505 }
2506 si->util = kv_info;
2507
@@ -2538,11 +2557,12 @@ struct configset_add_data {
2557 #define CONFIGSET_ADD_INIT { 0 }
2558
2559 static int config_set_callback(const char *key, const char *value,
2541 - const struct config_context *ctx UNUSED,
2560 + const struct config_context *ctx,
2561 void *cb)
2562 {
2563 struct configset_add_data *data = cb;
2545 - configset_add_value(data->config_reader, data->config_set, key, value);
2564 + configset_add_value(ctx->kvi, data->config_reader, data->config_set,
2565 + key, value);
2566 return 0;
2567 }
2568
@@ -4037,16 +4057,6 @@ const char *config_origin_type_name(enum config_origin_type type)
4057 }
4058 }
4059
4040 -const char *current_config_origin_type(void)
4041 -{
4042 - enum config_origin_type type = CONFIG_ORIGIN_UNKNOWN;
4043 -
4044 - if (reader_origin_type(&the_reader, &type))
4045 - BUG("current_config_origin_type called outside config callback");
4046 -
4047 - return config_origin_type_name(type);
4048 -}
4049 -
4060 const char *config_scope_name(enum config_scope scope)
4061 {
4062 switch (scope) {
@@ -4078,14 +4088,6 @@ static int reader_config_name(struct config_reader *reader, const char **out)
4088 return 0;
4089 }
4090
4081 -const char *current_config_name(void)
4082 -{
4083 - const char *name;
4084 - if (reader_config_name(&the_reader, &name))
4085 - BUG("current_config_name called outside config callback");
4086 - return name ? name : "";
4087 -}
4088 -
4091 enum config_scope current_config_scope(void)
4092 {
4093 if (the_reader.config_kvi)
config.h
+1 -2
@@ -387,9 +387,8 @@ void git_global_config(char **user, char **xdg);
387 int git_config_parse_parameter(const char *, config_fn_t fn, void *data);
388
389 enum config_scope current_config_scope(void);
390 -const char *current_config_origin_type(void);
391 -const char *current_config_name(void);
390 const char *config_origin_type_name(enum config_origin_type type);
391 +void kvi_from_param(struct key_value_info *out);
392
393 /*
394 * Match and parse a config key of the form:
t/t1300-config.sh
+27
@@ -1668,6 +1668,21 @@ test_expect_success 'urlmatch' '
1668 test_cmp expect actual
1669 '
1670
1671 +test_expect_success 'urlmatch with --show-scope' '
1672 + cat >.git/config <<-\EOF &&
1673 + [http "https://weak.example.com"]
1674 + sslVerify = false
1675 + cookieFile = /tmp/cookie.txt
1676 + EOF
1677 +
1678 + cat >expect <<-EOF &&
1679 + local http.cookiefile /tmp/cookie.txt
1680 + local http.sslverify false
1681 + EOF
1682 + git config --get-urlmatch --show-scope HTTP https://weak.example.com >actual &&
1683 + test_cmp expect actual
1684 +'
1685 +
1686 test_expect_success 'urlmatch favors more specific URLs' '
1687 cat >.git/config <<-\EOF &&
1688 [http "https://example.com/"]
@@ -2055,6 +2070,12 @@ test_expect_success '--show-origin blob ref' '
2070 test_cmp expect output
2071 '
2072
2073 +test_expect_success '--show-origin with --default' '
2074 + git config --show-origin --default foo some.key >actual &&
2075 + echo "command line: foo" >expect &&
2076 + test_cmp expect actual
2077 +'
2078 +
2079 test_expect_success '--show-scope with --list' '
2080 cat >expect <<-EOF &&
2081 global user.global=true
@@ -2123,6 +2144,12 @@ test_expect_success '--show-scope with --show-origin' '
2144 test_cmp expect output
2145 '
2146
2147 +test_expect_success '--show-scope with --default' '
2148 + git config --show-scope --default foo some.key >actual &&
2149 + echo "command foo" >expect &&
2150 + test_cmp expect actual
2151 +'
2152 +
2153 test_expect_success 'override global and system config' '
2154 test_when_finished rm -f \"\$HOME\"/.gitconfig &&
2155 cat >"$HOME"/.gitconfig <<-EOF &&