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

Re: [PATCH v2 2/3] config: handle conditional include when $GIT_DIR is not set up

From
Jeff King <peff@peff.net>
Date
Apr 18, 2017, 03:46 UTC
Message-ID
<20170418034610.oiex6x3d4pcyzbzy@sigill.intra.peff.net>
In-Reply-To
<xmqqzifefmog.fsf@gitster.mtv.corp.google.com>
On Mon, Apr 17, 2017 at 07:56:47PM -0700, Junio C Hamano wrote:
Show 19 quoted lines
> Junio C Hamano <gitster@pobox.com> writes:
> 
> >> +	if (!have_git_dir() && opts.git_dir) {
> >>  		struct git_config_source repo_config;
> >>  
> >>  		memset(&repo_config, 0, sizeof(repo_config));
> >
> > But this one I do not quite understand.  When have_git_dir() was
> > false and asked discover_git_directory() to set opts.git_dir, we
> > enter the body of this block and then end up doing
> >
> > 	git_config_with_options(cb, data &repo_config, &opts);
> >
> > with repo_config set to the discovered git directory plus "/config";
> > we'd read the repository configuration twice, in other words.
> 
> Ahh, nevermind.  The fix to make the "usual sequence" pay attention
> to the opts->git_dir comes in 3/3 and in that step this redundant
> reading is also removed, so all is well at the end.

I think 2/3 is actually OK as-is. We know we didn't read repo config in the first call to git_config_with_options() because have_git_dir() is unset.

So 2/3 doesn't make anything worse; the bug already existed before Duy's series (it goes back to my original read_early_config() hack).

-Peff
Previous: Junio C HamanoNext: Nguyễn Thái Ngọc Duy
Message 15 of 24 in “includeIf breaks calling dashed externals”
  1. Bert WesargApr 14, 2017
  2. Jeff KingApr 14, 2017
  3. Duy NguyenApr 15, 2017
  4. Jeff KingApr 16, 2017
  5. 1/2 config: prepare to pass more info in git_config_with_options()Nguyễn Thái Ngọc Duy, Apr 16, 2017
  6. 2/2 config: handle conditional include when $GIT_DIR is not set upNguyễn Thái Ngọc Duy, Apr 16, 2017
  7. Jeff KingApr 16, 2017
  8. Duy NguyenApr 17, 2017
  9. Jeff KingApr 18, 2017
  10. Duy NguyenApr 17, 2017
  11. 1/3 config: prepare to pass more info in git_config_with_options()Nguyễn Thái Ngọc Duy, Apr 17, 2017
  12. 2/3 config: handle conditional include when $GIT_DIR is not set upNguyễn Thái Ngọc Duy, Apr 17, 2017
  13. Junio C HamanoApr 18, 2017
  14. Junio C HamanoApr 18, 2017
  15. Jeff KingApr 18, 2017
  16. 3/3 config: correct file reading order in read_early_config()Nguyễn Thái Ngọc Duy, Apr 17, 2017
  17. Jeff KingApr 18, 2017
  18. Junio C HamanoApr 18, 2017
  19. Jeff KingApr 18, 2017
  20. Junio C HamanoApr 18, 2017
  21. Junio C HamanoApr 18, 2017
  22. Jeff KingApr 18, 2017
  23. Jeff KingApr 16, 2017
  24. Duy NguyenApr 17, 2017

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.