From 999d90262727a8e7a25a40dd71073040a2605437 Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Wed, 1 Aug 2018 12:34:11 -0700 Subject: [PATCH 1/3] t1300: document current behavior of setting options This documents current behavior of the config machinery, when changing the value of some settings. This patch just serves to provide a baseline for the follow up that will fix some issues with the current behavior. Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- t/t1300-config.sh | 86 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/t/t1300-config.sh b/t/t1300-config.sh index e43982a9c1..c15c19bf78 100755 --- a/t/t1300-config.sh +++ b/t/t1300-config.sh @@ -1188,6 +1188,92 @@ test_expect_success 'last one wins: three level vars' ' test_cmp expect actual ' +test_expect_success 'old-fashioned settings are case insensitive' ' + test_when_finished "rm -f testConfig testConfig_expect testConfig_actual" && + + cat >testConfig_actual <<-EOF && + [V.A] + r = value1 + EOF + q_to_tab >testConfig_expect <<-EOF && + [V.A] + Qr = value2 + EOF + git config -f testConfig_actual "v.a.r" value2 && + test_cmp testConfig_expect testConfig_actual && + + cat >testConfig_actual <<-EOF && + [V.A] + r = value1 + EOF + q_to_tab >testConfig_expect <<-EOF && + [V.A] + QR = value2 + EOF + git config -f testConfig_actual "V.a.R" value2 && + test_cmp testConfig_expect testConfig_actual && + + cat >testConfig_actual <<-EOF && + [V.A] + r = value1 + EOF + q_to_tab >testConfig_expect <<-EOF && + [V.A] + r = value1 + Qr = value2 + EOF + git config -f testConfig_actual "V.A.r" value2 && + test_cmp testConfig_expect testConfig_actual && + + cat >testConfig_actual <<-EOF && + [V.A] + r = value1 + EOF + q_to_tab >testConfig_expect <<-EOF && + [V.A] + r = value1 + Qr = value2 + EOF + git config -f testConfig_actual "v.A.r" value2 && + test_cmp testConfig_expect testConfig_actual +' + +test_expect_success 'setting different case sensitive subsections ' ' + test_when_finished "rm -f testConfig testConfig_expect testConfig_actual" && + + cat >testConfig_actual <<-EOF && + [V "A"] + R = v1 + [K "E"] + Y = v1 + [a "b"] + c = v1 + [d "e"] + f = v1 + EOF + q_to_tab >testConfig_expect <<-EOF && + [V "A"] + Qr = v2 + [K "E"] + Qy = v2 + [a "b"] + Qc = v2 + [d "e"] + f = v1 + Qf = v2 + EOF + # exact match + git config -f testConfig_actual a.b.c v2 && + # match section and subsection, key is cased differently. + git config -f testConfig_actual K.E.y v2 && + # section and key are matched case insensitive, but subsection needs + # to match; When writing out new values only the key is adjusted + git config -f testConfig_actual v.A.r v2 && + # subsection is not matched: + git config -f testConfig_actual d.E.f v2 && + test_cmp testConfig_expect testConfig_actual +' + for VAR in a .a a. a.0b a."b c". a."b c".0d do test_expect_success "git -c $VAR=VAL rejects invalid '$VAR'" ' From 2d84f13dcb67762c2506e71232556e196f32600b Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Wed, 8 Aug 2018 12:50:19 -0700 Subject: [PATCH 2/3] config: fix case sensitive subsection names on writing A user reported a submodule issue regarding a section mix-up, but it could be boiled down to the following test case: $ git init test && cd test $ git config foo."Bar".key test $ git config foo."bar".key test $ tail -n 3 .git/config [foo "Bar"] key = test key = test Sub sections are case sensitive and we have a test for correctly reading them. However we do not have a test for writing out config correctly with case sensitive subsection names, which is why this went unnoticed in 6ae996f2acf (git_config_set: make use of the config parser's event stream, 2018-04-09) Unfortunately we have to make a distinction between old style configuration that looks like [foo.Bar] key = test and the new quoted style as seen above. The old style is documented as case-agnostic, hence we need to keep 'strncasecmp'; although the resulting setting for the old style config differs from the configuration. That will be fixed in a follow up patch. Reported-by: JP Sugarbroad Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- config.c | 12 +++++++++++- t/t1300-config.sh | 1 + 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/config.c b/config.c index 0205052978..27e800c7ce 100644 --- a/config.c +++ b/config.c @@ -35,6 +35,7 @@ struct config_source { int eof; struct strbuf value; struct strbuf var; + unsigned subsection_case_sensitive : 1; int (*do_fgetc)(struct config_source *c); int (*do_ungetc)(int c, struct config_source *conf); @@ -603,6 +604,7 @@ static int get_value(config_fn_t fn, void *data, struct strbuf *name) static int get_extended_base_var(struct strbuf *name, int c) { + cf->subsection_case_sensitive = 0; do { if (c == '\n') goto error_incomplete_line; @@ -639,6 +641,7 @@ error_incomplete_line: static int get_base_var(struct strbuf *name) { + cf->subsection_case_sensitive = 1; for (;;) { int c = get_next_char(); if (cf->eof) @@ -2328,14 +2331,21 @@ static int store_aux_event(enum config_event_t type, store->parsed[store->parsed_nr].type = type; if (type == CONFIG_EVENT_SECTION) { + int (*cmpfn)(const char *, const char *, size_t); + if (cf->var.len < 2 || cf->var.buf[cf->var.len - 1] != '.') return error("invalid section name '%s'", cf->var.buf); + if (cf->subsection_case_sensitive) + cmpfn = strncasecmp; + else + cmpfn = strncmp; + /* Is this the section we were looking for? */ store->is_keys_section = store->parsed[store->parsed_nr].is_keys_section = cf->var.len - 1 == store->baselen && - !strncasecmp(cf->var.buf, store->key, store->baselen); + !cmpfn(cf->var.buf, store->key, store->baselen); if (store->is_keys_section) { store->section_seen = 1; ALLOC_GROW(store->seen, store->seen_nr + 1, diff --git a/t/t1300-config.sh b/t/t1300-config.sh index c15c19bf78..77c5899d00 100755 --- a/t/t1300-config.sh +++ b/t/t1300-config.sh @@ -1260,6 +1260,7 @@ test_expect_success 'setting different case sensitive subsections ' ' Qc = v2 [d "e"] f = v1 + [d "E"] Qf = v2 EOF # exact match From bff7df7a87a775d931dc40e6f7058966f53f1fc8 Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Wed, 8 Aug 2018 12:50:20 -0700 Subject: [PATCH 3/3] git-config: document accidental multi-line setting in deprecated syntax The bug was noticed when writing the previous patch; a fix for this bug is not easy though: If we choose to ignore the case of the subsection (and revert most of the code of the previous patch, just keeping s/strncasecmp/strcmp/), then we'd introduce new sections using the new syntax, such that -------- [section.subsection] key = value1 -------- git config section.Subsection.key value2 would result in -------- [section.subsection] key = value1 [section.Subsection] key = value2 -------- which is even more confusing. A proper fix would replace the first occurrence of 'key'. As the syntax is deprecated, let's prefer to not spend time on fixing the behavior and just document it instead. Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- Documentation/git-config.txt | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt index 14da5fc157..1ac2eab948 100644 --- a/Documentation/git-config.txt +++ b/Documentation/git-config.txt @@ -430,6 +430,27 @@ http.sslverify false include::config.txt[] +BUGS +---- +When using the deprecated `[section.subsection]` syntax, changing a value +will result in adding a multi-line key instead of a change, if the subsection +is given with at least one uppercase character. For example when the config +looks like + +-------- + [section.subsection] + key = value1 +-------- + +and running `git config section.Subsection.key value2` will result in + +-------- + [section.subsection] + key = value1 + key = value2 +-------- + + GIT --- Part of the linkgit:git[1] suite