Re: [PATCH v2 1/1] config: work around bug with includeif:onbranch and early config
To
Jeff Hostetler
Cc
Johannes Schindelin
Johannes Schindelin via GitGitGadget
git@vger.kernel.org
Junio C Hamano
From
Jeff King
See Also
Prev Ref 1 Ref 2 Ref 3 Ref 4 Ref 5 Ref 6 Ref 7
Date
2019-08-06 12:56:59 UTC
On Thu, Aug 01, 2019 at 01:24:17PM -0400, Jeff Hostetler wrote:

> > By the way, I wondered why trace2's existing config reading did not
> > cause us to segfault because of this. It is because it invented the
> > "very early config" function which always ignores some config sources
> > (working around this problem, but also making it weirdly unlike most
> > other config).
> 
> Yes, I added the "very early config" to try to work around some of
> the chicken-n-egg problems.  I can't say that I was completely happy
> with having to do that.

I meant to comment a little further on this earlier. While I do think
it's unfortunate to have yet another set of special rules, I think what
you ended up with is probably the least-bad thing.  This trace2 config
read is happening so early (even after my movement patch) that it makes
me nervous to do anything at all complicated.

Not just for performance reasons, which you cited in the original
commit, but also for correctness and even security reasons.  This is
code we're running for every single binary before we even hit main(),
and it's probably a good thing that we are not loading .git/config
values from a potentially untrusted repository (e.g., upload-pack is
supposed to be safe to run in an untrusted repo).

-Peff