Merge branch 'ta/config-set-1'
authorJunio C Hamano <gitster@pobox.com>
Thu, 11 Sep 2014 17:33:25 +0000 (10:33 -0700)
committerJunio C Hamano <gitster@pobox.com>
Thu, 11 Sep 2014 17:33:25 +0000 (10:33 -0700)
Use the new caching config-set API in git_config() calls.

* ta/config-set-1:
add tests for `git_config_get_string_const()`
add a test for semantic errors in config files
rewrite git_config() to use the config-set API
config: add `git_die_config()` to the config-set API
change `git_config()` return value to void
add line number and file name info to `config_set`
config.c: fix accuracy of line number in errors
config.c: mark error and warnings strings for translation

1  2 
Documentation/technical/api-config.txt
cache.h
config.c
index 21f280ca6dbd85a1f4d1d599db1393077fc93bdf,7108888b8a2510e9c72b553855c5e1ef3b82e568..0d8b99b368aea13a322382314d88229acda0b1db
@@@ -155,6 -155,19 +155,19 @@@ as well as retrieval for the queried va
        Similar to `git_config_get_string`, but expands `~` or `~user` into
        the user's home directory when found at the beginning of the path.
  
+ `git_die_config(const char *key, const char *err, ...)`::
+       First prints the error message specified by the caller in `err` and then
+       dies printing the line number and the file name of the highest priority
+       value for the configuration variable `key`.
+ `void git_die_config_linenr(const char *key, const char *filename, int linenr)`::
+       Helper function which formats the die error message according to the
+       parameters entered. Used by `git_die_config()`. It can be used by callers
+       handling `git_config_get_value_multi()` to print the correct error message
+       for the desired value.
  See test-config.c for usage examples.
  
  Value Parsing Helpers
@@@ -279,33 -292,4 +292,33 @@@ They all behave similarly to the `git_c
  Writing Config Files
  --------------------
  
 -TODO
 +Git gives multiple entry points in the Config API to write config values to
 +files namely `git_config_set_in_file` and `git_config_set`, which write to
 +a specific config file or to `.git/config` respectively. They both take a
 +key/value pair as parameter.
 +In the end they both call `git_config_set_multivar_in_file` which takes four
 +parameters:
 +
 +- the name of the file, as a string, to which key/value pairs will be written.
 +
 +- the name of key, as a string. This is in canonical "flat" form: the section,
 +  subsection, and variable segments will be separated by dots, and the section
 +  and variable segments will be all lowercase.
 +  E.g., `core.ignorecase`, `diff.SomeType.textconv`.
 +
 +- the value of the variable, as a string. If value is equal to NULL, it will
 +  remove the matching key from the config file.
 +
 +- the value regex, as a string. It will disregard key/value pairs where value
 +  does not match.
 +
 +- a multi_replace value, as an int. If value is equal to zero, nothing or only
 +  one matching key/value is replaced, else all matching key/values (regardless
 +  how many) are removed, before the new pair is written.
 +
 +It returns 0 on success.
 +
 +Also, there are functions `git_config_rename_section` and
 +`git_config_rename_section_in_file` with parameters `old_name` and `new_name`
 +for renaming or removing sections in the config files. If NULL is passed
 +through `new_name` parameter, the section will be removed from the config file.
diff --combined cache.h
index 4d5b76c76ab2275f51c504cac62cda7e563e1d1d,2693a3736a7dbb058938ae4e2732e7cc0cf2336c..dfa1a5696d448b407644276df58fb24e25c57113
+++ b/cache.h
@@@ -8,6 -8,7 +8,7 @@@
  #include "gettext.h"
  #include "convert.h"
  #include "trace.h"
+ #include "string-list.h"
  
  #include SHA1_HEADER
  #ifndef git_SHA_CTX
@@@ -585,7 -586,6 +586,7 @@@ extern NORETURN void unable_to_lock_ind
  extern int hold_lock_file_for_update(struct lock_file *, const char *path, int);
  extern int hold_lock_file_for_append(struct lock_file *, const char *path, int);
  extern int commit_lock_file(struct lock_file *);
 +extern int reopen_lock_file(struct lock_file *);
  extern void update_index_if_able(struct index_state *, struct lock_file *);
  
  extern int hold_locked_index(struct lock_file *, int);
@@@ -1062,7 -1062,6 +1063,7 @@@ extern const char *git_author_info(int)
  extern const char *git_committer_info(int);
  extern const char *fmt_ident(const char *name, const char *email, const char *date_str, int);
  extern const char *fmt_name(const char *name, const char *email);
 +extern const char *ident_default_name(void);
  extern const char *ident_default_email(void);
  extern const char *git_editor(void);
  extern const char *git_pager(int stdout_is_tty);
@@@ -1296,7 -1295,7 +1297,7 @@@ extern int git_config_from_buf(config_f
                               const char *buf, size_t len, void *data);
  extern void git_config_push_parameter(const char *text);
  extern int git_config_from_parameters(config_fn_t fn, void *data);
- extern int git_config(config_fn_t fn, void *);
+ extern void git_config(config_fn_t fn, void *);
  extern int git_config_with_options(config_fn_t fn, void *,
                                   struct git_config_source *config_source,
                                   int respect_includes);
@@@ -1353,9 -1352,32 +1354,32 @@@ extern int parse_config_key(const char 
                            const char **subsection, int *subsection_len,
                            const char **key);
  
+ struct config_set_element {
+       struct hashmap_entry ent;
+       char *key;
+       struct string_list value_list;
+ };
+ struct configset_list_item {
+       struct config_set_element *e;
+       int value_index;
+ };
+ /*
+  * the contents of the list are ordered according to their
+  * position in the config files and order of parsing the files.
+  * (i.e. key-value pair at the last position of .git/config will
+  * be at the last item of the list)
+  */
+ struct configset_list {
+       struct configset_list_item *items;
+       unsigned int nr, alloc;
+ };
  struct config_set {
        struct hashmap config_hash;
        int hash_initialized;
+       struct configset_list list;
  };
  
  extern void git_configset_init(struct config_set *cs);
@@@ -1385,6 -1407,14 +1409,14 @@@ extern int git_config_get_bool_or_int(c
  extern int git_config_get_maybe_bool(const char *key, int *dest);
  extern int git_config_get_pathname(const char *key, const char **dest);
  
+ struct key_value_info {
+       const char *filename;
+       int linenr;
+ };
+ extern NORETURN void git_die_config(const char *key, const char *err, ...) __attribute__((format(printf, 2, 3)));
+ extern NORETURN void git_die_config_linenr(const char *key, const char *filename, int linenr);
  extern int committer_ident_sufficiently_given(void);
  extern int author_ident_sufficiently_given(void);
  
diff --combined config.c
index 01e0bdbafd4d43a7fb983449c78d711d22dd2375,387e86bb6ce6307d3a1398b82344b7e2a8de817f..83c913ad0928aba882a86f361170cd256adbd445
+++ b/config.c
@@@ -35,12 -35,6 +35,6 @@@ struct config_source 
        long (*do_ftell)(struct config_source *c);
  };
  
- struct config_set_element {
-       struct hashmap_entry ent;
-       char *key;
-       struct string_list value_list;
- };
  static struct config_source *cf;
  
  static int zlib_compression_seen;
@@@ -177,27 -171,19 +171,27 @@@ void git_config_push_parameter(const ch
  int git_config_parse_parameter(const char *text,
                               config_fn_t fn, void *data)
  {
 +      const char *value;
        struct strbuf **pair;
 +
        pair = strbuf_split_str(text, '=', 2);
        if (!pair[0])
                return error("bogus config parameter: %s", text);
 -      if (pair[0]->len && pair[0]->buf[pair[0]->len - 1] == '=')
 +
 +      if (pair[0]->len && pair[0]->buf[pair[0]->len - 1] == '=') {
                strbuf_setlen(pair[0], pair[0]->len - 1);
 +              value = pair[1] ? pair[1]->buf : "";
 +      } else {
 +              value = NULL;
 +      }
 +
        strbuf_trim(pair[0]);
        if (!pair[0]->len) {
                strbuf_list_free(pair);
                return error("bogus config parameter: %s", text);
        }
        strbuf_tolower(pair[0]);
 -      if (fn(pair[0]->buf, pair[1] ? pair[1]->buf : NULL, data) < 0) {
 +      if (fn(pair[0]->buf, value, data) < 0) {
                strbuf_list_free(pair);
                return -1;
        }
@@@ -252,6 -238,7 +246,7 @@@ static int get_next_char(void
                cf->linenr++;
        if (c == EOF) {
                cf->eof = 1;
+               cf->linenr++;
                c = '\n';
        }
        return c;
@@@ -327,6 -314,7 +322,7 @@@ static int get_value(config_fn_t fn, vo
  {
        int c;
        char *value;
+       int ret;
  
        /* Get the full name */
        for (;;) {
                if (!value)
                        return -1;
        }
-       return fn(name->buf, value, data);
+       /*
+        * We already consumed the \n, but we need linenr to point to
+        * the line we just parsed during the call to fn to get
+        * accurate line number in error messages.
+        */
+       cf->linenr--;
+       ret = fn(name->buf, value, data);
+       cf->linenr++;
+       return ret;
  }
  
  static int get_extended_base_var(struct strbuf *name, int c)
@@@ -465,9 -461,9 +469,9 @@@ static int git_parse_source(config_fn_
                        break;
        }
        if (cf->die_on_error)
-               die("bad config file line %d in %s", cf->linenr, cf->name);
+               die(_("bad config file line %d in %s"), cf->linenr, cf->name);
        else
-               return error("bad config file line %d in %s", cf->linenr, cf->name);
+               return error(_("bad config file line %d in %s"), cf->linenr, cf->name);
  }
  
  static int parse_unit_factor(const char *end, uintmax_t *val)
@@@ -583,9 -579,9 +587,9 @@@ static void die_bad_number(const char *
                value = "";
  
        if (cf && cf->name)
-               die("bad numeric config value '%s' for '%s' in %s: %s",
+               die(_("bad numeric config value '%s' for '%s' in %s: %s"),
                    value, name, cf->name, reason);
-       die("bad numeric config value '%s' for '%s': %s", value, name, reason);
+       die(_("bad numeric config value '%s' for '%s': %s"), value, name, reason);
  }
  
  int git_config_int(const char *name, const char *value)
@@@ -670,7 -666,7 +674,7 @@@ int git_config_pathname(const char **de
                return config_error_nonbool(var);
        *dest = expand_user_path(value);
        if (!*dest)
-               die("Failed to expand user dir in: '%s'", value);
+               die(_("failed to expand user dir in: '%s'"), value);
        return 0;
  }
  
@@@ -748,7 -744,7 +752,7 @@@ static int git_default_core_config(cons
                if (level == -1)
                        level = Z_DEFAULT_COMPRESSION;
                else if (level < 0 || level > Z_BEST_COMPRESSION)
-                       die("bad zlib compression level %d", level);
+                       die(_("bad zlib compression level %d"), level);
                zlib_compression_level = level;
                zlib_compression_seen = 1;
                return 0;
                if (level == -1)
                        level = Z_DEFAULT_COMPRESSION;
                else if (level < 0 || level > Z_BEST_COMPRESSION)
-                       die("bad zlib compression level %d", level);
+                       die(_("bad zlib compression level %d"), level);
                core_compression_level = level;
                core_compression_seen = 1;
                if (!zlib_compression_seen)
                return git_config_string(&editor_program, var, value);
  
        if (!strcmp(var, "core.commentchar")) {
 -              const char *comment;
 -              int ret = git_config_string(&comment, var, value);
 -              if (ret)
 -                      return ret;
 -              else if (!strcasecmp(comment, "auto"))
 +              if (!value)
 +                      return config_error_nonbool(var);
 +              else if (!strcasecmp(value, "auto"))
                        auto_comment_line_char = 1;
 -              else if (comment[0] && !comment[1]) {
 -                      comment_line_char = comment[0];
 +              else if (value[0] && !value[1]) {
 +                      comment_line_char = value[0];
                        auto_comment_line_char = 0;
                } else
                        return error("core.commentChar should only be one character");
                else if (!strcmp(value, "link"))
                        object_creation_mode = OBJECT_CREATION_USES_HARDLINKS;
                else
-                       die("Invalid mode for object creation: %s", value);
+                       die(_("invalid mode for object creation: %s"), value);
                return 0;
        }
  
@@@ -1181,7 -1179,7 +1185,7 @@@ int git_config_early(config_fn_t fn, vo
  
        switch (git_config_from_parameters(fn, data)) {
        case -1: /* error */
-               die("unable to parse command-line config");
+               die(_("unable to parse command-line config"));
                break;
        case 0: /* found nothing */
                break;
@@@ -1228,9 -1226,48 +1232,48 @@@ int git_config_with_options(config_fn_
        return ret;
  }
  
- int git_config(config_fn_t fn, void *data)
+ static void git_config_raw(config_fn_t fn, void *data)
+ {
+       if (git_config_with_options(fn, data, NULL, 1) < 0)
+               /*
+                * git_config_with_options() normally returns only
+                * positive values, as most errors are fatal, and
+                * non-fatal potential errors are guarded by "if"
+                * statements that are entered only when no error is
+                * possible.
+                *
+                * If we ever encounter a non-fatal error, it means
+                * something went really wrong and we should stop
+                * immediately.
+                */
+               die(_("unknown error occured while reading the configuration files"));
+ }
+ static void configset_iter(struct config_set *cs, config_fn_t fn, void *data)
  {
-       return git_config_with_options(fn, data, NULL, 1);
+       int i, value_index;
+       struct string_list *values;
+       struct config_set_element *entry;
+       struct configset_list *list = &cs->list;
+       struct key_value_info *kv_info;
+       for (i = 0; i < list->nr; i++) {
+               entry = list->items[i].e;
+               value_index = list->items[i].value_index;
+               values = &entry->value_list;
+               if (fn(entry->key, values->items[value_index].string, data) < 0) {
+                       kv_info = values->items[value_index].util;
+                       git_die_config_linenr(entry->key, kv_info->filename, kv_info->linenr);
+               }
+       }
+ }
+ static void git_config_check_init(void);
+ void git_config(config_fn_t fn, void *data)
+ {
+       git_config_check_init();
+       configset_iter(&the_config_set, fn, data);
  }
  
  static struct config_set_element *configset_find_element(struct config_set *cs, const char *key)
  static int configset_add_value(struct config_set *cs, const char *key, const char *value)
  {
        struct config_set_element *e;
+       struct string_list_item *si;
+       struct configset_list_item *l_item;
+       struct key_value_info *kv_info = xmalloc(sizeof(*kv_info));
        e = configset_find_element(cs, key);
        /*
         * Since the keys are being fed by git_config*() callback mechanism, they
                string_list_init(&e->value_list, 1);
                hashmap_add(&cs->config_hash, e);
        }
-       string_list_append_nodup(&e->value_list, value ? xstrdup(value) : NULL);
+       si = string_list_append_nodup(&e->value_list, value ? xstrdup(value) : NULL);
+       ALLOC_GROW(cs->list.items, cs->list.nr + 1, cs->list.alloc);
+       l_item = &cs->list.items[cs->list.nr++];
+       l_item->e = e;
+       l_item->value_index = e->value_list.nr - 1;
+       if (cf) {
+               kv_info->filename = strintern(cf->name);
+               kv_info->linenr = cf->linenr;
+       } else {
+               /* for values read from `git_config_from_parameters()` */
+               kv_info->filename = NULL;
+               kv_info->linenr = -1;
+       }
+       si->util = kv_info;
  
        return 0;
  }
@@@ -1285,6 -1341,9 +1347,9 @@@ void git_configset_init(struct config_s
  {
        hashmap_init(&cs->config_hash, (hashmap_cmp_fn)config_set_element_cmp, 0);
        cs->hash_initialized = 1;
+       cs->list.nr = 0;
+       cs->list.alloc = 0;
+       cs->list.items = NULL;
  }
  
  void git_configset_clear(struct config_set *cs)
        hashmap_iter_init(&cs->config_hash, &iter);
        while ((entry = hashmap_iter_next(&iter))) {
                free(entry->key);
-               string_list_clear(&entry->value_list, 0);
+               string_list_clear(&entry->value_list, 1);
        }
        hashmap_free(&cs->config_hash, 1);
        cs->hash_initialized = 0;
+       free(cs->list.items);
+       cs->list.nr = 0;
+       cs->list.alloc = 0;
+       cs->list.items = NULL;
  }
  
  static int config_set_callback(const char *key, const char *value, void *cb)
@@@ -1419,7 -1482,7 +1488,7 @@@ static void git_config_check_init(void
        if (the_config_set.hash_initialized)
                return;
        git_configset_init(&the_config_set);
-       git_config(config_set_callback, &the_config_set);
+       git_config_raw(config_set_callback, &the_config_set);
  }
  
  void git_config_clear(void)
@@@ -1443,8 -1506,12 +1512,12 @@@ const struct string_list *git_config_ge
  
  int git_config_get_string_const(const char *key, const char **dest)
  {
+       int ret;
        git_config_check_init();
-       return git_configset_get_string_const(&the_config_set, key, dest);
+       ret = git_configset_get_string_const(&the_config_set, key, dest);
+       if (ret < 0)
+               git_die_config(key, NULL);
+       return ret;
  }
  
  int git_config_get_string(const char *key, char **dest)
@@@ -1485,8 -1552,39 +1558,39 @@@ int git_config_get_maybe_bool(const cha
  
  int git_config_get_pathname(const char *key, const char **dest)
  {
+       int ret;
        git_config_check_init();
-       return git_configset_get_pathname(&the_config_set, key, dest);
+       ret = git_configset_get_pathname(&the_config_set, key, dest);
+       if (ret < 0)
+               git_die_config(key, NULL);
+       return ret;
+ }
+ NORETURN
+ void git_die_config_linenr(const char *key, const char *filename, int linenr)
+ {
+       if (!filename)
+               die(_("unable to parse '%s' from command-line config"), key);
+       else
+               die(_("bad config variable '%s' in file '%s' at line %d"),
+                   key, filename, linenr);
+ }
+ NORETURN __attribute__((format(printf, 2, 3)))
+ void git_die_config(const char *key, const char *err, ...)
+ {
+       const struct string_list *values;
+       struct key_value_info *kv_info;
+       if (err) {
+               va_list params;
+               va_start(params, err);
+               vreportf("error: ", err, params);
+               va_end(params);
+       }
+       values = git_config_get_value_multi(key);
+       kv_info = values->items[values->nr - 1].util;
+       git_die_config_linenr(key, kv_info->filename, kv_info->linenr);
  }
  
  /*
@@@ -1522,7 -1620,7 +1626,7 @@@ static int store_aux(const char *key, c
        case KEY_SEEN:
                if (matches(key, value)) {
                        if (store.seen == 1 && store.multi_replace == 0) {
-                               warning("%s has multiple values", key);
+                               warning(_("%s has multiple values"), key);
                        }
  
                        ALLOC_GROW(store.offset, store.seen + 1,