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