As promised, this version addresses the comments on v3.
= Changes since v4
- Squash 6-7/12 since `test_must_fail` doesn't catch BUG()
- Move a hunk to later in the series where it belongs
- Replace a memcpy with `*a = *b`
= Changes since v3
- Rebase onto newer 'master'
- Move the 'remove UNUSED from tr2_cfg_cb' hunk from 9/12 -> 8/12. It should
have been there all along; v3 8/12 didn't build at all.
Glen Choo (11):
config: inline git_color_default_config
urlmatch.h: use config_fn_t type
config: add ctx arg to config_fn_t
config.c: pass ctx in configsets
config: pass ctx with config files
config.c: pass ctx with CLI config
trace2: plumb config kvi
config: pass kvi to die_bad_number()
config.c: remove config_reader from configsets
config: add kvi.path, use it to evaluate includes
config: pass source to config_parser_event_fn_t
alias.c | 3 +-
archive-tar.c | 5 +-
archive-zip.c | 1 +
builtin/add.c | 8 +-
builtin/blame.c | 5 +-
builtin/branch.c | 8 +-
builtin/cat-file.c | 5 +-
builtin/checkout.c | 12 +-
builtin/clean.c | 9 +-
builtin/clone.c | 11 +-
builtin/column.c | 3 +-
builtin/commit-graph.c | 3 +-
builtin/commit.c | 20 +-
builtin/config.c | 72 ++-
builtin/difftool.c | 5 +-
builtin/fetch.c | 13 +-
builtin/fsmonitor--daemon.c | 11 +-
builtin/grep.c | 12 +-
builtin/help.c | 5 +-
builtin/index-pack.c | 9 +-
builtin/log.c | 12 +-
builtin/merge.c | 7 +-
builtin/multi-pack-index.c | 1 +
builtin/pack-objects.c | 19 +-
builtin/patch-id.c | 5 +-
builtin/pull.c | 5 +-
builtin/push.c | 5 +-
builtin/read-tree.c | 5 +-
builtin/rebase.c | 5 +-
builtin/receive-pack.c | 15 +-
builtin/reflog.c | 7 +-
builtin/remote.c | 15 +-
builtin/repack.c | 5 +-
builtin/reset.c | 5 +-
builtin/send-pack.c | 5 +-
builtin/show-branch.c | 8 +-
builtin/stash.c | 5 +-
builtin/submodule--helper.c | 3 +-
builtin/tag.c | 9 +-
builtin/var.c | 5 +-
builtin/worktree.c | 5 +-
bundle-uri.c | 9 +-
color.c | 8 -
color.h | 6 +-
compat/mingw.c | 3 +-
compat/mingw.h | 4 +-
config.c | 552 +++++++-----------
config.h | 80 ++-
connect.c | 4 +-
.../coccinelle/config_fn_ctx.pending.cocci | 144 +++++
contrib/coccinelle/git_config_number.cocci | 27 +
convert.c | 4 +-
credential.c | 1 +
delta-islands.c | 4 +-
diff.c | 19 +-
diff.h | 7 +-
fetch-pack.c | 5 +-
fmt-merge-msg.c | 7 +-
fmt-merge-msg.h | 3 +-
fsck.c | 12 +-
fsck.h | 4 +-
git-compat-util.h | 2 +
gpg-interface.c | 7 +-
grep.c | 7 +-
grep.h | 4 +-
help.c | 9 +-
http.c | 15 +-
ident.c | 4 +-
ident.h | 4 +-
imap-send.c | 7 +-
ll-merge.c | 1 +
ls-refs.c | 1 +
mailinfo.c | 5 +-
notes-utils.c | 4 +-
notes.c | 4 +-
pager.c | 5 +-
pretty.c | 1 +
promisor-remote.c | 4 +-
remote.c | 8 +-
revision.c | 4 +-
scalar.c | 4 +-
sequencer.c | 29 +-
setup.c | 18 +-
submodule-config.c | 31 +-
submodule-config.h | 3 +-
t/helper/test-config.c | 24 +-
t/helper/test-userdiff.c | 4 +-
t/t1300-config.sh | 27 +
trace2.c | 4 +-
trace2.h | 3 +-
trace2/tr2_cfg.c | 16 +-
trace2/tr2_sysenv.c | 3 +-
trace2/tr2_tgt.h | 4 +-
trace2/tr2_tgt_event.c | 4 +-
trace2/tr2_tgt_normal.c | 4 +-
trace2/tr2_tgt_perf.c | 4 +-
trailer.c | 2 +
upload-pack.c | 18 +-
urlmatch.c | 7 +-
urlmatch.h | 8 +-
worktree.c | 2 +-
xdiff-interface.c | 5 +-
xdiff-interface.h | 4 +-
103 files changed, 960 insertions(+), 638 deletions(-)
create mode 100644 contrib/coccinelle/config_fn_ctx.pending.cocci
create mode 100644 contrib/coccinelle/git_config_number.cocci
base-commit: 6ff334181cfb6485d3ba50843038209a2a253907
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1497%2Fchooglen%2Fconfig%2Fno-global-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1497/chooglen/config/no-global-v5
Pull-Request: https://github.com/git/git/pull/1497
Range-diff vs v4:
1: 7bfffb454c5 = 1: 7bfffb454c5 config: inline git_color_default_config
2: 739c519ce62 = 2: 739c519ce62 urlmatch.h: use config_fn_t type
3: a9a0a50f32a = 3: a9a0a50f32a config: add ctx arg to config_fn_t
4: 39b2e291f86 = 4: 39b2e291f86 config.c: pass ctx in configsets
5: bfc6d2833c5 = 5: bfc6d2833c5 config: pass ctx with config files
6: 897bdc759b5 < -: ----------- builtin/config.c: test misuse of format_config()
7: 33e4437737d ! 6: 7b24eefbcf3 config.c: pass ctx with CLI config
@@ Commit message
* git_config_parse_parameter() hasn't been setting config source
information, so plumb "kvi" there too.
- * "git config --get-urlmatch --show-scope" iterates config to collect
- values, but then attempts to display the scope after config iteration.
- 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" (we don't allow this combination of args
- because of this bug), but that is left unchanged for now.
+ * 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 --default" doesn't have config source metadata when
- displaying the default value. 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().
+ * "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@xxxxxxxxxxxxxxxxxxxxx/
Signed-off-by: Glen Choo <chooglen@xxxxxxxxxx>
@@ config.c: static int configset_find_element(struct config_set *set, const char *
const char *value)
{
@@ config.c: static int configset_add_value(struct config_reader *reader,
- l_item->e = e;
- l_item->value_index = e->value_list.nr - 1;
-
-- if (!reader->source)
-- BUG("configset_add_value has no source");
+ if (!reader->source)
+ BUG("configset_add_value has no source");
if (reader->source->name) {
- kvi_from_source(reader->source, current_config_scope(), kv_info);
+ kvi_from_source(reader->source, kvi_p->scope, kv_info);
@@ config.h: void git_global_config(char **user, char **xdg);
* Match and parse a config key of the form:
## t/t1300-config.sh ##
-@@ t/t1300-config.sh: test_expect_success 'urlmatch with --show-scope' '
- EOF
+@@ t/t1300-config.sh: test_expect_success 'urlmatch' '
+ test_cmp expect actual
+ '
- cat >expect <<-EOF &&
-- unknown http.cookiefile /tmp/cookie.txt
-- unknown http.sslverify false
++test_expect_success 'urlmatch with --show-scope' '
++ cat >.git/config <<-\EOF &&
++ [http "https://weak.example.com";]
++ sslVerify = false
++ cookieFile = /tmp/cookie.txt
++ EOF
++
++ cat >expect <<-EOF &&
+ local http.cookiefile /tmp/cookie.txt
+ local http.sslverify false
- EOF
- git config --get-urlmatch --show-scope HTTP https://weak.example.com >actual &&
- test_cmp expect actual
++ EOF
++ git config --get-urlmatch --show-scope HTTP https://weak.example.com >actual &&
++ test_cmp expect actual
++'
++
+ test_expect_success 'urlmatch favors more specific URLs' '
+ cat >.git/config <<-\EOF &&
+ [http "https://example.com/";]
@@ t/t1300-config.sh: test_expect_success '--show-origin blob ref' '
+ test_cmp expect output
'
- test_expect_success '--show-origin with --default' '
-- test_must_fail git config --show-origin --default foo some.key
++test_expect_success '--show-origin with --default' '
+ git config --show-origin --default foo some.key >actual &&
+ echo "command line: foo" >expect &&
+ test_cmp expect actual
- '
-
++'
++
test_expect_success '--show-scope with --list' '
+ cat >expect <<-EOF &&
+ global user.global=true
@@ t/t1300-config.sh: test_expect_success '--show-scope with --show-origin' '
-
- test_expect_success '--show-scope with --default' '
- git config --show-scope --default foo some.key >actual &&
-- echo "unknown foo" >expect &&
-+ echo "command foo" >expect &&
- test_cmp expect actual
+ test_cmp expect output
'
++test_expect_success '--show-scope with --default' '
++ git config --show-scope --default foo some.key >actual &&
++ echo "command foo" >expect &&
++ test_cmp expect actual
++'
++
+ test_expect_success 'override global and system config' '
+ test_when_finished rm -f \"\$HOME\"/.gitconfig &&
+ cat >"$HOME"/.gitconfig <<-EOF &&
8: 9bd5f60282c = 7: 7d64dcbdade trace2: plumb config kvi
9: 114723ee4a7 = 8: 9e71c10ca0a config: pass kvi to die_bad_number()
10: 807057b6d7f ! 9: 4776600e790 config.c: remove config_reader from configsets
@@ config.c: static int configset_add_value(const struct key_value_info *kvi_p,
l_item->e = e;
l_item->value_index = e->value_list.nr - 1;
+- if (!reader->source)
+- BUG("configset_add_value has no source");
- if (reader->source->name) {
- kvi_from_source(reader->source, kvi_p->scope, kv_info);
- } else {
- kvi_from_param(kv_info);
- }
-+ memcpy(kv_info, kvi_p, sizeof(struct key_value_info));
++ *kv_info = *kvi_p;
si->util = kv_info;
return 0;
11: 3f0f84df972 = 10: 2b33977aba6 config: add kvi.path, use it to evaluate includes
12: fe2f154fe8b = 11: 8347d3c9b80 config: pass source to config_parser_event_fn_t
