Re: [PATCH 4/4] config: don't implicitly use gitdir

[Brandon Williams <bmwill@google.com>]

On 06/13, Jeff King wrote:
> On Mon, Jun 12, 2017 at 11:16:27PM -0700, Brandon Williams wrote:
> 
> > > > *puzzled* Why wasn't this needed before, then?  The rest of the patch
> > > > should result in no functional change, but this part seems different.
> > > 
> > > Now I'm puzzled, too. The original that got filled in lazily by the
> > > config functions was always get_git_dir(). I can buy the argument that
> > > this was a bug (I'm not familiar enough with worktree to say one way or
> > > the other), but if it's a fix it should definitely go into another
> > > patch.
> > 
> > Well actually... in do_git_config_sequence 'git_path("config")' is
> > called which will convert gitdir to commondir under the hood.  you can't
> > use vanilla gitdir because the config isn't stored in a worktree's
> > gitdir but rather in the commondir as the config is shared by all
> > worktrees.
> 
> Sorry, I missed the fact that there were two sites changed on the first
> read.

Well I missed that fact when I first wrote these patches too :)

> 
> > So maybe we actually need to add a field to the 'config_options' struct
> > of 'commondir' such that the commondir can be used to load the actual
> > config file and 'gitdir' can be used to handle the 'IncludeIf' stuff.
> 
> On reflection, I suspect that probably is the case. If you have a
> workdir in ~/foo, you probably want to match IncludeIf against that
> instead of wherever the common dir happens to be.

K I'll look into adding that then.  I will say keeping track of
'commondir' vs 'gitdir' does get slightly confusing.

-- 
Brandon Williams