Re: [PATCH 4/4] clone: allow configurable default for `-o`/`--origin`

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 11, 2020, 21:00 UTC
Message-ID: <xmqqwo10m3kv.fsf@gitster.c.googlers.com>
In-Reply-To: <5c519376c29a28bd89a712cf0b8125fc1c2c81be.1599848727.git.gitgitgadget@gmail.com>

"Sean Barag via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 9 quoted lines

> diff --git a/builtin/clone.c b/builtin/clone.c
> index 1cd62d0001..aeb41f15f3 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -53,6 +53,7 @@ static int option_shallow_submodules;
>  static int deepen;
>  static char *option_template, *option_depth, *option_since;
>  static char *option_origin = NULL;
> +static char *remote_name = "origin";

This has a side effect of making all the code locations that used to refer to option_origin much easier to read, like ...

Show 6 quoted lines

> @@ -721,7 +722,7 @@ static void update_head(const struct ref *our, const struct ref *remote,
>  		if (!option_bare) {
>  			update_ref(msg, "HEAD", &our->old_oid, NULL, 0,
>  				   UPDATE_REFS_DIE_ON_ERR);
> -			install_branch_config(0, head, option_origin, our->name);
> +			install_branch_config(0, head, remote_name, our->name);

... this place ;-)  Happy.

Show 6 quoted lines

>  static int git_clone_config(const char *k, const char *v, void *cb)
>  {
> +	if (!strcmp(k, "clone.defaultremotename") && !option_origin)
> +		remote_name = xstrdup(v);
>  	return git_default_config(k, v, cb);
>  }

clone.defaultremotename is a single valued configuration variable, and this correctly implements the "last one wins" behaviour (but previous remote_name will leak every time clone.defaultremotename is seen in the config stream).

Also this code arrangement is not quite satisfactory. It means that we cannot re-read any configuration variable that does not have an accompanying command line option. I thought the whole point of doing the write_config() was so that anything came from the command line option can be written back to the configuration file, so I am not sure what the harm would be to update remote_name from the configuration whether option_origin is used or not here. Perhaps add "clone.defaultremotename" to the set of configuration setting write_config() uses, when --option is given from the command line, and remove this special case?

By the way, I now realized why 2/4's "read twice" is OK. init_db() calls create_default_files() and we do clare the cached configset by calling git_config_clear() there.

Thanks.