Re: [PATCH 2/4] clone: call git_config before parse_options
- From
Derrick Stolee <stolee@gmail.com>
- Date
- Sep 11, 2020, 18:59 UTC
- 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:
Show 35 quoted lines
> 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.
Show 7 quoted lines
> 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