{"thread":{"id":"37130","subject":"[PATCH v9 0/3] git config cache & special querying api utilizing the cache","startedAt":"2014-07-15T14:29:29Z","lastAt":"2014-07-17T17:12:26Z","messageCount":26,"participants":["Tanay Abhra","Junio C Hamano","Matthieu Moy"],"isPatch":true,"patchVersion":9,"patchTotal":3},"messages":[{"id":"246056","messageId":"1405434571-25459-1-git-send-email-tanayabh@gmail.com","threadId":"37130","inReplyTo":null,"subject":"[PATCH v9 0/3] git config cache & special querying api utilizing the cache","fromName":"Tanay Abhra","fromEmail":"tanayabh@gmail.com","sentAt":"2014-07-15T14:29:29Z","receivedAt":"2014-07-15T14:29:29Z","isPatch":true,"sender":{"key":"tanayabh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2097229?v=4"},"body":"Hi,\n\n[PATCH v9]: Applied most of the review comments mentioned in [11]. Mostly asthetic changes.\n\ttest-config now clears the config_set before exiting. Most of the tests now use the\n\tcheck-config function. check_config_init() now handles return values correctly.\n\tDiff between v8 and v9 is at the bottom. Thanks to Junio and Matthieu for the review.\n\n[PATCH V8]: Moved the contents of config-set.c to config.c for future convenience. Reverted\n\ttest 'find value with misspelled key' to the one in v5. See [10] for the discussion.\n\n[PATCH V7]: Style nits and a broken && chain corrected in `t/t1308-config-set.sh`. See\n\t[9] for the nits.\n\n[PATCH V6]: Style nits and mistakes corrected. Diff between v6 and v5[8] is at the bottom.\n\t\tThanks to Matthieu, Ramsay and Ram for their suggestions.\n\n[PATCH V5]: `config_set` now uses a single hashmap. Corrected style nits raised in\n\t\t\tthe thread[7]. Thanks to Junio and Matthieu for their suggestions.\n\n[PATCH v4]: Introduced `config_set` construct which points to a ordered set of\n\tconfig-files cached as hashmaps. Added relevant API functions. For more\n\tdetails see the documentation. Rewrote the git_config_get* family to use\n\t`config_set` internally. Added tests for both config_set API and git_config_get\n\tfamily. Added type specific API functions which parses the found value and\n\tconverts it into a specific type.\n\tMost of the changes implemented are the result of discussion in [6].\n\tThanks to Eric, Ramsay, Junio, Matthieu & Karsten for their suggestions\n\tand review.\n\n[PATCH v3]: Added flag for NULL values that were causing segfaults in some cases.\n\tAdded test-config for usage examples.\n\tMinor changes and corrections. Refer to discussion thread[5] for more details.\n\tThanks to Matthieu, Jeff and Junio for their valuable suggestions.\n\n[PATCH v2]:Changed the string_list to a struct instead of pointer to a struct.\n\tAdded string-list initilization functions.\n\tMinor mistakes corrected acoording to review comments[4]. Thanks to\n\tEric and Matthieu for their review.\n\n[PATCH V1]:Most of the invaluable suggestions by Eric Sunshine, Torsten Bogershausen and\n\tJeff King has been implemented[1]. Complete rewrite of config_cache*() family\n\tusing git_config() as hook as suggested by Jeff. Thanks for the review.\n\n[RFC V2]: Improved according to the suggestions by Eric Sunshine and Torsten Bogershausen.\n\tAdded cache invalidation when config file is changed.[2]\n\tI am using git_config_set_multivar_in_file() as an update hook.\n\nThis is patch is for this year's GSoC. My project is\n\"Git Config API improvements\". The link of my proposal is appended below [3].\n\nThe aim of this patch series is to generate a cache for querying values from\nthe config files in a non-callback manner as the current method reads and\nparses the config files every time a value is queried for.\n\nThe cache is generated from hooking the update_cache function to the current\nparsing and callback mechanism in config.c. It is implemented as an hashmap\nusing the hashmap-api with variables and its corresponding values list as\nits members. The values in the list are sorted in order of increasing priority.\nThe cache is initialised the first time when any of the new query functions is\ncalled. It is invalidated by using git_config_set_multivar_in_file() as an\nupdate hook.\n\nWe add two new functions to the config-api, git_config_get_string() and\ngit_config_get_string_multi() for querying in a non callback manner from\nthe cache.\n\n[1] http://marc.info/?t=140172066200006&r=1&w=2\n[2] http://git.661346.n2.nabble.com/RFC-PATCH-0-2-Git-config-cache-amp-special-querying-api-utilizing-the-cache-td7611691.html\n[3] https://drive.google.com/file/d/0B4suZ-aHqDcnSUZJRXVTTnZUN1E/edit?usp=sharing\n[4] http://thread.gmane.org/gmane.comp.version-control.git/251073/focus=251369\n[5] http://thread.gmane.org/gmane.comp.version-control.git/251704/\n[6] http://thread.gmane.org/gmane.comp.version-control.git/252329/\n[7] http://marc.info/?t=140428115200001&r=1&w=2\n[8] http://article.gmane.org/gmane.comp.version-control.git/252942/\n[9] http://thread.gmane.org/gmane.comp.version-control.git/252959/\n[10] http://article.gmane.org/gmane.comp.version-control.git/253113\n[11] http://thread.gmane.org/gmane.comp.version-control.git/253248\n\n\nTanay Abhra (2):\n  config set\n  test-config\n\n .gitignore                             |   1 +\n Documentation/technical/api-config.txt | 135 ++++++++++++++++\n Makefile                               |   1 +\n cache.h                                |  31 ++++\n config.c                               | 276 +++++++++++++++++++++++++++++++++\n t/t1308-config-set.sh                  | 212 +++++++++++++++++++++++++\n test-config.c                          | 140 +++++++++++++++++\n 7 files changed, 796 insertions(+)\n create mode 100755 t/t1308-config-set.sh\n create mode 100644 test-config.c\n\n-- \n1.9.0.GIT\n\n-- 8< --\ndiff --git a/Documentation/technical/api-config.txt b/Documentation/technical/api-config.txt\nindex bb43830..48255a2 100644\n--- a/Documentation/technical/api-config.txt\n+++ b/Documentation/technical/api-config.txt\n@@ -241,7 +241,8 @@ Configset API provides functions for the above mentioned work flow, including:\n `int git_configset_add_file(struct config_set *cs, const char *filename)`::\n \n \tParses the file and adds the variable-value pairs to the `config_set`,\n-\tdies if there is an error in parsing the file.\n+\tdies if there is an error in parsing the file. Returns 0 on success, or\n+\t-1 if the file doesnot exist or cannot be read.\n \n `int git_configset_get_value(struct config_set *cs, const char *key, const char **value)`::\n \ndiff --git a/config.c b/config.c\nindex aa58275..89e2d67 100644\n--- a/config.c\n+++ b/config.c\n@@ -35,7 +35,7 @@ struct config_source {\n \tlong (*do_ftell)(struct config_source *c);\n };\n \n-struct config_hash_entry {\n+struct config_set_element {\n \tstruct hashmap_entry ent;\n \tchar *key;\n \tstruct string_list value_list;\n@@ -1227,48 +1227,10 @@ int git_config(config_fn_t fn, void *data)\n \treturn git_config_with_options(fn, data, NULL, 1);\n }\n \n-static int config_hash_add_value(const char *, const char *, struct hashmap *);\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 configset_init(struct config_set *cs)\n-{\n-\tif (!cs->hash_initialized) {\n-\t\thashmap_init(&cs->config_hash, (hashmap_cmp_fn)config_hash_entry_cmp, 0);\n-\t\tcs->hash_initialized = 1;\n-\t}\n-}\n-\n-static int config_hash_callback(const char *key, const char *value, void *cb)\n-{\n-\tstruct config_set *cs = cb;\n-\tconfig_hash_add_value(key, value, &cs->config_hash);\n-\treturn 0;\n-}\n-\n-int git_configset_add_file(struct config_set *cs, const char *filename)\n-{\n-\tint ret = 0;\n-\tconfigset_init(cs);\n-\tret = git_config_from_file(config_hash_callback, filename, cs);\n-\tif (!ret)\n-\t\treturn 0;\n-\telse {\n-\t\thashmap_free(&cs->config_hash, 1);\n-\t\tcs->hash_initialized = 0;\n-\t\treturn -1;\n-\t}\n-}\n-\n-static struct config_hash_entry *config_hash_find_entry(const char *key,\n-\t\t\t\t\t\t\tstruct hashmap *config_hash)\n+static struct config_set_element *configset_find_element(struct config_set *cs, const char *key)\n {\n-\tstruct config_hash_entry k;\n-\tstruct config_hash_entry *found_entry;\n+\tstruct config_set_element k;\n+\tstruct config_set_element *found_entry;\n \tchar *normalized_key;\n \tint ret;\n \t/*\n@@ -1282,21 +1244,21 @@ static struct config_hash_entry *config_hash_find_entry(const char *key,\n \n \thashmap_entry_init(&k, strhash(normalized_key));\n \tk.key = normalized_key;\n-\tfound_entry = hashmap_get(config_hash, &k, NULL);\n+\tfound_entry = hashmap_get(&cs->config_hash, &k, NULL);\n \tfree(normalized_key);\n \treturn found_entry;\n }\n \n-static struct string_list *configset_get_list(const char *key, struct config_set *cs)\n+const struct string_list *git_configset_get_value_multi(struct config_set *cs, const char *key)\n {\n-\tstruct config_hash_entry *e = config_hash_find_entry(key, &cs->config_hash);\n+\tstruct config_set_element *e = configset_find_element(cs, key);\n \treturn e ? &e->value_list : NULL;\n }\n \n-static int config_hash_add_value(const char *key, const char *value, struct hashmap *config_hash)\n+static int configset_add_value(struct config_set *cs, const char *key, const char *value)\n {\n-\tstruct config_hash_entry *e;\n-\te = config_hash_find_entry(key, config_hash);\n+\tstruct config_set_element *e;\n+\te = configset_find_element(cs, key);\n \t/*\n \t * Since the keys are being fed by git_config*() callback mechanism, they\n \t * are already normalized. So simply add them without any further munging.\n@@ -1307,43 +1269,28 @@ static int config_hash_add_value(const char *key, const char *value, struct hash\n \t\te->key = xstrdup(key);\n \t\tmemset(&e->value_list, 0, sizeof(e->value_list));\n \t\te->value_list.strdup_strings = 1;\n-\t\thashmap_add(config_hash, e);\n+\t\thashmap_add(&cs->config_hash, e);\n \t}\n \tstring_list_append_nodup(&e->value_list, value ? xstrdup(value) : NULL);\n \n \treturn 0;\n }\n \n-void git_configset_init(struct config_set *cs)\n-{\n-\tcs->hash_initialized = 0;\n-}\n-\n-int git_configset_get_value(struct config_set *cs, const char *key, const char **value)\n+static int config_set_element_cmp(const struct config_set_element *e1,\n+\t\t\t\t const struct config_set_element *e2, const void *unused)\n {\n-\tstruct string_list *values = NULL;\n-\t/*\n-\t * Follows \"last one wins\" semantic, i.e., if there are multiple matches for the\n-\t * queried key in the files of the configset, the value returned will be the last\n-\t * value in the value list for that key.\n-\t */\n-\tvalues = configset_get_list(key, cs);\n-\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+\treturn strcmp(e1->key, e2->key);\n }\n \n-const struct string_list *git_configset_get_value_multi(struct config_set *cs, const char *key)\n+void git_configset_init(struct config_set *cs)\n {\n-\treturn configset_get_list(key, cs);\n+\thashmap_init(&cs->config_hash, (hashmap_cmp_fn)config_set_element_cmp, 0);\n+\tcs->hash_initialized = 1;\n }\n \n void git_configset_clear(struct config_set *cs)\n {\n-\tstruct config_hash_entry *entry;\n+\tstruct config_set_element *entry;\n \tstruct hashmap_iter iter;\n \tif (!cs->hash_initialized)\n \t\treturn;\n@@ -1354,6 +1301,43 @@ void git_configset_clear(struct config_set *cs)\n \t\tstring_list_clear(&entry->value_list, 0);\n \t}\n \thashmap_free(&cs->config_hash, 1);\n+\tcs->hash_initialized = 0;\n+}\n+\n+static int config_hash_callback(const char *key, const char *value, void *cb)\n+{\n+\tstruct config_set *cs = cb;\n+\tconfigset_add_value(cs, key, value);\n+\treturn 0;\n+}\n+\n+int git_configset_add_file(struct config_set *cs, const char *filename)\n+{\n+\tint ret = 0;\n+\tret = git_config_from_file(config_hash_callback, filename, cs);\n+\tif (!ret)\n+\t\treturn 0;\n+\telse {\n+\t\tgit_configset_clear(cs);\n+\t\treturn -1;\n+\t}\n+}\n+\n+int git_configset_get_value(struct config_set *cs, const char *key, const char **value)\n+{\n+\tconst struct string_list *values = NULL;\n+\t/*\n+\t * Follows \"last one wins\" semantic, i.e., if there are multiple matches for the\n+\t * queried key in the files of the configset, the value returned will be the last\n+\t * value in the value list for that key.\n+\t */\n+\tvalues = git_configset_get_value_multi(cs, key);\n+\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_configset_get_string(struct config_set *cs, const char *key, const char **dest)\n@@ -1427,20 +1411,24 @@ int git_configset_get_pathname(struct config_set *cs, const char *key, const cha\n \t\treturn 1;\n }\n \n-static int git_config_check_init(void)\n+static void git_config_check_init(void)\n {\n \tint ret = 0;\n \tif (the_config_set.hash_initialized)\n-\t\treturn 0;\n-\tconfigset_init(&the_config_set);\n+\t\treturn;\n+\tgit_configset_init(&the_config_set);\n \tret = git_config(config_hash_callback, &the_config_set);\n \tif (ret >= 0)\n-\t\treturn 0;\n-\telse {\n-\t\thashmap_free(&the_config_set.config_hash, 1);\n-\t\tthe_config_set.hash_initialized = 0;\n-\t\treturn -1;\n-\t}\n+\t\treturn;\n+\telse\n+\t\tdie(\"Unknown error when parsing one of the configuration files.\");\n+}\n+\n+void git_config_clear(void)\n+{\n+\tif (!the_config_set.hash_initialized)\n+\t\treturn;\n+\tgit_configset_clear(&the_config_set);\n }\n \n int git_config_get_value(const char *key, const char **value)\n@@ -1455,14 +1443,6 @@ const struct string_list *git_config_get_value_multi(const char *key)\n \treturn git_configset_get_value_multi(&the_config_set, key);\n }\n \n-void git_config_clear(void)\n-{\n-\tif (!the_config_set.hash_initialized)\n-\t\treturn;\n-\tgit_configset_clear(&the_config_set);\n-\tthe_config_set.hash_initialized = 0;\n-}\n-\n int git_config_get_string(const char *key, const char **dest)\n {\n \tgit_config_check_init();\ndiff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh\nindex 87a29f1..94085eb 100755\n--- a/t/t1308-config-set.sh\n+++ b/t/t1308-config-set.sh\n@@ -4,17 +4,33 @@ test_description='Test git config-set API in different settings'\n \n . ./test-lib.sh\n \n-# 'check section.key value' verifies that the entry for section.key is\n-# 'value'\n-check() {\n-\techo \"$2\" >expected &&\n-\ttest-config get_value \"$1\" >actual 2>&1 &&\n-\ttest_cmp expected actual\n+# 'check_config get_* section.key value' verifies that the entry for\n+# section.key is 'value'\n+check_config () {\n+\tcase \"$1\" in\n+\texpect_code)\n+\t\tif [ \"$#\" -lt 5 ];\n+\t\tthen\n+\t\t\t>expect\n+\t\telse\n+\t\t\tprintf \"%s\\n\" \"$5\" >expect\n+\t\tfi &&\n+\t\ttest_expect_code \"$2\" test-config \"$3\" \"$4\" >actual\n+\t\t;;\n+\t*)\n+\t\top=$1 key=$2 &&\n+\t\tshift &&\n+\t\tshift &&\n+\t\tprintf \"%s\\n\" \"$@\" >expect &&\n+\t\ttest-config \"$op\" \"$key\" >actual\n+\t\t;;\n+\tesac &&\n+\ttest_cmp expect actual\n }\n \n test_expect_success 'setup default config' '\n-\tcat >.git/config << EOF\n-\t[core]\n+\tcat >.git/config <<\\EOF\n+\t[case]\n \t\tpenguin = very blue\n \t\tMovie = BadPhysics\n \t\tUPPERCASE = true\n@@ -35,7 +51,7 @@ test_expect_success 'setup default config' '\n \t\thi = upper-case\n \t[my \"foo bar\"]\n \t\thi = lower-case\n-\t[core]\n+\t[case]\n \t\tbaz = bat\n \t\tbaz = hask\n \t[lamb]\n@@ -51,78 +67,74 @@ test_expect_success 'setup default config' '\n '\n \n test_expect_success 'get value for a simple key' '\n-\tcheck core.penguin \"very blue\"\n+\tcheck_config get_value case.penguin \"very blue\"\n '\n \n test_expect_success 'get value for a key with value as an empty string' '\n-\tcheck core.my \"\"\n+\tcheck_config get_value case.my \"\"\n '\n \n test_expect_success 'get value for a key with value as NULL' '\n-\tcheck core.foo \"(NULL)\"\n+\tcheck_config get_value case.foo \"(NULL)\"\n '\n \n test_expect_success 'upper case key' '\n-\tcheck core.UPPERCASE \"true\"\n+\tcheck_config get_value case.UPPERCASE \"true\" &&\n+\tcheck_config get_value case.uppercase \"true\"\n '\n \n test_expect_success 'mixed case key' '\n-\tcheck core.MixedCase \"true\"\n+\tcheck_config get_value case.MixedCase \"true\" &&\n+\tcheck_config get_value case.MIXEDCASE \"true\" &&\n+\tcheck_config get_value case.mixedcase \"true\"\n '\n \n test_expect_success 'key and value with mixed case' '\n-\tcheck core.Movie \"BadPhysics\"\n+\tcheck_config get_value case.Movie \"BadPhysics\"\n '\n \n test_expect_success 'key with case sensitive subsection' '\n-\tcheck \"my.Foo bAr.hi\" \"mixed-case\" &&\n-\tcheck \"my.FOO BAR.hi\" \"upper-case\" &&\n-\tcheck \"my.foo bar.hi\" \"lower-case\"\n+\tcheck_config get_value \"my.Foo bAr.hi\" \"mixed-case\" &&\n+\tcheck_config get_value \"my.FOO BAR.hi\" \"upper-case\" &&\n+\tcheck_config get_value \"my.foo bar.hi\" \"lower-case\"\n '\n \n test_expect_success 'key with case insensitive section header' '\n-\tcheck cores.baz \"ball\" &&\n-\tcheck Cores.baz \"ball\" &&\n-\tcheck CORES.baz \"ball\" &&\n-\tcheck coreS.baz \"ball\"\n+\tcheck_config get_value cores.baz \"ball\" &&\n+\tcheck_config get_value Cores.baz \"ball\" &&\n+\tcheck_config get_value CORES.baz \"ball\" &&\n+\tcheck_config get_value coreS.baz \"ball\"\n+'\n+\n+test_expect_success 'key with case insensitive section header & variable' '\n+\tcheck_config get_value CORES.BAZ \"ball\" &&\n+\tcheck_config get_value cores.baz \"ball\" &&\n+\tcheck_config get_value cores.BaZ \"ball\" &&\n+\tcheck_config get_value cOreS.bAz \"ball\"\n '\n \n test_expect_success 'find value with misspelled key' '\n-\techo \"Value not found for \\\"my.fOo Bar.hi\\\"\" >expect &&\n-\ttest_must_fail test-config get_value \"my.fOo Bar.hi\" >actual &&\n-\ttest_cmp expect actual\n+\tcheck_config expect_code 1 get_value \"my.fOo Bar.hi\" \"Value not found for \\\"my.fOo Bar.hi\\\"\"\n '\n \n test_expect_success 'find value with the highest priority' '\n-\tcheck core.baz \"hask\"\n+\tcheck_config get_value case.baz \"hask\"\n '\n \n test_expect_success 'find integer value for a key' '\n-\techo 65 >expect &&\n-\ttest-config get_int lamb.chop >actual &&\n-\ttest_cmp expect actual\n+\tcheck_config get_int lamb.chop 65\n '\n \n test_expect_success 'find integer if value is non parse-able' '\n-\techo 65 >expect &&\n-\ttest_must_fail test-config get_int lamb.head >actual &&\n-\ttest_must_fail test_cmp expect actual\n+\tcheck_config expect_code 128 get_int lamb.head\n '\n \n test_expect_success 'find bool value for the entered key' '\n-\tcat >expect <<-\\EOF &&\n-\t1\n-\t0\n-\t1\n-\t1\n-\t1\n-\tEOF\n-\ttest-config get_bool goat.head >actual &&\n-\ttest-config get_bool goat.skin >>actual &&\n-\ttest-config get_bool goat.nose >>actual &&\n-\ttest-config get_bool goat.horns >>actual &&\n-\ttest-config get_bool goat.legs >>actual &&\n-\ttest_cmp expect actual\n+\tcheck_config get_bool goat.head 1 &&\n+\tcheck_config get_bool goat.skin 0 &&\n+\tcheck_config get_bool goat.nose 1 &&\n+\tcheck_config get_bool goat.horns 1 &&\n+\tcheck_config get_bool goat.legs 1\n '\n \n test_expect_success 'find multiple values' '\n@@ -131,17 +143,17 @@ test_expect_success 'find multiple values' '\n \tbat\n \thask\n \tEOF\n-\ttest-config get_value_multi \"core.baz\">actual &&\n+\ttest-config get_value_multi \"case.baz\">actual &&\n \ttest_cmp expect actual\n '\n \n test_expect_success 'find value from a configset' '\n \tcat >config2 <<-\\EOF &&\n-\t[core]\n+\t[case]\n \t\tbaz = lama\n \t[my]\n \t\tnew = silk\n-\t[core]\n+\t[case]\n \t\tbaz = ball\n \tEOF\n \techo silk >expect &&\n@@ -151,7 +163,7 @@ test_expect_success 'find value from a configset' '\n \n test_expect_success 'find value with highest priority from a configset' '\n \techo hask > expect &&\n-\ttest-config configset_get_value core.baz config2 .git/config  >actual &&\n+\ttest-config configset_get_value case.baz config2 .git/config  >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -163,7 +175,37 @@ test_expect_success 'find value_list for a key from a configset' '\n \tlama\n \tball\n \tEOF\n-\ttest-config configset_get_value core.baz config2 .git/config  >actual &&\n+\ttest-config configset_get_value case.baz config2 .git/config  >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'proper error on non-existant files' '\n+\techo \"Error reading configuration file non-existant-file.\" >expect &&\n+\ttest_expect_code 2 test-config configset_get_value foo.bar non-existant-file 2>actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'proper error on non-accessible  files' '\n+\tchmod -r .git/config &&\n+\ttest_when_finished \"chmod +r .git/config\" &&\n+\techo \"Error reading configuration file .git/config.\" >expect &&\n+\ttest_expect_code 2 test-config configset_get_value foo.bar .git/config 2>actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'proper error on error in default config files' '\n+\tcp .git/config .git/config.old &&\n+\ttest_when_finished \"mv .git/config.old .git/config\" &&\n+\techo \"[\" >> .git/config &&\n+\techo \"fatal: bad config file line 35 in .git/config\" >expect &&\n+\ttest_expect_code 128 test-config get_value foo.bar 2>actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'proper error on error in custom config files' '\n+\techo \"[\" >> syntax-error &&\n+\techo \"fatal: bad config file line 1 in syntax-error\" >expect &&\n+\ttest_expect_code 128 test-config configset_get_value foo.bar syntax-error 2>actual &&\n \ttest_cmp expect actual\n '\n \ndiff --git a/test-config.c b/test-config.c\nindex dc313c2..cad35f4 100644\n--- a/test-config.c\n+++ b/test-config.c\n@@ -41,17 +41,17 @@ int main(int argc, char **argv)\n \n \tif (argc < 2) {\n \t\tfprintf(stderr, \"Please, provide a command name on the command-line\\n\");\n-\t\treturn 1;\n+\t\tgoto exit1;\n \t} else if (argc == 3 && !strcmp(argv[1], \"get_value\")) {\n \t\tif (!git_config_get_value(argv[2], &v)) {\n \t\t\tif (!v)\n \t\t\t\tprintf(\"(NULL)\\n\");\n \t\t\telse\n \t\t\t\tprintf(\"%s\\n\", v);\n-\t\t\treturn 0;\n+\t\t\tgoto exit0;\n \t\t} else {\n \t\t\tprintf(\"Value not found for \\\"%s\\\"\\n\", argv[2]);\n-\t\t\treturn 1;\n+\t\t\tgoto exit1;\n \t\t}\n \t} else if (argc == 3 && !strcmp(argv[1], \"get_value_multi\")) {\n \t\tstrptr = git_config_get_value_multi(argv[2]);\n@@ -63,46 +63,50 @@ int main(int argc, char **argv)\n \t\t\t\telse\n \t\t\t\t\tprintf(\"%s\\n\", v);\n \t\t\t}\n-\t\t\treturn 0;\n+\t\t\tgoto exit0;\n \t\t} else {\n \t\t\tprintf(\"Value not found for \\\"%s\\\"\\n\", argv[2]);\n-\t\t\treturn 1;\n+\t\t\tgoto exit1;\n \t\t}\n \t} else if (argc == 3 && !strcmp(argv[1], \"get_int\")) {\n \t\tif (!git_config_get_int(argv[2], &val)) {\n \t\t\tprintf(\"%d\\n\", val);\n-\t\t\treturn 0;\n+\t\t\tgoto exit0;\n \t\t} else {\n \t\t\tprintf(\"Value not found for \\\"%s\\\"\\n\", argv[2]);\n-\t\t\treturn 1;\n+\t\t\tgoto exit1;\n \t\t}\n \t} else if (argc == 3 && !strcmp(argv[1], \"get_bool\")) {\n \t\tif (!git_config_get_bool(argv[2], &val)) {\n \t\t\tprintf(\"%d\\n\", val);\n-\t\t\treturn 0;\n+\t\t\tgoto exit0;\n \t\t} else {\n \t\t\tprintf(\"Value not found for \\\"%s\\\"\\n\", argv[2]);\n-\t\t\treturn 1;\n+\t\t\tgoto exit1;\n \t\t}\n \t} else if (!strcmp(argv[1], \"configset_get_value\")) {\n \t\tfor (i = 3; i < argc; i++) {\n-\t\t\tif (git_configset_add_file(&cs, argv[i]))\n-\t\t\t\treturn 2;\n+\t\t\tif (git_configset_add_file(&cs, argv[i])) {\n+\t\t\t\tfprintf(stderr, \"Error reading configuration file %s.\\n\", argv[i]);\n+\t\t\t\tgoto exit2;\n+\t\t\t}\n \t\t}\n \t\tif (!git_configset_get_value(&cs, argv[2], &v)) {\n \t\t\tif (!v)\n \t\t\t\tprintf(\"(NULL)\\n\");\n \t\t\telse\n \t\t\t\tprintf(\"%s\\n\", v);\n-\t\t\treturn 0;\n+\t\t\tgoto exit0;\n \t\t} else {\n \t\t\tprintf(\"Value not found for \\\"%s\\\"\\n\", argv[2]);\n-\t\t\treturn 1;\n+\t\t\tgoto exit1;\n \t\t}\n \t} else if (!strcmp(argv[1], \"configset_get_value_multi\")) {\n \t\tfor (i = 3; i < argc; i++) {\n-\t\t\tif (git_configset_add_file(&cs, argv[i]))\n-\t\t\t\treturn 2;\n+\t\t\tif (git_configset_add_file(&cs, argv[i])) {\n+\t\t\t\tfprintf(stderr, \"Error reading configuration file %s.\\n\", argv[i]);\n+\t\t\t\tgoto exit2;\n+\t\t\t}\n \t\t}\n \t\tstrptr = git_configset_get_value_multi(&cs, argv[2]);\n \t\tif (strptr) {\n@@ -113,13 +117,24 @@ int main(int argc, char **argv)\n \t\t\t\telse\n \t\t\t\t\tprintf(\"%s\\n\", v);\n \t\t\t}\n-\t\t\treturn 0;\n+\t\t\tgoto exit0;\n \t\t} else {\n \t\t\tprintf(\"Value not found for \\\"%s\\\"\\n\", argv[2]);\n-\t\t\treturn 1;\n+\t\t\tgoto exit1;\n \t\t}\n \t}\n \n-\tfprintf(stderr, \"%s: Please check the syntax and the function name\\n\", argv[0]);\n+\tdie(\"%s: Please check the syntax and the function name\", argv[0]);\n+\n+exit0:\n+\tgit_configset_clear(&cs);\n+\treturn 0;\n+\n+exit1:\n+\tgit_configset_clear(&cs);\n \treturn 1;\n+\n+exit2:\n+\tgit_configset_clear(&cs);\n+\treturn 2;\n }\n\n-- 8< --\n"},{"id":"246058","messageId":"1405434571-25459-2-git-send-email-tanayabh@gmail.com","threadId":"37130","inReplyTo":"1405434571-25459-1-git-send-email-tanayabh@gmail.com","subject":"[PATCH v9 1/2] add `config_set` API for caching config-like files","fromName":"Tanay Abhra","fromEmail":"tanayabh@gmail.com","sentAt":"2014-07-15T14:29:30Z","receivedAt":"2014-07-15T14:29:30Z","isPatch":true,"sender":{"key":"tanayabh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2097229?v=4"},"body":"Currently `git_config()` uses a callback mechanism and file rereads for\nconfig values. Due to this approach, it is not uncommon for the config\nfiles to be parsed several times during the run of a git program, with\ndifferent callbacks picking out different variables useful to themselves.\n\nAdd a `config_set`, that can be used to construct an in-memory cache for\nconfig-like files that the caller specifies (i.e., files like `.gitmodules`,\n`~/.gitconfig` etc.). Add two external functions `git_configset_get_value`\nand `git_configset_get_value_multi` for querying from the config sets.\n`git_configset_get_value` follows `last one wins` semantic (i.e. if there\nare multiple matches for the queried key in the files of the configset the\nvalue returned will be the last entry in `value_list`).\n`git_configset_get_value_multi` returns a list of values sorted in order of\nincreasing priority (i.e. last match will be at the end of the list). Add\ntype specific query functions like `git_configset_get_bool` and similar.\n\nAdd a default `config_set`, `the_config_set` to cache all key-value pairs\nread from usual config files (repo specific .git/config, user wide\n~/.gitconfig, XDG config and the global /etc/gitconfig). `the_config_set`\nis populated using `git_config()`.\n\nAdd two external functions `git_config_get_value` and \n`git_config_get_value_multi` for querying in a non-callback manner from\n`the_config_set`. Also, add type specific query functions that are\nimplemented as a thin wrapper around the `config_set` API.\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\nSigned-off-by: Tanay Abhra <tanayabh@gmail.com>\n---\n Documentation/technical/api-config.txt | 135 ++++++++++++++++\n cache.h                                |  31 ++++\n config.c                               | 276 +++++++++++++++++++++++++++++++++\n 3 files changed, 442 insertions(+)\n\ndiff --git a/Documentation/technical/api-config.txt b/Documentation/technical/api-config.txt\nindex 230b3a0..48255a2 100644\n--- a/Documentation/technical/api-config.txt\n+++ b/Documentation/technical/api-config.txt\n@@ -77,6 +77,81 @@ 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_value`\n+and `git_config_get_value_multi`. They both read values from an internal\n+cache generated previously from reading the config files.\n+\n+`int git_config_get_value(const char *key, const char **value)`::\n+\n+\tFinds the highest-priority value for the configuration variable `key`,\n+\tstores the pointer to it in `value` and returns 0. When the\n+\tconfiguration variable `key` is not found, returns 1 without touching\n+\t`value`. The caller should not free or modify `value`, as it is owned\n+\tby the cache.\n+\n+`const struct string_list *git_config_get_value_multi(const char *key)`::\n+\n+\tFinds and returns the value list, sorted in order of increasing priority\n+\tfor the configuration variable `key`. When the configuration variable\n+\t`key` is not found, returns NULL. The caller should not free or modify\n+\tthe returned pointer, as it is owned by the cache.\n+\n+`void git_config_clear(void)`::\n+\n+\tResets and invalidates the config cache.\n+\n+The config API also provides type specific API functions which do conversion\n+as well as retrieval for the queried variable, including:\n+\n+`int git_config_get_int(const char *key, int *dest)`::\n+\n+\tFinds and parses the value to an integer for the configuration variable\n+\t`key`. Dies on error; otherwise, stores the value of the parsed integer in\n+\t`dest` and returns 0. When the configuration variable `key` is not found,\n+\treturns 1 without touching `dest`.\n+\n+`int git_config_get_ulong(const char *key, unsigned long *dest)`::\n+\n+\tSimilar to `git_config_get_int` but for unsigned longs.\n+\n+`int git_config_get_bool(const char *key, int *dest)`::\n+\n+\tFinds and parses the value into a boolean value, for the configuration\n+\tvariable `key` respecting keywords like \"true\" and \"false\". Integer\n+\tvalues are converted into true/false values (when they are non-zero or\n+\tzero, respectively). Other values cause a die(). If parsing is successful,\n+\tstores the value of the parsed result in `dest` and returns 0. When the\n+\tconfiguration variable `key` is not found, returns 1 without touching\n+\t`dest`.\n+\n+`int git_config_get_bool_or_int(const char *key, int *is_bool, int *dest)`::\n+\n+\tSimilar to `git_config_get_bool`, except that integers are copied as-is,\n+\tand `is_bool` flag is unset.\n+\n+`int git_config_get_maybe_bool(const char *key, int *dest)`::\n+\n+\tSimilar to `git_config_get_bool`, except that it returns -1 on error\n+\trather than dying.\n+\n+`int git_config_get_string(const char *key, const char **dest)`::\n+\n+\tAllocates and copies the retrieved string into the `dest` parameter for\n+\tthe configuration variable `key`; if NULL string is given, prints an\n+\terror message and returns -1. When the configuration variable `key` is\n+\tnot found, returns 1 without touching `dest`.\n+\n+`int git_config_get_pathname(const char *key, const char **dest)`::\n+\n+\tSimilar to `git_config_get_string`, but expands `~` or `~user` into\n+\tthe user's home directory when found at the beginning of the path.\n+\n+See test-config.c for usage examples.\n+\n Value Parsing Helpers\n ---------------------\n \n@@ -134,6 +209,66 @@ int read_file_with_include(const char *file, config_fn_t fn, void *data)\n `git_config` respects includes automatically. The lower-level\n `git_config_from_file` does not.\n \n+Custom Configsets\n+-----------------\n+\n+A `config_set` can be used to construct an in-memory cache for\n+config-like files that the caller specifies (i.e., files like `.gitmodules`,\n+`~/.gitconfig` etc.). For example,\n+\n+---------------------------------------\n+struct config_set gm_config;\n+git_configset_init(&gm_config);\n+int b;\n+/* we add config files to the config_set */\n+git_configset_add_file(&gm_config, \".gitmodules\");\n+git_configset_add_file(&gm_config, \".gitmodules_alt\");\n+\n+if (!git_configset_get_bool(gm_config, \"submodule.frotz.ignore\", &b)) {\n+\t/* hack hack hack */\n+}\n+\n+/* when we are done with the configset */\n+git_configset_clear(&gm_config);\n+----------------------------------------\n+\n+Configset API provides functions for the above mentioned work flow, including:\n+\n+`void git_configset_init(struct config_set *cs)`::\n+\n+\tInitializes the config_set `cs`.\n+\n+`int git_configset_add_file(struct config_set *cs, const char *filename)`::\n+\n+\tParses the file and adds the variable-value pairs to the `config_set`,\n+\tdies if there is an error in parsing the file. Returns 0 on success, or\n+\t-1 if the file doesnot exist or cannot be read.\n+\n+`int git_configset_get_value(struct config_set *cs, const char *key, const char **value)`::\n+\n+\tFinds the highest-priority value for the configuration variable `key`\n+\tand config set `cs`, stores the pointer to it in `value` and returns 0.\n+\tWhen the configuration variable `key` is not found, returns 1 without\n+\ttouching `value`. The caller should not free or modify `value`, as it\n+\tis owned by the cache.\n+\n+`const struct string_list *git_configset_get_value_multi(struct config_set *cs, const char *key)`::\n+\n+\tFinds and returns the value list, sorted in order of increasing priority\n+\tfor the configuration variable `key` and config set `cs`. When the\n+\tconfiguration variable `key` is not found, returns NULL. The caller\n+\tshould not free or modify the returned pointer, as it is owned by the cache.\n+\n+`void git_configset_clear(struct config_set *cs)`::\n+\n+\tClears `config_set` structure, removes all saved variable-value pairs.\n+\n+In addition to above functions, the `config_set` API provides type specific\n+functions in the vein of `git_config_get_int` and family but with an extra\n+parameter, pointer to struct `config_set`.\n+They all behave similarly to the `git_config_get*()` family described in\n+\"Querying For Specific Variables\" above.\n+\n Writing Config Files\n --------------------\n \ndiff --git a/cache.h b/cache.h\nindex 44aa439..ec5fd93 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1329,6 +1329,37 @@ extern int parse_config_key(const char *var,\n \t\t\t    const char **subsection, int *subsection_len,\n \t\t\t    const char **key);\n \n+struct config_set {\n+\tstruct hashmap config_hash;\n+\tint hash_initialized;\n+};\n+\n+extern void git_configset_init(struct config_set *cs);\n+extern int git_configset_add_file(struct config_set *cs, const char *filename);\n+extern int git_configset_get_value(struct config_set *cs, const char *key, const char **value);\n+extern const struct string_list *git_configset_get_value_multi(struct config_set *cs, const char *key);\n+extern void git_configset_clear(struct config_set *cs);\n+extern void git_configset_iter(struct config_set *cs, config_fn_t fn, void *data);\n+extern int git_configset_get_string(struct config_set *cs, const char *key, const char **dest);\n+extern int git_configset_get_int(struct config_set *cs, const char *key, int *dest);\n+extern int git_configset_get_ulong(struct config_set *cs, const char *key, unsigned long *dest);\n+extern int git_configset_get_bool(struct config_set *cs, const char *key, int *dest);\n+extern int git_configset_get_bool_or_int(struct config_set *cs, const char *key, int *is_bool, int *dest);\n+extern int git_configset_get_maybe_bool(struct config_set *cs, const char *key, int *dest);\n+extern int git_configset_get_pathname(struct config_set *cs, const char *key, const char **dest);\n+\n+extern int git_config_get_value(const char *key, const char **value);\n+extern const struct string_list *git_config_get_value_multi(const char *key);\n+extern void git_config_clear(void);\n+extern void git_config_iter(config_fn_t fn, void *data);\n+extern int git_config_get_string(const char *key, const char **dest);\n+extern int git_config_get_int(const char *key, int *dest);\n+extern int git_config_get_ulong(const char *key, unsigned long *dest);\n+extern int git_config_get_bool(const char *key, int *dest);\n+extern int git_config_get_bool_or_int(const char *key, int *is_bool, int *dest);\n+extern int git_config_get_maybe_bool(const char *key, int *dest);\n+extern int git_config_get_pathname(const char *key, const char **dest);\n+\n extern int committer_ident_sufficiently_given(void);\n extern int author_ident_sufficiently_given(void);\n \ndiff --git a/config.c b/config.c\nindex ba882a1..89e2d67 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@@ -33,10 +35,23 @@ struct config_source {\n \tlong (*do_ftell)(struct config_source *c);\n };\n \n+struct config_set_element {\n+\tstruct hashmap_entry ent;\n+\tchar *key;\n+\tstruct string_list value_list;\n+};\n+\n static struct config_source *cf;\n \n static int zlib_compression_seen;\n \n+/*\n+ * Default config_set that contains key-value pairs from the usual set of config\n+ * config files (i.e repo specific .git/config, user wide ~/.gitconfig, XDG\n+ * config file and the global /etc/gitconfig)\n+ */\n+static struct config_set the_config_set;\n+\n static int config_file_fgetc(struct config_source *conf)\n {\n \treturn fgetc(conf->u.file);\n@@ -1212,6 +1227,264 @@ int git_config(config_fn_t fn, void *data)\n \treturn git_config_with_options(fn, data, NULL, 1);\n }\n \n+static struct config_set_element *configset_find_element(struct config_set *cs, const char *key)\n+{\n+\tstruct config_set_element k;\n+\tstruct config_set_element *found_entry;\n+\tchar *normalized_key;\n+\tint ret;\n+\t/*\n+\t * `key` may come from the user, so normalize it before using it\n+\t * for querying entries from the hashmap.\n+\t */\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(&cs->config_hash, &k, NULL);\n+\tfree(normalized_key);\n+\treturn found_entry;\n+}\n+\n+const struct string_list *git_configset_get_value_multi(struct config_set *cs, const char *key)\n+{\n+\tstruct config_set_element *e = configset_find_element(cs, key);\n+\treturn e ? &e->value_list : NULL;\n+}\n+\n+static int configset_add_value(struct config_set *cs, const char *key, const char *value)\n+{\n+\tstruct config_set_element *e;\n+\te = configset_find_element(cs, key);\n+\t/*\n+\t * Since the keys are being fed by git_config*() callback mechanism, they\n+\t * are already normalized. So simply add them without any further munging.\n+\t */\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\tmemset(&e->value_list, 0, sizeof(e->value_list));\n+\t\te->value_list.strdup_strings = 1;\n+\t\thashmap_add(&cs->config_hash, e);\n+\t}\n+\tstring_list_append_nodup(&e->value_list, value ? xstrdup(value) : NULL);\n+\n+\treturn 0;\n+}\n+\n+static int config_set_element_cmp(const struct config_set_element *e1,\n+\t\t\t\t const struct config_set_element *e2, const void *unused)\n+{\n+\treturn strcmp(e1->key, e2->key);\n+}\n+\n+void git_configset_init(struct config_set *cs)\n+{\n+\thashmap_init(&cs->config_hash, (hashmap_cmp_fn)config_set_element_cmp, 0);\n+\tcs->hash_initialized = 1;\n+}\n+\n+void git_configset_clear(struct config_set *cs)\n+{\n+\tstruct config_set_element *entry;\n+\tstruct hashmap_iter iter;\n+\tif (!cs->hash_initialized)\n+\t\treturn;\n+\n+\thashmap_iter_init(&cs->config_hash, &iter);\n+\twhile ((entry = hashmap_iter_next(&iter))) {\n+\t\tfree(entry->key);\n+\t\tstring_list_clear(&entry->value_list, 0);\n+\t}\n+\thashmap_free(&cs->config_hash, 1);\n+\tcs->hash_initialized = 0;\n+}\n+\n+static int config_hash_callback(const char *key, const char *value, void *cb)\n+{\n+\tstruct config_set *cs = cb;\n+\tconfigset_add_value(cs, key, value);\n+\treturn 0;\n+}\n+\n+int git_configset_add_file(struct config_set *cs, const char *filename)\n+{\n+\tint ret = 0;\n+\tret = git_config_from_file(config_hash_callback, filename, cs);\n+\tif (!ret)\n+\t\treturn 0;\n+\telse {\n+\t\tgit_configset_clear(cs);\n+\t\treturn -1;\n+\t}\n+}\n+\n+int git_configset_get_value(struct config_set *cs, const char *key, const char **value)\n+{\n+\tconst struct string_list *values = NULL;\n+\t/*\n+\t * Follows \"last one wins\" semantic, i.e., if there are multiple matches for the\n+\t * queried key in the files of the configset, the value returned will be the last\n+\t * value in the value list for that key.\n+\t */\n+\tvalues = git_configset_get_value_multi(cs, key);\n+\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_configset_get_string(struct config_set *cs, const char *key, const char **dest)\n+{\n+\tconst char *value;\n+\tif (!git_configset_get_value(cs, key, &value))\n+\t\treturn git_config_string(dest, key, value);\n+\telse\n+\t\treturn 1;\n+}\n+\n+int git_configset_get_int(struct config_set *cs, const char *key, int *dest)\n+{\n+\tconst char *value;\n+\tif (!git_configset_get_value(cs, key, &value)) {\n+\t\t*dest = git_config_int(key, value);\n+\t\treturn 0;\n+\t} else\n+\t\treturn 1;\n+}\n+\n+int git_configset_get_ulong(struct config_set *cs, const char *key, unsigned long *dest)\n+{\n+\tconst char *value;\n+\tif (!git_configset_get_value(cs, key, &value)) {\n+\t\t*dest = git_config_ulong(key, value);\n+\t\treturn 0;\n+\t} else\n+\t\treturn 1;\n+}\n+\n+int git_configset_get_bool(struct config_set *cs, const char *key, int *dest)\n+{\n+\tconst char *value;\n+\tif (!git_configset_get_value(cs, key, &value)) {\n+\t\t*dest = git_config_bool(key, value);\n+\t\treturn 0;\n+\t} else\n+\t\treturn 1;\n+}\n+\n+int git_configset_get_bool_or_int(struct config_set *cs, const char *key,\n+\t\t\t\tint *is_bool, int *dest)\n+{\n+\tconst char *value;\n+\tif (!git_configset_get_value(cs, key, &value)) {\n+\t\t*dest = git_config_bool_or_int(key, value, is_bool);\n+\t\treturn 0;\n+\t} else\n+\t\treturn 1;\n+}\n+\n+int git_configset_get_maybe_bool(struct config_set *cs, const char *key, int *dest)\n+{\n+\tconst char *value;\n+\tif (!git_configset_get_value(cs, key, &value)) {\n+\t\t*dest = git_config_maybe_bool(key, value);\n+\t\tif (*dest == -1)\n+\t\t\treturn -1;\n+\t\treturn 0;\n+\t} else\n+\t\treturn 1;\n+}\n+\n+int git_configset_get_pathname(struct config_set *cs, const char *key, const char **dest)\n+{\n+\tconst char *value;\n+\tif (!git_configset_get_value(cs, key, &value))\n+\t\treturn git_config_pathname(dest, key, value);\n+\telse\n+\t\treturn 1;\n+}\n+\n+static void git_config_check_init(void)\n+{\n+\tint ret = 0;\n+\tif (the_config_set.hash_initialized)\n+\t\treturn;\n+\tgit_configset_init(&the_config_set);\n+\tret = git_config(config_hash_callback, &the_config_set);\n+\tif (ret >= 0)\n+\t\treturn;\n+\telse\n+\t\tdie(\"Unknown error when parsing one of the configuration files.\");\n+}\n+\n+void git_config_clear(void)\n+{\n+\tif (!the_config_set.hash_initialized)\n+\t\treturn;\n+\tgit_configset_clear(&the_config_set);\n+}\n+\n+int git_config_get_value(const char *key, const char **value)\n+{\n+\tgit_config_check_init();\n+\treturn git_configset_get_value(&the_config_set, key, value);\n+}\n+\n+const struct string_list *git_config_get_value_multi(const char *key)\n+{\n+\tgit_config_check_init();\n+\treturn git_configset_get_value_multi(&the_config_set, key);\n+}\n+\n+int git_config_get_string(const char *key, const char **dest)\n+{\n+\tgit_config_check_init();\n+\treturn git_configset_get_string(&the_config_set, key, dest);\n+}\n+\n+int git_config_get_int(const char *key, int *dest)\n+{\n+\tgit_config_check_init();\n+\treturn git_configset_get_int(&the_config_set, key, dest);\n+}\n+\n+int git_config_get_ulong(const char *key, unsigned long *dest)\n+{\n+\tgit_config_check_init();\n+\treturn git_configset_get_ulong(&the_config_set, key, dest);\n+}\n+\n+int git_config_get_bool(const char *key, int *dest)\n+{\n+\tgit_config_check_init();\n+\treturn git_configset_get_bool(&the_config_set, key, dest);\n+}\n+\n+int git_config_get_bool_or_int(const char *key, int *is_bool, int *dest)\n+{\n+\tgit_config_check_init();\n+\treturn git_configset_get_bool_or_int(&the_config_set, key, is_bool, dest);\n+}\n+\n+int git_config_get_maybe_bool(const char *key, int *dest)\n+{\n+\tgit_config_check_init();\n+\treturn git_configset_get_maybe_bool(&the_config_set, key, dest);\n+}\n+\n+int git_config_get_pathname(const char *key, const char **dest)\n+{\n+\tgit_config_check_init();\n+\treturn git_configset_get_pathname(&the_config_set, key, dest);\n+}\n+\n /*\n  * Find all the stuff for git_config_set() below.\n  */\n@@ -1707,6 +1980,9 @@ int git_config_set_multivar_in_file(const char *config_filename,\n \tlock = NULL;\n \tret = 0;\n \n+\t/* Invalidate the config cache */\n+\tgit_config_clear();\n+\n out_free:\n \tif (lock)\n \t\trollback_lock_file(lock);\n-- \n1.9.0.GIT\n"},{"id":"246057","messageId":"1405434571-25459-3-git-send-email-tanayabh@gmail.com","threadId":"37130","inReplyTo":"1405434571-25459-1-git-send-email-tanayabh@gmail.com","subject":"[PATCH v9 2/2] test-config: add tests for the config_set API","fromName":"Tanay Abhra","fromEmail":"tanayabh@gmail.com","sentAt":"2014-07-15T14:29:31Z","receivedAt":"2014-07-15T14:29:31Z","isPatch":true,"sender":{"key":"tanayabh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2097229?v=4"},"body":"Expose the `config_set` C API as a set of simple commands in order to\nfacilitate testing. Add tests for the `config_set` API as well as for\n`git_config_get_*()` family for the usual config files.\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\nSigned-off-by: Tanay Abhra <tanayabh@gmail.com>\n---\n .gitignore            |   1 +\n Makefile              |   1 +\n t/t1308-config-set.sh | 212 ++++++++++++++++++++++++++++++++++++++++++++++++++\n test-config.c         | 140 +++++++++++++++++++++++++++++++++\n 4 files changed, 354 insertions(+)\n create mode 100755 t/t1308-config-set.sh\n create mode 100644 test-config.c\n\ndiff --git a/.gitignore b/.gitignore\nindex 42294e5..7677533 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -177,6 +177,7 @@\n /gitweb/static/gitweb.min.*\n /test-chmtime\n /test-ctype\n+/test-config\n /test-date\n /test-delta\n /test-dump-cache-tree\ndiff --git a/Makefile b/Makefile\nindex b92418d..e070eb8 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -549,6 +549,7 @@ PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))\n \n TEST_PROGRAMS_NEED_X += test-chmtime\n TEST_PROGRAMS_NEED_X += test-ctype\n+TEST_PROGRAMS_NEED_X += test-config\n TEST_PROGRAMS_NEED_X += test-date\n TEST_PROGRAMS_NEED_X += test-delta\n TEST_PROGRAMS_NEED_X += test-dump-cache-tree\ndiff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh\nnew file mode 100755\nindex 0000000..94085eb\n--- /dev/null\n+++ b/t/t1308-config-set.sh\n@@ -0,0 +1,212 @@\n+#!/bin/sh\n+\n+test_description='Test git config-set API in different settings'\n+\n+. ./test-lib.sh\n+\n+# 'check_config get_* section.key value' verifies that the entry for\n+# section.key is 'value'\n+check_config () {\n+\tcase \"$1\" in\n+\texpect_code)\n+\t\tif [ \"$#\" -lt 5 ];\n+\t\tthen\n+\t\t\t>expect\n+\t\telse\n+\t\t\tprintf \"%s\\n\" \"$5\" >expect\n+\t\tfi &&\n+\t\ttest_expect_code \"$2\" test-config \"$3\" \"$4\" >actual\n+\t\t;;\n+\t*)\n+\t\top=$1 key=$2 &&\n+\t\tshift &&\n+\t\tshift &&\n+\t\tprintf \"%s\\n\" \"$@\" >expect &&\n+\t\ttest-config \"$op\" \"$key\" >actual\n+\t\t;;\n+\tesac &&\n+\ttest_cmp expect actual\n+}\n+\n+test_expect_success 'setup default config' '\n+\tcat >.git/config <<\\EOF\n+\t[case]\n+\t\tpenguin = very blue\n+\t\tMovie = BadPhysics\n+\t\tUPPERCASE = true\n+\t\tMixedCase = true\n+\t\tmy =\n+\t\tfoo\n+\t\tbaz = sam\n+\t[Cores]\n+\t\tWhatEver = Second\n+\t\tbaz = bar\n+\t[cores]\n+\t\tbaz = bat\n+\t[CORES]\n+\t\tbaz = ball\n+\t[my \"Foo bAr\"]\n+\t\thi = mixed-case\n+\t[my \"FOO BAR\"]\n+\t\thi = upper-case\n+\t[my \"foo bar\"]\n+\t\thi = lower-case\n+\t[case]\n+\t\tbaz = bat\n+\t\tbaz = hask\n+\t[lamb]\n+\t\tchop = 65\n+\t\thead = none\n+\t[goat]\n+\t\tlegs = 4\n+\t\thead = true\n+\t\tskin = false\n+\t\tnose = 1\n+\t\thorns\n+\tEOF\n+'\n+\n+test_expect_success 'get value for a simple key' '\n+\tcheck_config get_value case.penguin \"very blue\"\n+'\n+\n+test_expect_success 'get value for a key with value as an empty string' '\n+\tcheck_config get_value case.my \"\"\n+'\n+\n+test_expect_success 'get value for a key with value as NULL' '\n+\tcheck_config get_value case.foo \"(NULL)\"\n+'\n+\n+test_expect_success 'upper case key' '\n+\tcheck_config get_value case.UPPERCASE \"true\" &&\n+\tcheck_config get_value case.uppercase \"true\"\n+'\n+\n+test_expect_success 'mixed case key' '\n+\tcheck_config get_value case.MixedCase \"true\" &&\n+\tcheck_config get_value case.MIXEDCASE \"true\" &&\n+\tcheck_config get_value case.mixedcase \"true\"\n+'\n+\n+test_expect_success 'key and value with mixed case' '\n+\tcheck_config get_value case.Movie \"BadPhysics\"\n+'\n+\n+test_expect_success 'key with case sensitive subsection' '\n+\tcheck_config get_value \"my.Foo bAr.hi\" \"mixed-case\" &&\n+\tcheck_config get_value \"my.FOO BAR.hi\" \"upper-case\" &&\n+\tcheck_config get_value \"my.foo bar.hi\" \"lower-case\"\n+'\n+\n+test_expect_success 'key with case insensitive section header' '\n+\tcheck_config get_value cores.baz \"ball\" &&\n+\tcheck_config get_value Cores.baz \"ball\" &&\n+\tcheck_config get_value CORES.baz \"ball\" &&\n+\tcheck_config get_value coreS.baz \"ball\"\n+'\n+\n+test_expect_success 'key with case insensitive section header & variable' '\n+\tcheck_config get_value CORES.BAZ \"ball\" &&\n+\tcheck_config get_value cores.baz \"ball\" &&\n+\tcheck_config get_value cores.BaZ \"ball\" &&\n+\tcheck_config get_value cOreS.bAz \"ball\"\n+'\n+\n+test_expect_success 'find value with misspelled key' '\n+\tcheck_config expect_code 1 get_value \"my.fOo Bar.hi\" \"Value not found for \\\"my.fOo Bar.hi\\\"\"\n+'\n+\n+test_expect_success 'find value with the highest priority' '\n+\tcheck_config get_value case.baz \"hask\"\n+'\n+\n+test_expect_success 'find integer value for a key' '\n+\tcheck_config get_int lamb.chop 65\n+'\n+\n+test_expect_success 'find integer if value is non parse-able' '\n+\tcheck_config expect_code 128 get_int lamb.head\n+'\n+\n+test_expect_success 'find bool value for the entered key' '\n+\tcheck_config get_bool goat.head 1 &&\n+\tcheck_config get_bool goat.skin 0 &&\n+\tcheck_config get_bool goat.nose 1 &&\n+\tcheck_config get_bool goat.horns 1 &&\n+\tcheck_config get_bool goat.legs 1\n+'\n+\n+test_expect_success 'find multiple values' '\n+\tcat >expect <<-\\EOF &&\n+\tsam\n+\tbat\n+\thask\n+\tEOF\n+\ttest-config get_value_multi \"case.baz\">actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'find value from a configset' '\n+\tcat >config2 <<-\\EOF &&\n+\t[case]\n+\t\tbaz = lama\n+\t[my]\n+\t\tnew = silk\n+\t[case]\n+\t\tbaz = ball\n+\tEOF\n+\techo silk >expect &&\n+\ttest-config configset_get_value my.new config2 .git/config >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'find value with highest priority from a configset' '\n+\techo hask > expect &&\n+\ttest-config configset_get_value case.baz config2 .git/config  >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'find value_list for a key from a configset' '\n+\tcat >except <<-\\EOF &&\n+\tsam\n+\tbat\n+\thask\n+\tlama\n+\tball\n+\tEOF\n+\ttest-config configset_get_value case.baz config2 .git/config  >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'proper error on non-existant files' '\n+\techo \"Error reading configuration file non-existant-file.\" >expect &&\n+\ttest_expect_code 2 test-config configset_get_value foo.bar non-existant-file 2>actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'proper error on non-accessible files' '\n+\tchmod -r .git/config &&\n+\ttest_when_finished \"chmod +r .git/config\" &&\n+\techo \"Error reading configuration file .git/config.\" >expect &&\n+\ttest_expect_code 2 test-config configset_get_value foo.bar .git/config 2>actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'proper error on error in default config files' '\n+\tcp .git/config .git/config.old &&\n+\ttest_when_finished \"mv .git/config.old .git/config\" &&\n+\techo \"[\" >> .git/config &&\n+\techo \"fatal: bad config file line 35 in .git/config\" >expect &&\n+\ttest_expect_code 128 test-config get_value foo.bar 2>actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'proper error on error in custom config files' '\n+\techo \"[\" >> syntax-error &&\n+\techo \"fatal: bad config file line 1 in syntax-error\" >expect &&\n+\ttest_expect_code 128 test-config configset_get_value foo.bar syntax-error 2>actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_done\ndiff --git a/test-config.c b/test-config.c\nnew file mode 100644\nindex 0000000..cad35f4\n--- /dev/null\n+++ b/test-config.c\n@@ -0,0 +1,140 @@\n+#include \"cache.h\"\n+#include \"string-list.h\"\n+\n+/*\n+ * This program exposes the C API of the configuration mechanism\n+ * as a set of simple commands in order to facilitate testing.\n+ *\n+ * Reads stdin and prints result of command to stdout:\n+ *\n+ * get_value -> prints the value with highest priority for the entered key\n+ *\n+ * get_value_multi -> prints all values for the entered key in increasing order\n+ *\t\t     of priority\n+ *\n+ * get_int -> print integer value for the entered key or die\n+ *\n+ * get_bool -> print bool value for the entered key or die\n+ *\n+ * configset_get_value -> returns value with the highest priority for the entered key\n+ * \t\t\tfrom a config_set constructed from files entered as arguments.\n+ *\n+ * configset_get_value_multi -> returns value_list for the entered key sorted in\n+ * \t\t\t\tascending order of priority from a config_set\n+ * \t\t\t\tconstructed from files entered as arguments.\n+ *\n+ * Examples:\n+ *\n+ * To print the value with highest priority for key \"foo.bAr Baz.rock\":\n+ * \ttest-config get_value \"foo.bAr Baz.rock\"\n+ *\n+ */\n+\n+\n+int main(int argc, char **argv)\n+{\n+\tint i, val;\n+\tconst char *v;\n+\tconst struct string_list *strptr;\n+\tstruct config_set cs;\n+\tgit_configset_init(&cs);\n+\n+\tif (argc < 2) {\n+\t\tfprintf(stderr, \"Please, provide a command name on the command-line\\n\");\n+\t\tgoto exit1;\n+\t} else if (argc == 3 && !strcmp(argv[1], \"get_value\")) {\n+\t\tif (!git_config_get_value(argv[2], &v)) {\n+\t\t\tif (!v)\n+\t\t\t\tprintf(\"(NULL)\\n\");\n+\t\t\telse\n+\t\t\t\tprintf(\"%s\\n\", v);\n+\t\t\tgoto exit0;\n+\t\t} else {\n+\t\t\tprintf(\"Value not found for \\\"%s\\\"\\n\", argv[2]);\n+\t\t\tgoto exit1;\n+\t\t}\n+\t} else if (argc == 3 && !strcmp(argv[1], \"get_value_multi\")) {\n+\t\tstrptr = git_config_get_value_multi(argv[2]);\n+\t\tif (strptr) {\n+\t\t\tfor (i = 0; i < strptr->nr; i++) {\n+\t\t\t\tv = strptr->items[i].string;\n+\t\t\t\tif (!v)\n+\t\t\t\t\tprintf(\"(NULL)\\n\");\n+\t\t\t\telse\n+\t\t\t\t\tprintf(\"%s\\n\", v);\n+\t\t\t}\n+\t\t\tgoto exit0;\n+\t\t} else {\n+\t\t\tprintf(\"Value not found for \\\"%s\\\"\\n\", argv[2]);\n+\t\t\tgoto exit1;\n+\t\t}\n+\t} else if (argc == 3 && !strcmp(argv[1], \"get_int\")) {\n+\t\tif (!git_config_get_int(argv[2], &val)) {\n+\t\t\tprintf(\"%d\\n\", val);\n+\t\t\tgoto exit0;\n+\t\t} else {\n+\t\t\tprintf(\"Value not found for \\\"%s\\\"\\n\", argv[2]);\n+\t\t\tgoto exit1;\n+\t\t}\n+\t} else if (argc == 3 && !strcmp(argv[1], \"get_bool\")) {\n+\t\tif (!git_config_get_bool(argv[2], &val)) {\n+\t\t\tprintf(\"%d\\n\", val);\n+\t\t\tgoto exit0;\n+\t\t} else {\n+\t\t\tprintf(\"Value not found for \\\"%s\\\"\\n\", argv[2]);\n+\t\t\tgoto exit1;\n+\t\t}\n+\t} else if (!strcmp(argv[1], \"configset_get_value\")) {\n+\t\tfor (i = 3; i < argc; i++) {\n+\t\t\tif (git_configset_add_file(&cs, argv[i])) {\n+\t\t\t\tfprintf(stderr, \"Error reading configuration file %s.\\n\", argv[i]);\n+\t\t\t\tgoto exit2;\n+\t\t\t}\n+\t\t}\n+\t\tif (!git_configset_get_value(&cs, argv[2], &v)) {\n+\t\t\tif (!v)\n+\t\t\t\tprintf(\"(NULL)\\n\");\n+\t\t\telse\n+\t\t\t\tprintf(\"%s\\n\", v);\n+\t\t\tgoto exit0;\n+\t\t} else {\n+\t\t\tprintf(\"Value not found for \\\"%s\\\"\\n\", argv[2]);\n+\t\t\tgoto exit1;\n+\t\t}\n+\t} else if (!strcmp(argv[1], \"configset_get_value_multi\")) {\n+\t\tfor (i = 3; i < argc; i++) {\n+\t\t\tif (git_configset_add_file(&cs, argv[i])) {\n+\t\t\t\tfprintf(stderr, \"Error reading configuration file %s.\\n\", argv[i]);\n+\t\t\t\tgoto exit2;\n+\t\t\t}\n+\t\t}\n+\t\tstrptr = git_configset_get_value_multi(&cs, argv[2]);\n+\t\tif (strptr) {\n+\t\t\tfor (i = 0; i < strptr->nr; i++) {\n+\t\t\t\tv = strptr->items[i].string;\n+\t\t\t\tif (!v)\n+\t\t\t\t\tprintf(\"(NULL)\\n\");\n+\t\t\t\telse\n+\t\t\t\t\tprintf(\"%s\\n\", v);\n+\t\t\t}\n+\t\t\tgoto exit0;\n+\t\t} else {\n+\t\t\tprintf(\"Value not found for \\\"%s\\\"\\n\", argv[2]);\n+\t\t\tgoto exit1;\n+\t\t}\n+\t}\n+\n+\tdie(\"%s: Please check the syntax and the function name\", argv[0]);\n+\n+exit0:\n+\tgit_configset_clear(&cs);\n+\treturn 0;\n+\n+exit1:\n+\tgit_configset_clear(&cs);\n+\treturn 1;\n+\n+exit2:\n+\tgit_configset_clear(&cs);\n+\treturn 2;\n+}\n-- \n1.9.0.GIT\n"},{"id":"246060","messageId":"xmqqoawqwrov.fsf@gitster.dls.corp.google.com","threadId":"37130","inReplyTo":"1405434571-25459-2-git-send-email-tanayabh@gmail.com","subject":"Re: [PATCH v9 1/2] add `config_set` API for caching config-like files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-07-15T15:46:24Z","receivedAt":"2014-07-15T15:46:24Z","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> diff --git a/config.c b/config.c\n> index ba882a1..89e2d67 100644\n> --- a/config.c\n> +++ b/config.c\n> ...\n> +const struct string_list *git_configset_get_value_multi(struct config_set *cs, const char *key)\n> +{\n> +\tstruct config_set_element *e = configset_find_element(cs, key);\n> +\treturn e ? &e->value_list : NULL;\n> +}\n\nSomehow I find the placement of this function out of place.  Didn't\nyou get the same impression while proofreading the entire file after\nyou are done writing this patch?\n\nThe right place for it would probably be immediately before\ngit_configset_get_value(), I would think.\n\n> +int git_configset_add_file(struct config_set *cs, const char *filename)\n> +{\n> +\tint ret = 0;\n> +\tret = git_config_from_file(config_hash_callback, filename, cs);\n> +\tif (!ret)\n> +\t\treturn 0;\n> +\telse {\n> +\t\tgit_configset_clear(cs);\n> +\t\treturn -1;\n> +\t}\n> +}\n\nIf I add contents from file \"A\" successfully and then attempt to\nfurther add contents from file \"B\" which fails, I would lose\ncontents I read from \"A\"?\n\nIt would not be worth trying to make it transactional (i.e. making\nsure cs contains what was there before the config-from-file was\ncalled if it failed), so in such a case we may end up seeing a\nmixture of full contents from A and partial contents from B, which\nis just as useless as a cleared configset, so it is not wrong to\nclear it per-se.  An alternative might be to return without\nclearing, as we are leaving the configset in a useless state either\nway and give the caller a choice between clearing it and continue,\nand dying without even spending unnecessary cycles to clear.  That\nmight be more consistent with what git_config_check_init() does,\nwhere you die without bothering to clear what was half-read into the\nconfigset.\n\nWhatever behaviour is chosen in the error codepath, it needs to be\ndocumented.\n\nThanks.\n"},{"id":"246061","messageId":"xmqqk37ewr5r.fsf@gitster.dls.corp.google.com","threadId":"37130","inReplyTo":"1405434571-25459-3-git-send-email-tanayabh@gmail.com","subject":"Re: [PATCH v9 2/2] test-config: add tests for the config_set API","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-07-15T15:57:52Z","receivedAt":"2014-07-15T15:57:52Z","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> Expose the `config_set` C API as a set of simple commands in order to\n> facilitate testing. Add tests for the `config_set` API as well as for\n> `git_config_get_*()` family for the usual config files.\n>\n> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n> Signed-off-by: Tanay Abhra <tanayabh@gmail.com>\n> ---\n>  .gitignore            |   1 +\n>  Makefile              |   1 +\n>  t/t1308-config-set.sh | 212 ++++++++++++++++++++++++++++++++++++++++++++++++++\n>  test-config.c         | 140 +++++++++++++++++++++++++++++++++\n>  4 files changed, 354 insertions(+)\n>  create mode 100755 t/t1308-config-set.sh\n>  create mode 100644 test-config.c\n>\n> diff --git a/.gitignore b/.gitignore\n> index 42294e5..7677533 100644\n> --- a/.gitignore\n> +++ b/.gitignore\n> @@ -177,6 +177,7 @@\n>  /gitweb/static/gitweb.min.*\n>  /test-chmtime\n>  /test-ctype\n> +/test-config\n>  /test-date\n>  /test-delta\n>  /test-dump-cache-tree\n> diff --git a/Makefile b/Makefile\n> index b92418d..e070eb8 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -549,6 +549,7 @@ PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))\n>  \n>  TEST_PROGRAMS_NEED_X += test-chmtime\n>  TEST_PROGRAMS_NEED_X += test-ctype\n> +TEST_PROGRAMS_NEED_X += test-config\n>  TEST_PROGRAMS_NEED_X += test-date\n>  TEST_PROGRAMS_NEED_X += test-delta\n>  TEST_PROGRAMS_NEED_X += test-dump-cache-tree\n> diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh\n> new file mode 100755\n> index 0000000..94085eb\n> --- /dev/null\n> +++ b/t/t1308-config-set.sh\n> @@ -0,0 +1,212 @@\n> +#!/bin/sh\n> +\n> +test_description='Test git config-set API in different settings'\n> +\n> +. ./test-lib.sh\n> +\n> +# 'check_config get_* section.key value' verifies that the entry for\n> +# section.key is 'value'\n> +check_config () {\n> +\tcase \"$1\" in\n> +\texpect_code)\n> +\t\tif [ \"$#\" -lt 5 ];\n> +\t\tthen\n\nSpell out \"test\" and drop unnecessary semicolon, i.e.\n\n\tif test $# -lt 5\n        then\n\n> +\t\t\t>expect\n> +\t\telse\n> +\t\t\tprintf \"%s\\n\" \"$5\" >expect\n\nThe other \"expecting success\" side of this function allows to expect\nmore than one line of output, but this one only allows you to expect\nat most one line?  Why?\n\n> +\t\tfi &&\n> +\t\ttest_expect_code \"$2\" test-config \"$3\" \"$4\" >actual\n> +\t\t;;\n> +\t*)\n> +\t\top=$1 key=$2 &&\n> +\t\tshift &&\n> +\t\tshift &&\n> +\t\tprintf \"%s\\n\" \"$@\" >expect &&\n> +\t\ttest-config \"$op\" \"$key\" >actual\n> +\t\t;;\n> +\tesac &&\n> +\ttest_cmp expect actual\n\nPerhaps you meant to say something like this instead?\n\n        if test \"$1\" = expect_code\n        then\n        \texpect_code=\"$2\" && shift && shift\n\telse\n        \texpect_code=0\n\tfi &&\n\top=$1 key=$2 && shift && shift\n        if test $# != 0\n        then\n        \tprintf \"%s\\n\" \"$@\"\n\tfi >expect &&\n        test_expect_code $expet_code test-config \"$op\" \"$key\" >actual &&\n\ttest_cmp expect actual\n\nI dunno.\n\n> +test_expect_success 'setup default config' '\n> +\tcat >.git/config <<\\EOF\n\nSo the default .git/config that was prepared by \"git init\" is\ndiscarded and replaced with this?  Shouldn't it be\n\n\tcat >>.git/config <<\\EOF\n\ninstead?\n\n> +test_expect_success 'find multiple values' '\n> +\tcat >expect <<-\\EOF &&\n> +\tsam\n> +\tbat\n> +\thask\n> +\tEOF\n> +\ttest-config get_value_multi \"case.baz\">actual &&\n> +\ttest_cmp expect actual\n> +'\n\nHmmm, wasn't the whole point of the helper to allow us to make\nthings like the above into a one-liner, perhaps like this?\n\n      check_config get_value_multi case.baz sam bat hask\n\nI suspect the same applies to most if not all uses of test-config\nin the remainder of this patch.\n"},{"id":"246063","messageId":"53C5553A.2090700@gmail.com","threadId":"37130","inReplyTo":"xmqqoawqwrov.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v9 1/2] add `config_set` API for caching config-like files","fromName":"Tanay Abhra","fromEmail":"tanayabh@gmail.com","sentAt":"2014-07-15T16:22:18Z","receivedAt":"2014-07-15T16:22:18Z","isPatch":true,"sender":{"key":"tanayabh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2097229?v=4"},"body":"\n\nOn 7/15/2014 9:16 PM, Junio C Hamano wrote:\n> Tanay Abhra <tanayabh@gmail.com> writes:\n> \n>> diff --git a/config.c b/config.c\n>> index ba882a1..89e2d67 100644\n>> --- a/config.c\n>> +++ b/config.c\n>> ...\n>> +const struct string_list *git_configset_get_value_multi(struct config_set *cs, const char *key)\n>> +{\n>> +\tstruct config_set_element *e = configset_find_element(cs, key);\n>> +\treturn e ? &e->value_list : NULL;\n>> +}\n> \n> Somehow I find the placement of this function out of place.  Didn't\n> you get the same impression while proofreading the entire file after\n> you are done writing this patch?\n> \n> The right place for it would probably be immediately before\n> git_configset_get_value(), I would think.\n>\n\nNoted. My fault, will correct in the reroll.\n\n>> +int git_configset_add_file(struct config_set *cs, const char *filename)\n>> +{\n>> +\tint ret = 0;\n>> +\tret = git_config_from_file(config_hash_callback, filename, cs);\n>> +\tif (!ret)\n>> +\t\treturn 0;\n>> +\telse {\n>> +\t\tgit_configset_clear(cs);\n>> +\t\treturn -1;\n>> +\t}\n>> +}\n> \n> If I add contents from file \"A\" successfully and then attempt to\n> further add contents from file \"B\" which fails, I would lose\n> contents I read from \"A\"?\n> \n> It would not be worth trying to make it transactional (i.e. making\n> sure cs contains what was there before the config-from-file was\n> called if it failed), so in such a case we may end up seeing a\n> mixture of full contents from A and partial contents from B, which\n> is just as useless as a cleared configset, so it is not wrong to\n> clear it per-se.  An alternative might be to return without\n> clearing, as we are leaving the configset in a useless state either\n> way and give the caller a choice between clearing it and continue,\n> and dying without even spending unnecessary cycles to clear.  That\n> might be more consistent with what git_config_check_init() does,\n> where you die without bothering to clear what was half-read into the\n> configset.\n> \n> Whatever behaviour is chosen in the error codepath, it needs to be\n> documented.\n>\n\nSince syntactical errors in reading the file will cause a die when we\nuse either `git_config_from_file` or `git_config`, I don't think\nincomplete parsing would trigger the error path, as it will die before\nreaching there.\n\nSo, the best way would be just to return without clearing and let the\nuser take the decision if he wants to go on with the incomplete\nconfig_set or clear it when he sees that configset_add_file() failed.\n\nDie in git_config_check_init() is never triggered expect in rare race\nconditions like between access_or_die() and git_config_from_file() in\n\n\tif (git_config_system() && !access_or_die(git_etc_gitconfig(), R_OK, 0)) {\n\t\tret += git_config_from_file(fn, git_etc_gitconfig(),\n\t\t\t\t\t    data);\n\t\tfound += 1;\n\t}\nwhich I think would never happen in reality, dunno. I think I will take\nout the die() in git_config_check_init(). Thus, the behavior of both\nfunctions git_config_check_init() & git_configset_add_file will be consistent.\n\nThanks.\n"},{"id":"246068","messageId":"53C55FB7.7060405@gmail.com","threadId":"37130","inReplyTo":"xmqqk37ewr5r.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v9 2/2] test-config: add tests for the config_set API","fromName":"Tanay Abhra","fromEmail":"tanayabh@gmail.com","sentAt":"2014-07-15T17:07:03Z","receivedAt":"2014-07-15T17:07:03Z","isPatch":true,"sender":{"key":"tanayabh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2097229?v=4"},"body":"\n\nOn 7/15/2014 9:27 PM, Junio C Hamano wrote:\n>> +test_expect_success 'setup default config' '\n>> +\tcat >.git/config <<\\EOF\n> \n> So the default .git/config that was prepared by \"git init\" is\n> discarded and replaced with this?  Shouldn't it be\n> \n> \tcat >>.git/config <<\\EOF\n> \n> instead?\n>\n\nMost of tests like t1300-repo-config.sh or t1303-wacky-config.sh\nclears the default config, will it be okay to clear it in\nthis test series also? I need it for\ntest_expect_success 'proper error on error in default config files' '\nwhich requires me to compare the line no at which the error was found.\n\n>> +test_expect_success 'find multiple values' '\n>> +\tcat >expect <<-\\EOF &&\n>> +\tsam\n>> +\tbat\n>> +\thask\n>> +\tEOF\n>> +\ttest-config get_value_multi \"case.baz\">actual &&\n>> +\ttest_cmp expect actual\n>> +'\n> \n> Hmmm, wasn't the whole point of the helper to allow us to make\n> things like the above into a one-liner, perhaps like this?\n> \n>       check_config get_value_multi case.baz sam bat hask\n\nNoted and corrected.\n\n> I suspect the same applies to most if not all uses of test-config\n> in the remainder of this patch.\n>\n\nI cant use it for configset_get_value_* as it may have variable number\nof files as arguments. :)\n\nThanks,\nTanay Abhra.\n"},{"id":"246074","messageId":"xmqqy4vuv5pe.fsf@gitster.dls.corp.google.com","threadId":"37130","inReplyTo":"53C55FB7.7060405@gmail.com","subject":"Re: [PATCH v9 2/2] test-config: add tests for the config_set API","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-07-15T18:26:37Z","receivedAt":"2014-07-15T18:26:37Z","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> ... I need it for\n> test_expect_success 'proper error on error in default config files' '\n> which requires me to compare the line no at which the error was found.\n\nI see.  We may want to later rethink the way you validate the line\nnumbers, but in the meantime I think the version posted should\nsuffice, if you cannot do better than that.\n\nThanks.\n"},{"id":"246157","messageId":"53C664F7.9060108@gmail.com","threadId":"37130","inReplyTo":"xmqqoawqwrov.fsf@gitster.dls.corp.google.com","subject":"[PATCH v9r1 1/2] add `config_set` API for caching config-like files","fromName":"Tanay Abhra","fromEmail":"tanayabh@gmail.com","sentAt":"2014-07-16T11:41:43Z","receivedAt":"2014-07-16T11:41:43Z","isPatch":true,"sender":{"key":"tanayabh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2097229?v=4"},"body":"Currently `git_config()` uses a callback mechanism and file rereads for\nconfig values. Due to this approach, it is not uncommon for the config\nfiles to be parsed several times during the run of a git program, with\ndifferent callbacks picking out different variables useful to themselves.\n\nAdd a `config_set`, that can be used to construct an in-memory cache for\nconfig-like files that the caller specifies (i.e., files like `.gitmodules`,\n`~/.gitconfig` etc.). Add two external functions `git_configset_get_value`\nand `git_configset_get_value_multi` for querying from the config sets.\n`git_configset_get_value` follows `last one wins` semantic (i.e. if there\nare multiple matches for the queried key in the files of the configset the\nvalue returned will be the last entry in `value_list`).\n`git_configset_get_value_multi` returns a list of values sorted in order of\nincreasing priority (i.e. last match will be at the end of the list). Add\ntype specific query functions like `git_configset_get_bool` and similar.\n\nAdd a default `config_set`, `the_config_set` to cache all key-value pairs\nread from usual config files (repo specific .git/config, user wide\n~/.gitconfig, XDG config and the global /etc/gitconfig). `the_config_set`\nis populated using `git_config()`.\n\nAdd two external functions `git_config_get_value` and\n`git_config_get_value_multi` for querying in a non-callback manner from\n`the_config_set`. Also, add type specific query functions that are\nimplemented as a thin wrapper around the `config_set` API.\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\nSigned-off-by: Tanay Abhra <tanayabh@gmail.com>\n---\n Documentation/technical/api-config.txt | 137 +++++++++++++++++\n cache.h                                |  30 ++++\n config.c                               | 264 +++++++++++++++++++++++++++++++++\n 3 files changed, 431 insertions(+)\n\ndiff --git a/Documentation/technical/api-config.txt b/Documentation/technical/api-config.txt\nindex 230b3a0..fc0e379 100644\n--- a/Documentation/technical/api-config.txt\n+++ b/Documentation/technical/api-config.txt\n@@ -77,6 +77,81 @@ 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_value`\n+and `git_config_get_value_multi`. They both read values from an internal\n+cache generated previously from reading the config files.\n+\n+`int git_config_get_value(const char *key, const char **value)`::\n+\n+\tFinds the highest-priority value for the configuration variable `key`,\n+\tstores the pointer to it in `value` and returns 0. When the\n+\tconfiguration variable `key` is not found, returns 1 without touching\n+\t`value`. The caller should not free or modify `value`, as it is owned\n+\tby the cache.\n+\n+`const struct string_list *git_config_get_value_multi(const char *key)`::\n+\n+\tFinds and returns the value list, sorted in order of increasing priority\n+\tfor the configuration variable `key`. When the configuration variable\n+\t`key` is not found, returns NULL. The caller should not free or modify\n+\tthe returned pointer, as it is owned by the cache.\n+\n+`void git_config_clear(void)`::\n+\n+\tResets and invalidates the config cache.\n+\n+The config API also provides type specific API functions which do conversion\n+as well as retrieval for the queried variable, including:\n+\n+`int git_config_get_int(const char *key, int *dest)`::\n+\n+\tFinds and parses the value to an integer for the configuration variable\n+\t`key`. Dies on error; otherwise, stores the value of the parsed integer in\n+\t`dest` and returns 0. When the configuration variable `key` is not found,\n+\treturns 1 without touching `dest`.\n+\n+`int git_config_get_ulong(const char *key, unsigned long *dest)`::\n+\n+\tSimilar to `git_config_get_int` but for unsigned longs.\n+\n+`int git_config_get_bool(const char *key, int *dest)`::\n+\n+\tFinds and parses the value into a boolean value, for the configuration\n+\tvariable `key` respecting keywords like \"true\" and \"false\". Integer\n+\tvalues are converted into true/false values (when they are non-zero or\n+\tzero, respectively). Other values cause a die(). If parsing is successful,\n+\tstores the value of the parsed result in `dest` and returns 0. When the\n+\tconfiguration variable `key` is not found, returns 1 without touching\n+\t`dest`.\n+\n+`int git_config_get_bool_or_int(const char *key, int *is_bool, int *dest)`::\n+\n+\tSimilar to `git_config_get_bool`, except that integers are copied as-is,\n+\tand `is_bool` flag is unset.\n+\n+`int git_config_get_maybe_bool(const char *key, int *dest)`::\n+\n+\tSimilar to `git_config_get_bool`, except that it returns -1 on error\n+\trather than dying.\n+\n+`int git_config_get_string(const char *key, const char **dest)`::\n+\n+\tAllocates and copies the retrieved string into the `dest` parameter for\n+\tthe configuration variable `key`; if NULL string is given, prints an\n+\terror message and returns -1. When the configuration variable `key` is\n+\tnot found, returns 1 without touching `dest`.\n+\n+`int git_config_get_pathname(const char *key, const char **dest)`::\n+\n+\tSimilar to `git_config_get_string`, but expands `~` or `~user` into\n+\tthe user's home directory when found at the beginning of the path.\n+\n+See test-config.c for usage examples.\n+\n Value Parsing Helpers\n ---------------------\n\n@@ -134,6 +209,68 @@ int read_file_with_include(const char *file, config_fn_t fn, void *data)\n `git_config` respects includes automatically. The lower-level\n `git_config_from_file` does not.\n\n+Custom Configsets\n+-----------------\n+\n+A `config_set` can be used to construct an in-memory cache for\n+config-like files that the caller specifies (i.e., files like `.gitmodules`,\n+`~/.gitconfig` etc.). For example,\n+\n+---------------------------------------\n+struct config_set gm_config;\n+git_configset_init(&gm_config);\n+int b;\n+/* we add config files to the config_set */\n+git_configset_add_file(&gm_config, \".gitmodules\");\n+git_configset_add_file(&gm_config, \".gitmodules_alt\");\n+\n+if (!git_configset_get_bool(gm_config, \"submodule.frotz.ignore\", &b)) {\n+\t/* hack hack hack */\n+}\n+\n+/* when we are done with the configset */\n+git_configset_clear(&gm_config);\n+----------------------------------------\n+\n+Configset API provides functions for the above mentioned work flow, including:\n+\n+`void git_configset_init(struct config_set *cs)`::\n+\n+\tInitializes the config_set `cs`.\n+\n+`int git_configset_add_file(struct config_set *cs, const char *filename)`::\n+\n+\tParses the file and adds the variable-value pairs to the `config_set`,\n+\tdies if there is an error in parsing the file. Returns 0 on success, or\n+\t-1 if the file doesnot exist or is inaccessible. The user has to decide\n+\tif he wants to free the incomplete configset or continue using it when\n+\tthe function returns -1.\n+\n+`int git_configset_get_value(struct config_set *cs, const char *key, const char **value)`::\n+\n+\tFinds the highest-priority value for the configuration variable `key`\n+\tand config set `cs`, stores the pointer to it in `value` and returns 0.\n+\tWhen the configuration variable `key` is not found, returns 1 without\n+\ttouching `value`. The caller should not free or modify `value`, as it\n+\tis owned by the cache.\n+\n+`const struct string_list *git_configset_get_value_multi(struct config_set *cs, const char *key)`::\n+\n+\tFinds and returns the value list, sorted in order of increasing priority\n+\tfor the configuration variable `key` and config set `cs`. When the\n+\tconfiguration variable `key` is not found, returns NULL. The caller\n+\tshould not free or modify the returned pointer, as it is owned by the cache.\n+\n+`void git_configset_clear(struct config_set *cs)`::\n+\n+\tClears `config_set` structure, removes all saved variable-value pairs.\n+\n+In addition to above functions, the `config_set` API provides type specific\n+functions in the vein of `git_config_get_int` and family but with an extra\n+parameter, pointer to struct `config_set`.\n+They all behave similarly to the `git_config_get*()` family described in\n+\"Querying For Specific Variables\" above.\n+\n Writing Config Files\n --------------------\n\ndiff --git a/cache.h b/cache.h\nindex 44aa439..c67639d 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1329,6 +1329,36 @@ extern int parse_config_key(const char *var,\n \t\t\t    const char **subsection, int *subsection_len,\n \t\t\t    const char **key);\n\n+struct config_set {\n+\tstruct hashmap config_hash;\n+\tint hash_initialized;\n+};\n+\n+extern void git_configset_init(struct config_set *cs);\n+extern int git_configset_add_file(struct config_set *cs, const char *filename);\n+extern int git_configset_get_value(struct config_set *cs, const char *key, const char **value);\n+extern const struct string_list *git_configset_get_value_multi(struct config_set *cs, const char *key);\n+extern void git_configset_clear(struct config_set *cs);\n+extern int git_configset_get_string(struct config_set *cs, const char *key, const char **dest);\n+extern int git_configset_get_int(struct config_set *cs, const char *key, int *dest);\n+extern int git_configset_get_ulong(struct config_set *cs, const char *key, unsigned long *dest);\n+extern int git_configset_get_bool(struct config_set *cs, const char *key, int *dest);\n+extern int git_configset_get_bool_or_int(struct config_set *cs, const char *key, int *is_bool, int *dest);\n+extern int git_configset_get_maybe_bool(struct config_set *cs, const char *key, int *dest);\n+extern int git_configset_get_pathname(struct config_set *cs, const char *key, const char **dest);\n+\n+extern int git_config_get_value(const char *key, const char **value);\n+extern const struct string_list *git_config_get_value_multi(const char *key);\n+extern void git_config_clear(void);\n+extern void git_config_iter(config_fn_t fn, void *data);\n+extern int git_config_get_string(const char *key, const char **dest);\n+extern int git_config_get_int(const char *key, int *dest);\n+extern int git_config_get_ulong(const char *key, unsigned long *dest);\n+extern int git_config_get_bool(const char *key, int *dest);\n+extern int git_config_get_bool_or_int(const char *key, int *is_bool, int *dest);\n+extern int git_config_get_maybe_bool(const char *key, int *dest);\n+extern int git_config_get_pathname(const char *key, const char **dest);\n+\n extern int committer_ident_sufficiently_given(void);\n extern int author_ident_sufficiently_given(void);\n\ndiff --git a/config.c b/config.c\nindex ba882a1..d14f761 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@@ -33,10 +35,23 @@ struct config_source {\n \tlong (*do_ftell)(struct config_source *c);\n };\n\n+struct config_set_element {\n+\tstruct hashmap_entry ent;\n+\tchar *key;\n+\tstruct string_list value_list;\n+};\n+\n static struct config_source *cf;\n\n static int zlib_compression_seen;\n\n+/*\n+ * Default config_set that contains key-value pairs from the usual set of config\n+ * config files (i.e repo specific .git/config, user wide ~/.gitconfig, XDG\n+ * config file and the global /etc/gitconfig)\n+ */\n+static struct config_set the_config_set;\n+\n static int config_file_fgetc(struct config_source *conf)\n {\n \treturn fgetc(conf->u.file);\n@@ -1212,6 +1227,252 @@ int git_config(config_fn_t fn, void *data)\n \treturn git_config_with_options(fn, data, NULL, 1);\n }\n\n+static struct config_set_element *configset_find_element(struct config_set *cs, const char *key)\n+{\n+\tstruct config_set_element k;\n+\tstruct config_set_element *found_entry;\n+\tchar *normalized_key;\n+\tint ret;\n+\t/*\n+\t * `key` may come from the user, so normalize it before using it\n+\t * for querying entries from the hashmap.\n+\t */\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(&cs->config_hash, &k, NULL);\n+\tfree(normalized_key);\n+\treturn found_entry;\n+}\n+\n+static int configset_add_value(struct config_set *cs, const char *key, const char *value)\n+{\n+\tstruct config_set_element *e;\n+\te = configset_find_element(cs, key);\n+\t/*\n+\t * Since the keys are being fed by git_config*() callback mechanism, they\n+\t * are already normalized. So simply add them without any further munging.\n+\t */\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\tmemset(&e->value_list, 0, sizeof(e->value_list));\n+\t\te->value_list.strdup_strings = 1;\n+\t\thashmap_add(&cs->config_hash, e);\n+\t}\n+\tstring_list_append_nodup(&e->value_list, value ? xstrdup(value) : NULL);\n+\n+\treturn 0;\n+}\n+\n+static int config_set_element_cmp(const struct config_set_element *e1,\n+\t\t\t\t const struct config_set_element *e2, const void *unused)\n+{\n+\treturn strcmp(e1->key, e2->key);\n+}\n+\n+void git_configset_init(struct config_set *cs)\n+{\n+\thashmap_init(&cs->config_hash, (hashmap_cmp_fn)config_set_element_cmp, 0);\n+\tcs->hash_initialized = 1;\n+}\n+\n+void git_configset_clear(struct config_set *cs)\n+{\n+\tstruct config_set_element *entry;\n+\tstruct hashmap_iter iter;\n+\tif (!cs->hash_initialized)\n+\t\treturn;\n+\n+\thashmap_iter_init(&cs->config_hash, &iter);\n+\twhile ((entry = hashmap_iter_next(&iter))) {\n+\t\tfree(entry->key);\n+\t\tstring_list_clear(&entry->value_list, 0);\n+\t}\n+\thashmap_free(&cs->config_hash, 1);\n+\tcs->hash_initialized = 0;\n+}\n+\n+static int config_hash_callback(const char *key, const char *value, void *cb)\n+{\n+\tstruct config_set *cs = cb;\n+\tconfigset_add_value(cs, key, value);\n+\treturn 0;\n+}\n+\n+int git_configset_add_file(struct config_set *cs, const char *filename)\n+{\n+\treturn git_config_from_file(config_hash_callback, filename, cs);\n+}\n+\n+int git_configset_get_value(struct config_set *cs, const char *key, const char **value)\n+{\n+\tconst struct string_list *values = NULL;\n+\t/*\n+\t * Follows \"last one wins\" semantic, i.e., if there are multiple matches for the\n+\t * queried key in the files of the configset, the value returned will be the last\n+\t * value in the value list for that key.\n+\t */\n+\tvalues = git_configset_get_value_multi(cs, key);\n+\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+const struct string_list *git_configset_get_value_multi(struct config_set *cs, const char *key)\n+{\n+\tstruct config_set_element *e = configset_find_element(cs, key);\n+\treturn e ? &e->value_list : NULL;\n+}\n+\n+int git_configset_get_string(struct config_set *cs, const char *key, const char **dest)\n+{\n+\tconst char *value;\n+\tif (!git_configset_get_value(cs, key, &value))\n+\t\treturn git_config_string(dest, key, value);\n+\telse\n+\t\treturn 1;\n+}\n+\n+int git_configset_get_int(struct config_set *cs, const char *key, int *dest)\n+{\n+\tconst char *value;\n+\tif (!git_configset_get_value(cs, key, &value)) {\n+\t\t*dest = git_config_int(key, value);\n+\t\treturn 0;\n+\t} else\n+\t\treturn 1;\n+}\n+\n+int git_configset_get_ulong(struct config_set *cs, const char *key, unsigned long *dest)\n+{\n+\tconst char *value;\n+\tif (!git_configset_get_value(cs, key, &value)) {\n+\t\t*dest = git_config_ulong(key, value);\n+\t\treturn 0;\n+\t} else\n+\t\treturn 1;\n+}\n+\n+int git_configset_get_bool(struct config_set *cs, const char *key, int *dest)\n+{\n+\tconst char *value;\n+\tif (!git_configset_get_value(cs, key, &value)) {\n+\t\t*dest = git_config_bool(key, value);\n+\t\treturn 0;\n+\t} else\n+\t\treturn 1;\n+}\n+\n+int git_configset_get_bool_or_int(struct config_set *cs, const char *key,\n+\t\t\t\tint *is_bool, int *dest)\n+{\n+\tconst char *value;\n+\tif (!git_configset_get_value(cs, key, &value)) {\n+\t\t*dest = git_config_bool_or_int(key, value, is_bool);\n+\t\treturn 0;\n+\t} else\n+\t\treturn 1;\n+}\n+\n+int git_configset_get_maybe_bool(struct config_set *cs, const char *key, int *dest)\n+{\n+\tconst char *value;\n+\tif (!git_configset_get_value(cs, key, &value)) {\n+\t\t*dest = git_config_maybe_bool(key, value);\n+\t\tif (*dest == -1)\n+\t\t\treturn -1;\n+\t\treturn 0;\n+\t} else\n+\t\treturn 1;\n+}\n+\n+int git_configset_get_pathname(struct config_set *cs, const char *key, const char **dest)\n+{\n+\tconst char *value;\n+\tif (!git_configset_get_value(cs, key, &value))\n+\t\treturn git_config_pathname(dest, key, value);\n+\telse\n+\t\treturn 1;\n+}\n+\n+static void git_config_check_init(void)\n+{\n+\tif (the_config_set.hash_initialized)\n+\t\treturn;\n+\tgit_configset_init(&the_config_set);\n+\tgit_config(config_hash_callback, &the_config_set);\n+}\n+\n+void git_config_clear(void)\n+{\n+\tif (!the_config_set.hash_initialized)\n+\t\treturn;\n+\tgit_configset_clear(&the_config_set);\n+}\n+\n+int git_config_get_value(const char *key, const char **value)\n+{\n+\tgit_config_check_init();\n+\treturn git_configset_get_value(&the_config_set, key, value);\n+}\n+\n+const struct string_list *git_config_get_value_multi(const char *key)\n+{\n+\tgit_config_check_init();\n+\treturn git_configset_get_value_multi(&the_config_set, key);\n+}\n+\n+int git_config_get_string(const char *key, const char **dest)\n+{\n+\tgit_config_check_init();\n+\treturn git_configset_get_string(&the_config_set, key, dest);\n+}\n+\n+int git_config_get_int(const char *key, int *dest)\n+{\n+\tgit_config_check_init();\n+\treturn git_configset_get_int(&the_config_set, key, dest);\n+}\n+\n+int git_config_get_ulong(const char *key, unsigned long *dest)\n+{\n+\tgit_config_check_init();\n+\treturn git_configset_get_ulong(&the_config_set, key, dest);\n+}\n+\n+int git_config_get_bool(const char *key, int *dest)\n+{\n+\tgit_config_check_init();\n+\treturn git_configset_get_bool(&the_config_set, key, dest);\n+}\n+\n+int git_config_get_bool_or_int(const char *key, int *is_bool, int *dest)\n+{\n+\tgit_config_check_init();\n+\treturn git_configset_get_bool_or_int(&the_config_set, key, is_bool, dest);\n+}\n+\n+int git_config_get_maybe_bool(const char *key, int *dest)\n+{\n+\tgit_config_check_init();\n+\treturn git_configset_get_maybe_bool(&the_config_set, key, dest);\n+}\n+\n+int git_config_get_pathname(const char *key, const char **dest)\n+{\n+\tgit_config_check_init();\n+\treturn git_configset_get_pathname(&the_config_set, key, dest);\n+}\n+\n /*\n  * Find all the stuff for git_config_set() below.\n  */\n@@ -1707,6 +1968,9 @@ int git_config_set_multivar_in_file(const char *config_filename,\n \tlock = NULL;\n \tret = 0;\n\n+\t/* Invalidate the config cache */\n+\tgit_config_clear();\n+\n out_free:\n \tif (lock)\n \t\trollback_lock_file(lock);\n-- \n1.9.0.GIT\n"},{"id":"246158","messageId":"53C665A0.2060705@gmail.com","threadId":"37130","inReplyTo":"xmqqk37ewr5r.fsf@gitster.dls.corp.google.com","subject":"[PATCH v9r1 2/2] test-config: add tests for the config_set API","fromName":"Tanay Abhra","fromEmail":"tanayabh@gmail.com","sentAt":"2014-07-16T11:44:32Z","receivedAt":"2014-07-16T11:44:32Z","isPatch":true,"sender":{"key":"tanayabh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2097229?v=4"},"body":"Expose the `config_set` C API as a set of simple commands in order to\nfacilitate testing. Add tests for the `config_set` API as well as for\n`git_config_get_*()` family for the usual config files.\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\nSigned-off-by: Tanay Abhra <tanayabh@gmail.com>\n---\n .gitignore            |   1 +\n Makefile              |   1 +\n t/t1308-config-set.sh | 200 ++++++++++++++++++++++++++++++++++++++++++++++++++\n test-config.c         | 140 +++++++++++++++++++++++++++++++++++\n 4 files changed, 342 insertions(+)\n create mode 100755 t/t1308-config-set.sh\n create mode 100644 test-config.c\n\ndiff --git a/.gitignore b/.gitignore\nindex 42294e5..7677533 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -177,6 +177,7 @@\n /gitweb/static/gitweb.min.*\n /test-chmtime\n /test-ctype\n+/test-config\n /test-date\n /test-delta\n /test-dump-cache-tree\ndiff --git a/Makefile b/Makefile\nindex b92418d..e070eb8 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -549,6 +549,7 @@ PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))\n\n TEST_PROGRAMS_NEED_X += test-chmtime\n TEST_PROGRAMS_NEED_X += test-ctype\n+TEST_PROGRAMS_NEED_X += test-config\n TEST_PROGRAMS_NEED_X += test-date\n TEST_PROGRAMS_NEED_X += test-delta\n TEST_PROGRAMS_NEED_X += test-dump-cache-tree\ndiff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh\nnew file mode 100755\nindex 0000000..cf8f2d7\n--- /dev/null\n+++ b/t/t1308-config-set.sh\n@@ -0,0 +1,200 @@\n+#!/bin/sh\n+\n+test_description='Test git config-set API in different settings'\n+\n+. ./test-lib.sh\n+\n+# 'check_config get_* section.key value' verifies that the entry for\n+# section.key is 'value'\n+check_config () {\n+\tif test \"$1\" = expect_code\n+\tthen\n+\t\texpect_code=\"$2\" && shift && shift\n+\telse\n+\t\texpect_code=0\n+\tfi &&\n+\top=$1 key=$2 && shift && shift\n+\tif test $# != 0\n+\tthen\n+\t\tprintf \"%s\\n\" \"$@\"\n+\tfi >expect &&\n+\ttest_expect_code $expect_code test-config \"$op\" \"$key\" >actual &&\n+\ttest_cmp expect actual\n+}\n+\n+test_expect_success 'setup default config' '\n+\tcat >.git/config <<\\EOF\n+\t[case]\n+\t\tpenguin = very blue\n+\t\tMovie = BadPhysics\n+\t\tUPPERCASE = true\n+\t\tMixedCase = true\n+\t\tmy =\n+\t\tfoo\n+\t\tbaz = sam\n+\t[Cores]\n+\t\tWhatEver = Second\n+\t\tbaz = bar\n+\t[cores]\n+\t\tbaz = bat\n+\t[CORES]\n+\t\tbaz = ball\n+\t[my \"Foo bAr\"]\n+\t\thi = mixed-case\n+\t[my \"FOO BAR\"]\n+\t\thi = upper-case\n+\t[my \"foo bar\"]\n+\t\thi = lower-case\n+\t[case]\n+\t\tbaz = bat\n+\t\tbaz = hask\n+\t[lamb]\n+\t\tchop = 65\n+\t\thead = none\n+\t[goat]\n+\t\tlegs = 4\n+\t\thead = true\n+\t\tskin = false\n+\t\tnose = 1\n+\t\thorns\n+\tEOF\n+'\n+\n+test_expect_success 'get value for a simple key' '\n+\tcheck_config get_value case.penguin \"very blue\"\n+'\n+\n+test_expect_success 'get value for a key with value as an empty string' '\n+\tcheck_config get_value case.my \"\"\n+'\n+\n+test_expect_success 'get value for a key with value as NULL' '\n+\tcheck_config get_value case.foo \"(NULL)\"\n+'\n+\n+test_expect_success 'upper case key' '\n+\tcheck_config get_value case.UPPERCASE \"true\" &&\n+\tcheck_config get_value case.uppercase \"true\"\n+'\n+\n+test_expect_success 'mixed case key' '\n+\tcheck_config get_value case.MixedCase \"true\" &&\n+\tcheck_config get_value case.MIXEDCASE \"true\" &&\n+\tcheck_config get_value case.mixedcase \"true\"\n+'\n+\n+test_expect_success 'key and value with mixed case' '\n+\tcheck_config get_value case.Movie \"BadPhysics\"\n+'\n+\n+test_expect_success 'key with case sensitive subsection' '\n+\tcheck_config get_value \"my.Foo bAr.hi\" \"mixed-case\" &&\n+\tcheck_config get_value \"my.FOO BAR.hi\" \"upper-case\" &&\n+\tcheck_config get_value \"my.foo bar.hi\" \"lower-case\"\n+'\n+\n+test_expect_success 'key with case insensitive section header' '\n+\tcheck_config get_value cores.baz \"ball\" &&\n+\tcheck_config get_value Cores.baz \"ball\" &&\n+\tcheck_config get_value CORES.baz \"ball\" &&\n+\tcheck_config get_value coreS.baz \"ball\"\n+'\n+\n+test_expect_success 'key with case insensitive section header & variable' '\n+\tcheck_config get_value CORES.BAZ \"ball\" &&\n+\tcheck_config get_value cores.baz \"ball\" &&\n+\tcheck_config get_value cores.BaZ \"ball\" &&\n+\tcheck_config get_value cOreS.bAz \"ball\"\n+'\n+\n+test_expect_success 'find value with misspelled key' '\n+\tcheck_config expect_code 1 get_value \"my.fOo Bar.hi\" \"Value not found for \\\"my.fOo Bar.hi\\\"\"\n+'\n+\n+test_expect_success 'find value with the highest priority' '\n+\tcheck_config get_value case.baz \"hask\"\n+'\n+\n+test_expect_success 'find integer value for a key' '\n+\tcheck_config get_int lamb.chop 65\n+'\n+\n+test_expect_success 'find integer if value is non parse-able' '\n+\tcheck_config expect_code 128 get_int lamb.head\n+'\n+\n+test_expect_success 'find bool value for the entered key' '\n+\tcheck_config get_bool goat.head 1 &&\n+\tcheck_config get_bool goat.skin 0 &&\n+\tcheck_config get_bool goat.nose 1 &&\n+\tcheck_config get_bool goat.horns 1 &&\n+\tcheck_config get_bool goat.legs 1\n+'\n+\n+test_expect_success 'find multiple values' '\n+\tcheck_config get_value_multi case.baz sam bat hask\n+'\n+\n+test_expect_success 'find value from a configset' '\n+\tcat >config2 <<-\\EOF &&\n+\t[case]\n+\t\tbaz = lama\n+\t[my]\n+\t\tnew = silk\n+\t[case]\n+\t\tbaz = ball\n+\tEOF\n+\techo silk >expect &&\n+\ttest-config configset_get_value my.new config2 .git/config >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'find value with highest priority from a configset' '\n+\techo hask > expect &&\n+\ttest-config configset_get_value case.baz config2 .git/config  >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'find value_list for a key from a configset' '\n+\tcat >except <<-\\EOF &&\n+\tsam\n+\tbat\n+\thask\n+\tlama\n+\tball\n+\tEOF\n+\ttest-config configset_get_value case.baz config2 .git/config  >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'proper error on non-existant files' '\n+\techo \"Error reading configuration file non-existant-file.\" >expect &&\n+\ttest_expect_code 2 test-config configset_get_value foo.bar non-existant-file 2>actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'proper error on non-accessible  files' '\n+\tchmod -r .git/config &&\n+\ttest_when_finished \"chmod +r .git/config\" &&\n+\techo \"Error reading configuration file .git/config.\" >expect &&\n+\ttest_expect_code 2 test-config configset_get_value foo.bar .git/config 2>actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'proper error on error in default config files' '\n+\tcp .git/config .git/config.old &&\n+\ttest_when_finished \"mv .git/config.old .git/config\" &&\n+\techo \"[\" >> .git/config &&\n+\techo \"fatal: bad config file line 35 in .git/config\" >expect &&\n+\ttest_expect_code 128 test-config get_value foo.bar 2>actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'proper error on error in custom config files' '\n+\techo \"[\" >> syntax-error &&\n+\techo \"fatal: bad config file line 1 in syntax-error\" >expect &&\n+\ttest_expect_code 128 test-config configset_get_value foo.bar syntax-error 2>actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_done\ndiff --git a/test-config.c b/test-config.c\nnew file mode 100644\nindex 0000000..cad35f4\n--- /dev/null\n+++ b/test-config.c\n@@ -0,0 +1,140 @@\n+#include \"cache.h\"\n+#include \"string-list.h\"\n+\n+/*\n+ * This program exposes the C API of the configuration mechanism\n+ * as a set of simple commands in order to facilitate testing.\n+ *\n+ * Reads stdin and prints result of command to stdout:\n+ *\n+ * get_value -> prints the value with highest priority for the entered key\n+ *\n+ * get_value_multi -> prints all values for the entered key in increasing order\n+ *\t\t     of priority\n+ *\n+ * get_int -> print integer value for the entered key or die\n+ *\n+ * get_bool -> print bool value for the entered key or die\n+ *\n+ * configset_get_value -> returns value with the highest priority for the entered key\n+ * \t\t\tfrom a config_set constructed from files entered as arguments.\n+ *\n+ * configset_get_value_multi -> returns value_list for the entered key sorted in\n+ * \t\t\t\tascending order of priority from a config_set\n+ * \t\t\t\tconstructed from files entered as arguments.\n+ *\n+ * Examples:\n+ *\n+ * To print the value with highest priority for key \"foo.bAr Baz.rock\":\n+ * \ttest-config get_value \"foo.bAr Baz.rock\"\n+ *\n+ */\n+\n+\n+int main(int argc, char **argv)\n+{\n+\tint i, val;\n+\tconst char *v;\n+\tconst struct string_list *strptr;\n+\tstruct config_set cs;\n+\tgit_configset_init(&cs);\n+\n+\tif (argc < 2) {\n+\t\tfprintf(stderr, \"Please, provide a command name on the command-line\\n\");\n+\t\tgoto exit1;\n+\t} else if (argc == 3 && !strcmp(argv[1], \"get_value\")) {\n+\t\tif (!git_config_get_value(argv[2], &v)) {\n+\t\t\tif (!v)\n+\t\t\t\tprintf(\"(NULL)\\n\");\n+\t\t\telse\n+\t\t\t\tprintf(\"%s\\n\", v);\n+\t\t\tgoto exit0;\n+\t\t} else {\n+\t\t\tprintf(\"Value not found for \\\"%s\\\"\\n\", argv[2]);\n+\t\t\tgoto exit1;\n+\t\t}\n+\t} else if (argc == 3 && !strcmp(argv[1], \"get_value_multi\")) {\n+\t\tstrptr = git_config_get_value_multi(argv[2]);\n+\t\tif (strptr) {\n+\t\t\tfor (i = 0; i < strptr->nr; i++) {\n+\t\t\t\tv = strptr->items[i].string;\n+\t\t\t\tif (!v)\n+\t\t\t\t\tprintf(\"(NULL)\\n\");\n+\t\t\t\telse\n+\t\t\t\t\tprintf(\"%s\\n\", v);\n+\t\t\t}\n+\t\t\tgoto exit0;\n+\t\t} else {\n+\t\t\tprintf(\"Value not found for \\\"%s\\\"\\n\", argv[2]);\n+\t\t\tgoto exit1;\n+\t\t}\n+\t} else if (argc == 3 && !strcmp(argv[1], \"get_int\")) {\n+\t\tif (!git_config_get_int(argv[2], &val)) {\n+\t\t\tprintf(\"%d\\n\", val);\n+\t\t\tgoto exit0;\n+\t\t} else {\n+\t\t\tprintf(\"Value not found for \\\"%s\\\"\\n\", argv[2]);\n+\t\t\tgoto exit1;\n+\t\t}\n+\t} else if (argc == 3 && !strcmp(argv[1], \"get_bool\")) {\n+\t\tif (!git_config_get_bool(argv[2], &val)) {\n+\t\t\tprintf(\"%d\\n\", val);\n+\t\t\tgoto exit0;\n+\t\t} else {\n+\t\t\tprintf(\"Value not found for \\\"%s\\\"\\n\", argv[2]);\n+\t\t\tgoto exit1;\n+\t\t}\n+\t} else if (!strcmp(argv[1], \"configset_get_value\")) {\n+\t\tfor (i = 3; i < argc; i++) {\n+\t\t\tif (git_configset_add_file(&cs, argv[i])) {\n+\t\t\t\tfprintf(stderr, \"Error reading configuration file %s.\\n\", argv[i]);\n+\t\t\t\tgoto exit2;\n+\t\t\t}\n+\t\t}\n+\t\tif (!git_configset_get_value(&cs, argv[2], &v)) {\n+\t\t\tif (!v)\n+\t\t\t\tprintf(\"(NULL)\\n\");\n+\t\t\telse\n+\t\t\t\tprintf(\"%s\\n\", v);\n+\t\t\tgoto exit0;\n+\t\t} else {\n+\t\t\tprintf(\"Value not found for \\\"%s\\\"\\n\", argv[2]);\n+\t\t\tgoto exit1;\n+\t\t}\n+\t} else if (!strcmp(argv[1], \"configset_get_value_multi\")) {\n+\t\tfor (i = 3; i < argc; i++) {\n+\t\t\tif (git_configset_add_file(&cs, argv[i])) {\n+\t\t\t\tfprintf(stderr, \"Error reading configuration file %s.\\n\", argv[i]);\n+\t\t\t\tgoto exit2;\n+\t\t\t}\n+\t\t}\n+\t\tstrptr = git_configset_get_value_multi(&cs, argv[2]);\n+\t\tif (strptr) {\n+\t\t\tfor (i = 0; i < strptr->nr; i++) {\n+\t\t\t\tv = strptr->items[i].string;\n+\t\t\t\tif (!v)\n+\t\t\t\t\tprintf(\"(NULL)\\n\");\n+\t\t\t\telse\n+\t\t\t\t\tprintf(\"%s\\n\", v);\n+\t\t\t}\n+\t\t\tgoto exit0;\n+\t\t} else {\n+\t\t\tprintf(\"Value not found for \\\"%s\\\"\\n\", argv[2]);\n+\t\t\tgoto exit1;\n+\t\t}\n+\t}\n+\n+\tdie(\"%s: Please check the syntax and the function name\", argv[0]);\n+\n+exit0:\n+\tgit_configset_clear(&cs);\n+\treturn 0;\n+\n+exit1:\n+\tgit_configset_clear(&cs);\n+\treturn 1;\n+\n+exit2:\n+\tgit_configset_clear(&cs);\n+\treturn 2;\n+}\n-- \n1.9.0.GIT\n"},{"id":"246159","messageId":"vpqvbqxjzfx.fsf@anie.imag.fr","threadId":"37130","inReplyTo":"53C665A0.2060705@gmail.com","subject":"Re: [PATCH v9r1 2/2] test-config: add tests for the config_set API","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2014-07-16T11:49:38Z","receivedAt":"2014-07-16T11:49:38Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Tanay Abhra <tanayabh@gmail.com> writes:\n\n> +# 'check_config get_* section.key value' verifies that the entry for\n> +# section.key is 'value'\n> +check_config () {\n> +\tif test \"$1\" = expect_code\n> +\tthen\n> +\t\texpect_code=\"$2\" && shift && shift\n> +\telse\n> +\t\texpect_code=0\n> +\tfi &&\n> +\top=$1 key=$2 && shift && shift\n> +\tif test $# != 0\n\nBroken &&-chain after shift.\n\n(No time for more review right now, but I'll have time in ~3h from now)\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"246160","messageId":"53C66E70.3050507@gmail.com","threadId":"37130","inReplyTo":"vpqvbqxjzfx.fsf@anie.imag.fr","subject":"[PATCH v9r2 1/2] add `config_set` API for caching config-like files","fromName":"Tanay Abhra","fromEmail":"tanayabh@gmail.com","sentAt":"2014-07-16T12:22:08Z","receivedAt":"2014-07-16T12:22:08Z","isPatch":true,"sender":{"key":"tanayabh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2097229?v=4"},"body":"Currently `git_config()` uses a callback mechanism and file rereads for\nconfig values. Due to this approach, it is not uncommon for the config\nfiles to be parsed several times during the run of a git program, with\ndifferent callbacks picking out different variables useful to themselves.\n\nAdd a `config_set`, that can be used to construct an in-memory cache for\nconfig-like files that the caller specifies (i.e., files like `.gitmodules`,\n`~/.gitconfig` etc.). Add two external functions `git_configset_get_value`\nand `git_configset_get_value_multi` for querying from the config sets.\n`git_configset_get_value` follows `last one wins` semantic (i.e. if there\nare multiple matches for the queried key in the files of the configset the\nvalue returned will be the last entry in `value_list`).\n`git_configset_get_value_multi` returns a list of values sorted in order of\nincreasing priority (i.e. last match will be at the end of the list). Add\ntype specific query functions like `git_configset_get_bool` and similar.\n\nAdd a default `config_set`, `the_config_set` to cache all key-value pairs\nread from usual config files (repo specific .git/config, user wide\n~/.gitconfig, XDG config and the global /etc/gitconfig). `the_config_set`\nis populated using `git_config()`.\n\nAdd two external functions `git_config_get_value` and\n`git_config_get_value_multi` for querying in a non-callback manner from\n`the_config_set`. Also, add type specific query functions that are\nimplemented as a thin wrapper around the `config_set` API.\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\nSigned-off-by: Tanay Abhra <tanayabh@gmail.com>\n Documentation/technical/api-config.txt | 137 +++++++++++++++++\n cache.h                                |  30 ++++\n config.c                               | 264 +++++++++++++++++++++++++++++++++\n 3 files changed, 431 insertions(+)\n\ndiff --git a/Documentation/technical/api-config.txt b/Documentation/technical/api-config.txt\nindex 230b3a0..fc0e379 100644\n--- a/Documentation/technical/api-config.txt\n+++ b/Documentation/technical/api-config.txt\n@@ -77,6 +77,81 @@ 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_value`\n+and `git_config_get_value_multi`. They both read values from an internal\n+cache generated previously from reading the config files.\n+\n+`int git_config_get_value(const char *key, const char **value)`::\n+\n+\tFinds the highest-priority value for the configuration variable `key`,\n+\tstores the pointer to it in `value` and returns 0. When the\n+\tconfiguration variable `key` is not found, returns 1 without touching\n+\t`value`. The caller should not free or modify `value`, as it is owned\n+\tby the cache.\n+\n+`const struct string_list *git_config_get_value_multi(const char *key)`::\n+\n+\tFinds and returns the value list, sorted in order of increasing priority\n+\tfor the configuration variable `key`. When the configuration variable\n+\t`key` is not found, returns NULL. The caller should not free or modify\n+\tthe returned pointer, as it is owned by the cache.\n+\n+`void git_config_clear(void)`::\n+\n+\tResets and invalidates the config cache.\n+\n+The config API also provides type specific API functions which do conversion\n+as well as retrieval for the queried variable, including:\n+\n+`int git_config_get_int(const char *key, int *dest)`::\n+\n+\tFinds and parses the value to an integer for the configuration variable\n+\t`key`. Dies on error; otherwise, stores the value of the parsed integer in\n+\t`dest` and returns 0. When the configuration variable `key` is not found,\n+\treturns 1 without touching `dest`.\n+\n+`int git_config_get_ulong(const char *key, unsigned long *dest)`::\n+\n+\tSimilar to `git_config_get_int` but for unsigned longs.\n+\n+`int git_config_get_bool(const char *key, int *dest)`::\n+\n+\tFinds and parses the value into a boolean value, for the configuration\n+\tvariable `key` respecting keywords like \"true\" and \"false\". Integer\n+\tvalues are converted into true/false values (when they are non-zero or\n+\tzero, respectively). Other values cause a die(). If parsing is successful,\n+\tstores the value of the parsed result in `dest` and returns 0. When the\n+\tconfiguration variable `key` is not found, returns 1 without touching\n+\t`dest`.\n+\n+`int git_config_get_bool_or_int(const char *key, int *is_bool, int *dest)`::\n+\n+\tSimilar to `git_config_get_bool`, except that integers are copied as-is,\n+\tand `is_bool` flag is unset.\n+\n+`int git_config_get_maybe_bool(const char *key, int *dest)`::\n+\n+\tSimilar to `git_config_get_bool`, except that it returns -1 on error\n+\trather than dying.\n+\n+`int git_config_get_string(const char *key, const char **dest)`::\n+\n+\tAllocates and copies the retrieved string into the `dest` parameter for\n+\tthe configuration variable `key`; if NULL string is given, prints an\n+\terror message and returns -1. When the configuration variable `key` is\n+\tnot found, returns 1 without touching `dest`.\n+\n+`int git_config_get_pathname(const char *key, const char **dest)`::\n+\n+\tSimilar to `git_config_get_string`, but expands `~` or `~user` into\n+\tthe user's home directory when found at the beginning of the path.\n+\n+See test-config.c for usage examples.\n+\n Value Parsing Helpers\n ---------------------\n\n@@ -134,6 +209,68 @@ int read_file_with_include(const char *file, config_fn_t fn, void *data)\n `git_config` respects includes automatically. The lower-level\n `git_config_from_file` does not.\n\n+Custom Configsets\n+-----------------\n+\n+A `config_set` can be used to construct an in-memory cache for\n+config-like files that the caller specifies (i.e., files like `.gitmodules`,\n+`~/.gitconfig` etc.). For example,\n+\n+---------------------------------------\n+struct config_set gm_config;\n+git_configset_init(&gm_config);\n+int b;\n+/* we add config files to the config_set */\n+git_configset_add_file(&gm_config, \".gitmodules\");\n+git_configset_add_file(&gm_config, \".gitmodules_alt\");\n+\n+if (!git_configset_get_bool(gm_config, \"submodule.frotz.ignore\", &b)) {\n+\t/* hack hack hack */\n+}\n+\n+/* when we are done with the configset */\n+git_configset_clear(&gm_config);\n+----------------------------------------\n+\n+Configset API provides functions for the above mentioned work flow, including:\n+\n+`void git_configset_init(struct config_set *cs)`::\n+\n+\tInitializes the config_set `cs`.\n+\n+`int git_configset_add_file(struct config_set *cs, const char *filename)`::\n+\n+\tParses the file and adds the variable-value pairs to the `config_set`,\n+\tdies if there is an error in parsing the file. Returns 0 on success, or\n+\t-1 if the file doesnot exist or is inaccessible. The user has to decide\n+\tif he wants to free the incomplete configset or continue using it when\n+\tthe function returns -1.\n+\n+`int git_configset_get_value(struct config_set *cs, const char *key, const char **value)`::\n+\n+\tFinds the highest-priority value for the configuration variable `key`\n+\tand config set `cs`, stores the pointer to it in `value` and returns 0.\n+\tWhen the configuration variable `key` is not found, returns 1 without\n+\ttouching `value`. The caller should not free or modify `value`, as it\n+\tis owned by the cache.\n+\n+`const struct string_list *git_configset_get_value_multi(struct config_set *cs, const char *key)`::\n+\n+\tFinds and returns the value list, sorted in order of increasing priority\n+\tfor the configuration variable `key` and config set `cs`. When the\n+\tconfiguration variable `key` is not found, returns NULL. The caller\n+\tshould not free or modify the returned pointer, as it is owned by the cache.\n+\n+`void git_configset_clear(struct config_set *cs)`::\n+\n+\tClears `config_set` structure, removes all saved variable-value pairs.\n+\n+In addition to above functions, the `config_set` API provides type specific\n+functions in the vein of `git_config_get_int` and family but with an extra\n+parameter, pointer to struct `config_set`.\n+They all behave similarly to the `git_config_get*()` family described in\n+\"Querying For Specific Variables\" above.\n+\n Writing Config Files\n --------------------\n\ndiff --git a/cache.h b/cache.h\nindex 44aa439..c67639d 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1329,6 +1329,36 @@ extern int parse_config_key(const char *var,\n \t\t\t    const char **subsection, int *subsection_len,\n \t\t\t    const char **key);\n\n+struct config_set {\n+\tstruct hashmap config_hash;\n+\tint hash_initialized;\n+};\n+\n+extern void git_configset_init(struct config_set *cs);\n+extern int git_configset_add_file(struct config_set *cs, const char *filename);\n+extern int git_configset_get_value(struct config_set *cs, const char *key, const char **value);\n+extern const struct string_list *git_configset_get_value_multi(struct config_set *cs, const char *key);\n+extern void git_configset_clear(struct config_set *cs);\n+extern int git_configset_get_string(struct config_set *cs, const char *key, const char **dest);\n+extern int git_configset_get_int(struct config_set *cs, const char *key, int *dest);\n+extern int git_configset_get_ulong(struct config_set *cs, const char *key, unsigned long *dest);\n+extern int git_configset_get_bool(struct config_set *cs, const char *key, int *dest);\n+extern int git_configset_get_bool_or_int(struct config_set *cs, const char *key, int *is_bool, int *dest);\n+extern int git_configset_get_maybe_bool(struct config_set *cs, const char *key, int *dest);\n+extern int git_configset_get_pathname(struct config_set *cs, const char *key, const char **dest);\n+\n+extern int git_config_get_value(const char *key, const char **value);\n+extern const struct string_list *git_config_get_value_multi(const char *key);\n+extern void git_config_clear(void);\n+extern void git_config_iter(config_fn_t fn, void *data);\n+extern int git_config_get_string(const char *key, const char **dest);\n+extern int git_config_get_int(const char *key, int *dest);\n+extern int git_config_get_ulong(const char *key, unsigned long *dest);\n+extern int git_config_get_bool(const char *key, int *dest);\n+extern int git_config_get_bool_or_int(const char *key, int *is_bool, int *dest);\n+extern int git_config_get_maybe_bool(const char *key, int *dest);\n+extern int git_config_get_pathname(const char *key, const char **dest);\n+\n extern int committer_ident_sufficiently_given(void);\n extern int author_ident_sufficiently_given(void);\n\ndiff --git a/config.c b/config.c\nindex ba882a1..d08c768 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@@ -33,10 +35,23 @@ struct config_source {\n \tlong (*do_ftell)(struct config_source *c);\n };\n\n+struct config_set_element {\n+\tstruct hashmap_entry ent;\n+\tchar *key;\n+\tstruct string_list value_list;\n+};\n+\n static struct config_source *cf;\n\n static int zlib_compression_seen;\n\n+/*\n+ * Default config_set that contains key-value pairs from the usual set of config\n+ * config files (i.e repo specific .git/config, user wide ~/.gitconfig, XDG\n+ * config file and the global /etc/gitconfig)\n+ */\n+static struct config_set the_config_set;\n+\n static int config_file_fgetc(struct config_source *conf)\n {\n \treturn fgetc(conf->u.file);\n@@ -1212,6 +1227,252 @@ int git_config(config_fn_t fn, void *data)\n \treturn git_config_with_options(fn, data, NULL, 1);\n }\n\n+static struct config_set_element *configset_find_element(struct config_set *cs, const char *key)\n+{\n+\tstruct config_set_element k;\n+\tstruct config_set_element *found_entry;\n+\tchar *normalized_key;\n+\tint ret;\n+\t/*\n+\t * `key` may come from the user, so normalize it before using it\n+\t * for querying entries from the hashmap.\n+\t */\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(&cs->config_hash, &k, NULL);\n+\tfree(normalized_key);\n+\treturn found_entry;\n+}\n+\n+static int configset_add_value(struct config_set *cs, const char *key, const char *value)\n+{\n+\tstruct config_set_element *e;\n+\te = configset_find_element(cs, key);\n+\t/*\n+\t * Since the keys are being fed by git_config*() callback mechanism, they\n+\t * are already normalized. So simply add them without any further munging.\n+\t */\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\tmemset(&e->value_list, 0, sizeof(e->value_list));\n+\t\te->value_list.strdup_strings = 1;\n+\t\thashmap_add(&cs->config_hash, e);\n+\t}\n+\tstring_list_append_nodup(&e->value_list, value ? xstrdup(value) : NULL);\n+\n+\treturn 0;\n+}\n+\n+static int config_set_element_cmp(const struct config_set_element *e1,\n+\t\t\t\t const struct config_set_element *e2, const void *unused)\n+{\n+\treturn strcmp(e1->key, e2->key);\n+}\n+\n+void git_configset_init(struct config_set *cs)\n+{\n+\thashmap_init(&cs->config_hash, (hashmap_cmp_fn)config_set_element_cmp, 0);\n+\tcs->hash_initialized = 1;\n+}\n+\n+void git_configset_clear(struct config_set *cs)\n+{\n+\tstruct config_set_element *entry;\n+\tstruct hashmap_iter iter;\n+\tif (!cs->hash_initialized)\n+\t\treturn;\n+\n+\thashmap_iter_init(&cs->config_hash, &iter);\n+\twhile ((entry = hashmap_iter_next(&iter))) {\n+\t\tfree(entry->key);\n+\t\tstring_list_clear(&entry->value_list, 0);\n+\t}\n+\thashmap_free(&cs->config_hash, 1);\n+\tcs->hash_initialized = 0;\n+}\n+\n+static int config_set_callback(const char *key, const char *value, void *cb)\n+{\n+\tstruct config_set *cs = cb;\n+\tconfigset_add_value(cs, key, value);\n+\treturn 0;\n+}\n+\n+int git_configset_add_file(struct config_set *cs, const char *filename)\n+{\n+\treturn git_config_from_file(config_set_callback, filename, cs);\n+}\n+\n+int git_configset_get_value(struct config_set *cs, const char *key, const char **value)\n+{\n+\tconst struct string_list *values = NULL;\n+\t/*\n+\t * Follows \"last one wins\" semantic, i.e., if there are multiple matches for the\n+\t * queried key in the files of the configset, the value returned will be the last\n+\t * value in the value list for that key.\n+\t */\n+\tvalues = git_configset_get_value_multi(cs, key);\n+\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+const struct string_list *git_configset_get_value_multi(struct config_set *cs, const char *key)\n+{\n+\tstruct config_set_element *e = configset_find_element(cs, key);\n+\treturn e ? &e->value_list : NULL;\n+}\n+\n+int git_configset_get_string(struct config_set *cs, const char *key, const char **dest)\n+{\n+\tconst char *value;\n+\tif (!git_configset_get_value(cs, key, &value))\n+\t\treturn git_config_string(dest, key, value);\n+\telse\n+\t\treturn 1;\n+}\n+\n+int git_configset_get_int(struct config_set *cs, const char *key, int *dest)\n+{\n+\tconst char *value;\n+\tif (!git_configset_get_value(cs, key, &value)) {\n+\t\t*dest = git_config_int(key, value);\n+\t\treturn 0;\n+\t} else\n+\t\treturn 1;\n+}\n+\n+int git_configset_get_ulong(struct config_set *cs, const char *key, unsigned long *dest)\n+{\n+\tconst char *value;\n+\tif (!git_configset_get_value(cs, key, &value)) {\n+\t\t*dest = git_config_ulong(key, value);\n+\t\treturn 0;\n+\t} else\n+\t\treturn 1;\n+}\n+\n+int git_configset_get_bool(struct config_set *cs, const char *key, int *dest)\n+{\n+\tconst char *value;\n+\tif (!git_configset_get_value(cs, key, &value)) {\n+\t\t*dest = git_config_bool(key, value);\n+\t\treturn 0;\n+\t} else\n+\t\treturn 1;\n+}\n+\n+int git_configset_get_bool_or_int(struct config_set *cs, const char *key,\n+\t\t\t\tint *is_bool, int *dest)\n+{\n+\tconst char *value;\n+\tif (!git_configset_get_value(cs, key, &value)) {\n+\t\t*dest = git_config_bool_or_int(key, value, is_bool);\n+\t\treturn 0;\n+\t} else\n+\t\treturn 1;\n+}\n+\n+int git_configset_get_maybe_bool(struct config_set *cs, const char *key, int *dest)\n+{\n+\tconst char *value;\n+\tif (!git_configset_get_value(cs, key, &value)) {\n+\t\t*dest = git_config_maybe_bool(key, value);\n+\t\tif (*dest == -1)\n+\t\t\treturn -1;\n+\t\treturn 0;\n+\t} else\n+\t\treturn 1;\n+}\n+\n+int git_configset_get_pathname(struct config_set *cs, const char *key, const char **dest)\n+{\n+\tconst char *value;\n+\tif (!git_configset_get_value(cs, key, &value))\n+\t\treturn git_config_pathname(dest, key, value);\n+\telse\n+\t\treturn 1;\n+}\n+\n+static void git_config_check_init(void)\n+{\n+\tif (the_config_set.hash_initialized)\n+\t\treturn;\n+\tgit_configset_init(&the_config_set);\n+\tgit_config(config_set_callback, &the_config_set);\n+}\n+\n+void git_config_clear(void)\n+{\n+\tif (!the_config_set.hash_initialized)\n+\t\treturn;\n+\tgit_configset_clear(&the_config_set);\n+}\n+\n+int git_config_get_value(const char *key, const char **value)\n+{\n+\tgit_config_check_init();\n+\treturn git_configset_get_value(&the_config_set, key, value);\n+}\n+\n+const struct string_list *git_config_get_value_multi(const char *key)\n+{\n+\tgit_config_check_init();\n+\treturn git_configset_get_value_multi(&the_config_set, key);\n+}\n+\n+int git_config_get_string(const char *key, const char **dest)\n+{\n+\tgit_config_check_init();\n+\treturn git_configset_get_string(&the_config_set, key, dest);\n+}\n+\n+int git_config_get_int(const char *key, int *dest)\n+{\n+\tgit_config_check_init();\n+\treturn git_configset_get_int(&the_config_set, key, dest);\n+}\n+\n+int git_config_get_ulong(const char *key, unsigned long *dest)\n+{\n+\tgit_config_check_init();\n+\treturn git_configset_get_ulong(&the_config_set, key, dest);\n+}\n+\n+int git_config_get_bool(const char *key, int *dest)\n+{\n+\tgit_config_check_init();\n+\treturn git_configset_get_bool(&the_config_set, key, dest);\n+}\n+\n+int git_config_get_bool_or_int(const char *key, int *is_bool, int *dest)\n+{\n+\tgit_config_check_init();\n+\treturn git_configset_get_bool_or_int(&the_config_set, key, is_bool, dest);\n+}\n+\n+int git_config_get_maybe_bool(const char *key, int *dest)\n+{\n+\tgit_config_check_init();\n+\treturn git_configset_get_maybe_bool(&the_config_set, key, dest);\n+}\n+\n+int git_config_get_pathname(const char *key, const char **dest)\n+{\n+\tgit_config_check_init();\n+\treturn git_configset_get_pathname(&the_config_set, key, dest);\n+}\n+\n /*\n  * Find all the stuff for git_config_set() below.\n  */\n@@ -1707,6 +1968,9 @@ int git_config_set_multivar_in_file(const char *config_filename,\n \tlock = NULL;\n \tret = 0;\n\n+\t/* Invalidate the config cache */\n+\tgit_config_clear();\n+\n out_free:\n \tif (lock)\n \t\trollback_lock_file(lock);\n-- \n1.9.0.GIT\n"},{"id":"246161","messageId":"53C66EA0.6040401@gmail.com","threadId":"37130","inReplyTo":"vpqvbqxjzfx.fsf@anie.imag.fr","subject":"[PATCH v9r2 2/2] test-config: add tests for the config_set API","fromName":"Tanay Abhra","fromEmail":"tanayabh@gmail.com","sentAt":"2014-07-16T12:22:56Z","receivedAt":"2014-07-16T12:22:56Z","isPatch":true,"sender":{"key":"tanayabh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2097229?v=4"},"body":"Expose the `config_set` C API as a set of simple commands in order to\nfacilitate testing. Add tests for the `config_set` API as well as for\n`git_config_get_*()` family for the usual config files.\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\nSigned-off-by: Tanay Abhra <tanayabh@gmail.com>\n---\n .gitignore            |   1 +\n Makefile              |   1 +\n t/t1308-config-set.sh | 200 ++++++++++++++++++++++++++++++++++++++++++++++++++\n test-config.c         | 140 +++++++++++++++++++++++++++++++++++\n 4 files changed, 342 insertions(+)\n create mode 100755 t/t1308-config-set.sh\n create mode 100644 test-config.c\n\ndiff --git a/.gitignore b/.gitignore\nindex edb1dcf..fcdee42 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -178,6 +178,7 @@\n /gitweb/static/gitweb.min.*\n /test-chmtime\n /test-ctype\n+/test-config\n /test-date\n /test-delta\n /test-dump-cache-tree\ndiff --git a/Makefile b/Makefile\nindex b92418d..e070eb8 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -549,6 +549,7 @@ PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))\n\n TEST_PROGRAMS_NEED_X += test-chmtime\n TEST_PROGRAMS_NEED_X += test-ctype\n+TEST_PROGRAMS_NEED_X += test-config\n TEST_PROGRAMS_NEED_X += test-date\n TEST_PROGRAMS_NEED_X += test-delta\n TEST_PROGRAMS_NEED_X += test-dump-cache-tree\ndiff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh\nnew file mode 100755\nindex 0000000..4752fd9\n--- /dev/null\n+++ b/t/t1308-config-set.sh\n@@ -0,0 +1,200 @@\n+#!/bin/sh\n+\n+test_description='Test git config-set API in different settings'\n+\n+. ./test-lib.sh\n+\n+# 'check_config get_* section.key value' verifies that the entry for\n+# section.key is 'value'\n+check_config () {\n+\tif test \"$1\" = expect_code\n+\tthen\n+\t\texpect_code=\"$2\" && shift && shift\n+\telse\n+\t\texpect_code=0\n+\tfi &&\n+\top=$1 key=$2 && shift && shift &&\n+\tif test $# != 0\n+\tthen\n+\t\tprintf \"%s\\n\" \"$@\"\n+\tfi >expect &&\n+\ttest_expect_code $expect_code test-config \"$op\" \"$key\" >actual &&\n+\ttest_cmp expect actual\n+}\n+\n+test_expect_success 'setup default config' '\n+\tcat >.git/config <<\\EOF\n+\t[case]\n+\t\tpenguin = very blue\n+\t\tMovie = BadPhysics\n+\t\tUPPERCASE = true\n+\t\tMixedCase = true\n+\t\tmy =\n+\t\tfoo\n+\t\tbaz = sam\n+\t[Cores]\n+\t\tWhatEver = Second\n+\t\tbaz = bar\n+\t[cores]\n+\t\tbaz = bat\n+\t[CORES]\n+\t\tbaz = ball\n+\t[my \"Foo bAr\"]\n+\t\thi = mixed-case\n+\t[my \"FOO BAR\"]\n+\t\thi = upper-case\n+\t[my \"foo bar\"]\n+\t\thi = lower-case\n+\t[case]\n+\t\tbaz = bat\n+\t\tbaz = hask\n+\t[lamb]\n+\t\tchop = 65\n+\t\thead = none\n+\t[goat]\n+\t\tlegs = 4\n+\t\thead = true\n+\t\tskin = false\n+\t\tnose = 1\n+\t\thorns\n+\tEOF\n+'\n+\n+test_expect_success 'get value for a simple key' '\n+\tcheck_config get_value case.penguin \"very blue\"\n+'\n+\n+test_expect_success 'get value for a key with value as an empty string' '\n+\tcheck_config get_value case.my \"\"\n+'\n+\n+test_expect_success 'get value for a key with value as NULL' '\n+\tcheck_config get_value case.foo \"(NULL)\"\n+'\n+\n+test_expect_success 'upper case key' '\n+\tcheck_config get_value case.UPPERCASE \"true\" &&\n+\tcheck_config get_value case.uppercase \"true\"\n+'\n+\n+test_expect_success 'mixed case key' '\n+\tcheck_config get_value case.MixedCase \"true\" &&\n+\tcheck_config get_value case.MIXEDCASE \"true\" &&\n+\tcheck_config get_value case.mixedcase \"true\"\n+'\n+\n+test_expect_success 'key and value with mixed case' '\n+\tcheck_config get_value case.Movie \"BadPhysics\"\n+'\n+\n+test_expect_success 'key with case sensitive subsection' '\n+\tcheck_config get_value \"my.Foo bAr.hi\" \"mixed-case\" &&\n+\tcheck_config get_value \"my.FOO BAR.hi\" \"upper-case\" &&\n+\tcheck_config get_value \"my.foo bar.hi\" \"lower-case\"\n+'\n+\n+test_expect_success 'key with case insensitive section header' '\n+\tcheck_config get_value cores.baz \"ball\" &&\n+\tcheck_config get_value Cores.baz \"ball\" &&\n+\tcheck_config get_value CORES.baz \"ball\" &&\n+\tcheck_config get_value coreS.baz \"ball\"\n+'\n+\n+test_expect_success 'key with case insensitive section header & variable' '\n+\tcheck_config get_value CORES.BAZ \"ball\" &&\n+\tcheck_config get_value cores.baz \"ball\" &&\n+\tcheck_config get_value cores.BaZ \"ball\" &&\n+\tcheck_config get_value cOreS.bAz \"ball\"\n+'\n+\n+test_expect_success 'find value with misspelled key' '\n+\tcheck_config expect_code 1 get_value \"my.fOo Bar.hi\" \"Value not found for \\\"my.fOo Bar.hi\\\"\"\n+'\n+\n+test_expect_success 'find value with the highest priority' '\n+\tcheck_config get_value case.baz \"hask\"\n+'\n+\n+test_expect_success 'find integer value for a key' '\n+\tcheck_config get_int lamb.chop 65\n+'\n+\n+test_expect_success 'find integer if value is non parse-able' '\n+\tcheck_config expect_code 128 get_int lamb.head\n+'\n+\n+test_expect_success 'find bool value for the entered key' '\n+\tcheck_config get_bool goat.head 1 &&\n+\tcheck_config get_bool goat.skin 0 &&\n+\tcheck_config get_bool goat.nose 1 &&\n+\tcheck_config get_bool goat.horns 1 &&\n+\tcheck_config get_bool goat.legs 1\n+'\n+\n+test_expect_success 'find multiple values' '\n+\tcheck_config get_value_multi case.baz sam bat hask\n+'\n+\n+test_expect_success 'find value from a configset' '\n+\tcat >config2 <<-\\EOF &&\n+\t[case]\n+\t\tbaz = lama\n+\t[my]\n+\t\tnew = silk\n+\t[case]\n+\t\tbaz = ball\n+\tEOF\n+\techo silk >expect &&\n+\ttest-config configset_get_value my.new config2 .git/config >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'find value with highest priority from a configset' '\n+\techo hask > expect &&\n+\ttest-config configset_get_value case.baz config2 .git/config  >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'find value_list for a key from a configset' '\n+\tcat >except <<-\\EOF &&\n+\tsam\n+\tbat\n+\thask\n+\tlama\n+\tball\n+\tEOF\n+\ttest-config configset_get_value case.baz config2 .git/config  >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'proper error on non-existant files' '\n+\techo \"Error reading configuration file non-existant-file.\" >expect &&\n+\ttest_expect_code 2 test-config configset_get_value foo.bar non-existant-file 2>actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'proper error on non-accessible  files' '\n+\tchmod -r .git/config &&\n+\ttest_when_finished \"chmod +r .git/config\" &&\n+\techo \"Error reading configuration file .git/config.\" >expect &&\n+\ttest_expect_code 2 test-config configset_get_value foo.bar .git/config 2>actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'proper error on error in default config files' '\n+\tcp .git/config .git/config.old &&\n+\ttest_when_finished \"mv .git/config.old .git/config\" &&\n+\techo \"[\" >> .git/config &&\n+\techo \"fatal: bad config file line 35 in .git/config\" >expect &&\n+\ttest_expect_code 128 test-config get_value foo.bar 2>actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'proper error on error in custom config files' '\n+\techo \"[\" >> syntax-error &&\n+\techo \"fatal: bad config file line 1 in syntax-error\" >expect &&\n+\ttest_expect_code 128 test-config configset_get_value foo.bar syntax-error 2>actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_done\ndiff --git a/test-config.c b/test-config.c\nnew file mode 100644\nindex 0000000..cad35f4\n--- /dev/null\n+++ b/test-config.c\n@@ -0,0 +1,140 @@\n+#include \"cache.h\"\n+#include \"string-list.h\"\n+\n+/*\n+ * This program exposes the C API of the configuration mechanism\n+ * as a set of simple commands in order to facilitate testing.\n+ *\n+ * Reads stdin and prints result of command to stdout:\n+ *\n+ * get_value -> prints the value with highest priority for the entered key\n+ *\n+ * get_value_multi -> prints all values for the entered key in increasing order\n+ *\t\t     of priority\n+ *\n+ * get_int -> print integer value for the entered key or die\n+ *\n+ * get_bool -> print bool value for the entered key or die\n+ *\n+ * configset_get_value -> returns value with the highest priority for the entered key\n+ * \t\t\tfrom a config_set constructed from files entered as arguments.\n+ *\n+ * configset_get_value_multi -> returns value_list for the entered key sorted in\n+ * \t\t\t\tascending order of priority from a config_set\n+ * \t\t\t\tconstructed from files entered as arguments.\n+ *\n+ * Examples:\n+ *\n+ * To print the value with highest priority for key \"foo.bAr Baz.rock\":\n+ * \ttest-config get_value \"foo.bAr Baz.rock\"\n+ *\n+ */\n+\n+\n+int main(int argc, char **argv)\n+{\n+\tint i, val;\n+\tconst char *v;\n+\tconst struct string_list *strptr;\n+\tstruct config_set cs;\n+\tgit_configset_init(&cs);\n+\n+\tif (argc < 2) {\n+\t\tfprintf(stderr, \"Please, provide a command name on the command-line\\n\");\n+\t\tgoto exit1;\n+\t} else if (argc == 3 && !strcmp(argv[1], \"get_value\")) {\n+\t\tif (!git_config_get_value(argv[2], &v)) {\n+\t\t\tif (!v)\n+\t\t\t\tprintf(\"(NULL)\\n\");\n+\t\t\telse\n+\t\t\t\tprintf(\"%s\\n\", v);\n+\t\t\tgoto exit0;\n+\t\t} else {\n+\t\t\tprintf(\"Value not found for \\\"%s\\\"\\n\", argv[2]);\n+\t\t\tgoto exit1;\n+\t\t}\n+\t} else if (argc == 3 && !strcmp(argv[1], \"get_value_multi\")) {\n+\t\tstrptr = git_config_get_value_multi(argv[2]);\n+\t\tif (strptr) {\n+\t\t\tfor (i = 0; i < strptr->nr; i++) {\n+\t\t\t\tv = strptr->items[i].string;\n+\t\t\t\tif (!v)\n+\t\t\t\t\tprintf(\"(NULL)\\n\");\n+\t\t\t\telse\n+\t\t\t\t\tprintf(\"%s\\n\", v);\n+\t\t\t}\n+\t\t\tgoto exit0;\n+\t\t} else {\n+\t\t\tprintf(\"Value not found for \\\"%s\\\"\\n\", argv[2]);\n+\t\t\tgoto exit1;\n+\t\t}\n+\t} else if (argc == 3 && !strcmp(argv[1], \"get_int\")) {\n+\t\tif (!git_config_get_int(argv[2], &val)) {\n+\t\t\tprintf(\"%d\\n\", val);\n+\t\t\tgoto exit0;\n+\t\t} else {\n+\t\t\tprintf(\"Value not found for \\\"%s\\\"\\n\", argv[2]);\n+\t\t\tgoto exit1;\n+\t\t}\n+\t} else if (argc == 3 && !strcmp(argv[1], \"get_bool\")) {\n+\t\tif (!git_config_get_bool(argv[2], &val)) {\n+\t\t\tprintf(\"%d\\n\", val);\n+\t\t\tgoto exit0;\n+\t\t} else {\n+\t\t\tprintf(\"Value not found for \\\"%s\\\"\\n\", argv[2]);\n+\t\t\tgoto exit1;\n+\t\t}\n+\t} else if (!strcmp(argv[1], \"configset_get_value\")) {\n+\t\tfor (i = 3; i < argc; i++) {\n+\t\t\tif (git_configset_add_file(&cs, argv[i])) {\n+\t\t\t\tfprintf(stderr, \"Error reading configuration file %s.\\n\", argv[i]);\n+\t\t\t\tgoto exit2;\n+\t\t\t}\n+\t\t}\n+\t\tif (!git_configset_get_value(&cs, argv[2], &v)) {\n+\t\t\tif (!v)\n+\t\t\t\tprintf(\"(NULL)\\n\");\n+\t\t\telse\n+\t\t\t\tprintf(\"%s\\n\", v);\n+\t\t\tgoto exit0;\n+\t\t} else {\n+\t\t\tprintf(\"Value not found for \\\"%s\\\"\\n\", argv[2]);\n+\t\t\tgoto exit1;\n+\t\t}\n+\t} else if (!strcmp(argv[1], \"configset_get_value_multi\")) {\n+\t\tfor (i = 3; i < argc; i++) {\n+\t\t\tif (git_configset_add_file(&cs, argv[i])) {\n+\t\t\t\tfprintf(stderr, \"Error reading configuration file %s.\\n\", argv[i]);\n+\t\t\t\tgoto exit2;\n+\t\t\t}\n+\t\t}\n+\t\tstrptr = git_configset_get_value_multi(&cs, argv[2]);\n+\t\tif (strptr) {\n+\t\t\tfor (i = 0; i < strptr->nr; i++) {\n+\t\t\t\tv = strptr->items[i].string;\n+\t\t\t\tif (!v)\n+\t\t\t\t\tprintf(\"(NULL)\\n\");\n+\t\t\t\telse\n+\t\t\t\t\tprintf(\"%s\\n\", v);\n+\t\t\t}\n+\t\t\tgoto exit0;\n+\t\t} else {\n+\t\t\tprintf(\"Value not found for \\\"%s\\\"\\n\", argv[2]);\n+\t\t\tgoto exit1;\n+\t\t}\n+\t}\n+\n+\tdie(\"%s: Please check the syntax and the function name\", argv[0]);\n+\n+exit0:\n+\tgit_configset_clear(&cs);\n+\treturn 0;\n+\n+exit1:\n+\tgit_configset_clear(&cs);\n+\treturn 1;\n+\n+exit2:\n+\tgit_configset_clear(&cs);\n+\treturn 2;\n+}\n-- \n1.9.0.GIT\n"},{"id":"246164","messageId":"vpqha2h9tjw.fsf@anie.imag.fr","threadId":"37130","inReplyTo":"53C66E70.3050507@gmail.com","subject":"Re: [PATCH v9r2 1/2] add `config_set` API for caching config-like files","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2014-07-16T16:06:59Z","receivedAt":"2014-07-16T16:06:59Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Tanay Abhra <tanayabh@gmail.com> writes:\n\n> implemented as a thin wrapper around the `config_set` API.\n>\n> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n> Signed-off-by: Tanay Abhra <tanayabh@gmail.com>\n>  Documentation/technical/api-config.txt | 137 +++++++++++++++++\n>  cache.h                                |  30 ++++\n>  config.c                               | 264 +++++++++++++++++++++++++++++++++\n>  3 files changed, 431 insertions(+)\n\n(you broke the patch by removing the ---)\n\n> +static void git_config_check_init(void)\n> +{\n> +\tif (the_config_set.hash_initialized)\n> +\t\treturn;\n> +\tgit_configset_init(&the_config_set);\n> +\tgit_config(config_set_callback, &the_config_set);\n> +}\n\nSo, you're now ignoring the return value of git_config. What is the\nrationale for this? In particular, why did you reject the \"die\"\npossibility (I understood that you were inclined to take this option, so\nI'm curious why you changed your mind).\n\nOTOH, you're transmitting the return value without dying here:\n\n+int git_configset_add_file(struct config_set *cs, const char *filename)\n+{\n+\treturn git_config_from_file(config_set_callback, filename, cs);\n+}\n\nand I think this one is correct, as we cannot tell in advance how\nserious an error would be for any callers. And we do test this (I think\nwe can improve a bit, I'll send a fixup patch).\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"246165","messageId":"1405526952-25019-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"37130","inReplyTo":"vpqha2h9tjw.fsf@anie.imag.fr","subject":"[PATCH 1/3] fixup for patch 2: minor style fix","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2014-07-16T16:09:10Z","receivedAt":"2014-07-16T16:09:10Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\n t/t1308-config-set.sh | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh\nindex 4752fd9..ea031bf 100755\n--- a/t/t1308-config-set.sh\n+++ b/t/t1308-config-set.sh\n@@ -150,8 +150,8 @@ test_expect_success 'find value from a configset' '\n '\n \n test_expect_success 'find value with highest priority from a configset' '\n-\techo hask > expect &&\n-\ttest-config configset_get_value case.baz config2 .git/config  >actual &&\n+\techo hask >expect &&\n+\ttest-config configset_get_value case.baz config2 .git/config >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -163,7 +163,7 @@ test_expect_success 'find value_list for a key from a configset' '\n \tlama\n \tball\n \tEOF\n-\ttest-config configset_get_value case.baz config2 .git/config  >actual &&\n+\ttest-config configset_get_value case.baz config2 .git/config >actual &&\n \ttest_cmp expect actual\n '\n \n@@ -173,7 +173,7 @@ test_expect_success 'proper error on non-existant files' '\n \ttest_cmp expect actual\n '\n \n-test_expect_success 'proper error on non-accessible  files' '\n+test_expect_success 'proper error on non-accessible files' '\n \tchmod -r .git/config &&\n \ttest_when_finished \"chmod +r .git/config\" &&\n \techo \"Error reading configuration file .git/config.\" >expect &&\n@@ -184,14 +184,14 @@ test_expect_success 'proper error on non-accessible  files' '\n test_expect_success 'proper error on error in default config files' '\n \tcp .git/config .git/config.old &&\n \ttest_when_finished \"mv .git/config.old .git/config\" &&\n-\techo \"[\" >> .git/config &&\n+\techo \"[\" >>.git/config &&\n \techo \"fatal: bad config file line 35 in .git/config\" >expect &&\n \ttest_expect_code 128 test-config get_value foo.bar 2>actual &&\n \ttest_cmp expect actual\n '\n \n test_expect_success 'proper error on error in custom config files' '\n-\techo \"[\" >> syntax-error &&\n+\techo \"[\" >>syntax-error &&\n \techo \"fatal: bad config file line 1 in syntax-error\" >expect &&\n \ttest_expect_code 128 test-config configset_get_value foo.bar syntax-error 2>actual &&\n \ttest_cmp expect actual\n-- \n2.0.0.262.gdafc651\n"},{"id":"246166","messageId":"1405526952-25019-2-git-send-email-Matthieu.Moy@imag.fr","threadId":"37130","inReplyTo":"1405526952-25019-1-git-send-email-Matthieu.Moy@imag.fr","subject":"[PATCH 2/3] fixup for patch 2: actually check the return value","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2014-07-16T16:09:11Z","receivedAt":"2014-07-16T16:09:11Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\nI won't fight for this, but I think it makes sense.\n\n t/t1308-config-set.sh |  4 ++--\n test-config.c         | 10 ++++++----\n 2 files changed, 8 insertions(+), 6 deletions(-)\n\ndiff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh\nindex ea031bf..f0307b7 100755\n--- a/t/t1308-config-set.sh\n+++ b/t/t1308-config-set.sh\n@@ -168,7 +168,7 @@ test_expect_success 'find value_list for a key from a configset' '\n '\n \n test_expect_success 'proper error on non-existant files' '\n-\techo \"Error reading configuration file non-existant-file.\" >expect &&\n+\techo \"Error (-1) reading configuration file non-existant-file.\" >expect &&\n \ttest_expect_code 2 test-config configset_get_value foo.bar non-existant-file 2>actual &&\n \ttest_cmp expect actual\n '\n@@ -176,7 +176,7 @@ test_expect_success 'proper error on non-existant files' '\n test_expect_success 'proper error on non-accessible files' '\n \tchmod -r .git/config &&\n \ttest_when_finished \"chmod +r .git/config\" &&\n-\techo \"Error reading configuration file .git/config.\" >expect &&\n+\techo \"Error (-1) reading configuration file .git/config.\" >expect &&\n \ttest_expect_code 2 test-config configset_get_value foo.bar .git/config 2>actual &&\n \ttest_cmp expect actual\n '\ndiff --git a/test-config.c b/test-config.c\nindex cad35f4..9dd1b22 100644\n--- a/test-config.c\n+++ b/test-config.c\n@@ -86,8 +86,9 @@ int main(int argc, char **argv)\n \t\t}\n \t} else if (!strcmp(argv[1], \"configset_get_value\")) {\n \t\tfor (i = 3; i < argc; i++) {\n-\t\t\tif (git_configset_add_file(&cs, argv[i])) {\n-\t\t\t\tfprintf(stderr, \"Error reading configuration file %s.\\n\", argv[i]);\n+\t\t\tint err;\n+\t\t\tif ((err = git_configset_add_file(&cs, argv[i]))) {\n+\t\t\t\tfprintf(stderr, \"Error (%d) reading configuration file %s.\\n\", err, argv[i]);\n \t\t\t\tgoto exit2;\n \t\t\t}\n \t\t}\n@@ -103,8 +104,9 @@ int main(int argc, char **argv)\n \t\t}\n \t} else if (!strcmp(argv[1], \"configset_get_value_multi\")) {\n \t\tfor (i = 3; i < argc; i++) {\n-\t\t\tif (git_configset_add_file(&cs, argv[i])) {\n-\t\t\t\tfprintf(stderr, \"Error reading configuration file %s.\\n\", argv[i]);\n+\t\t\tint err;\n+\t\t\tif ((err = git_configset_add_file(&cs, argv[i]))) {\n+\t\t\t\tfprintf(stderr, \"Error (%d) reading configuration file %s.\\n\", err, argv[i]);\n \t\t\t\tgoto exit2;\n \t\t\t}\n \t\t}\n-- \n2.0.0.262.gdafc651\n"},{"id":"246167","messageId":"1405526952-25019-3-git-send-email-Matthieu.Moy@imag.fr","threadId":"37130","inReplyTo":"1405526952-25019-1-git-send-email-Matthieu.Moy@imag.fr","subject":"[PATCH 3/3] fixup for patch 1: typo","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2014-07-16T16:09:12Z","receivedAt":"2014-07-16T16:09:12Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\n Documentation/technical/api-config.txt | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/Documentation/technical/api-config.txt b/Documentation/technical/api-config.txt\nindex fc0e379..8a86e45 100644\n--- a/Documentation/technical/api-config.txt\n+++ b/Documentation/technical/api-config.txt\n@@ -242,7 +242,7 @@ Configset API provides functions for the above mentioned work flow, including:\n \n \tParses the file and adds the variable-value pairs to the `config_set`,\n \tdies if there is an error in parsing the file. Returns 0 on success, or\n-\t-1 if the file doesnot exist or is inaccessible. The user has to decide\n+\t-1 if the file does not exist or is inaccessible. The user has to decide\n \tif he wants to free the incomplete configset or continue using it when\n \tthe function returns -1.\n \n-- \n2.0.0.262.gdafc651\n"},{"id":"246168","messageId":"53C6ABEB.3060205@gmail.com","threadId":"37130","inReplyTo":"vpqha2h9tjw.fsf@anie.imag.fr","subject":"Re: [PATCH v9r2 1/2] add `config_set` API for caching config-like files","fromName":"Tanay Abhra","fromEmail":"tanayabh@gmail.com","sentAt":"2014-07-16T16:44:27Z","receivedAt":"2014-07-16T16:44:27Z","isPatch":true,"sender":{"key":"tanayabh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2097229?v=4"},"body":"\n\nOn 7/16/2014 9:36 PM, Matthieu Moy wrote:\n> Tanay Abhra <tanayabh@gmail.com> writes:\n> \n>> implemented as a thin wrapper around the `config_set` API.\n>>\n>> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n>> Signed-off-by: Tanay Abhra <tanayabh@gmail.com>\n>>  Documentation/technical/api-config.txt | 137 +++++++++++++++++\n>>  cache.h                                |  30 ++++\n>>  config.c                               | 264 +++++++++++++++++++++++++++++++++\n>>  3 files changed, 431 insertions(+)\n> \n> (you broke the patch by removing the ---)\n>\n\nYikes, sorry about that.\n\n>> +static void git_config_check_init(void)\n>> +{\n>> +\tif (the_config_set.hash_initialized)\n>> +\t\treturn;\n>> +\tgit_configset_init(&the_config_set);\n>> +\tgit_config(config_set_callback, &the_config_set);\n>> +}\n> \n> So, you're now ignoring the return value of git_config. What is the\n> rationale for this? In particular, why did you reject the \"die\"\n> possibility (I understood that you were inclined to take this option, so\n> I'm curious why you changed your mind).\n>\n\nThe errors (non accessible, non existent files etc) were already being caught by\ngit_config_early(). Since git_config() only returns positive values except\nthe weird race case you mentioned, I thought the die confused the reader\nof the patch more than it provided error checking. I also tried myself\nsimulating the race condition but failed. All the callers of git_config()\nalso ignore the return value, so I ended up ignoring the return value myself.\n\n> OTOH, you're transmitting the return value without dying here:\n> \n> +int git_configset_add_file(struct config_set *cs, const char *filename)\n> +{\n> +\treturn git_config_from_file(config_set_callback, filename, cs);\n> +}\n> \n> and I think this one is correct, as we cannot tell in advance how\n> serious an error would be for any callers. And we do test this (I think\n> we can improve a bit, I'll send a fixup patch).\n>\n\nAfter reading the commit log that you mentioned and some previous ones before\nthat I surmised that the official slant was to silently ignore nonexistent\nfiles. Though an access_or_warn() check was placed on most of the files\nlike git attributes, since non accessible file errors may be a user configuration\nerror. So, I decided to ignore the return value.\n\nBut I do think that an access_or_warn() check should be put on git config --file\nand git_configset_add_file since other parts of git follow it. What do\nyou think about it, still I will send followup patch correcting the git config\n--file condition where it silently ignores the file access error and continues?\n"},{"id":"246169","messageId":"53C6AE9F.2030908@gmail.com","threadId":"37130","inReplyTo":"1405526952-25019-2-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH 2/3] fixup for patch 2: actually check the return value","fromName":"Tanay Abhra","fromEmail":"tanayabh@gmail.com","sentAt":"2014-07-16T16:55:59Z","receivedAt":"2014-07-16T16:55:59Z","isPatch":true,"sender":{"key":"tanayabh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2097229?v=4"},"body":"I think it would be unnecessary for the current iteration.\nCurrently git_configset_add_file has only two possible return values\n-1 or 0. I could add specialized error values for ENOENT or ENOTDIR\nor EACCES, but the logs show that we silently ignore the first two.\nI can add an access warn for the third. What do you think?\n\nThanks,\nTanay.\n\nOn 7/16/2014 9:39 PM, Matthieu Moy wrote:\n> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n> ---\n> I won't fight for this, but I think it makes sense.\n> \n>  t/t1308-config-set.sh |  4 ++--\n>  test-config.c         | 10 ++++++----\n>  2 files changed, 8 insertions(+), 6 deletions(-)\n> \n> diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh\n> index ea031bf..f0307b7 100755\n> --- a/t/t1308-config-set.sh\n> +++ b/t/t1308-config-set.sh\n> @@ -168,7 +168,7 @@ test_expect_success 'find value_list for a key from a configset' '\n>  '\n>  \n>  test_expect_success 'proper error on non-existant files' '\n> -\techo \"Error reading configuration file non-existant-file.\" >expect &&\n> +\techo \"Error (-1) reading configuration file non-existant-file.\" >expect &&\n>  \ttest_expect_code 2 test-config configset_get_value foo.bar non-existant-file 2>actual &&\n>  \ttest_cmp expect actual\n>  '\n> @@ -176,7 +176,7 @@ test_expect_success 'proper error on non-existant files' '\n>  test_expect_success 'proper error on non-accessible files' '\n>  \tchmod -r .git/config &&\n>  \ttest_when_finished \"chmod +r .git/config\" &&\n> -\techo \"Error reading configuration file .git/config.\" >expect &&\n> +\techo \"Error (-1) reading configuration file .git/config.\" >expect &&\n>  \ttest_expect_code 2 test-config configset_get_value foo.bar .git/config 2>actual &&\n>  \ttest_cmp expect actual\n>  '\n> diff --git a/test-config.c b/test-config.c\n> index cad35f4..9dd1b22 100644\n> --- a/test-config.c\n> +++ b/test-config.c\n> @@ -86,8 +86,9 @@ int main(int argc, char **argv)\n>  \t\t}\n>  \t} else if (!strcmp(argv[1], \"configset_get_value\")) {\n>  \t\tfor (i = 3; i < argc; i++) {\n> -\t\t\tif (git_configset_add_file(&cs, argv[i])) {\n> -\t\t\t\tfprintf(stderr, \"Error reading configuration file %s.\\n\", argv[i]);\n> +\t\t\tint err;\n> +\t\t\tif ((err = git_configset_add_file(&cs, argv[i]))) {\n> +\t\t\t\tfprintf(stderr, \"Error (%d) reading configuration file %s.\\n\", err, argv[i]);\n>  \t\t\t\tgoto exit2;\n>  \t\t\t}\n>  \t\t}\n> @@ -103,8 +104,9 @@ int main(int argc, char **argv)\n>  \t\t}\n>  \t} else if (!strcmp(argv[1], \"configset_get_value_multi\")) {\n>  \t\tfor (i = 3; i < argc; i++) {\n> -\t\t\tif (git_configset_add_file(&cs, argv[i])) {\n> -\t\t\t\tfprintf(stderr, \"Error reading configuration file %s.\\n\", argv[i]);\n> +\t\t\tint err;\n> +\t\t\tif ((err = git_configset_add_file(&cs, argv[i]))) {\n> +\t\t\t\tfprintf(stderr, \"Error (%d) reading configuration file %s.\\n\", err, argv[i]);\n>  \t\t\t\tgoto exit2;\n>  \t\t\t}\n>  \t\t}\n> \n"},{"id":"246170","messageId":"vpqzjg9z115.fsf@anie.imag.fr","threadId":"37130","inReplyTo":"53C6ABEB.3060205@gmail.com","subject":"Re: [PATCH v9r2 1/2] add `config_set` API for caching config-like files","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2014-07-16T17:06:14Z","receivedAt":"2014-07-16T17:06:14Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Tanay Abhra <tanayabh@gmail.com> writes:\n\n> On 7/16/2014 9:36 PM, Matthieu Moy wrote:\n>> Tanay Abhra <tanayabh@gmail.com> writes:\n>>> +static void git_config_check_init(void)\n>>> +{\n>>> +\tif (the_config_set.hash_initialized)\n>>> +\t\treturn;\n>>> +\tgit_configset_init(&the_config_set);\n>>> +\tgit_config(config_set_callback, &the_config_set);\n>>> +}\n>> \n>> So, you're now ignoring the return value of git_config. What is the\n>> rationale for this? In particular, why did you reject the \"die\"\n>> possibility (I understood that you were inclined to take this option, so\n>> I'm curious why you changed your mind).\n>>\n>\n> The errors (non accessible, non existent files etc) were already being caught by\n> git_config_early(). Since git_config() only returns positive values except\n> the weird race case you mentioned, I thought the die confused the reader\n> of the patch more than it provided error checking. I also tried myself\n> simulating the race condition but failed. All the callers of git_config()\n> also ignore the return value, so I ended up ignoring the return value myself.\n\nOK. My preference would be to die, but your argument makes sense.\n\n> But I do think that an access_or_warn() check should be put on git config --file\n> and git_configset_add_file since other parts of git follow it. What do\n> you think about it, still I will send followup patch correcting the git config\n> --file condition where it silently ignores the file access error and continues?\n\nI think it should be done, but should not be your priority (i.e. it's\ngood to do it, but only if the patch is trivial enough).\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"246171","messageId":"vpqpph5z0tk.fsf@anie.imag.fr","threadId":"37130","inReplyTo":"53C6AE9F.2030908@gmail.com","subject":"Re: [PATCH 2/3] fixup for patch 2: actually check the return value","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2014-07-16T17:10:47Z","receivedAt":"2014-07-16T17:10:47Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Tanay Abhra <tanayabh@gmail.com> writes:\n\n> I think it would be unnecessary for the current iteration.\n> Currently git_configset_add_file has only two possible return values\n> -1 or 0.\n\nYes. My point is just to check that statement in tests. (I'm usually\nwary of statements like \"is obviously true so I don't need to test\nit\" ;-) )\n\n> I could add specialized error values for ENOENT or ENOTDIR or EACCES,\n> but the logs show that we silently ignore the first two. I can add an\n> access warn for the third. What do you think?\n\nI think the code is fine as it is.\n\nBut anyway, it's not terribly important.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"246221","messageId":"vpqvbqwpalt.fsf@anie.imag.fr","threadId":"37130","inReplyTo":"53C6C2BD.3030703@gmail.com","subject":"Re: [PATCH v9r2 1/2] add `config_set` API for caching config-like files","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2014-07-17T10:01:50Z","receivedAt":"2014-07-17T10:01:50Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Tanay Abhra <tanayabh@gmail.com> writes:\n\n> I have a question about renaming git_config() to git_config_raw().\n> I was working on a patch and I am getting stuck here.\n> It fails in t1001, t1020 & t4018. The reason for the last test failure\n> is unknown right now. For the first two, it boils down to this,\n>\n> \t\tgit init --bare bare-ancestor-aliased.git &&\n> \t\tcd bare-ancestor-aliased.git &&\n> \t\techo \"[alias] aliasedinit = init\" >>config &&\n> \t\tmkdir plain-nested &&\n> \t\tcd plain-nested &&\n> \t\tgit aliasedinit\n\nGit tries to read .git/config relative to the current directory, and\ntries to resolve aliases from it. The problem is: if one tries to do\nthis from a subdirectory inside the repo, .git/config is not the right\npath, and the alias lookup fails.\n\nI'll investigate more later.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"246223","messageId":"53C7AE43.5080402@gmail.com","threadId":"37130","inReplyTo":"vpqvbqwpalt.fsf@anie.imag.fr","subject":"Re: [PATCH v9r2 1/2] add `config_set` API for caching config-like files","fromName":"Tanay Abhra","fromEmail":"tanayabh@gmail.com","sentAt":"2014-07-17T11:06:43Z","receivedAt":"2014-07-17T11:06:43Z","isPatch":true,"sender":{"key":"tanayabh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2097229?v=4"},"body":"On 7/17/2014 3:31 PM, Matthieu Moy wrote:\n> Tanay Abhra <tanayabh@gmail.com> writes:\n> \n>> I have a question about renaming git_config() to git_config_raw().\n>> I was working on a patch and I am getting stuck here.\n>> It fails in t1001, t1020 & t4018. The reason for the last test failure\n>> is unknown right now. For the first two, it boils down to this,\n>>\n>> \t\tgit init --bare bare-ancestor-aliased.git &&\n>> \t\tcd bare-ancestor-aliased.git &&\n>> \t\techo \"[alias] aliasedinit = init\" >>config &&\n>> \t\tmkdir plain-nested &&\n>> \t\tcd plain-nested &&\n>> \t\tgit aliasedinit\n> \n> Git tries to read .git/config relative to the current directory, and\n> tries to resolve aliases from it. The problem is: if one tries to do\n> this from a subdirectory inside the repo, .git/config is not the right\n> path, and the alias lookup fails.\n> \n> I'll investigate more later.\n>\n\nHmn, this does the trick,\n-- 8< --\ndiff --git a/cache.h b/cache.h\nindex c67639d..66b52f1 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1272,6 +1272,7 @@ extern int git_config_from_buf(config_fn_t fn, const char *name,\n \t\t\t       const char *buf, size_t len, void *data);\n extern void git_config_push_parameter(const char *text);\n extern int git_config_from_parameters(config_fn_t fn, void *data);\n+extern int git_config_raw(config_fn_t fn, void *);\n extern int git_config(config_fn_t fn, void *);\n extern int git_config_with_options(config_fn_t fn, void *,\n \t\t\t\t   struct git_config_source *config_source,\ndiff --git a/config.c b/config.c\nindex d14f761..9e3f99a 100644\n--- a/config.c\n+++ b/config.c\n@@ -1222,11 +1222,36 @@ int git_config_with_options(config_fn_t fn, void *data,\n \treturn ret;\n }\n\n-int git_config(config_fn_t fn, void *data)\n+extern int git_config_raw(config_fn_t fn, void *data)\n {\n \treturn git_config_with_options(fn, data, NULL, 1);\n }\n\n+int git_configset_iter(struct config_set *cs, config_fn_t fn, void *data)\n+{\n+\tint i;\n+\tstruct string_list *strptr;\n+\tstruct config_set_element *entry;\n+\tstruct hashmap_iter iter;\n+\thashmap_iter_init(&cs->config_hash, &iter);\n+\twhile ((entry = hashmap_iter_next(&iter))) {\n+\t\tstrptr = &entry->value_list;\n+\t\tfor (i = 0; i < strptr->nr; i++) {\n+\t\t\tif (fn(entry->key, strptr->items[i].string, data) < 0)\n+\t\t\t\tdie(\"bad config file\");\n+\t\t}\n+\t}\n+\treturn 0;\n+}\n+\n+static void git_config_check_init(void);\n+\n+int git_config(config_fn_t fn, void *data)\n+{\n+\tgit_config_check_init();\n+\treturn git_configset_iter(&the_config_set, fn, data);\n+}\n+\n static struct config_set_element *configset_find_element(struct config_set *cs, const char *key)\n {\n \tstruct config_set_element k;\n@@ -1409,7 +1434,7 @@ static void git_config_check_init(void)\n \tif (the_config_set.hash_initialized)\n \t\treturn;\n \tgit_configset_init(&the_config_set);\n-\tgit_config(config_hash_callback, &the_config_set);\n+\tgit_config_raw(config_hash_callback, &the_config_set);\n }\n\n void git_config_clear(void)\ndiff --git a/pager.c b/pager.c\nindex 8b5cbc5..b4237e6 100644\n--- a/pager.c\n+++ b/pager.c\n@@ -177,7 +177,7 @@ int check_pager_config(const char *cmd)\n \tc.cmd = cmd;\n \tc.want = -1;\n \tc.value = NULL;\n-\tgit_config(pager_command_config, &c);\n+\tgit_config_raw(pager_command_config, &c);\n \tif (c.value)\n \t\tpager_program = c.value;\n \treturn c.want;\n-- 8< --\n\nThe offending part is in git.c, line number 540:\n\nstatic void execv_dashed_external(const char **argv)\n{\n\tstruct strbuf cmd = STRBUF_INIT;\n\tconst char *tmp;\n\tint status;\n\n\tif (use_pager == -1)\n\t\tuse_pager = check_pager_config(argv[0]);\n----------------------------^^^^^^^^^^^^^^^^^^^^^^^^^^^\n\tcommit_pager_choice();\n\n\nwhich cause git_config() to be called before handle_alias\ncan call it in git.c line no 587,\n\nstatic int run_argv(int *argcp, const char ***argv)\n{\n\tint done_alias = 0;\n\n\twhile (1) {\n\t\t/* See if it's a builtin */\n\t\thandle_builtin(*argcp, *argv);\n\n\t\t/* .. then try the external ones */\n\t\texecv_dashed_external(*argv);\n/* calls git_config() first, skips the .git/config file*/\n\t\t/* It could be an alias -- this works around the insanity\n\t\t * of overriding \"git log\" with \"git show\" by having\n\t\t * alias.log = show\n\t\t */\n\t\tif (done_alias)\n\t\t\tbreak;\n\t\tsave_env();\n\t\tif (!handle_alias(argcp, argv))\n/* does setup_git_directory_gently() before calling git_config() */\n\t\t\tbreak;\n\t\tdone_alias = 1;\n\t}\n\n\treturn done_alias;\n}\n\nI am searching for a more elegant solution to this problem.\nThanks,\nTanay Abhra.\n"},{"id":"246224","messageId":"vpqy4vsme4w.fsf@anie.imag.fr","threadId":"37130","inReplyTo":"vpqvbqwpalt.fsf@anie.imag.fr","subject":"Re: [PATCH v9r2 1/2] add `config_set` API for caching config-like files","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2014-07-17T11:13:51Z","receivedAt":"2014-07-17T11:13:51Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n\n> Tanay Abhra <tanayabh@gmail.com> writes:\n>\n>> I have a question about renaming git_config() to git_config_raw().\n>> I was working on a patch and I am getting stuck here.\n>> It fails in t1001, t1020 & t4018. The reason for the last test failure\n>> is unknown right now. For the first two, it boils down to this,\n>>\n>> \t\tgit init --bare bare-ancestor-aliased.git &&\n>> \t\tcd bare-ancestor-aliased.git &&\n>> \t\techo \"[alias] aliasedinit = init\" >>config &&\n>> \t\tmkdir plain-nested &&\n>> \t\tcd plain-nested &&\n>> \t\tgit aliasedinit\n>\n> Git tries to read .git/config relative to the current directory, and\n> tries to resolve aliases from it. The problem is: if one tries to do\n> this from a subdirectory inside the repo, .git/config is not the right\n> path, and the alias lookup fails.\n>\n> I'll investigate more later.\n\nThis fixes the first two tests (it should be squashed into your PATCH 1\nregardless of the rename git_config -> git_config_raw):\n\ncommit 42315d10e21a1273b73671a3f8c9f7640c4feb44 (HEAD, config-v9)\nAuthor: Matthieu Moy <Matthieu.Moy@imag.fr>\nDate:   Thu Jul 17 13:12:21 2014 +0200\n\n    clear the config cache in setup_git_dir\n\ndiff --git a/setup.c b/setup.c\nindex 0a22f8b..c0d31f5 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -625,6 +625,15 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)\n        int one_filesystem = 1;\n \n        /*\n+        * We may have read an incomplete configuration before\n+        * setting-up the git directory. If so, clear the cache so\n+        * that the next queries to the configuration reload complete\n+        * configuration (including the per-repo config file that we\n+        * ignored previously).\n+        */\n+       git_config_clear();\n+\n+       /*\n         * Let's assume that we are in a git repository.\n         * If it turns out later that we are somewhere else, the value will be\n         * updated accordingly.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"246225","messageId":"vpqiomwmd5t.fsf@anie.imag.fr","threadId":"37130","inReplyTo":"53C7AE43.5080402@gmail.com","subject":"Re: [PATCH v9r2 1/2] add `config_set` API for caching config-like files","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2014-07-17T11:34:54Z","receivedAt":"2014-07-17T11:34:54Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Tanay Abhra <tanayabh@gmail.com> writes:\n\n> I am searching for a more elegant solution to this problem.\n\nThe \"efficient\" (not sure about elegant) solution would be to keep one\nconfigset per file, and re-parse only the files needed.\n\nI find the solution I posted in the other thread relatively \"elegant\":\ninvalidate the config cache when things change.\n\nThe last test failure seems more tricky. This (dirty) patch fixes the\nfailure:\n\n--- a/builtin/diff.c\n+++ b/builtin/diff.c\n@@ -255,6 +255,8 @@ static int builtin_diff_files(struct rev_info *revs, int argc, const char **argv\n        return run_diff_files(revs, options);\n }\n \n+int git_config_raw(config_fn_t fn, void *data);\n+\n int cmd_diff(int argc, const char **argv, const char *prefix)\n {\n        int i;\n@@ -317,7 +319,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)\n \n        if (!no_index)\n                gitmodules_config();\n-       git_config(git_diff_ui_config, NULL);\n+       git_config_raw(git_diff_ui_config, NULL);\n \n        init_revisions(&rev, prefix);\n \n\nBut this one does not:\n\ndiff --git a/builtin/diff.c b/builtin/diff.c\nindex 0f247d2..2012e81 100644\n--- a/builtin/diff.c\n+++ b/builtin/diff.c\n@@ -317,7 +317,9 @@ int cmd_diff(int argc, const char **argv, const char *prefix)\n \n        if (!no_index)\n                gitmodules_config();\n+       git_config_clear();\n        git_config(git_diff_ui_config, NULL);\n+       git_config_clear();\n \n        init_revisions(&rev, prefix);\n \n\nSo it's not just a matter of invalid cache not cleared, it's a real\ndifference between git_config() and git_config_raw(). The guilty part is\nprobably userdiff_config(const char *k, const char *v) in userdiff.c,\nthat parses the xfuncname config option.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"246252","messageId":"xmqq1ttjrjt1.fsf@gitster.dls.corp.google.com","threadId":"37130","inReplyTo":"vpqy4vsme4w.fsf@anie.imag.fr","subject":"Re: [PATCH v9r2 1/2] add `config_set` API for caching config-like files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-07-17T17:12:26Z","receivedAt":"2014-07-17T17:12:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n\n> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n>\n>> Tanay Abhra <tanayabh@gmail.com> writes:\n>>\n>>> I have a question about renaming git_config() to git_config_raw().\n>>> I was working on a patch and I am getting stuck here.\n>>> It fails in t1001, t1020 & t4018. The reason for the last test failure\n>>> is unknown right now. For the first two, it boils down to this,\n>>>\n>>> \t\tgit init --bare bare-ancestor-aliased.git &&\n>>> \t\tcd bare-ancestor-aliased.git &&\n>>> \t\techo \"[alias] aliasedinit = init\" >>config &&\n>>> \t\tmkdir plain-nested &&\n>>> \t\tcd plain-nested &&\n>>> \t\tgit aliasedinit\n>>\n>> Git tries to read .git/config relative to the current directory, and\n>> tries to resolve aliases from it. The problem is: if one tries to do\n>> this from a subdirectory inside the repo, .git/config is not the right\n>> path, and the alias lookup fails.\n>>\n>> I'll investigate more later.\n>\n> This fixes the first two tests (it should be squashed into your PATCH 1\n> regardless of the rename git_config -> git_config_raw):\n>\n> commit 42315d10e21a1273b73671a3f8c9f7640c4feb44 (HEAD, config-v9)\n> Author: Matthieu Moy <Matthieu.Moy@imag.fr>\n> Date:   Thu Jul 17 13:12:21 2014 +0200\n>\n>     clear the config cache in setup_git_dir\n>\n> diff --git a/setup.c b/setup.c\n> index 0a22f8b..c0d31f5 100644\n> --- a/setup.c\n> +++ b/setup.c\n> @@ -625,6 +625,15 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)\n>         int one_filesystem = 1;\n>  \n>         /*\n> +        * We may have read an incomplete configuration before\n> +        * setting-up the git directory. If so, clear the cache so\n> +        * that the next queries to the configuration reload complete\n> +        * configuration (including the per-repo config file that we\n> +        * ignored previously).\n> +        */\n> +       git_config_clear();\n\nVery sensible.\n\n\n> +       /*\n>          * Let's assume that we are in a git repository.\n>          * If it turns out later that we are somewhere else, the value will be\n>          * updated accordingly.\n"}]}