config: silence warnings for command names with invalid keys

When we are running the git command "foo", we may have to
look up the config keys "pager.foo" and "alias.foo". These
config schemes are mis-designed, as the command names can be
anything, but the config syntax has some restrictions. For
example:

$ git foo_bar
error: invalid key: pager.foo_bar
error: invalid key: alias.foo_bar
git: 'foo_bar' is not a git command. See 'git --help'.

You cannot name an alias with an underscore. And if you have
an external command with one, you cannot configure its
pager.

In the long run, we may develop a different config scheme
for these features. But in the near term (and because we'll
need to support the existing scheme indefinitely), we should
at least squelch the error messages shown above.

These errors come from git_config_parse_key. Ideally we
would pass a "quiet" flag to the config machinery, but there
are many layers between the pager code and the key parsing.
Passing a flag through all of those would be an invasive
change.

Instead, let's provide a config function to report on
whether a key is syntactically valid, and have the pager and
alias code skip lookup for bogus keys. We can build this
easily around the existing git_config_parse_key, with two
minor modifications:

1. We now handle a NULL store_key, to validate but not
write out the normalized key.

2. We accept a "quiet" flag to avoid writing to stderr.
This doesn't need to be a full-blown public "flags"
field, because we can make the existing implementation
a static helper function, keeping the mess contained
inside config.c.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/alias.c b/alias.c
index 6aa164a362427ffa5dc8616bcb01aec3f15b462b..a11229db9e67b7b651356be3fb1d4b3d1014a8bd 100644 (file)
--- a/alias.c
+++ b/alias.c
@@ -5,7 +5,8 @@ char *alias_lookup(const char *alias)
        char *v = NULL;
        struct strbuf key = STRBUF_INIT;
        strbuf_addf(&key, "alias.%s", alias);
        char *v = NULL;
        struct strbuf key = STRBUF_INIT;
        strbuf_addf(&key, "alias.%s", alias);

-       git_config_get_string(key.buf, &v);
+       if (git_config_key_is_valid(key.buf))
+               git_config_get_string(key.buf, &v);

        strbuf_release(&key);
        return v;
 }
        strbuf_release(&key);
        return v;
 }

diff --git a/cache.h b/cache.h
index 22b7b8129072f3905c68f02fe19f463bdb738b9e..1121bbd2303acadedb005f95f237e5cf2912278b 100644 (file)
--- a/cache.h
+++ b/cache.h
@@ -1366,6 +1366,7 @@ extern int git_config_pathname(const char **, const char *, const char *);
 extern int git_config_set_in_file(const char *, const char *, const char *);
 extern int git_config_set(const char *, const char *);
 extern int git_config_parse_key(const char *, char **, int *);
 extern int git_config_set_in_file(const char *, const char *, const char *);
 extern int git_config_set(const char *, const char *);
 extern int git_config_parse_key(const char *, char **, int *);

+extern int git_config_key_is_valid(const char *key);

 extern int git_config_set_multivar(const char *, const char *, const char *, int);
 extern int git_config_set_multivar_in_file(const char *, const char *, const char *, const char *, int);
 extern int git_config_rename_section(const char *, const char *);
 extern int git_config_set_multivar(const char *, const char *, const char *, int);
 extern int git_config_set_multivar_in_file(const char *, const char *, const char *, const char *, int);
 extern int git_config_rename_section(const char *, const char *);

diff --git a/config.c b/config.c
index 752e2e227f56edfb7b2ad168798c110c14a2c5e8..12692d9b6f4d2d37dea8c8aef6690d75ca19a49d 100644 (file)
--- a/config.c
+++ b/config.c
@@ -1843,7 +1843,7 @@ int git_config_set(const char *key, const char *value)
  * baselen - pointer to int which will hold the length of the
  *           section + subsection part, can be NULL
  */
  * baselen - pointer to int which will hold the length of the
  *           section + subsection part, can be NULL
  */

-int git_config_parse_key(const char *key, char **store_key, int *baselen_)
+static int git_config_parse_key_1(const char *key, char **store_key, int *baselen_, int quiet)

 {
        int i, dot, baselen;
        const char *last_dot = strrchr(key, '.');
 {
        int i, dot, baselen;
        const char *last_dot = strrchr(key, '.');


@@ -1854,12 +1854,14 @@ int git_config_parse_key(const char *key, char **store_key, int *baselen_)
         */
 
        if (last_dot == NULL || last_dot == key) {
         */
 
        if (last_dot == NULL || last_dot == key) {

-               error("key does not contain a section: %s", key);
+               if (!quiet)
+                       error("key does not contain a section: %s", key);

                return -CONFIG_NO_SECTION_OR_NAME;
        }
 
        if (!last_dot[1]) {
                return -CONFIG_NO_SECTION_OR_NAME;
        }
 
        if (!last_dot[1]) {

-               error("key does not contain variable name: %s", key);
+               if (!quiet)
+                       error("key does not contain variable name: %s", key);

                return -CONFIG_NO_SECTION_OR_NAME;
        }
 
                return -CONFIG_NO_SECTION_OR_NAME;
        }
 


@@ -1870,7 +1872,8 @@ int git_config_parse_key(const char *key, char **store_key, int *baselen_)
        /*
         * Validate the key and while at it, lower case it for matching.
         */
        /*
         * Validate the key and while at it, lower case it for matching.
         */

-       *store_key = xmalloc(strlen(key) + 1);
+       if (store_key)
+               *store_key = xmalloc(strlen(key) + 1);

 
        dot = 0;
        for (i = 0; key[i]; i++) {
 
        dot = 0;
        for (i = 0; key[i]; i++) {


@@ -1881,26 +1884,42 @@ int git_config_parse_key(const char *key, char **store_key, int *baselen_)
                if (!dot || i > baselen) {
                        if (!iskeychar(c) ||
                            (i == baselen + 1 && !isalpha(c))) {
                if (!dot || i > baselen) {
                        if (!iskeychar(c) ||
                            (i == baselen + 1 && !isalpha(c))) {

-                               error("invalid key: %s", key);
+                               if (!quiet)
+                                       error("invalid key: %s", key);

                                goto out_free_ret_1;
                        }
                        c = tolower(c);
                } else if (c == '\n') {
                                goto out_free_ret_1;
                        }
                        c = tolower(c);
                } else if (c == '\n') {

-                       error("invalid key (newline): %s", key);
+                       if (!quiet)
+                               error("invalid key (newline): %s", key);

                        goto out_free_ret_1;
                }
                        goto out_free_ret_1;
                }

-               (*store_key)[i] = c;
+               if (store_key)
+                       (*store_key)[i] = c;

        }
        }

-       (*store_key)[i] = 0;
+       if (store_key)
+               (*store_key)[i] = 0;

 
        return 0;
 
 out_free_ret_1:
 
        return 0;
 
 out_free_ret_1:

-       free(*store_key);
-       *store_key = NULL;
+       if (store_key) {
+               free(*store_key);
+               *store_key = NULL;
+       }

        return -CONFIG_INVALID_KEY;
 }
 
        return -CONFIG_INVALID_KEY;
 }
 

+int git_config_parse_key(const char *key, char **store_key, int *baselen)
+{
+       return git_config_parse_key_1(key, store_key, baselen, 0);
+}
+
+int git_config_key_is_valid(const char *key)
+{
+       return !git_config_parse_key_1(key, NULL, NULL, 1);
+}
+

 /*
  * If value==NULL, unset in (remove from) config,
  * if value_regex!=NULL, disregard key/value pairs where value does not match.
 /*
  * If value==NULL, unset in (remove from) config,
  * if value_regex!=NULL, disregard key/value pairs where value does not match.

diff --git a/pager.c b/pager.c
index f6e8c331924496ca6656cd05d7de6497310502ab..09c232a0b42b8cabe3172c5e5db5cd9b7e94407d 100644 (file)
--- a/pager.c
+++ b/pager.c
@@ -149,7 +149,8 @@ int check_pager_config(const char *cmd)
        struct strbuf key = STRBUF_INIT;
        const char *value = NULL;
        strbuf_addf(&key, "pager.%s", cmd);
        struct strbuf key = STRBUF_INIT;
        const char *value = NULL;
        strbuf_addf(&key, "pager.%s", cmd);

-       if (!git_config_get_value(key.buf, &value)) {
+       if (git_config_key_is_valid(key.buf) &&
+           !git_config_get_value(key.buf, &value)) {

                int b = git_config_maybe_bool(key.buf, value);
                if (b >= 0)
                        want = b;
                int b = git_config_maybe_bool(key.buf, value);
                if (b >= 0)
                        want = b;

diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
index da958a8b569e6e8ad0c4d2e20b0454a0180155cc..2dd71c055cee29d482448c3f38d276d908ea7f4e 100755 (executable)
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -447,4 +447,13 @@ test_expect_success TTY 'external command pagers override sub-commands' '
        test_cmp expect actual
 '
 
        test_cmp expect actual
 '
 

+test_expect_success 'command with underscores does not complain' '
+       write_script git-under_score <<-\EOF &&
+       echo ok
+       EOF
+       git --exec-path=. under_score >actual 2>&1 &&
+       echo ok >expect &&
+       test_cmp expect actual
+'
+

 test_done
 test_done