Re: [RFC/PATCH 0/4] config include directives

On Fri, Jan 27, 2012 at 10:51:34AM +0100, Ævar Arnfjörð Bjarmason wrote:

If you write the function like that it means your patch series just
works since values encountered later will override earlier ones, but
have you checked git's code to make sure we don't have anything like:

    static int ignore_add_errors_is_set = 0;
    static int add_config(const char *var, const char *value, void *cb)
    {
    	if (!ignore_add_errors_is_set &&
            (!strcmp(var, "add.ignoreerrors") ||
    	     !strcmp(var, "add.ignore-errors"))) {
    		ignore_add_errors = git_config_bool(var, value);
            ignore_add_errors_is_set = 1;
    		return 0;
    	}
    	return git_default_config(var, value, cb);
    }

Which would mean that the include config support would be silently
ignored.

I'm not sure what the issue is. If you write code like this, it will
already ignore the second invocation when it is found later in the same
file, or when it is found in a later file (i.e., in both .git/config and
.gitconfig). So I don't think includes introduce a new problem with
respect to code like this (and no, I didn't check exhaustively, but I
don't recall seeing code like this in git).

A bigger potential problem is multi-key values that form lists. For
example, I cannot use a later "remote.foo.url" line to override an
earlier one; instead, it gets appended to the list of URLs for "foo".
In practice, it's not a problem because the list-like options don't tend
to be found in multiple places. And again, this is not a new problem of
includes, since we already handle multiple files.

Accidentally including the same file twice would cause duplicates for
multi-key values. But I'm going to take Junio's suggestion to avoid
including the same file twice (which also prevents infinite loops due to
cycles).

-Peff