On 06/12, Jonathan Nieder wrote:
> Hi again,
>
> Brandon Williams wrote:
> > On 06/12, Jonathan Nieder wrote:
>
> >> Alternatively, could this patch rename git_config_with_options? That
> >> way any other patch in flight that calls git_config_with_options would
> >> conflict with this patch, giving us an opportunity to make sure it
> >> also sets git_dir. As another nice side benefit it would make it easy
> >> for someone reading the patch to verify it didn't miss any callers.
> [...]
> > And I don't know if I agree with renaming a function just to rename it.
>
> I forgot to say: another way to accomplish the same thing can be to
> reorder the function's arguments. The relevant thing is to make code
> that calls the function without being aware of the new requirements
> fail to compile.
>
> [...]
> >> Brandon Williams wrote:
>
> >>> opts.respect_includes = 1;
> >>> + if (have_git_dir())
> >>> + opts.git_dir = get_git_common_dir();
> >>
> >> curious: Why get_git_common_dir() instead of get_git_dir()?
> >
> > Needs to be commondir since the config is stored in the common git
> > directory and not a per worktree git directory.
>
> *puzzled* Why wasn't this needed before, then? The rest of the patch
> should result in no functional change, but this part seems different.
there is no functional change, this is what always happened.
git_path("config") will replace gitdir with commondir under the hood.
>
> Please add some explanation to the commit message.
>
> Thanks,
> Jonathan
--
Brandon Williams