| Message ID | b910987d3aa778b39b6e27a75fe685a4797b8422.1541730538.git.liu.denton@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC,v2] remote: add --save-push option to git remote set-url | expand |
Denton Liu <liu.denton@gmail.com> writes: > This adds the --save-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-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 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. > +With `--save-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 or if any push URLs are defined because behavior would be ambiguous. 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. > diff --git a/builtin/remote.c b/builtin/remote.c > index f7edf7f2cb..0eaec7ef38 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-push <name> <url>"), > NULL > }; Needs update? > @@ -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-push <name> <url>"), > NULL > }; Needs update? > + if (save_push) { > + if (remote->url_nr != 1 || remote->pushurl_nr != 0) > + die(_("--save-push can only be used when one url and no pushurl is defined"), remotename); 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.
diff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt index 0cad37fb81..8ce85fe2f2 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-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-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 or if any push URLs are defined because 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..0eaec7ef38 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-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-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_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-push", &save_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_push && (push_mode || add_mode || delete_mode)) + die(_("--save-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,15 @@ static int set_url(int argc, const char **argv) urlset = remote->pushurl; urlset_nr = remote->pushurl_nr; } else { + if (save_push) { + if (remote->url_nr != 1 || remote->pushurl_nr != 0) + die(_("--save-push can only be used when one url and no pushurl is defined"), remotename); + + strbuf_addf(&name_buf, "remote.%s.pushurl", remotename); + git_config_set(name_buf.buf, remote->url[0]); + strbuf_reset(&name_buf); + } + strbuf_addf(&name_buf, "remote.%s.url", remotename); urlset = remote->url; urlset_nr = remote->url_nr;
This adds the --save-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-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> --- I decided to address your comments and reroll the patch one more time. Even though the option isn't _that_ commonly used, I've managed to use it a couple of times since I've implemented so I believe that this option should be included. --- Documentation/git-remote.txt | 5 +++++ builtin/remote.c | 25 ++++++++++++++++++++----- 2 files changed, 25 insertions(+), 5 deletions(-)