{"thread":{"id":"37157","subject":"[PATCH v10 0/4] git config cache & special querying api utilizing the cache","startedAt":"2014-07-18T09:18:58Z","lastAt":"2014-07-18T21:52:40Z","messageCount":11,"participants":["Tanay Abhra","Matthieu Moy","Junio C Hamano"],"isPatch":true,"patchVersion":10,"patchTotal":4},"messages":[{"id":"246290","messageId":"1405675142-20300-1-git-send-email-tanayabh@gmail.com","threadId":"37157","inReplyTo":null,"subject":"[PATCH v10 0/4] git config cache & special querying api utilizing the cache","fromName":"Tanay Abhra","fromEmail":"tanayabh@gmail.com","sentAt":"2014-07-18T09:18:58Z","receivedAt":"2014-07-18T09:18:58Z","isPatch":true,"sender":{"key":"tanayabh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2097229?v=4"},"body":"Hi,\n\n[PATCH v10]: Minor fixes according to [12]. Re added string_list initializer function.\n\tThanks to Junio and Matthieu for their suggestions.\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[12] http://thread.gmane.org/gmane.comp.version-control.git/253562\n\n\nTanay Abhra (4):\n  string-list: add string_list initialiser helper functions\n  Use string-list initializaer functions to rewrite\n  config set\n  test-config\n\n .gitignore                                  |   1 +\n Documentation/technical/api-config.txt      | 137 +++++++++++++++\n Documentation/technical/api-string-list.txt |   5 +\n Makefile                                    |   1 +\n builtin/commit.c                            |   3 +-\n cache.h                                     |  30 ++++\n config.c                                    | 263 ++++++++++++++++++++++++++++\n merge-recursive.c                           |   9 +-\n string-list.c                               |   6 +\n string-list.h                               |   2 +\n submodule.c                                 |   5 +-\n t/t1308-config-set.sh                       | 200 +++++++++++++++++++++\n test-config.c                               | 142 +++++++++++++++\n transport.c                                 |   4 +-\n 14 files changed, 793 insertions(+), 15 deletions(-)\n create mode 100755 t/t1308-config-set.sh\n create mode 100644 test-config.c\n\n-- \n1.9.0.GIT\n"},{"id":"246291","messageId":"1405675142-20300-2-git-send-email-tanayabh@gmail.com","threadId":"37157","inReplyTo":"1405675142-20300-1-git-send-email-tanayabh@gmail.com","subject":"[PATCH v10 1/4] string-list: add string_list initializer helper function","fromName":"Tanay Abhra","fromEmail":"tanayabh@gmail.com","sentAt":"2014-07-18T09:18:59Z","receivedAt":"2014-07-18T09:18:59Z","isPatch":true,"sender":{"key":"tanayabh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2097229?v=4"},"body":"The string-list API has STRING_LIST_INIT_* macros to be used\nto define variables with initializers, but lacks functions\nto initialise an uninitialized piece of memory to be used as\na string-list at the run-time.\nIntroduce `string_list_init()` function for that.\n\nSigned-off-by: Tanay Abhra <tanayabh@gmail.com>\n---\n Documentation/technical/api-string-list.txt | 5 +++++\n string-list.c                               | 6 ++++++\n string-list.h                               | 2 ++\n 3 files changed, 13 insertions(+)\n\ndiff --git a/Documentation/technical/api-string-list.txt b/Documentation/technical/api-string-list.txt\nindex f1add51..d51a657 100644\n--- a/Documentation/technical/api-string-list.txt\n+++ b/Documentation/technical/api-string-list.txt\n@@ -68,6 +68,11 @@ Functions\n \n * General ones (works with sorted and unsorted lists as well)\n \n+`string_list_init`::\n+\n+\tInitialize the members of the string_list, set `strdup_strings`\n+\tmember according to the value of the second parameter.\n+\n `filter_string_list`::\n \n \tApply a function to each item in a list, retaining only the\ndiff --git a/string-list.c b/string-list.c\nindex aabb25e..db38b62 100644\n--- a/string-list.c\n+++ b/string-list.c\n@@ -1,6 +1,12 @@\n #include \"cache.h\"\n #include \"string-list.h\"\n \n+void string_list_init(struct string_list *list, int strdup_strings)\n+{\n+\tmemset(list, 0, sizeof(*list));\n+\tlist->strdup_strings = strdup_strings;\n+}\n+\n /* if there is no exact match, point to the index where the entry could be\n  * inserted */\n static int get_entry_index(const struct string_list *list, const char *string,\ndiff --git a/string-list.h b/string-list.h\nindex dd5e294..494eb5d 100644\n--- a/string-list.h\n+++ b/string-list.h\n@@ -18,6 +18,8 @@ struct string_list {\n #define STRING_LIST_INIT_NODUP { NULL, 0, 0, 0, NULL }\n #define STRING_LIST_INIT_DUP   { NULL, 0, 0, 1, NULL }\n \n+void string_list_init(struct string_list *list, int strdup_strings);\n+\n void print_string_list(const struct string_list *p, const char *text);\n void string_list_clear(struct string_list *list, int free_util);\n \n-- \n1.9.0.GIT\n"},{"id":"246292","messageId":"1405675142-20300-3-git-send-email-tanayabh@gmail.com","threadId":"37157","inReplyTo":"1405675142-20300-1-git-send-email-tanayabh@gmail.com","subject":"[PATCH v10 2/4] replace memset with string-list initializers","fromName":"Tanay Abhra","fromEmail":"tanayabh@gmail.com","sentAt":"2014-07-18T09:19:00Z","receivedAt":"2014-07-18T09:19:00Z","isPatch":true,"sender":{"key":"tanayabh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2097229?v=4"},"body":"Using memset and then manually setting values of the string-list\nmembers is not future proof as the internal representation of\nstring-list may change any time.\nUse `string_list_init()` or STRING_LIST_INIT_* macros instead of\nmemset.\n\nSigned-off-by: Tanay Abhra <tanayabh@gmail.com>\n---\n builtin/commit.c  | 3 +--\n merge-recursive.c | 9 +++------\n submodule.c       | 5 +----\n transport.c       | 4 +---\n 4 files changed, 6 insertions(+), 15 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 72eb3be..5b196ca 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -420,8 +420,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix,\n \t\t\tdie(_(\"cannot do a partial commit during a cherry-pick.\"));\n \t}\n \n-\tmemset(&partial, 0, sizeof(partial));\n-\tpartial.strdup_strings = 1;\n+\tstring_list_init(&partial, 1);\n \tif (list_paths(&partial, !current_head ? NULL : \"HEAD\", prefix, &pathspec))\n \t\texit(1);\n \ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 5814d05..1d332b8 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -2059,12 +2059,9 @@ void init_merge_options(struct merge_options *o)\n \tif (o->verbosity >= 5)\n \t\to->buffer_output = 0;\n \tstrbuf_init(&o->obuf, 0);\n-\tmemset(&o->current_file_set, 0, sizeof(struct string_list));\n-\to->current_file_set.strdup_strings = 1;\n-\tmemset(&o->current_directory_set, 0, sizeof(struct string_list));\n-\to->current_directory_set.strdup_strings = 1;\n-\tmemset(&o->df_conflict_file_set, 0, sizeof(struct string_list));\n-\to->df_conflict_file_set.strdup_strings = 1;\n+\tstring_list_init(&o->current_file_set, 1);\n+\tstring_list_init(&o->current_directory_set, 1);\n+\tstring_list_init(&o->df_conflict_file_set, 1);\n }\n \n int parse_merge_opt(struct merge_options *o, const char *s)\ndiff --git a/submodule.c b/submodule.c\nindex 48e3b44..c3a61e7 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -544,10 +544,7 @@ static int push_submodule(const char *path)\n int push_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_name)\n {\n \tint i, ret = 1;\n-\tstruct string_list needs_pushing;\n-\n-\tmemset(&needs_pushing, 0, sizeof(struct string_list));\n-\tneeds_pushing.strdup_strings = 1;\n+\tstruct string_list needs_pushing = STRING_LIST_INIT_DUP;\n \n \tif (!find_unpushed_submodules(new_sha1, remotes_name, &needs_pushing))\n \t\treturn 1;\ndiff --git a/transport.c b/transport.c\nindex 59c9727..d32aaf2 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -1188,10 +1188,8 @@ int transport_push(struct transport *transport,\n \t\tif ((flags & (TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND |\n \t\t\t      TRANSPORT_RECURSE_SUBMODULES_CHECK)) && !is_bare_repository()) {\n \t\t\tstruct ref *ref = remote_refs;\n-\t\t\tstruct string_list needs_pushing;\n+\t\t\tstruct string_list needs_pushing = STRING_LIST_INIT_DUP;\n \n-\t\t\tmemset(&needs_pushing, 0, sizeof(struct string_list));\n-\t\t\tneeds_pushing.strdup_strings = 1;\n \t\t\tfor (; ref; ref = ref->next)\n \t\t\t\tif (!is_null_sha1(ref->new_sha1) &&\n \t\t\t\t    find_unpushed_submodules(ref->new_sha1,\n-- \n1.9.0.GIT\n"},{"id":"246293","messageId":"1405675142-20300-4-git-send-email-tanayabh@gmail.com","threadId":"37157","inReplyTo":"1405675142-20300-1-git-send-email-tanayabh@gmail.com","subject":"[PATCH v10 3/4] add `config_set` API for caching config-like files","fromName":"Tanay Abhra","fromEmail":"tanayabh@gmail.com","sentAt":"2014-07-18T09:19:01Z","receivedAt":"2014-07-18T09:19:01Z","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                               | 263 +++++++++++++++++++++++++++++++++\n 3 files changed, 430 insertions(+)\n\ndiff --git a/Documentation/technical/api-config.txt b/Documentation/technical/api-config.txt\nindex 230b3a0..8a86e45 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 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+`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 92fc9f1..e53651c 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1348,6 +1348,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..fe9f399 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,251 @@ 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\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+\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 +1967,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":"246294","messageId":"1405675142-20300-5-git-send-email-tanayabh@gmail.com","threadId":"37157","inReplyTo":"1405675142-20300-1-git-send-email-tanayabh@gmail.com","subject":"[PATCH v10 4/4] test-config: add tests for the config_set API","fromName":"Tanay Abhra","fromEmail":"tanayabh@gmail.com","sentAt":"2014-07-18T09:19:02Z","receivedAt":"2014-07-18T09:19:02Z","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         | 142 +++++++++++++++++++++++++++++++++++\n 4 files changed, 344 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 81e12c0..5bfb234 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 840057c..8d8da72 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..f0307b7\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 (-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+\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 (-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+\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..9dd1b22\n--- /dev/null\n+++ b/test-config.c\n@@ -0,0 +1,142 @@\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\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+\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\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+\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":"246295","messageId":"1849775492.2606791.1405676407530.JavaMail.zimbra@imag.fr","threadId":"37157","inReplyTo":"1405675142-20300-4-git-send-email-tanayabh@gmail.com","subject":"Re: [PATCH v10 3/4] add `config_set` API for caching config-like files","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2014-07-18T09:40:07Z","receivedAt":"2014-07-18T09:40:07Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"----- Original Message -----\n>  Documentation/technical/api-config.txt | 137 +++++++++++++++++\n>  cache.h                                |  30 ++++\n>  config.c                               | 263\n>  +++++++++++++++++++++++++++++++++\n>  3 files changed, 430 insertions(+)\n\nI think the added call to git_config_clear() I proposed yesterday in setup_git_directory_gently_1 should be part of this patch (with the associated comment), just like this call:\n\n> @@ -1707,6 +1967,9 @@ int git_config_set_multivar_in_file(const char\n> *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\nI have limited access to my email and no way to apply the patches today, so I can't do a detailed review. But other than the remark above, I guess the patch series is now all right and ready to cook in pu.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"246297","messageId":"53C8EE71.5000703@gmail.com","threadId":"37157","inReplyTo":"1849775492.2606791.1405676407530.JavaMail.zimbra@imag.fr","subject":"Re: [PATCH v10 3/4] add `config_set` API for caching config-like files","fromName":"Tanay Abhra","fromEmail":"tanayabh@gmail.com","sentAt":"2014-07-18T09:52:49Z","receivedAt":"2014-07-18T09:52:49Z","isPatch":true,"sender":{"key":"tanayabh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2097229?v=4"},"body":"\n\nOn 7/18/2014 3:10 PM, Matthieu Moy wrote:\n> ----- Original Message -----\n>>  Documentation/technical/api-config.txt | 137 +++++++++++++++++\n>>  cache.h                                |  30 ++++\n>>  config.c                               | 263\n>>  +++++++++++++++++++++++++++++++++\n>>  3 files changed, 430 insertions(+)\n> \n> I think the added call to git_config_clear() I proposed yesterday in setup_git_directory_gently_1 should be part of this patch (with the associated comment), just like this call:\n>\n\nOh, my bad, I thought you meant that I incorporate it with the git_config_raw() patch.\nI wanted to ask, can we call setup_git_directory_gently() earlier in execv_dashed_external()\nin git.c, which calls check_pager_config() for the first time which causes the\nincomplete cache to formed.\nIf we can do it, we won't have to clear the cache every time setup_git_directory_gently_1()\nis called.\n\n>> @@ -1707,6 +1967,9 @@ int git_config_set_multivar_in_file(const char\n>> *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> \n> I have limited access to my email and no way to apply the patches today, so I can't do a detailed review. But other than the remark above, I guess the patch series is now all right and ready to cook in pu.\n> \n"},{"id":"246299","messageId":"912531111.2607532.1405677832726.JavaMail.zimbra@imag.fr","threadId":"37157","inReplyTo":"53C8EE71.5000703@gmail.com","subject":"Re: [PATCH v10 3/4] add `config_set` API for caching config-like files","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2014-07-18T10:03:52Z","receivedAt":"2014-07-18T10:03:52Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"I think my proposal is a fix, and yours is a workaround. Semantically, calling setup_git_directory changes the config, so it should invalidate the cache. Your proposal consists in not filling-in the cache before it is invalidated, which works, but isn't very future-proof: if anybody fills-in the cache before the setup in the future, it will break it.\n\nNow, both options can go together, then moving the check_pager_config call after setup_git_directory would avoid re-filling-in the cache. Intuitively, I'd say it's even needed in case git is started from a subdirectory and the .git/config contains pager-relevant stuff, but there might be complications.\n\n----- Original Message -----\n> \n> \n> On 7/18/2014 3:10 PM, Matthieu Moy wrote:\n> > ----- Original Message -----\n> >>  Documentation/technical/api-config.txt | 137 +++++++++++++++++\n> >>  cache.h                                |  30 ++++\n> >>  config.c                               | 263\n> >>  +++++++++++++++++++++++++++++++++\n> >>  3 files changed, 430 insertions(+)\n> > \n> > I think the added call to git_config_clear() I proposed yesterday in\n> > setup_git_directory_gently_1 should be part of this patch (with the\n> > associated comment), just like this call:\n> >\n> \n> Oh, my bad, I thought you meant that I incorporate it with the\n> git_config_raw() patch.\n> I wanted to ask, can we call setup_git_directory_gently() earlier in\n> execv_dashed_external()\n> in git.c, which calls check_pager_config() for the first time which causes\n> the\n> incomplete cache to formed.\n> If we can do it, we won't have to clear the cache every time\n> setup_git_directory_gently_1()\n> is called.\n> \n> >> @@ -1707,6 +1967,9 @@ int git_config_set_multivar_in_file(const char\n> >> *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> > \n> > I have limited access to my email and no way to apply the patches today, so\n> > I can't do a detailed review. But other than the remark above, I guess the\n> > patch series is now all right and ready to cook in pu.\n> > \n> \n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"246318","messageId":"xmqqzjg6mwhm.fsf@gitster.dls.corp.google.com","threadId":"37157","inReplyTo":"1405675142-20300-5-git-send-email-tanayabh@gmail.com","subject":"Re: [PATCH v10 4/4] test-config: add tests for the config_set API","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-07-18T17:01:57Z","receivedAt":"2014-07-18T17:01:57Z","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> +test_expect_success 'proper error on non-existant files' '\n> +\techo \"Error (-1) reading configuration file non-existant-file.\" >expect &&\n\ns/tant/tent/ perhaps?\ncf. http://en.wiktionary.org/wiki/non-existant\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 (-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\nDoesn't this one need some prerequisite?\n"},{"id":"246320","messageId":"xmqqvbqumvvg.fsf@gitster.dls.corp.google.com","threadId":"37157","inReplyTo":"1405675142-20300-2-git-send-email-tanayabh@gmail.com","subject":"Re: [PATCH v10 1/4] string-list: add string_list initializer helper function","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-07-18T17:15:15Z","receivedAt":"2014-07-18T17:15:15Z","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> The string-list API has STRING_LIST_INIT_* macros to be used\n> to define variables with initializers, but lacks functions\n> to initialise an uninitialized piece of memory to be used as\n> a string-list at the run-time.\n> Introduce `string_list_init()` function for that.\n>\n> Signed-off-by: Tanay Abhra <tanayabh@gmail.com>\n> ---\n\nLooks sensible, modulo s/initialise/initialize/ which perhaps is\noriginally my fault.\n\nWill queue; thanks.\n\n>  Documentation/technical/api-string-list.txt | 5 +++++\n>  string-list.c                               | 6 ++++++\n>  string-list.h                               | 2 ++\n>  3 files changed, 13 insertions(+)\n>\n> diff --git a/Documentation/technical/api-string-list.txt b/Documentation/technical/api-string-list.txt\n> index f1add51..d51a657 100644\n> --- a/Documentation/technical/api-string-list.txt\n> +++ b/Documentation/technical/api-string-list.txt\n> @@ -68,6 +68,11 @@ Functions\n>  \n>  * General ones (works with sorted and unsorted lists as well)\n>  \n> +`string_list_init`::\n> +\n> +\tInitialize the members of the string_list, set `strdup_strings`\n> +\tmember according to the value of the second parameter.\n> +\n>  `filter_string_list`::\n>  \n>  \tApply a function to each item in a list, retaining only the\n> diff --git a/string-list.c b/string-list.c\n> index aabb25e..db38b62 100644\n> --- a/string-list.c\n> +++ b/string-list.c\n> @@ -1,6 +1,12 @@\n>  #include \"cache.h\"\n>  #include \"string-list.h\"\n>  \n> +void string_list_init(struct string_list *list, int strdup_strings)\n> +{\n> +\tmemset(list, 0, sizeof(*list));\n> +\tlist->strdup_strings = strdup_strings;\n> +}\n> +\n>  /* if there is no exact match, point to the index where the entry could be\n>   * inserted */\n>  static int get_entry_index(const struct string_list *list, const char *string,\n> diff --git a/string-list.h b/string-list.h\n> index dd5e294..494eb5d 100644\n> --- a/string-list.h\n> +++ b/string-list.h\n> @@ -18,6 +18,8 @@ struct string_list {\n>  #define STRING_LIST_INIT_NODUP { NULL, 0, 0, 0, NULL }\n>  #define STRING_LIST_INIT_DUP   { NULL, 0, 0, 1, NULL }\n>  \n> +void string_list_init(struct string_list *list, int strdup_strings);\n> +\n>  void print_string_list(const struct string_list *p, const char *text);\n>  void string_list_clear(struct string_list *list, int free_util);\n"},{"id":"246341","messageId":"vpqwqba1gif.fsf@anie.imag.fr","threadId":"37157","inReplyTo":"xmqqzjg6mwhm.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v10 4/4] test-config: add tests for the config_set API","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2014-07-18T21:52:40Z","receivedAt":"2014-07-18T21:52:40Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Tanay Abhra <tanayabh@gmail.com> writes:\n>\n>> +test_expect_success 'proper error on non-existant files' '\n>> +\techo \"Error (-1) reading configuration file non-existant-file.\" >expect &&\n>\n> s/tant/tent/ perhaps?\n> cf. http://en.wiktionary.org/wiki/non-existant\n\nIt seems so. My bad.\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 (-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>\n> Doesn't this one need some prerequisite?\n\nMy bad too ;-). There should have been SANITY,POSIXPERM I guess (Tanay:\nsearch \"prereq\" in t/README if you don't see what we're talking about).\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"}]}