Thread (1 message) 1 message, 1 author, 2016-06-15
DORMANTno replies

[PATCH 05/23] builtin-remote: better handling of multiple remote HEADs

From: Jay Soffian <hidden>
Date: 2016-06-15 22:46:16
Subsystem: the rest · Maintainer: Linus Torvalds

It is not currently possible to determine the remote HEAD unambiguously
when multiple remote branches share the same SHA1 as the remote HEAD.

In this situation, git remote set-head --auto should not try to guess
which HEAD the user wants. This patch causes set-head to provide a
useful error instead:

$ git remote set-head origin --auto
error: Multiple remote HEAD branches. Please choose one explicitly with:
  git remote set-head origin another
  git remote set-head origin master

Also, the output of git remote show now shows the multiple HEADs:

$ git remote show origin
* remote origin
  URL: ...
  HEAD branches:
    another
    master

Signed-off-by: Jay Soffian <redacted>
---
 builtin-clone.c   |    2 +-
 builtin-remote.c  |   56 +++++++++++++++++++++++++++++++++++------------------
 remote.c          |   28 ++++++++++++++++++-------
 remote.h          |    6 ++++-
 t/t5505-remote.sh |   51 ++++++++++++++++++++++++++++++++++++++++++++++-
 5 files changed, 112 insertions(+), 31 deletions(-)
diff --git a/builtin-clone.c b/builtin-clone.c
index d179d1c..d57818c 100644
--- a/builtin-clone.c
+++ b/builtin-clone.c
@@ -510,7 +510,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 		mapped_refs = write_remote_refs(refs, &refspec, reflog_msg.buf);
 
 		head_points_at = guess_remote_head(refs, mapped_refs,
-						   &remote_head);
+						   &remote_head, NULL);
 	}
 	else {
 		warning("You appear to have cloned an empty repository.");
diff --git a/builtin-remote.c b/builtin-remote.c
index fcb166b..608c0f3 100644
--- a/builtin-remote.c
+++ b/builtin-remote.c
@@ -212,8 +212,7 @@ static void read_branches(void)
 
 struct ref_states {
 	struct remote *remote;
-	struct string_list new, stale, tracked;
-	char *head_name;
+	struct string_list new, stale, tracked, heads;
 };
 
 static int handle_one_branch(const char *refname,
@@ -273,24 +272,26 @@ static int get_ref_states(const struct ref *ref, struct ref_states *states)
 	return 0;
 }
 
-static char *get_head_name(const struct ref *refs)
+static int get_head_names(const struct ref *refs,
+	const char *remote_name, struct ref_states *states)
 {
-	const struct ref *head_points_at;
-	struct ref *mapped_refs = NULL;
-	struct ref **tail = &mapped_refs;
+	struct ref *ref, *matches = NULL;
+	struct ref *fetch_map = NULL, **fetch_map_tail = &fetch_map;
 	struct refspec refspec;
 
 	refspec.force = 0;
 	refspec.pattern = 1;
 	refspec.src = refspec.dst = "refs/heads/";
+	states->heads.strdup_strings = 1;
+	get_fetch_map(refs, &refspec, &fetch_map_tail, 0);
+	guess_remote_head(refs, fetch_map, NULL, &matches);
+	for(ref = matches; ref; ref = ref->next)
+		string_list_append(abbrev_branch(ref->name), &states->heads);
 
-	get_fetch_map(refs, &refspec, &tail, 0);
-
-	head_points_at = guess_remote_head(refs, mapped_refs, NULL);
-	if (head_points_at)
-		return xstrdup(abbrev_branch(head_points_at->name));
+	free_refs(fetch_map);
+	free_refs(matches);
 
-	return NULL;
+	return 0;
 }
 
 struct known_remote {
@@ -659,7 +660,7 @@ static void free_remote_ref_states(struct ref_states *states)
 	string_list_clear(&states->new, 0);
 	string_list_clear(&states->stale, 0);
 	string_list_clear(&states->tracked, 0);
-	free(states->head_name);
+	string_list_clear(&states->heads, 0);
 }
 
 static int get_remote_ref_states(const char *name,
@@ -681,7 +682,7 @@ static int get_remote_ref_states(const char *name,
 		ref = transport_get_remote_refs(transport);
 		transport_disconnect(transport);
 
-		states->head_name = get_head_name(ref);
+		get_head_names(ref, name, states);
 		get_ref_states(ref, states);
 	}
 
@@ -726,9 +727,15 @@ static int show(int argc, const char **argv)
 		printf("* remote %s\n  URL: %s\n", *argv,
 			states.remote->url_nr > 0 ?
 				states.remote->url[0] : "(no URL)");
-		if (!no_query)
-			printf("  HEAD: %s\n", states.head_name ?
-				states.head_name : "(unknown)");
+		if (no_query)
+			printf("  HEAD branch: (not queried)\n");
+		else if (!states.heads.nr)
+			printf("  HEAD branch: (unknown)\n");
+		else if (states.heads.nr == 1)
+			printf("  HEAD branch: %s\n",
+				states.heads.items[0].string);
+		else
+			show_list("  HEAD branch%s:", &states.heads, "");
 
 		for (i = 0; i < branch_list.nr; i++) {
 			struct string_list_item *branch = branch_list.items + i;
@@ -780,7 +787,7 @@ static int show(int argc, const char **argv)
 
 static int set_head(int argc, const char **argv)
 {
-	int opt_a = 0, opt_d = 0, result = 0;
+	int i, opt_a = 0, opt_d = 0, result = 0;
 	struct strbuf buf = STRBUF_INIT, buf2 = STRBUF_INIT;
 	char *head_name = NULL;
 
@@ -802,7 +809,16 @@ static int set_head(int argc, const char **argv)
 		struct ref_states states;
 		memset(&states, 0, sizeof(states));
 		get_remote_ref_states(argv[0], &states, 1);
-		head_name = xstrdup(states.head_name);
+		if (!states.heads.nr)
+			result |= error("Cannot determine remote HEAD");
+		else if (states.heads.nr > 1) {
+			result |= error("Multiple remote HEAD branches. "
+					"Please choose one explicitly with:");
+			for (i = 0; i < states.heads.nr; i++)
+				fprintf(stderr, "  git remote set-head %s %s\n",
+					argv[0], states.heads.items[i].string);
+		} else
+			head_name = xstrdup(states.heads.items[0].string);
 		free_remote_ref_states(&states);
 	} else if (opt_d && !opt_a && argc == 1) {
 		if (delete_ref(buf.buf, NULL, REF_NODEREF))
@@ -818,6 +834,8 @@ static int set_head(int argc, const char **argv)
 			result |= error("Not a valid ref: %s", buf2.buf);
 		else if (create_symref(buf.buf, buf2.buf, "remote set-head"))
 			result |= error("Could not setup %s", buf.buf);
+		if (opt_a)
+			printf("%s/HEAD set to %s\n", argv[0], head_name);
 		free(head_name);
 	}
 
diff --git a/remote.c b/remote.c
index 447f091..6385a22 100644
--- a/remote.c
+++ b/remote.c
@@ -1379,18 +1379,23 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb)
 
 const struct ref *guess_remote_head(const struct ref *refs,
 				    const struct ref *mapped_refs,
-				    const struct ref **remote_head_p)
+				    const struct ref **remote_head_p,
+				    struct ref **all_matches_p)
 {
 	const struct ref *remote_head = NULL;
 	const struct ref *remote_master = NULL;
+	const struct ref *ret = NULL;
 	const struct ref *r;
+	struct ref **tail = all_matches_p;
+
 	for (r = refs; r; r = r->next)
 		if (!strcmp(r->name, "HEAD"))
 			remote_head = r;
 
-	for (r = mapped_refs; r; r = r->next)
-		if (!strcmp(r->name, "refs/heads/master"))
-			remote_master = r;
+	if (!all_matches_p)
+		for (r = mapped_refs; r; r = r->next)
+			if (!strcmp(r->name, "refs/heads/master"))
+				remote_master = r;
 
 	if (remote_head_p)
 		*remote_head_p = remote_head;
@@ -1407,9 +1412,16 @@ const struct ref *guess_remote_head(const struct ref *refs,
 	/* Look for another ref that points there */
 	for (r = mapped_refs; r; r = r->next)
 		if (r != remote_head &&
-		    !hashcmp(r->old_sha1, remote_head->old_sha1))
-			return r;
+		    !hashcmp(r->old_sha1, remote_head->old_sha1)) {
+			struct ref *cpy;
+			if (!ret)
+				ret = r;
+			if (!all_matches_p)
+				break;
+			*tail = cpy = copy_ref(r);
+			cpy->peer_ref = NULL;
+			tail = &cpy->next;
+		}
 
-	/* Nothing is the same */
-	return NULL;
+	return ret;
 }
diff --git a/remote.h b/remote.h
index cabb14a..8409d42 100644
--- a/remote.h
+++ b/remote.h
@@ -141,9 +141,13 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb);
  * Look in refs for HEAD. Then look for a matching SHA1 in mapped_refs,
  * first checking if refs/heads/master matches. Return NULL if nothing matches
  * or if there is no HEAD in refs. remote_head_p is assigned HEAD if not NULL.
+ * If all_matches_p is NULL, return after the first possible match. Otherwise
+ * all_matches_p is set to a ref list of each branch head with the same SHA1 as
+ * HEAD.
  */
 const struct ref *guess_remote_head(const struct ref *refs,
 				    const struct ref *mapped_refs,
-				    const struct ref **remote_head_p);
+				    const struct ref **remote_head_p,
+				    struct ref **all_matches_p);
 
 #endif
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index 826d0c3..9ea923b 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -136,7 +136,7 @@ EOF
 cat > test/expect << EOF
 * remote origin
   URL: $(pwd)/one
-  HEAD: master
+  HEAD branch: master
   Remote branch merged with 'git pull' while on branch master
     master
   New remote branch (next fetch will store in remotes/origin)
@@ -147,6 +147,11 @@ cat > test/expect << EOF
   Local branches pushed with 'git push'
     master:upstream
     +refs/tags/lastbackup
+* remote two
+  URL: ../two
+  HEAD branches:
+    another
+    master
 EOF
 
 test_expect_success 'show' '
@@ -155,6 +160,7 @@ test_expect_success 'show' '
 		refs/heads/master:refs/heads/upstream &&
 	 git fetch &&
 	 git branch -d -r origin/master &&
+	 git config --add remote.two.url ../two &&
 	 (cd ../one &&
 	  echo 1 > file &&
 	  test_tick &&
@@ -163,13 +169,14 @@ test_expect_success 'show' '
 		refs/heads/master:refs/heads/upstream &&
 	 git config --add remote.origin.push \
 		+refs/tags/lastbackup &&
-	 git remote show origin > output &&
+	 git remote show origin two > output &&
 	 test_cmp expect output)
 '
 
 cat > test/expect << EOF
 * remote origin
   URL: $(pwd)/one
+  HEAD branch: (not queried)
   Remote branch merged with 'git pull' while on branch master
     master
   Tracked remote branches
@@ -198,6 +205,46 @@ test_expect_success 'prune' '
 	 test_must_fail git rev-parse refs/remotes/origin/side)
 '
 
+test_expect_success 'set-head --delete' '
+	(cd test &&
+	 git symbolic-ref refs/remotes/origin/HEAD &&
+	 git remote set-head --delete origin &&
+	 test_must_fail git symbolic-ref refs/remotes/origin/HEAD)
+'
+
+test_expect_success 'set-head --auto' '
+	(cd test &&
+	 git remote set-head --auto origin &&
+	 echo refs/remotes/origin/master >expect &&
+	 git symbolic-ref refs/remotes/origin/HEAD >output &&
+	 test_cmp expect output
+	)
+'
+
+cat >test/expect <<EOF
+error: Multiple remote HEAD branches. Please choose one explicitly with:
+  git remote set-head two another
+  git remote set-head two master
+EOF
+
+test_expect_success 'set-head --auto fails w/multiple HEADs' '
+	(cd test &&
+	 test_must_fail git remote set-head --auto two >output 2>&1 &&
+	test_cmp expect output)
+'
+
+cat >test/expect <<EOF
+refs/remotes/origin/side2
+EOF
+
+test_expect_success 'set-head explicit' '
+	(cd test &&
+	 git remote set-head origin side2 &&
+	 git symbolic-ref refs/remotes/origin/HEAD >output &&
+	 git remote set-head origin master &&
+	 test_cmp expect output)
+'
+
 cat > test/expect << EOF
 Pruning origin
 URL: $(pwd)/one
-- 
1.6.2.rc1.291.g83eb
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help