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

[Jeff King <peff@peff.net>]

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))