Re: [PATCH v3] remote: add --fetch and --both options to set-url

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/25/2014 12:48 PM, Peter Wu wrote:
git remote set-url knew about the '--push' option to update just the
pushurl, but it does not have a similar option for "update fetch URL and
leave whatever was in place for the push URL".

This patch adds support for a '--fetch' option which implements that use
case in a backwards compatible way: if no --both, --push or --fetch
options are given, then the push URL is modified too if it was not set
before. This is the case since the push URL is implicitly based on the
fetch URL.

A '--both' option is added to make the command independent of previous
pushurl settings. For the --add and --delete set operations, it will
always set the push and/ or the fetch URLs. For the primary mode of
operation (without --add or --delete), it will drop pushurl as the
implicit push URL is the (fetch) URL.

The documentation has also been updated and a missing '--push' option
is added to the 'git remote -h' command.

Tests are also added to verify the documented behavior.

Signed-off-by: Peter Wu <peter@xxxxxxxxxxxxx>
---

  v2: fixed test case
  v3: added --both option, changed --fetch --push behavior, added more tests for
      --add/--delete cases, refactored to reduce duplication (not special-casing
      add_mode without oldurl, just skip initially setting oldurl).

Translators note: the help text gained more translatable strings and some
strings got additional options.
---
  Documentation/git-remote.txt |  17 ++++--
  builtin/remote.c             | 140 +++++++++++++++++++++++++++----------------
  t/t5505-remote.sh            | 125 ++++++++++++++++++++++++++++++++++++++
  3 files changed, 228 insertions(+), 54 deletions(-)

diff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt
index cb103c8..bdd0305 100644
--- a/Documentation/git-remote.txt
+++ b/Documentation/git-remote.txt
@@ -15,9 +15,9 @@ SYNOPSIS
  'git remote remove' <name>
  'git remote set-head' <name> (-a | --auto | -d | --delete | <branch>)
  'git remote set-branches' [--add] <name> <branch>...
-'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' [--both | --fetch | --push] <name> <newurl> [<oldurl>]
+'git remote set-url' [--both | --fetch | --push] '--add' <name> <newurl>
+'git remote set-url' [--both | --fetch | --push] '--delete' <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>)...]
@@ -134,7 +134,16 @@ Changes URL remote points to. Sets first URL remote points to matching
  regex <oldurl> (first URL if no <oldurl> is given) to <newurl>. If
  <oldurl> doesn't match any URL, error occurs and nothing is changed.
  +
-With '--push', push URLs are manipulated instead of fetch URLs.
+With '--both', both the fetch and push URLs are manipulated.
++
+With '--fetch', only fetch URLs are manipulated.
++
+With '--push', only push URLs are manipulated.
++
+If none of the '--both', '--fetch' or --push' options are given, then
+'--both' applies only if no push URL was set before. Otherwise '--fetch'
+is assumed for historical reasons. This default may change in the
+future to '--both' to avoid surprises depending on the configuration.
  +
  With '--add', instead of changing some URL, new URL is added.
  +
diff --git a/builtin/remote.c b/builtin/remote.c
index 7f28f92..a250b23 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -18,9 +18,9 @@ static const char * const builtin_remote_usage[] = {
  	N_("git remote prune [-n | --dry-run] <name>"),
  	N_("git remote [-v | --verbose] update [-p | --prune] [(<group> | <remote>)...]"),
  	N_("git remote set-branches [--add] <name> <branch>..."),
-	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 [--both | --fetch | --push] <name> <newurl> [<oldurl>]"),
+	N_("git remote set-url [--both | --fetch | --push] --add <name> <newurl>"),
+	N_("git remote set-url [--both | --fetch | --push] --delete <name> <url>"),
  	NULL
  };
@@ -66,9 +66,9 @@ static const char * const builtin_remote_update_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 [--both | --fetch | --push] <name> <newurl> [<oldurl>]"),
+	N_("git remote set-url [--both | --fetch | --push] --add <name> <newurl>"),
+	N_("git remote set-url [--both | --fetch | --push] --delete <name> <url>"),
  	NULL
  };
@@ -1503,21 +1503,35 @@ static int set_branches(int argc, const char **argv)
  	return set_remote_branches(argv[0], argv + 1, add_mode);
  }
+#define MODIFY_TYPE_FETCH (1 << 0)
+#define MODIFY_TYPE_PUSH        (1 << 1)
+#define MODIFY_TYPE_BOTH        (MODIFY_TYPE_FETCH | MODIFY_TYPE_PUSH)
+#define MODIFY_TYPE_HISTORIC    (MODIFY_TYPE_FETCH | (1 << 2))
+
  static int set_url(int argc, const char **argv)
  {
-	int i, push_mode = 0, add_mode = 0, delete_mode = 0;
+	int i, add_mode = 0, delete_mode = 0;
+	int modify_type = MODIFY_TYPE_HISTORIC;
  	int matches = 0, negative_matches = 0;
  	const char *remotename = NULL;
  	const char *newurl = NULL;
  	const char *oldurl = NULL;
  	struct remote *remote;
  	regex_t old_regex;
-	const char **urlset;
-	int urlset_nr;
-	struct strbuf name_buf = STRBUF_INIT;
+	struct strbuf name_buf_fetch = STRBUF_INIT;
+	struct strbuf name_buf_push = STRBUF_INIT;
  	struct option options[] = {
-		OPT_BOOL('\0', "push", &push_mode,
-			 N_("manipulate push URLs")),
+		OPT_GROUP(""),
+		OPT_SET_INT('\0', "fetch", &modify_type,
+			N_("manipulate just fetch URLs"),
+			MODIFY_TYPE_FETCH),
+		OPT_SET_INT('\0', "push", &modify_type,
+			N_("manipulate just push URLs"),
+			MODIFY_TYPE_PUSH),
+		OPT_SET_INT('\0', "both", &modify_type,
+			N_("manipulate both push and fetch URLs"),
+			MODIFY_TYPE_BOTH),
+		OPT_GROUP(""),
  		OPT_BOOL('\0', "add", &add_mode,
  			 N_("add URL")),
  		OPT_BOOL('\0', "delete", &delete_mode,
@@ -1535,7 +1549,8 @@ static int set_url(int argc, const char **argv)
remotename = argv[1];
  	newurl = argv[2];
-	if (argc > 3)
+	/* The old URL is only meaningful for the primary non-set operation. */
+	if (argc > 3 && !add_mode && !delete_mode)
  		oldurl = argv[3];
if (delete_mode)
@@ -1545,47 +1560,72 @@ static int set_url(int argc, const char **argv)
  		die(_("No such remote '%s'"), remotename);
  	remote = remote_get(remotename);
- if (push_mode) {
-		strbuf_addf(&name_buf, "remote.%s.pushurl", remotename);
-		urlset = remote->pushurl;
-		urlset_nr = remote->pushurl_nr;
-	} else {
-		strbuf_addf(&name_buf, "remote.%s.url", remotename);
-		urlset = remote->url;
-		urlset_nr = remote->url_nr;
+	strbuf_addf(&name_buf_fetch, "remote.%s.url", remotename);
+	strbuf_addf(&name_buf_push, "remote.%s.pushurl", remotename);
+
+	if (oldurl && !add_mode) {
+		/* Old URL specified, or deletion. Demand that one matches. */
+		if (regcomp(&old_regex, oldurl, REG_EXTENDED))
+			die(_("Invalid old URL pattern: %s"), oldurl);
+
+		if (modify_type & MODIFY_TYPE_FETCH)
+			for (i = 0; i < remote->url_nr; i++)
+				if (!regexec(&old_regex, remote->url[i], 0, NULL, 0))
+					matches++;
+				else
+					negative_matches++;
+		if (delete_mode && !negative_matches && modify_type & MODIFY_TYPE_FETCH)
+			die(_("Will not delete all non-push URLs"));
+		if (modify_type & MODIFY_TYPE_PUSH)
+			for (i = 0; i < remote->pushurl_nr; i++)
+				if (!regexec(&old_regex, remote->pushurl[i], 0, NULL, 0))
+					matches++;
+				else
+					negative_matches++;
There are 2 warning  "dangling else", line 1570 and 1578
I think we should use:

			for (i = 0; i < remote->pushurl_nr; i++) {
				if (!regexec(&old_regex, remote->pushurl[i], 0, NULL, 0))
					matches++;
				else
					negative_matches++;
			}


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]