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

[Jeff King <peff@peff.net>]

On Mon, Apr 17, 2017 at 09:13:35AM +0700, Duy Nguyen wrote:

> > So we call get_git_dir() always when we're not in early config. Even if
> > we don't have a git dir! Doesn't this mean that programs operating
> > outside of a repo will still hit the BUG? I.e.:
> > 
> >   git config --global includeif.gitdir:/whatever.path foo
> >   cd /not/a/git/dir
> >   git diff --no-index foo bar
> > 
> > ?
> > 
> > I think instead the logic should be:
> > 
> >   if (opts->git_dir)
> > 	strbuf_add_absolute_path(&text, opts->git_dir);
> >   else if (have_git_dir())
> > 	strbuf_add_absolute_path(&text, get_git_dir());
> >   else
> > 	goto done;
> 
> Do we care about the case when the override instruction is "we don't
> have $GIT_DIR, act as if it does not exist, even though have_git_dir()
> returns true"?
> 
> I'm guessing no, we won't run into that situation (and am inclined to
> restructure the code as you suggested). Just throwing it out there if
> I'm mistaken.

I can't think of a case where that would matter. We're always prodding
the function to look at something it might not, and I don't think there
would be any need to convince it _not_ to look at something.

-Peff