[PATCH 4/2] remote: only update remote-tracking branch if updating refspec

From: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
Date: Sep 10, 2011, 19:39 UTC
Message-ID: <1315683563-11416-1-git-send-email-martin.von.zweigbergk@gmail.com>
In-Reply-To: <7vr53rx9r6.fsf@alter.siamese.dyndns.org>

'git remote rename' will only update the remote's fetch refspec if it looks like a default one. If the remote has no default fetch refspec, as in

[remote "origin"]
    url = git://git.kernel.org/pub/scm/git/git.git
    fetch = +refs/heads/*:refs/remotes/upstream/*

we would not update the fetch refspec and even if there is a ref called "refs/remotes/origin/master", we should not rename it, since it was not created by fetching from the remote.

Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
---

Some questions on style:

1. Should I wrap the statement in "else" block in braces when "then"
   block has braces? I couldn't find anything conclusive by looking at
   existing code.

2. Is it ok (as I did) to return from the function prematurely even
   though it is not an error case? This will of course make the
   function more brittle when it comes to adding code at the end of
   it. Would you prefer to invert the condition and put the remainder
   of the function inside the "then" block? Or even to extract the
   code into a new function?

 builtin/remote.c  |   10 +++++++---
 t/t5505-remote.sh |    3 ++-
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/builtin/remote.c b/builtin/remote.c
index 61326cb..b25dfb4 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -625,7 +625,7 @@ static int mv(int argc, const char **argv)
 		old_remote_context = STRBUF_INIT;
 	struct string_list remote_branches = STRING_LIST_INIT_NODUP;
 	struct rename_info rename;
-	int i;
+	int i, refspec_updated = 0;
 
 	if (argc != 3)
 		usage_with_options(builtin_remote_rename_usage, options);
@@ -667,12 +667,13 @@ static int mv(int argc, const char **argv)
 		strbuf_reset(&buf2);
 		strbuf_addstr(&buf2, oldremote->fetch_refspec[i]);
 		ptr = strstr(buf2.buf, old_remote_context.buf);
-		if (ptr)
+		if (ptr) {
+			refspec_updated = 1;
 			strbuf_splice(&buf2,
 				      ptr-buf2.buf + strlen(":refs/remotes/"),
 				      strlen(rename.old), rename.new,
 				      strlen(rename.new));
-		else
+		} else
 			warning("Not updating non-default fetch respec\n"
 				"\t%s\n"
 				"\tPlease update the configuration manually if necessary.",
@@ -695,6 +696,9 @@ static int mv(int argc, const char **argv)
 		}
 	}
 
+	if (!refspec_updated)
+		return 0;
+
 	/*
 	 * First remove symrefs, then rename the rest, finally create
 	 * the new symrefs.
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index 15186c8..e8af615 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -637,7 +637,8 @@ test_expect_success 'rename does not update a non-default fetch refspec' '
 	(cd four.one &&
 	 git config remote.origin.fetch +refs/heads/*:refs/heads/origin/* &&
 	 git remote rename origin upstream &&
-	 test "$(git config remote.upstream.fetch)" = "+refs/heads/*:refs/heads/origin/*")
+	 test "$(git config remote.upstream.fetch)" = "+refs/heads/*:refs/heads/origin/*" &&
+	 git rev-parse -q origin/master)
 
 '

-- 
1.7.7.rc0.317.gd0afe