From 552955ed7fcbc3706f2d6192487df2fb8f3283c8 Mon Sep 17 00:00:00 2001 From: Sean Barag Date: Thu, 1 Oct 2020 03:46:11 +0000 Subject: clone: use more conventional config/option layering Parsing command-line options before reading from config required careful handling to ensure CLI options were treated with higher priority. Read config first to let parsed CLI naively overwrite matching config values. Helped-by: Junio C Hamano Helped-by: Johannes Schindelin Signed-off-by: Sean Barag Signed-off-by: Junio C Hamano --- builtin/clone.c | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) (limited to 'builtin') diff --git a/builtin/clone.c b/builtin/clone.c index b087ee40c2..258bfd65b7 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -851,8 +851,22 @@ 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); +} + static int write_one_config(const char *key, const char *value, void *data) { + /* + * give git_clone_config 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_clone_config(key, value, data); + if (apply_failed) + return apply_failed; + return git_config_set_multivar_gently(key, value ? value : "true", CONFIG_REGEX_NONE, 0); @@ -964,6 +978,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix) struct strvec ref_prefixes = STRVEC_INIT; packet_trace_identity("clone"); + + git_config(git_clone_config, NULL); + argc = parse_options(argc, argv, prefix, builtin_clone_options, builtin_clone_usage, 0); @@ -1125,9 +1142,17 @@ int cmd_clone(int argc, const char **argv, const char *prefix) if (real_git_dir) git_dir = real_git_dir; + /* + * additional config can be injected with -c, make sure it's included + * after init_db, which clears the entire config environment. + */ write_config(&option_config); - git_config(git_default_config, NULL); + /* + * re-read config after init_db and write_config to pick up any config + * injected by --template and --config, respectively. + */ + git_config(git_clone_config, NULL); if (option_bare) { if (option_mirror) -- cgit v1.2.3 From f2c6fda88624370fda1fc706a0f2ceda7d50d6ab Mon Sep 17 00:00:00 2001 From: Sean Barag Date: Thu, 1 Oct 2020 03:46:13 +0000 Subject: refs: consolidate remote name validation In preparation for a future patch, extract from remote.c a function that validates possible remote names so that its rules can be used consistently in other places. Helped-by: Derrick Stolee Signed-off-by: Sean Barag Signed-off-by: Junio C Hamano --- builtin/remote.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) (limited to 'builtin') diff --git a/builtin/remote.c b/builtin/remote.c index 542f56e387..b2e0959746 100644 --- a/builtin/remote.c +++ b/builtin/remote.c @@ -194,8 +194,7 @@ static int add(int argc, const char **argv) if (remote_is_configured(remote, 1)) die(_("remote %s already exists."), name); - strbuf_addf(&buf2, "refs/heads/test:refs/remotes/%s/test", name); - if (!valid_fetch_refspec(buf2.buf)) + if (!valid_remote_name(name)) die(_("'%s' is not a valid remote name"), name); strbuf_addf(&buf, "remote.%s.url", name); @@ -696,11 +695,9 @@ static int mv(int argc, const char **argv) if (remote_is_configured(newremote, 1)) die(_("remote %s already exists."), rename.new_name); - strbuf_addf(&buf, "refs/heads/test:refs/remotes/%s/test", rename.new_name); - if (!valid_fetch_refspec(buf.buf)) + if (!valid_remote_name(rename.new_name)) die(_("'%s' is not a valid remote name"), rename.new_name); - strbuf_reset(&buf); strbuf_addf(&buf, "remote.%s", rename.old_name); strbuf_addf(&buf2, "remote.%s", rename.new_name); if (git_config_rename_section(buf.buf, buf2.buf) < 1) -- cgit v1.2.3 From ebe7e28a3600f4e67e9a1781e335adb36f3f139b Mon Sep 17 00:00:00 2001 From: Sean Barag Date: Thu, 1 Oct 2020 03:46:14 +0000 Subject: clone: validate --origin option before use Providing a bad origin name to `git clone` currently reports an 'invalid refspec' error instead of a more explicit message explaining that the `--origin` option was malformed. This behavior dates back to since 8434c2f1 (Build in clone, 2008-04-27). Reintroduce validation for the provided `--origin` option, but notably _don't_ include a multi-level check (e.g. "foo/bar") that was present in the original `git-clone.sh`. `git remote` allows multi-level remote names since at least 46220ca100 (remote.c: Fix overtight refspec validation, 2008-03-20), so that appears to be the desired behavior. Helped-by: Junio C Hamano Helped-by: Derrick Stolee Helped-by: Jeff King Signed-off-by: Sean Barag Signed-off-by: Junio C Hamano --- builtin/clone.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'builtin') diff --git a/builtin/clone.c b/builtin/clone.c index 258bfd65b7..78364a0861 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -1012,6 +1012,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix) if (!option_origin) option_origin = "origin"; + if (!valid_remote_name(option_origin)) + die(_("'%s' is not a valid remote name"), option_origin); + repo_name = argv[0]; path = get_repo_path(repo_name, &is_bundle); -- cgit v1.2.3 From 75ca3906b1ea6a00a20ef16c889e9d6a15a8defc Mon Sep 17 00:00:00 2001 From: Sean Barag Date: Thu, 1 Oct 2020 03:46:15 +0000 Subject: clone: read new remote name from remote_name instead of option_origin In a future patch, the name of the remote created by `git clone` may come from multiple sources. To avoid confusion, convert most uses of option_origin to remote_name, leaving option_origin to exclusively represent the -o/--origin option. Helped-by: Derrick Stolee Signed-off-by: Sean Barag Signed-off-by: Junio C Hamano --- builtin/clone.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) (limited to 'builtin') diff --git a/builtin/clone.c b/builtin/clone.c index 78364a0861..c33fa127ce 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"; static char *option_branch = NULL; static struct string_list option_not = STRING_LIST_INIT_NODUP; static const char *real_git_dir; @@ -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); } } else if (our) { struct commit *c = lookup_commit_reference(the_repository, @@ -919,12 +920,12 @@ static void write_refspec_config(const char *src_ref_prefix, } /* Configure the remote */ if (value.len) { - strbuf_addf(&key, "remote.%s.fetch", option_origin); + strbuf_addf(&key, "remote.%s.fetch", remote_name); git_config_set_multivar(key.buf, value.buf, "^$", 0); strbuf_reset(&key); if (option_mirror) { - strbuf_addf(&key, "remote.%s.mirror", option_origin); + strbuf_addf(&key, "remote.%s.mirror", remote_name); git_config_set(key.buf, "true"); strbuf_reset(&key); } @@ -1009,11 +1010,11 @@ int cmd_clone(int argc, const char **argv, const char *prefix) option_no_checkout = 1; } - if (!option_origin) - option_origin = "origin"; + if (option_origin) + remote_name = option_origin; - if (!valid_remote_name(option_origin)) - die(_("'%s' is not a valid remote name"), option_origin); + if (!valid_remote_name(remote_name)) + die(_("'%s' is not a valid remote name"), remote_name); repo_name = argv[0]; @@ -1164,15 +1165,15 @@ int cmd_clone(int argc, const char **argv, const char *prefix) git_config_set("core.bare", "true"); } else { - strbuf_addf(&branch_top, "refs/remotes/%s/", option_origin); + strbuf_addf(&branch_top, "refs/remotes/%s/", remote_name); } - strbuf_addf(&key, "remote.%s.url", option_origin); + strbuf_addf(&key, "remote.%s.url", remote_name); git_config_set(key.buf, repo); strbuf_reset(&key); if (option_no_tags) { - strbuf_addf(&key, "remote.%s.tagOpt", option_origin); + strbuf_addf(&key, "remote.%s.tagOpt", remote_name); git_config_set(key.buf, "--no-tags"); strbuf_reset(&key); } @@ -1183,7 +1184,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix) if (option_sparse_checkout && git_sparse_checkout_init(dir)) return 1; - remote = remote_get(option_origin); + remote = remote_get(remote_name); strbuf_addf(&default_refspec, "+%s*:%s*", src_ref_prefix, branch_top.buf); @@ -1296,7 +1297,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix) if (!our_head_points_at) die(_("Remote branch %s not found in upstream %s"), - option_branch, option_origin); + option_branch, remote_name); } else our_head_points_at = remote_head_points_at; @@ -1304,7 +1305,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix) else { if (option_branch) die(_("Remote branch %s not found in upstream %s"), - option_branch, option_origin); + option_branch, remote_name); warning(_("You appear to have cloned an empty repository.")); mapped_refs = NULL; @@ -1316,7 +1317,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix) const char *branch = git_default_branch_name(); char *ref = xstrfmt("refs/heads/%s", branch); - install_branch_config(0, branch, option_origin, ref); + install_branch_config(0, branch, remote_name, ref); free(ref); } } @@ -1325,7 +1326,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix) remote_head_points_at, &branch_top); if (filter_options.choice) - partial_clone_register(option_origin, &filter_options); + partial_clone_register(remote_name, &filter_options); if (is_local) clone_local(path, git_dir); -- cgit v1.2.3 From de9ed3ef3740f8227cc924e845032954d1f1b1b7 Mon Sep 17 00:00:00 2001 From: Sean Barag Date: Thu, 1 Oct 2020 03:46:16 +0000 Subject: clone: allow configurable default for `-o`/`--origin` While the default remote name of "origin" can be changed at clone-time with `git clone`'s `--origin` option, it was previously not possible to specify a default value for the name of that remote. Add support for a new `clone.defaultRemoteName` config, with the newly-created remote name resolved in priority order: 1. (Highest priority) A remote name passed directly to `git clone -o` 2. A `clone.defaultRemoteName=new_name` in config `git clone -c` 3. A `clone.defaultRemoteName` value set in `/path/to/template/config`, where `--template=/path/to/template` is provided 4. A `clone.defaultRemoteName` value set in a non-template config file 5. The default value of `origin` Helped-by: Junio C Hamano Helped-by: Johannes Schindelin Helped-by: Derrick Stolee Helped-by: Andrei Rybak Signed-off-by: Sean Barag Signed-off-by: Junio C Hamano --- builtin/clone.c | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) (limited to 'builtin') diff --git a/builtin/clone.c b/builtin/clone.c index c33fa127ce..a144e7f35b 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -53,7 +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"; +static char *remote_name = NULL; static char *option_branch = NULL; static struct string_list option_not = STRING_LIST_INIT_NODUP; static const char *real_git_dir; @@ -854,6 +854,10 @@ static int checkout(int submodule_progress) static int git_clone_config(const char *k, const char *v, void *cb) { + if (!strcmp(k, "clone.defaultremotename")) { + free(remote_name); + remote_name = xstrdup(v); + } return git_default_config(k, v, cb); } @@ -1010,12 +1014,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix) option_no_checkout = 1; } - if (option_origin) - remote_name = option_origin; - - if (!valid_remote_name(remote_name)) - die(_("'%s' is not a valid remote name"), remote_name); - repo_name = argv[0]; path = get_repo_path(repo_name, &is_bundle); @@ -1158,6 +1156,19 @@ int cmd_clone(int argc, const char **argv, const char *prefix) */ git_config(git_clone_config, NULL); + /* + * apply the remote name provided by --origin only after this second + * call to git_config, to ensure it overrides all config-based values. + */ + if (option_origin != NULL) + remote_name = xstrdup(option_origin); + + if (remote_name == NULL) + remote_name = xstrdup("origin"); + + if (!valid_remote_name(remote_name)) + die(_("'%s' is not a valid remote name"), remote_name); + if (option_bare) { if (option_mirror) src_ref_prefix = "refs/"; @@ -1358,6 +1369,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix) junk_mode = JUNK_LEAVE_REPO; err = checkout(submodule_progress); + free(remote_name); strbuf_release(&reflog_msg); strbuf_release(&branch_top); strbuf_release(&key); -- cgit v1.2.3