From: Derrick Stolee <stolee@gmail.com>
Date: Fri, 11 Sep 2020 18:59:59 GMT
Subject: Re: [PATCH 2/4] clone: call git_config before parse_options
Message-ID: <48b160f0-0cb3-e120-dc11-1d5de7e02af1@gmail.com>
In-Reply-To: <51ef776f8523d29dfe03d15f0d1958f5c456c057.1599848727.git.gitgitgadget@gmail.com>

On 9/11/2020 2:25 PM, Sean Barag via GitGitGadget wrote:
> From: Sean Barag <sean@barag.org>
> 
> While Junio's request [1] was to avoids the unusual  "write config then
> immediately read it" pattern that exists in `cmd_clone`, Johannes
> mentioned that --template can write new config values that aren't
> automatically included in the environment [2]. This requires a config
> re-read after `init_db` is called.
> 
> Moving the initial config up does allow settings from config to be
> overwritten by ones provided via CLI options in a more natural way
> though, so that part of Junio's suggestion remains.
> 
> [1] https://lore.kernel.org/git/pull.710.git.1598456751674.gitgitgadget@gmail.com/
> [2] https://github.com/gitgitgadget/git/pull/727#issuecomment-689740195
> 
> Signed-off-by: Sean Barag <sean@barag.org>
> Thanks-to: Junio C Hamano <gitster@pobox.com>
> Thanks-to: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  builtin/clone.c | 26 +++++++++++++++++++++++++-
>  1 file changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/builtin/clone.c b/builtin/clone.c
> index b087ee40c2..bf095815f0 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -851,8 +851,21 @@ static int checkout(int submodule_progress)
>  	return err;
>  }
>  
> +static int git_clone_config(const char *k, const char *v, void *cb)
> +{
> +	return git_default_config(k, v, cb);
> +}
> +

Introducing this no-op seems a bit premature, but as long
as it makes your future patches cleaner, then it is
appropriate.

>  static int write_one_config(const char *key, const char *value, void *data)
>  {
> +	/*
> +	 * give git_config_default a chance to write config values back to the environment, since
> +	 * git_config_set_multivar_gently only deals with config-file writes
> +	 */
> +	int apply_failed = git_default_config(key, value, data);

However, you change this to git_clone_config() in patch 4. Perhaps
use git_clone_config() here instead?

Thanks,
-Stolee