Re: The config include mechanism doesn't allow for overwriting
On Mon, Oct 22, 2012 at 05:55:00PM +0200, Ævar Arnfjörð Bjarmason wrote:
Show 12 quoted lines
> I was hoping to write something like this:
>
> [user]
> name = Luser
> email = some-default@example.com
> [include]
> path = ~/.gitconfig.d/user-email
>
> Where that file would contain:
>
> [user]
> email = local-email@example.com
The intent is that it would work as you expect, and produce local-email@example.com.
Show 5 quoted lines
> But when you do that git prints:
>
> $ git config --get user.email
> some-default@example.com
> error: More than one value for the key user.email: local-email@example.com
Ugh. The config code just feeds all the values sequentially to the callback. The normal callbacks within git will overwrite old values, whether from earlier in the file, from a file with lower priority (e.g., /etc/gitconfig versus ~/.gitconfig), or from an earlier included. Which you can check with:
$ git var GIT_AUTHOR_IDENT
Luser <local-email@example.com> 1350936694 -0400
But git-config takes it upon itself to detect duplicates in its callback. Which is just silly, since it is not something that regular git would do. git-config should behave as much like the internal git parser as possible.
> I think config inclusion is much less useful when you can't clobber
> previously assigned values.
Agreed. But I think the bug is in git-config, not in the include mechanism. I think I'd like to do something like the patch below, which just reuses the regular config code for git-config, collects the values, and then reports them. It does mean we use a little more memory (for the sake of simplicity, we store values instead of streaming them out), but the code is much shorter, less confusing, and automatically matches what regular git_config() does.
It fails a few tests in t1300, but it looks like those tests are testing for the behavior we have identified as wrong, and should be fixed.
---
builtin/config.c | 111 ++++++++++++-----------------------
1 file changed, 38 insertions(+), 73 deletions(-)
diff --git a/builtin/config.c b/builtin/config.c
index d6a066b..72cb0a8 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -16,7 +16,6 @@ static int do_not_match;
static int use_key_regexp;
static int do_all;
static int do_not_match;
-static int seen;
static char delim = '=';
static char key_delim = ' ';
static char term = '\n';
@@ -110,12 +109,19 @@ static int show_config(const char *key_, const char *value_, void *cb)
return 0;
}
-static int show_config(const char *key_, const char *value_, void *cb)
+struct strbuf_list {
+ struct strbuf *items;
+ int nr;
+ int alloc;
+};
+
+static int collect_config(const char *key_, const char *value_, void *cb)
{
+ struct strbuf_list *values = cb;
+ struct strbuf *buf;
char value[256];
const char *vptr = value;
int must_free_vptr = 0;
- int dup_error = 0;
int must_print_delim = 0;
if (!use_key_regexp && strcmp(key_, key))
@@ -126,12 +132,15 @@ static int show_config(const char *key_, const char *value_, void *cb)
(do_not_match ^ !!regexec(regexp, (value_?value_:""), 0, NULL, 0)))
return 0;
+ ALLOC_GROW(values->items, values->nr + 1, values->alloc);
+ buf = &values->items[values->nr++];
+ strbuf_init(buf, 0);
+
if (show_keys) {
- printf("%s", key_);
+ strbuf_addstr(buf, key_);
must_print_delim = 1;
}
- if (seen && !do_all)
- dup_error = 1;
+
if (types == TYPE_INT)
sprintf(value, "%d", git_config_int(key_, value_?value_:""));
else if (types == TYPE_BOOL)
@@ -153,16 +162,12 @@ static int show_config(const char *key_, const char *value_, void *cb)
vptr = "";
must_print_delim = 0;
}
- seen++;
- if (dup_error) {
- error("More than one value for the key %s: %s",
- key_, vptr);
- }
- else {
- if (must_print_delim)
- printf("%c", key_delim);
- printf("%s%c", vptr, term);
- }
+
+ if (must_print_delim)
+ strbuf_addch(buf, key_delim);
+ strbuf_addstr(buf, vptr);
+ strbuf_addch(buf, term);
+
if (must_free_vptr)
/* If vptr must be freed, it's a pointer to a
* dynamically allocated buffer, it's safe to cast to
@@ -175,20 +180,8 @@ static int get_value(const char *key_, const char *regex_)
static int get_value(const char *key_, const char *regex_)
{
- int ret = CONFIG_GENERIC_ERROR;
- char *global = NULL, *xdg = NULL, *repo_config = NULL;
- const char *system_wide = NULL, *local;
- struct config_include_data inc = CONFIG_INCLUDE_INIT;
- config_fn_t fn;
- void *data;
-
- local = given_config_file;
- if (!local) {
- local = repo_config = git_pathdup("config");
- if (git_config_system())
- system_wide = git_etc_gitconfig();
- home_config_paths(&global, &xdg, "config");
- }
+ struct strbuf_list values = {0};
+ int i;
if (use_key_regexp) {
char *tl;
@@ -211,14 +204,11 @@ static int get_value(const char *key_, const char *regex_)
if (regcomp(key_regexp, key, REG_EXTENDED)) {
fprintf(stderr, "Invalid key pattern: %s\n", key_);
free(key);
- ret = CONFIG_INVALID_PATTERN;
- goto free_strings;
+ return CONFIG_INVALID_PATTERN;
}
} else {
- if (git_config_parse_key(key_, &key, NULL)) {
- ret = CONFIG_INVALID_KEY;
- goto free_strings;
- }
+ if (git_config_parse_key(key_, &key, NULL))
+ return CONFIG_INVALID_KEY;
}
if (regex_) {
@@ -230,37 +220,12 @@ static int get_value(const char *key_, const char *regex_)
regexp = (regex_t*)xmalloc(sizeof(regex_t));
if (regcomp(regexp, regex_, REG_EXTENDED)) {
fprintf(stderr, "Invalid pattern: %s\n", regex_);
- ret = CONFIG_INVALID_PATTERN;
- goto free_strings;
+ return CONFIG_INVALID_PATTERN;
}
}
- fn = show_config;
- data = NULL;
- if (respect_includes) {
- inc.fn = fn;
- inc.data = data;
- fn = git_config_include;
- data = &inc;
- }
-
- if (do_all && system_wide)
- git_config_from_file(fn, system_wide, data);
- if (do_all && xdg)
- git_config_from_file(fn, xdg, data);
- if (do_all && global)
- git_config_from_file(fn, global, data);
- if (do_all)
- git_config_from_file(fn, local, data);
- git_config_from_parameters(fn, data);
- if (!do_all && !seen)
- git_config_from_file(fn, local, data);
- if (!do_all && !seen && global)
- git_config_from_file(fn, global, data);
- if (!do_all && !seen && xdg)
- git_config_from_file(fn, xdg, data);
- if (!do_all && !seen && system_wide)
- git_config_from_file(fn, system_wide, data);
+ git_config_with_options(collect_config, &values,
+ given_config_file, respect_includes);
free(key);
if (regexp) {
@@ -268,16 +233,16 @@ static int get_value(const char *key_, const char *regex_)
free(regexp);
}
- if (do_all)
- ret = !seen;
- else
- ret = (seen == 1) ? 0 : seen > 1 ? 2 : 1;
+ if (!values.nr)
+ return 1;
-free_strings:
- free(repo_config);
- free(global);
- free(xdg);
- return ret;
+ for (i = 0; i < values.nr; i++) {
+ struct strbuf *buf = values.items + i;
+ if (do_all || i == values.nr - 1)
+ fwrite(buf->buf, 1, buf->len, stdout);
+ strbuf_release(buf);
+ }
+ return 0;
}
static char *normalize_value(const char *key, const char *value)
Ævar Arnfjörð Bjarmason
