{"thread":{"id":"35993","subject":"RFC GSoC idea: new \"git config\" features","startedAt":"2014-02-28T12:51:28Z","lastAt":"2014-03-14T21:00:56Z","messageCount":8,"participants":["Michael Haggerty","Junio C Hamano","Jeff King","Matthieu Moy"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"235603","messageId":"53108650.2020708@alum.mit.edu","threadId":"35993","inReplyTo":null,"subject":"RFC GSoC idea: new \"git config\" features","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-02-28T12:51:28Z","receivedAt":"2014-02-28T12:51:28Z","isPatch":false,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"I just wrote up another double-idea that has been stewing in my head for\na while:\n\n* Allow configuration values to be unset via a config file\n* Fix \"git config --unset\" to clean up detritus from sections that are\nleft empty.\n\nThese ideas are more \"out there\" than the last, and might be too\ncontroversial to be implemented, let alone as a GSoC project.  I'd\ndefinitely like some feedback.\n\nAnd if you like this idea or the other one I proposed, please volunteer\nto be a co-mentor!  I will be traveling for a few weeks this summer, so\nI *won't* be able to be the sole mentor to a student.\n\nI wrote up this idea in the following pull request:\n\n    https://github.com/git/git.github.io/pull/6\n\nI will also append the text, for your mailing-list-reading convenience.\n\nMichael\n\n### `git config` improvements\n\nThis project proposes the following two improvements to `git config`.\nPlease note that this project has a significant \"political\" component\nto it, because some of the details of the features will be\ncontroversial.\n\n#### Unsetting configuration options\n\nSome Git configuration options have an effect by their mere existence.\n(I.e., setting the option to \"false\" or the empty string is different\nthan leaving it unset altogether.)  Also, some configuration options\ncan take multiple values.\n\nHowever, there is no way for an option file to \"unset\" an option--that\nis, to change the option back to \"unset\".  This is awkward, because\nconfiguration values are read from various places (`/etc/gitconfig`,\n`~/.config/git/config` or `~/.gitconfig`, and `$GIT_DIR/config`, plus\nperhaps files that are included by other configuration files).\nTherefore, if an option is set in one of the earlier files, there is\nno way for it to be unset in a later one.  The unwanted option might\nhave even been set in a file like `/etc/gitconfig` that the user\ndoesn't have permission to modify.\n\nIt would be nice to have a syntax that can be used to unset any\npreviously-defined values for an option.  Perhaps\n\n    [section \"subsection\"]\n            !option\n\nThe above is currently currently a syntax error that causes Git to\nterminate, so some thought has to go into a transition plan for\nenabling this feature.  Maybe a syntax has to be invented that\nconforms to the current format, like\n\n    [unset]\n            name = section.subsection.option\n\nBecause options are currently processed as they are read, this change\nwill require the code that reads options files to be changed\nsignificantly.\n\nLeave yourself a lot of time to attain a consensus on the mailing list\nabout how this can be done while retaining reasonable backwards\ncompatibility.\n\n#### Tidy configuration files\n\nWhen a configuration file is repeatedly modified, often garbage is\nleft behind.  For example, after\n\n    git config my.option true\n    git config --unset my.option\n    git config my.option true\n    git config --unset my.option\n\nthe bottom of the configuration file is left with the useless lines\n\n    [my]\n    [my]\n\nIt would be nice to clean up such garbage when rewriting the\nconfiguration file.  This project is a bit tricky because of the\npossible presence of comments.  For example, what if an empty section\nlooks like this:\n\n    [my]\n            # This section is for my own private settings\n\nor this:\n\n    [my]\n    # This section is for my own private settings\n\nor this:\n\n    # This section is for my own private settings:\n    [my]\n\n?  In some such cases it might be desireable either to retain the\nsection even though it is empty, or to delete the comment along with\nthe section.  Very likely there will be some obvious patterns when\neverybody agrees that an empty section can be deleted, and other, more\ncontroversial cases where you will have to reach a consensus on the\nmailing list about what should be done.\n\n - Language: C\n - Difficulty: medium\n - Possible mentors: Michael Haggerty\n\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"235662","messageId":"xmqqwqgft3bj.fsf@gitster.dls.corp.google.com","threadId":"35993","inReplyTo":"53108650.2020708@alum.mit.edu","subject":"Re: RFC GSoC idea: new \"git config\" features","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-02-28T20:00:48Z","receivedAt":"2014-02-28T20:00:48Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Haggerty <mhagger@alum.mit.edu> writes:\n\n> I just wrote up another double-idea that has been stewing in my head for\n> a while:\n>\n> * Allow configuration values to be unset via a config file\n> * Fix \"git config --unset\" to clean up detritus from sections that are\n> left empty.\n\nThe former is *way* too large for a GSoC project.  Most\nconfiguration variables are meant to be read sequencially and affect\nin-core variables directly, like\n\n        /* file-scope global */\n\tstatic int frotz = -1;  /* unset */\n\n        static int parse_config_frotz(const char *key, const char *value, void *cb)\n\t{\n        \tif (!strcmp(key, \"core.frotz\"))\n                \tfrotz = git_config_int(value);\n\t\treturn 0;\n\t}\n\n\t... and somewhere ...\n        \tgit_config(parse_config_frotz, NULL);\n\nThe config parsers are distributed and there is no single registry\nthat knows how in-core variables owned by each subsystem represent\nan \"unset\" value.  In the above example, -1 is such a sentinel\nvalue, but in some other contexts, the subsystem may choose to use\nINT_MAX.  The only way to allow \"resetting to previous\" is to\n\n (1) come up with a way to pass \"this key is being reset to\n     'unspecified'\" to existing git_config() callback functions\n     (like parse_config_frotz() in the above illustration), which\n     may or may not involve changing the function signature of the\n     callbacks;\n\n (2) go through all the git_config() callback functions and make\n     them understand the new \"reset to 'unspecified'\" convention.\n\nwhich may not sound too bad at the first glance (especially, the\nfirst one is almost trivial).\n\nBut the side effects these callbacks may cause are not limited to\nsetting a simple scaler variable (like 'frotz' in the illustration)\nbut would include things that are hard to undo once done\n(e.g. calling a set-up function with a lot of side effects).\n\nThe latter, on the other hand, should be a change that is of a\nfairly limited scope, and would be a good fit for a GSoC project\n(incidentally, it has been one of the items on my leftover-bits list\nhttp://git-blame.blogspot.com/p/leftover-bits.html for quite some\ntime).\n\nThanks.\n"},{"id":"235683","messageId":"53112794.2070007@alum.mit.edu","threadId":"35993","inReplyTo":"xmqqwqgft3bj.fsf@gitster.dls.corp.google.com","subject":"Re: RFC GSoC idea: new \"git config\" features","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-03-01T00:19:32Z","receivedAt":"2014-03-01T00:19:32Z","isPatch":false,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 02/28/2014 09:00 PM, Junio C Hamano wrote:\n> Michael Haggerty <mhagger@alum.mit.edu> writes:\n> \n>> I just wrote up another double-idea that has been stewing in my head for\n>> a while:\n>>\n>> * Allow configuration values to be unset via a config file\n>> * Fix \"git config --unset\" to clean up detritus from sections that are\n>> left empty.\n> \n> The former is *way* too large for a GSoC project.  Most\n> configuration variables are meant to be read sequencially and affect\n> in-core variables directly, like\n> \n>         /* file-scope global */\n> \tstatic int frotz = -1;  /* unset */\n> \n>         static int parse_config_frotz(const char *key, const char *value, void *cb)\n> \t{\n>         \tif (!strcmp(key, \"core.frotz\"))\n>                 \tfrotz = git_config_int(value);\n> \t\treturn 0;\n> \t}\n> \n> \t... and somewhere ...\n>         \tgit_config(parse_config_frotz, NULL);\n> \n> The config parsers are distributed and there is no single registry\n> that knows how in-core variables owned by each subsystem represent\n> an \"unset\" value.  In the above example, -1 is such a sentinel\n> value, but in some other contexts, the subsystem may choose to use\n> INT_MAX.  The only way to allow \"resetting to previous\" is to\n> \n>  (1) come up with a way to pass \"this key is being reset to\n>      'unspecified'\" to existing git_config() callback functions\n>      (like parse_config_frotz() in the above illustration), which\n>      may or may not involve changing the function signature of the\n>      callbacks;\n> \n>  (2) go through all the git_config() callback functions and make\n>      them understand the new \"reset to 'unspecified'\" convention.\n\nI absolutely understand that changing all of the config parsers is not\nfeasible.  But I had imagined a third route:\n\n(3) parse the config once, storing the raw values to records in\n    memory.  When an \"unset\" is seen, delete any previous records that\n    have accumulated for that key.  After the whole config has been\n    read, iterate through the records, feeding the surviving values\n    into the callback in the order they were originally read (minus\n    deletions).\n\nDo you see any problems with this way of implementing the functionality\n(aside from slightly increased overhead)?\n\nAnd once we have a way to store config records in memory, it might also\nmake sense to reuse the parsed values for later config inquiries (after\nchecking that the files have not changed since the last read).  After\nthis second step the net performance change might even be advantageous.\n\n> which may not sound too bad at the first glance (especially, the\n> first one is almost trivial).\n> \n> But the side effects these callbacks may cause are not limited to\n> setting a simple scaler variable (like 'frotz' in the illustration)\n> but would include things that are hard to undo once done\n> (e.g. calling a set-up function with a lot of side effects).\n> \n> The latter, on the other hand, should be a change that is of a\n> fairly limited scope, and would be a good fit for a GSoC project\n> (incidentally, it has been one of the items on my leftover-bits list\n> http://git-blame.blogspot.com/p/leftover-bits.html for quite some\n> time).\n\nBut only the latter part would be a bit meager as a GSoC project, don't\nyou think?\n\nThanks for the feedback.\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"235703","messageId":"20140301075247.GF20397@sigill.intra.peff.net","threadId":"35993","inReplyTo":"53112794.2070007@alum.mit.edu","subject":"Re: RFC GSoC idea: new \"git config\" features","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-03-01T07:52:47Z","receivedAt":"2014-03-01T07:52:47Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Mar 01, 2014 at 01:19:32AM +0100, Michael Haggerty wrote:\n\n> I absolutely understand that changing all of the config parsers is not\n> feasible.  But I had imagined a third route:\n> \n> (3) parse the config once, storing the raw values to records in\n>     memory.  When an \"unset\" is seen, delete any previous records that\n>     have accumulated for that key.  After the whole config has been\n>     read, iterate through the records, feeding the surviving values\n>     into the callback in the order they were originally read (minus\n>     deletions).\n> \n> Do you see any problems with this way of implementing the functionality\n> (aside from slightly increased overhead)?\n\nYeah, this is something I have considered many times. It does have some\noverhead, but the existing system is not great either. As you noted, we\noften read the config several times for a given program invocation.\n\nBut moreover, we linearly strcmp each config key we find against each\none we know about. In some cases we return early if a sub-function is\nlooking for `starts_with(key, \"foo.\")`, but in most cases we just look\nfor \"foo.bar\", \"foo.baz\", and so on.\n\nIf we had the keys in-memory, we could reverse this: config code asks\nfor keys it cares about, and we can do an optimized lookup (binary\nsearch, hash, etc).\n\nThat also makes many constructs easier to express. Recently we had a\nproblem where the parsing order of \"remote.pushdefault\" and\n\"branch.*.pushremote\" mattered, because they were read into the same\nvariable. The solution is to use two variables and reconcile them after\nall config is read. But if you can just query the config subsystem\ndirectly, the boilerplate of reading them into strings goes away, and\nyou can just do:\n\n  x = config_string_getf(\"branch.%s.pushremote\", current_branch);\n  if (!x)\n          x = config_string_get(\"remote.pushdefault\");\n  if (!x)\n          x = config_string_getf(\"branch.%s.remote\", current_branch);\n  if (!x)\n          x = \"origin\";\n\nAs it is now, the code that does this has a lot more boilerplate, and is\nsplit across several functions.\n\nAnother example is the way we have to manage \"deferred\" config in\ngit-status (see 84b4202). This might be more clear if we could simply\n`config_get_bool(\"status.branch\")` at the right moment.\n\nThe talk of efficiency is probably a red-herring here. I do not think\nconfig-reading is a significant source of slow-down in the current code.\nBut I'd be in favor of something that reduced boilerplate and made the\ncode easier to read.\n\n> > But the side effects these callbacks may cause are not limited to\n> > setting a simple scaler variable (like 'frotz' in the illustration)\n> > but would include things that are hard to undo once done\n> > (e.g. calling a set-up function with a lot of side effects).\n\nMost callbacks would convert to a query system in a pretty\nstraightforward way, but some that have side effects might be tricky.\nConverting them all may be too large for a GSoC project, but I think you\ncould do it gradually:\n\n  1. Convert the parser to read into an in-memory representation, but\n     leave git_config() as a wrapper which iterates over it.\n\n  2. Add query functions like config_string_get() above.\n\n  3. Convert callbacks to query functions one by one.\n\n  4. Eventually drop git_config().\n\nA GSoC project could take us partway through (3).\n\n-Peff\n"},{"id":"235709","messageId":"vpq38j2tc6f.fsf@anie.imag.fr","threadId":"35993","inReplyTo":"20140301075247.GF20397@sigill.intra.peff.net","subject":"Re: RFC GSoC idea: new \"git config\" features","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2014-03-01T11:01:44Z","receivedAt":"2014-03-01T11:01:44Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Jeff King <peff@peff.net> writes:\n\n> If we had the keys in-memory, we could reverse this: config code asks\n> for keys it cares about, and we can do an optimized lookup (binary\n> search, hash, etc).\n\nI'm actually dreaming of a system where a configuration variable could\nbe \"declared\" in Git's source code, with associated type (list/single\nvalue, boolean/string/path/...), default value and documentation (and\nthen Documentation/config.txt could become a generated file). One could\nimagine a lot of possibilities like\n\n$ git config --describe <some-variable>\nType: boolean\nDefault value: true\nDescription: ...\n\nSomehow, do for config variables what has been done for command-line\noption parsing.\n\nMigrating the whole code to such system would take time, but creating\nthe system and applying it to a few examples might be feasible as a GSoC\nproject.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"235916","messageId":"xmqqzjl7nj0p.fsf@gitster.dls.corp.google.com","threadId":"35993","inReplyTo":"20140301075247.GF20397@sigill.intra.peff.net","subject":"Re: RFC GSoC idea: new \"git config\" features","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-03T20:07:18Z","receivedAt":"2014-03-03T20:07:18Z","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> Most callbacks would convert to a query system in a pretty\n> straightforward way, but some that have side effects might be tricky.\n> Converting them all may be too large for a GSoC project, but I think you\n> could do it gradually:\n>\n>   1. Convert the parser to read into an in-memory representation, but\n>      leave git_config() as a wrapper which iterates over it.\n>\n>   2. Add query functions like config_string_get() above.\n>\n>   3. Convert callbacks to query functions one by one.\n>\n>   4. Eventually drop git_config().\n>\n> A GSoC project could take us partway through (3).\n\nI actually discarded the \"read from these config files to preparsed\nstructure to memory, later to be consumed by repeated calls to the\ngit_config() callback functions, making the only difference from the\ncurrent scheme that the preparsed structure will be reset when there\nis the new 'reset to the original' definition\" as obvious and\nuninteresting.\n\nThis is one of these times that I find myself blessed with capable\nothers that can go beyond, building on top of such an idea that I\nmay have discarded without thinking it through, around me ;-)\n\nYes, the new abstraction like config_<type>_get() that can live\nalongside the existing \"git_config() feeds callback chain\neverything\" and gradually replace the latter, would be a good way\nforward.  Given that we read configuration multiple times anyway for\ndifferent purposes, even without the new abstraction, the end result\nmight perform better if we read the files once and reused in later\ncalls to git_config().\n\nThanks.\n"},{"id":"236717","messageId":"20140314044326.GE31906@sigill.intra.peff.net","threadId":"35993","inReplyTo":"vpq38j2tc6f.fsf@anie.imag.fr","subject":"Re: RFC GSoC idea: new \"git config\" features","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-03-14T04:43:26Z","receivedAt":"2014-03-14T04:43:26Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Mar 01, 2014 at 12:01:44PM +0100, Matthieu Moy wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > If we had the keys in-memory, we could reverse this: config code asks\n> > for keys it cares about, and we can do an optimized lookup (binary\n> > search, hash, etc).\n> \n> I'm actually dreaming of a system where a configuration variable could\n> be \"declared\" in Git's source code, with associated type (list/single\n> value, boolean/string/path/...), default value and documentation (and\n> then Documentation/config.txt could become a generated file). One could\n> imagine a lot of possibilities like\n\nYes, I think something like that would be very nice. I am not a big\nfan of code generation, but if we had config queries like\n\"config_get_bool\", then I think it would be reasonably pleasant to take\na spec like:\n\n  Key: help.browser\n  Type: string\n  Description: Specify the browser for help...\n\nand turn it into:\n\n  const char *config_get_help_browser(void)\n  {\n          return config_get_string(\"help.browser\");\n  }\n\nSo technically code generation, but all the heavy lifting is done behind\nthe scenes. We're not saving lines in the result so much as avoiding\nrepeating ourselves (that is, the generated code is only mapping the\nconfig-type from the spec into a C type and function name that gives us\nextra compile-time safety).\n\nHowever, I skimmed through config.txt looking for a key to use in my\nexample above, and there are a surprising number of one-off semantics\n(e.g., things that are mostly bool, but can be \"auto\" or take some other\nspecial value). We may find that the \"Type\" field has a surprising\nnumber of variants that makes a technique like this annoying. But I'd\nreserve judgement until somebody actually tries encoding a significant\nchunk of the config keys and we see what it looks like.\n\n> Migrating the whole code to such system would take time, but creating\n> the system and applying it to a few examples might be feasible as a GSoC\n> project.\n\nAgreed, as long as we have enough examples to feel confident that the\ninfrastructure is sufficient.\n\n-Peff\n"},{"id":"236748","messageId":"xmqq61ngh4vr.fsf@gitster.dls.corp.google.com","threadId":"35993","inReplyTo":"20140314044326.GE31906@sigill.intra.peff.net","subject":"Re: RFC GSoC idea: new \"git config\" features","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-14T21:00:56Z","receivedAt":"2014-03-14T21:00:56Z","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 Sat, Mar 01, 2014 at 12:01:44PM +0100, Matthieu Moy wrote:\n>\n>> Jeff King <peff@peff.net> writes:\n>> \n>> > If we had the keys in-memory, we could reverse this: config code asks\n>> > for keys it cares about, and we can do an optimized lookup (binary\n>> > search, hash, etc).\n>> \n>> I'm actually dreaming of a system where a configuration variable could\n>> be \"declared\" in Git's source code, with associated type (list/single\n>> value, boolean/string/path/...), default value and documentation (and\n>> then Documentation/config.txt could become a generated file). One could\n>> imagine a lot of possibilities like\n>\n> Yes, I think something like that would be very nice. ...\n> ...\n>> Migrating the whole code to such system would take time, but creating\n>> the system and applying it to a few examples might be feasible as a GSoC\n>> project.\n>\n> Agreed, as long as we have enough examples to feel confident that the\n> infrastructure is sufficient.\n\nI agree that it would give us a lot of enhancement opportunities if\nwe had a central catalog of what the supported configuration\nvariables are and what semantics (e.g. type, multi-value-ness, etc.)\nthey have.\n\nOne thing we need to be careful about is that we still must support\nrandom configuration items that git-core does not care about at all\nbut scripts (and future versions of git-core) read off of, though.\n"}]}