{"thread":{"id":"45200","subject":"[PATCH v5 1/1] config: add conditional include","startedAt":"2017-02-23T12:24:08Z","lastAt":"2017-03-07T18:48:40Z","messageCount":39,"participants":["Nguyễn Thái Ngọc Duy","Junio C Hamano","Duy Nguyen","Ramsay Jones","Philip Oakley","Jeff King","Stefan Beller"],"isPatch":true,"patchVersion":5,"patchTotal":1},"messages":[{"id":"312390","messageId":"20170223122346.12222-2-pclouds@gmail.com","threadId":"45200","inReplyTo":"20170223122346.12222-1-pclouds@gmail.com","subject":"[PATCH v5 1/1] config: add conditional include","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-02-23T12:23:46Z","receivedAt":"2017-02-23T12:24:08Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This allows some more flexibility in managing configuration across\nrepositories. The most often seen use case on the mailing list is when\nthe user needs to use different email addresses on different\nrepositories. If these repositories share something that we can use to\ngroup them up, then we can set the same configuration for groups\nautomatically.\n\nIn this patch, the only supported grouping is based on $GIT_DIR (in\nabsolute path), so you would need to group repositories by directory, or\nsomething like that to take advantage of it.\n\nWe already have include.path for unconditional includes. This patch goes\nwith include-if.xxx.path to make it clearer that a condition is\nrequired.\n\nSimilar to include.path, older git versions that don't understand\ninclude-if will simply ignore them.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n Documentation/config.txt | 54 +++++++++++++++++++++++++\n config.c | 100 ++++++++++++++++++++++++++++++++++++++++++++++\n t/t1305-config-include.sh | 56 ++++++++++++++++++++++++++\n 3 files changed, 210 insertions(+)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 015346c417..8cadf2b776 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -91,6 +91,49 @@ found at the location of the include directive. If the value of the\n relative to the configuration file in which the include directive was\n found. See below for examples.\n \n+Conditional includes\n+~~~~~~~~~~~~~~~~~~~~\n+\n+You can include one config file from another conditionally by setting\n+a special `include-if..path` variable to the name of the\n+file to be included. The variable is treated the same way as\n+`include.path`.\n+\n+The condition starts with a keyword, followed by a colon and a\n+pattern. The interpretation of the pattern depends on the keyword.\n+Supported keywords are:\n+\n+`gitdir`::\n+\tThe environment variable `GIT_DIR` must match the following\n+\tpattern for files to be included. The pattern can contain\n+\tstandard globbing wildcards and two additional ones, `**/` and\n+\t`/**`, that can match multiple path components. Please refer\n+\tto linkgit:gitignore[5] for details. For convenience:\n+\n+ * If the pattern starts with `~/`, `~` will be substituted with the\n+ content of the environment variable `HOME`.\n+\n+ * If the pattern starts with `./`, it is replaced with the directory\n+ containing the current config file.\n+\n+ * If the pattern does not start with either `~/`, `./` or `/`, `**/`\n+ will be automatically prepended. For example, the pattern `foo/bar`\n+ becomes `**/foo/bar` and would match `/any/path/to/foo/bar`.\n+\n+ * If the pattern ends with `/`, `**` will be automatically added. For\n+ example, the pattern `foo/` becomes `foo/**`. In other words, it\n+ matches \"foo\" and everything inside, recursively.\n+\n+`gitdir/i`::\n+\tThis is the same as `gitdir` except that matching is done\n+\tcase-insensitively (e.g. on case-insensitive file sytems)\n+\n+A few more notes on matching:\n+\n+ * Symlinks in `$GIT_DIR` are not resolved before matching.\n+\n+ * Note that \"../\" is not special and will match literally, which is\n+ unlikely what you want.\n \n Example\n ~~~~~~~\n@@ -119,6 +162,17 @@ Example\n \t\tpath = foo ; expand \"foo\" relative to the current file\n \t\tpath = ~/foo ; expand \"foo\" in your `$HOME` directory\n \n+\t; include if $GIT_DIR is /path/to/foo/.git\n+\t[include-if \"gitdir:/path/to/foo/.git\"]\n+\t\tpath = /path/to/foo.inc\n+\n+\t; include for all repositories inside /path/to/group\n+\t[include-if \"gitdir:/path/to/group/\"]\n+\t\tpath = /path/to/foo.inc\n+\n+\t; include for all repositories inside $HOME/to/group\n+\t[include-if \"gitdir:~/to/group/\"]\n+\t\tpath = /path/to/foo.inc\n \n Values\n ~~~~~~\ndiff --git a/config.c b/config.c\nindex c6b874a7bf..3090fbf1a8 100644\n--- a/config.c\n+++ b/config.c\n@@ -13,6 +13,7 @@\n #include \"hashmap.h\"\n #include \"string-list.h\"\n #include \"utf8.h\"\n+#include \"dir.h\"\n \n struct config_source {\n \tstruct config_source *prev;\n@@ -170,9 +171,101 @@ static int handle_path_include(const char *path, struct config_include_data *inc\n \treturn ret;\n }\n \n+static int prepare_include_condition_pattern(struct strbuf *pat)\n+{\n+\tstruct strbuf path = STRBUF_INIT;\n+\tint prefix = 0;\n+\n+\t/* TODO: maybe support ~user/ too */\n+\tif (pat->buf[0] == '~' && is_dir_sep(pat->buf[1])) {\n+\t\tconst char *home = getenv(\"HOME\");\n+\n+\t\tif (!home)\n+\t\t\treturn error(_(\"$HOME is not defined\"));\n+\n+\t\tstrbuf_add_absolute_path(&path, home);\n+\t\tstrbuf_splice(pat, 0, 1, path.buf, path.len);\n+\t\tprefix = path.len + 1 /*slash*/;\n+\t} else if (pat->buf[0] == '.' && is_dir_sep(pat->buf[1])) {\n+\t\tconst char *slash;\n+\n+\t\tif (!cf || !cf->path)\n+\t\t\treturn error(_(\"relative config include \"\n+\t\t\t\t \"conditionals must come from files\"));\n+\n+\t\t/* TODO: escape wildcards */\n+\t\tstrbuf_add_absolute_path(&path, cf->path);\n+\t\tslash = find_last_dir_sep(path.buf);\n+\t\tif (!slash)\n+\t\t\tdie(\"BUG: how is this possible?\");\n+\t\tstrbuf_splice(pat, 0, 1, path.buf, slash - path.buf);\n+\t\tprefix = slash - path.buf + 1 /* slash */;\n+\t} else if (!is_absolute_path(pat->buf))\n+\t\tstrbuf_insert(pat, 0, \"**/\", 3);\n+\n+\tif (pat->len && is_dir_sep(pat->buf[pat->len - 1]))\n+\t\tstrbuf_addstr(pat, \"**\");\n+\n+\tstrbuf_release(&path);\n+\treturn prefix;\n+}\n+\n+static int include_by_gitdir(const char *cond, size_t cond_len, int icase)\n+{\n+\tstruct strbuf text = STRBUF_INIT;\n+\tstruct strbuf pattern = STRBUF_INIT;\n+\tint ret = 0, prefix;\n+\n+\tstrbuf_add_absolute_path(&text, get_git_dir());\n+\tstrbuf_add(&pattern, cond, cond_len);\n+\tprefix = prepare_include_condition_pattern(&pattern);\n+\n+\tif (prefix < 0)\n+\t\tgoto done;\n+\n+\tif (prefix > 0) {\n+\t\t/*\n+\t\t * perform literal matching on the prefix part so that\n+\t\t * any wildcard character in it can't create side effects.\n+\t\t */\n+\t\tif (text.len < prefix)\n+\t\t\tgoto done;\n+\t\tif (!icase && strncmp(pattern.buf, text.buf, prefix))\n+\t\t\tgoto done;\n+\t\tif (icase && strncasecmp(pattern.buf, text.buf, prefix))\n+\t\t\tgoto done;\n+\t}\n+\n+\tret = !wildmatch(pattern.buf + prefix, text.buf + prefix,\n+\t\t\t icase ? WM_CASEFOLD : 0, NULL);\n+\n+done:\n+\tstrbuf_release(&pattern);\n+\tstrbuf_release(&text);\n+\treturn ret;\n+}\n+\n+static int include_condition_is_true(const char *cond, size_t cond_len)\n+{\n+\t/* no condition (i.e., \"include.path\") is always true */\n+\tif (!cond)\n+\t\treturn 1;\n+\n+\tif (skip_prefix_mem(cond, cond_len, \"gitdir:\", &cond, &cond_len))\n+\t\treturn include_by_gitdir(cond, cond_len, 0);\n+\telse if (skip_prefix_mem(cond, cond_len, \"gitdir/i:\", &cond, &cond_len))\n+\t\treturn include_by_gitdir(cond, cond_len, 1);\n+\n+\terror(_(\"unrecognized include condition: %.*s\"), (int)cond_len, cond);\n+\t/* unknown conditionals are always false */\n+\treturn 0;\n+}\n+\n int git_config_include(const char *var, const char *value, void *data)\n {\n \tstruct config_include_data *inc = data;\n+\tconst char *cond, *key;\n+\tint cond_len;\n \tint ret;\n \n \t/*\n@@ -185,6 +278,13 @@ int git_config_include(const char *var, const char *value, void *data)\n \n \tif (!strcmp(var, \"include.path\"))\n \t\tret = handle_path_include(value, inc);\n+\n+\tif (!parse_config_key(var, \"include-if\", &cond, &cond_len, &key) &&\n+\t include_condition_is_true(cond, cond_len)) {\n+\t\tif (!strcmp(key, \"path\"))\n+\t\t\tret = handle_path_include(value, inc);\n+\t\t/* else we do not know about this type of include; ignore */\n+\t}\n \treturn ret;\n }\n \ndiff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh\nindex 9ba2ba11c3..d5b586e270 100755\n--- a/t/t1305-config-include.sh\n+++ b/t/t1305-config-include.sh\n@@ -152,6 +152,62 @@ test_expect_success 'relative includes from stdin line fail' '\n \ttest_must_fail git config --file - test.one\n '\n \n+test_expect_success 'conditional include, both unanchored' '\n+\tgit init foo &&\n+\t(\n+\t\tcd foo &&\n+\t\techo \"[include-if \\\"gitdir:foo/\\\"]path=bar\" >>.git/config &&\n+\t\techo \"[test]one=1\" >.git/bar &&\n+\t\techo 1 >expect &&\n+\t\tgit config test.one >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+test_expect_success 'conditional include, $HOME expansion' '\n+\t(\n+\t\tcd foo &&\n+\t\techo \"[include-if \\\"gitdir:~/foo/\\\"]path=bar2\" >>.git/config &&\n+\t\techo \"[test]two=2\" >.git/bar2 &&\n+\t\techo 2 >expect &&\n+\t\tgit config test.two >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+test_expect_success 'conditional include, full pattern' '\n+\t(\n+\t\tcd foo &&\n+\t\techo \"[include-if \\\"gitdir:**/foo/**\\\"]path=bar3\" >>.git/config &&\n+\t\techo \"[test]three=3\" >.git/bar3 &&\n+\t\techo 3 >expect &&\n+\t\tgit config test.three >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+test_expect_success 'conditional include, relative path' '\n+\techo \"[include-if \\\"gitdir:./foo/.git\\\"]path=bar4\" >>.gitconfig &&\n+\techo \"[test]four=4\" >bar4 &&\n+\t(\n+\t\tcd foo &&\n+\t\techo 4 >expect &&\n+\t\tgit config test.four >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+test_expect_success 'conditional include, both unanchored, icase' '\n+\t(\n+\t\tcd foo &&\n+\t\techo \"[include-if \\\"gitdir/i:FOO/\\\"]path=bar5\" >>.git/config &&\n+\t\techo \"[test]five=5\" >.git/bar5 &&\n+\t\techo 5 >expect &&\n+\t\tgit config test.five >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n test_expect_success 'include cycles are detected' '\n \tcat >.gitconfig <<-\\EOF &&\n \t[test]value = gitconfig\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"312391","messageId":"20170223122346.12222-1-pclouds@gmail.com","threadId":"45200","inReplyTo":null,"subject":"[PATCH v5 0/1] Conditional config include","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-02-23T12:23:45Z","receivedAt":"2017-02-23T12:31:19Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Let's try this again. v4 and before can be found in the original\nthread [1]. The remaining issues of v4 were\n\nOn Fri, Aug 19, 2016 at 8:54 PM, Jeff King wrote:\n> On Sat, Aug 13, 2016 at 03:40:59PM +0700, Duy Nguyen wrote:\n>\n>> Ping..\n>\n> There was some discussion after v4. I think the open issues are:\n>\n> - the commit message is rather terse (it should describe motivation,\n> and can refer to the docs for the \"how\")\n\nFixed\n\n>\n> - the syntax might be more clear as:\n>\n> [include-if \"gitdir:...\"]\n>\n> or\n>\n> [include \"gitdir-is:...\"]\n\nI'll go with include-if. It feels a bit awkward to add -is to every\nnew type of condition.\n\n> - there is an open question of whether we would like to go this route,\n> maintaining backwards compatibility syntactically (and thus having\n> these includes quietly skipped in older versions), or introduce a\n> breaking syntax that could be more convenient, like:\n>\n> [if-gitdir(...):section]\n> conditional-but-no-include-necessary = true\n>\n> I do not have a strong opinion myself, though I had written the\n> original [include] assuming syntactic compatibility, and I think it\n> has worked out (e.g., you can manipulate include.path via \"git\n> config\" just as you would any other key).\n\nI'll go with the what we have done, at least it's proven not provoking\ntoo bad effects. So no breaking syntax.\n\n[1] http://public-inbox.org/git/20160714153311.2166-1-pclouds@gmail.com/\n\nNguyễn Thái Ngọc Duy (1):\n config: add conditional include\n\n Documentation/config.txt | 54 +++++++++++++++++++++++++\n config.c | 100 ++++++++++++++++++++++++++++++++++++++++++++++\n t/t1305-config-include.sh | 56 ++++++++++++++++++++++++++\n 3 files changed, 210 insertions(+)\n\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"312429","messageId":"xmqqwpcg7k6r.fsf@gitster.mtv.corp.google.com","threadId":"45200","inReplyTo":"20170223122346.12222-2-pclouds@gmail.com","subject":"Re: [PATCH v5 1/1] config: add conditional include","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-02-23T19:59:08Z","receivedAt":"2017-02-23T19:59:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n>> There was some discussion after v4. I think the open issues are:\n>>\n>> - the commit message is rather terse (it should describe motivation,\n>> and can refer to the docs for the \"how\")\n\n> This allows some more flexibility in managing configuration across\n> repositories. \n\nThat is not an ideal opening to describe motivation without people\nknowing what \"this\" is ;-) Of course, the person who wrote the\nsentence know it already after writing the patch and the subject,\nbut others don't.\n\n\tSometimes a set of repositories want to share configuration\n\tsettings among themselves that are distinct from other such\n\tsets of repositories. A user may work on two projects, each\n\tof which have multiple repositories, and use one user.email\n\tfor one project while using another for the other. Having\n\tthe settings in ~/.gitconfig, which would work for just one\n\tset of repositories, would not well in such a situation.\n\n\tExtend the include.path mechanism that lets a config file\n\tinclude another config file, so that the inclusion can be\n\tdone only when some conditions hold. Then ~/.gitconfig can\n\tsay \"include config-project-A only when working on project-A\"\n for each project A the user works on.\n\n In this patch, the only supported grouping is based on\n $GIT_DIR (in absolute path), so you would need to group\n repositories by directory, or something like that to take\n advantage of it.\n\n We already have include.path for unconditional\n includes. This patch goes with include-if.xxx.path to make\n it clearer that a condition is required.\n\n Similar to include.path, older git versions that don't\n understand include-if will simply ignore them.\n\nor something along that line?\n\n> +Conditional includes\n> +~~~~~~~~~~~~~~~~~~~~\n> +\n> +You can include one config file from another conditionally by setting\n> +a special `include-if..path` variable to the name of the\n> +file to be included. The variable is treated the same way as\n> +`include.path`.\n\nDrop \"special\", as all configuration variables are \"special\" in their\nown sense, it does not add any useful information.\n\nI think we avoid '-' in Git-native variable and section names, so\n\"include-if\" would become an odd-man-out.\n\nThe variable is obviously not treated the same way as include.path ;-)\n\n When includeIf..path variable is set in a\n configuration file, the configuration file named by that\n variable is included (in a way similar to how include.path\n works) only if the holds true.\n\n> +The condition starts with a keyword, followed by a colon and a\n> +pattern. The interpretation of the pattern depends on the keyword.\n\n\"a pattern\"? I think it is \"some data whose format and meaning\ndepends on the keyword\".\n\nHence...\n\n> +Supported keywords are:\n> +\n> +`gitdir`::\n> +\tThe environment variable `GIT_DIR` must match the following\n> +\tpattern for files to be included. The pattern can contain\n\n\tThe data that follows the keyword `gitdir:` is used as a\n\tglob pattern. If the location of the .git directory (which\n\tmay be auto-discovered, or come from `GIT_DIR` environment\n\tvariable) match the pattern, the `` becomes true.\n\n> + ...\n> + * If the pattern ends with `/`, `**` will be automatically added. For\n> + example, the pattern `foo/` becomes `foo/**`. In other words, it\n> + matches \"foo\" and everything inside, recursively.\n> +\n> +`gitdir/i`::\n> +\tThis is the same as `gitdir` except that matching is done\n> +\tcase-insensitively (e.g. on case-insensitive file sytems)\n> +\n> +A few more notes on matching:\n\nAs future may not be about path or matching at all, this\nbelongs to `gitdir`:: section (and it would be obvious that that\napplies to `gitdir/i`:: because we say \"this is the same as...\").\n\n> + * Symlinks in `$GIT_DIR` are not resolved before matching.\n> +\n> + * Note that \"../\" is not special and will match literally, which is\n> + unlikely what you want.\n\n> +static int prepare_include_condition_pattern(struct strbuf *pat)\n> +{\n> +\tstruct strbuf path = STRBUF_INIT;\n> +\tint prefix = 0;\n> +\n> +\t/* TODO: maybe support ~user/ too */\n> +\tif (pat->buf[0] == '~' && is_dir_sep(pat->buf[1])) {\n> +\t\tconst char *home = getenv(\"HOME\");\n> +\n> +\t\tif (!home)\n> +\t\t\treturn error(_(\"$HOME is not defined\"));\n\nInstead of half-duplicating it here yourself, can't we let\nexpand_user_path() do its thing?\n\n> +static int include_condition_is_true(const char *cond, size_t cond_len)\n> +{\n> +\t/* no condition (i.e., \"include.path\") is always true */\n> +\tif (!cond)\n> +\t\treturn 1;\n> +\n> +\tif (skip_prefix_mem(cond, cond_len, \"gitdir:\", &cond, &cond_len))\n> +\t\treturn include_by_gitdir(cond, cond_len, 0);\n> +\telse if (skip_prefix_mem(cond, cond_len, \"gitdir/i:\", &cond, &cond_len))\n> +\t\treturn include_by_gitdir(cond, cond_len, 1);\n\nThis may be OK for now, but it should be trivial to start from a\ntable with two entries, i.e.\n\n\tstruct include_cond {\n\t\tconst char *keyword;\n\t\tint (*fn)(const char *, size_t);\n\t};\n\nand will show a better way to do things to those who follow your\nfootsteps.\n\n"},{"id":"312508","messageId":"CACsJy8CerU-=PF6wqzUAM02jkrVVGJ5MA0NgL6z9bHn5KM6jiw@mail.gmail.com","threadId":"45200","inReplyTo":"xmqqwpcg7k6r.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v5 1/1] config: add conditional include","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-02-24T09:37:48Z","receivedAt":"2017-02-24T09:47:00Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Feb 24, 2017 at 2:59 AM, Junio C Hamano wrote:\n> The variable is obviously not treated the same way as include.path ;-)\n>\n> When includeIf..path variable is set in a\n> configuration file, the configuration file named by that\n> variable is included (in a way similar to how include.path\n> works) only if the holds true.\n\nYeah. I was thinking \"value\" and writing \"variable\" instead (or\nperhaps I meant to write \"variable value\" and accidentally'd the last\nword again).\n\n>> + /* TODO: maybe support ~user/ too */\n>> + if (pat->buf[0] == '~' && is_dir_sep(pat->buf[1])) {\n>> + const char *home = getenv(\"HOME\");\n>> +\n>> + if (!home)\n>> + return error(_(\"$HOME is not defined\"));\n>\n> Instead of half-duplicating it here yourself, can't we let\n> expand_user_path() do its thing?\n\nThis comment came up in previous iterations. I perform explicit\nexpansion to make sure we don't apply wildcards on the expanded \"~\".\nBut I guess going with expand_user_path() is better (less confusion\nfor future readers). We can come back to it when that \"don't apply\nwildcards\" concern becomes real.\n\n>> +static int include_condition_is_true(const char *cond, size_t cond_len)\n>> +{\n>> + /* no condition (i.e., \"include.path\") is always true */\n>> + if (!cond)\n>> + return 1;\n>> +\n>> + if (skip_prefix_mem(cond, cond_len, \"gitdir:\", &cond, &cond_len))\n>> + return include_by_gitdir(cond, cond_len, 0);\n>> + else if (skip_prefix_mem(cond, cond_len, \"gitdir/i:\", &cond, &cond_len))\n>> + return include_by_gitdir(cond, cond_len, 1);\n>\n> This may be OK for now, but it should be trivial to start from a\n> table with two entries, i.e.\n>\n> struct include_cond {\n> const char *keyword;\n> int (*fn)(const char *, size_t);\n> };\n>\n> and will show a better way to do things to those who follow your\n> footsteps.\n\nYeah I don't see a third include coming soon and did not go with that.\nLet's way for it and refactor then.\n-- \nDuy\n"},{"id":"312511","messageId":"20170224131425.32409-2-pclouds@gmail.com","threadId":"45200","inReplyTo":"20170224131425.32409-1-pclouds@gmail.com","subject":"[PATCH v6 1/1] config: add conditional include","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-02-24T13:14:23Z","receivedAt":"2017-02-24T13:15:18Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Sometimes a set of repositories want to share configuration settings\namong themselves that are distinct from other such sets of repositories.\nA user may work on two projects, each of which have multiple\nrepositories, and use one user.email for one project while using another\nfor the other.\n\nSetting $GIT_DIR/.config works, but if the penalty of forgetting to\nupdate $GIT_DIR/.config is high (especially when you end up cloning\noften), it may not be the best way to go. Having the settings in\n~/.gitconfig, which would work for just one set of repositories, would\nnot well in such a situation. Having separate ${HOME}s may add more\nproblems than it solves.\n\nExtend the include.path mechanism that lets a config file include\nanother config file, so that the inclusion can be done only when some\nconditions hold. Then ~/.gitconfig can say \"include config-project-A\nonly when working on project-A\" for each project A the user works on.\n\nIn this patch, the only supported grouping is based on $GIT_DIR (in\nabsolute path), so you would need to group repositories by directory, or\nsomething like that to take advantage of it.\n\nWe already have include.path for unconditional includes. This patch goes\nwith includeIf..path to make it clearer that a condition is\nrequired. The new config has the same backward compatibility approach as\ninclude.path: older git versions that don't understand includeIf will\nsimply ignore them.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n Documentation/config.txt | 61 +++++++++++++++++++++++++++++\n config.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++\n t/t1305-config-include.sh | 56 +++++++++++++++++++++++++++\n 3 files changed, 214 insertions(+)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 015346c417..6c0cd2a273 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -91,6 +91,56 @@ found at the location of the include directive. If the value of the\n relative to the configuration file in which the include directive was\n found. See below for examples.\n \n+Conditional includes\n+~~~~~~~~~~~~~~~~~~~~\n+\n+You can include one config file from another conditionally by setting\n+a `includeIf..path` variable to the name of the file to be\n+included. The variable's value is treated the same way as `include.path`.\n+\n+The condition starts with a keyword followed by a colon and some data\n+whose format and meaning depends on the keyword. Supported keywords\n+are:\n+\n+`gitdir`::\n+\n+\tThe data that follows the keyword `gitdir:` is used as a glob\n+\tpattern. If the location of the .git directory match the\n+\tpattern, the include condition is met.\n++\n+The .git location which may be auto-discovered, or come from\n+`$GIT_DIR` environment variable. If the repository auto discovered via\n+a .git file (e.g. from submodules, or a linked worktree), the .git\n+location would be the final location, not where the .git file is.\n++\n+The pattern can contain standard globbing wildcards and two additional\n+ones, `**/` and `/**`, that can match multiple path components. Please\n+refer to linkgit:gitignore[5] for details. For convenience:\n+\n+ * If the pattern starts with `~/`, `~` will be substituted with the\n+ content of the environment variable `HOME`.\n+\n+ * If the pattern starts with `./`, it is replaced with the directory\n+ containing the current config file.\n+\n+ * If the pattern does not start with either `~/`, `./` or `/`, `**/`\n+ will be automatically prepended. For example, the pattern `foo/bar`\n+ becomes `**/foo/bar` and would match `/any/path/to/foo/bar`.\n+\n+ * If the pattern ends with `/`, `**` will be automatically added. For\n+ example, the pattern `foo/` becomes `foo/**`. In other words, it\n+ matches \"foo\" and everything inside, recursively.\n+\n+`gitdir/i`::\n+\tThis is the same as `gitdir` except that matching is done\n+\tcase-insensitively (e.g. on case-insensitive file sytems)\n+\n+A few more notes on matching with `gitdir` and `gitdir/i`:\n+\n+ * Symlinks in `$GIT_DIR` are not resolved before matching.\n+\n+ * Note that \"../\" is not special and will match literally, which is\n+ unlikely what you want.\n \n Example\n ~~~~~~~\n@@ -119,6 +169,17 @@ Example\n \t\tpath = foo ; expand \"foo\" relative to the current file\n \t\tpath = ~/foo ; expand \"foo\" in your `$HOME` directory\n \n+\t; include if $GIT_DIR is /path/to/foo/.git\n+\t[include-if \"gitdir:/path/to/foo/.git\"]\n+\t\tpath = /path/to/foo.inc\n+\n+\t; include for all repositories inside /path/to/group\n+\t[include-if \"gitdir:/path/to/group/\"]\n+\t\tpath = /path/to/foo.inc\n+\n+\t; include for all repositories inside $HOME/to/group\n+\t[include-if \"gitdir:~/to/group/\"]\n+\t\tpath = /path/to/foo.inc\n \n Values\n ~~~~~~\ndiff --git a/config.c b/config.c\nindex c6b874a7bf..ad16802c8a 100644\n--- a/config.c\n+++ b/config.c\n@@ -13,6 +13,7 @@\n #include \"hashmap.h\"\n #include \"string-list.h\"\n #include \"utf8.h\"\n+#include \"dir.h\"\n \n struct config_source {\n \tstruct config_source *prev;\n@@ -170,9 +171,99 @@ static int handle_path_include(const char *path, struct config_include_data *inc\n \treturn ret;\n }\n \n+static int prepare_include_condition_pattern(struct strbuf *pat)\n+{\n+\tstruct strbuf path = STRBUF_INIT;\n+\tchar *expanded;\n+\tint prefix = 0;\n+\n+\texpanded = expand_user_path(pat->buf);\n+\tif (expanded) {\n+\t\tstrbuf_reset(pat);\n+\t\tstrbuf_addstr(pat, expanded);\n+\t\tfree(expanded);\n+\t}\n+\n+\tif (pat->buf[0] == '.' && is_dir_sep(pat->buf[1])) {\n+\t\tconst char *slash;\n+\n+\t\tif (!cf || !cf->path)\n+\t\t\treturn error(_(\"relative config include \"\n+\t\t\t\t \"conditionals must come from files\"));\n+\n+\t\t/* TODO: escape wildcards */\n+\t\tstrbuf_add_absolute_path(&path, cf->path);\n+\t\tslash = find_last_dir_sep(path.buf);\n+\t\tif (!slash)\n+\t\t\tdie(\"BUG: how is this possible?\");\n+\t\tstrbuf_splice(pat, 0, 1, path.buf, slash - path.buf);\n+\t\tprefix = slash - path.buf + 1 /* slash */;\n+\t} else if (!is_absolute_path(pat->buf))\n+\t\tstrbuf_insert(pat, 0, \"**/\", 3);\n+\n+\tif (pat->len && is_dir_sep(pat->buf[pat->len - 1]))\n+\t\tstrbuf_addstr(pat, \"**\");\n+\n+\tstrbuf_release(&path);\n+\treturn prefix;\n+}\n+\n+static int include_by_gitdir(const char *cond, size_t cond_len, int icase)\n+{\n+\tstruct strbuf text = STRBUF_INIT;\n+\tstruct strbuf pattern = STRBUF_INIT;\n+\tint ret = 0, prefix;\n+\n+\tstrbuf_add_absolute_path(&text, get_git_dir());\n+\tstrbuf_add(&pattern, cond, cond_len);\n+\tprefix = prepare_include_condition_pattern(&pattern);\n+\n+\tif (prefix < 0)\n+\t\tgoto done;\n+\n+\tif (prefix > 0) {\n+\t\t/*\n+\t\t * perform literal matching on the prefix part so that\n+\t\t * any wildcard character in it can't create side effects.\n+\t\t */\n+\t\tif (text.len < prefix)\n+\t\t\tgoto done;\n+\t\tif (!icase && strncmp(pattern.buf, text.buf, prefix))\n+\t\t\tgoto done;\n+\t\tif (icase && strncasecmp(pattern.buf, text.buf, prefix))\n+\t\t\tgoto done;\n+\t}\n+\n+\tret = !wildmatch(pattern.buf + prefix, text.buf + prefix,\n+\t\t\t icase ? WM_CASEFOLD : 0, NULL);\n+\n+done:\n+\tstrbuf_release(&pattern);\n+\tstrbuf_release(&text);\n+\treturn ret;\n+}\n+\n+static int include_condition_is_true(const char *cond, size_t cond_len)\n+{\n+\t/* no condition (i.e., \"include.path\") is always true */\n+\tif (!cond)\n+\t\treturn 1;\n+\n+\tif (skip_prefix_mem(cond, cond_len, \"gitdir:\", &cond, &cond_len))\n+\t\treturn include_by_gitdir(cond, cond_len, 0);\n+\telse if (skip_prefix_mem(cond, cond_len, \"gitdir/i:\", &cond, &cond_len))\n+\t\treturn include_by_gitdir(cond, cond_len, 1);\n+\n+\terror(_(\"unrecognized include condition: %.*s\"), (int)cond_len, cond);\n+\t/* unknown conditionals are always false */\n+\treturn 0;\n+}\n+\n int git_config_include(const char *var, const char *value, void *data)\n {\n \tstruct config_include_data *inc = data;\n+\tconst char *cond, *key;\n+\tint cond_len;\n \tint ret;\n \n \t/*\n@@ -185,6 +276,12 @@ int git_config_include(const char *var, const char *value, void *data)\n \n \tif (!strcmp(var, \"include.path\"))\n \t\tret = handle_path_include(value, inc);\n+\n+\tif (!parse_config_key(var, \"includeif\", &cond, &cond_len, &key) &&\n+\t include_condition_is_true(cond, cond_len) &&\n+\t !strcmp(key, \"path\"))\n+\t\tret = handle_path_include(value, inc);\n+\n \treturn ret;\n }\n \ndiff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh\nindex 9ba2ba11c3..f0cd2056ba 100755\n--- a/t/t1305-config-include.sh\n+++ b/t/t1305-config-include.sh\n@@ -152,6 +152,62 @@ test_expect_success 'relative includes from stdin line fail' '\n \ttest_must_fail git config --file - test.one\n '\n \n+test_expect_success 'conditional include, both unanchored' '\n+\tgit init foo &&\n+\t(\n+\t\tcd foo &&\n+\t\techo \"[includeIf \\\"gitdir:foo/\\\"]path=bar\" >>.git/config &&\n+\t\techo \"[test]one=1\" >.git/bar &&\n+\t\techo 1 >expect &&\n+\t\tgit config test.one >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+test_expect_success 'conditional include, $HOME expansion' '\n+\t(\n+\t\tcd foo &&\n+\t\techo \"[includeIf \\\"gitdir:~/foo/\\\"]path=bar2\" >>.git/config &&\n+\t\techo \"[test]two=2\" >.git/bar2 &&\n+\t\techo 2 >expect &&\n+\t\tgit config test.two >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+test_expect_success 'conditional include, full pattern' '\n+\t(\n+\t\tcd foo &&\n+\t\techo \"[includeIf \\\"gitdir:**/foo/**\\\"]path=bar3\" >>.git/config &&\n+\t\techo \"[test]three=3\" >.git/bar3 &&\n+\t\techo 3 >expect &&\n+\t\tgit config test.three >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+test_expect_success 'conditional include, relative path' '\n+\techo \"[includeIf \\\"gitdir:./foo/.git\\\"]path=bar4\" >>.gitconfig &&\n+\techo \"[test]four=4\" >bar4 &&\n+\t(\n+\t\tcd foo &&\n+\t\techo 4 >expect &&\n+\t\tgit config test.four >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+test_expect_success 'conditional include, both unanchored, icase' '\n+\t(\n+\t\tcd foo &&\n+\t\techo \"[includeIf \\\"gitdir/i:FOO/\\\"]path=bar5\" >>.git/config &&\n+\t\techo \"[test]five=5\" >.git/bar5 &&\n+\t\techo 5 >expect &&\n+\t\tgit config test.five >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n test_expect_success 'include cycles are detected' '\n \tcat >.gitconfig <<-\\EOF &&\n \t[test]value = gitconfig\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"312512","messageId":"20170224131425.32409-3-pclouds@gmail.com","threadId":"45200","inReplyTo":"20170224131425.32409-1-pclouds@gmail.com","subject":"[PATCH v6 0/1] Conditional config include","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-02-24T13:14:24Z","receivedAt":"2017-02-24T13:15:29Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"v6 has lots of text changes, most of which I shamelessly copied from\nJunio's, with some minor edits. The biggest edit is the mention of .git\nfiles, which is probably a good idea since .git files are used by\nmulti worktrees and submodules.\n\nI could not move the \"notes about matching\" block into the gitdir\nblock though. I needed to indent it once (not twice like the \"for\nconvenience\" block) but my asciidoc-foo did not manage it so I settled\nfor \"a few more notes on matching _with gitdir_\" without moving it.\n\nCode changes include renaming include-if to includeIf, and using\nexpand_user_path() instead of rolling my own.\n\nNguyễn Thái Ngọc Duy (1):\n config: add conditional include\n\n Documentation/config.txt | 61 +++++++++++++++++++++++++++++\n config.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++\n t/t1305-config-include.sh | 56 +++++++++++++++++++++++++++\n 3 files changed, 214 insertions(+)\n\n-- \n2.11.0.157.gd943d85\n"},{"id":"312513","messageId":"20170224131425.32409-4-pclouds@gmail.com","threadId":"45200","inReplyTo":"20170224131425.32409-1-pclouds@gmail.com","subject":"[PATCH v6 1/1] config: add conditional include","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-02-24T13:14:25Z","receivedAt":"2017-02-24T13:17:09Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Sometimes a set of repositories want to share configuration settings\namong themselves that are distinct from other such sets of repositories.\nA user may work on two projects, each of which have multiple\nrepositories, and use one user.email for one project while using another\nfor the other.\n\nSetting $GIT_DIR/.config works, but if the penalty of forgetting to\nupdate $GIT_DIR/.config is high (especially when you end up cloning\noften), it may not be the best way to go. Having the settings in\n~/.gitconfig, which would work for just one set of repositories, would\nnot well in such a situation. Having separate ${HOME}s may add more\nproblems than it solves.\n\nExtend the include.path mechanism that lets a config file include\nanother config file, so that the inclusion can be done only when some\nconditions hold. Then ~/.gitconfig can say \"include config-project-A\nonly when working on project-A\" for each project A the user works on.\n\nIn this patch, the only supported grouping is based on $GIT_DIR (in\nabsolute path), so you would need to group repositories by directory, or\nsomething like that to take advantage of it.\n\nWe already have include.path for unconditional includes. This patch goes\nwith includeIf..path to make it clearer that a condition is\nrequired. The new config has the same backward compatibility approach as\ninclude.path: older git versions that don't understand includeIf will\nsimply ignore them.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n Documentation/config.txt | 61 +++++++++++++++++++++++++++++\n config.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++\n t/t1305-config-include.sh | 56 +++++++++++++++++++++++++++\n 3 files changed, 214 insertions(+)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 015346c417..6c0cd2a273 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -91,6 +91,56 @@ found at the location of the include directive. If the value of the\n relative to the configuration file in which the include directive was\n found. See below for examples.\n \n+Conditional includes\n+~~~~~~~~~~~~~~~~~~~~\n+\n+You can include one config file from another conditionally by setting\n+a `includeIf..path` variable to the name of the file to be\n+included. The variable's value is treated the same way as `include.path`.\n+\n+The condition starts with a keyword followed by a colon and some data\n+whose format and meaning depends on the keyword. Supported keywords\n+are:\n+\n+`gitdir`::\n+\n+\tThe data that follows the keyword `gitdir:` is used as a glob\n+\tpattern. If the location of the .git directory match the\n+\tpattern, the include condition is met.\n++\n+The .git location which may be auto-discovered, or come from\n+`$GIT_DIR` environment variable. If the repository auto discovered via\n+a .git file (e.g. from submodules, or a linked worktree), the .git\n+location would be the final location, not where the .git file is.\n++\n+The pattern can contain standard globbing wildcards and two additional\n+ones, `**/` and `/**`, that can match multiple path components. Please\n+refer to linkgit:gitignore[5] for details. For convenience:\n+\n+ * If the pattern starts with `~/`, `~` will be substituted with the\n+ content of the environment variable `HOME`.\n+\n+ * If the pattern starts with `./`, it is replaced with the directory\n+ containing the current config file.\n+\n+ * If the pattern does not start with either `~/`, `./` or `/`, `**/`\n+ will be automatically prepended. For example, the pattern `foo/bar`\n+ becomes `**/foo/bar` and would match `/any/path/to/foo/bar`.\n+\n+ * If the pattern ends with `/`, `**` will be automatically added. For\n+ example, the pattern `foo/` becomes `foo/**`. In other words, it\n+ matches \"foo\" and everything inside, recursively.\n+\n+`gitdir/i`::\n+\tThis is the same as `gitdir` except that matching is done\n+\tcase-insensitively (e.g. on case-insensitive file sytems)\n+\n+A few more notes on matching with `gitdir` and `gitdir/i`:\n+\n+ * Symlinks in `$GIT_DIR` are not resolved before matching.\n+\n+ * Note that \"../\" is not special and will match literally, which is\n+ unlikely what you want.\n \n Example\n ~~~~~~~\n@@ -119,6 +169,17 @@ Example\n \t\tpath = foo ; expand \"foo\" relative to the current file\n \t\tpath = ~/foo ; expand \"foo\" in your `$HOME` directory\n \n+\t; include if $GIT_DIR is /path/to/foo/.git\n+\t[include-if \"gitdir:/path/to/foo/.git\"]\n+\t\tpath = /path/to/foo.inc\n+\n+\t; include for all repositories inside /path/to/group\n+\t[include-if \"gitdir:/path/to/group/\"]\n+\t\tpath = /path/to/foo.inc\n+\n+\t; include for all repositories inside $HOME/to/group\n+\t[include-if \"gitdir:~/to/group/\"]\n+\t\tpath = /path/to/foo.inc\n \n Values\n ~~~~~~\ndiff --git a/config.c b/config.c\nindex c6b874a7bf..ad16802c8a 100644\n--- a/config.c\n+++ b/config.c\n@@ -13,6 +13,7 @@\n #include \"hashmap.h\"\n #include \"string-list.h\"\n #include \"utf8.h\"\n+#include \"dir.h\"\n \n struct config_source {\n \tstruct config_source *prev;\n@@ -170,9 +171,99 @@ static int handle_path_include(const char *path, struct config_include_data *inc\n \treturn ret;\n }\n \n+static int prepare_include_condition_pattern(struct strbuf *pat)\n+{\n+\tstruct strbuf path = STRBUF_INIT;\n+\tchar *expanded;\n+\tint prefix = 0;\n+\n+\texpanded = expand_user_path(pat->buf);\n+\tif (expanded) {\n+\t\tstrbuf_reset(pat);\n+\t\tstrbuf_addstr(pat, expanded);\n+\t\tfree(expanded);\n+\t}\n+\n+\tif (pat->buf[0] == '.' && is_dir_sep(pat->buf[1])) {\n+\t\tconst char *slash;\n+\n+\t\tif (!cf || !cf->path)\n+\t\t\treturn error(_(\"relative config include \"\n+\t\t\t\t \"conditionals must come from files\"));\n+\n+\t\t/* TODO: escape wildcards */\n+\t\tstrbuf_add_absolute_path(&path, cf->path);\n+\t\tslash = find_last_dir_sep(path.buf);\n+\t\tif (!slash)\n+\t\t\tdie(\"BUG: how is this possible?\");\n+\t\tstrbuf_splice(pat, 0, 1, path.buf, slash - path.buf);\n+\t\tprefix = slash - path.buf + 1 /* slash */;\n+\t} else if (!is_absolute_path(pat->buf))\n+\t\tstrbuf_insert(pat, 0, \"**/\", 3);\n+\n+\tif (pat->len && is_dir_sep(pat->buf[pat->len - 1]))\n+\t\tstrbuf_addstr(pat, \"**\");\n+\n+\tstrbuf_release(&path);\n+\treturn prefix;\n+}\n+\n+static int include_by_gitdir(const char *cond, size_t cond_len, int icase)\n+{\n+\tstruct strbuf text = STRBUF_INIT;\n+\tstruct strbuf pattern = STRBUF_INIT;\n+\tint ret = 0, prefix;\n+\n+\tstrbuf_add_absolute_path(&text, get_git_dir());\n+\tstrbuf_add(&pattern, cond, cond_len);\n+\tprefix = prepare_include_condition_pattern(&pattern);\n+\n+\tif (prefix < 0)\n+\t\tgoto done;\n+\n+\tif (prefix > 0) {\n+\t\t/*\n+\t\t * perform literal matching on the prefix part so that\n+\t\t * any wildcard character in it can't create side effects.\n+\t\t */\n+\t\tif (text.len < prefix)\n+\t\t\tgoto done;\n+\t\tif (!icase && strncmp(pattern.buf, text.buf, prefix))\n+\t\t\tgoto done;\n+\t\tif (icase && strncasecmp(pattern.buf, text.buf, prefix))\n+\t\t\tgoto done;\n+\t}\n+\n+\tret = !wildmatch(pattern.buf + prefix, text.buf + prefix,\n+\t\t\t icase ? WM_CASEFOLD : 0, NULL);\n+\n+done:\n+\tstrbuf_release(&pattern);\n+\tstrbuf_release(&text);\n+\treturn ret;\n+}\n+\n+static int include_condition_is_true(const char *cond, size_t cond_len)\n+{\n+\t/* no condition (i.e., \"include.path\") is always true */\n+\tif (!cond)\n+\t\treturn 1;\n+\n+\tif (skip_prefix_mem(cond, cond_len, \"gitdir:\", &cond, &cond_len))\n+\t\treturn include_by_gitdir(cond, cond_len, 0);\n+\telse if (skip_prefix_mem(cond, cond_len, \"gitdir/i:\", &cond, &cond_len))\n+\t\treturn include_by_gitdir(cond, cond_len, 1);\n+\n+\terror(_(\"unrecognized include condition: %.*s\"), (int)cond_len, cond);\n+\t/* unknown conditionals are always false */\n+\treturn 0;\n+}\n+\n int git_config_include(const char *var, const char *value, void *data)\n {\n \tstruct config_include_data *inc = data;\n+\tconst char *cond, *key;\n+\tint cond_len;\n \tint ret;\n \n \t/*\n@@ -185,6 +276,12 @@ int git_config_include(const char *var, const char *value, void *data)\n \n \tif (!strcmp(var, \"include.path\"))\n \t\tret = handle_path_include(value, inc);\n+\n+\tif (!parse_config_key(var, \"includeif\", &cond, &cond_len, &key) &&\n+\t include_condition_is_true(cond, cond_len) &&\n+\t !strcmp(key, \"path\"))\n+\t\tret = handle_path_include(value, inc);\n+\n \treturn ret;\n }\n \ndiff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh\nindex 9ba2ba11c3..f0cd2056ba 100755\n--- a/t/t1305-config-include.sh\n+++ b/t/t1305-config-include.sh\n@@ -152,6 +152,62 @@ test_expect_success 'relative includes from stdin line fail' '\n \ttest_must_fail git config --file - test.one\n '\n \n+test_expect_success 'conditional include, both unanchored' '\n+\tgit init foo &&\n+\t(\n+\t\tcd foo &&\n+\t\techo \"[includeIf \\\"gitdir:foo/\\\"]path=bar\" >>.git/config &&\n+\t\techo \"[test]one=1\" >.git/bar &&\n+\t\techo 1 >expect &&\n+\t\tgit config test.one >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+test_expect_success 'conditional include, $HOME expansion' '\n+\t(\n+\t\tcd foo &&\n+\t\techo \"[includeIf \\\"gitdir:~/foo/\\\"]path=bar2\" >>.git/config &&\n+\t\techo \"[test]two=2\" >.git/bar2 &&\n+\t\techo 2 >expect &&\n+\t\tgit config test.two >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+test_expect_success 'conditional include, full pattern' '\n+\t(\n+\t\tcd foo &&\n+\t\techo \"[includeIf \\\"gitdir:**/foo/**\\\"]path=bar3\" >>.git/config &&\n+\t\techo \"[test]three=3\" >.git/bar3 &&\n+\t\techo 3 >expect &&\n+\t\tgit config test.three >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+test_expect_success 'conditional include, relative path' '\n+\techo \"[includeIf \\\"gitdir:./foo/.git\\\"]path=bar4\" >>.gitconfig &&\n+\techo \"[test]four=4\" >bar4 &&\n+\t(\n+\t\tcd foo &&\n+\t\techo 4 >expect &&\n+\t\tgit config test.four >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+test_expect_success 'conditional include, both unanchored, icase' '\n+\t(\n+\t\tcd foo &&\n+\t\techo \"[includeIf \\\"gitdir/i:FOO/\\\"]path=bar5\" >>.git/config &&\n+\t\techo \"[test]five=5\" >.git/bar5 &&\n+\t\techo 5 >expect &&\n+\t\tgit config test.five >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n test_expect_success 'include cycles are detected' '\n \tcat >.gitconfig <<-\\EOF &&\n \t[test]value = gitconfig\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"312514","messageId":"CACsJy8AFJBCs0LhUW3K9EsbKScyE7obGjf6hTT3DZjdn-4mLbA@mail.gmail.com","threadId":"45200","inReplyTo":"20170224131425.32409-3-pclouds@gmail.com","subject":"Re: [PATCH v6 0/1] Conditional config include","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-02-24T13:18:06Z","receivedAt":"2017-02-24T13:20:11Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"And sorry for duplicates :P Somehow I managed to --dry-run correctly\nthe first time, then had _two_ 000*.patch in the command line. And\nsend-email happily let me shoot myself in the foot.\n\n\n-- \nDuy\n"},{"id":"312515","messageId":"20170224131425.32409-1-pclouds@gmail.com","threadId":"45200","inReplyTo":"20170223122346.12222-1-pclouds@gmail.com","subject":"[PATCH v6 0/1] Conditional config include","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-02-24T13:14:22Z","receivedAt":"2017-02-24T13:23:36Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"v6 has lots of text changes, most of which I shamelessly copied from\nJunio's, with some minor edits. The biggest edit is the mention of .git\nfiles, which is probably a good idea since .git files are used by\nmulti worktrees and submodules.\n\nI could not move the \"notes about matching\" block into the gitdir\nblock though. I needed to indent it once (not twice like the \"for\nconvenience\" block) but my asciidoc-foo did not manage it so I settled\nfor \"a few more notes on matching _with gitdir_\" without moving it.\n\nCode changes include renaming include-if to includeIf, and using\nexpand_user_path() instead of rolling my own.\n\nNguyễn Thái Ngọc Duy (1):\n config: add conditional include\n\n Documentation/config.txt | 61 +++++++++++++++++++++++++++++\n config.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++\n t/t1305-config-include.sh | 56 +++++++++++++++++++++++++++\n 3 files changed, 214 insertions(+)\n\n-- \n2.11.0.157.gd943d85\n"},{"id":"312527","messageId":"xmqq1sun5vo6.fsf@gitster.mtv.corp.google.com","threadId":"45200","inReplyTo":"CACsJy8CerU-=PF6wqzUAM02jkrVVGJ5MA0NgL6z9bHn5KM6jiw@mail.gmail.com","subject":"Re: [PATCH v5 1/1] config: add conditional include","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-02-24T17:46:17Z","receivedAt":"2017-02-24T17:54:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n>>> + if (skip_prefix_mem(cond, cond_len, \"gitdir:\", &cond, &cond_len))\n>>> + return include_by_gitdir(cond, cond_len, 0);\n>>> + else if (skip_prefix_mem(cond, cond_len, \"gitdir/i:\", &cond, &cond_len))\n>>> + return include_by_gitdir(cond, cond_len, 1);\n>>\n>> This may be OK for now, but it should be trivial to start from a\n>> table with two entries, i.e.\n>>\n>> struct include_cond {\n>> const char *keyword;\n>> int (*fn)(const char *, size_t);\n>> };\n>>\n>> and will show a better way to do things to those who follow your\n>> footsteps.\n>\n> Yeah I don't see a third include coming soon and did not go with that.\n> Let's way for it and refactor then.\n\nI would have said exactly that in my message if you already had\ninclude_by_gitdir() and include_by_gitdir_i() as separate functions.\n\nBut I didn't, because the above code gives an excuse to the person\nwho adds the third one to be lazy and add another \"else if\" for\nexpediency, because making it table-driven would require an extra\nwork to add two wrapper functions that do not have anything to do\nwith the third one being added.\n\n"},{"id":"312531","messageId":"xmqqfuj34etq.fsf@gitster.mtv.corp.google.com","threadId":"45200","inReplyTo":"20170224131425.32409-2-pclouds@gmail.com","subject":"Re: [PATCH v6 1/1] config: add conditional include","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-02-24T18:35:29Z","receivedAt":"2017-02-24T18:35:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> +Conditional includes\n> +~~~~~~~~~~~~~~~~~~~~\n> +\n> +You can include one config file from another conditionally by setting\n> +a `includeIf..path` variable to the name of the file to be\n> +included. The variable's value is treated the same way as `include.path`.\n> +\n> +The condition starts with a keyword followed by a colon and some data\n> +whose format and meaning depends on the keyword. Supported keywords\n> +are:\n> +\n> +`gitdir`::\n> +\n> +\tThe data that follows the keyword `gitdir:` is used as a glob\n> +\tpattern. If the location of the .git directory match the\n> +\tpattern, the include condition is met.\n\ns/match/&es/, I think.\n\n> ++\n> +The .git location which may be auto-discovered, or come from\n\ns/ which//?\n\n> +`$GIT_DIR` environment variable. If the repository auto discovered via\n\ns/If the /In a/?\n\n> +a .git file (e.g. from submodules, or a linked worktree), the .git\n> +location would be the final location, not where the .git file is.\n\nOK.\n\n> @@ -170,9 +171,99 @@ static int handle_path_include(const char *path, struct config_include_data *inc\n> \treturn ret;\n> }\n> \n> +static int prepare_include_condition_pattern(struct strbuf *pat)\n> +{\n> + ...\n> +\t\t/* TODO: escape wildcards */\n> +\t\tstrbuf_add_absolute_path(&path, cf->path);\n\nIs this still TODO? As this one returns the prefix length (which\nprobably wants to be commented before the function) and this codepath\ncomputes the prefix to cover the path to here, doesn't caller already\ndo the right thing?\n\n> +static int include_condition_is_true(const char *cond, size_t cond_len)\n> +{\n> +\t/* no condition (i.e., \"include.path\") is always true */\n\nDoes this want to say \"includeIf.path\" instead? \"include.path\" is\ndone by handle_path_include() without involving a call to this\nfunction.\n\n> +\tif (!cond)\n> +\t\treturn 1;\n> +\n> +\tif (skip_prefix_mem(cond, cond_len, \"gitdir:\", &cond, &cond_len))\n> +\t\treturn include_by_gitdir(cond, cond_len, 0);\n> +\telse if (skip_prefix_mem(cond, cond_len, \"gitdir/i:\", &cond, &cond_len))\n> +\t\treturn include_by_gitdir(cond, cond_len, 1);\n> +\n> +\terror(_(\"unrecognized include condition: %.*s\"), (int)cond_len, cond);\n> +\t/* unknown conditionals are always false */\n> +\treturn 0;\n> +}\n\nThanks.\n"},{"id":"312543","messageId":"f1c87812-3dc1-fbfa-d12a-c9e7ddf9bb4e@ramsayjones.plus.com","threadId":"45200","inReplyTo":"20170224131425.32409-2-pclouds@gmail.com","subject":"Re: [PATCH v6 1/1] config: add conditional include","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2017-02-24T19:48:35Z","receivedAt":"2017-02-24T19:48:43Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 24/02/17 13:14, Nguyễn Thái Ngọc Duy wrote:\n[snip] \n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n> Documentation/config.txt | 61 +++++++++++++++++++++++++++++\n> config.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++\n> t/t1305-config-include.sh | 56 +++++++++++++++++++++++++++\n> 3 files changed, 214 insertions(+)\n> \n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index 015346c417..6c0cd2a273 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -91,6 +91,56 @@ found at the location of the include directive. If the value of the\n> relative to the configuration file in which the include directive was\n> found. See below for examples.\n> \n> +Conditional includes\n> +~~~~~~~~~~~~~~~~~~~~\n> +\n> +You can include one config file from another conditionally by setting\n> +a `includeIf..path` variable to the name of the file to be\n> +included. The variable's value is treated the same way as `include.path`.\n> +\n> +The condition starts with a keyword followed by a colon and some data\n> +whose format and meaning depends on the keyword. Supported keywords\n> +are:\n> +\n> +`gitdir`::\n> +\n> +\tThe data that follows the keyword `gitdir:` is used as a glob\n> +\tpattern. If the location of the .git directory match the\n> +\tpattern, the include condition is met.\n> ++\n> +The .git location which may be auto-discovered, or come from\n> +`$GIT_DIR` environment variable. If the repository auto discovered via\n> +a .git file (e.g. from submodules, or a linked worktree), the .git\n> +location would be the final location, not where the .git file is.\n> ++\n> +The pattern can contain standard globbing wildcards and two additional\n> +ones, `**/` and `/**`, that can match multiple path components. Please\n> +refer to linkgit:gitignore[5] for details. For convenience:\n> +\n> + * If the pattern starts with `~/`, `~` will be substituted with the\n> + content of the environment variable `HOME`.\n> +\n> + * If the pattern starts with `./`, it is replaced with the directory\n> + containing the current config file.\n> +\n> + * If the pattern does not start with either `~/`, `./` or `/`, `**/`\n> + will be automatically prepended. For example, the pattern `foo/bar`\n> + becomes `**/foo/bar` and would match `/any/path/to/foo/bar`.\n> +\n> + * If the pattern ends with `/`, `**` will be automatically added. For\n> + example, the pattern `foo/` becomes `foo/**`. In other words, it\n> + matches \"foo\" and everything inside, recursively.\n> +\n> +`gitdir/i`::\n> +\tThis is the same as `gitdir` except that matching is done\n> +\tcase-insensitively (e.g. on case-insensitive file sytems)\n> +\n> +A few more notes on matching with `gitdir` and `gitdir/i`:\n> +\n> + * Symlinks in `$GIT_DIR` are not resolved before matching.\n> +\n> + * Note that \"../\" is not special and will match literally, which is\n> + unlikely what you want.\n> \n> Example\n> ~~~~~~~\n> @@ -119,6 +169,17 @@ Example\n> \t\tpath = foo ; expand \"foo\" relative to the current file\n> \t\tpath = ~/foo ; expand \"foo\" in your `$HOME` directory\n> \n> +\t; include if $GIT_DIR is /path/to/foo/.git\n> +\t[include-if \"gitdir:/path/to/foo/.git\"]\n\ns/include-if/includeIf/\n\n> +\t\tpath = /path/to/foo.inc\n> +\n> +\t; include for all repositories inside /path/to/group\n> +\t[include-if \"gitdir:/path/to/group/\"]\n\nditto\n\n> +\t\tpath = /path/to/foo.inc\n> +\n> +\t; include for all repositories inside $HOME/to/group\n> +\t[include-if \"gitdir:~/to/group/\"]\n\nditto\n\nATB,\nRamsay Jones\n"},{"id":"312574","messageId":"29A09E4EDB1F4F4D9E77E67A7A8A33FF@PhilipOakley","threadId":"45200","inReplyTo":"20170224131425.32409-2-pclouds@gmail.com","subject":"Re: [PATCH v6 1/1] config: add conditional include","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":null,"receivedAt":"2017-02-24T22:08:24Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Nguyễn Thái Ngọc Duy\" \n> Sometimes a set of repositories want to share configuration settings\n> among themselves that are distinct from other such sets of repositories.\n> A user may work on two projects, each of which have multiple\n> repositories, and use one user.email for one project while using another\n> for the other.\n>\n> Setting $GIT_DIR/.config works, but if the penalty of forgetting to\n> update $GIT_DIR/.config is high (especially when you end up cloning\n> often), it may not be the best way to go. Having the settings in\n> ~/.gitconfig, which would work for just one set of repositories, would\n> not well in such a situation. Having separate ${HOME}s may add more\n> problems than it solves.\n>\n> Extend the include.path mechanism that lets a config file include\n> another config file, so that the inclusion can be done only when some\n> conditions hold. Then ~/.gitconfig can say \"include config-project-A\n> only when working on project-A\" for each project A the user works on.\n>\n> In this patch, the only supported grouping is based on $GIT_DIR (in\n> absolute path), so you would need to group repositories by directory, or\n> something like that to take advantage of it.\n>\n> We already have include.path for unconditional includes. This patch goes\n> with includeIf..path to make it clearer that a condition is\n> required. The new config has the same backward compatibility approach as\n> include.path: older git versions that don't understand includeIf will\n> simply ignore them.\n>\n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n> Documentation/config.txt | 61 +++++++++++++++++++++++++++++\n> config.c | 97 \n> +++++++++++++++++++++++++++++++++++++++++++++++\n> t/t1305-config-include.sh | 56 +++++++++++++++++++++++++++\n> 3 files changed, 214 insertions(+)\n>\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index 015346c417..6c0cd2a273 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -91,6 +91,56 @@ found at the location of the include directive. If the \n> value of the\n> relative to the configuration file in which the include directive was\n> found. See below for examples.\n>\n> +Conditional includes\n> +~~~~~~~~~~~~~~~~~~~~\n> +\n> +You can include one config file from another conditionally by setting\n\nOn first reading I thought this implied you can only have one `includeIf` \nwithin the config file.\nI think it is meant to mean that each `includeIf`could include one other \nfile, and that users can have multiple `includeIf` lines.\n\n\n> +a `includeIf..path` variable to the name of the file to be\n> +included. The variable's value is treated the same way as `include.path`.\n> +\n\n--\nPhilip \n\n"},{"id":"312678","messageId":"CACsJy8Bq-P8Aw+f0omndCmQU54n_p7ZwDVDK9APuH_3vXDac2g@mail.gmail.com","threadId":"45200","inReplyTo":"29A09E4EDB1F4F4D9E77E67A7A8A33FF@PhilipOakley","subject":"Re: [PATCH v6 1/1] config: add conditional include","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-02-26T03:02:18Z","receivedAt":"2017-02-26T03:12:55Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sat, Feb 25, 2017 at 5:08 AM, Philip Oakley wrote:\n>> +Conditional includes\n>> +~~~~~~~~~~~~~~~~~~~~\n>> +\n>> +You can include one config file from another conditionally by setting\n>\n>\n> On first reading I thought this implied you can only have one `includeIf`\n> within the config file.\n> I think it is meant to mean that each `includeIf`could include one other\n> file, and that users can have multiple `includeIf` lines.\n\nYes. Not sure how to put it better though (I basically copied the\nfirst paragraph from the unconditional include section above, which\nshares the same confusion). Perhaps just write \"the variable can be\nspecified multiple times\"? Or \"multiple variables include multiple\ntimes, the last variable does not override the previous ones\"?\n-- \nDuy\n"},{"id":"312680","messageId":"20170226060726.mvdip4vnbdngypzx@sigill.intra.peff.net","threadId":"45200","inReplyTo":"xmqq1sun5vo6.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v5 1/1] config: add conditional include","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-02-26T06:07:26Z","receivedAt":"2017-02-26T06:09:58Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 24, 2017 at 09:46:17AM -0800, Junio C Hamano wrote:\n\n> Duy Nguyen writes:\n> \n> >>> + if (skip_prefix_mem(cond, cond_len, \"gitdir:\", &cond, &cond_len))\n> >>> + return include_by_gitdir(cond, cond_len, 0);\n> >>> + else if (skip_prefix_mem(cond, cond_len, \"gitdir/i:\", &cond, &cond_len))\n> >>> + return include_by_gitdir(cond, cond_len, 1);\n> >>\n> >> This may be OK for now, but it should be trivial to start from a\n> >> table with two entries, i.e.\n> >>\n> >> struct include_cond {\n> >> const char *keyword;\n> >> int (*fn)(const char *, size_t);\n> >> };\n> >>\n> >> and will show a better way to do things to those who follow your\n> >> footsteps.\n> >\n> > Yeah I don't see a third include coming soon and did not go with that.\n> > Let's way for it and refactor then.\n> \n> I would have said exactly that in my message if you already had\n> include_by_gitdir() and include_by_gitdir_i() as separate functions.\n> \n> But I didn't, because the above code gives an excuse to the person\n> who adds the third one to be lazy and add another \"else if\" for\n> expediency, because making it table-driven would require an extra\n> work to add two wrapper functions that do not have anything to do\n> with the third one being added.\n\nI don't think driving that with a two-entry table is the right thing\nhere. We are as likely to add another \"foobar:\" entry as we are to add\nanother modifier \"/i\" modifier to \"gitdir:\", and it is unclear whether\nthat modifier would be mutually exclusive with \"/i\".\n\nE.g., imagine we add \"/re\" for a regex, but allow a case-insensitive\nregex with an \"i\", giving something like \"gitdir/i/re:\". Now you would\nwant to drive it by matching \"gitdir\" first, and then collecting the\n\"/i/re\" independently, to avoid an explosion of matches.\n\nDriving that with a table is much more complex. I'd just as soon keep\nthings as simple as possible for now and worry about flagging it in\nreview when something new gets added.\n\n-Peff\n"},{"id":"312681","messageId":"20170226061220.nizc4xeucx6ks6vq@sigill.intra.peff.net","threadId":"45200","inReplyTo":"20170224131425.32409-4-pclouds@gmail.com","subject":"Re: [PATCH v6 1/1] config: add conditional include","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-02-26T06:12:20Z","receivedAt":"2017-02-26T06:22:48Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 24, 2017 at 08:14:25PM +0700, Nguyễn Thái Ngọc Duy wrote:\n\n> +static int include_condition_is_true(const char *cond, size_t cond_len)\n> +{\n> +\t/* no condition (i.e., \"include.path\") is always true */\n> +\tif (!cond)\n> +\t\treturn 1;\n> +\n> +\tif (skip_prefix_mem(cond, cond_len, \"gitdir:\", &cond, &cond_len))\n> +\t\treturn include_by_gitdir(cond, cond_len, 0);\n> +\telse if (skip_prefix_mem(cond, cond_len, \"gitdir/i:\", &cond, &cond_len))\n> +\t\treturn include_by_gitdir(cond, cond_len, 1);\n> +\n> +\terror(_(\"unrecognized include condition: %.*s\"), (int)cond_len, cond);\n> +\t/* unknown conditionals are always false */\n> +\treturn 0;\n> +}\n\nThis \"error()\" is a nice warning to people who have misspelled the\ncondition (or are surprised that an old version does not respect their\ncondition). But it seems out of place with the rest of the compatibility\nstrategy, which is to quietly ignore include types we don't understand.\n\nFor example, if your patch ships in v2.13 and we add the \"foo:\"\ncondition in v2.14, then with config like:\n\n [includeif \"foo:bar\"]\n path = whatever\n\nyou get:\n\n v2.12: silently ignore\n v2.13: loudly ignore\n v2.14: works\n\nwhich seems odd.\n\nIf we do keep it, it should probably be warning(). I would expect\n\"error:\" to indicate that some requested operation could not be\ncompleted, but this is just \"by the way, I ignored this garbage\".\n\n-Peff\n\nPS I know we try to avoid dashes in the section names, but \"includeIf\"\n and \"includeif\" just look really ugly to me. Maybe it is just me,\n though.\n"},{"id":"312685","messageId":"744E670407404147B290392DAD79DF44@PhilipOakley","threadId":"45200","inReplyTo":"CACsJy8Bq-P8Aw+f0omndCmQU54n_p7ZwDVDK9APuH_3vXDac2g@mail.gmail.com","subject":"Re: [PATCH v6 1/1] config: add conditional include","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":null,"receivedAt":"2017-02-26T12:28:06Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Duy Nguyen\" \n> On Sat, Feb 25, 2017 at 5:08 AM, Philip Oakley \n> wrote:\n>>> +Conditional includes\n>>> +~~~~~~~~~~~~~~~~~~~~\n>>> +\n>>> +You can include one config file from another conditionally by setting\n>>\n>>\n>> On first reading I thought this implied you can only have one `includeIf`\n>> within the config file.\n>> I think it is meant to mean that each `includeIf`could include one other\n>> file, and that users can have multiple `includeIf` lines.\n>\n> Yes. Not sure how to put it better though (I basically copied the\n> first paragraph from the unconditional include section above, which\n> shares the same confusion). Perhaps just write \"the variable can be\n> specified multiple times\"? Or \"multiple variables include multiple\n> times, the last variable does not override the previous ones\"?\n> -- \n\nMy attempt, based on updating the `Includes` section would be something \nlike:\n\n`You can include a config file from another by setting the special \n`include.path` variable to the name of the file to be included. The variable \ntakes a pathname as its value, and is subject to tilde expansion. \n`include.path` supports multiple key values.`\n\nThe subtle change was to s/one/a/ at the start, and then add the final short \nsentence that states that the section's variables can have multiple key \nvalues.\n\nI copied the 'multiple key values' phrase from the man page intro for \nconsitency, though 'multivalued' could just as easily be used as it is the \nterm used by the 'Configuration File' section that this is part of \nhttps://git-scm.com/docs/git-config#_configuration_file.\n\nEven shorter may be:\n`You can include a config file from another by setting the special \n`include.path` variable to the name of the file to be included. The variable \n(can be multivalued) takes a pathname as its value, and is subject to tilde \nexpansion.`\n\n\nThe Conditional Includes would follow suit.\n\nPhilip\n\n\n\n\n\n"},{"id":"312752","messageId":"xmqqinnvxyog.fsf@gitster.mtv.corp.google.com","threadId":"45200","inReplyTo":"20170226060726.mvdip4vnbdngypzx@sigill.intra.peff.net","subject":"Re: [PATCH v5 1/1] config: add conditional include","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-02-27T18:42:55Z","receivedAt":"2017-02-27T18:51:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I don't think driving that with a two-entry table is the right thing\n> here. We are as likely to add another \"foobar:\" entry as we are to add\n> another modifier \"/i\" modifier to \"gitdir:\", and it is unclear whether\n> that modifier would be mutually exclusive with \"/i\".\n\nOK, I didn't take /i as something that was meant as a modifier; I\ntook the \"gitdir:\" and \"gitdir/i:\" are totally different tests that\nare spelled similarly, but for the implementation expediency, called\ninto a single helper function without having a layer that presents\nthe same function signature in the middle to make it drivable by a\ntable.\n\nLet's leave it to the review of a future patch that wants to add a\nthird condition then. At that time, we will have more things to\nlook at to make a better decision.\n\n\n\n"},{"id":"312937","messageId":"20170301112631.16497-2-pclouds@gmail.com","threadId":"45200","inReplyTo":"20170301112631.16497-1-pclouds@gmail.com","subject":"[PATCH v7 1/3] config.txt: clarify multiple key values in include.path","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-03-01T11:26:29Z","receivedAt":"2017-03-01T11:27:22Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"The phrasing in this paragraph may give an impression that you can only\nuse it once. Rephrase it a bit.\n\nHelped-by: Philip Oakley \nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n Documentation/config.txt | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 015346c417..4748efbf36 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -79,10 +79,10 @@ escape sequences) are invalid.\n Includes\n ~~~~~~~~\n \n-You can include one config file from another by setting the special\n+You can include a config file from another by setting the special\n `include.path` variable to the name of the file to be included. The\n variable takes a pathname as its value, and is subject to tilde\n-expansion.\n+expansion. `include.path` supports multiple key values.\n \n The\n included file is expanded immediately, as if its contents had been\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"312938","messageId":"20170301112631.16497-3-pclouds@gmail.com","threadId":"45200","inReplyTo":"20170301112631.16497-1-pclouds@gmail.com","subject":"[PATCH v7 2/3] config.txt: reflow the second include.path paragraph","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-03-01T11:26:30Z","receivedAt":"2017-03-01T11:27:23Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy \n---\n Documentation/config.txt | 9 ++++-----\n 1 file changed, 4 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 4748efbf36..1fad746efd 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -84,12 +84,11 @@ You can include a config file from another by setting the special\n variable takes a pathname as its value, and is subject to tilde\n expansion. `include.path` supports multiple key values.\n \n-The\n-included file is expanded immediately, as if its contents had been\n+The included file is expanded immediately, as if its contents had been\n found at the location of the include directive. If the value of the\n-`include.path` variable is a relative path, the path is considered to be\n-relative to the configuration file in which the include directive was\n-found. See below for examples.\n+`include.path` variable is a relative path, the path is considered to\n+be relative to the configuration file in which the include directive\n+was found. See below for examples.\n \n \n Example\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"312939","messageId":"20170301112631.16497-1-pclouds@gmail.com","threadId":"45200","inReplyTo":"20170224131425.32409-1-pclouds@gmail.com","subject":"[PATCH v7 0/3] Conditional config include","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-03-01T11:26:28Z","receivedAt":"2017-03-01T11:27:31Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"v7 changes:\n\n - typo fix here and there\n - delete a dead TODO comment\n - delete the error on unrecognized condition (we probably want a\n better, general mechanism for troubleshooting config keys anyway)\n - some fix up on include.path document because the \"one or many\"\n confusion started from there\n\nI don't have a good answer for Jeff's PS about includeIf ugliness. I\nagree that includeif is ugly but includeIf looks a bit better. I don't\nsee a better option (if only \"include\" does not start or end with a\nvowel...). Maybe includewith? Suggestions are welcome.\n\nInterdiff below (just on patch 3, not the first two)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex aab3df04fb..2a41e84bab 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -93,9 +93,11 @@ was found. See below for examples.\n Conditional includes\n ~~~~~~~~~~~~~~~~~~~~\n \n-You can include one config file from another conditionally by setting\n-a `includeIf..path` variable to the name of the file to be\n-included. The variable's value is treated the same way as `include.path`.\n+You can include a config file from another conditionally by setting a\n+`includeIf..path` variable to the name of the file to be\n+included. The variable's value is treated the same way as\n+`include.path`. `includeIf.path` supports multiple key\n+values.\n \n The condition starts with a keyword followed by a colon and some data\n whose format and meaning depends on the keyword. Supported keywords\n@@ -104,13 +106,14 @@ are:\n `gitdir`::\n \n \tThe data that follows the keyword `gitdir:` is used as a glob\n-\tpattern. If the location of the .git directory match the\n+\tpattern. If the location of the .git directory matches the\n \tpattern, the include condition is met.\n +\n-The .git location which may be auto-discovered, or come from\n-`$GIT_DIR` environment variable. If the repository auto discovered via\n-a .git file (e.g. from submodules, or a linked worktree), the .git\n-location would be the final location, not where the .git file is.\n+The .git location may be auto-discovered, or come from `$GIT_DIR`\n+environment variable. If the repository is auto discovered via a .git\n+file (e.g. from submodules, or a linked worktree), the .git location\n+would be the final location where the .git directory is, not where the\n+.git file is.\n +\n The pattern can contain standard globbing wildcards and two additional\n ones, `**/` and `/**`, that can match multiple path components. Please\n@@ -169,15 +172,15 @@ Example\n \t\tpath = ~/foo ; expand \"foo\" in your `$HOME` directory\n \n \t; include if $GIT_DIR is /path/to/foo/.git\n-\t[include-if \"gitdir:/path/to/foo/.git\"]\n+\t[includeIf \"gitdir:/path/to/foo/.git\"]\n \t\tpath = /path/to/foo.inc\n \n \t; include for all repositories inside /path/to/group\n-\t[include-if \"gitdir:/path/to/group/\"]\n+\t[includeIf \"gitdir:/path/to/group/\"]\n \t\tpath = /path/to/foo.inc\n \n \t; include for all repositories inside $HOME/to/group\n-\t[include-if \"gitdir:~/to/group/\"]\n+\t[includeIf \"gitdir:~/to/group/\"]\n \t\tpath = /path/to/foo.inc\n \n Values\ndiff --git a/config.c b/config.c\nindex ad16802c8a..0dac0f4cb2 100644\n--- a/config.c\n+++ b/config.c\n@@ -191,7 +191,6 @@ static int prepare_include_condition_pattern(struct strbuf *pat)\n \t\t\treturn error(_(\"relative config include \"\n \t\t\t\t \"conditionals must come from files\"));\n \n-\t\t/* TODO: escape wildcards */\n \t\tstrbuf_add_absolute_path(&path, cf->path);\n \t\tslash = find_last_dir_sep(path.buf);\n \t\tif (!slash)\n@@ -245,16 +244,12 @@ static int include_by_gitdir(const char *cond, size_t cond_len, int icase)\n \n static int include_condition_is_true(const char *cond, size_t cond_len)\n {\n-\t/* no condition (i.e., \"include.path\") is always true */\n-\tif (!cond)\n-\t\treturn 1;\n \n \tif (skip_prefix_mem(cond, cond_len, \"gitdir:\", &cond, &cond_len))\n \t\treturn include_by_gitdir(cond, cond_len, 0);\n \telse if (skip_prefix_mem(cond, cond_len, \"gitdir/i:\", &cond, &cond_len))\n \t\treturn include_by_gitdir(cond, cond_len, 1);\n \n-\terror(_(\"unrecognized include condition: %.*s\"), (int)cond_len, cond);\n \t/* unknown conditionals are always false */\n \treturn 0;\n }\n@@ -278,7 +273,7 @@ int git_config_include(const char *var, const char *value, void *data)\n \t\tret = handle_path_include(value, inc);\n \n \tif (!parse_config_key(var, \"includeif\", &cond, &cond_len, &key) &&\n-\t include_condition_is_true(cond, cond_len) &&\n+\t (cond && include_condition_is_true(cond, cond_len)) &&\n \t !strcmp(key, \"path\"))\n \t\tret = handle_path_include(value, inc);\n\nNguyễn Thái Ngọc Duy (3):\n config.txt: clarify multiple key values in include.path\n config.txt: reflow the second include.path paragraph\n config: add conditional include\n\n Documentation/config.txt | 77 +++++++++++++++++++++++++++++++++++----\n config.c | 92 +++++++++++++++++++++++++++++++++++++++++++++++\n t/t1305-config-include.sh | 56 +++++++++++++++++++++++++++++\n 3 files changed, 218 insertions(+), 7 deletions(-)\n\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"312940","messageId":"20170301112631.16497-4-pclouds@gmail.com","threadId":"45200","inReplyTo":"20170301112631.16497-1-pclouds@gmail.com","subject":"[PATCH v7 3/3] config: add conditional include","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2017-03-01T11:26:31Z","receivedAt":"2017-03-01T11:28:44Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Sometimes a set of repositories want to share configuration settings\namong themselves that are distinct from other such sets of repositories.\nA user may work on two projects, each of which have multiple\nrepositories, and use one user.email for one project while using another\nfor the other.\n\nSetting $GIT_DIR/.config works, but if the penalty of forgetting to\nupdate $GIT_DIR/.config is high (especially when you end up cloning\noften), it may not be the best way to go. Having the settings in\n~/.gitconfig, which would work for just one set of repositories, would\nnot well in such a situation. Having separate ${HOME}s may add more\nproblems than it solves.\n\nExtend the include.path mechanism that lets a config file include\nanother config file, so that the inclusion can be done only when some\nconditions hold. Then ~/.gitconfig can say \"include config-project-A\nonly when working on project-A\" for each project A the user works on.\n\nIn this patch, the only supported grouping is based on $GIT_DIR (in\nabsolute path), so you would need to group repositories by directory, or\nsomething like that to take advantage of it.\n\nWe already have include.path for unconditional includes. This patch goes\nwith includeIf..path to make it clearer that a condition is\nrequired. The new config has the same backward compatibility approach as\ninclude.path: older git versions that don't understand includeIf will\nsimply ignore them.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n Documentation/config.txt | 64 +++++++++++++++++++++++++++++++++\n config.c | 92 +++++++++++++++++++++++++++++++++++++++++++++++\n t/t1305-config-include.sh | 56 +++++++++++++++++++++++++++++\n 3 files changed, 212 insertions(+)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 1fad746efd..2a41e84bab 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -90,6 +90,59 @@ found at the location of the include directive. If the value of the\n be relative to the configuration file in which the include directive\n was found. See below for examples.\n \n+Conditional includes\n+~~~~~~~~~~~~~~~~~~~~\n+\n+You can include a config file from another conditionally by setting a\n+`includeIf..path` variable to the name of the file to be\n+included. The variable's value is treated the same way as\n+`include.path`. `includeIf.path` supports multiple key\n+values.\n+\n+The condition starts with a keyword followed by a colon and some data\n+whose format and meaning depends on the keyword. Supported keywords\n+are:\n+\n+`gitdir`::\n+\n+\tThe data that follows the keyword `gitdir:` is used as a glob\n+\tpattern. If the location of the .git directory matches the\n+\tpattern, the include condition is met.\n++\n+The .git location may be auto-discovered, or come from `$GIT_DIR`\n+environment variable. If the repository is auto discovered via a .git\n+file (e.g. from submodules, or a linked worktree), the .git location\n+would be the final location where the .git directory is, not where the\n+.git file is.\n++\n+The pattern can contain standard globbing wildcards and two additional\n+ones, `**/` and `/**`, that can match multiple path components. Please\n+refer to linkgit:gitignore[5] for details. For convenience:\n+\n+ * If the pattern starts with `~/`, `~` will be substituted with the\n+ content of the environment variable `HOME`.\n+\n+ * If the pattern starts with `./`, it is replaced with the directory\n+ containing the current config file.\n+\n+ * If the pattern does not start with either `~/`, `./` or `/`, `**/`\n+ will be automatically prepended. For example, the pattern `foo/bar`\n+ becomes `**/foo/bar` and would match `/any/path/to/foo/bar`.\n+\n+ * If the pattern ends with `/`, `**` will be automatically added. For\n+ example, the pattern `foo/` becomes `foo/**`. In other words, it\n+ matches \"foo\" and everything inside, recursively.\n+\n+`gitdir/i`::\n+\tThis is the same as `gitdir` except that matching is done\n+\tcase-insensitively (e.g. on case-insensitive file sytems)\n+\n+A few more notes on matching via `gitdir` and `gitdir/i`:\n+\n+ * Symlinks in `$GIT_DIR` are not resolved before matching.\n+\n+ * Note that \"../\" is not special and will match literally, which is\n+ unlikely what you want.\n \n Example\n ~~~~~~~\n@@ -118,6 +171,17 @@ Example\n \t\tpath = foo ; expand \"foo\" relative to the current file\n \t\tpath = ~/foo ; expand \"foo\" in your `$HOME` directory\n \n+\t; include if $GIT_DIR is /path/to/foo/.git\n+\t[includeIf \"gitdir:/path/to/foo/.git\"]\n+\t\tpath = /path/to/foo.inc\n+\n+\t; include for all repositories inside /path/to/group\n+\t[includeIf \"gitdir:/path/to/group/\"]\n+\t\tpath = /path/to/foo.inc\n+\n+\t; include for all repositories inside $HOME/to/group\n+\t[includeIf \"gitdir:~/to/group/\"]\n+\t\tpath = /path/to/foo.inc\n \n Values\n ~~~~~~\ndiff --git a/config.c b/config.c\nindex c6b874a7bf..0dac0f4cb2 100644\n--- a/config.c\n+++ b/config.c\n@@ -13,6 +13,7 @@\n #include \"hashmap.h\"\n #include \"string-list.h\"\n #include \"utf8.h\"\n+#include \"dir.h\"\n \n struct config_source {\n \tstruct config_source *prev;\n@@ -170,9 +171,94 @@ static int handle_path_include(const char *path, struct config_include_data *inc\n \treturn ret;\n }\n \n+static int prepare_include_condition_pattern(struct strbuf *pat)\n+{\n+\tstruct strbuf path = STRBUF_INIT;\n+\tchar *expanded;\n+\tint prefix = 0;\n+\n+\texpanded = expand_user_path(pat->buf);\n+\tif (expanded) {\n+\t\tstrbuf_reset(pat);\n+\t\tstrbuf_addstr(pat, expanded);\n+\t\tfree(expanded);\n+\t}\n+\n+\tif (pat->buf[0] == '.' && is_dir_sep(pat->buf[1])) {\n+\t\tconst char *slash;\n+\n+\t\tif (!cf || !cf->path)\n+\t\t\treturn error(_(\"relative config include \"\n+\t\t\t\t \"conditionals must come from files\"));\n+\n+\t\tstrbuf_add_absolute_path(&path, cf->path);\n+\t\tslash = find_last_dir_sep(path.buf);\n+\t\tif (!slash)\n+\t\t\tdie(\"BUG: how is this possible?\");\n+\t\tstrbuf_splice(pat, 0, 1, path.buf, slash - path.buf);\n+\t\tprefix = slash - path.buf + 1 /* slash */;\n+\t} else if (!is_absolute_path(pat->buf))\n+\t\tstrbuf_insert(pat, 0, \"**/\", 3);\n+\n+\tif (pat->len && is_dir_sep(pat->buf[pat->len - 1]))\n+\t\tstrbuf_addstr(pat, \"**\");\n+\n+\tstrbuf_release(&path);\n+\treturn prefix;\n+}\n+\n+static int include_by_gitdir(const char *cond, size_t cond_len, int icase)\n+{\n+\tstruct strbuf text = STRBUF_INIT;\n+\tstruct strbuf pattern = STRBUF_INIT;\n+\tint ret = 0, prefix;\n+\n+\tstrbuf_add_absolute_path(&text, get_git_dir());\n+\tstrbuf_add(&pattern, cond, cond_len);\n+\tprefix = prepare_include_condition_pattern(&pattern);\n+\n+\tif (prefix < 0)\n+\t\tgoto done;\n+\n+\tif (prefix > 0) {\n+\t\t/*\n+\t\t * perform literal matching on the prefix part so that\n+\t\t * any wildcard character in it can't create side effects.\n+\t\t */\n+\t\tif (text.len < prefix)\n+\t\t\tgoto done;\n+\t\tif (!icase && strncmp(pattern.buf, text.buf, prefix))\n+\t\t\tgoto done;\n+\t\tif (icase && strncasecmp(pattern.buf, text.buf, prefix))\n+\t\t\tgoto done;\n+\t}\n+\n+\tret = !wildmatch(pattern.buf + prefix, text.buf + prefix,\n+\t\t\t icase ? WM_CASEFOLD : 0, NULL);\n+\n+done:\n+\tstrbuf_release(&pattern);\n+\tstrbuf_release(&text);\n+\treturn ret;\n+}\n+\n+static int include_condition_is_true(const char *cond, size_t cond_len)\n+{\n+\n+\tif (skip_prefix_mem(cond, cond_len, \"gitdir:\", &cond, &cond_len))\n+\t\treturn include_by_gitdir(cond, cond_len, 0);\n+\telse if (skip_prefix_mem(cond, cond_len, \"gitdir/i:\", &cond, &cond_len))\n+\t\treturn include_by_gitdir(cond, cond_len, 1);\n+\n+\t/* unknown conditionals are always false */\n+\treturn 0;\n+}\n+\n int git_config_include(const char *var, const char *value, void *data)\n {\n \tstruct config_include_data *inc = data;\n+\tconst char *cond, *key;\n+\tint cond_len;\n \tint ret;\n \n \t/*\n@@ -185,6 +271,12 @@ int git_config_include(const char *var, const char *value, void *data)\n \n \tif (!strcmp(var, \"include.path\"))\n \t\tret = handle_path_include(value, inc);\n+\n+\tif (!parse_config_key(var, \"includeif\", &cond, &cond_len, &key) &&\n+\t (cond && include_condition_is_true(cond, cond_len)) &&\n+\t !strcmp(key, \"path\"))\n+\t\tret = handle_path_include(value, inc);\n+\n \treturn ret;\n }\n \ndiff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh\nindex 9ba2ba11c3..f0cd2056ba 100755\n--- a/t/t1305-config-include.sh\n+++ b/t/t1305-config-include.sh\n@@ -152,6 +152,62 @@ test_expect_success 'relative includes from stdin line fail' '\n \ttest_must_fail git config --file - test.one\n '\n \n+test_expect_success 'conditional include, both unanchored' '\n+\tgit init foo &&\n+\t(\n+\t\tcd foo &&\n+\t\techo \"[includeIf \\\"gitdir:foo/\\\"]path=bar\" >>.git/config &&\n+\t\techo \"[test]one=1\" >.git/bar &&\n+\t\techo 1 >expect &&\n+\t\tgit config test.one >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+test_expect_success 'conditional include, $HOME expansion' '\n+\t(\n+\t\tcd foo &&\n+\t\techo \"[includeIf \\\"gitdir:~/foo/\\\"]path=bar2\" >>.git/config &&\n+\t\techo \"[test]two=2\" >.git/bar2 &&\n+\t\techo 2 >expect &&\n+\t\tgit config test.two >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+test_expect_success 'conditional include, full pattern' '\n+\t(\n+\t\tcd foo &&\n+\t\techo \"[includeIf \\\"gitdir:**/foo/**\\\"]path=bar3\" >>.git/config &&\n+\t\techo \"[test]three=3\" >.git/bar3 &&\n+\t\techo 3 >expect &&\n+\t\tgit config test.three >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+test_expect_success 'conditional include, relative path' '\n+\techo \"[includeIf \\\"gitdir:./foo/.git\\\"]path=bar4\" >>.gitconfig &&\n+\techo \"[test]four=4\" >bar4 &&\n+\t(\n+\t\tcd foo &&\n+\t\techo 4 >expect &&\n+\t\tgit config test.four >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+test_expect_success 'conditional include, both unanchored, icase' '\n+\t(\n+\t\tcd foo &&\n+\t\techo \"[includeIf \\\"gitdir/i:FOO/\\\"]path=bar5\" >>.git/config &&\n+\t\techo \"[test]five=5\" >.git/bar5 &&\n+\t\techo 5 >expect &&\n+\t\tgit config test.five >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n test_expect_success 'include cycles are detected' '\n \tcat >.gitconfig <<-\\EOF &&\n \t[test]value = gitconfig\n-- \n2.11.0.157.gd943d85\n\n"},{"id":"312951","messageId":"32efbc7b-182e-2169-78a7-c959661f8207@ramsayjones.plus.com","threadId":"45200","inReplyTo":"20170301112631.16497-4-pclouds@gmail.com","subject":"Re: [PATCH v7 3/3] config: add conditional include","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2017-03-01T17:16:28Z","receivedAt":"2017-03-01T17:16:41Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 01/03/17 11:26, Nguyễn Thái Ngọc Duy wrote:\n> Sometimes a set of repositories want to share configuration settings\n> among themselves that are distinct from other such sets of repositories.\n> A user may work on two projects, each of which have multiple\n> repositories, and use one user.email for one project while using another\n> for the other.\n> \n> Setting $GIT_DIR/.config works, but if the penalty of forgetting to\n> update $GIT_DIR/.config is high (especially when you end up cloning\n> often), it may not be the best way to go. Having the settings in\n> ~/.gitconfig, which would work for just one set of repositories, would\n> not well in such a situation. Having separate ${HOME}s may add more\n> problems than it solves.\n> \n> Extend the include.path mechanism that lets a config file include\n> another config file, so that the inclusion can be done only when some\n> conditions hold. Then ~/.gitconfig can say \"include config-project-A\n> only when working on project-A\" for each project A the user works on.\n> \n> In this patch, the only supported grouping is based on $GIT_DIR (in\n> absolute path), so you would need to group repositories by directory, or\n> something like that to take advantage of it.\n> \n> We already have include.path for unconditional includes. This patch goes\n> with includeIf..path to make it clearer that a condition is\n> required. The new config has the same backward compatibility approach as\n> include.path: older git versions that don't understand includeIf will\n> simply ignore them.\n> \n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n> Documentation/config.txt | 64 +++++++++++++++++++++++++++++++++\n> config.c | 92 +++++++++++++++++++++++++++++++++++++++++++++++\n> t/t1305-config-include.sh | 56 +++++++++++++++++++++++++++++\n> 3 files changed, 212 insertions(+)\n> \n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index 1fad746efd..2a41e84bab 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -90,6 +90,59 @@ found at the location of the include directive. If the value of the\n> be relative to the configuration file in which the include directive\n> was found. See below for examples.\n> \n> +Conditional includes\n> +~~~~~~~~~~~~~~~~~~~~\n> +\n> +You can include a config file from another conditionally by setting a\n> +`includeIf..path` variable to the name of the file to be\n> +included. The variable's value is treated the same way as\n> +`include.path`. `includeIf.path` supports multiple key\n----------------------------------------^^\ns/path/.path/\n\nATB,\nRamsay Jones\n"},{"id":"312955","messageId":"xmqqshmwhp3q.fsf@gitster.mtv.corp.google.com","threadId":"45200","inReplyTo":"20170301112631.16497-2-pclouds@gmail.com","subject":"Re: [PATCH v7 1/3] config.txt: clarify multiple key values in include.path","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-01T17:40:57Z","receivedAt":"2017-03-01T17:50:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> The phrasing in this paragraph may give an impression that you can only\n> use it once. Rephrase it a bit.\n>\n> Helped-by: Philip Oakley \n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n> Documentation/config.txt | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index 015346c417..4748efbf36 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -79,10 +79,10 @@ escape sequences) are invalid.\n> Includes\n> ~~~~~~~~\n> \n> -You can include one config file from another by setting the special\n> +You can include a config file from another by setting the special\n> `include.path` variable to the name of the file to be included. The\n> variable takes a pathname as its value, and is subject to tilde\n> -expansion.\n> +expansion. `include.path` supports multiple key values.\n\nMultiple key values or just simply multiple values? I think it is\nthe latter.\n\nBut to the intended target audience, I think\n\n\t`include.path` can be given multiple times.\n\nis easier to understand. It's not like you can (or want to)\nenumerate with \"git config --get-all include.path\" to learn all the\nvalues (for the single key \"include.value\"), and it is better not to\nlead readers to think of this in terms of in the first\nplace (which is already clarified in the text that follows).\n\n> The\n> included file is expanded immediately, as if its contents had been\n"},{"id":"312956","messageId":"xmqqk288hokz.fsf@gitster.mtv.corp.google.com","threadId":"45200","inReplyTo":"20170301112631.16497-1-pclouds@gmail.com","subject":"Re: [PATCH v7 0/3] Conditional config include","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-01T17:52:12Z","receivedAt":"2017-03-01T17:52:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> Nguyễn Thái Ngọc Duy (3):\n> config.txt: clarify multiple key values in include.path\n> config.txt: reflow the second include.path paragraph\n> config: add conditional include\n\nThanks. The primary change looked good (it looked good already in\nthe previous round).\n\nHere is a list of minor suggestions on top.\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 2a41e84bab..5faabc7934 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -82,7 +82,7 @@ Includes\n You can include a config file from another by setting the special\n `include.path` variable to the name of the file to be included. The\n variable takes a pathname as its value, and is subject to tilde\n-expansion. `include.path` supports multiple key values.\n+expansion. `include.path` can be given multiple times.\n \n The included file is expanded immediately, as if its contents had been\n found at the location of the include directive. If the value of the\n@@ -96,8 +96,7 @@ Conditional includes\n You can include a config file from another conditionally by setting a\n `includeIf..path` variable to the name of the file to be\n included. The variable's value is treated the same way as\n-`include.path`. `includeIf.path` supports multiple key\n-values.\n+`include.path`. `includeIf..path` can be given multiple times.\n \n The condition starts with a keyword followed by a colon and some data\n whose format and meaning depends on the keyword. Supported keywords\n"},{"id":"312967","messageId":"xmqqo9xkhosj.fsf@gitster.mtv.corp.google.com","threadId":"45200","inReplyTo":"20170301112631.16497-4-pclouds@gmail.com","subject":"Re: [PATCH v7 3/3] config: add conditional include","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-01T17:47:40Z","receivedAt":"2017-03-01T19:30:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> int git_config_include(const char *var, const char *value, void *data)\n> {\n> \tstruct config_include_data *inc = data;\n> +\tconst char *cond, *key;\n> +\tint cond_len;\n> \tint ret;\n> \n> \t/*\n> @@ -185,6 +271,12 @@ int git_config_include(const char *var, const char *value, void *data)\n> \n> \tif (!strcmp(var, \"include.path\"))\n> \t\tret = handle_path_include(value, inc);\n> +\n> +\tif (!parse_config_key(var, \"includeif\", &cond, &cond_len, &key) &&\n> +\t (cond && include_condition_is_true(cond, cond_len)) &&\n> +\t !strcmp(key, \"path\"))\n> +\t\tret = handle_path_include(value, inc);\n> +\n> \treturn ret;\n> }\n\nSo \"includeif.path\" (misspelled one without any condition) falls\nthrough to \"return ret\" and gives the value we got from inc->fn().\nI am OK with that (i.e. \"missing condition is false\").\n\nOr we can make it go to handle_path_include(), effectively making\nthe \"include.path\" a short-hand for \"includeIf.path\". I am also OK\nwith that (i.e. \"missing condition is true\").\n\nOr we could even have \"include.[.]path\" without\n\"includeIf\"? I am not sure if it is a bad idea that paints\nourselves in a corner, but somehow I find it tempting.\n"},{"id":"313135","messageId":"20170303063329.ji6do6eqjbpuwmxz@sigill.intra.peff.net","threadId":"45200","inReplyTo":"20170301112631.16497-1-pclouds@gmail.com","subject":"Re: [PATCH v7 0/3] Conditional config include","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-03-03T06:33:29Z","receivedAt":"2017-03-03T06:40:55Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Mar 01, 2017 at 06:26:28PM +0700, Nguyễn Thái Ngọc Duy wrote:\n\n> I don't have a good answer for Jeff's PS about includeIf ugliness. I\n> agree that includeif is ugly but includeIf looks a bit better. I don't\n> see a better option (if only \"include\" does not start or end with a\n> vowel...). Maybe includewith? Suggestions are welcome.\n\nI actually think \"include-if\" _looks_ better, although maybe the\ninconsistency with \"-\" is something we don't want to encourage (though I\nalso think the implicit include..path was OK, too). Feel free to\njust ignore me. I will live with it either way.\n\nFor those following on the mailing list, there is some discussion at:\n\n https://github.com/git/git/commit/484f78e46d00c6d35f20058671a8c76bb924fb33\n\nI think that is mostly focused around another failing in the\nerror-handling of the config code, and that does not need to be\naddressed by this series (though of course I'd welcome any fixes).\n\nBut there's a test failure that probably does need to be dealt with\nbefore this graduates to 'next'.\n\n-Peff\n"},{"id":"313137","messageId":"20170303063052.xkjoipuaohcwdl4n@sigill.intra.peff.net","threadId":"45200","inReplyTo":"xmqqo9xkhosj.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v7 3/3] config: add conditional include","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-03-03T06:30:52Z","receivedAt":"2017-03-03T06:57:43Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Mar 01, 2017 at 09:47:40AM -0800, Junio C Hamano wrote:\n\n> > @@ -185,6 +271,12 @@ int git_config_include(const char *var, const char *value, void *data)\n> > \n> > \tif (!strcmp(var, \"include.path\"))\n> > \t\tret = handle_path_include(value, inc);\n> > +\n> > +\tif (!parse_config_key(var, \"includeif\", &cond, &cond_len, &key) &&\n> > +\t (cond && include_condition_is_true(cond, cond_len)) &&\n> > +\t !strcmp(key, \"path\"))\n> > +\t\tret = handle_path_include(value, inc);\n> > +\n> > \treturn ret;\n> > }\n> \n> So \"includeif.path\" (misspelled one without any condition) falls\n> through to \"return ret\" and gives the value we got from inc->fn().\n> I am OK with that (i.e. \"missing condition is false\").\n\nYeah, I think that is sensible, just as not-understood nonsense like\n\"include.foobar\" would be ignored, as well.\n\n> Or we can make it go to handle_path_include(), effectively making\n> the \"include.path\" a short-hand for \"includeIf.path\". I am also OK\n> with that (i.e. \"missing condition is true\").\n\nI think we want \"missing condition is false\". Certainly an empty\ncondition like \"includeIf..path\" is false, as are conditions we don't\nunderstand.\n\n> Or we could even have \"include.[.]path\" without\n> \"includeIf\"? I am not sure if it is a bad idea that paints\n> ourselves in a corner, but somehow I find it tempting.\n\nThat was how I had originally envisioned the namespace working when I\nintroduced include.path long ago. And I think Duy's v1 used that, but\nthe feedback was that it was not sufficiently obvious that the\nsubsection was a conditional.\n\n-Peff\n"},{"id":"313149","messageId":"20170303095252.GA12072@ash","threadId":"45200","inReplyTo":"20170303063329.ji6do6eqjbpuwmxz@sigill.intra.peff.net","subject":"Re: [PATCH v7 0/3] Conditional config include","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-03-03T09:52:52Z","receivedAt":"2017-03-03T09:53:20Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Mar 03, 2017 at 01:33:29AM -0500, Jeff King wrote:\n> For those following on the mailing list, there is some discussion at:\n> \n> https://github.com/git/git/commit/484f78e46d00c6d35f20058671a8c76bb924fb33\n> \n> I think that is mostly focused around another failing in the\n> error-handling of the config code, and that does not need to be\n> addressed by this series (though of course I'd welcome any fixes).\n> \n> But there's a test failure that probably does need to be dealt with\n> before this graduates to 'next'.\n\nThe patch to fix it is this\n\ndiff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh\nindex f0cd2056ba..e833939320 100755\n--- a/t/t1305-config-include.sh\n+++ b/t/t1305-config-include.sh\n@@ -102,7 +102,7 @@ test_expect_success 'config modification does not affect includes' '\n \n test_expect_success 'missing include files are ignored' '\n \tcat >.gitconfig <<-\\EOF &&\n-\t[include]path = foo\n+\t[include]path = non-existent\n \t[test]value = yes\n \tEOF\n \techo yes >expect &&\n\nJunio could you squash this in?\n\nA lot more explanation is in another series [1] I just sent. Not sure\nif we want to repeat the same in this commit's message.\n\nAnd this series without that squash, when combined with [1], will fail\nthis test even on Linux. Good thing imo.\n\n[1] http://public-inbox.org/git/20170303094252.11706-1-pclouds@gmail.com/\n--\nDuy\n"},{"id":"313214","messageId":"xmqqlgsm9q2i.fsf@gitster.mtv.corp.google.com","threadId":"45200","inReplyTo":"20170303063329.ji6do6eqjbpuwmxz@sigill.intra.peff.net","subject":"Re: [PATCH v7 0/3] Conditional config include","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-03T18:24:05Z","receivedAt":"2017-03-03T18:24:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> For those following on the mailing list, there is some discussion at:\n>\n> https://github.com/git/git/commit/484f78e46d00c6d35f20058671a8c76bb924fb33\n>\n> I think that is mostly focused around another failing in the\n> error-handling of the config code, and that does not need to be\n> addressed by this series (though of course I'd welcome any fixes).\n\nThanks. Without a message like this, the list may have never known\nabout the discussion taken elsewhere. I'd appreciate such a report\nto appear on list the next time much earlier ;-)\n\nWhen built with FREAD_READS_DIRECTORIES=Yes on Linux, the error in\nthe test can easily reproduce.\n\nIn early days of UNIX it was sometimes handy to be able to read the\nbytes off of directory to \"investigate\", but we are not a filesystem\napplication, and I do not offhand see any reason why we should be\nrelying on being able to successfully fopen() a directory for\nreading. A FILE * successfully opened that just returns EOF when\nread is totally useless for any purpose anyway. \n\nWhen the path to be opened from the end user (either from the\ncommand line or in a configuration file) is a directory, it is\nbetter to diagnose it as a user error, and if the path was computed\nby our code, it may be a bug.\n\nI am wondering if we should enable this on Linux, at least in\nDEVELOPER builds but possibly even on the release builds, to catch\nthese problems more easily.\n\n\n"},{"id":"313220","messageId":"xmqq4lza9o5r.fsf@gitster.mtv.corp.google.com","threadId":"45200","inReplyTo":"20170303063052.xkjoipuaohcwdl4n@sigill.intra.peff.net","subject":"Re: [PATCH v7 3/3] config: add conditional include","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-03T19:05:20Z","receivedAt":"2017-03-03T19:14:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> That was how I had originally envisioned the namespace working when I\n> introduced include.path long ago. And I think Duy's v1 used that, but\n> the feedback was that it was not sufficiently obvious that the\n> subsection was a conditional.\n\nI am not sure about \"obviousness\", but I agree that we do not know\nthat \"conditional include\" would be the only thing we want for the\nsecond level for \"include.path\" directive. \"include-if..path\"\nis better for that reason.\n\nI presume that you could still do\n\n\t[include \"if:gitdir=$path\"]\n\t\tpath = ...\n\ni.e. design the second level to begin with a token that tells\nreaders what it means (and assign \"if:\" token for \"conditional\ninclude\"), but I do not think it is worth it.\n\nI also imagine that\n\n\t[include]\n\t\tcondition = ...\n\t\tpath = ...\n\nis easier to read and write by end-users, but it probably is not\nfeasible because it requires too invasive change to the current code\nto teach it to grok such construct.\n\nBetween \"include-if\" and \"includeIf\", if people find the latter not\ntoo ugly, I'd prefer to keep it the way Duy posted. Because of the\nway \"include.path\" and \"include-if..path\" work, we can declare\nthat they are not like ordinary configuration variable definition\nat all but are higher-level directives and that may be a sufficient\njustification to allow \"-\" in its name, though, if people find\n\"includeIf\" too ugly a name to live.\n\n"},{"id":"313221","messageId":"xmqqpohy897y.fsf@gitster.mtv.corp.google.com","threadId":"45200","inReplyTo":"20170303095252.GA12072@ash","subject":"Re: [PATCH v7 0/3] Conditional config include","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-03T19:13:21Z","receivedAt":"2017-03-03T19:14:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> The patch to fix it is this\n>\n> diff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh\n> index f0cd2056ba..e833939320 100755\n> --- a/t/t1305-config-include.sh\n> +++ b/t/t1305-config-include.sh\n> @@ -102,7 +102,7 @@ test_expect_success 'config modification does not affect includes' '\n> \n> test_expect_success 'missing include files are ignored' '\n> \tcat >.gitconfig <<-\\EOF &&\n> -\t[include]path = foo\n> +\t[include]path = non-existent\n> \t[test]value = yes\n> \tEOF\n> \techo yes >expect &&\n>\n> Junio could you squash this in?\n\nSure.\n"},{"id":"313234","messageId":"xmqqwpc66pbh.fsf@gitster.mtv.corp.google.com","threadId":"45200","inReplyTo":"20170303095252.GA12072@ash","subject":"Re: [PATCH v7 0/3] Conditional config include","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-03T21:08:34Z","receivedAt":"2017-03-03T21:13:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> On Fri, Mar 03, 2017 at 01:33:29AM -0500, Jeff King wrote:\n>> For those following on the mailing list, there is some discussion at:\n>> \n>> https://github.com/git/git/commit/484f78e46d00c6d35f20058671a8c76bb924fb33\n>> \n>> I think that is mostly focused around another failing in the\n>> error-handling of the config code, and that does not need to be\n>> addressed by this series (though of course I'd welcome any fixes).\n>> \n>> But there's a test failure that probably does need to be dealt with\n>> before this graduates to 'next'.\n>\n> The patch to fix it is this\n>\n> diff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh\n> index f0cd2056ba..e833939320 100755\n> --- a/t/t1305-config-include.sh\n> +++ b/t/t1305-config-include.sh\n> @@ -102,7 +102,7 @@ test_expect_success 'config modification does not affect includes' '\n> \n> test_expect_success 'missing include files are ignored' '\n> \tcat >.gitconfig <<-\\EOF &&\n> -\t[include]path = foo\n> +\t[include]path = non-existent\n> \t[test]value = yes\n> \tEOF\n> \techo yes >expect &&\n>\n> Junio could you squash this in?\n>\n\nI said yes, but I won't, as the series already has SQUASH??? for\nother things, so I'll just queue this at the tip. I _might_ get\ninclined to do the squashing and cleaning up myself later, but\nplease don't hold your breath.\n\n"},{"id":"313242","messageId":"20170303222456.ouujfjwusk4l45n4@sigill.intra.peff.net","threadId":"45200","inReplyTo":"xmqq4lza9o5r.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v7 3/3] config: add conditional include","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-03-03T22:24:57Z","receivedAt":"2017-03-03T22:25:04Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Mar 03, 2017 at 11:05:20AM -0800, Junio C Hamano wrote:\n\n> I am not sure about \"obviousness\", but I agree that we do not know\n> that \"conditional include\" would be the only thing we want for the\n> second level for \"include.path\" directive. \"include-if..path\"\n> is better for that reason.\n> \n> I presume that you could still do\n> \n> \t[include \"if:gitdir=$path\"]\n> \t\tpath = ...\n> \n> i.e. design the second level to begin with a token that tells\n> readers what it means (and assign \"if:\" token for \"conditional\n> include\"), but I do not think it is worth it.\n\nYep, all true.\n\n> I also imagine that\n> \n> \t[include]\n> \t\tcondition = ...\n> \t\tpath = ...\n> \n> is easier to read and write by end-users, but it probably is not\n> feasible because it requires too invasive change to the current code\n> to teach it to grok such construct.\n\nI am against that as it introduces a dependency in the presence and\nordering between two config variables, which can yield some surprises.\n\n> Between \"include-if\" and \"includeIf\", if people find the latter not\n> too ugly, I'd prefer to keep it the way Duy posted. Because of the\n> way \"include.path\" and \"include-if..path\" work, we can declare\n> that they are not like ordinary configuration variable definition\n> at all but are higher-level directives and that may be a sufficient\n> justification to allow \"-\" in its name, though, if people find\n> \"includeIf\" too ugly a name to live.\n\nOK. I can live with includeIf.\n\n-Peff\n"},{"id":"313243","messageId":"20170303222215.vwulypicszoh2hds@sigill.intra.peff.net","threadId":"45200","inReplyTo":"xmqqlgsm9q2i.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v7 0/3] Conditional config include","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-03-03T22:22:15Z","receivedAt":"2017-03-03T22:29:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Mar 03, 2017 at 10:24:05AM -0800, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > For those following on the mailing list, there is some discussion at:\n> >\n> > https://github.com/git/git/commit/484f78e46d00c6d35f20058671a8c76bb924fb33\n> >\n> > I think that is mostly focused around another failing in the\n> > error-handling of the config code, and that does not need to be\n> > addressed by this series (though of course I'd welcome any fixes).\n> \n> Thanks. Without a message like this, the list may have never known\n> about the discussion taken elsewhere. I'd appreciate such a report\n> to appear on list the next time much earlier ;-)\n> \n> When built with FREAD_READS_DIRECTORIES=Yes on Linux, the error in\n> the test can easily reproduce.\n\nHeh. I had no idea we had FREAD_READS_DIRECTORIES. I think Duy and I\nreinvented it in another thread. ;)\n\nI agree that may be worth setting on Linux (though note that we _do_\nignore other stdio read errors in the rest of the code path, which may\nbe something we want to address).\n\n-Peff\n"},{"id":"313248","messageId":"xmqqlgsm6j4k.fsf@gitster.mtv.corp.google.com","threadId":"45200","inReplyTo":"20170303222215.vwulypicszoh2hds@sigill.intra.peff.net","subject":"Re: [PATCH v7 0/3] Conditional config include","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-03T23:22:19Z","receivedAt":"2017-03-03T23:28:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Heh. I had no idea we had FREAD_READS_DIRECTORIES. I think Duy and I\n> reinvented it in another thread. ;)\n\nYou two were not alone. I was planning to reinvent it before I went\nto bed last night and then found it already was there this morning ;-)\n\n> I agree that may be worth setting on Linux (though note that we _do_\n> ignore other stdio read errors in the rest of the code path, which may\n> be something we want to address).\n\nYes, we need an error check on fopen() in git_config_from_file()\nregardless.\n\n"},{"id":"313390","messageId":"CAGZ79kboaKfzMEyDjg-m2oK8CX6B56i52ZcWhCaq87ECE9x2Dw@mail.gmail.com","threadId":"45200","inReplyTo":"20170223122346.12222-2-pclouds@gmail.com","subject":"Re: [PATCH v5 1/1] config: add conditional include","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-03-06T22:44:27Z","receivedAt":"2017-03-07T00:37:47Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Being late to the review party here.\n\n> +static int include_condition_is_true(const char *cond, size_t cond_len)\n> +{\n...\n> +\n> + error(_(\"unrecognized include condition: %.*s\"), (int)cond_len, cond);\n> + /* unknown conditionals are always false */\n> + return 0;\n> +}\n\nThanks for putting an error message here. I was looking at what\nis currently queued as origin/nd/conditional-config-include,\nwhich doesn't have this error() (yet / not any more?)\n\nI'd strongly suggest to keep the error message here as that way\na user can diagnose e.g. a typo in the condition easily.\n\nIf we plan to extend this list of conditions in the future, and a user\nswitches between versions of git, then they may see this message\non a regular basis (whenever they use the 'old' version).\n\nIn that case it may be only a warning() instead of an error(),\nbut I have no strong opinion on that.\n\n---\nReason why I am reviewing this series now:\n\nI thought about extending the config system for submodule\nusage (see debate at [1]).\n\nMy gut reaction was to have a condition for \"if a superproject\nexists\" and then include a special config (e.g. \"config.super\"\nin the superprojects $GIT_DIR).\n\nHowever while reviewing these patches I realized\nI am not interested in conditional includes, but when setting\nup the submodules we know for a fact that we have a superproject,\nso no conditional needed. Instead we need a special markup\nof paths, i.e. we want to have an easy way to say\n\"$GIT_DIR of superproject\". Ideas how to do that?\n\nThanks,\nStefan\n\n[1] https://public-inbox.org/git/84fcb0bd-85dc-0142-dd58-47a04eaa7c2b@durchholz.org/\n"},{"id":"313407","messageId":"20170307084717.i2jru77v3rhd443e@sigill.intra.peff.net","threadId":"45200","inReplyTo":"CAGZ79kboaKfzMEyDjg-m2oK8CX6B56i52ZcWhCaq87ECE9x2Dw@mail.gmail.com","subject":"Re: [PATCH v5 1/1] config: add conditional include","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-03-07T08:47:18Z","receivedAt":"2017-03-07T11:34:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 06, 2017 at 02:44:27PM -0800, Stefan Beller wrote:\n\n> > +static int include_condition_is_true(const char *cond, size_t cond_len)\n> > +{\n> ...\n> > +\n> > + error(_(\"unrecognized include condition: %.*s\"), (int)cond_len, cond);\n> > + /* unknown conditionals are always false */\n> > + return 0;\n> > +}\n> \n> Thanks for putting an error message here. I was looking at what\n> is currently queued as origin/nd/conditional-config-include,\n> which doesn't have this error() (yet / not any more?)\n\nIt's \"not any more\". It was in the original and I asked for it to be\nremoved during the last review.\n\n> I'd strongly suggest to keep the error message here as that way\n> a user can diagnose e.g. a typo in the condition easily.\n> \n> If we plan to extend this list of conditions in the future, and a user\n> switches between versions of git, then they may see this message\n> on a regular basis (whenever they use the 'old' version).\n\nThat would make it unlike the rest of the config-include mechanism\n(which quietly ignores things it doesn't understand, like include.foo,\nor include.foo.path), as well as the config code in general (which\nignores misspelt keys).\n\nSome of that \"quiet when you don't understand it\" is historical\nnecessity. Older versions _can't_ complain about not knowing\ninclude.path, because they don't yet know it's worth complaining about.\nLikewise here, if this ships in v2.13 and a new condition \"foo:\" ships\nin v2.14, you get:\n\n v2.12 - no complaint; we don't even understand includeIf at all\n v2.13 - complain; we know includeIf, but not \"foo:\"\n v2.14 - works as expected\n\nWhich is kind of weird and inconsistent. But maybe the typo-detection\ncase is more important to get right than consistency across historical\nversions.\n\n-Peff\n"},{"id":"313440","messageId":"CAGZ79kbMP5n3Pteq1OtOzK69dZkv1QmpiFm8jhYV7+LKVHvi3A@mail.gmail.com","threadId":"45200","inReplyTo":"20170307084717.i2jru77v3rhd443e@sigill.intra.peff.net","subject":"Re: [PATCH v5 1/1] config: add conditional include","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-03-07T18:39:28Z","receivedAt":"2017-03-07T18:48:40Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Mar 7, 2017 at 12:47 AM, Jeff King wrote:\n> On Mon, Mar 06, 2017 at 02:44:27PM -0800, Stefan Beller wrote:\n>\n>> > +static int include_condition_is_true(const char *cond, size_t cond_len)\n>> > +{\n>> ...\n>> > +\n>> > + error(_(\"unrecognized include condition: %.*s\"), (int)cond_len, cond);\n>> > + /* unknown conditionals are always false */\n>> > + return 0;\n>> > +}\n>>\n>> Thanks for putting an error message here. I was looking at what\n>> is currently queued as origin/nd/conditional-config-include,\n>> which doesn't have this error() (yet / not any more?)\n>\n> It's \"not any more\". It was in the original and I asked for it to be\n> removed during the last review.\n\nOkay. The joys of contradicting opinions on a mailing list. :)\n\n>\n>> I'd strongly suggest to keep the error message here as that way\n>> a user can diagnose e.g. a typo in the condition easily.\n>>\n>> If we plan to extend this list of conditions in the future, and a user\n>> switches between versions of git, then they may see this message\n>> on a regular basis (whenever they use the 'old' version).\n>\n> That would make it unlike the rest of the config-include mechanism\n> (which quietly ignores things it doesn't understand, like include.foo,\n> or include.foo.path), as well as the config code in general (which\n> ignores misspelt keys).\n>\n> Some of that \"quiet when you don't understand it\" is historical\n> necessity. Older versions _can't_ complain about not knowing\n> include.path, because they don't yet know it's worth complaining about.\n\nagreed\n\n> Likewise here, if this ships in v2.13 and a new condition \"foo:\" ships\n> in v2.14, you get:\n>\n> v2.12 - no complaint; we don't even understand includeIf at all\n> v2.13 - complain; we know includeIf, but not \"foo:\"\n> v2.14 - works as expected\n>\n> Which is kind of weird and inconsistent. But maybe the typo-detection\n> case is more important to get right than consistency across historical\n> versions.\n\nOh, I see. I was contemplating a future in which 2.12 is not used anymore.\n\nWhen looking at other examples, such as url.<...>.insteadOf we also do not\nwarn about typos (well we can't actually).\n\nIn diff..(command/binary/..) we know the limited set of drivers,\nwhich is similar to the situation we have here.\n\nMaybe a compromise between typo checking (edit distance < 2 -> warn;\nsilent for larger distances) and the consistency over time is desired.\nBut this is even more code to write.\n\nSo for now I retract my strong opinion and be happy with what is\npresented as-is for the reasons Peff gave.\n\nThanks,\nStefan\n"}]}