{"thread":{"id":"39828","subject":"[RFC] ident: support per-path configs by matching the path against a pattern","startedAt":"2015-07-10T09:36:38Z","lastAt":"2015-07-10T21:07:46Z","messageCount":7,"participants":["Sebastian Schuberth","Junio C Hamano","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"265978","messageId":"0000014e7752e758-a0bf7acb-2d0f-4492-8004-8eeeb9b2f042-000000@eu-west-1.amazonses.com","threadId":"39828","inReplyTo":null,"subject":"[RFC] ident: support per-path configs by matching the path against a pattern","fromName":"Sebastian Schuberth","fromEmail":"sschuberth@gmail.com","sentAt":"2015-07-10T09:36:38Z","receivedAt":"2015-07-10T09:36:38Z","isPatch":false,"sender":{"key":"sschuberth@gmail.com","avatar":"https://avatars.githubusercontent.com/u/349154?v=4"},"body":"Support per-path identities by configuring Git like\n\n    $ git config user.<pattern>.email <email address>\n\ne.g.\n\n    $ git config user.github.email sschuberth+github@gmail.com\n\nIn this example, the middle \"github\" pattern is searched for\ncase-insensitively in the absolute path name to the current git work tree.\nIf there is a match the configured email address is used instead of what\notherwise would be the default email address.\n\nNote that repository-local identities still take precedence over global /\nsystem ones even if the pattern configured in the global / system identity\nmatches the path to the local work tree.\n\nThis change avoids the need to use external tools like [1].\n\nTODO: Once the community agrees that this is a feature worth having, add\ntests and adjust the docs.\n\n[1] https://github.com/prydonius/karn\n\nSigned-off-by: Sebastian Schuberth <sschuberth@gmail.com>\n---\n ident.c | 18 ++++++++++++++++--\n 1 file changed, 16 insertions(+), 2 deletions(-)\n\ndiff --git a/ident.c b/ident.c\nindex 5ff1aad..2429ed8 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -390,7 +390,21 @@ int author_ident_sufficiently_given(void)\n \n int git_ident_config(const char *var, const char *value, void *data)\n {\n-\tif (!strcmp(var, \"user.name\")) {\n+\t/* the caller has already checked that var starts with \"user.\" */\n+\n+\tconst char *first_period = strchr(var, '.');\n+\tconst char *last_period = strrchr(var, '.');\n+\n+\tif (first_period < last_period) {\n+\t\t++first_period;\n+\t\tchar *pattern = xstrndup(first_period, last_period - first_period);\n+\t\tconst char *match = strcasestr(get_git_work_tree(), pattern);\n+\t\tfree(pattern);\n+\t\tif (!match)\n+\t\t\treturn 0;\n+\t}\n+\n+\tif (ends_with(var, \".name\")) {\n \t\tif (!value)\n \t\t\treturn config_error_nonbool(var);\n \t\tstrbuf_reset(&git_default_name);\n@@ -400,7 +414,7 @@ int git_ident_config(const char *var, const char *value, void *data)\n \t\treturn 0;\n \t}\n \n-\tif (!strcmp(var, \"user.email\")) {\n+\tif (ends_with(var, \".email\")) {\n \t\tif (!value)\n \t\t\treturn config_error_nonbool(var);\n \t\tstrbuf_reset(&git_default_email);\n\n---\nhttps://github.com/git/git/pull/161\n"},{"id":"265986","messageId":"xmqqr3ogkpz5.fsf@gitster.dls.corp.google.com","threadId":"39828","inReplyTo":"0000014e7752e758-a0bf7acb-2d0f-4492-8004-8eeeb9b2f042-000000@eu-west-1.amazonses.com","subject":"Re: [RFC] ident: support per-path configs by matching the path against a pattern","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-10T15:10:54Z","receivedAt":"2015-07-10T15:10:54Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sebastian Schuberth <sschuberth@gmail.com> writes:\n\n> Support per-path identities by configuring Git like\n>\n>     $ git config user.<pattern>.email <email address>\n>\n> e.g.\n>\n>     $ git config user.github.email sschuberth+github@gmail.com\n>\n> In this example, the middle \"github\" pattern is searched for\n> case-insensitively in the absolute path name to the current git work tree.\n\nI do agree it is a good feature to have to allow you to keep a\ncentralized registry of possible configuration in a single place,\ne.g. $HOME/.gitconfig, and selectively apply pieces for multiple\nplaces.\n\nHaving said that, a few comments.\n\n - It feels very hacky to only do this for the ident.  You would\n   want to have, (conceptually, not necessarily at the syntax level)\n   something more along the lines of:\n\n   \tif path matches this pattern\n        \t[user]\n                \temail = address\n                        name = name\n\tend\n\n   to allow any configuration to be covered by this new \"selectively\n   use from the centralized registry\" feature.\n\n - It is iffy to match the pattern with \"working tree\".  There are\n   pros and cons but a viable alternative would be to match with the\n   location of the $GIT_DIR.  Given that we now are slowly moving to\n   a world where a single $GIT_DIR can have multiple working trees,\n   and that user.name ultimately matters per-project you are\n   contributing to, it is more likely that you would want to tie it\n   to a local repository, i.e. $GIT_DIR, no matter where working\n   trees you have a checkout out of that repository live.  They\n   would want to share the same setting.\n\n   On the other hand, some things may be truly per working tree even\n   if it is tied to the same local repository.\n\n   I am not saying it is better to tie this to $GIT_DIR not\n   $GIT_WORK_TREE.  I am not necessarily saying we should support\n   both.  I am only saying that we need to consider if tying only to\n   $GIT_WORK_TREE makes sense (and if so, clearly communicate that\n   to the end users).  I couldn't read from your patch [*1*] what\n   your stance on this point.\n\n - The pattern defined to be case-insensitive substring would not be\n   a sane general design.  I would have expected it to be fnmatch\n   pattern that may match case insensitively only on case insensitive\n   filesystems.\n\n\n[Footnote]\n\n*1* this is one of the reasons why I usually ignore \"I'll do the doc\nand test if people agree it is a good idea.\"  Often, we cannot judge\nif it is a good idea without these things.\n"},{"id":"265988","messageId":"20150710154308.GA29395@peff.net","threadId":"39828","inReplyTo":"xmqqr3ogkpz5.fsf@gitster.dls.corp.google.com","subject":"Re: [RFC] ident: support per-path configs by matching the path against a pattern","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-07-10T15:43:08Z","receivedAt":"2015-07-10T15:43:08Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jul 10, 2015 at 08:10:54AM -0700, Junio C Hamano wrote:\n\n> I do agree it is a good feature to have to allow you to keep a\n> centralized registry of possible configuration in a single place,\n> e.g. $HOME/.gitconfig, and selectively apply pieces for multiple\n> places.\n> \n> Having said that, a few comments.\n> \n>  - It feels very hacky to only do this for the ident.  You would\n>    want to have, (conceptually, not necessarily at the syntax level)\n>    something more along the lines of:\n> \n>    \tif path matches this pattern\n>         \t[user]\n>                 \temail = address\n>                         name = name\n> \tend\n> \n>    to allow any configuration to be covered by this new \"selectively\n>    use from the centralized registry\" feature.\n\nYeah, I agree it would be nice to cover all config keys. Since it's\nsyntactically difficult to add conditionals to the existing config\nformat, I tried to leave open a space for this in the \"include\" design.\nThat is, right now:\n\n  [include]\n  path = foo\n\nwill unconditionally include \"foo\". But something like:\n\n  [include \"gitdir:bar\"]\n  path = foo\n\ncould do so only when the \"gitdir:bar\" conditional is satisfied (where\nthat is just a syntax I made up to mean fnmatch(\"bar\", $GIT_DIR)). So\nlike user.<pattern>.*, we still put our section-specific hack into one\nspecial section, but that one place is capable of chaining to multiple\nother config keys. :)\n\nMy only request is that any conditional we add have some prefix (like\n\"gitdir:\") so that we have space to add more types later if we choose.\n\nThere's discussion on this topic somewhere on the list, but I didn't\nbother to dig it up. I don't think it adds anything over what I\nsummarized above.\n\n-Peff\n"},{"id":"265992","messageId":"20150710164652.GA30113@peff.net","threadId":"39828","inReplyTo":"20150710154308.GA29395@peff.net","subject":"Re: [RFC] ident: support per-path configs by matching the path against a pattern","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-07-10T16:46:52Z","receivedAt":"2015-07-10T16:46:52Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jul 10, 2015 at 11:43:08AM -0400, Jeff King wrote:\n\n> But something like:\n> \n>   [include \"gitdir:bar\"]\n>   path = foo\n> \n> could do so only when the \"gitdir:bar\" conditional is satisfied (where\n> that is just a syntax I made up to mean fnmatch(\"bar\", $GIT_DIR)). So\n> like user.<pattern>.*, we still put our section-specific hack into one\n> special section, but that one place is capable of chaining to multiple\n> other config keys. :)\n\nHere's a sketch if anybody is inclined to pick it up and run with it.\nNote that I did not think too hard about little things like the\nde-anchoring.\n\n---\ndiff --git a/config.c b/config.c\nindex 29fa012..47b01f0 100644\n--- a/config.c\n+++ b/config.c\n@@ -139,9 +139,45 @@ 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\tint ret;\n+\n+\t\tstrbuf_add_absolute_path(&text, get_git_dir());\n+\n+\t\t/* de-anchor match for convenience */\n+\t\tstrbuf_addstr(&pattern, \"**\");\n+\t\tstrbuf_add(&pattern, value, cond_len - (value - cond));\n+\t\tstrbuf_addstr(&pattern, \"**\");\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@@ -152,8 +188,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"},{"id":"265999","messageId":"xmqqtwtbkbis.fsf@gitster.dls.corp.google.com","threadId":"39828","inReplyTo":"20150710164652.GA30113@peff.net","subject":"Re: [RFC] ident: support per-path configs by matching the path against a pattern","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-10T20:23:07Z","receivedAt":"2015-07-10T20:23:07Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Here's a sketch if anybody is inclined to pick it up and run with it.\n> Note that I did not think too hard about little things like the\n> de-anchoring.\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> @@ -152,8 +188,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\nI really like these two lines.\n\nWhoever designed that parse_config_key() interface either is a\ngenious or had a foresight with a crystal ball, or perhaps both.\n\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"},{"id":"266002","messageId":"20150710205821.GB1181@peff.net","threadId":"39828","inReplyTo":"xmqqtwtbkbis.fsf@gitster.dls.corp.google.com","subject":"Re: [RFC] ident: support per-path configs by matching the path against a pattern","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-07-10T20:58:21Z","receivedAt":"2015-07-10T20:58:21Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jul 10, 2015 at 01:23:07PM -0700, Junio C Hamano wrote:\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> \n> I really like these two lines.\n> \n> Whoever designed that parse_config_key() interface either is a\n> genious or had a foresight with a crystal ball, or perhaps both.\n\nHeh. I peeked at the archive, and I think it was actually designed by\ncommittee. Perhaps we are greater than the sum of our parts? ;)\n\n-Peff\n"},{"id":"266004","messageId":"xmqqlhenk9gd.fsf@gitster.dls.corp.google.com","threadId":"39828","inReplyTo":"20150710205821.GB1181@peff.net","subject":"Re: [RFC] ident: support per-path configs by matching the path against a pattern","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-10T21:07:46Z","receivedAt":"2015-07-10T21:07:46Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Fri, Jul 10, 2015 at 01:23:07PM -0700, Junio C Hamano wrote:\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>> \n>> I really like these two lines.\n>> \n>> Whoever designed that parse_config_key() interface either is a\n>> genious or had a foresight with a crystal ball, or perhaps both.\n>\n> Heh. I peeked at the archive, and I think it was actually designed by\n> committee. Perhaps we are greater than the sum of our parts? ;)\n\nIt does not look like a camel, though ;-).\n\nThanks.  Let's see if Sebastian finds the direction satisfactory.\n"}]}