On Tue, Jun 13, 2017 at 11:26:06AM -0700, Brandon Williams wrote: > So because I've been looking at the config machinery lately, I've > noticed a lot of issues with how things are handled with respect to > gitdir vs commondir. Essentially the config resides at commondir/config > always, and only at gitdir/config when not working with a worktree. > Because of this, your patches point out a bug in how early config is > handled. I'll illustrate this using aliases. > > Before this series (because aliases are read using the standard config > machinery): > > > git init main > > git -C main config alias.test '!echo hello' > > git -C main test > hello > > git -C main worktree add ../worktree > > git -C worktree test > hello > > After this series (using read_early_config()): > > > git init main > > git -C main config alias.test '!echo hello' > > git -C main test > hello > > git -C main worktree add ../worktree > > git -C worktree test > git: 'test' is not a git command. See 'git --help'. > > The issue is that read_early_config passes the gitdir and not the > commondir when reading the config. Good catch. > The solution would be to add a 'commondir' field to the config_options > struct and populate that before reading the config. I'm planning on > fixing this in v2 of my config cleanup series which I'll hopefully have > finished by the end of the day. I think that read_early_config() really meant to set the commondir, as it was always about actual config-file lookup. So it was sort-of buggy already, though I suspect it was pretty hard to trigger. But I agree that since include_by_gitdir now reads the same struct field, swapping it out fixes the config-reading at the cost of breaking that function. And we really need to pass both in. I'm not sure whether we should care about this for Dscho's series or not. I think his series _does_ make the problem easier to trigger. But it's a minor enough bug that I think I'd be OK with letting your solution proceed independently. -Peff