Re: [PATCH 1/4] config: add include directive

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 26, 2012, 20:58 UTC
Message-ID: <7vipjy2nbi.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20120126073752.GA30474@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 8 quoted lines

> Include directives are turned on for regular git config
> parsing (i.e., when you call git_config()), as well as for
> lookups via the "git config" program. They are not turned on
> in other cases, including:
>
>   1. Parsing of other config-like files, like .gitmodules.
>      There isn't a real need, and I'd rather be conservative
>      and avoid unnecessary incompatibility or confusion.

This is a good design decision, I think, but I am not sure where this "we do not let gitmodules to honor inclusion" is implemented in this patch. I would have expected a patch to "git-submodule.sh" that adds --no-includes where it invokes "git config"?

Show 7 quoted lines

> +Includes
> +~~~~~~~~
> +
> +You can include one config file from another by setting the special
> +`include.path` variable to the name of the file to be included. The
> +included file is expanded immediately, as if its contents had been
> +found at the location of the include directive.

I cannot offer better wording, but the first reaction I had to this was "Eh, if I include a file A that includes another file B, to which config file the '[include] path = B' directive found in file A relative to?"

Logically it should be relative to A, and I think your implementation makes it so, but the above "as if its contents had been found at..." sounds as if it should be the same as writing '[include] path = B' in the original config file that included A.

By the way, I was a bit surprised that you did not have to add the source stacking to git_config_from_file().

It was added in 924aaf3 (config.c: Make git_config() work correctly when called recursively, 2011-06-16) to address the situation where git_config() ends up calling git_config() recursively. My gut feeling is that logically that issue shouldn't be limited to configuration that is coming from file, which in turn makes me suspect that the source stacking may be better in one level higher than git_config_from_file() so that the ones coming from parameters could also be treated as a different kind of source to read configuration from.

But in practice, including from the command line (i.e. -c include.path=...) is somewhat crazy and we probably would not want to support it, so...