{"thread":{"id":"36235","subject":"[RFC][GSoC] Calling for comments regarding rough draft of proposal","startedAt":"2014-03-19T19:52:28Z","lastAt":"2014-03-19T21:30:59Z","messageCount":2,"participants":["tanay abhra","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"237109","messageId":"CAEc54XAvdOFQLFE_odEKDGjrqNo+vtYfHafUvKS0OazUQ1r-Ag@mail.gmail.com","threadId":"36235","inReplyTo":null,"subject":"[RFC][GSoC] Calling for comments regarding rough draft of proposal","fromName":"tanay abhra","fromEmail":"tanayabh@gmail.com","sentAt":"2014-03-19T19:52:28Z","receivedAt":"2014-03-19T19:52:28Z","isPatch":false,"sender":{"key":"tanayabh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2097229?v=4"},"body":"Hi,\n      I have already done the microproject, which has been merged into\nmain last week. I have prepared a rough draft of my proposal for\nreview, read all the previous mailing list threads about it.I am\nreading the codebase little by little.\n\nPlease suggest improvements on the following topics,\n\n1.I have read one-third of config.c and will complete reading it by\ntomorrow.Is there any other  piece of code relevant to this proposal?\n\n2.Other things I should add to the proposal that I have left off?I am\ngetting confused what extra details I should add to the proposal. I\nwill add\nthe informal parts(my background, schedule for summer etc) of the\nproposal later.\n\n3.Did I understand anything wrong or if my approach to solving\nproblems is incorrect,if yes, I will redraft my proposal according to\nyour suggestions.\n\n------------------------------------------------------------------------------\n#GSoC Proposal : Git configuration API improvements\n-------------------------------------------------------------------------------\n\n#Proposed Improvements\n\n* Fix \"git config --unset\" to clean up detritus from sections that are\nleft empty.\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\nmemory rather than re-reading the configuration\n  files.\n\n* Add new API calls that allow the cache to be inquired easily and\nefficiently.  Rewrite other functions like\n `git_config_int()` to be cache-aware.\n\n* Rewrite callers to use the new API wherever possible.\n\n* How to invalidate the cache correctly in the case that the\nconfiguration is changed while `git` is executing.\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\nApproach:-\n\nWe parse the config file once, storing the raw values to records in\nmemory. After the whole config has been read, iterate through the records,\nfeeding the surviving values into the callback in the order they were\noriginally 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\nI propose two approaches for the format of the internal cache,\n\n1.Using a hashmap to map keys to their values.This would bring as an\n advantage, constant time lookups for the values.The implementation\n will be similar to \"dict\" data structure in python,\n\n for example, section.subsection --mapped-to--> multi_value_string\n\n This approach loses the relative order of different config keys.\n\n2.Another approach would be to actually represent the syntax tree of the\n  config file in memory. That would make lookups of individual keys more\n  expensive, but would enable other manipulation. E.g., if the syntax\n  tree included nodes for comments and other non-semantic constructs, then\n  we can use it for a complete rewrite.\n\n And \"git config\" becomes:\n\n  1. Read the tree.\n\n  2. Perform operations on the tree (add nodes, delete nodes, etc).\n\n  3. Write out the tree.\n\nand things like \"remove the section header when the last item in the\nsection is removed\" become trivial during step 2.\n\n\nI still prefer the hashmap way of implementing the cache,as empty\nsection headers  are not so problematic(no processing pitfalls) and\nare sometimes annotated with comments  which become redundant and\nconfusing if the section header is removed.As for the aesthetic\nproblem\nI propose a different solution for it below.\n\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 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\nAlso,setting a config value, appends the key-value pair at the end of\nfile without checking for empty main keys\neven if the main key(like [my]) is already present and empty.It works\nfine if the main key with an already present\nsub-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\nAlso, a possible detriment is presence of comments,\nFor Example:-\n    [my]\n            # This section is for my own private settings\n\nExpected output:\n\n  1. When we delete the last key in a section, we should be\n     able to delete the section header.\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.\n\nPossible approaches:-\n\n1.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  I am going through the code to find find other solution for this problem.\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\nThanks,\nTanay Abhra.\n"},{"id":"237121","messageId":"xmqqtxat27vw.fsf@gitster.dls.corp.google.com","threadId":"36235","inReplyTo":"CAEc54XAvdOFQLFE_odEKDGjrqNo+vtYfHafUvKS0OazUQ1r-Ag@mail.gmail.com","subject":"Re: [RFC][GSoC] Calling for comments regarding rough draft of proposal","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-19T21:30:59Z","receivedAt":"2014-03-19T21:30:59Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"tanay abhra <tanayabh@gmail.com> writes:\n\n> 2.Other things I should add to the proposal that I have left off?I am\n> getting confused what extra details I should add to the proposal. I\n> will add\n> the informal parts(my background, schedule for summer etc) of the\n> proposal later.\n\nI would not label the schedule and success criteria \"informal\";\nwithout them how would one judge if the proposal has merits?\n\nOther things like your background and previous achievements would\nbecome relevant, after it is decided that the proposed project has\nmerits, to see if you are a good fit to work on that project, so I\nagree with your message that it is sensible to defer them before the\nother parts of the proposal is ironed out.\n\n> #Proposed Improvements\n>\n> * Fix \"git config --unset\" to clean up detritus from sections that are\n> left empty.\n>\n> * Read the configuration from files once and cache the results in an\n> appropriate data structure in memory.\n>\n> * Change `git_config()` to iterate through the pre-read values in\n> memory rather than re-reading the configuration\n>   files.\n>\n> * Add new API calls that allow the cache to be inquired easily and\n> efficiently.  Rewrite other functions like\n>  `git_config_int()` to be cache-aware.\n\nI think we already had a discussion to point out git_config_int() is\nnot a good example for this bullet point (check the list archive).\nThe approach seciton seems to use a more sensible example (point 2).\n\n> * Rewrite callers to use the new API wherever possible.\n>\n> * How to invalidate the cache correctly in the case that the\n> configuration is changed while `git` is executing.\n\nI wouldn't list this as an item of \"list of improvements\".\n\nIt is merely a point you have to be careful about because you are\ndoing other \"improvements\" based on \"read all into memory first and\ndo not re-read files\" approach, no?  In the current code, when\nsomebody does git_config_set() and then later uses git_config() to\ngrab the value of the variable set with the first call, we will read\nthe value written to the file with the first call.  With the\nproposed change, if you parse from the file upfront, callers to\ngit_config_set() will need to somehow invalidate that stale copy in\nmemory, either updating only the changed part (harder) or just\ndiscarding the cache (easy).\n\n> ##Changing the git_config api to retrieve values from memory\n>\n> Approach:-\n>\n> We parse the config file once, storing the raw values to records in\n> memory. After the whole config has been read, iterate through the records,\n> feeding the surviving values into the callback in the order they were\n> originally read\n> (minus deletions).\n>\n> Path to follow for the api conversion,\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() which will inquire\n> cache for values efficiently.\n>\n> 3. Convert callbacks to query functions one by one.\n>\n> I propose two approaches for the format of the internal cache,\n>\n> 1.Using a hashmap to map keys to their values.This would bring as an\n>  advantage, constant time lookups for the values.The implementation\n>  will be similar to \"dict\" data structure in python,\n>\n>  for example, section.subsection --mapped-to--> multi_value_string\n\nI have no idea what you wanted to illustrate with that \"example\" at\nall.\n\n>  This approach loses the relative order of different config keys.\n\nAs long as it keeps the order of multi-value elements, it should\nnot be a problem.\n\n> 2.Another approach would be to actually represent the syntax tree of the\n>   config file in memory. That would make lookups of individual keys more\n>   expensive, but would enable other manipulation. E.g., if the syntax\n>   tree included nodes for comments and other non-semantic constructs, then\n>   we can use it for a complete rewrite.\n\n\"for a complete rewrite\" of what?\n\n>  And \"git config\" becomes:\n>\n>   1. Read the tree.\n>\n>   2. Perform operations on the tree (add nodes, delete nodes, etc).\n>\n>   3. Write out the tree.\n>\n> and things like \"remove the section header when the last item in the\n> section is removed\" become trivial during step 2.\n\nAre you saying you will try both approaches during the summer?\n\nYou should be able to look-up quickly *and* to preserve order at the\nsame time within one approach, by either annotating the tree with a\nhash, or the other way around to annotate the hash with each node\nremembering where in the original file it came from (which you will\nneed to keep in order to report errors anyway).\n\n> ----------------------------------------------------------------------\n> ##Tidy configuration files\n>\n> When a configuration file is repeatedly modified, often garbage is\n> left 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>\n> the bottom of the configuration file is left with the useless lines\n>\n>     [pull]\n>     [pull]\n>\n> Also,setting a config value, appends the key-value pair at the end of\n> file without checking for empty main keys\n> even if the main key(like [my]) is already present and empty.It works\n> fine if the main key with an already present\n> sub-key.\n>\n> for 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\n> gives\n>     [pull]\n>     [pull]\n>         rebase = true\n>         option = true\n>\n> Also, a possible detriment is presence of comments,\n> For Example:-\n>     [my]\n>             # This section is for my own private settings\n\nAnother example you need to consider is\n\n\t# Comment regarding the entire [my] section\n        [my]\n        \t# Comment on the item foo\n                foo = true\n\t\t# Comment on the item bar\n                bar = false\n\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, we should be\n>      able to delete the section header.\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.\n>\n> Possible approaches:-\n>\n> 1.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>   I am going through the code to find find other solution for this problem.\n>\n> links:-\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>\n> Thanks,\n> Tanay Abhra.\n"}]}