{"thread":{"id":"42714","subject":"[PATCH] config: add conditional include","startedAt":"2016-06-26T07:07:25Z","lastAt":"2016-08-24T14:17:31Z","messageCount":46,"participants":["Nguyễn Thái Ngọc Duy","Jeff King","Duy Nguyen","Matthieu Moy","Junio C Hamano","Eric Sunshine","Johannes Schindelin","Jakub Narębski"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"290183","messageId":"20160626070617.30211-1-pclouds@gmail.com","threadId":"42714","inReplyTo":null,"subject":"[PATCH] config: add conditional include","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-06-26T07:06:17Z","receivedAt":"2016-06-26T07:07:25Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"If the path argument in \"include\" starts with \"gitdir:\", it is\nfollowed by a wildmatch pattern. The include is only effective if\n$GIT_DIR matches the pattern. This is very useful to add configuration\nto a group of repositories.\n\nFor convenience\n\n - \"~\" is expanded to $USER\n\n - if the pattern ends with '/', \"**\" will be appended (e.g. foo/\n becomes foo/**). In other words, \"foo/\" automatically matches\n everything in starting with \"foo/\".\n\n - if the pattern contains no slashes, it's wrapped around by \"**/\"\n and \"/**\" (e.g. \"foo\" becomes \"**/foo/**\"). In other words, \"foo\"\n matches any directory component in $GIT_DIR.\n\nThe combination of the first two is used to group repositories by\npath. While the last one could be used to match worktree's basename.\n\nThis code is originally written by Jeff King [1]. All genius designs\nare his. All bugs are mine (claiming bugs is just more fun :).\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/273811/focus=273825\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n Original thread is [1]. Sebastian may not need it but I do and not\n just for user.* stuff. So I'm bringing it back. I deleted Jeff's\n de-anchoring and replaced with something a bit more restrictive. Once\n we settle that, I'll add tests and stuff.\n\n config.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--\n 1 file changed, 55 insertions(+), 2 deletions(-)\n\ndiff --git a/config.c b/config.c\nindex f51c56b..dd55a5f 100644\n--- a/config.c\n+++ b/config.c\n@@ -140,9 +140,58 @@ static int handle_path_include(const char *path, struct config_include_data *inc\n \treturn ret;\n }\n \n+static int include_condition_is_true(const char *cond, int cond_len)\n+{\n+\tconst char *value;\n+\n+\t/* no condition (i.e., \"include.path\") is always true */\n+\tif (!cond)\n+\t\treturn 1;\n+\n+\t/*\n+\t * It's OK to run over cond_len in our checks here, as that just pushes\n+\t * us past the final \".\", which cannot match any of our prefixes.\n+\t */\n+\tif (skip_prefix(cond, \"gitdir:\", &value)) {\n+\t\tstruct strbuf text = STRBUF_INIT;\n+\t\tstruct strbuf pattern = STRBUF_INIT;\n+\t\tchar *buf;\n+\t\tint ret;\n+\n+\t\tstrbuf_add_absolute_path(&text, get_git_dir());\n+\n+\t\tstrbuf_add(&pattern, value, cond_len - (value - cond));\n+\t\tbuf = expand_user_path(pattern.buf);\n+\t\tif (buf) {\n+\t\t\tstrbuf_reset(&pattern);\n+\t\t\tstrbuf_addstr(&pattern, buf);\n+\t\t\tfree(buf);\n+\t\t}\n+\n+\t\tif (pattern.len && pattern.buf[pattern.len - 1] == '/') {\n+\t\t\t/* foo/ matches recursively */\n+\t\t\tstrbuf_addstr(&pattern, \"**\");\n+\t\t} else if (!strchr(pattern.buf, '/')) {\n+\t\t\t/* no slashes match one directory component */\n+\t\t\tstrbuf_insert(&pattern, 0, \"**/\", 3);\n+\t\t\tstrbuf_addstr(&pattern, \"/**\");\n+\t\t}\n+\n+\t\tret = !wildmatch(pattern.buf, text.buf, 0, NULL);\n+\t\tstrbuf_release(&pattern);\n+\t\tstrbuf_release(&text);\n+\t\treturn ret;\n+\t}\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@@ -153,8 +202,12 @@ int git_config_include(const char *var, const char *value, void *data)\n \tif (ret < 0)\n \t\treturn ret;\n \n-\tif (!strcmp(var, \"include.path\"))\n-\t\tret = handle_path_include(value, inc);\n+\tif (!parse_config_key(var, \"include\", &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 \n-- \n2.8.2.526.g02eed6d\n\n"},{"id":"290201","messageId":"20160626182715.GA12546@sigill.intra.peff.net","threadId":"42714","inReplyTo":"20160626070617.30211-1-pclouds@gmail.com","subject":"Re: [PATCH] config: add conditional include","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-26T18:27:16Z","receivedAt":"2016-06-26T18:28:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Jun 26, 2016 at 09:06:17AM +0200, Nguyễn Thái Ngọc Duy wrote:\n\n> If the path argument in \"include\" starts with \"gitdir:\", it is\n> followed by a wildmatch pattern. The include is only effective if\n> $GIT_DIR matches the pattern. This is very useful to add configuration\n> to a group of repositories.\n\nI think this needs some more introduction to the concept. When you say\n\"path argument\" here, I assumed you meant the value of include.path. But\nyou really mean: we are introducing a new concept for the \"subsection\"\nfield of include.*, which is to provide restrictions for conditional\nincludes.\n\nIt also may be worth discussing the motivation or examples.\n\n> For convenience\n> \n> - \"~\" is expanded to $USER\n> \n> - if the pattern ends with '/', \"**\" will be appended (e.g. foo/\n> becomes foo/**). In other words, \"foo/\" automatically matches\n> everything in starting with \"foo/\".\n> \n> - if the pattern contains no slashes, it's wrapped around by \"**/\"\n> and \"/**\" (e.g. \"foo\" becomes \"**/foo/**\"). In other words, \"foo\"\n> matches any directory component in $GIT_DIR.\n>\n> The combination of the first two is used to group repositories by\n> path. While the last one could be used to match worktree's basename.\n\nThis is a nice description, but it probably belongs in the\ndocumentation.\n\nI don't have any real opinion on the rules themselves, though they seem\nreasonable to me (though in the first one I assume you mean $HOME).\n\n> This code is originally written by Jeff King [1]. All genius designs\n> are his. All bugs are mine (claiming bugs is just more fun :).\n\nHeh. I have written this code in a \"something like this\" form at least 3\ntimes over the years. Conditional includes were always something I\nplanned into the original scheme, but had never actually found a good\nuse for it. ;)\n\n> +\t/*\n> +\t * It's OK to run over cond_len in our checks here, as that just pushes\n> +\t * us past the final \".\", which cannot match any of our prefixes.\n> +\t */\n> +\tif (skip_prefix(cond, \"gitdir:\", &value)) {\n\nThis would benefit from the skip_prefix_mem I proposed in:\n\n http://article.gmane.org/gmane.comp.version-control.git/298050\n\n(and which is ae989a61dad98debe9899823ca987305f8e8020d in Junio's tree,\nthough it is only in pu so far, I think).\n\nThat eliminates the need for the comment, and auto-update cond_len, so\nthat later:\n\n> +\t\tstrbuf_add(&pattern, value, cond_len - (value - cond));\n\n...you do not have to do extra computation to get the correct length.\n\nThis is a tangent, but I wonder if expand_user_path() should take a\nbuf/len. It always puts the result into a new strbuf anyway, so it would\nnot be a big deal to do so. Skimming the output of grep, though, it\nlooks like this might be the only caller that would be helped.\n\n> +\t\tbuf = expand_user_path(pattern.buf);\n> +\t\tif (buf) {\n> +\t\t\tstrbuf_reset(&pattern);\n> +\t\t\tstrbuf_addstr(&pattern, buf);\n> +\t\t\tfree(buf);\n> +\t\t}\n\nMaybe strbuf_attach() would be shorter here?\n\n> +\t\t} else if (!strchr(pattern.buf, '/')) {\n> +\t\t\t/* no slashes match one directory component */\n> +\t\t\tstrbuf_insert(&pattern, 0, \"**/\", 3);\n> +\t\t\tstrbuf_addstr(&pattern, \"/**\");\n> +\t\t}\n\nI guess it's a little funny that \"foo\" and \"foo/bar\" are matched quite\ndifferently. I wonder if a simpler rule would just be: relative paths\nare unanchored.\n\n-Peff\n"},{"id":"290246","messageId":"CACsJy8DAMe3YH-f_Qm8FEmanXepnwF2z1L6DDGoJf1eysmW2xQ@mail.gmail.com","threadId":"42714","inReplyTo":"20160626182715.GA12546@sigill.intra.peff.net","subject":"Re: [PATCH] config: add conditional include","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-06-27T16:14:17Z","receivedAt":"2016-06-27T16:14:54Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sun, Jun 26, 2016 at 8:27 PM, Jeff King wrote:\n> On Sun, Jun 26, 2016 at 09:06:17AM +0200, Nguyễn Thái Ngọc Duy wrote:\n>\n>> If the path argument in \"include\" starts with \"gitdir:\", it is\n>> followed by a wildmatch pattern. The include is only effective if\n>> $GIT_DIR matches the pattern. This is very useful to add configuration\n>> to a group of repositories.\n>\n> I think this needs some more introduction to the concept. When you say\n> \"path argument\" here, I assumed you meant the value of include.path. But\n> you really mean: we are introducing a new concept for the \"subsection\"\n> field of include.*, which is to provide restrictions for conditional\n> includes.\n\nYep.\n\n>\n> It also may be worth discussing the motivation or examples.\n>\n>> For convenience\n>>\n>> - \"~\" is expanded to $USER\n>>\n>> - if the pattern ends with '/', \"**\" will be appended (e.g. foo/\n>> becomes foo/**). In other words, \"foo/\" automatically matches\n>> everything in starting with \"foo/\".\n>>\n>> - if the pattern contains no slashes, it's wrapped around by \"**/\"\n>> and \"/**\" (e.g. \"foo\" becomes \"**/foo/**\"). In other words, \"foo\"\n>> matches any directory component in $GIT_DIR.\n>>\n>> The combination of the first two is used to group repositories by\n>> path. While the last one could be used to match worktree's basename.\n>\n> This is a nice description, but it probably belongs in the\n> documentation.\n\nYeah.. just too lazy for proper documentation at this stage.\n\n>\n> I don't have any real opinion on the rules themselves, though they seem\n> reasonable to me (though in the first one I assume you mean $HOME).\n\nYep $HOME, what was I thinking...\n\n(skipping all the technical suggestions, will do.. most likely)\n\n>> + } else if (!strchr(pattern.buf, '/')) {\n>> + /* no slashes match one directory component */\n>> + strbuf_insert(&pattern, 0, \"**/\", 3);\n>> + strbuf_addstr(&pattern, \"/**\");\n>> + }\n>\n> I guess it's a little funny that \"foo\" and \"foo/bar\" are matched quite\n> differently. I wonder if a simpler rule would just be: relative paths\n> are unanchored.\n\nI modeled it after .gitignore patterns, but that's probably not a good\nfit here. Making all relative paths un-anchored means I can't say\n\"paths that end with this suffix\". How useful that statement is, I\ncan't say though. Or if you mean only prepend \"**/\" to relative paths,\nnot \"/**\" then that door is still open.\n\n(after a couple more minutes..) hmm.. I think that \"paths that end\nwith ...\" may have its use. The degenerated case is \"match $(basename\n)\", not very useful for gitdir when most of the time $(basename)\nis \".git\". It could be useful for \"worktree:\" matching and would\nreduce false positives a bit (compared to un-anchoring both ends).\nConditionally including some config per worktree this way is also an\ninteresting way to deal with per-worktree config, but I need more time\nto think about that...\n-- \nDuy\n"},{"id":"290249","messageId":"20160627162052.GA4532@sigill.intra.peff.net","threadId":"42714","inReplyTo":"CACsJy8DAMe3YH-f_Qm8FEmanXepnwF2z1L6DDGoJf1eysmW2xQ@mail.gmail.com","subject":"Re: [PATCH] config: add conditional include","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-27T16:20:53Z","receivedAt":"2016-06-27T16:20:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 27, 2016 at 06:14:17PM +0200, Duy Nguyen wrote:\n\n> >> + } else if (!strchr(pattern.buf, '/')) {\n> >> + /* no slashes match one directory component */\n> >> + strbuf_insert(&pattern, 0, \"**/\", 3);\n> >> + strbuf_addstr(&pattern, \"/**\");\n> >> + }\n> >\n> > I guess it's a little funny that \"foo\" and \"foo/bar\" are matched quite\n> > differently. I wonder if a simpler rule would just be: relative paths\n> > are unanchored.\n> \n> I modeled it after .gitignore patterns, but that's probably not a good\n> fit here. Making all relative paths un-anchored means I can't say\n> \"paths that end with this suffix\". How useful that statement is, I\n> can't say though. Or if you mean only prepend \"**/\" to relative paths,\n> not \"/**\" then that door is still open.\n\nI didn't really mean anything, as I had not thought about it that\ncarefully. :)\n\nYou do allow distinguishing the suffix thing with \"/\" at the end in the\nrule above, though. So between the two rules:\n\n - slash at the end is a shorthand for \"/**\"\n\n - no-slash at the beginning (i.e., a non-absolute path) is a shorthand\n for \"**/\" at the beginning\n\nyou should be able to specify anything.\n\nI do agree that there's value in trying to make the rules consistent\nwith other parts of git, though. I don't know the corner cases of\ngitignore and gitattributes well enough to compare off the top of my\nhead, though (though I suspect you do. :) ).\n\n-Peff\n"},{"id":"290252","messageId":"CACsJy8D6TxkMWJTUCeDd8tuMyTRXcvWdMw4VL3nO7jbLEgjE-A@mail.gmail.com","threadId":"42714","inReplyTo":"20160627162052.GA4532@sigill.intra.peff.net","subject":"Re: [PATCH] config: add conditional include","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-06-27T16:32:39Z","receivedAt":"2016-06-27T16:33:39Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Jun 27, 2016 at 6:20 PM, Jeff King wrote:\n> You do allow distinguishing the suffix thing with \"/\" at the end in the\n> rule above, though. So between the two rules:\n>\n> - slash at the end is a shorthand for \"/**\"\n>\n> - no-slash at the beginning (i.e., a non-absolute path) is a shorthand\n> for \"**/\" at the beginning\n\nNeither slash or \"./\" at the beginning...\n\n> you should be able to specify anything.\n\n...then yeah it looks pretty good. With the exception of \"./\" we can\nstill have paths relative to where the the config file is. For\n$HOME/.gitconfig that eliminates the need for expanding \"~\"\n($HOME/.config/git/config may still need it, unless we allow \"../\"\ntoo, but that complicates matching).\n\n> I do agree that there's value in trying to make the rules consistent\n> with other parts of git, though. I don't know the corner cases of\n> gitignore and gitattributes well enough to compare off the top of my\n> head, though (though I suspect you do. :) ).\n\nNaah the complication of .gitignore and .gitattributes have long\nexceeded my brain's capacity.\n-- \nDuy\n"},{"id":"290253","messageId":"20160627163559.GA5296@sigill.intra.peff.net","threadId":"42714","inReplyTo":"CACsJy8D6TxkMWJTUCeDd8tuMyTRXcvWdMw4VL3nO7jbLEgjE-A@mail.gmail.com","subject":"Re: [PATCH] config: add conditional include","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-27T16:35:59Z","receivedAt":"2016-06-27T16:36:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 27, 2016 at 06:32:39PM +0200, Duy Nguyen wrote:\n\n> On Mon, Jun 27, 2016 at 6:20 PM, Jeff King wrote:\n> > You do allow distinguishing the suffix thing with \"/\" at the end in the\n> > rule above, though. So between the two rules:\n> >\n> > - slash at the end is a shorthand for \"/**\"\n> >\n> > - no-slash at the beginning (i.e., a non-absolute path) is a shorthand\n> > for \"**/\" at the beginning\n> \n> Neither slash or \"./\" at the beginning...\n> \n> > you should be able to specify anything.\n> \n> ...then yeah it looks pretty good. With the exception of \"./\" we can\n> still have paths relative to where the the config file is. For\n> $HOME/.gitconfig that eliminates the need for expanding \"~\"\n> ($HOME/.config/git/config may still need it, unless we allow \"../\"\n> too, but that complicates matching).\n\nYeah, I didn't think actual relative paths were that interesting, but if\nwe declare that they are relative to the config file, I agree that\nworks pretty well. And I agree that \"./\" is a convenient way to anchor\nthem.\n\n-Peff\n"},{"id":"290398","messageId":"20160628172641.26381-1-pclouds@gmail.com","threadId":"42714","inReplyTo":"20160626070617.30211-1-pclouds@gmail.com","subject":"[PATCH v2 0/2] Config conditional include","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-06-28T17:26:39Z","receivedAt":"2016-06-28T17:32:50Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Second try. The anchoring rules now look a lot better. I cherry picked\nJeff's skip_prefix_mem() for now to avoid dependency.\n\nThere's a surprise about core.ignorecase. We are matching paths, so we\nshould match case-insensitively if core.ignorecase tells us so. And it\ngets a bit tricky if core.ignorecase is defined in the same config\nfile. I don't think we have ever told the user that keys are processed\nfrom top down. We do now.\n\nIt makes me wonder if we should allow users the option to match case\ninsensitively regardless of filesystem too. Something close to\npathspec magic. But that probably has little use in real world for a\nlot more work.\n\nThe '/' vs '\\\\' battle on Windows, I'll leave it to Windows guys again.\n\nI'm also not adding \"worktree:\" condition. If that happens, it\nprobably happens in the per-worktree config file series.\n\nTo leave room for future expansion, perhaps we should declare that ':'\ncan't appear in the pattern, so we can add more prefix later, and even\nstack them, e.g. \"regexp:gitdir:\". The prefix can't be one char\nlong. That should be enough for windows to specify full path\nincluding the drive letter.\n\nJeff King (1):\n add skip_prefix_mem helper\n\nNguyễn Thái Ngọc Duy (1):\n config: add conditional include\n\n Documentation/config.txt | 40 +++++++++++++++++++++\n config.c | 89 +++++++++++++++++++++++++++++++++++++++++++++--\n git-compat-util.h | 17 +++++++++\n t/t1305-config-include.sh | 45 ++++++++++++++++++++++++\n 4 files changed, 189 insertions(+), 2 deletions(-)\n\n-- \n2.8.2.531.gd073806\n\n"},{"id":"290399","messageId":"20160628172641.26381-3-pclouds@gmail.com","threadId":"42714","inReplyTo":"20160628172641.26381-1-pclouds@gmail.com","subject":"[PATCH v2 2/2] config: add conditional include","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-06-28T17:26:41Z","receivedAt":"2016-06-28T17:41:29Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Main description is already in config.txt. Here is a dev-only note\nabout Windows support.\n\nWhile prepare_include_condition_pattern() is Windows-friendly (because\nit does not hard code '/'). The reality could be uglier because\ninternally get_git_dir() may return a path with '/' only or worse, a\nmix of '/' and '\\\\'.\n\nAt some point, we need to teach wildmatch() that '/' and '\\' should be\ntreated the same way (via a flag) as well. Then we could care less\nabout '/' vs '\\\\'. But a Windows dev probably has to do it.\n\nHelped-by: Jeff King \nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n Documentation/config.txt | 40 +++++++++++++++++++++\n config.c | 89 +++++++++++++++++++++++++++++++++++++++++++++--\n t/t1305-config-include.sh | 45 ++++++++++++++++++++++++\n 3 files changed, 172 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 58673cf..c8ad0bf 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -91,6 +91,46 @@ 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+Included files can be grouped into subsections where subsectino name\n+is the condition when the files are included. The condition starts\n+with an condition type string, followed by a colon and a pattern.\n+\n+Only \"gitdir\" type is supported, where files are included if\n+`$GIT_DIR` matches the specified pattern. For example,\n+\n+\n+\t[include \"gitdir:/path/to/foo.git\"]\n+\t\tpath = /path/to/foo.inc\n+\n+would only include \"/path/to/foo.inc\" if `$GIT_DIR` is\n+/path/to/foo.git.\n+\n+The following pattern is a wildcard pattern with two additional\n+wildcards `**/` and `/**`. See linkgit:gitignore[5] for more\n+information. For convenience:\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+ * If the pattern starts with `~/`, `~` will be substitued with the\n+ environment variable `HOME`.\n+\n+ * If the pattern starts with `./`, it is replaced with the directory\n+ where the current config file is. For example if the config file\n+ that contains the \"include\" subsection is `$HOME/.gitconfig` then\n+ the pattern `./foo` would match the path `$HOME/foo`\n+\n+A few more notes:\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+ * On case-insensitive file systems, you may need to specify\n+ core.ignoreCase before the `include` subsections in order to match\n+ case-insensitively if core.ignoreCase is declared in the same file.\n \n Example\n ~~~~~~~\ndiff --git a/config.c b/config.c\nindex f51c56b..97c450e 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@@ -140,9 +141,89 @@ 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_condition_is_true(const char *cond, int cond_len)\n+{\n+\tconst char *value;\n+\tsize_t value_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:\", &value, &value_len)) {\n+\t\tstruct strbuf text = STRBUF_INIT;\n+\t\tstruct strbuf pattern = STRBUF_INIT;\n+\t\tint ret, prefix;\n+\n+\t\tstrbuf_add_absolute_path(&text, get_git_dir());\n+\t\tstrbuf_add(&pattern, value, value_len);\n+\t\tprefix = prepare_include_condition_pattern(&pattern);\n+\n+\t\tif (prefix < 0)\n+\t\t\treturn 0;\n+\n+\t\tif (prefix > 0 &&\n+\t\t (text.len < prefix ||\n+\t\t fspathncmp(pattern.buf, text.buf, prefix)))\n+\t\t\treturn 0;\n+\n+\t\tret = !wildmatch(pattern.buf + prefix, text.buf + prefix,\n+\t\t\t\t ignore_case ? WM_CASEFOLD : 0,\n+\t\t\t\t NULL);\n+\t\tstrbuf_release(&pattern);\n+\t\tstrbuf_release(&text);\n+\t\treturn ret;\n+\t}\n+\n+\terror(_(\"unrecognized include condition: %.*s\"), 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@@ -153,8 +234,12 @@ int git_config_include(const char *var, const char *value, void *data)\n \tif (ret < 0)\n \t\treturn ret;\n \n-\tif (!strcmp(var, \"include.path\"))\n-\t\tret = handle_path_include(value, inc);\n+\tif (!parse_config_key(var, \"include\", &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 9ba2ba1..30351f2 100755\n--- a/t/t1305-config-include.sh\n+++ b/t/t1305-config-include.sh\n@@ -152,6 +152,51 @@ 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 \\\"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 \\\"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 \\\"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 \\\"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 'include cycles are detected' '\n \tcat >.gitconfig <<-\\EOF &&\n \t[test]value = gitconfig\n-- \n2.8.2.531.gd073806\n\n"},{"id":"290401","messageId":"20160628172641.26381-2-pclouds@gmail.com","threadId":"42714","inReplyTo":"20160628172641.26381-1-pclouds@gmail.com","subject":"[PATCH v2 1/2] add skip_prefix_mem helper","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-06-28T17:26:40Z","receivedAt":"2016-06-28T17:45:54Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"From: Jeff King \n\nThe skip_prefix function has been very useful for\nsimplifying pointer arithmetic and avoiding repeated magic\nnumbers, but we have no equivalent for length-limited\nbuffers. So we're stuck with:\n\n if (3 <= len && skip_prefix(buf, \"foo\", &buf))\n\t len -= 3;\n\nThat's not that complicated, but it needs to use magic\nnumbers for the length of the prefix (or else write out\nstrlen(\"foo\"), repeating the string). By using a helper, we\ncan get the string length behind the scenes (and often at\ncompile time for string literals).\n\nSigned-off-by: Jeff King \nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n git-compat-util.h | 17 +++++++++++++++++\n 1 file changed, 17 insertions(+)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 49d4029..c99cddc 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -473,6 +473,23 @@ static inline int skip_prefix(const char *str, const char *prefix,\n \treturn 0;\n }\n \n+/*\n+ * Like skip_prefix, but promises never to read past \"len\" bytes of the input\n+ * buffer, and returns the remaining number of bytes in \"out\" via \"outlen\".\n+ */\n+static inline int skip_prefix_mem(const char *buf, size_t len,\n+\t\t\t\t const char *prefix,\n+\t\t\t\t const char **out, size_t *outlen)\n+{\n+\tsize_t prefix_len = strlen(prefix);\n+\tif (prefix_len <= len && !memcmp(buf, prefix, prefix_len)) {\n+\t\t*out = buf + prefix_len;\n+\t\t*outlen = len - prefix_len;\n+\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\n+\n /*\n * If buf ends with suffix, return 1 and subtract the length of the suffix\n * from *len. Otherwise, return 0 and leave *len untouched.\n-- \n2.8.2.531.gd073806\n\n"},{"id":"290430","messageId":"20160628202809.GA21002@sigill.intra.peff.net","threadId":"42714","inReplyTo":"20160628172641.26381-1-pclouds@gmail.com","subject":"Re: [PATCH v2 0/2] Config conditional include","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-28T20:28:09Z","receivedAt":"2016-06-28T20:41:55Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 28, 2016 at 07:26:39PM +0200, Nguyễn Thái Ngọc Duy wrote:\n\n> There's a surprise about core.ignorecase. We are matching paths, so we\n> should match case-insensitively if core.ignorecase tells us so. And it\n> gets a bit tricky if core.ignorecase is defined in the same config\n> file. I don't think we have ever told the user that keys are processed\n> from top down. We do now.\n\nHrm. I'm not excited about introducing ordering issues into the config\nparsing. But I think it's actually even more complicated than that.\n\ncore.ignorecase is generally about the working tree, not the git\nrepository directory, which may reside on another filesystem entirely\n(though I would not be surprised if we've blurred that line already).\n\nI wonder if it would be that bad to just punt on the issue, and say that\nthese include-match globs are always case-insensitive, or something.\nTrue, that does not allow one to distinguish between config for \"foo\"\nand \"Foo\" directories, but I find it unlikely anybody would ever want\nto. And if we define it that way from day one, then nobody expects it to\nwork.\n\n> It makes me wonder if we should allow users the option to match case\n> insensitively regardless of filesystem too. Something close to\n> pathspec magic. But that probably has little use in real world for a\n> lot more work.\n\nYeah, if we had a builtin syntax for \"add these options to the\nwildmatch\", it wouldn't be so bad, but I think we'd be inventing that\nsyntax.\n\n> The '/' vs '\\\\' battle on Windows, I'll leave it to Windows guys again.\n\nOof. Me too. :)\n\n> To leave room for future expansion, perhaps we should declare that ':'\n> can't appear in the pattern, so we can add more prefix later, and even\n> stack them, e.g. \"regexp:gitdir:\". The prefix can't be one char\n> long. That should be enough for windows to specify full path\n> including the drive letter.\n\nIt seems unnecessarily restrictive to place rules about what can go in\nthe pattern provided by the user, when we can easily restrict the\ncharacters on the keyword side. For example \"gitdir/regexp:\" is\na natural extension of \"gitdir:\", and is backwards compatible\nas long as we use something besides \":\" for the option separator.\n\n-Peff\n"},{"id":"290432","messageId":"20160628204959.GB21002@sigill.intra.peff.net","threadId":"42714","inReplyTo":"20160628172641.26381-3-pclouds@gmail.com","subject":"Re: [PATCH v2 2/2] config: add conditional include","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-28T20:49:59Z","receivedAt":"2016-06-28T20:50:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 28, 2016 at 07:26:41PM +0200, Nguyễn Thái Ngọc Duy wrote:\n\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index 58673cf..c8ad0bf 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -91,6 +91,46 @@ 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> +Included files can be grouped into subsections where subsectino name\n> +is the condition when the files are included. The condition starts\n> +with an condition type string, followed by a colon and a pattern.\n\ns/subsectino/subsection/\ns/an condition/a condition/\n\nI think the first sentence may parse more easily as:\n\n Included files can be grouped into subsections, where the subsection\n name is a condition that must be met for the files to be included.\n\nI wonder if it would make more sense to refer to \"condition type string\"\nas a \"condition keyword\" or something.\n\nI think we should probably also claim only that the bit to the right of\nthe colon is keyword-specific data. For some conditions it will not be a\nglob, and may not even be a pattern at all. And then each keyword can\ndescribe its syntax (we may end up wanting to factor out the particular\nset of rules, but I think that can wait until we have a second keyword\nthat uses them).\n\n> +Only \"gitdir\" type is supported, where files are included if\n> +`$GIT_DIR` matches the specified pattern. For example,\n\nMaybe start this as a list like:\n\n The currently supported keywords are:\n\n `gitdir`::\n ...\n\nto make it clear that the \"only\" is potentially temporary. We should\nprobably also document that unknown keywords are always treated as\nfalse.\n\n> + * If the pattern starts with `~/`, `~` will be substitued with the\n> + environment variable `HOME`.\n\ns/substitued/substituted/\n\n> + * If the pattern starts with `./`, it is replaced with the directory\n> + where the current config file is. For example if the config file\n\nPerhaps replace \"directory where the current config file is\" with\n\"directory containing the current config file\". Also, perhaps \"config\nfile that contains the include directive\" is more descriptive than\n\"current\" (we use similar language when describing relative include\npaths).\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\nWhy did you drop expand_user_path() here?\n\nI guess it's to do with knowing the length of the prefix portion? I'm\nnot sure I understand why the prefix is necessary. I thought the goal\nwas to construct a wildmatch pattern that could be used against\nget_git_dir().\n\n> +static int include_condition_is_true(const char *cond, int cond_len)\n> +{\n> +\tconst char *value;\n> +\tsize_t value_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:\", &value, &value_len)) {\n\nIt's not wrong to use extra variables, but it's safe to feed the\noriginals, like:\n\n if (skip_prefix_mem(cond, cond_len, \"gitdir:\", &cond, &cond_len))\n\nThe variables are guaranteed to be untouched if we didn't match (so your\nerror() below is fine).\n\n-Peff\n"},{"id":"290433","messageId":"vpqk2h9qbp8.fsf@anie.imag.fr","threadId":"42714","inReplyTo":"20160628202809.GA21002@sigill.intra.peff.net","subject":"Re: [PATCH v2 0/2] Config conditional include","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2016-06-28T20:51:15Z","receivedAt":"2016-06-28T20:51:45Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Jeff King writes:\n\n> On Tue, Jun 28, 2016 at 07:26:39PM +0200, Nguyễn Thái Ngọc Duy wrote:\n>\n>> There's a surprise about core.ignorecase. We are matching paths, so we\n>> should match case-insensitively if core.ignorecase tells us so. And it\n>> gets a bit tricky if core.ignorecase is defined in the same config\n>> file. I don't think we have ever told the user that keys are processed\n>> from top down. We do now.\n>\n> Hrm. I'm not excited about introducing ordering issues into the config\n> parsing.\n\nThere's already at least one case of ordering-sensitive variables, that\nwe encountered when writting the config cache during Tanay Abhra's GSoC:\ndiff..funcname Vs diff..xfuncname. Git applies the \"last\none wins\" policy, which is the normal rule for a single-valued variable,\nbut in this case, a \"funcname\" definition can override an \"xfuncname\"\ndef. To preserve this behavior we had to introduce ordering in the\ncache, but to me this was a design mistake to rely on order.\n\nIn short: we already have one, but I'm not excited either about\nintroducing new ones.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"290435","messageId":"20160628210309.GC21002@sigill.intra.peff.net","threadId":"42714","inReplyTo":"vpqk2h9qbp8.fsf@anie.imag.fr","subject":"Re: [PATCH v2 0/2] Config conditional include","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-28T21:03:09Z","receivedAt":"2016-06-28T21:04:14Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 28, 2016 at 10:51:15PM +0200, Matthieu Moy wrote:\n\n> Jeff King writes:\n> \n> > On Tue, Jun 28, 2016 at 07:26:39PM +0200, Nguyễn Thái Ngọc Duy wrote:\n> >\n> >> There's a surprise about core.ignorecase. We are matching paths, so we\n> >> should match case-insensitively if core.ignorecase tells us so. And it\n> >> gets a bit tricky if core.ignorecase is defined in the same config\n> >> file. I don't think we have ever told the user that keys are processed\n> >> from top down. We do now.\n> >\n> > Hrm. I'm not excited about introducing ordering issues into the config\n> > parsing.\n> \n> There's already at least one case of ordering-sensitive variables, that\n> we encountered when writting the config cache during Tanay Abhra's GSoC:\n> diff..funcname Vs diff..xfuncname. Git applies the \"last\n> one wins\" policy, which is the normal rule for a single-valued variable,\n> but in this case, a \"funcname\" definition can override an \"xfuncname\"\n> def. To preserve this behavior we had to introduce ordering in the\n> cache, but to me this was a design mistake to rely on order.\n> \n> In short: we already have one, but I'm not excited either about\n> introducing new ones.\n\nI still see funcname versus xfuncname as fundamentally a \"last one wins\"\nscenario; it's just that the two options are sort-of synonyms. But we\nare still talking about the same linear-ish parsing scheme, and I think\nit just makes the implementation a little more complicated.\n\nI'm much more worried about something that impacts how we parse the\nconfig, and is set up in a possibly unrelated config-parsing sequence.\nSo whether ignorecase will work depends on more variables:\n\n - are we doing our config parse before or after somebody has called\n git_config() at the start of a program?\n\n - if before (or during), does our callback call git_default_core_config()?\n\n - if so, did core.ignorecase appear before our include? (Almost\n certainly not, if our include is in ~/.gitconfig, because we parse\n from least-specific to most-specific).\n\nSo here it is not the implementation that is complicated, but the\nuser-facing behavior. It's very difficult to predict when your include\nwill kick in, and there is a good chance it will behave differently for\ndifferent programs.\n\nIn general I think the best bet here is to lazy-load such values from\nthe config-cache (so we _know_ that we got a complete parse before we\nlook at the value). But that creates a recursion problem when we try to\nlazy-load from inside the config parser itself.\n\n-Peff\n"},{"id":"290441","messageId":"xmqqk2h96k1q.fsf@gitster.mtv.corp.google.com","threadId":"42714","inReplyTo":"20160628172641.26381-1-pclouds@gmail.com","subject":"Re: [PATCH v2 0/2] Config conditional include","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-28T22:11:13Z","receivedAt":"2016-06-28T22:12:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Honestly, I'd really prefer to see those with topics in 'pu' that\nhas seen reviews but not yet updated to go back to and polish them\nto help move things forward, with the goal to have them in 'next'\nsooner so that we can have fixes and features that are sufficiently\nvetted and tested in the next release, before starting yet another\nnew topic that will be left hanging in 'pu'.\n\nIn your case, I am talking about nd/icase, nd/fetch-ref-summary, and\nnd/connect-ssh-command-config topics.\n\nThanks.\n"},{"id":"290445","messageId":"CAPig+cRFZg1z5mhKiO7=0a+bCbdhJ07FubUstNrHYmKUrTc6nA@mail.gmail.com","threadId":"42714","inReplyTo":"20160628172641.26381-3-pclouds@gmail.com","subject":"Re: [PATCH v2 2/2] config: add conditional include","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-06-28T23:11:25Z","receivedAt":"2016-06-28T23:19:00Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Jun 28, 2016 at 1:26 PM, Nguyễn Thái Ngọc Duy wrote:\n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n> diff --git a/config.c b/config.c\n> @@ -140,9 +141,89 @@ static int handle_path_include(const char *path, struct config_include_data *inc\n> +static int include_condition_is_true(const char *cond, int cond_len)\n> +{\n> + const char *value;\n> + size_t value_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:\", &value, &value_len)) {\n> + struct strbuf text = STRBUF_INIT;\n> + struct strbuf pattern = STRBUF_INIT;\n> + int ret, prefix;\n> +\n> + strbuf_add_absolute_path(&text, get_git_dir());\n> + strbuf_add(&pattern, value, value_len);\n> + prefix = prepare_include_condition_pattern(&pattern);\n> +\n> + if (prefix < 0)\n> + return 0;\n> +\n> + if (prefix > 0 &&\n> + (text.len < prefix ||\n> + fspathncmp(pattern.buf, text.buf, prefix)))\n> + return 0;\n\nAre the above two \"return\"s leaking 'text' and 'pattern' strbufs?\n\n> +\n> + ret = !wildmatch(pattern.buf + prefix, text.buf + prefix,\n> + ignore_case ? WM_CASEFOLD : 0,\n> + NULL);\n> + strbuf_release(&pattern);\n> + strbuf_release(&text);\n> + return ret;\n> + }\n> +\n> + error(_(\"unrecognized include condition: %.*s\"), cond_len, cond);\n> + /* unknown conditionals are always false */\n> + return 0;\n> +}\n"},{"id":"290452","messageId":"CACsJy8BQu6vcxdt0tESjBJVnbmvmW8oRNkLfMA157s1bE-S_eA@mail.gmail.com","threadId":"42714","inReplyTo":"20160628204959.GB21002@sigill.intra.peff.net","subject":"Re: [PATCH v2 2/2] config: add conditional include","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-06-29T04:06:44Z","receivedAt":"2016-06-29T04:07:22Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Jun 28, 2016 at 10:49 PM, Jeff King wrote:\n>> +static int prepare_include_condition_pattern(struct strbuf *pat)\n>> +{\n>> + struct strbuf path = STRBUF_INIT;\n>> + int prefix = 0;\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>> + strbuf_add_absolute_path(&path, home);\n>> + strbuf_splice(pat, 0, 1, path.buf, path.len);\n>> + prefix = path.len + 1 /*slash*/;\n>\n> Why did you drop expand_user_path() here?\n>\n> I guess it's to do with knowing the length of the prefix portion? I'm\n> not sure I understand why the prefix is necessary. I thought the goal\n> was to construct a wildmatch pattern that could be used against\n> get_git_dir().\n\nWe need to make sure any '*', '?' and '[' in the $HOME (or `pwd`)\nportion are not automatically promoted to wildcards. One option is\nsplit the pattern in two parts, the prefix part is matched literally\nthen the rest passed to wildmatch(). The other option is escape\n$HOME/`pwd`, but I think that's more complicated (or at least slower).\n-- \nDuy\n"},{"id":"290453","messageId":"CACsJy8B_Lojr1FScTruPaFsftfhAoGSBnXmC4NjycdVf84JT9w@mail.gmail.com","threadId":"42714","inReplyTo":"20160628202809.GA21002@sigill.intra.peff.net","subject":"Re: [PATCH v2 0/2] Config conditional include","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-06-29T04:09:36Z","receivedAt":"2016-06-29T04:11:31Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Jun 28, 2016 at 10:28 PM, Jeff King wrote:\n> On Tue, Jun 28, 2016 at 07:26:39PM +0200, Nguyễn Thái Ngọc Duy wrote:\n>\n>> There's a surprise about core.ignorecase. We are matching paths, so we\n>> should match case-insensitively if core.ignorecase tells us so. And it\n>> gets a bit tricky if core.ignorecase is defined in the same config\n>> file. I don't think we have ever told the user that keys are processed\n>> from top down. We do now.\n>\n> Hrm. I'm not excited about introducing ordering issues into the config\n> parsing. But I think it's actually even more complicated than that.\n>\n> core.ignorecase is generally about the working tree, not the git\n> repository directory, which may reside on another filesystem entirely\n> (though I would not be surprised if we've blurred that line already).\n>\n> I wonder if it would be that bad to just punt on the issue, and say that\n> these include-match globs are always case-insensitive, or something.\n> True, that does not allow one to distinguish between config for \"foo\"\n> and \"Foo\" directories, but I find it unlikely anybody would ever want\n> to. And if we define it that way from day one, then nobody expects it to\n> work.\n\nYou already opened a path for this with your gitdir/regexp suggestion:\nwe could support case-sensitive match with \"gitdir:\" then\ncase-insensitive match with \"gitdir/i:\". Everybody is happy.\n-- \nDuy\n"},{"id":"291278","messageId":"20160712164216.24072-1-pclouds@gmail.com","threadId":"42714","inReplyTo":"20160628172641.26381-3-pclouds@gmail.com","subject":"[PATCH v3] config: add conditional include","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-07-12T16:42:16Z","receivedAt":"2016-07-12T16:44:53Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Helped-by: Jeff King \nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n v3 fixes some memory leak and typos. Most importantly it no longer\n depends on core.ignorecase for case-insensitive matching. The choice\n must be explicitly made by the user when they choose \"gitdir\" or\n \"gitdir/i\" keyword.\n\n Documentation/config.txt | 48 ++++++++++++++++++++++\n config.c | 102 +++++++++++++++++++++++++++++++++++++++++++++-\n t/t1305-config-include.sh | 56 +++++++++++++++++++++++++\n 3 files changed, 204 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex db05dec..18623ee 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -91,6 +91,43 @@ 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+Included files can be grouped into subsections where the subsection\n+name is the condition that must be met for the files to be included.\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 +156,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 \"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 \"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 \"gitdir:~/to/group/\"]\n+\t\tpath = /path/to/foo.inc\n \n Values\n ~~~~~~\ndiff --git a/config.c b/config.c\nindex bea937e..ff44e00 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@@ -168,9 +169,102 @@ 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+\tint prefix = 0;\n+\n+\t/* TODO: maybe support ~user/ too */\n+\tif (pat->buf[0] == '~' && is_dir_sep(pat->buf[1])) {\n+\t\tstruct strbuf path = STRBUF_INIT;\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\tstrbuf_release(&path);\n+\t} else if (pat->buf[0] == '.' && is_dir_sep(pat->buf[1])) {\n+\t\tstruct strbuf path = STRBUF_INIT;\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\tstrbuf_release(&path);\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+\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@@ -181,8 +275,12 @@ int git_config_include(const char *var, const char *value, void *data)\n \tif (ret < 0)\n \t\treturn ret;\n \n-\tif (!strcmp(var, \"include.path\"))\n-\t\tret = handle_path_include(value, inc);\n+\tif (!parse_config_key(var, \"include\", &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 9ba2ba1..83501ec 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 \\\"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 \\\"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 \\\"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 \\\"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 \\\"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.9.1.564.gb2f7278\n\n"},{"id":"291328","messageId":"vpqa8hmroji.fsf@anie.imag.fr","threadId":"42714","inReplyTo":"20160712164216.24072-1-pclouds@gmail.com","subject":"Re: [PATCH v3] config: add conditional include","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2016-07-13T07:21:37Z","receivedAt":"2016-07-13T07:23:31Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Nguyễn Thái Ngọc Duy writes:\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\nIt's unclear to me whether the whole content of GIT_DIR must match, or\nwhether the pattern should be included (or a be prefix) of $GIT_DIR.\nFrom this text, I read it as \"the whole content\", but ...\n\n> +\t; include for all repositories inside /path/to/group\n> +\t[include \"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 \"gitdir:~/to/group/\"]\n> +\t\tpath = /path/to/foo.inc\n\n... here it seems it only has to be a prefix.\n\n> +static int prepare_include_condition_pattern(struct strbuf *pat)\n> +{\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\tstruct strbuf path = STRBUF_INIT;\n> +\t\tconst char *home = getenv(\"HOME\");\n> +\n> +\t\tif (!home)\n> +\t\t\treturn error(_(\"$HOME is not defined\"));\n\nexpand_user_path in path.c seems to do the same as you're doing (but\ndoes deal with ~user). Any reason not to use it?\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"291329","messageId":"20160713072607.GA15153@sigill.intra.peff.net","threadId":"42714","inReplyTo":"vpqa8hmroji.fsf@anie.imag.fr","subject":"Re: [PATCH v3] config: add conditional include","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-07-13T07:26:07Z","receivedAt":"2016-07-13T07:28:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jul 13, 2016 at 09:21:37AM +0200, Matthieu Moy wrote:\n\n> > +static int prepare_include_condition_pattern(struct strbuf *pat)\n> > +{\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\tstruct strbuf path = STRBUF_INIT;\n> > +\t\tconst char *home = getenv(\"HOME\");\n> > +\n> > +\t\tif (!home)\n> > +\t\t\treturn error(_(\"$HOME is not defined\"));\n> \n> expand_user_path in path.c seems to do the same as you're doing (but\n> does deal with ~user). Any reason not to use it?\n\nI had a similar question, which Duy answered in:\n\n http://article.gmane.org/gmane.comp.version-control.git/298528\n\nIt does feel pretty hacky, though (especially for a case that seems\nunlikely to come up: people having wildcard patterns in the name of\ntheir home directory).\n\n-Peff\n"},{"id":"291340","messageId":"vpqmvllhff6.fsf@anie.imag.fr","threadId":"42714","inReplyTo":"20160713072607.GA15153@sigill.intra.peff.net","subject":"Re: [PATCH v3] config: add conditional include","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2016-07-13T12:48:45Z","receivedAt":"2016-07-13T12:52:13Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Jeff King writes:\n\n> On Wed, Jul 13, 2016 at 09:21:37AM +0200, Matthieu Moy wrote:\n>\n>> > +static int prepare_include_condition_pattern(struct strbuf *pat)\n>> > +{\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\tstruct strbuf path = STRBUF_INIT;\n>> > +\t\tconst char *home = getenv(\"HOME\");\n>> > +\n>> > +\t\tif (!home)\n>> > +\t\t\treturn error(_(\"$HOME is not defined\"));\n>> \n>> expand_user_path in path.c seems to do the same as you're doing (but\n>> does deal with ~user). Any reason not to use it?\n>\n> I had a similar question, which Duy answered in:\n>\n> http://article.gmane.org/gmane.comp.version-control.git/298528\n>\n> It does feel pretty hacky, though (especially for a case that seems\n> unlikely to come up: people having wildcard patterns in the name of\n> their home directory).\n\nAh, OK. Then I'd suggest at least an improvement in the comment:\n\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 * perform literal matching on the expanded prefix part\n+\t\t * so that any wildcard character in it (e.g in the\n+\t\t * expansion of ~) can't create side effects.\n \t\t */\n\nI would also rename the variable prefix -> expanded_prefix. As-is, the\ncode is really hard to understand IMHO.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"291356","messageId":"CACsJy8DrPPfvp3C=tBL0qBmAwdEAZtXrxPLNW3HgcgJGO1u3QA@mail.gmail.com","threadId":"42714","inReplyTo":"vpqa8hmroji.fsf@anie.imag.fr","subject":"Re: [PATCH v3] config: add conditional include","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-07-13T15:57:14Z","receivedAt":"2016-07-13T15:58:48Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Jul 13, 2016 at 9:21 AM, Matthieu Moy\n wrote:\n> Nguyễn Thái Ngọc Duy writes:\n>\n>> +`gitdir`::\n>> + The environment variable `GIT_DIR` must match the following\n>> + pattern for files to be included. The pattern can contain\n>> + standard globbing wildcards and two additional ones, `**/` and\n>> + `/**`, that can match multiple path components. Please refer\n>> + to linkgit:gitignore[5] for details. For convenience:\n>\n> It's unclear to me whether the whole content of GIT_DIR must match, or\n> whether the pattern should be included (or a be prefix) of $GIT_DIR.\n> From this text, I read it as \"the whole content\", but ...\n>\n>> + ; include for all repositories inside /path/to/group\n>> + [include \"gitdir:/path/to/group/\"]\n>> + path = /path/to/foo.inc\n>> +\n>> + ; include for all repositories inside $HOME/to/group\n>> + [include \"gitdir:~/to/group/\"]\n>> + path = /path/to/foo.inc\n>\n> ... here it seems it only has to be a prefix.\n\nI should have written \"with two additional ones... and a few\nexceptions\"., One of the bullet point below would say the trailing\nslash is rewritten to \"/**\" so it becomes prefix match. If it proves\nconfusing, I will probably just get rid of that.\n-- \nDuy\n"},{"id":"291422","messageId":"20160714153311.2166-1-pclouds@gmail.com","threadId":"42714","inReplyTo":"20160712164216.24072-1-pclouds@gmail.com","subject":"[PATCH v4] config: add conditional include","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-07-14T15:33:11Z","receivedAt":"2016-07-14T15:34:08Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Helped-by: Jeff King \nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n The diff from v3 is mostly clarification in code and document.\n\n diff --git a/Documentation/config.txt b/Documentation/config.txt\n index 18623ee..d971334 100644\n --- a/Documentation/config.txt\n +++ b/Documentation/config.txt\n @@ -99,10 +99,9 @@ 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 +\tpattern for files to be included. The pattern shares the same\n +\tsyntax as patterns in link:gitignore[5] with a few exceptions\n +\tbelow:\n \n * If the pattern starts with `~/`, `~` will be substituted with the\n content of the environment variable `HOME`.\n diff --git a/config.c b/config.c\n index ff44e00..690f3d5 100644\n --- a/config.c\n +++ b/config.c\n @@ -183,6 +183,11 @@ static int prepare_include_condition_pattern(struct strbuf *pat)\n \n \t\tstrbuf_add_absolute_path(&path, home);\n \t\tstrbuf_splice(pat, 0, 1, path.buf, path.len);\n +\t\t/*\n +\t\t * This part, path.buf[0..len], should be considered\n +\t\t * a literal string even if it has wildcards in it,\n +\t\t * because those wildcards are not wanted by the user.\n +\t\t */\n \t\tprefix = path.len + 1 /*slash*/;\n \t\tstrbuf_release(&path);\n \t} else if (pat->buf[0] == '.' && is_dir_sep(pat->buf[1])) {\n @@ -198,6 +203,11 @@ static int prepare_include_condition_pattern(struct strbuf *pat)\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\t/*\n +\t\t * This part, path.buf[0..slash], should be consider\n +\t\t * a literal string even if it has wildcards in it,\n +\t\t * because those wildcards are not wanted by the user.\n +\t\t */\n \t\tprefix = slash - path.buf + 1 /* slash */;\n \t\tstrbuf_release(&path);\n \t} else if (!is_absolute_path(pat->buf))\n @@ -224,8 +234,9 @@ static int include_by_gitdir(const char *cond, size_t cond_len, int icase)\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 * perform literal matching on the expanded prefix\n +\t\t * part so that any wildcard character in it (e.g in\n +\t\t * the expansion of ~) can't create side effects.\n \t\t */\n \t\tif (text.len < prefix)\n \t\t\tgoto done;\n Documentation/config.txt | 47 +++++++++++++++++++\n config.c | 113 +++++++++++++++++++++++++++++++++++++++++++++-\n t/t1305-config-include.sh | 56 +++++++++++++++++++++++\n 3 files changed, 214 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex db05dec..d971334 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -91,6 +91,42 @@ 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+Included files can be grouped into subsections where the subsection\n+name is the condition that must be met for the files to be included.\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 shares the same\n+\tsyntax as patterns in link:gitignore[5] with a few exceptions\n+\tbelow:\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 +155,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 \"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 \"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 \"gitdir:~/to/group/\"]\n+\t\tpath = /path/to/foo.inc\n \n Values\n ~~~~~~\ndiff --git a/config.c b/config.c\nindex bea937e..690f3d5 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@@ -168,9 +169,113 @@ 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+\tint prefix = 0;\n+\n+\t/* TODO: maybe support ~user/ too */\n+\tif (pat->buf[0] == '~' && is_dir_sep(pat->buf[1])) {\n+\t\tstruct strbuf path = STRBUF_INIT;\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\t/*\n+\t\t * This part, path.buf[0..len], should be considered\n+\t\t * a literal string even if it has wildcards in it,\n+\t\t * because those wildcards are not wanted by the user.\n+\t\t */\n+\t\tprefix = path.len + 1 /*slash*/;\n+\t\tstrbuf_release(&path);\n+\t} else if (pat->buf[0] == '.' && is_dir_sep(pat->buf[1])) {\n+\t\tstruct strbuf path = STRBUF_INIT;\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\t/*\n+\t\t * This part, path.buf[0..slash], should be consider\n+\t\t * a literal string even if it has wildcards in it,\n+\t\t * because those wildcards are not wanted by the user.\n+\t\t */\n+\t\tprefix = slash - path.buf + 1 /* slash */;\n+\t\tstrbuf_release(&path);\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+\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 expanded prefix\n+\t\t * part so that any wildcard character in it (e.g in\n+\t\t * the expansion of ~) 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@@ -181,8 +286,12 @@ int git_config_include(const char *var, const char *value, void *data)\n \tif (ret < 0)\n \t\treturn ret;\n \n-\tif (!strcmp(var, \"include.path\"))\n-\t\tret = handle_path_include(value, inc);\n+\tif (!parse_config_key(var, \"include\", &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 9ba2ba1..83501ec 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 \\\"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 \\\"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 \\\"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 \\\"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 \\\"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.9.1.566.gbd532d4\n\n"},{"id":"291427","messageId":"alpine.DEB.2.20.1607141750190.6426@virtualbox","threadId":"42714","inReplyTo":"20160714153311.2166-1-pclouds@gmail.com","subject":"Re: [PATCH v4] config: add conditional include","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-07-14T15:53:06Z","receivedAt":"2016-07-14T15:53:21Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Duy,\n\nOn Thu, 14 Jul 2016, Nguyễn Thái Ngọc Duy wrote:\n\n> Helped-by: Jeff King \n> Signed-off-by: Nguyễn Thái Ngọc Duy \n\nThis commit message is quite a bit short on details. I still fail to see\nwhat use case this would benefit, and I already start to suspect that the\nchange would open a can of worms that might not be desired.\n\n> +\t; include if $GIT_DIR is /path/to/foo/.git\n> +\t[include \"gitdir:/path/to/foo/.git\"]\n> +\t\tpath = /path/to/foo.inc\n\nI find this way to specify a conditional unintuitive. Reading\n\"gitdir:/path/to/foo/.git\" suggests to me that the Git dir is *re-set*,\nnot that this is a condition.\n\nI would have expected something like\n\n\t[include \"if-gitdir-is:/path/to/foo/.git\"]\n\ninstead.\n\nCiao,\nDscho"},{"id":"291431","messageId":"CACsJy8AjVX1Say0srEq+ezGg=CzmbjBAt4PnuikXiqdnVC4G6g@mail.gmail.com","threadId":"42714","inReplyTo":"alpine.DEB.2.20.1607141750190.6426@virtualbox","subject":"Re: [PATCH v4] config: add conditional include","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-07-14T16:13:55Z","receivedAt":"2016-07-14T16:14:35Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Jul 14, 2016 at 5:53 PM, Johannes Schindelin\n wrote:\n> Hi Duy,\n>\n> On Thu, 14 Jul 2016, Nguyễn Thái Ngọc Duy wrote:\n>\n>> Helped-by: Jeff King \n>> Signed-off-by: Nguyễn Thái Ngọc Duy \n>\n> This commit message is quite a bit short on details.\n\nBecause it's fully documented in config.txt.\n\n> I still fail to see what use case this would benefit,\n\nIt allows you to group repos together. The first mail that started all\nthis [1] is one of the use cases: you may want to use one identity in\na group of repos, and another identity in some other. I want some\nmore, to use a private key one some repos and another private key on\nsome other. Some more settings may be shareable this way, like coding\nstyle-related (trailing spaces or not...)\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/273811\n\n> and I already start to suspect that the change would open a can of worms that might not be desired.\n\nYou can choose not to use it, I guess. From what I see it's nothing\nsuper tricky. The config hierarchy we have now is: system -> per-user\n-> repo. This could change this to: system -> per-user ->\nper-directory -> repo. You could create a different hierarchy, but\nwith git-config now showing where a config var comes from, it's not\nhard to troubleshoot.\n\n>> + ; include if $GIT_DIR is /path/to/foo/.git\n>> + [include \"gitdir:/path/to/foo/.git\"]\n>> + path = /path/to/foo.inc\n>\n> I find this way to specify a conditional unintuitive. Reading\n> \"gitdir:/path/to/foo/.git\" suggests to me that the Git dir is *re-set*,\n> not that this is a condition.\n\nWell.. to me it's no different than [remote \"foo\"] to apply stuff to \"foo\".\n\n> I would have expected something like\n>\n> [include \"if-gitdir-is:/path/to/foo/.git\"]\n>\n> instead.\n\nIf we do this, should we change gitdir/i to\nif-git-dir-case-insensitively-is ? I think we are supposed to read\ndocuments before using any feature, not just guessing then trial and\nerror.\n-- \nDuy\n"},{"id":"291596","messageId":"alpine.DEB.2.20.1607161507250.28832@virtualbox","threadId":"42714","inReplyTo":"CACsJy8AjVX1Say0srEq+ezGg=CzmbjBAt4PnuikXiqdnVC4G6g@mail.gmail.com","subject":"Re: [PATCH v4] config: add conditional include","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-07-16T13:30:45Z","receivedAt":"2016-07-16T13:31:14Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Duy,\n\nOn Thu, 14 Jul 2016, Duy Nguyen wrote:\n\n> On Thu, Jul 14, 2016 at 5:53 PM, Johannes Schindelin\n> wrote:\n> > Hi Duy,\n> >\n> > On Thu, 14 Jul 2016, Nguyễn Thái Ngọc Duy wrote:\n> >\n> >> Helped-by: Jeff King \n> >> Signed-off-by: Nguyễn Thái Ngọc Duy \n> >\n> > This commit message is quite a bit short on details.\n> \n> Because it's fully documented in config.txt.\n\nSurely there is more information left for the commit message, such as:\nwhat motivated the patch, what alternative solutions were considered, was\nany thought given to how this might break down, etc\n\n> > I still fail to see what use case this would benefit,\n> \n> It allows you to group repos together. The first mail that started all\n> this [1] is one of the use cases: you may want to use one identity in\n> a group of repos, and another identity in some other. I want some\n> more, to use a private key one some repos and another private key on\n> some other. Some more settings may be shareable this way, like coding\n> style-related (trailing spaces or not...)\n> \n> [1] http://thread.gmane.org/gmane.comp.version-control.git/273811\n> \n> > and I already start to suspect that the change would open a can of worms that might not be desired.\n> \n> You can choose not to use it, I guess.\n\nSadly, as the maintainer I am unable to share in that luxury of yours.\n\n> From what I see it's nothing super tricky. The config hierarchy we have\n> now is: system -> per-user -> repo. This could change this to: system ->\n> per-user -> per-directory -> repo. You could create a different\n> hierarchy, but with git-config now showing where a config var comes\n> from, it's not hard to troubleshoot.\n\nThe more indirection you allow, the harder it gets. And that problem is\nexacerbated when allowing conditional includes.\n\n> >> + ; include if $GIT_DIR is /path/to/foo/.git\n> >> + [include \"gitdir:/path/to/foo/.git\"]\n> >> + path = /path/to/foo.inc\n> >\n> > I find this way to specify a conditional unintuitive. Reading\n> > \"gitdir:/path/to/foo/.git\" suggests to me that the Git dir is *re-set*,\n> > not that this is a condition.\n> \n> Well.. to me it's no different than [remote \"foo\"] to apply stuff to \"foo\".\n\nExcept that \"include\" is an imperative and \"remote\" is not.\n\nQuite frankly, this conditional business scares me. If you introduce it\nfor [include], users will want it for every config setting. And the\ncurrent syntax is just not up to, say, making user.name conditional on\nanything.\n\nAs an alternative solution to your problem, you could of course avoid all\nconditional includes. Simply by adding the include.path settings\nexplicitly to the configs that require them. Now, that would make reasoning\nand trouble-shooting simple, wouldn't it?\n\nAnd the most beautiful aspect of it: no patch needed.\n\nCiao,\nDscho"},{"id":"291598","messageId":"CACsJy8AvxC-p9=no+tYBc80-MWfApQ_QBagk0x7NXJDG9D=ZRw@mail.gmail.com","threadId":"42714","inReplyTo":"alpine.DEB.2.20.1607161507250.28832@virtualbox","subject":"Re: [PATCH v4] config: add conditional include","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-07-16T14:48:58Z","receivedAt":"2016-07-16T14:49:33Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sat, Jul 16, 2016 at 3:30 PM, Johannes Schindelin\n wrote:\n> As an alternative solution to your problem, you could of course avoid all\n> conditional includes. Simply by adding the include.path settings\n> explicitly to the configs that require them. Now, that would make reasoning\n> and trouble-shooting simple, wouldn't it?\n\nI can't. Repos can be created and destroyed often (it's in the\nprocess), and there are many of them. Using a wrong identity (among\nother incorrect config settings) is a serious problem and I cannot\nguarantee myself that I will never make a mistake, forgetting to\ninclude stuff on new clones.\n\n>> > I still fail to see what use case this would benefit,\n>>\n>> It allows you to group repos together. The first mail that started all\n>> this [1] is one of the use cases: you may want to use one identity in\n>> a group of repos, and another identity in some other. I want some\n>> more, to use a private key one some repos and another private key on\n>> some other. Some more settings may be shareable this way, like coding\n>> style-related (trailing spaces or not...)\n>>\n>> [1] http://thread.gmane.org/gmane.comp.version-control.git/273811\n>>\n>> > and I already start to suspect that the change would open a can of worms that might not be desired.\n>>\n>> You can choose not to use it, I guess.\n>\n> Sadly, as the maintainer I am unable to share in that luxury of yours.\n\nI need this. And I post it because people may need it too. But if it's\na bad thing, I guess I'll keep this patch on my tree then.\n-- \nDuy\n"},{"id":"291599","messageId":"20160716150835.GA24374@sigill.intra.peff.net","threadId":"42714","inReplyTo":"alpine.DEB.2.20.1607161507250.28832@virtualbox","subject":"Re: [PATCH v4] config: add conditional include","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-07-16T15:08:36Z","receivedAt":"2016-07-16T15:08:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jul 16, 2016 at 03:30:45PM +0200, Johannes Schindelin wrote:\n\n> > >> + ; include if $GIT_DIR is /path/to/foo/.git\n> > >> + [include \"gitdir:/path/to/foo/.git\"]\n> > >> + path = /path/to/foo.inc\n> > >\n> > > I find this way to specify a conditional unintuitive. Reading\n> > > \"gitdir:/path/to/foo/.git\" suggests to me that the Git dir is *re-set*,\n> > > not that this is a condition.\n> > \n> > Well.. to me it's no different than [remote \"foo\"] to apply stuff to \"foo\".\n> \n> Except that \"include\" is an imperative and \"remote\" is not.\n\nIn the very original version of config includes, I had planned out:\n\n [include-if \"...some condition...\"]\n path = ...\n\nLater, since \"[include ...]\" had no other meaning, I think it got\nshortened in discussion. But it would be easy to accept include-if (or\neven accept either, for maximum confusion :) ).\n\n> Quite frankly, this conditional business scares me. If you introduce it\n> for [include], users will want it for every config setting. And the\n> current syntax is just not up to, say, making user.name conditional on\n> anything.\n\nThey already have it for every config setting with this. The reason to\nadd it to [include] and not as a general syntax is that you can put\nuser.name into your included file, and then conditionally include it.\n\nThat is not as nice as \"if this then that\" in a single file, but it is\nbackwards compatible with the existing syntax, and is probably fine in\npractice. Each included file becomes a \"profile\" of multiple settings\nthat you apply.\n\n> As an alternative solution to your problem, you could of course avoid all\n> conditional includes. Simply by adding the include.path settings\n> explicitly to the configs that require them. Now, that would make reasoning\n> and trouble-shooting simple, wouldn't it?\n> \n> And the most beautiful aspect of it: no patch needed.\n\nAnd you can just \"cat\" the included files directly into your\n.git/config. We don't even need include.path. Or ~/.gitconfig, for that\nmatter. But sometimes dynamic things are convenient.\n\n-Peff\n"},{"id":"291600","messageId":"alpine.DEB.2.20.1607161834420.28832@virtualbox","threadId":"42714","inReplyTo":"20160716150835.GA24374@sigill.intra.peff.net","subject":"Re: [PATCH v4] config: add conditional include","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-07-16T16:36:03Z","receivedAt":"2016-07-16T16:36:27Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn Sat, 16 Jul 2016, Jeff King wrote:\n\n> On Sat, Jul 16, 2016 at 03:30:45PM +0200, Johannes Schindelin wrote:\n> \n> > As an alternative solution to your problem, you could of course avoid all\n> > conditional includes. Simply by adding the include.path settings\n> > explicitly to the configs that require them. Now, that would make reasoning\n> > and trouble-shooting simple, wouldn't it?\n> > \n> > And the most beautiful aspect of it: no patch needed.\n> \n> And you can just \"cat\" the included files directly into your\n> .git/config. We don't even need include.path. Or ~/.gitconfig, for that\n> matter. But sometimes dynamic things are convenient.\n\nWell, apparently you are not convinced by my argument. I thought it was\npretty sound, but if you disagree, it cannot have been...\n\nSo I'll shut up ;-)\n\nCiao,\nDscho\n"},{"id":"291601","messageId":"20160716164747.GA24933@sigill.intra.peff.net","threadId":"42714","inReplyTo":"alpine.DEB.2.20.1607161834420.28832@virtualbox","subject":"Re: [PATCH v4] config: add conditional include","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-07-16T16:47:48Z","receivedAt":"2016-07-16T16:47:58Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jul 16, 2016 at 06:36:03PM +0200, Johannes Schindelin wrote:\n\n> > On Sat, Jul 16, 2016 at 03:30:45PM +0200, Johannes Schindelin wrote:\n> > \n> > > As an alternative solution to your problem, you could of course avoid all\n> > > conditional includes. Simply by adding the include.path settings\n> > > explicitly to the configs that require them. Now, that would make reasoning\n> > > and trouble-shooting simple, wouldn't it?\n> > > \n> > > And the most beautiful aspect of it: no patch needed.\n> > \n> > And you can just \"cat\" the included files directly into your\n> > .git/config. We don't even need include.path. Or ~/.gitconfig, for that\n> > matter. But sometimes dynamic things are convenient.\n> \n> Well, apparently you are not convinced by my argument. I thought it was\n> pretty sound, but if you disagree, it cannot have been...\n\nHeh. Don't get me wrong, I do think there's room for digging ourselves a\ndeep hole with conditional includes, because anything dynamic opens up a\nquestion of _when_ it is evaluated, and in what context. So any\ncondition should be something that we would consider static through the\nwhole run of the program. Looking at the \"gitdir\" is right on the\nborderline of that, but I think it's OK, because we already have to\ninvalidate any cached config when we setup the gitdir, because\n\"$GIT_DIR/config\" will have become a new source.\n\nSo I agree it's something we need to be thoughtful about, but I think\nthis particular instance is useful and probably not going to bite us.\n\n-Peff\n"},{"id":"291618","messageId":"alpine.DEB.2.20.1607171003010.28832@virtualbox","threadId":"42714","inReplyTo":"20160716164747.GA24933@sigill.intra.peff.net","subject":"Re: [PATCH v4] config: add conditional include","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-07-17T08:15:55Z","receivedAt":"2016-07-17T08:16:31Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn Sat, 16 Jul 2016, Jeff King wrote:\n\n> On Sat, Jul 16, 2016 at 06:36:03PM +0200, Johannes Schindelin wrote:\n> \n> > > On Sat, Jul 16, 2016 at 03:30:45PM +0200, Johannes Schindelin wrote:\n> > > \n> > > > As an alternative solution to your problem, you could of course avoid all\n> > > > conditional includes. Simply by adding the include.path settings\n> > > > explicitly to the configs that require them. Now, that would make reasoning\n> > > > and trouble-shooting simple, wouldn't it?\n> > > > \n> > > > And the most beautiful aspect of it: no patch needed.\n> > > \n> > > And you can just \"cat\" the included files directly into your\n> > > .git/config. We don't even need include.path. Or ~/.gitconfig, for that\n> > > matter. But sometimes dynamic things are convenient.\n> > \n> > Well, apparently you are not convinced by my argument. I thought it was\n> > pretty sound, but if you disagree, it cannot have been...\n> \n> Heh. Don't get me wrong, I do think there's room for digging ourselves a\n> deep hole with conditional includes, because anything dynamic opens up a\n> question of _when_ it is evaluated, and in what context. So any\n> condition should be something that we would consider static through the\n> whole run of the program. Looking at the \"gitdir\" is right on the\n> borderline of that, but I think it's OK, because we already have to\n> invalidate any cached config when we setup the gitdir, because\n> \"$GIT_DIR/config\" will have become a new source.\n> \n> So I agree it's something we need to be thoughtful about, but I think\n> this particular instance is useful and probably not going to bite us.\n\nFWIW I am slightly less worried about the conditional includes (it is\nalready a horrible mess to figure out too-long include chains now, before\nhaving conditional includes, for example). I am slightly more worried\nabout eventually needing to introduce support for something like\n\n\t[if-gitdir(...):section]\n\t\tthisSettingIsConditional = ...\n\nor even\n\n\t[if (worktree==...):section]\n\t\tanotherConditional = ...\n\nand then having two incompatible conditional constructs, one generic, the\nother one specific to [include].\n\nIn other words, if we already introduce a conditional construct, I'd\nrather have one that could easily be used for other conditions/sections\nwhen (and if) needed.\n\nI, for one, would rather have my repository-specific overrides in one single\nfile than having a bunch of files that are included conditionally and may\nneed to override one another: I can see the entries much easier in the\nsingle file (and group them by section) than in the multiple files. My\nworking memory is just too filled up with other stuff to remember the\ncontents of the other file(s).\n\nBut I guess that boils down to preference.\n\nCiao,\nDscho\n"},{"id":"291806","messageId":"20160720133130.GA19122@sigill.intra.peff.net","threadId":"42714","inReplyTo":"alpine.DEB.2.20.1607171003010.28832@virtualbox","subject":"Re: [PATCH v4] config: add conditional include","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-07-20T13:31:31Z","receivedAt":"2016-07-20T13:31:38Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Jul 17, 2016 at 10:15:55AM +0200, Johannes Schindelin wrote:\n\n> FWIW I am slightly less worried about the conditional includes (it is\n> already a horrible mess to figure out too-long include chains now, before\n> having conditional includes, for example). I am slightly more worried\n> about eventually needing to introduce support for something like\n> \n> \t[if-gitdir(...):section]\n> \t\tthisSettingIsConditional = ...\n> \n> or even\n> \n> \t[if (worktree==...):section]\n> \t\tanotherConditional = ...\n> \n> and then having two incompatible conditional constructs, one generic, the\n> other one specific to [include].\n> \n> In other words, if we already introduce a conditional construct, I'd\n> rather have one that could easily be used for other conditions/sections\n> when (and if) needed.\n\nI had assumed we would resist introducing anything like that, simply\nbecause of backwards compatibility issues with the syntax. But I admit\nthat was just an assumption in my head; future compatibility with\nreality is not guaranteed. :)\n\n-Peff\n"},{"id":"291833","messageId":"578FA931.4050708@gmail.com","threadId":"42714","inReplyTo":"20160716164747.GA24933@sigill.intra.peff.net","subject":"Re: [PATCH v4] config: add conditional include","fromName":"Jakub Narębski","fromEmail":"jnareb@gmail.com","sentAt":"2016-07-20T16:39:13Z","receivedAt":"2016-07-20T16:39:27Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"W dniu 2016-07-16 o 18:47, Jeff King pisze:\n\n> Heh. Don't get me wrong, I do think there's room for digging ourselves a\n> deep hole with conditional includes, because anything dynamic opens up a\n> question of _when_ it is evaluated, and in what context. So any\n> condition should be something that we would consider static through the\n> whole run of the program. Looking at the \"gitdir\" is right on the\n> borderline of that, but I think it's OK, because we already have to\n> invalidate any cached config when we setup the gitdir, because\n> \"$GIT_DIR/config\" will have become a new source.\n> \n> So I agree it's something we need to be thoughtful about, but I think\n> this particular instance is useful and probably not going to bite us.\n\nA question: we have a way to track where the value came from (tracking\nincludes). Do we have a way to check where the value didn't came from,\nbecause for example error in the include condition?\n\n-- \nJakub Narębski\n\n"},{"id":"291887","messageId":"xmqq8twwc6a2.fsf@gitster.mtv.corp.google.com","threadId":"42714","inReplyTo":"20160720133130.GA19122@sigill.intra.peff.net","subject":"Re: [PATCH v4] config: add conditional include","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-20T22:07:49Z","receivedAt":"2016-07-20T22:07:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Sun, Jul 17, 2016 at 10:15:55AM +0200, Johannes Schindelin wrote:\n>\n>> FWIW I am slightly less worried about the conditional includes (it is\n>> already a horrible mess to figure out too-long include chains now, before\n>> having conditional includes, for example). I am slightly more worried\n>> about eventually needing to introduce support for something like\n>> \n>> \t[if-gitdir(...):section]\n>> \t\tthisSettingIsConditional = ...\n>> \n>> or even\n>> \n>> \t[if (worktree==...):section]\n>> \t\tanotherConditional = ...\n>> \n>> and then having two incompatible conditional constructs, one generic, the\n>> other one specific to [include].\n>> \n>> In other words, if we already introduce a conditional construct, I'd\n>> rather have one that could easily be used for other conditions/sections\n>> when (and if) needed.\n>\n> I had assumed we would resist introducing anything like that, simply\n> because of backwards compatibility issues with the syntax. But I admit\n> that was just an assumption in my head; future compatibility with\n> reality is not guaranteed. :)\n\nI actually read that assumption between lines and almost wrote the\nsame response that begins with \"I think the untold assumption ever\nsince the inclusion mechanism was introduced is...\" ;-)\n\nA config file with \"[include] path=...\" in it would not include from\nthe named path by ancient verison of Git, but at least it won't\ncause its parser to barf, and the assumption has been that it is a\ngood property to keep when we introduce new and incompatible\nfeatures.\n\nI can however understand it if somebody thinks it actually is better\nto actively break older Git implementations by forcing them to stop\nparsing when we introduce constructs that will lead them to do wrong\nthings (e.g. missing some configuration defintions by not reading\nfrom the file that the user wanted to be read from), rather than\nmaking them silently ignore.\n"},{"id":"298951","messageId":"CACsJy8Bw0ZNu-6SB0P3dBZCLMJWJkbUqb64H_QOcn4UH+_AcNA@mail.gmail.com","threadId":"42714","inReplyTo":"20160714153311.2166-1-pclouds@gmail.com","subject":"Re: [PATCH v4] config: add conditional include","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-08-13T08:40:59Z","receivedAt":"2016-08-13T08:43:09Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Ping..\n\nOn Thu, Jul 14, 2016 at 10:33 PM, Nguyễn Thái Ngọc Duy\n wrote:\n> Helped-by: Jeff King \n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n> The diff from v3 is mostly clarification in code and document.\n>\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index 18623ee..d971334 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -99,10 +99,9 @@ Supported keywords are:\n>\n> `gitdir`::\n> The environment variable `GIT_DIR` must match the following\n> - pattern for files to be included. The pattern can contain\n> - standard globbing wildcards and two additional ones, `**/` and\n> - `/**`, that can match multiple path components. Please refer\n> - to linkgit:gitignore[5] for details. For convenience:\n> + pattern for files to be included. The pattern shares the same\n> + syntax as patterns in link:gitignore[5] with a few exceptions\n> + below:\n>\n> * If the pattern starts with `~/`, `~` will be substituted with the\n> content of the environment variable `HOME`.\n> diff --git a/config.c b/config.c\n> index ff44e00..690f3d5 100644\n> --- a/config.c\n> +++ b/config.c\n> @@ -183,6 +183,11 @@ static int prepare_include_condition_pattern(struct strbuf *pat)\n>\n> strbuf_add_absolute_path(&path, home);\n> strbuf_splice(pat, 0, 1, path.buf, path.len);\n> + /*\n> + * This part, path.buf[0..len], should be considered\n> + * a literal string even if it has wildcards in it,\n> + * because those wildcards are not wanted by the user.\n> + */\n> prefix = path.len + 1 /*slash*/;\n> strbuf_release(&path);\n> } else if (pat->buf[0] == '.' && is_dir_sep(pat->buf[1])) {\n> @@ -198,6 +203,11 @@ static int prepare_include_condition_pattern(struct strbuf *pat)\n> if (!slash)\n> die(\"BUG: how is this possible?\");\n> strbuf_splice(pat, 0, 1, path.buf, slash - path.buf);\n> + /*\n> + * This part, path.buf[0..slash], should be consider\n> + * a literal string even if it has wildcards in it,\n> + * because those wildcards are not wanted by the user.\n> + */\n> prefix = slash - path.buf + 1 /* slash */;\n> strbuf_release(&path);\n> } else if (!is_absolute_path(pat->buf))\n> @@ -224,8 +234,9 @@ static int include_by_gitdir(const char *cond, size_t cond_len, int icase)\n>\n> if (prefix > 0) {\n> /*\n> - * perform literal matching on the prefix part so that\n> - * any wildcard character in it can't create side effects.\n> + * perform literal matching on the expanded prefix\n> + * part so that any wildcard character in it (e.g in\n> + * the expansion of ~) can't create side effects.\n> */\n> if (text.len < prefix)\n> goto done;\n> Documentation/config.txt | 47 +++++++++++++++++++\n> config.c | 113 +++++++++++++++++++++++++++++++++++++++++++++-\n> t/t1305-config-include.sh | 56 +++++++++++++++++++++++\n> 3 files changed, 214 insertions(+), 2 deletions(-)\n>\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index db05dec..d971334 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -91,6 +91,42 @@ 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> +Included files can be grouped into subsections where the subsection\n> +name is the condition that must be met for the files to be included.\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> + The environment variable `GIT_DIR` must match the following\n> + pattern for files to be included. The pattern shares the same\n> + syntax as patterns in link:gitignore[5] with a few exceptions\n> + below:\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> + This is the same as `gitdir` except that matching is done\n> + case-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 +155,17 @@ Example\n> path = foo ; expand \"foo\" relative to the current file\n> path = ~/foo ; expand \"foo\" in your `$HOME` directory\n>\n> + ; include if $GIT_DIR is /path/to/foo/.git\n> + [include \"gitdir:/path/to/foo/.git\"]\n> + path = /path/to/foo.inc\n> +\n> + ; include for all repositories inside /path/to/group\n> + [include \"gitdir:/path/to/group/\"]\n> + path = /path/to/foo.inc\n> +\n> + ; include for all repositories inside $HOME/to/group\n> + [include \"gitdir:~/to/group/\"]\n> + path = /path/to/foo.inc\n>\n> Values\n> ~~~~~~\n> diff --git a/config.c b/config.c\n> index bea937e..690f3d5 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> struct config_source *prev;\n> @@ -168,9 +169,113 @@ static int handle_path_include(const char *path, struct config_include_data *inc\n> return ret;\n> }\n>\n> +static int prepare_include_condition_pattern(struct strbuf *pat)\n> +{\n> + int prefix = 0;\n> +\n> + /* TODO: maybe support ~user/ too */\n> + if (pat->buf[0] == '~' && is_dir_sep(pat->buf[1])) {\n> + struct strbuf path = STRBUF_INIT;\n> + const char *home = getenv(\"HOME\");\n> +\n> + if (!home)\n> + return error(_(\"$HOME is not defined\"));\n> +\n> + strbuf_add_absolute_path(&path, home);\n> + strbuf_splice(pat, 0, 1, path.buf, path.len);\n> + /*\n> + * This part, path.buf[0..len], should be considered\n> + * a literal string even if it has wildcards in it,\n> + * because those wildcards are not wanted by the user.\n> + */\n> + prefix = path.len + 1 /*slash*/;\n> + strbuf_release(&path);\n> + } else if (pat->buf[0] == '.' && is_dir_sep(pat->buf[1])) {\n> + struct strbuf path = STRBUF_INIT;\n> + const char *slash;\n> +\n> + if (!cf || !cf->path)\n> + return error(_(\"relative config include \"\n> + \"conditionals must come from files\"));\n> +\n> + strbuf_add_absolute_path(&path, cf->path);\n> + slash = find_last_dir_sep(path.buf);\n> + if (!slash)\n> + die(\"BUG: how is this possible?\");\n> + strbuf_splice(pat, 0, 1, path.buf, slash - path.buf);\n> + /*\n> + * This part, path.buf[0..slash], should be consider\n> + * a literal string even if it has wildcards in it,\n> + * because those wildcards are not wanted by the user.\n> + */\n> + prefix = slash - path.buf + 1 /* slash */;\n> + strbuf_release(&path);\n> + } else if (!is_absolute_path(pat->buf))\n> + strbuf_insert(pat, 0, \"**/\", 3);\n> +\n> + if (pat->len && is_dir_sep(pat->buf[pat->len - 1]))\n> + strbuf_addstr(pat, \"**\");\n> +\n> + return prefix;\n> +}\n> +\n> +static int include_by_gitdir(const char *cond, size_t cond_len, int icase)\n> +{\n> + struct strbuf text = STRBUF_INIT;\n> + struct strbuf pattern = STRBUF_INIT;\n> + int ret = 0, prefix;\n> +\n> + strbuf_add_absolute_path(&text, get_git_dir());\n> + strbuf_add(&pattern, cond, cond_len);\n> + prefix = prepare_include_condition_pattern(&pattern);\n> +\n> + if (prefix < 0)\n> + goto done;\n> +\n> + if (prefix > 0) {\n> + /*\n> + * perform literal matching on the expanded prefix\n> + * part so that any wildcard character in it (e.g in\n> + * the expansion of ~) can't create side effects.\n> + */\n> + if (text.len < prefix)\n> + goto done;\n> + if (!icase && strncmp(pattern.buf, text.buf, prefix))\n> + goto done;\n> + if (icase && strncasecmp(pattern.buf, text.buf, prefix))\n> + goto done;\n> + }\n> +\n> + ret = !wildmatch(pattern.buf + prefix, text.buf + prefix,\n> + icase ? WM_CASEFOLD : 0, NULL);\n> +\n> +done:\n> + strbuf_release(&pattern);\n> + strbuf_release(&text);\n> + return ret;\n> +}\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> + error(_(\"unrecognized include condition: %.*s\"), (int)cond_len, cond);\n> + /* unknown conditionals are always false */\n> + return 0;\n> +}\n> +\n> int git_config_include(const char *var, const char *value, void *data)\n> {\n> struct config_include_data *inc = data;\n> + const char *cond, *key;\n> + int cond_len;\n> int ret;\n>\n> /*\n> @@ -181,8 +286,12 @@ int git_config_include(const char *var, const char *value, void *data)\n> if (ret < 0)\n> return ret;\n>\n> - if (!strcmp(var, \"include.path\"))\n> - ret = handle_path_include(value, inc);\n> + if (!parse_config_key(var, \"include\", &cond, &cond_len, &key) &&\n> + include_condition_is_true(cond, cond_len)) {\n> + if (!strcmp(key, \"path\"))\n> + ret = handle_path_include(value, inc);\n> + /* else we do not know about this type of include; ignore */\n> + }\n> return ret;\n> }\n>\n> diff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh\n> index 9ba2ba1..83501ec 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> test_must_fail git config --file - test.one\n> '\n>\n> +test_expect_success 'conditional include, both unanchored' '\n> + git init foo &&\n> + (\n> + cd foo &&\n> + echo \"[include \\\"gitdir:foo/\\\"]path=bar\" >>.git/config &&\n> + echo \"[test]one=1\" >.git/bar &&\n> + echo 1 >expect &&\n> + git config test.one >actual &&\n> + test_cmp expect actual\n> + )\n> +'\n> +\n> +test_expect_success 'conditional include, $HOME expansion' '\n> + (\n> + cd foo &&\n> + echo \"[include \\\"gitdir:~/foo/\\\"]path=bar2\" >>.git/config &&\n> + echo \"[test]two=2\" >.git/bar2 &&\n> + echo 2 >expect &&\n> + git config test.two >actual &&\n> + test_cmp expect actual\n> + )\n> +'\n> +\n> +test_expect_success 'conditional include, full pattern' '\n> + (\n> + cd foo &&\n> + echo \"[include \\\"gitdir:**/foo/**\\\"]path=bar3\" >>.git/config &&\n> + echo \"[test]three=3\" >.git/bar3 &&\n> + echo 3 >expect &&\n> + git config test.three >actual &&\n> + test_cmp expect actual\n> + )\n> +'\n> +\n> +test_expect_success 'conditional include, relative path' '\n> + echo \"[include \\\"gitdir:./foo/.git\\\"]path=bar4\" >>.gitconfig &&\n> + echo \"[test]four=4\" >bar4 &&\n> + (\n> + cd foo &&\n> + echo 4 >expect &&\n> + git config test.four >actual &&\n> + test_cmp expect actual\n> + )\n> +'\n> +\n> +test_expect_success 'conditional include, both unanchored, icase' '\n> + (\n> + cd foo &&\n> + echo \"[include \\\"gitdir/i:FOO/\\\"]path=bar5\" >>.git/config &&\n> + echo \"[test]five=5\" >.git/bar5 &&\n> + echo 5 >expect &&\n> + git config test.five >actual &&\n> + test_cmp expect actual\n> + )\n> +'\n> +\n> test_expect_success 'include cycles are detected' '\n> cat >.gitconfig <<-\\EOF &&\n> [test]value = gitconfig\n> --\n> 2.9.1.566.gbd532d4\n>\n\n\n\n-- \nDuy\n"},{"id":"299700","messageId":"20160819135408.ckyw6ruseicvg2jt@sigill.intra.peff.net","threadId":"42714","inReplyTo":"CACsJy8Bw0ZNu-6SB0P3dBZCLMJWJkbUqb64H_QOcn4UH+_AcNA@mail.gmail.com","subject":"Re: [PATCH v4] config: add conditional include","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-08-19T13:54:08Z","receivedAt":"2016-08-19T13:54:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Aug 13, 2016 at 03:40:59PM +0700, Duy Nguyen wrote:\n\n> Ping..\n\nThere 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 - the syntax might be more clear as:\n\n [include-if \"gitdir:...\"]\n\n or\n\n [include \"gitdir-is:...\"]\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\n-Peff\n"},{"id":"299800","messageId":"5c131421-ae7f-8a37-76ab-0fd05cbe3530@gmail.com","threadId":"42714","inReplyTo":"20160819135408.ckyw6ruseicvg2jt@sigill.intra.peff.net","subject":"Re: [PATCH v4] config: add conditional include","fromName":"Jakub Narębski","fromEmail":"jnareb@gmail.com","sentAt":"2016-08-20T21:08:19Z","receivedAt":"2016-08-20T21:12:06Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"W dniu 19.08.2016 o 15:54, Jeff King pisze:\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> \n> - the syntax might be more clear as:\n> \n> [include-if \"gitdir:...\"]\n> \n> or\n> \n> [include \"gitdir-is:...\"]\n\nOr\n\n [include \"if-gitdir:...\"]\n\nto continue bikeshedding.\n\n-- \nJakub Narębski\n\n"},{"id":"299838","messageId":"CACsJy8C30=-LGMYQJ6MO17L8Vv1q=iQGC=R8TDhC5qM1f5Lh5A@mail.gmail.com","threadId":"42714","inReplyTo":"5c131421-ae7f-8a37-76ab-0fd05cbe3530@gmail.com","subject":"Re: [PATCH v4] config: add conditional include","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-08-22T12:43:49Z","receivedAt":"2016-08-22T12:44:26Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sun, Aug 21, 2016 at 4:08 AM, Jakub Narębski wrote:\n> W dniu 19.08.2016 o 15:54, Jeff King pisze:\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>>\n>> - the syntax might be more clear as:\n>>\n>> [include-if \"gitdir:...\"]\n>>\n>> or\n>>\n>> [include \"gitdir-is:...\"]\n>\n> Or\n>\n> [include \"if-gitdir:...\"]\n\nI like this one. I can re-roll to address the first two bullet point,\nif the last one, the open question, will not become a blocker later\non.\n-- \nDuy\n"},{"id":"299843","messageId":"vpqr39harit.fsf@anie.imag.fr","threadId":"42714","inReplyTo":"CACsJy8C30=-LGMYQJ6MO17L8Vv1q=iQGC=R8TDhC5qM1f5Lh5A@mail.gmail.com","subject":"Re: [PATCH v4] config: add conditional include","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2016-08-22T12:59:22Z","receivedAt":"2016-08-22T13:00:29Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Duy Nguyen writes:\n\n> On Sun, Aug 21, 2016 at 4:08 AM, Jakub Narębski wrote:\n>> W dniu 19.08.2016 o 15:54, Jeff King pisze:\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>>>\n>>> - the syntax might be more clear as:\n>>>\n>>> [include-if \"gitdir:...\"]\n>>>\n>>> or\n>>>\n>>> [include \"gitdir-is:...\"]\n>>\n>> Or\n>>\n>> [include \"if-gitdir:...\"]\n>\n> I like this one. I can re-roll to address the first two bullet point,\n> if the last one, the open question, will not become a blocker later\n> on.\n\nI think the syntax should be design to allow arbitrary boolean\nexpression later if needed. Then, I prefer one of\n\n [include-if \"gitdir-is:...\"]\n [include \"gitdir-is:...\"]\n\nbecause it may later be extended to e.g.\n\n [include-if \"not(gitdir-is:...)\"]\n [include-if \"gitdir-matches:regex\"]\n [include-if \"gitdir-is:... and git-version-greater:2.9\"]\n ...\n\nI actually already use \"conditional include on version number\" because I\nuse push.default=upstream which makes older versions of Git crash, but\nfortunately these versions of Git also ignore the \"include\" directive so\nhaving this push.default=upstream in an included file works. It's a\nhack, it worked once but it won't work again.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"299845","messageId":"CACsJy8BNP8GDRxDGTQwcYWTovN9o11TCkUJ28=5zav3QnGv9nQ@mail.gmail.com","threadId":"42714","inReplyTo":"vpqr39harit.fsf@anie.imag.fr","subject":"Re: [PATCH v4] config: add conditional include","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-08-22T13:09:39Z","receivedAt":"2016-08-22T13:10:51Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Aug 22, 2016 at 7:59 PM, Matthieu Moy\n wrote:\n> Duy Nguyen writes:\n>\n>> On Sun, Aug 21, 2016 at 4:08 AM, Jakub Narębski wrote:\n>>> W dniu 19.08.2016 o 15:54, Jeff King pisze:\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>>>>\n>>>> - the syntax might be more clear as:\n>>>>\n>>>> [include-if \"gitdir:...\"]\n>>>>\n>>>> or\n>>>>\n>>>> [include \"gitdir-is:...\"]\n>>>\n>>> Or\n>>>\n>>> [include \"if-gitdir:...\"]\n>>\n>> I like this one. I can re-roll to address the first two bullet point,\n>> if the last one, the open question, will not become a blocker later\n>> on.\n>\n> I think the syntax should be design to allow arbitrary boolean\n> expression later if needed.\n\nI would be against that. We may extend it more in future, but it\nshould be under control, not full boolean expressions.\n\n> Then, I prefer one of\n>\n> [include-if \"gitdir-is:...\"]\n> [include \"gitdir-is:...\"]\n>\n> because it may later be extended to e.g.\n>\n> [include-if \"not(gitdir-is:...)\"]\n> [include-if \"gitdir-matches:regex\"]\n> [include-if \"gitdir-is:... and git-version-greater:2.9\"]\n> ...\n>\n> I actually already use \"conditional include on version number\" because I\n> use push.default=upstream which makes older versions of Git crash, but\n> fortunately these versions of Git also ignore the \"include\" directive so\n> having this push.default=upstream in an included file works. It's a\n> hack, it worked once but it won't work again.\n\nWe probably have a way to stop old git from reading new git's\nfeatures, including ones in config files: the config group\nextensions.*. Assuming that \"[include \"blah\"]\" is new stuff, git can\nbe taught to accept that only when extensions.blah is present (which\nold git would bail). It discourages adding too many fancy features\n(because extensions.* in your config file would be a mess), which is\nIMO a good point.\n-- \nDuy\n"},{"id":"299847","messageId":"vpqlgzp9bw1.fsf@anie.imag.fr","threadId":"42714","inReplyTo":"CACsJy8BNP8GDRxDGTQwcYWTovN9o11TCkUJ28=5zav3QnGv9nQ@mail.gmail.com","subject":"Re: [PATCH v4] config: add conditional include","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2016-08-22T13:22:22Z","receivedAt":"2016-08-22T13:22:30Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Duy Nguyen writes:\n\n> On Mon, Aug 22, 2016 at 7:59 PM, Matthieu Moy\n> wrote:\n>> Duy Nguyen writes:\n>>\n>>> On Sun, Aug 21, 2016 at 4:08 AM, Jakub Narębski wrote:\n>>>> W dniu 19.08.2016 o 15:54, Jeff King pisze:\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>>>>>\n>>>>> - the syntax might be more clear as:\n>>>>>\n>>>>> [include-if \"gitdir:...\"]\n>>>>>\n>>>>> or\n>>>>>\n>>>>> [include \"gitdir-is:...\"]\n>>>>\n>>>> Or\n>>>>\n>>>> [include \"if-gitdir:...\"]\n>>>\n>>> I like this one. I can re-roll to address the first two bullet point,\n>>> if the last one, the open question, will not become a blocker later\n>>> on.\n>>\n>> I think the syntax should be design to allow arbitrary boolean\n>> expression later if needed.\n>\n> I would be against that. We may extend it more in future, but it\n> should be under control, not full boolean expressions.\n\nWhy?\n\nI'm not saying we absolutely need it, but if we allow several kinds of\nconditions (gitdir-is:... and others in the future), then it's natural\nto allow combining them, and arbitrary boolean expression is both simple\nand powerful (operators and/or/not and parenthesis and you have\neverything you'll ever need).\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"299849","messageId":"CACsJy8BokiiDbheN1_CqWWgK3xRn8YP30t7ojjVeDnok5_M4Ow@mail.gmail.com","threadId":"42714","inReplyTo":"vpqlgzp9bw1.fsf@anie.imag.fr","subject":"Re: [PATCH v4] config: add conditional include","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-08-22T13:32:42Z","receivedAt":"2016-08-22T13:33:48Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Aug 22, 2016 at 8:22 PM, Matthieu Moy\n wrote:\n>>> I think the syntax should be design to allow arbitrary boolean\n>>> expression later if needed.\n>>\n>> I would be against that. We may extend it more in future, but it\n>> should be under control, not full boolean expressions.\n>\n> Why?\n>\n> I'm not saying we absolutely need it, but if we allow several kinds of\n> conditions (gitdir-is:... and others in the future), then it's natural\n> to allow combining them, and arbitrary boolean expression is both simple\n> and powerful (operators and/or/not and parenthesis and you have\n> everything you'll ever need).\n\nFor starter, we don't want another debate \"python vs ruby vs lua vs\n...\" as the scripting language :) (for the record I vote Scheme! maybe\nwith infix syntax)\n\nSeriously though, for things that are evaluated in the background\nevery single time a command is executed and things that come from many\ndifferent places (/etc, $HOME, $XDG, $GIT_DIR) I think absolute\nflexibility just makes it harder to pinpoint when things go wrong.\nCombination in this case would be a bad thing, not a good one. By\njudging case by case before introducing a new condition type, we\nprobably can see what sort of combination would be and whether we\ncould accept that.\n-- \nDuy\n"},{"id":"299928","messageId":"alpine.DEB.2.20.1608231541420.4924@virtualbox","threadId":"42714","inReplyTo":"CACsJy8BokiiDbheN1_CqWWgK3xRn8YP30t7ojjVeDnok5_M4Ow@mail.gmail.com","subject":"Re: [PATCH v4] config: add conditional include","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-08-23T13:42:35Z","receivedAt":"2016-08-23T13:42:55Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Duy,\n\nOn Mon, 22 Aug 2016, Duy Nguyen wrote:\n\n> On Mon, Aug 22, 2016 at 8:22 PM, Matthieu Moy\n> wrote:\n> >>> I think the syntax should be design to allow arbitrary boolean\n> >>> expression later if needed.\n> >>\n> >> I would be against that. We may extend it more in future, but it\n> >> should be under control, not full boolean expressions.\n> >\n> > Why?\n> >\n> > I'm not saying we absolutely need it, but if we allow several kinds of\n> > conditions (gitdir-is:... and others in the future), then it's natural\n> > to allow combining them, and arbitrary boolean expression is both simple\n> > and powerful (operators and/or/not and parenthesis and you have\n> > everything you'll ever need).\n> \n> For starter, we don't want another debate \"python vs ruby vs lua vs\n> ...\" as the scripting language :) (for the record I vote Scheme! maybe\n> with infix syntax)\n\nFWIW I do not think that Matthieu implied that this has to be implemented.\nBut it does not make sense to slam the door shut prematurely, either.\n\nMeaning: the more doors you can keep open with the new syntax, the better.\n\nCiao,\nDscho\n"},{"id":"300037","messageId":"CACsJy8CY1HqSTtmPw+H1yGXV_tBPt3GVU162ZTVecXFhPTnaYA@mail.gmail.com","threadId":"42714","inReplyTo":"alpine.DEB.2.20.1608231541420.4924@virtualbox","subject":"Re: [PATCH v4] config: add conditional include","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-08-24T09:37:34Z","receivedAt":"2016-08-24T10:06:00Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Aug 23, 2016 at 8:42 PM, Johannes Schindelin\n wrote:\n> Hi Duy,\n>\n> On Mon, 22 Aug 2016, Duy Nguyen wrote:\n>\n>> On Mon, Aug 22, 2016 at 8:22 PM, Matthieu Moy\n>> wrote:\n>> >>> I think the syntax should be design to allow arbitrary boolean\n>> >>> expression later if needed.\n>> >>\n>> >> I would be against that. We may extend it more in future, but it\n>> >> should be under control, not full boolean expressions.\n>> >\n>> > Why?\n>> >\n>> > I'm not saying we absolutely need it, but if we allow several kinds of\n>> > conditions (gitdir-is:... and others in the future), then it's natural\n>> > to allow combining them, and arbitrary boolean expression is both simple\n>> > and powerful (operators and/or/not and parenthesis and you have\n>> > everything you'll ever need).\n>>\n>> For starter, we don't want another debate \"python vs ruby vs lua vs\n>> ...\" as the scripting language :) (for the record I vote Scheme! maybe\n>> with infix syntax)\n>\n> FWIW I do not think that Matthieu implied that this has to be implemented.\n> But it does not make sense to slam the door shut prematurely, either.\n>\n> Meaning: the more doors you can keep open with the new syntax, the better.\n\nIt works for me either way. So I'm going to assume we want\n\"[include-if \"gitdir-is:...\"]\", unless people think it needs more\ndiscussion (then just write something, anything, so I can put it back\nin my backlog)\n-- \nDuy\n"},{"id":"300045","messageId":"febf6605-70e7-4653-1e3e-cddc1882e037@gmail.com","threadId":"42714","inReplyTo":"CACsJy8CY1HqSTtmPw+H1yGXV_tBPt3GVU162ZTVecXFhPTnaYA@mail.gmail.com","subject":"Re: [PATCH v4] config: add conditional include","fromName":"Jakub Narębski","fromEmail":"jnareb@gmail.com","sentAt":"2016-08-24T12:44:29Z","receivedAt":"2016-08-24T12:44:40Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"W dniu 24.08.2016 o 11:37, Duy Nguyen pisze:\n\n> It works for me either way. So I'm going to assume we want\n> \"[include-if \"gitdir-is:...\"]\", unless people think it needs more\n> discussion (then just write something, anything, so I can put it back\n> in my backlog)\n\nA final note: [include \"if-gitdir:...\"] is inclusive by default\n(I think), that is old Git would include it unconditionally,\nwhile [include-if \"gitdir-is:...\"] is exclusive by default, that\nis old Git would ignore it.\n\nBut I might be mistaken.\n\nP.S. I personally prefer [include-if ...]\n-- \nJakub Narębski\n\n"},{"id":"300049","messageId":"20160824141702.rszccucv62rqrcxs@sigill.intra.peff.net","threadId":"42714","inReplyTo":"febf6605-70e7-4653-1e3e-cddc1882e037@gmail.com","subject":"Re: [PATCH v4] config: add conditional include","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-08-24T14:17:02Z","receivedAt":"2016-08-24T14:17:31Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Aug 24, 2016 at 02:44:29PM +0200, Jakub Narębski wrote:\n\n> W dniu 24.08.2016 o 11:37, Duy Nguyen pisze:\n> \n> > It works for me either way. So I'm going to assume we want\n> > \"[include-if \"gitdir-is:...\"]\", unless people think it needs more\n> > discussion (then just write something, anything, so I can put it back\n> > in my backlog)\n> \n> A final note: [include \"if-gitdir:...\"] is inclusive by default\n> (I think), that is old Git would include it unconditionally,\n> while [include-if \"gitdir-is:...\"] is exclusive by default, that\n> is old Git would ignore it.\n> \n> But I might be mistaken.\n\nI don't think this is the case. Conditionals were considered when\nthe include feature was added, and anything except \"include.path\" is\nexplicitly ignored. So either syntax will behave the same.\n\n-Peff\n"}]}