Re: [PATCH] builtin-remote: better handling of multiple remote HEADs

On Sat, Feb 14, 2009 at 03:21:30PM -0500, Daniel Barkalow wrote:

I haven't checked lately, but I think that what's actually needed is to 
have the locate_head() function notice if the struct ref for HEAD actually 
has the symref field non-NULL, and report that as the unambiguous answer. 

Indeed. Something like the patch below works (on top of Jay's patches).
But it has two shortcomings:

  1. There is no test script, since we have no infrastructure for
     testing over http. I might be able to build something on top of
     what's in the http-push tests.

     I was hoping we could do the same trick for local file repos, which
     would be easy to test. But the transport code just treats them as
     regular pack uploaders; only some specialized code in clone cares
     about the difference.

     In theory we could add a new transport for local repos. I don't
     think it would make sense for its get_remote_refs function to get
     _all_ of the refs, but it could specially peek at HEAD and set the
     symref field appropriately.

  2. The guess_remote_head function is getting a little long. I think it
     would help to refactor it into two functions; one for finding the
     remote HEAD in the refs list, and the other for guessing at a ref
     which matches the HEAD.

I will try to make something a little neater later today.

This should also allow it to automatically pick up any other 
disambiguation by future sources of lists of refs that include HEAD, 
whether that's git protocol extensions, filesystem access to the repo, or 
foreign VCSes where some branches is inherently primary, or whatever.

Yes, I think the symref field for the ref is a very sensible way of
communicating the information for that reason.

Patch is below.

---

diff --git a/remote.c b/remote.c
index 6385a22..afbaccc 100644
--- a/remote.c
+++ b/remote.c

@@ -1404,6 +1404,20 @@ const struct ref *guess_remote_head(const struct ref *refs,
 	if (!remote_head)
 		return NULL;
 
+	/* if the underlying transport can represent symrefs,
+	 * then we don't need to guess at all */
+	if (remote_head->symref) {
+		for (r = mapped_refs; r; r = r->next) {
+			if (!strcmp(r->name, remote_head->symref)) {
+				if (all_matches_p) {
+					*all_matches_p = copy_ref(r);
+					(*all_matches_p)->peer_ref = NULL;
+				}
+				return r;
+			}
+		}
+	}
+
 	/* If refs/heads/master could be right, it is. */
 	if (remote_master && !hashcmp(remote_master->old_sha1,
 				      remote_head->old_sha1))