On 02/06/2012 07:27 AM, Jeff King wrote: > 3. perform cycle and duplicate detection on included files > [...] > > We first read the global config, which sets the value to "global", then > includes foo, which overwrites it to "foo". Then we read the repo > config, which sets the value to "repo", and then does _not_ actually > read foo. Because git config is read linearly and later values tend to > overwrite earlier ones, we would want to suppress the _first_ instance > of a file, not the second (or really, the final if it is included many > times). But that is impossible to do without generating a complete graph > of includes and then pruning the early ones. ISTM that the main goal was to prevent infinite recursion, not avoid a little bit of redundant reading. 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. > [...] > So I'm actually thinking I should drop the duplicate suppression and > just do some sort of sanity check on include-depth to break cycles > (i.e., just die because it's a crazy thing to do, and we are really just > trying to tell the user their config is broken rather than go into an > infinite loop). As a bonus, it makes the code much simpler, too. I agree that it is more sensible to error out than to ignore a recursive include. It might nevertheless be useful to have an "include call stack" available for generating output for errors that occur when parsing config files; something like: Error when parsing /home/me/bogusconfigfile line 75 which was included from /home/me/okconfigfile line 13 which was included from /home/me/.gitconfig line 22 or Error: /home/me/bogusconfigfile included recursively by /home/me/bogusfile2 line 75 which was included from /home/me/bogusconfigfile line 13 which was included from /home/me/.gitconfig line 22 OTOH such error stack dumps could be generated when unwinding the C call stack without the need for a separate "include call stack". 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 This usage could not be implemented using the C stack, because the C stack cannot be unwound multiple times. But these are all just wild ideas. I doubt that people's config files will become so complicated that this much infrastructure is needed. Michael -- Michael Haggerty mhagger@xxxxxxxxxxxx http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html