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
