Re: [PATCH 0/2] config includes, take 2

From: Jeff King <peff@peff.net>
Date: Feb 6, 2012, 10:06 UTC
Message-ID: <20120206100622.GC4300@sigill.intra.peff.net>
In-Reply-To: <4F2FA330.7020803@alum.mit.edu>

On Mon, Feb 06, 2012 at 10:53:52AM +0100, Michael Haggerty wrote:

> ISTM that the main goal was to prevent infinite recursion, not avoid a
> little bit of redundant reading.

It was both, actually. There was a sense that we should not end up with duplicate entries by reading the same file twice. However, entries which actually create lists (and could therefore create duplicates) are by far the minority compared to entries which overwrite. And it is the overwrite-style entries which are harmed by suppressing duplicates.

Show 5 quoted lines

> If that is the goal, all you have to record is the "include stack"
> context that caused the currently-being-included file to be read and
> make sure that the currently-being-included file didn't appear earlier
> on the stack.  The fact that the same file was included earlier (but
> not as part of the same include chain) needn't be considered an error.

I considered this, but decided the complexity wasn't worth it, especially because getting it accurate means cooperation from git_config_from_file, which otherwise doesn't know or care about this mechanism. Instead I keep a simple depth counter and barf at a reasonable maximum, printing the "from" and "to" files. Yes, it's not nearly as elegant as keeping the whole stack, but I really don't expect people to have complex super-deep includes, nor for it to be difficult to hunt down the cause of a cycle.

Maybe that is short-sighted or lazy of me, but I'd just really be surprised if people ever went more than 1 or 2 layers deep, or if they actually ended up with a cycle at all.

There is a stack kept already by git_config_from_file, but it doesn't respect the call-chain (so if I start a new git_config() call from inside a callback, it is still part of the same stack).

We could possibly put a marker in the stack for that situation, and then it would be usable for include cycle-detection.

Show 8 quoted lines

> However, one could even imagine a command like
> 
>     $ git config --where-defined user.email
>     user.email defined by /home/me/myfile2 line 75
>         which was included from /home/me/myfile1 line 13
>         which was included from /home/me/.gitconfig line 22
>     user.email redefined by /home/me/project/.git/companyconfig line 8
>         which was included from /home/me/project/.git/config line 20

You can already implement this with the existing stack by providing a suitable callback (since its your callback, you'd know that there was no recursion of git_config, and therefore the stack refers only to a single set of includes).

-Peff