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

From: Jeff King <peff@peff.net>
Date: Jan 27, 2012, 17:34 UTC
Message-ID: <20120127173408.GA3053@sigill.intra.peff.net>
In-Reply-To: <CACBZZX59sur4_61LkN_sMOvXQ4Jdnt1P8O-UOgm0SooBQpjFdQ@mail.gmail.com>

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

Show 19 quoted lines

> 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