[PATCH v3] remote: add --save-to-push option to git remote set-url

[Denton Liu <liu.denton@gmail.com>]

This adds the --save-to-push option to `git remote set-url` such that
when executed, we move the remote.*.url to remote.*.pushurl and set
remote.*.url to the given url argument.

For example, if we have the following config:

	[remote "origin"]
		url = git@github.com:git/git.git

`git remote set-url --save-to-push origin https://github.com/git/git.git`
would change the config to the following:

	[remote "origin"]
		url = https://github.com/git/git.git
		pushurl = git@github.com:git/git.git

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
On Fri, Nov 09, 2018 at 12:15:22PM +0900, Junio C Hamano wrote:
> This sounds more like "saving to push" (i.e. what you are saving is
> the existing "url" and the "push" is a shorthand for "pushURL",
> which is the location the old value of "url" is aved to), not "save
> (the) push(URL)".  So if adding this option makes sense, I would say
> "--save-to-push" (or even "--save-to-pushURL") may be a more
> appropriate name for it.
> 

My original intention was for it to mean "save push behavior" but I
agree with you that it's unclear so I'm changing it to --save-to-push.

> Ambigous in what way?  You asked the current URL to be saved as a
> pushURL, so existing pushURL destinations should not come into play,
> I would think.  If there are more than one URL (not pushURL), on the
> other hand, I think you have a bigger problem (where would "git fetch"
> fetch from, and how would these multiple URLs are prevented from
> trashing refs/remotes/$remote/* with each other's refs?), so
> stopping the operation before "set-url" makes the problem worse is
> probably a good idea, but I think that is true with or without this
> new option.
> 

> I _think_ in the future (if this option turns out to be widely used)
> people may ask for this condition to be loosened somewhat, but it is
> relatively easy to start restrictive and then to loosen later, so I
> think this is OK for now.
> 

I agree, there's no reason why we shouldn't allow appending to the push
URLs if one already exists so I removed that removed that restriction.
---
 Documentation/git-remote.txt |  5 +++++
 builtin/remote.c             | 26 +++++++++++++++++++++-----
 t/t5505-remote.sh            | 11 +++++++++++
 3 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt
index 0cad37fb81..8f9d700252 100644
--- a/Documentation/git-remote.txt
+++ b/Documentation/git-remote.txt
@@ -19,6 +19,7 @@ SYNOPSIS
 'git remote set-url' [--push] <name> <newurl> [<oldurl>]
 'git remote set-url --add' [--push] <name> <newurl>
 'git remote set-url --delete' [--push] <name> <url>
+'git remote set-url --save-to-push' <name> <url>
 'git remote' [-v | --verbose] 'show' [-n] <name>...
 'git remote prune' [-n | --dry-run] <name>...
 'git remote' [-v | --verbose] 'update' [-p | --prune] [(<group> | <remote>)...]
@@ -155,6 +156,10 @@ With `--delete`, instead of changing existing URLs, all URLs matching
 regex <url> are deleted for remote <name>.  Trying to delete all
 non-push URLs is an error.
 +
+With `--save-to-push`, the current URL is saved into the push URL before
+setting the URL to <url>. Note that this command will not work if more than one
+URL is defined because the behavior would be ambiguous.
++
 Note that the push URL and the fetch URL, even though they can
 be set differently, must still refer to the same place.  What you
 pushed to the push URL should be what you would see if you
diff --git a/builtin/remote.c b/builtin/remote.c
index f7edf7f2cb..3249c3face 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -24,8 +24,9 @@ static const char * const builtin_remote_usage[] = {
 	N_("git remote set-branches [--add] <name> <branch>..."),
 	N_("git remote get-url [--push] [--all] <name>"),
 	N_("git remote set-url [--push] <name> <newurl> [<oldurl>]"),
-	N_("git remote set-url --add <name> <newurl>"),
-	N_("git remote set-url --delete <name> <url>"),
+	N_("git remote set-url --add [--push] <name> <newurl>"),
+	N_("git remote set-url --delete [--push] <name> <url>"),
+	N_("git remote set-url --save-to-push <name> <url>"),
 	NULL
 };
 
@@ -77,8 +78,9 @@ static const char * const builtin_remote_geturl_usage[] = {
 
 static const char * const builtin_remote_seturl_usage[] = {
 	N_("git remote set-url [--push] <name> <newurl> [<oldurl>]"),
-	N_("git remote set-url --add <name> <newurl>"),
-	N_("git remote set-url --delete <name> <url>"),
+	N_("git remote set-url --add [--push] <name> <newurl>"),
+	N_("git remote set-url --delete [--push] <name> <url>"),
+	N_("git remote set-url --save-to-push <name> <url>"),
 	NULL
 };
 
@@ -1519,7 +1521,7 @@ static int get_url(int argc, const char **argv)
 
 static int set_url(int argc, const char **argv)
 {
-	int i, push_mode = 0, add_mode = 0, delete_mode = 0;
+	int i, push_mode = 0, save_to_push = 0, add_mode = 0, delete_mode = 0;
 	int matches = 0, negative_matches = 0;
 	const char *remotename = NULL;
 	const char *newurl = NULL;
@@ -1532,6 +1534,8 @@ static int set_url(int argc, const char **argv)
 	struct option options[] = {
 		OPT_BOOL('\0', "push", &push_mode,
 			 N_("manipulate push URLs")),
+		OPT_BOOL('\0', "save-to-push", &save_to_push,
+			 N_("change fetching URL behavior")),
 		OPT_BOOL('\0', "add", &add_mode,
 			 N_("add URL")),
 		OPT_BOOL('\0', "delete", &delete_mode,
@@ -1543,6 +1547,8 @@ static int set_url(int argc, const char **argv)
 
 	if (add_mode && delete_mode)
 		die(_("--add --delete doesn't make sense"));
+	if (save_to_push && (push_mode || add_mode || delete_mode))
+		die(_("--save-to-push cannot be used with other options"));
 
 	if (argc < 3 || argc > 4 || ((add_mode || delete_mode) && argc != 3))
 		usage_with_options(builtin_remote_seturl_usage, options);
@@ -1564,6 +1570,16 @@ static int set_url(int argc, const char **argv)
 		urlset = remote->pushurl;
 		urlset_nr = remote->pushurl_nr;
 	} else {
+		if (save_to_push) {
+			if (remote->url_nr != 1)
+				die(_("--save-to-push can only be used when only one url is defined"), remotename);
+
+			strbuf_addf(&name_buf, "remote.%s.pushurl", remotename);
+			git_config_set_multivar(name_buf.buf,
+			      remote->url[0], "^$", 0);
+			strbuf_reset(&name_buf);
+		}
+
 		strbuf_addf(&name_buf, "remote.%s.url", remotename);
 		urlset = remote->url;
 		urlset_nr = remote->url_nr;
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index d2a2cdd453..434c1f828a 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -1194,6 +1194,17 @@ test_expect_success 'remote set-url --delete baz' '
 	cmp expect actual
 '
 
+test_expect_success 'remote set-url --save-to-push bbb' '
+	git remote set-url --save-to-push someremote bbb &&
+	echo bbb >expect &&
+	echo "YYY" >>expect &&
+	echo ccc >>expect &&
+	git config --get-all remote.someremote.url >actual &&
+	echo "YYY" >>actual &&
+	git config --get-all remote.someremote.pushurl >>actual &&
+	cmp expect actual
+'
+
 test_expect_success 'extra args: setup' '
 	# add a dummy origin so that this does not trigger failure
 	git remote add origin .
-- 
2.19.1