Subject: Re: [PATCH v2 04/32] config: don't implicitly use gitdir

From: Brandon Williams <[email-masked]>
Date: Mon, 12 Jun 2017 12:57:10 -0700

On 06/08, 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")' to get the path to the
> local repository configuration.  This allows 'do_git_config_sequence()'
> to not implicitly rely on global repository state.
> 
> Signed-off-by: Brandon Williams <[email-masked]>
> ---
>  builtin/config.c | 2 ++
>  config.c         | 4 ++--
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/builtin/config.c b/builtin/config.c
> index 753c40a5c..90f49a6ee 100644
> --- a/builtin/config.c
> +++ b/builtin/config.c
> @@ -539,6 +539,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)
>  		config_options.respect_includes = !given_config_source.file;
>  	else
>  		config_options.respect_includes = respect_includes_opt;
> +	if (have_git_dir())
> +		config_options.git_dir = get_git_common_dir();
>  
>  	if (end_null) {
>  		term = '\0';
> diff --git a/config.c b/config.c
> index 2390f98e3..ff09b27b8 100644
> --- a/config.c
> +++ b/config.c
> @@ -1548,8 +1548,6 @@ static int do_git_config_sequence(const struct config_options *opts,
>  
>  	if (opts->git_dir)
>  		repo_config = mkpathdup("%s/config", opts->git_dir);
> -	else if (have_git_dir())
> -		repo_config = git_pathdup("config");
>  	else
>  		repo_config = NULL;
>  
> @@ -1613,6 +1611,8 @@ static void git_config_raw(config_fn_t fn, void *data)
>  	struct config_options opts = {0};
>  
>  	opts.respect_includes = 1;
> +	if (have_git_dir())
> +		opts.git_dir = get_git_common_dir();
>  	if (git_config_with_options(fn, data, NULL, &opts) < 0)
>  		/*
>  		 * git_config_with_options() normally returns only

I missed a case in 'include_by_gitdir'.

Given that these config changes touch a lot of files (making a config.h
files and including it everyone) I think I'll break this into its own
series so that it can get reviewed and merged in faster.  This is
especially due to this series breaking pu right now.

-- 
Brandon Williams