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

[Jeff King <peff@peff.net>]

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.

> 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.

> 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