{"thread":{"id":"37253","subject":"[PATCH v5 0/7] Rewrite `git_config()` using config-set API","startedAt":"2014-07-31T09:44:10Z","lastAt":"2014-07-31T11:58:01Z","messageCount":14,"participants":["Tanay Abhra","Matthieu Moy"],"isPatch":true,"patchVersion":5,"patchTotal":7},"messages":[{"id":"247016","messageId":"1406799857-28048-1-git-send-email-tanayabh@gmail.com","threadId":"37253","inReplyTo":null,"subject":"[PATCH v5 0/7] Rewrite `git_config()` using config-set API","fromName":"Tanay Abhra","fromEmail":"tanayabh@gmail.com","sentAt":"2014-07-31T09:44:10Z","receivedAt":"2014-07-31T09:44:10Z","isPatch":true,"sender":{"key":"tanayabh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2097229?v=4"},"body":"[PATCH v5]: New patch added (3/7). git_config() now returns void.\n\n[PATCH v4]: One style nit corrected, also added key to error messages.\n\tDiff with v3 at the bottom for easy review.\n\n[PATCH V3]:All the suggestions in [3] applied. Built on top of [1].\n\n[PATCH V2]: All the suggestions in [2] incorporated. git_config() now follows\n\tcorrect parsing order. Reordered the patches. Removed xfuncname patch\n\tas it was unnecssary.\n\nThis series builds on the top of topic[1] in the mailing list with name\n\"git config cache & special querying API utilizing the cache\".\n\nThis series aims to do these three things,\n\n* Use the config-set API to rewrite git_config().\n\n* Solve any legacy bugs in the previous system while at it.\n\n* To be feature complete compared to the previous git_config() implementation,\n  which I think it is now. (added the line number and file name info just for\n  completeness)\n\nAlso, I haven't yet checked the exact improvements but still as a teaser,\ngit status now only rereads the configuration files twice instead of four\ntimes.\n\n[1]: http://thread.gmane.org/gmane.comp.version-control.git/254286\n[2]: http://thread.gmane.org/gmane.comp.version-control.git/254101\n[3]: http://thread.gmane.org/gmane.comp.version-control.git/254211\n\nTanay Abhra (7):\n  config.c: fix accuracy of line number in errors\n  add line number and file name info to `config_set`\n  change `git_config()` return value to void\n  rewrite git_config() to use the config-set API\n  add a test for semantic errors in config files\n  config: add `git_die_config()` to the config-set API\n  add tests for `git_config_get_string_const()`\n\n Documentation/technical/api-config.txt |   5 ++\n branch.c                               |   5 +-\n cache.h                                |  27 ++++++-\n config.c                               | 131 +++++++++++++++++++++++++++++----\n t/t1308-config-set.sh                  |  21 ++++++\n t/t4055-diff-context.sh                |   2 +-\n test-config.c                          |  10 +++\n 7 files changed, 181 insertions(+), 20 deletions(-)\n\n-- \n1.9.0.GIT\n"},{"id":"247017","messageId":"1406799857-28048-2-git-send-email-tanayabh@gmail.com","threadId":"37253","inReplyTo":"1406799857-28048-1-git-send-email-tanayabh@gmail.com","subject":"[PATCH v5 1/7] config.c: fix accuracy of line number in errors","fromName":"Tanay Abhra","fromEmail":"tanayabh@gmail.com","sentAt":"2014-07-31T09:44:11Z","receivedAt":"2014-07-31T09:44:11Z","isPatch":true,"sender":{"key":"tanayabh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2097229?v=4"},"body":"From: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>\n\nIf a callback returns a negative value to `git_config*()` family,\nthey call `die()` while printing the line number and the file name.\nCurrently the printed line number is off by one, thus printing the\nwrong line number.\n\nMake `linenr` point to the line we just parsed during the call\nto callback to get accurate line number in error messages.\n\nCommit-message-by: Tanay Abhra <tanayabh@gmail.com>\nSigned-off-by: Tanay Abhra <tanayabh@gmail.com>\n---\n config.c | 12 +++++++++++-\n 1 file changed, 11 insertions(+), 1 deletion(-)\n\ndiff --git a/config.c b/config.c\nindex a191328..ed5fc8e 100644\n--- a/config.c\n+++ b/config.c\n@@ -244,6 +244,7 @@ static int get_next_char(void)\n \t\tcf->linenr++;\n \tif (c == EOF) {\n \t\tcf->eof = 1;\n+\t\tcf->linenr++;\n \t\tc = '\\n';\n \t}\n \treturn c;\n@@ -319,6 +320,7 @@ static int get_value(config_fn_t fn, void *data, struct strbuf *name)\n {\n \tint c;\n \tchar *value;\n+\tint ret;\n \n \t/* Get the full name */\n \tfor (;;) {\n@@ -341,7 +343,15 @@ static int get_value(config_fn_t fn, void *data, struct strbuf *name)\n \t\tif (!value)\n \t\t\treturn -1;\n \t}\n-\treturn fn(name->buf, value, data);\n+\t/*\n+\t * We already consumed the \\n, but we need linenr to point to\n+\t * the line we just parsed during the call to fn to get\n+\t * accurate line number in error messages.\n+\t */\n+\tcf->linenr--;\n+\tret = fn(name->buf, value, data);\n+\tcf->linenr++;\n+\treturn ret;\n }\n \n static int get_extended_base_var(struct strbuf *name, int c)\n-- \n1.9.0.GIT\n"},{"id":"247018","messageId":"1406799857-28048-3-git-send-email-tanayabh@gmail.com","threadId":"37253","inReplyTo":"1406799857-28048-1-git-send-email-tanayabh@gmail.com","subject":"[PATCH v5 2/7] add line number and file name info to `config_set`","fromName":"Tanay Abhra","fromEmail":"tanayabh@gmail.com","sentAt":"2014-07-31T09:44:12Z","receivedAt":"2014-07-31T09:44:12Z","isPatch":true,"sender":{"key":"tanayabh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2097229?v=4"},"body":"Store file name and line number for each key-value pair in the cache\nduring parsing of the configuration files.\n\nSigned-off-by: Tanay Abhra <tanayabh@gmail.com>\n---\n config.c | 21 +++++++++++++++++++--\n 1 file changed, 19 insertions(+), 2 deletions(-)\n\ndiff --git a/config.c b/config.c\nindex ed5fc8e..4a15383 100644\n--- a/config.c\n+++ b/config.c\n@@ -1230,6 +1230,11 @@ int git_config_with_options(config_fn_t fn, void *data,\n \treturn ret;\n }\n \n+struct key_value_info {\n+\tconst char *filename;\n+\tint linenr;\n+};\n+\n int git_config(config_fn_t fn, void *data)\n {\n \treturn git_config_with_options(fn, data, NULL, 1);\n@@ -1260,6 +1265,9 @@ static struct config_set_element *configset_find_element(struct config_set *cs,\n static int configset_add_value(struct config_set *cs, const char *key, const char *value)\n {\n \tstruct config_set_element *e;\n+\tstruct string_list_item *si;\n+\tstruct key_value_info *kv_info = xmalloc(sizeof(*kv_info));\n+\n \te = configset_find_element(cs, key);\n \t/*\n \t * Since the keys are being fed by git_config*() callback mechanism, they\n@@ -1272,7 +1280,16 @@ static int configset_add_value(struct config_set *cs, const char *key, const cha\n \t\tstring_list_init(&e->value_list, 1);\n \t\thashmap_add(&cs->config_hash, e);\n \t}\n-\tstring_list_append_nodup(&e->value_list, value ? xstrdup(value) : NULL);\n+\tsi = string_list_append_nodup(&e->value_list, value ? xstrdup(value) : NULL);\n+\tif (cf) {\n+\t\tkv_info->filename = strintern(cf->name);\n+\t\tkv_info->linenr = cf->linenr;\n+\t} else {\n+\t\t/* for values read from `git_config_from_parameters()` */\n+\t\tkv_info->filename = strintern(\"parameter\");\n+\t\tkv_info->linenr = 0;\n+\t}\n+\tsi->util = kv_info;\n \n \treturn 0;\n }\n@@ -1299,7 +1316,7 @@ void git_configset_clear(struct config_set *cs)\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\tstring_list_clear(&entry->value_list, 1);\n \t}\n \thashmap_free(&cs->config_hash, 1);\n \tcs->hash_initialized = 0;\n-- \n1.9.0.GIT\n"},{"id":"247019","messageId":"1406799857-28048-4-git-send-email-tanayabh@gmail.com","threadId":"37253","inReplyTo":"1406799857-28048-1-git-send-email-tanayabh@gmail.com","subject":"[PATCH v5 3/7] change `git_config()` return value to void","fromName":"Tanay Abhra","fromEmail":"tanayabh@gmail.com","sentAt":"2014-07-31T09:44:13Z","receivedAt":"2014-07-31T09:44:13Z","isPatch":true,"sender":{"key":"tanayabh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2097229?v=4"},"body":"Currently `git_config()` returns an integer signifying an error code.\nDuring a previous rewrite of the function most of the code was shifted\nto `git_config_with_options()`. `git_config_with_options()` normally\nreturns positive values if its `config_source` parameter is set as NULL,\nas most errors are fatal, and non-fatal potential errors are guarded\nby \"if\" statements that are entered only when no error is possible.\n\nStill a negative value can be returned in case of race condition between\n`access_or_die()` & `git_config_from_file()`. Also, all callers of\n`git_config()` ignore the return value except for one case in branch.c.\n\nChange `git_config()` return value to void and make it die if it receives\na negative return value from `git_config_with_options()`.\n\nSigned-off-by: Tanay Abhra <tanayabh@gmail.com>\n---\n branch.c |  5 +----\n cache.h  |  2 +-\n config.c | 16 ++++++++++++++--\n 3 files changed, 16 insertions(+), 7 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex 46e8aa8..735767d 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -161,10 +161,7 @@ int read_branch_desc(struct strbuf *buf, const char *branch_name)\n \tstrbuf_addf(&name, \"branch.%s.description\", branch_name);\n \tcb.config_name = name.buf;\n \tcb.value = NULL;\n-\tif (git_config(read_branch_desc_cb, &cb) < 0) {\n-\t\tstrbuf_release(&name);\n-\t\treturn -1;\n-\t}\n+\tgit_config(read_branch_desc_cb, &cb);\n \tif (cb.value)\n \t\tstrbuf_addstr(buf, cb.value);\n \tstrbuf_release(&name);\ndiff --git a/cache.h b/cache.h\nindex 7292aef..c61919d 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1294,7 +1294,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(config_fn_t fn, void *);\n+extern void 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,\n \t\t\t\t   int respect_includes);\ndiff --git a/config.c b/config.c\nindex 4a15383..c0faaca 100644\n--- a/config.c\n+++ b/config.c\n@@ -1235,9 +1235,21 @@ struct key_value_info {\n \tint linenr;\n };\n \n-int git_config(config_fn_t fn, void *data)\n+void git_config(config_fn_t fn, void *data)\n {\n-\treturn git_config_with_options(fn, data, NULL, 1);\n+\tif (git_config_with_options(fn, data, NULL, 1) < 0)\n+\t\t/*\n+\t\t * git_config_with_options() normally returns only\n+\t\t * positive values, as most errors are fatal, and\n+\t\t * non-fatal potential errors are guarded by \"if\"\n+\t\t * statements that are entered only when no error is\n+\t\t * possible.\n+\t\t *\n+\t\t * If we ever encounter a non-fatal error, it means\n+\t\t * something went really wrong and we should stop\n+\t\t * immediately.\n+\t\t */\n+\t\tdie(\"Unknown error occured while reading the configuration files\");\n }\n \n static struct config_set_element *configset_find_element(struct config_set *cs, const char *key)\n-- \n1.9.0.GIT\n"},{"id":"247021","messageId":"1406799857-28048-5-git-send-email-tanayabh@gmail.com","threadId":"37253","inReplyTo":"1406799857-28048-1-git-send-email-tanayabh@gmail.com","subject":"[PATCH v5 4/7] rewrite git_config() to use the config-set API","fromName":"Tanay Abhra","fromEmail":"tanayabh@gmail.com","sentAt":"2014-07-31T09:44:14Z","receivedAt":"2014-07-31T09:44:14Z","isPatch":true,"sender":{"key":"tanayabh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2097229?v=4"},"body":"Of all the functions in `git_config*()` family, `git_config()` has the\nmost invocations in the whole code base. Each `git_config()` invocation\ncauses config file rereads which can be avoided using the config-set API.\n\nUse the config-set API to rewrite `git_config()` to use the config caching\nlayer to avoid config file rereads on each invocation during a git process\nlifetime. First invocation constructs the cache, and after that for each\nsuccessive invocation, `git_config()` feeds values from the config cache\ninstead of rereading the configuration files.\n\nSigned-off-by: Tanay Abhra <tanayabh@gmail.com>\n---\n cache.h                 | 24 +++++++++++++++++++++\n config.c                | 57 ++++++++++++++++++++++++++++++++++++++++++-------\n t/t4055-diff-context.sh |  2 +-\n 3 files changed, 74 insertions(+), 9 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex c61919d..4d3c6bd 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -8,6 +8,7 @@\n #include \"gettext.h\"\n #include \"convert.h\"\n #include \"trace.h\"\n+#include \"string-list.h\"\n \n #include SHA1_HEADER\n #ifndef git_SHA_CTX\n@@ -1351,9 +1352,32 @@ 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_element {\n+\tstruct hashmap_entry ent;\n+\tchar *key;\n+\tstruct string_list value_list;\n+};\n+\n+struct configset_list_item {\n+\tstruct config_set_element *e;\n+\tint value_index;\n+};\n+\n+/*\n+ * the contents of the list are ordered according to their\n+ * position in the config files and order of parsing the files.\n+ * (i.e. key-value pair at the last position of .git/config will\n+ * be at the last item of the list)\n+ */\n+struct configset_list {\n+\tstruct configset_list_item *items;\n+\tunsigned int nr, alloc;\n+};\n+\n struct config_set {\n \tstruct hashmap config_hash;\n \tint hash_initialized;\n+\tstruct configset_list list;\n };\n \n extern void git_configset_init(struct config_set *cs);\ndiff --git a/config.c b/config.c\nindex c0faaca..4937515 100644\n--- a/config.c\n+++ b/config.c\n@@ -35,12 +35,6 @@ 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@@ -1235,7 +1229,7 @@ struct key_value_info {\n \tint linenr;\n };\n \n-void git_config(config_fn_t fn, void *data)\n+static void git_config_raw(config_fn_t fn, void *data)\n {\n \tif (git_config_with_options(fn, data, NULL, 1) < 0)\n \t\t/*\n@@ -1252,6 +1246,39 @@ void git_config(config_fn_t fn, void *data)\n \t\tdie(\"Unknown error occured while reading the configuration files\");\n }\n \n+static void configset_iter(struct config_set *cs, config_fn_t fn, void *data)\n+{\n+\tint i, value_index;\n+\tstruct string_list *values;\n+\tstruct config_set_element *entry;\n+\tstruct configset_list *list = &cs->list;\n+\tstruct key_value_info *kv_info;\n+\n+\tfor (i = 0; i < list->nr; i++) {\n+\t\tentry = list->items[i].e;\n+\t\tvalue_index = list->items[i].value_index;\n+\t\tvalues = &entry->value_list;\n+\t\tif (fn(entry->key, values->items[value_index].string, data) < 0) {\n+\t\t\tkv_info = values->items[value_index].util;\n+\t\t\tif (!kv_info->linenr)\n+\t\t\t\tdie(\"unable to parse '%s' from command-line config\", entry->key);\n+\t\t\telse\n+\t\t\t\tdie(\"bad config variable '%s' at file line %d in %s\",\n+\t\t\t\t\tentry->key,\n+\t\t\t\t\tkv_info->linenr,\n+\t\t\t\t\tkv_info->filename);\n+\t\t}\n+\t}\n+}\n+\n+static void git_config_check_init(void);\n+\n+void git_config(config_fn_t fn, void *data)\n+{\n+\tgit_config_check_init();\n+\tconfigset_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@@ -1278,6 +1305,7 @@ static int configset_add_value(struct config_set *cs, const char *key, const cha\n {\n \tstruct config_set_element *e;\n \tstruct string_list_item *si;\n+\tstruct configset_list_item *l_item;\n \tstruct key_value_info *kv_info = xmalloc(sizeof(*kv_info));\n \n \te = configset_find_element(cs, key);\n@@ -1293,6 +1321,12 @@ static int configset_add_value(struct config_set *cs, const char *key, const cha\n \t\thashmap_add(&cs->config_hash, e);\n \t}\n \tsi = string_list_append_nodup(&e->value_list, value ? xstrdup(value) : NULL);\n+\n+\tALLOC_GROW(cs->list.items, cs->list.nr + 1, cs->list.alloc);\n+\tl_item = &cs->list.items[cs->list.nr++];\n+\tl_item->e = e;\n+\tl_item->value_index = e->value_list.nr - 1;\n+\n \tif (cf) {\n \t\tkv_info->filename = strintern(cf->name);\n \t\tkv_info->linenr = cf->linenr;\n@@ -1316,6 +1350,9 @@ 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+\tcs->list.nr = 0;\n+\tcs->list.alloc = 0;\n+\tcs->list.items = NULL;\n }\n \n void git_configset_clear(struct config_set *cs)\n@@ -1332,6 +1369,10 @@ void git_configset_clear(struct config_set *cs)\n \t}\n \thashmap_free(&cs->config_hash, 1);\n \tcs->hash_initialized = 0;\n+\tfree(cs->list.items);\n+\tcs->list.nr = 0;\n+\tcs->list.alloc = 0;\n+\tcs->list.items = NULL;\n }\n \n static int config_set_callback(const char *key, const char *value, void *cb)\n@@ -1450,7 +1491,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_set_callback, &the_config_set);\n+\tgit_config_raw(config_set_callback, &the_config_set);\n }\n \n void git_config_clear(void)\ndiff --git a/t/t4055-diff-context.sh b/t/t4055-diff-context.sh\nindex cd04543..741e080 100755\n--- a/t/t4055-diff-context.sh\n+++ b/t/t4055-diff-context.sh\n@@ -79,7 +79,7 @@ test_expect_success 'non-integer config parsing' '\n test_expect_success 'negative integer config parsing' '\n \tgit config diff.context -1 &&\n \ttest_must_fail git diff 2>output &&\n-\ttest_i18ngrep \"bad config file\" output\n+\ttest_i18ngrep \"bad config variable\" output\n '\n \n test_expect_success '-U0 is valid, so is diff.context=0' '\n-- \n1.9.0.GIT\n"},{"id":"247020","messageId":"1406799857-28048-6-git-send-email-tanayabh@gmail.com","threadId":"37253","inReplyTo":"1406799857-28048-1-git-send-email-tanayabh@gmail.com","subject":"[PATCH v5 5/7] add a test for semantic errors in config files","fromName":"Tanay Abhra","fromEmail":"tanayabh@gmail.com","sentAt":"2014-07-31T09:44:15Z","receivedAt":"2014-07-31T09:44:15Z","isPatch":true,"sender":{"key":"tanayabh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2097229?v=4"},"body":"Semantic errors (for example, for alias.* variables NULL values are\nnot allowed) in configuration files cause a die printing the line\nnumber and file name of the offending value.\n\nAdd a test documenting that such errors cause a die printing the\naccurate line number and file name.\n\nSigned-off-by: Tanay Abhra <tanayabh@gmail.com>\n---\n t/t1308-config-set.sh | 11 +++++++++++\n 1 file changed, 11 insertions(+)\n\ndiff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh\nindex 7fdf840..e2f9d0b 100755\n--- a/t/t1308-config-set.sh\n+++ b/t/t1308-config-set.sh\n@@ -197,4 +197,15 @@ test_expect_success 'proper error on error in custom config files' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'check line errors for malformed values' '\n+\tmv .git/config .git/config.old &&\n+\ttest_when_finished \"mv .git/config.old .git/config\" &&\n+\tcat >.git/config <<-\\EOF &&\n+\t[alias]\n+\t\tbr\n+\tEOF\n+\ttest_expect_code 128 git br 2>result &&\n+\tgrep \"fatal: bad config variable '\\''alias.br'\\'' at file line 2 in .git/config\" result\n+'\n+\n test_done\n-- \n1.9.0.GIT\n"},{"id":"247023","messageId":"1406799857-28048-7-git-send-email-tanayabh@gmail.com","threadId":"37253","inReplyTo":"1406799857-28048-1-git-send-email-tanayabh@gmail.com","subject":"[PATCH 6/7] config: add `git_die_config()` to the config-set API","fromName":"Tanay Abhra","fromEmail":"tanayabh@gmail.com","sentAt":"2014-07-31T09:44:16Z","receivedAt":"2014-07-31T09:44:16Z","isPatch":true,"sender":{"key":"tanayabh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2097229?v=4"},"body":"Add `git_die_config` that dies printing the line number and the file name\nof the highest priority value for the configuration variable `key`.\n\nIt has usage in non-callback based config value retrieval where we can\nraise an error and die if there is a semantic error.\nFor example,\n\n\tif (!git_config_get_value(key, &value)) {\n\t\t/* NULL values not allowed */\n\t\tif (!value)\n\t\t\tgit_config_die(key);\n\t\telse\n\t\t\t/* do work */\n\t}\n\nSigned-off-by: Tanay Abhra <tanayabh@gmail.com>\n---\n Documentation/technical/api-config.txt |  5 +++++\n cache.h                                |  1 +\n config.c                               | 27 +++++++++++++++++++++++++--\n 3 files changed, 31 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/technical/api-config.txt b/Documentation/technical/api-config.txt\nindex 815c1ee..e7ec7cc 100644\n--- a/Documentation/technical/api-config.txt\n+++ b/Documentation/technical/api-config.txt\n@@ -155,6 +155,11 @@ as well as retrieval for the queried variable, including:\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+`void git_die_config(const char *key)`::\n+\n+\tDies printing the line number and the file name of the highest\n+\tpriority value for the configuration variable `key`.\n+\n See test-config.c for usage examples.\n \n Value Parsing Helpers\ndiff --git a/cache.h b/cache.h\nindex 4d3c6bd..243f618 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1406,6 +1406,7 @@ 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+extern void git_die_config(const char *key);\n \n extern int committer_ident_sufficiently_given(void);\n extern int author_ident_sufficiently_given(void);\ndiff --git a/config.c b/config.c\nindex 4937515..03369b8 100644\n--- a/config.c\n+++ b/config.c\n@@ -1515,8 +1515,12 @@ const struct string_list *git_config_get_value_multi(const char *key)\n \n int git_config_get_string_const(const char *key, const char **dest)\n {\n+\tint ret;\n \tgit_config_check_init();\n-\treturn git_configset_get_string_const(&the_config_set, key, dest);\n+\tret = git_configset_get_string_const(&the_config_set, key, dest);\n+\tif (ret < 0)\n+\t\tgit_die_config(key);\n+\treturn ret;\n }\n \n int git_config_get_string(const char *key, char **dest)\n@@ -1557,10 +1561,29 @@ int git_config_get_maybe_bool(const char *key, int *dest)\n \n int git_config_get_pathname(const char *key, const char **dest)\n {\n+\tint ret;\n \tgit_config_check_init();\n-\treturn git_configset_get_pathname(&the_config_set, key, dest);\n+\tret = git_configset_get_pathname(&the_config_set, key, dest);\n+\tif (ret < 0)\n+\t\tgit_die_config(key);\n+\treturn ret;\n }\n \n+void git_die_config(const char *key)\n+{\n+\tconst struct string_list *values;\n+\tstruct key_value_info *kv_info;\n+\tvalues = git_config_get_value_multi(key);\n+\tkv_info = values->items[values->nr - 1].util;\n+\tif (!kv_info->linenr)\n+\t\tdie(\"unable to parse '%s' from command-line config\", key);\n+\telse\n+\t\tdie(\"bad config variable '%s' at file line %d in %s\",\n+\t\t\tkey,\n+\t\t\tkv_info->linenr,\n+\t\t\tkv_info->filename);\n+ }\n+\n /*\n  * Find all the stuff for git_config_set() below.\n  */\n-- \n1.9.0.GIT\n"},{"id":"247022","messageId":"1406799857-28048-8-git-send-email-tanayabh@gmail.com","threadId":"37253","inReplyTo":"1406799857-28048-1-git-send-email-tanayabh@gmail.com","subject":"[PATCH 7/7] add tests for `git_config_get_string_const()`","fromName":"Tanay Abhra","fromEmail":"tanayabh@gmail.com","sentAt":"2014-07-31T09:44:17Z","receivedAt":"2014-07-31T09:44:17Z","isPatch":true,"sender":{"key":"tanayabh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2097229?v=4"},"body":"Add tests for `git_config_get_string_const()`, check whether it\ndies printing the line number and the file name if a NULL\nvalue is retrieved for the given key.\n\nSigned-off-by: Tanay Abhra <tanayabh@gmail.com>\n---\n t/t1308-config-set.sh | 10 ++++++++++\n test-config.c         | 10 ++++++++++\n 2 files changed, 20 insertions(+)\n\ndiff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh\nindex e2f9d0b..f012dd6 100755\n--- a/t/t1308-config-set.sh\n+++ b/t/t1308-config-set.sh\n@@ -119,6 +119,16 @@ test_expect_success 'find integer value for a key' '\n \tcheck_config get_int lamb.chop 65\n '\n \n+test_expect_success 'find string value for a key' '\n+\tcheck_config get_string case.baz hask &&\n+\tcheck_config expect_code 1 get_string case.ba \"Value not found for \\\"case.ba\\\"\"\n+'\n+\n+test_expect_success 'check line error when NULL string is queried' '\n+\ttest_expect_code 128 test-config get_string case.foo 2>result &&\n+\tgrep \"fatal: bad config variable '\\''case.foo'\\'' at file line 7 in .git/config\" result\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 '\ndiff --git a/test-config.c b/test-config.c\nindex 9dd1b22..6a77552 100644\n--- a/test-config.c\n+++ b/test-config.c\n@@ -16,6 +16,8 @@\n  *\n  * get_bool -> print bool value for the entered key or die\n  *\n+ * get_string -> print string 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@@ -84,6 +86,14 @@ int main(int argc, char **argv)\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_string\")) {\n+\t\tif (!git_config_get_string_const(argv[2], &v)) {\n+\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\")) {\n \t\tfor (i = 3; i < argc; i++) {\n \t\t\tint err;\n-- \n1.9.0.GIT\n"},{"id":"247026","messageId":"vpqoaw5epsa.fsf@anie.imag.fr","threadId":"37253","inReplyTo":"1406799857-28048-4-git-send-email-tanayabh@gmail.com","subject":"Re: [PATCH v5 3/7] change `git_config()` return value to void","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2014-07-31T11:22:29Z","receivedAt":"2014-07-31T11:22:29Z","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> Signed-off-by: Tanay Abhra <tanayabh@gmail.com>\n\nI think I deserve a bit of credit here ;-).\n\n>  {\n> -\treturn git_config_with_options(fn, data, NULL, 1);\n> +\tif (git_config_with_options(fn, data, NULL, 1) < 0)\n> +\t\t/*\n> +\t\t * git_config_with_options() normally returns only\n> +\t\t * positive values, as most errors are fatal, and\n> +\t\t * non-fatal potential errors are guarded by \"if\"\n> +\t\t * statements that are entered only when no error is\n> +\t\t * possible.\n> +\t\t *\n> +\t\t * If we ever encounter a non-fatal error, it means\n> +\t\t * something went really wrong and we should stop\n> +\t\t * immediately.\n> +\t\t */\n> +\t\tdie(\"Unknown error occured while reading the configuration files\");\n>  }\n\nMy bad, but this should be die(_(\"...\"));, so that the message can be\ntranslated. Not really serious since it's not really meant to be seen by\nthe user, though.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"247027","messageId":"vpqegx1ephk.fsf@anie.imag.fr","threadId":"37253","inReplyTo":"1406799857-28048-5-git-send-email-tanayabh@gmail.com","subject":"Re: [PATCH v5 4/7] rewrite git_config() to use the config-set API","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2014-07-31T11:28:55Z","receivedAt":"2014-07-31T11:28:55Z","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> +\t\t\tif (!kv_info->linenr)\n> +\t\t\t\tdie(\"unable to parse '%s' from command-line config\", entry->key);\n> +\t\t\telse\n> +\t\t\t\tdie(\"bad config variable '%s' at file line %d in %s\",\n\nAlso two missing _(\"...\") here.\n\nNote that there are missing _(\"...\") in config.c before your patch too.\nI'll send a patch to fix this regardless of your series.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"247028","messageId":"vpq8un9epf9.fsf@anie.imag.fr","threadId":"37253","inReplyTo":"1406799857-28048-7-git-send-email-tanayabh@gmail.com","subject":"Re: [PATCH 6/7] config: add `git_die_config()` to the config-set API","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2014-07-31T11:30:18Z","receivedAt":"2014-07-31T11:30:18Z","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> +\tif (!kv_info->linenr)\n> +\t\tdie(\"unable to parse '%s' from command-line config\", key);\n> +\telse\n> +\t\tdie(\"bad config variable '%s' at file line %d in %s\",\n\nAnd two more missing _(...).\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"247030","messageId":"vpq1tt1epbw.fsf@anie.imag.fr","threadId":"37253","inReplyTo":"vpqoaw5epsa.fsf@anie.imag.fr","subject":"Re: [PATCH v5 3/7] change `git_config()` return value to void","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2014-07-31T11:32:19Z","receivedAt":"2014-07-31T11:32:19Z","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>> Signed-off-by: Tanay Abhra <tanayabh@gmail.com>\n>\n> I think I deserve a bit of credit here ;-).\n>\n>>  {\n>> -\treturn git_config_with_options(fn, data, NULL, 1);\n>> +\tif (git_config_with_options(fn, data, NULL, 1) < 0)\n>> +\t\t/*\n>> +\t\t * git_config_with_options() normally returns only\n>> +\t\t * positive values, as most errors are fatal, and\n>> +\t\t * non-fatal potential errors are guarded by \"if\"\n>> +\t\t * statements that are entered only when no error is\n>> +\t\t * possible.\n>> +\t\t *\n>> +\t\t * If we ever encounter a non-fatal error, it means\n>> +\t\t * something went really wrong and we should stop\n>> +\t\t * immediately.\n>> +\t\t */\n>> +\t\tdie(\"Unknown error occured while reading the configuration files\");\n>>  }\n>\n> My bad, but this should be die(_(\"...\"));, so that the message can be\n> translated. Not really serious since it's not really meant to be seen by\n> the user, though.\n\nAlso, other error messages do not start with a capital, hence it should\nbe \"unknown..., not \"Unknown. My bad again.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"247033","messageId":"53DA2E02.4000408@gmail.com","threadId":"37253","inReplyTo":"vpqoaw5epsa.fsf@anie.imag.fr","subject":"Re: [PATCH v5 3/7] change `git_config()` return value to void","fromName":"Tanay Abhra","fromEmail":"tanayabh@gmail.com","sentAt":"2014-07-31T11:52:34Z","receivedAt":"2014-07-31T11:52:34Z","isPatch":true,"sender":{"key":"tanayabh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2097229?v=4"},"body":"\n\nOn 7/31/2014 4:52 PM, Matthieu Moy wrote:\n> Tanay Abhra <tanayabh@gmail.com> writes:\n> \n>> Signed-off-by: Tanay Abhra <tanayabh@gmail.com>\n> \n> I think I deserve a bit of credit here ;-).\n>\n\nYes, but to show credit would I have to write \"from you\" or\n\"signed-off-by\"? :)\n\n>>  {\n>> -\treturn git_config_with_options(fn, data, NULL, 1);\n>> +\tif (git_config_with_options(fn, data, NULL, 1) < 0)\n>> +\t\t/*\n>> +\t\t * git_config_with_options() normally returns only\n>> +\t\t * positive values, as most errors are fatal, and\n>> +\t\t * non-fatal potential errors are guarded by \"if\"\n>> +\t\t * statements that are entered only when no error is\n>> +\t\t * possible.\n>> +\t\t *\n>> +\t\t * If we ever encounter a non-fatal error, it means\n>> +\t\t * something went really wrong and we should stop\n>> +\t\t * immediately.\n>> +\t\t */\n>> +\t\tdie(\"Unknown error occured while reading the configuration files\");\n>>  }\n> \n> My bad, but this should be die(_(\"...\"));, so that the message can be\n> translated. Not really serious since it's not really meant to be seen by\n> the user, though.\n>\n\nNoted. Though there are not many cases till I have read where error messages are marked\nfor translation.\n"},{"id":"247034","messageId":"vpq8un9bv06.fsf@anie.imag.fr","threadId":"37253","inReplyTo":"53DA2E02.4000408@gmail.com","subject":"Re: [PATCH v5 3/7] change `git_config()` return value to void","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2014-07-31T11:58:01Z","receivedAt":"2014-07-31T11:58:01Z","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/31/2014 4:52 PM, Matthieu Moy wrote:\n>> Tanay Abhra <tanayabh@gmail.com> writes:\n>> \n>>> Signed-off-by: Tanay Abhra <tanayabh@gmail.com>\n>> \n>> I think I deserve a bit of credit here ;-).\n>>\n>\n> Yes, but to show credit would I have to write \"from you\" or\n> \"signed-off-by\"? :)\n\nOriginal-patch-by: would be fine.\n\n>> My bad, but this should be die(_(\"...\"));, so that the message can be\n>> translated. Not really serious since it's not really meant to be seen by\n>> the user, though.\n>>\n>\n> Noted. Though there are not many cases till I have read where error messages are marked\n> for translation.\n\nYes, see my other message.\n\nOverall, there are still many untranslated messages in Git's codebase:\n\n$ git grep 'die(_(\"' | wc -l\n517\n$ git grep 'die(\"' | wc -l \n1247\n\nPart of the reason is that die() was written and used far before the\ntranslation effort has started.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"}]}