{"thread":{"id":"37011","subject":"[PATCH/RFC] config: Add config sets for config parsing & retrieval","startedAt":"2014-06-27T18:04:45Z","lastAt":"2014-06-27T21:07:49Z","messageCount":2,"participants":["Tanay Abhra","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"245085","messageId":"1403892285-9878-1-git-send-email-tanayabh@gmail.com","threadId":"37011","inReplyTo":null,"subject":"[PATCH/RFC] config: Add config sets for config parsing & retrieval","fromName":"Tanay Abhra","fromEmail":"tanayabh@gmail.com","sentAt":"2014-06-27T18:04:45Z","receivedAt":"2014-06-27T18:04:45Z","isPatch":true,"sender":{"key":"tanayabh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2097229?v=4"},"body":"Add a hash table to cache all key-value pairs read from config files\n(repo specific .git/config, user wide ~/.gitconfig and the global\n/etc/gitconfig). Add two external functions `git_config_get_string` and\n`git_config_get_string_multi` for querying in a non-callback manner from the\nhash table.\n\nAdd a way to construct custom config sets which contain multiple config\nfiles cached as hashmaps for value retrieval.\n---\n\n*NOTE*\n\nPlease ignore the commit message and the documentation. This is a WIP. This patch\nimplements the user facing API which was discussed thoroughly in discussion thread[1].\nA more polished patch will land next week, this patch is just to give a feel for what\nI inferred from the discussion.\n\nAPI functions:\n\n1. int git_config_get_value(char *k, char **v): The most generic function, traverses\nthe usual config functions for values. (internally uses the the_config_set struct)\n\n2. int git_configset_get_value_from(const char *key, const char **value,\n\t\t\t \tconst char *filename, struct config_set *cs)\nFor usage of this function, see the example given below.\n\nMost of the extra API functions will be constructed on the top of this patch.\nWorkflow for custom config_set works like this,\n\n\tchar *value;\n\tstruct config_set gm_config = {NULL, 0 ,0};\n\tgit_configset_add_file(\".gitmodule\", &gm_config);\n\t/* Add other files similarly */\n\tgit_configset_get_value_from(key, &value, \".gitmodule\", &gm_config);\n\t/* Work on config values free the set when done */\n\tgit_configset_clear(&gm_config);\n\nI have tested the workflow and it works fine. :)\n\nNow, about helper functions, if I add new helper functions for get_ family like\ngit_config_get_string, git_config_get_int and similar, won't the helper functions\nin config.c would become redundant. Is it really necessary to write new\nhelper functions for the non-callback code, instead of reusing the older ones?\n\n[1]: http://marc.info/?t=140351850200001&r=1&w=2\n\nCheers,\nTanay Abhra.\n\n Documentation/technical/api-config.txt |  26 +++++\n cache.h                                |   2 +\n config.c                               | 182 +++++++++++++++++++++++++++++++++\n 3 files changed, 210 insertions(+)\n\ndiff --git a/Documentation/technical/api-config.txt b/Documentation/technical/api-config.txt\nindex 230b3a0..13f3f40 100644\n--- a/Documentation/technical/api-config.txt\n+++ b/Documentation/technical/api-config.txt\n@@ -77,6 +77,32 @@ To read a specific file in git-config format, use\n `git_config_from_file`. This takes the same callback and data parameters\n as `git_config`.\n \n+Querying For Specific Variables\n+-------------------------------\n+\n+For programs wanting to query for specific variables in a non-callback\n+manner, the config API provides two functions `git_config_get_string`\n+and `git_config_get_string_multi`.They both read values from an internal\n+cache generated previously from reading the config files.\n+\n+`git_config_get_string` takes two parameters,\n+\n+- a key string in canonical flat form for which the corresponding value\n+  with the highest priority (i.e. value in the repo config will be\n+  preferred over value in user wide config for the same variable) will\n+  be retrieved.\n+\n+- a pointer to a string which will point to the retrieved value.\n+\n+`git_config_get_string` returns 0 for success, or -1 for no value found.\n+\n+`git_config_get_string_multi` returns a `string_list` containing all the\n+values for the key passed as parameter, sorted in order of increasing\n+priority (Note: NULL values are flagged as 1, check `util` for each\n+'string_list_item` for flag value).\n+\n+See test-config.c for usage examples.\n+\n Value Parsing Helpers\n ---------------------\n \ndiff --git a/cache.h b/cache.h\nindex cbe1935..fec0a63 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1294,6 +1294,8 @@ extern int check_repository_format_version(const char *var, const char *value, v\n extern int git_env_bool(const char *, int);\n extern int git_config_system(void);\n extern int config_error_nonbool(const char *);\n+extern int git_config_get_string(const char *, const char **);\n+extern const struct string_list *git_config_get_string_multi(const char *);\n #if defined(__GNUC__)\n #define config_error_nonbool(s) (config_error_nonbool(s), const_error())\n #endif\ndiff --git a/config.c b/config.c\nindex a1aef1c..332d92e 100644\n--- a/config.c\n+++ b/config.c\n@@ -9,6 +9,8 @@\n #include \"exec_cmd.h\"\n #include \"strbuf.h\"\n #include \"quote.h\"\n+#include \"hashmap.h\"\n+#include \"string-list.h\"\n \n struct config_source {\n \tstruct config_source *prev;\n@@ -37,6 +39,180 @@ static struct config_source *cf;\n \n static int zlib_compression_seen;\n \n+struct config_hash_entry {\n+\tstruct hashmap_entry ent;\n+\tchar *key;\n+\tstruct string_list value_list;\n+};\n+\n+struct config_set {\n+\tstruct config_set_item *item;\n+\tunsigned int nr, alloc;\n+};\n+\n+struct config_set_item {\n+\tstruct hashmap config_hash;\n+\tchar *filename;\n+\tint hashmap_initialized;\n+};\n+\n+static struct config_set_cb {\n+\tstruct config_set *cs;\n+\tstruct config_set_item *ct;\n+};\n+\n+static struct config_set the_config_set;\n+\n+static int config_hash_add_value(const char *, const char *,\n+\t\t\t\t  struct config_set_item *, struct config_set *);\n+\n+static int config_hash_entry_cmp(const struct config_hash_entry *e1,\n+\t\t\t\t const struct config_hash_entry *e2, const void *unused)\n+{\n+\treturn strcmp(e1->key, e2->key);\n+}\n+\n+static void config_hash_init(struct hashmap *config_hash)\n+{\n+\thashmap_init(config_hash, (hashmap_cmp_fn)config_hash_entry_cmp, 0);\n+}\n+\n+static int config_hash_callback(const char *key, const char *value, void *cb)\n+{\n+\tstruct config_set_cb *c = cb;\n+\tconfig_hash_add_value(key, value, c->ct, c->cs);\n+\treturn 0;\n+}\n+\n+static struct hashmap *get_config_hash(const char *filename, struct config_set *cs)\n+{\n+\tint i;\n+\tstruct config_set_item *ct;\n+\tstruct config_set_cb cb;\n+\tfor(i = 0; i < cs->nr; i++) {\n+\t\tif (!strcmp(cs->item[i].filename, filename))\n+\t\t\treturn &cs->item[i].config_hash;\n+\t}\n+\n+\tALLOC_GROW(cs->item, cs->nr + 1, cs->alloc);\n+\tct = &cs->item[cs->nr++];\n+\tct->filename = xstrdup(filename);\n+\tconfig_hash_init(&ct->config_hash);\n+\tct->hashmap_initialized = 1;\n+\tcb.cs = cs;\n+\tcb.ct = ct;\n+\t/*\n+\t * When filename is \"default\", hashmap is constructed from the usual set of\n+\t * config files (i.e repo specific .git/config, user wide ~/.gitconfig and the\n+\t * global /etc/gitconfig)\n+\t */\n+\tif (!strcmp(filename, \"default\"))\n+\t\tgit_config(config_hash_callback, &cb);\n+\telse\n+\t\tgit_config_from_file(config_hash_callback, filename, &cb);\n+\n+\treturn &ct->config_hash;\n+}\n+\n+static struct config_hash_entry *config_hash_find_entry(const char *key, const char* filename,\n+\t\t\t\t\t\t\tstruct config_set *cs)\n+{\n+\tstruct hashmap *config_hash;\n+\tstruct config_hash_entry k;\n+\tstruct config_hash_entry *found_entry;\n+\tchar *normalized_key;\n+\tint ret;\n+\tconfig_hash = get_config_hash(filename, cs);\n+\tret = git_config_parse_key(key, &normalized_key, NULL);\n+\n+\tif (ret)\n+\t\treturn NULL;\n+\n+\thashmap_entry_init(&k, strhash(normalized_key));\n+\tk.key = normalized_key;\n+\tfound_entry = hashmap_get(config_hash, &k, NULL);\n+\tfree(normalized_key);\n+\treturn found_entry;\n+}\n+\n+static struct string_list *config_hash_get_value(const char *key, const char* filename, \n+\t\t\t\t\t\t struct config_set *cs)\n+{\n+\tstruct config_hash_entry *e = config_hash_find_entry(key, filename, cs);\n+\treturn e ? &e->value_list : NULL;\n+}\n+\n+static int config_hash_add_value(const char *key, const char *value,\n+\t\t\t\t struct config_set_item *ct, struct config_set *cs)\n+{\n+\tstruct hashmap *config_hash = &ct->config_hash;\n+\tstruct config_hash_entry *e;\n+\te = config_hash_find_entry(key, ct->filename, cs);\n+\n+\tif (!e) {\n+\t\te = xmalloc(sizeof(*e));\n+\t\thashmap_entry_init(e, strhash(key));\n+\t\te->key = xstrdup(key);\n+\t\tstring_list_init_dup(&e->value_list);\n+\t\thashmap_add(config_hash, e);\n+\t}\n+\tstring_list_append_nodup(&e->value_list, value ? xstrdup(value) : NULL);\n+\n+\treturn 0;\n+}\n+\n+int git_configset_get_value_from(const char *key, const char **value,\n+\t\t\t\t const char *filename, struct config_set *cs)\n+{\n+\tstruct string_list *values;\n+\tvalues = config_hash_get_value(key, filename, cs);\n+\tif (!values)\n+\t\treturn -1;\n+\tassert(values->nr > 0);\n+\t*value = values->items[values->nr - 1].string;\n+\treturn 0;\n+}\n+\n+int git_config_get_value(const char *key, const char **value)\n+{\n+\treturn git_configset_get_value_from(key, value, \"default\", &the_config_set);\n+}\n+\n+const struct string_list *git_config_get_value_multi(const char *key)\n+{\n+\treturn config_hash_get_value(key, \"default\", &the_config_set);\n+}\n+\n+const struct string_list *git_configset_get_value_multi_from(const char *key, const char* filename,\n+\t\t\t\t\t\t\tstruct config_set *cs)\n+{\n+\treturn config_hash_get_value(key, filename, cs);\n+}\n+\n+void git_configset_add_file(char *filename, struct config_set *cs)\n+{\n+\tget_config_hash(filename, cs);\n+}\n+\n+void git_configset_clear(struct config_set *cs)\n+{\n+\tint i;\n+\tstruct hashmap *config_hash;\n+\tstruct config_hash_entry *entry;\n+\tstruct hashmap_iter iter;\n+\tfor (i = 0; i < cs->nr; i++) {\n+\t\tconfig_hash = &cs->item[i].config_hash;\n+\t\tfree(cs->item[i].filename);\n+\t\tcs->item[i].hashmap_initialized = 0;\n+\t\thashmap_iter_init(config_hash, &iter);\n+\t\twhile ((entry = hashmap_iter_next(&iter))) {\n+\t\t\tfree(entry->key);\n+\t\t\tstring_list_clear(&entry->value_list, 0);\n+\t\t}\n+\t\thashmap_free(config_hash, 1);\n+\t}\n+}\n+\n static int config_file_fgetc(struct config_source *conf)\n {\n \treturn fgetc(conf->u.file);\n@@ -1708,6 +1884,12 @@ int git_config_set_multivar_in_file(const char *config_filename,\n \tlock = NULL;\n \tret = 0;\n \n+\t/*\n+\t * content of config file has changed, so invalidate the\n+\t * config cache used by non-callback based query functions.\n+\t */\n+\tgit_configset_clear(&the_config_set);\n+\n out_free:\n \tif (lock)\n \t\trollback_lock_file(lock);\n-- \n1.9.0.GIT\n"},{"id":"245099","messageId":"xmqq8uoi12oq.fsf@gitster.dls.corp.google.com","threadId":"37011","inReplyTo":"1403892285-9878-1-git-send-email-tanayabh@gmail.com","subject":"Re: [PATCH/RFC] config: Add config sets for config parsing & retrieval","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-06-27T21:07:49Z","receivedAt":"2014-06-27T21:07:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tanay Abhra <tanayabh@gmail.com> writes:\n\n> API functions:\n>\n> 1. int git_config_get_value(char *k, char **v): The most generic function, traverses\n> the usual config functions for values. (internally uses the the_config_set struct)\n\nThis may also want to be accompanied by a set of more type-specific\nAPI functions, e.g.\n\n     long git_config_get_long(char *key);\n     unsigned long git_config_get_ulong(char *key);\n     char *git_config_get_string(char *key);\n     \nbut we would need to know how the error reporting should look like\nfor them.\n\nYou would also want to add a bit more before we can call the API to\nbe complete.  Here are two that comes to my mind:\n\n - Multi-valued variables\n\n   int git_config_get_multi(char *k, char ***v, int *cnt);\n\n   which may allocate an array of \"char *\" and return its address in\n   *v, while returning its length in *cnt.  Alternatively, we can\n   report the length of the array as the return value, signalling an\n   error with a negative return value, without having the *cnt\n   argument.\n\n - Iterating over keys\n\n   Existing git_config() function signature can be kept for\n   iterating over keys, I would think, even if we start caching the\n   previous read of the config files from the filesystem.\n\n\n> 2. int git_configset_get_value_from(const char *key, const char **value,\n> \t\t\t \tconst char *filename, struct config_set *cs)\n\nHaving to give \"filename\" each and every time I want to know about a\nsingle variable does not make any sense to me.  I would have\nexpected the API to be more like this:\n\n\tstruct config_set *cs = git_configset_init();\n        char *value;\n\n        git_configset_from_file(cs, \".gitmodule\");\n        git_configset_get_value(cs, \"submodule.kernel.update\", &value);\n\nThere will need to be a set of \"config-set\" variant of functions\nthat parallel the \"work on the_config_set\" variant above, e.g.\n\n\tlong git_configset_get_long(struct config_set *, char *);\n\tint git_configset_get_multi(struct config_set *, char *, char ***, int *);\n\tint git_configset_config(struct config_set *, config_fn_t, void *);\n\nThe last one is the parallel to the traditional git_config() but it\nwalks over a specific config_set.\n\nAnd once this is done, you wouldn't have any separate implementation of\ngit_config_get_value(). it will be a series of macros:\n\n\t#define git_config_get_value(k, v) \\\n        \tgit_configset_get_value(&the_config_set, (k), (v))\n\njust like all the *_cache() API that used to be functions are now\nimplemented as macros built around corresponding *_index() API\nfunctions (defined in cache.h).\n"}]}