{"thread":{"id":"49077","subject":"[PATCH 0/3] Resending sb/config-write-fix","startedAt":"2018-08-08T19:50:33Z","lastAt":"2018-08-08T20:28:05Z","messageCount":5,"participants":["Stefan Beller","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"354920","messageId":"20180808195020.37374-1-sbeller@google.com","threadId":"49077","inReplyTo":null,"subject":"[PATCH 0/3] Resending sb/config-write-fix","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-08T19:50:17Z","receivedAt":"2018-08-08T19:50:33Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This is a resend of sb/config-write-fix, with a slightly\nbetter commit message and a renamed variable.\n\nThanks,\nStefan\n\n\nStefan Beller (3):\n  t1300: document current behavior of setting options\n  config: fix case sensitive subsection names on writing\n  git-config: document accidental multi-line setting in deprecated\n    syntax\n\n Documentation/git-config.txt | 21 +++++++++\n config.c                     | 12 ++++-\n t/t1300-config.sh            | 87 ++++++++++++++++++++++++++++++++++++\n 3 files changed, 119 insertions(+), 1 deletion(-)\n\n./git-range-diff origin/sb/config-write-fix...HEAD >>0000-cover-letter.patch \n2.18.0.597.ga71716f1ad-goog\n\n1:  999d9026272 ! 1:  e40f57f3da1 t1300: document current behavior of setting options\n    @@ -7,7 +7,6 @@\n         for the follow up that will fix some issues with the current behavior.\n     \n         Signed-off-by: Stefan Beller <sbeller@google.com>\n    -    Signed-off-by: Junio C Hamano <gitster@pobox.com>\n     \n      diff --git a/t/t1300-config.sh b/t/t1300-config.sh\n      --- a/t/t1300-config.sh\n2:  c667e555066 ! 2:  f01cb1d9dae config: fix case sensitive subsection names on writing\n    @@ -2,8 +2,8 @@\n     \n         config: fix case sensitive subsection names on writing\n     \n    -    A use reported a submodule issue regarding strange case indentation\n    -    issues, but it could be boiled down to the following test case:\n    +    A user reported a submodule issue regarding a section mix-up,\n    +    but it could be boiled down to the following test case:\n     \n           $ git init test  && cd test\n           $ git config foo.\"Bar\".key test\n    @@ -32,7 +32,6 @@\n     \n         Reported-by: JP Sugarbroad <jpsugar@google.com>\n         Signed-off-by: Stefan Beller <sbeller@google.com>\n    -    Signed-off-by: Junio C Hamano <gitster@pobox.com>\n     \n      diff --git a/config.c b/config.c\n      --- a/config.c\n    @@ -41,7 +40,7 @@\n      \tint eof;\n      \tstruct strbuf value;\n      \tstruct strbuf var;\n    -+\tunsigned section_name_old_dot_style : 1;\n    ++\tunsigned subsection_case_sensitive : 1;\n      \n      \tint (*do_fgetc)(struct config_source *c);\n      \tint (*do_ungetc)(int c, struct config_source *conf);\n    @@ -49,7 +48,7 @@\n      \n      static int get_extended_base_var(struct strbuf *name, int c)\n      {\n    -+\tcf->section_name_old_dot_style = 0;\n    ++\tcf->subsection_case_sensitive = 0;\n      \tdo {\n      \t\tif (c == '\\n')\n      \t\t\tgoto error_incomplete_line;\n    @@ -57,7 +56,7 @@\n      \n      static int get_base_var(struct strbuf *name)\n      {\n    -+\tcf->section_name_old_dot_style = 1;\n    ++\tcf->subsection_case_sensitive = 1;\n      \tfor (;;) {\n      \t\tint c = get_next_char();\n      \t\tif (cf->eof)\n    @@ -70,7 +69,7 @@\n      \t\tif (cf->var.len < 2 || cf->var.buf[cf->var.len - 1] != '.')\n      \t\t\treturn error(\"invalid section name '%s'\", cf->var.buf);\n      \n    -+\t\tif (cf->section_name_old_dot_style)\n    ++\t\tif (cf->subsection_case_sensitive)\n     +\t\t\tcmpfn = strncasecmp;\n     +\t\telse\n     +\t\t\tcmpfn = strncmp;\n3:  6749bb283a8 ! 3:  6b5ad773490 git-config: document accidental multi-line setting in deprecated syntax\n    @@ -29,7 +29,6 @@\n         spend time on fixing the behavior and just document it instead.\n     \n         Signed-off-by: Stefan Beller <sbeller@google.com>\n    -    Signed-off-by: Junio C Hamano <gitster@pobox.com>\n     \n      diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt\n      --- a/Documentation/git-config.txt\n"},{"id":"354921","messageId":"20180808195020.37374-2-sbeller@google.com","threadId":"49077","inReplyTo":"20180808195020.37374-1-sbeller@google.com","subject":"[PATCH 1/3] t1300: document current behavior of setting options","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-08T19:50:18Z","receivedAt":"2018-08-08T19:50:35Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This documents current behavior of the config machinery, when changing\nthe value of some settings. This patch just serves to provide a baseline\nfor the follow up that will fix some issues with the current behavior.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n t/t1300-config.sh | 86 +++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 86 insertions(+)\n\ndiff --git a/t/t1300-config.sh b/t/t1300-config.sh\nindex e43982a9c1f..c15c19bf78d 100755\n--- a/t/t1300-config.sh\n+++ b/t/t1300-config.sh\n@@ -1188,6 +1188,92 @@ test_expect_success 'last one wins: three level vars' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'old-fashioned settings are case insensitive' '\n+\ttest_when_finished \"rm -f testConfig testConfig_expect testConfig_actual\" &&\n+\n+\tcat >testConfig_actual <<-EOF &&\n+\t\t[V.A]\n+\t\tr = value1\n+\tEOF\n+\tq_to_tab >testConfig_expect <<-EOF &&\n+\t\t[V.A]\n+\t\tQr = value2\n+\tEOF\n+\tgit config -f testConfig_actual \"v.a.r\" value2 &&\n+\ttest_cmp testConfig_expect testConfig_actual &&\n+\n+\tcat >testConfig_actual <<-EOF &&\n+\t\t[V.A]\n+\t\tr = value1\n+\tEOF\n+\tq_to_tab >testConfig_expect <<-EOF &&\n+\t\t[V.A]\n+\t\tQR = value2\n+\tEOF\n+\tgit config -f testConfig_actual \"V.a.R\" value2 &&\n+\ttest_cmp testConfig_expect testConfig_actual &&\n+\n+\tcat >testConfig_actual <<-EOF &&\n+\t\t[V.A]\n+\t\tr = value1\n+\tEOF\n+\tq_to_tab >testConfig_expect <<-EOF &&\n+\t\t[V.A]\n+\t\tr = value1\n+\t\tQr = value2\n+\tEOF\n+\tgit config -f testConfig_actual \"V.A.r\" value2 &&\n+\ttest_cmp testConfig_expect testConfig_actual &&\n+\n+\tcat >testConfig_actual <<-EOF &&\n+\t\t[V.A]\n+\t\tr = value1\n+\tEOF\n+\tq_to_tab >testConfig_expect <<-EOF &&\n+\t\t[V.A]\n+\t\tr = value1\n+\t\tQr = value2\n+\tEOF\n+\tgit config -f testConfig_actual \"v.A.r\" value2 &&\n+\ttest_cmp testConfig_expect testConfig_actual\n+'\n+\n+test_expect_success 'setting different case sensitive subsections ' '\n+\ttest_when_finished \"rm -f testConfig testConfig_expect testConfig_actual\" &&\n+\n+\tcat >testConfig_actual <<-EOF &&\n+\t\t[V \"A\"]\n+\t\tR = v1\n+\t\t[K \"E\"]\n+\t\tY = v1\n+\t\t[a \"b\"]\n+\t\tc = v1\n+\t\t[d \"e\"]\n+\t\tf = v1\n+\tEOF\n+\tq_to_tab >testConfig_expect <<-EOF &&\n+\t\t[V \"A\"]\n+\t\tQr = v2\n+\t\t[K \"E\"]\n+\t\tQy = v2\n+\t\t[a \"b\"]\n+\t\tQc = v2\n+\t\t[d \"e\"]\n+\t\tf = v1\n+\t\tQf = v2\n+\tEOF\n+\t# exact match\n+\tgit config -f testConfig_actual a.b.c v2 &&\n+\t# match section and subsection, key is cased differently.\n+\tgit config -f testConfig_actual K.E.y v2 &&\n+\t# section and key are matched case insensitive, but subsection needs\n+\t# to match; When writing out new values only the key is adjusted\n+\tgit config -f testConfig_actual v.A.r v2 &&\n+\t# subsection is not matched:\n+\tgit config -f testConfig_actual d.E.f v2 &&\n+\ttest_cmp testConfig_expect testConfig_actual\n+'\n+\n for VAR in a .a a. a.0b a.\"b c\". a.\"b c\".0d\n do\n \ttest_expect_success \"git -c $VAR=VAL rejects invalid '$VAR'\" '\n-- \n2.18.0.597.ga71716f1ad-goog\n\n"},{"id":"354922","messageId":"20180808195020.37374-3-sbeller@google.com","threadId":"49077","inReplyTo":"20180808195020.37374-1-sbeller@google.com","subject":"[PATCH 2/3] config: fix case sensitive subsection names on writing","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-08T19:50:19Z","receivedAt":"2018-08-08T19:50:37Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"A user reported a submodule issue regarding a section mix-up,\nbut it could be boiled down to the following test case:\n\n  $ git init test  && cd test\n  $ git config foo.\"Bar\".key test\n  $ git config foo.\"bar\".key test\n  $ tail -n 3 .git/config\n  [foo \"Bar\"]\n        key = test\n        key = test\n\nSub sections are case sensitive and we have a test for correctly reading\nthem. However we do not have a test for writing out config correctly with\ncase sensitive subsection names, which is why this went unnoticed in\n6ae996f2acf (git_config_set: make use of the config parser's event\nstream, 2018-04-09)\n\nUnfortunately we have to make a distinction between old style configuration\nthat looks like\n\n  [foo.Bar]\n        key = test\n\nand the new quoted style as seen above. The old style is documented as\ncase-agnostic, hence we need to keep 'strncasecmp'; although the\nresulting setting for the old style config differs from the configuration.\nThat will be fixed in a follow up patch.\n\nReported-by: JP Sugarbroad <jpsugar@google.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n config.c          | 12 +++++++++++-\n t/t1300-config.sh |  1 +\n 2 files changed, 12 insertions(+), 1 deletion(-)\n\ndiff --git a/config.c b/config.c\nindex 02050529788..27e800c7ce8 100644\n--- a/config.c\n+++ b/config.c\n@@ -35,6 +35,7 @@ struct config_source {\n \tint eof;\n \tstruct strbuf value;\n \tstruct strbuf var;\n+\tunsigned subsection_case_sensitive : 1;\n \n \tint (*do_fgetc)(struct config_source *c);\n \tint (*do_ungetc)(int c, struct config_source *conf);\n@@ -603,6 +604,7 @@ static int get_value(config_fn_t fn, void *data, struct strbuf *name)\n \n static int get_extended_base_var(struct strbuf *name, int c)\n {\n+\tcf->subsection_case_sensitive = 0;\n \tdo {\n \t\tif (c == '\\n')\n \t\t\tgoto error_incomplete_line;\n@@ -639,6 +641,7 @@ static int get_extended_base_var(struct strbuf *name, int c)\n \n static int get_base_var(struct strbuf *name)\n {\n+\tcf->subsection_case_sensitive = 1;\n \tfor (;;) {\n \t\tint c = get_next_char();\n \t\tif (cf->eof)\n@@ -2328,14 +2331,21 @@ static int store_aux_event(enum config_event_t type,\n \tstore->parsed[store->parsed_nr].type = type;\n \n \tif (type == CONFIG_EVENT_SECTION) {\n+\t\tint (*cmpfn)(const char *, const char *, size_t);\n+\n \t\tif (cf->var.len < 2 || cf->var.buf[cf->var.len - 1] != '.')\n \t\t\treturn error(\"invalid section name '%s'\", cf->var.buf);\n \n+\t\tif (cf->subsection_case_sensitive)\n+\t\t\tcmpfn = strncasecmp;\n+\t\telse\n+\t\t\tcmpfn = strncmp;\n+\n \t\t/* Is this the section we were looking for? */\n \t\tstore->is_keys_section =\n \t\t\tstore->parsed[store->parsed_nr].is_keys_section =\n \t\t\tcf->var.len - 1 == store->baselen &&\n-\t\t\t!strncasecmp(cf->var.buf, store->key, store->baselen);\n+\t\t\t!cmpfn(cf->var.buf, store->key, store->baselen);\n \t\tif (store->is_keys_section) {\n \t\t\tstore->section_seen = 1;\n \t\t\tALLOC_GROW(store->seen, store->seen_nr + 1,\ndiff --git a/t/t1300-config.sh b/t/t1300-config.sh\nindex c15c19bf78d..77c5899d000 100755\n--- a/t/t1300-config.sh\n+++ b/t/t1300-config.sh\n@@ -1260,6 +1260,7 @@ test_expect_success 'setting different case sensitive subsections ' '\n \t\tQc = v2\n \t\t[d \"e\"]\n \t\tf = v1\n+\t\t[d \"E\"]\n \t\tQf = v2\n \tEOF\n \t# exact match\n-- \n2.18.0.597.ga71716f1ad-goog\n\n"},{"id":"354923","messageId":"20180808195020.37374-4-sbeller@google.com","threadId":"49077","inReplyTo":"20180808195020.37374-1-sbeller@google.com","subject":"[PATCH 3/3] git-config: document accidental multi-line setting in deprecated syntax","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-08T19:50:20Z","receivedAt":"2018-08-08T19:50:40Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The bug was noticed when writing the previous patch; a fix for this bug\nis not easy though: If we choose to ignore the case of the subsection\n(and revert most of the code of the previous patch, just keeping\ns/strncasecmp/strcmp/), then we'd introduce new sections using the\nnew syntax, such that\n\n --------\n   [section.subsection]\n     key = value1\n --------\n\n  git config section.Subsection.key value2\n\nwould result in\n\n --------\n   [section.subsection]\n     key = value1\n   [section.Subsection]\n     key = value2\n --------\n\nwhich is even more confusing. A proper fix would replace the first\noccurrence of 'key'. As the syntax is deprecated, let's prefer to not\nspend time on fixing the behavior and just document it instead.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n Documentation/git-config.txt | 21 +++++++++++++++++++++\n 1 file changed, 21 insertions(+)\n\ndiff --git a/Documentation/git-config.txt b/Documentation/git-config.txt\nindex 14da5fc157e..1ac2eab9482 100644\n--- a/Documentation/git-config.txt\n+++ b/Documentation/git-config.txt\n@@ -430,6 +430,27 @@ http.sslverify false\n \n include::config.txt[]\n \n+BUGS\n+----\n+When using the deprecated `[section.subsection]` syntax, changing a value\n+will result in adding a multi-line key instead of a change, if the subsection\n+is given with at least one uppercase character. For example when the config\n+looks like\n+\n+--------\n+  [section.subsection]\n+    key = value1\n+--------\n+\n+and running `git config section.Subsection.key value2` will result in\n+\n+--------\n+  [section.subsection]\n+    key = value1\n+    key = value2\n+--------\n+\n+\n GIT\n ---\n Part of the linkgit:git[1] suite\n-- \n2.18.0.597.ga71716f1ad-goog\n\n"},{"id":"354926","messageId":"xmqq600kfvse.fsf@gitster-ct.c.googlers.com","threadId":"49077","inReplyTo":"20180808195020.37374-1-sbeller@google.com","subject":"Re: [PATCH 0/3] Resending sb/config-write-fix","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-08-08T20:28:01Z","receivedAt":"2018-08-08T20:28:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> This is a resend of sb/config-write-fix, with a slightly\n> better commit message and a renamed variable.\n>\n> Thanks,\n> Stefan\n\nThanks.  Let's declare victory and mark it to be merged to 'next'.\n"}]}