{"thread":{"id":"36497","subject":"[GSoC 2014] Accepted GSoC student introduction","startedAt":"2014-04-25T18:36:54Z","lastAt":"2014-04-25T18:36:54Z","messageCount":1,"participants":["tanay abhra"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"239688","messageId":"CAEc54XDU2OtSfnz1VsN1R+a4D0yAFm2EnDJUW1Wk8myT12jd5A@mail.gmail.com","threadId":"36497","inReplyTo":null,"subject":"[GSoC 2014] Accepted GSoC student introduction","fromName":"tanay abhra","fromEmail":"tanayabh@gmail.com","sentAt":"2014-04-25T18:36:54Z","receivedAt":"2014-04-25T18:36:54Z","isPatch":false,"sender":{"key":"tanayabh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2097229?v=4"},"body":"Hi,\n\nI am one of the three selected GSoC(Google summer of Code) students by\nGit this year.\nI am a third year computer science student from India. First, I want to thank\nyou all for guiding me through the patch review process and being\npatient with me. I learned a lot from you all\nand this helped me polishing my proposal, so I want to thank you all\nfor giving me this opportunity.\n\nMy mentors for the project are Matthieu Moy and Ramkumar Ramachandra. My\nproposal topic is \"Git configuration API improvements\" and I am\nappending the whole proposal below\nad verbatim for you all to review. I had posted it earlier but it may\nhave gotten lost in the noise.\nI am looking forward to having a great summer working on git.\n\n\n##GSoC Proposal : Git configuration API improvements\n\n\nAbstract:\n-----------\n\nThe current method for reading values from the config file (summarized\nfrom reading config.c, bulitin/config.c and\napi-config.txt)in the disk is parsing and reading the file each time a\nfunction calls for a config value.\ngit_config() feeds every variable and value to the callback function\nwhich decides what to do with them. Due to\nthis approach, it is not uncommon for the configuration to be parsed\nseveral times during the run of a Git\nprogram, with different callbacks picking out different variables\nuseful to themselves. I propose that instead of\nreading and parsing the config file multiple times from the disk\nduring a process run, we will construct a\nin-memory representation of the config file the first time a config\nvalue is queried for. Also, the callbacks will\nbe rewired to use the new API functions provided for efficient lookups\nfrom the internal cache. After this\nimplementation is validated, new features can be added such as\nunsetting values from config file.\n\n\n\n#Proposed Improvements\n\n* Fix \"git config --unset\" to clean up detritus from sections that are\nleft empty and some more aesthetic\nconcerns illustrated below .\n\n* Read the configuration from files once and cache the results in an\nappropriate data structure in memory.\n\n* Change git_config() to iterate through the pre-read values in memory\nrather than re-reading the configuration\n  files.\n\n* Add new API calls that allow the cache to be inquired easily and efficiently.\n\n* Rewrite callers to use the new API wherever possible.\n\n#Possible Problems with this approach,\n\n* How to invalidate the cache correctly in the case that the\nconfiguration is changed while `git` is executing.\n\n* What to do with the comments, which are generally found in config\nfiles, and can be places anywhere(above a\n  section header , below a section header etc.)?\n\n#Future Improvements\n\n* Allow configuration values to be unset via a config file\n\n--------------------------------------------------------------------------\n##Changing the git_config api to retrieve values from memory\n--------------------------------------------------------------------------\n\n#Current Implementation of the git-config subsystem:\n----------------------------------------------------\n\nConfig files are parsed linearly, and each variable found is passed to\na caller-provided callback function. The\ncallback function is responsible for any actions to be taken on the\nconfig option, and is free to ignore\nsome options. It is not uncommon for the configuration to be parsed\nseveral times during the run of a Git program,\nwith different callbacks picking out different variables useful to themselves.\n\nA config callback function takes three parameters:\n\n- the name of the parsed variable. This is in canonical \"flat\" form: the\n  section, subsection, and variable segments will be separated by dots,\n  and the section and variable segments will be all lowercase. E.g.,\n  `core.ignorecase`, `diff.SomeType.textconv`.\n\n- the value of the found variable, as a string. If the variable had no\n  value specified, the value will be NULL (typically this means it\n  should be interpreted as boolean true).\n\n- a void pointer passed in by the caller of the config API; this can\n  contain callback-specific data\n\nA config callback should return 0 for success, or -1 if the variable\ncould not be parsed properly.\n\n#Current Config Querying:\n-----------------------\n\nMost code in git,calls `git_config` to look up variables in all config\nfiles that Git knows about, using the normal precedence rules.\nTo do this, it calls with a callback function and void data pointer.\n\n`git_config` will read all config sources in order of increasing\npriority. The logic flow is attached as a image[1]\n. Thus a callback should typically overwrite previously-seenentries\nwith new ones (e.g., if both the user-wide\n`~/.gitconfig` and repo-specific `.git/config` contains `color.ui`,\nthe config machinery will first feed the\nuser-wide one to the callback, and then the repo-specific one; by\noverwriting, the higher-priority repo-specific\nvalue is left at the end).\n\nThe current precedence order rates the repo-config file the highest.\n\nThe `git_config_with_options` function lets the caller examine config\nwhile adjusting some of the default behavior\nof `git_config`. It takes two extra parameters:\n\n`filename`::\nIf this parameter is non-NULL, it specifies the name of a file to\nparse for configuration, rather than looking in the usual files. Regular\n`git_config` defaults to `NULL`.\n\n`respect_includes`::\nSpecify whether include directives should be followed in parsed files.\nRegular `git_config` defaults to `1`.\n\nThere is a special version of `git_config` called `git_config_early`.\nThis version takes an additional parameter to specify the repository\nconfig, instead of having it looked up via `git_path`. This is useful\nearly in a Git program before the repository has been found.\n\nI have made a logic diagram for better understanding the query config subsystem,\n[1]http://imgur.com/CToJXTr\n\n#Writing config files to disk:\n------------------------------\n\nGit provides two internal functions for setting and unsetting\nvariables in config files,\n\n*git_config_set(const char *key, const char *value)\n*git_config_set_in_file(const char *config_filename,const char *key,\nconst char *value)\n\nwho call git_config_set_multivar and git_config_set_multivar_in_file\nrespectively.\n\nIn the end git_config_set_multivar_in_file does all the work.\nFor setting and unsetting values it checks the argument value.\n\nIf value==NULL, unset in (remove from) config,\nif value_regex!=NULL, disregard key/value pairs where value does not match.\nif multi_replace==0, nothing, or only one matching key/value is replaced,\nelse all matching key/values (regardless how many) are removed,\nbefore the new pair is written.\n\nThis function does this:\n\n    - it locks the config file by creating \".git/config.lock\"\n\n    - it then parses the config using store_aux() as validator to find\n      the position on the key/value pair to replace. If it is to be unset,\n      it must be found exactly once.\n\n    - the config file is mmap()ed and the part before the match (if any) is\n     written to the lock file, then the changed part and the rest.\n\n   - the config file is removed and the lock file renamed to it.\n\n\n#New Approach and implementation details:\n----------------------------------------\n\nWe parse the config file once, storing the raw values to records in\nmemory. After the whole config has been read,\niterate through the records, feeding the surviving values into the\ncallback in the order they were originally read\n(minus deletions).\n\nPath to follow for the api conversion,\n\n1. Convert the parser to read into an in-memory representation, but\n   leave git_config() as a wrapper which iterates over it.\n\n2. Add query functions like config_string_get() which will inquire\ncache for values efficiently.\n\n3. Convert callbacks to query functions one by one.\n\n#Choosing a data structure for internal cache:\n----------------------------------------------\n\nCurrently, the callbacks receives key,value pairs after parsing in a\nflat form like,\"diff.sometype.textconv\" and\nvalue \"true\" .These key-value pairs easily translate to a hashmap or a\ndictionary structure.\nThis data-structure  reflects the configuration variables themselves,\nnot the way they appear in the config\nfile. A map (hashtable) associating to each config variable the\ncorresponding value (which maybe a scalar\nvalue,boolean or a list, depending on the variable) would be most\nappropriate in my opinion.They will be saved as\n\"string\":\"string list\" pairs as there are already many helper\nfunctions to help converting the string values into\ntheir appropriate data type.\n\nAdvantages of using a Hashmap:-\n------------------------------\n\n>>Constant time lookups(O(1)) and deletions.After the initial parsing ,the system would use the hashmaps for\n  lookups.\n>>Read-only accesses happen far more often than updates, so the data structure is optimised for lookups.\n>>Since the key,value pairs are string like for example, section.subsection.variable = value ,they are easily\n  hashable and are appropriate for this type of data structure.\n>>As for the implementation details, there is already a generic hashmap implementation built-in .(described in\n  api-hashmap.txt).I could roll out a new one if you want.\n>>Unsetting previously set value would become as easy as deleting the key-value pair ,or setting a invalid flag\n  for that pair during file parsing stage.\n\n#Complications in using a hashmap:-\n-----------------------------------\n\n>>To report errors in the config files, a node remembering where the key-value pair came from and from which file,\n  will have to be maintained. It would also be useful in writing\nkey-value pairs back to the file in order of\n  reading.\n>>In the current code, when somebody does git_config_set() and then later uses git_config() to grab the value of\n  the variable set with the first call, we will read the value written\nto the file with the first call. With the\n  proposed change, if we parse from the file upfront, callers to\ngit_config_set() will need to somehow invalidate\n  that stale copy in memory.\n\n  We could do two things about it,\n  1>Easy approach- Discard and rebuilt the cache every time git_config\nqueries for a value. This is currently, what\n  we are doing, reading and parsing the file every time, a value is\ncalled for using git_config.\n  2>Harder approach- Update the changed part. This seems doable, but I\nwill have to experiment with it to see if\n  it works or not as I am not too much familiar with the code base.\nBut take this  initial proposal, git_config_set\n  creates a lock file , updates it, deletes the original config file\nand renames the lockfile to a new config file.\n  We could take this approach and apply it to the in memory-cache and\nlock it until git_config_set works on it.\n\n >>Precedence order of variables read from different sources.\n\n  Currently the config is read from multiple sources (repo config file\n`.git/config`,user config file\n  `$HOME/.gitconfig` and system wide config file `/etc/gitconfig`)\nwith the repo config file having the highest\n  preceding order. It is read last in the current parsing order. In my\nimplementation the value of variables will\n  be implemented as list of strings. So it will be a case of \"last one\nwins\", where the value with the highest\n  precedence would be inserted last in the list. So a caller asking\nfor a value would just read the last value in\n  the list without worrying about no allocation or  ownership complexity.\n\n#Changing the parsing code\n--------------------------\n\nCurrently git_parse_source parses the file and feeds each value to the\ncallback function. git_config uses it. To be\nbackward compatible, I would change the git_parse_source to have a\nswitch for the default parsing behaviour and\nupdated parsing parsing behaviour.\n\nIn the updated one, the file will be read only once to construct the\nhash map and after that it would iterate over\nthe values querying the cache each time. The callback would be fed\nthese read values only.\n\n#Add new API Calls\n------------------\n\nA new set of helpers for retrieving values in a non-callback way will be added.\nPossible implementation of the API,\n\n>>extern int get_config_string_multi(const char * variable, int * num_values, const char **values)\n\nArgument list: key: used for retrieving the value list for the respective key\n               num_values: returns no of values in the list\n               vales: list containing the values as string\n\n>> A singleton wrapper would look like char *git_get_config_string(const char *name)\n   return the last value for the key \"name\"(thus implementing the last\none wins semantic)\n   Possible implementation (copied directly from the mailing list discussion )\n\n   const char *git_get_config_string(const char *name)\n        {\n        const char **values, *result;\n                int num_values;\n\n            if (git_get_config_string_multi(\"sample.str\", &num_values, &values))\n                return NULL;\n                result = num_values ? values[num_values - 1] : NULL;\n                free(values);\n        return result;\n    }\nAs there is already a set of helpers for validating the values and\nconverting them to appropriate data types\nin config.c there won't be any need for specialized functions for each\ndata type.\n\n#Rewrite callers:\n----------------\n\nAfter implementing the above ,this would be just busywork and pretty\nmuch doable.The callbacks would have to be\nreworked to use the new api functions.\n\n----------------------------------------------------------------------\n##Tidy configuration files\n----------------------------------------------------------------------\n\n#First case:\n\nWhen a configuration file is repeatedly modified, often garbage is\nleft behind.  For example, after\n\n    git config pull.rebase true\n    git config --unset pull.rebase\n    git config pull.rebase true\n    git config --unset pull.rebase\n\nthe bottom of the configuration file is left with the useless lines\n\n    [pull]\n    [pull]\n\n#Case 2\n\nAlso,setting a config value, appends the key-value pair at the end of\nfile without checking for matching empty main\nsection headers. It works fine if the main section header has an\nalready present sub-key.\n\nfor example:-\n    git config pull.rebase true\n    git config --unset pull.rebase\n    git config pull.rebase true\n    git config pull.option true\ngives\n    [pull]\n    [pull]\n        rebase = true\n        option = true\n#Case 3\n\nAlso, a possible detriment is presence of comments,\nFor Example:-\n    [my]\n            # This section is for my own private settings\nAnother example you need to consider is\n\n    # Comment regarding the entire [my] section\n        [my]\n            # Comment on the item foo\n                foo = true\n        # Comment on the item bar\n                bar = false\n\nWhat should \"git config --unset my.foo && git config -unset my.bar\" do?\nWhat if these are done in a different order?\n\n\n#Expected output:\n\n  1. When we delete the last key in a section, it automatically delete\nthe section header.(This is debatable)\n\n  2. When we add a key into a section, we should be able to\n     reuse the same section header, even if that section did\n     not have any keys in it already(empty section header).\n\n#Suggested Approaches :-\n--------------------------------------\n\nCase first\n  Leave the empty section header as it was and when a new value is set,\n  reuse the header instead of appending at the end of the config file.\n  This can be implemented by changing git_config_set a little, even\nwithout using the internal cache.\n  If we use the internal cache,to rewrite the file after setting or\nunsetting the value, this problem will vanish\n  automatically.\n\nCase second\n  This is also a git_config_set problem and can be solved by either\nusing the cache to rewrite the file every time\n  setting or unsetting the value, or reuse the empty section header\nwhen adding a new value pair into it.\n\nCase three\n  Its a controversial issue , in which the comments added by the user\nmight signify anything(commenting on section\n  header or a particular variable or the above header).I think the\ncomments should be left as it is , as some users\n  leave empty section headers and comments for future use. So removing\nthem may prove detrimental.But , other\n  suggestions are welcome for this problem .The mailing list itself\nwas conflicted about this option.\n\nlinks:-\n[1]http://thread.gmane.org/gmane.comp.version-control.git/219505\n[2]http://thread.gmane.org/gmane.comp.version-control.git/208113\n\n------------------------------------------------------------------------------------------------------------------\n\nCheers,\nTanay Abhra.\n"}]}