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

2017-06-13 Thread Jonathan Nieder
Jeff King wrote: > On Mon, Jun 12, 2017 at 06:38:17PM -0700, Jonathan Nieder wrote: >> 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

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

2017-06-13 Thread Brandon Williams
On 06/13, Jeff King wrote: > On Mon, Jun 12, 2017 at 10:52:43PM -0700, Brandon Williams wrote: > > > > >> 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 dire

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

2017-06-13 Thread Brandon Williams
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 origin

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

2017-06-13 Thread Jeff King
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 > >

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

2017-06-12 Thread Jeff King
On Mon, Jun 12, 2017 at 11:16:27PM -0700, Brandon Williams wrote: > > If the parameter is now required, then it might make sense for it to > > become an actual function parameter instead of being stuffed into the > > config_options struct. That would give you your breaking change, plus > > make it

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

2017-06-12 Thread Jeff King
On Mon, Jun 12, 2017 at 10:52:43PM -0700, Brandon Williams wrote: > > >> 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 th

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

2017-06-12 Thread Brandon Williams
On 06/12, Jeff King wrote: > On Mon, Jun 12, 2017 at 06:38:17PM -0700, Jonathan Nieder wrote: > > > 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_c

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

2017-06-12 Thread Brandon Williams
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, giv

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

2017-06-12 Thread Jeff King
On Mon, Jun 12, 2017 at 06:38:17PM -0700, Jonathan Nieder wrote: > 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 wi

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

2017-06-12 Thread Jonathan Nieder
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 set

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

2017-06-12 Thread Jonathan Nieder
Brandon Williams wrote: > On 06/12, Jonathan Nieder wrote: >> Brandon Williams wrote: >>> Commit 2185fde56 (config: handle conditional include when $GIT_DIR is >>> not set up) added a 'git_dir' field to the config_options struct. Let's >>> use this option field explicitly all the time instead of

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

2017-06-12 Thread Brandon Williams
On 06/12, Jonathan Nieder wrote: > Hi, > > Brandon Williams wrote: > > > Commit 2185fde56 (config: handle conditional include when $GIT_DIR is > > not set up) added a 'git_dir' field to the config_options struct. Let's > > use this option field explicitly all the time instead of occasionally > >

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

2017-06-12 Thread Jonathan Nieder
Hi, Brandon Williams wrote: > Commit 2185fde56 (config: handle conditional include when $GIT_DIR is > not set up) added a 'git_dir' field to the config_options struct. Let's > use this option field explicitly all the time instead of occasionally > falling back to calling 'git_pathdup("config")'

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

2017-06-12 Thread Brandon Williams
Commit 2185fde56 (config: handle conditional include when $GIT_DIR is not set up) added a 'git_dir' field to the config_options struct. Let's use this option field explicitly all the time instead of occasionally falling back to calling 'git_pathdup("config")' to get the path to the local repositor