git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2 1/1] config: work around bug with includeif:onbranch and early config

From
Jeff King <peff@peff.net>
Date
Aug 6, 2019, 12:56 UTC
Message-ID
<20190806125659.GA18442@sigill.intra.peff.net>
In-Reply-To
<7f4257d3-5ae0-3a83-84f1-bee83682fa36@jeffhostetler.com>
On Thu, Aug 01, 2019 at 01:24:17PM -0400, Jeff Hostetler wrote:
Show 9 quoted lines
> > 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
Previous: Johannes Schindelin
Message 17 of 17 in “Make the includeif:onbranch feature more robust”
  1. 0/1 Make the includeif:onbranch feature more robustJohannes Schindelin via GitGitGadget, Jul 31, 2019
  2. 1/1 config: work around bug with includeif:onbranch and early configJohannes Schindelin via GitGitGadget, Jul 31, 2019
  3. Junio C HamanoJul 31, 2019
  4. 0/1 Make the includeif:onbranch feature more robustJohannes Schindelin via GitGitGadget, Jul 31, 2019
  5. 1/1 config: work around bug with includeif:onbranch and early configJohannes Schindelin via GitGitGadget, Jul 31, 2019
  6. Jeff KingJul 31, 2019
  7. Johannes SchindelinJul 31, 2019
  8. Jeff KingJul 31, 2019
  9. Jeff KingAug 1, 2019
  10. Jeff HostetlerAug 1, 2019
  11. 0/3 the_repository initialization cleanupJeff King, Aug 6, 2019
  12. 1/3 t1309: use short branch name in includeIf.onbranch testJeff King, Aug 6, 2019
  13. 2/3 common-main: delay trace2 initializationJeff King, Aug 6, 2019
  14. 3/3 config: stop checking whether the_repository is NULLJeff King, Aug 6, 2019
  15. Jeff KingAug 6, 2019
  16. Johannes SchindelinAug 8, 2019
  17. Jeff KingAug 6, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.