git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] clone: add remote.cloneDefault config option
@ 2020-08-26 15:45 Sean Barag via GitGitGadget
  2020-08-26 18:46 ` Junio C Hamano
  0 siblings, 1 reply; 8+ messages in thread
From: Sean Barag via GitGitGadget @ 2020-08-26 15:45 UTC (permalink / raw)
  To: git; +Cc: Sean Barag, Sean Barag

From: Sean Barag <sean@barag.org>

While the default remote name of `origin` can be overridden both
pre-clone (with `git clone --origin foo`) and post-clone (with `git
remote rename origin foo`), it'd be handy to have a configurable
system-wide default for clones!  This commit implements
`remote.cloneDefault` as a parallel to `remote.pushDefault`,
with prioritized name resolution:

1. (Highest priority) `git clone`'s `-o` option
2. `git config`'s `remote.cloneDefault` value
3. `origin`

There should be no impact for existing users, as it's pretty unlikely
that anyone's already configured `remote.cloneDefault` and the porcelain
hasn't changed (as best I can tell at least!).

Signed-off-by: Sean Barag <sean@barag.org>
---
    clone: add remote.cloneDefault config option
    
    While the default remote name of origin can be overridden both pre-clone
    (with git clone --origin foo) and post-clone (with git remote rename
    origin foo), it'd be handy to have a configurable system-wide default
    for clones! This implementsremote.cloneDefault as a parallel to 
    remote.pushDefault, with prioritized name resolution:
    
     1. (Highest priority) git clone's -o option
     2. git config's remote.cloneDefault value
     3. origin
    
    There should be no impact for existing users, as it's pretty unlikely
    that anyone's already configured remote.cloneDefault and the porcelain
    hasn't changed (as best I can tell at least!).

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-710%2Fsjbarag%2Fadd-remote.cloneDefault-config-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-710/sjbarag/add-remote.cloneDefault-config-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/710

 Documentation/config/remote.txt |  4 ++++
 Documentation/git-clone.txt     |  5 +++--
 builtin/clone.c                 | 34 +++++++++++++++++++++++++++++++--
 t/t5606-clone-options.sh        | 14 ++++++++++++++
 4 files changed, 53 insertions(+), 4 deletions(-)

diff --git a/Documentation/config/remote.txt b/Documentation/config/remote.txt
index a8e6437a90..debb21ecbf 100644
--- a/Documentation/config/remote.txt
+++ b/Documentation/config/remote.txt
@@ -3,6 +3,10 @@ remote.pushDefault::
 	`branch.<name>.remote` for all branches, and is overridden by
 	`branch.<name>.pushRemote` for specific branches.
 
+remote.cloneDefault::
+	The name of the remote to create during `git clone <url>`.
+	Defaults to "origin".
+
 remote.<name>.url::
 	The URL of a remote repository.  See linkgit:git-fetch[1] or
 	linkgit:git-push[1].
diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index c898310099..2e101ba4f4 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -183,8 +183,9 @@ objects from the source repository into a pack in the cloned repository.
 
 -o <name>::
 --origin <name>::
-	Instead of using the remote name `origin` to keep track
-	of the upstream repository, use `<name>`.
+	The remote name used to keep track of the upstream repository.
+	Overrides `remote.cloneDefault` from the config, and defaults
+	to `origin`.
 
 -b <name>::
 --branch <name>::
diff --git a/builtin/clone.c b/builtin/clone.c
index b087ee40c2..b0dbb848c6 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -941,6 +941,29 @@ static int path_exists(const char *path)
 	return !stat(path, &sb);
 }
 
+struct clone_default_info
+{
+	enum config_scope scope;
+	struct strbuf remote_name;
+	int linenr;
+};
+
+static int config_read_clone_default(const char *key, const char *value,
+	void *cb)
+{
+	struct clone_default_info* info = cb;
+	if (strcmp(key, "remote.clonedefault") || !value) {
+		return 0;
+	}
+
+	info->scope = current_config_scope();
+	strbuf_reset(&info->remote_name);
+	strbuf_addstr(&info->remote_name, value);
+	info->linenr = current_config_line();
+
+	return 0;
+}
+
 int cmd_clone(int argc, const char **argv, const char *prefix)
 {
 	int is_bundle = 0, is_local;
@@ -992,8 +1015,15 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 		option_no_checkout = 1;
 	}
 
-	if (!option_origin)
-		option_origin = "origin";
+	if (!option_origin) {
+		struct clone_default_info clone_default = { CONFIG_SCOPE_UNKNOWN, STRBUF_INIT, -1 };
+		git_config(config_read_clone_default, &clone_default);
+		if (strcmp("", (const char*) clone_default.remote_name.buf))
+			option_origin = clone_default.remote_name.buf;
+		else
+			option_origin = "origin";
+	}
+
 
 	repo_name = argv[0];
 
diff --git a/t/t5606-clone-options.sh b/t/t5606-clone-options.sh
index e69427f881..8aac67b385 100755
--- a/t/t5606-clone-options.sh
+++ b/t/t5606-clone-options.sh
@@ -19,6 +19,20 @@ test_expect_success 'clone -o' '
 
 '
 
+test_expect_success 'clone respects remote.cloneDefault' '
+
+	git -c remote.cloneDefault=bar clone parent clone-config &&
+	(cd clone-config && git rev-parse --verify refs/remotes/bar/master)
+
+'
+
+test_expect_success 'clone chooses correct remote name' '
+
+	git -c remote.cloneDefault=bar clone -o foo parent clone-o-and-config &&
+	(cd clone-o-and-config && git rev-parse --verify refs/remotes/foo/master)
+
+'
+
 test_expect_success 'redirected clone does not show progress' '
 
 	git clone "file://$(pwd)/parent" clone-redirected >out 2>err &&

base-commit: e9b77c84a0a0df029f2a3a8114e9f22186e7da24
-- 
gitgitgadget

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH v2 7/7] clone: allow configurable default for `-o`/`--origin`
@ 2020-09-29 19:59 Junio C Hamano
  2020-09-29 23:47 ` [PATCH] clone: add remote.cloneDefault config option Sean Barag
  0 siblings, 1 reply; 8+ messages in thread
From: Junio C Hamano @ 2020-09-29 19:59 UTC (permalink / raw)
  To: Sean Barag via GitGitGadget
  Cc: git, Johannes Schindelin, Derrick Stolee, Jeff King,
	Eric Sunshine, Taylor Blau, Sean Barag, Andrei Rybak

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

>  static int git_clone_config(const char *k, const char *v, void *cb)
>  {
> +	if (!strcmp(k, "clone.defaultremotename")) {
> +		if (remote_name != default_remote_name)
> +			free(remote_name);
> +		remote_name = xstrdup(v);

This feels strange.  The usual arrangement is

    - initialize the variable to NULL (or any value that the code
      can tell that nobody touched it);

    - let git_config() callback to update the variable, taking care
      of freeing and strduping as needed.  Note that free(NULL) is
      kosher.

    - let parse_options() to further update the variable, taking
      care of freeing and strduping as needed.

    - finally, if the variable is still NULL, give it its default
      value.

so there is no room for the "if the variable has the value of the
fallback default, do things differently" logic to be in the
git_config() callback function.

> @@ -976,6 +982,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
>  	int submodule_progress;
>  
>  	struct strvec ref_prefixes = STRVEC_INIT;

Missing blank line here.

> +	remote_name = default_remote_name;

Isn't the reason why the git_config() callback we saw above has an
unusual special case for default_remote_name because we have this
assignment way too early?  Would it make the control flow in a more
natural way if we removed this, and then after parse_options() did
the "-o" handling, add something like

	if (!remote_name)
		remote_name = xstrdup("origin");

instead?

> @@ -1153,10 +1154,20 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
>  
>  	/*
>  	 * re-read config after init_db and write_config to pick up any config
> -	 * injected by --template and --config, respectively
> +	 * injected by --template and --config, respectively.
>  	 */

Squash this "oops, I forgot to finish the sentence" to the step the
mistake was introduced, i.e. "use more conventional..."

>  	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)
> +		remote_name = option_origin;

And here would be where you'd fall back

	if (!remote_name)
		remote_name = xstrdup("origin");

Note that you'd need to dup the option_origin for consistency if you
want to free() it at the end.

> +	if (!valid_remote_name(remote_name))
> +		die(_("'%s' is not a valid remote name"), remote_name);
> +

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-09-29 23:48 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-26 15:45 [PATCH] clone: add remote.cloneDefault config option Sean Barag via GitGitGadget
2020-08-26 18:46 ` Junio C Hamano
2020-08-26 19:04   ` Derrick Stolee
2020-08-26 19:59     ` Junio C Hamano
2020-08-27  3:38       ` Sean Barag
2020-08-27  4:21         ` Junio C Hamano
2020-08-27 14:00           ` Sean Barag
2020-09-29 19:59 [PATCH v2 7/7] clone: allow configurable default for `-o`/`--origin` Junio C Hamano
2020-09-29 23:47 ` [PATCH] clone: add remote.cloneDefault config option Sean Barag

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).