git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 22/23] remote: refactor guess_remote_head

From
Jay Soffian <jaysoffian@gmail.com>
Date
Feb 24, 2009, 09:51 UTC
Message-ID
<9fcb7dbd8ec6fbed4701098a2fecc5728521dbc7.1235467368.git.jaysoffian@gmail.com>
In-Reply-To
<cover.1235467368.git.jaysoffian@gmail.com>
From: Jeff King <peff@peff.net>

This function had a lot of complications which made it hard to read and extend, and confusing to call (because of optional unused parameters). This patch attempts to address that:

  - we used to manually search through the ref lists; this
    should be a one-line call to find_ref_by_name
  - guess_remote_head used to do two things: find the HEAD
    ref, and then find a matching ref. And only one of the
    two callers actually cared about returning the HEAD ref.
    Since it's a one-liner, just have the caller do it
    themselves (and remember it or not as they wish).
  - there were two ways of getting results out of the
    function (a return value, and an "all_matched" out
    parameter), but no caller cared about both. One of them
    returned a pointer into the passed-in ref list and one
    of them returned a newly allocated list (with the
    peer_ref fields stripped). Let's be simple and
    consistent: the return value is always a ref copy with
    a valid peer_ref. If "all" is requested, all candidates
    are returned. Otherwise, the best candidate is returned.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
 builtin-clone.c  |    5 +--
 builtin-remote.c |    5 ++-
 remote.c         |   62 ++++++++++++++++++++++-------------------------------
 remote.h         |   17 ++++++--------
 4 files changed, 38 insertions(+), 51 deletions(-)
diff --git a/builtin-clone.c b/builtin-clone.c
index d57818c..7130cab 100644
--- a/builtin-clone.c
+++ b/builtin-clone.c
@@ -508,9 +508,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 		clear_extra_refs();
 
 		mapped_refs = write_remote_refs(refs, &refspec, reflog_msg.buf);
-
-		head_points_at = guess_remote_head(refs, mapped_refs,
-						   &remote_head, NULL);
+		remote_head = find_ref_by_name(refs, "HEAD");
+		head_points_at = guess_remote_head(remote_head, mapped_refs, 0);
 	}
 	else {
 		warning("You appear to have cloned an empty repository.");
diff --git a/builtin-remote.c b/builtin-remote.c
index 80ab7af..d5a575f 100644
--- a/builtin-remote.c
+++ b/builtin-remote.c
@@ -385,7 +385,7 @@ static int get_push_ref_states_noquery(struct ref_states *states)
 static int get_head_names(const struct ref *remote_refs,
 	const char *remote_name, struct ref_states *states)
 {
-	struct ref *ref, *matches = NULL;
+	struct ref *ref, *matches;
 	struct ref *fetch_map = NULL, **fetch_map_tail = &fetch_map;
 	struct refspec refspec;
 
@@ -394,7 +394,8 @@ static int get_head_names(const struct ref *remote_refs,
 	refspec.src = refspec.dst = "refs/heads/";
 	states->heads.strdup_strings = 1;
 	get_fetch_map(remote_refs, &refspec, &fetch_map_tail, 0);
-	guess_remote_head(remote_refs, fetch_map, NULL, &matches);
+	matches = guess_remote_head(find_ref_by_name(remote_refs, "HEAD"),
+					fetch_map, 1);
 	for(ref = matches; ref; ref = ref->next)
 		string_list_append(abbrev_branch(ref->name), &states->heads);
 
diff --git a/remote.c b/remote.c
index f07ecf4..96d5c95 100644
--- a/remote.c
+++ b/remote.c
@@ -1431,53 +1431,43 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb)
 	return 1;
 }
 
-const struct ref *guess_remote_head(const struct ref *refs,
-				    const struct ref *mapped_refs,
-				    const struct ref **remote_head_p,
-				    struct ref **all_matches_p)
+struct ref *copy_ref_with_peer(const struct ref *src)
 {
-	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;
-
-	if (!all_matches_p)
-		for (r = mapped_refs; r; r = r->next)
-			if (!strcmp(r->name, "refs/heads/master"))
-				remote_master = r;
+	struct ref *dst = copy_ref(src);
+	dst->peer_ref = copy_ref(src->peer_ref);
+	return dst;
+}
 
-	if (remote_head_p)
-		*remote_head_p = remote_head;
+struct ref *guess_remote_head(const struct ref *head,
+			      const struct ref *refs,
+			      int all)
+{
+	const struct ref *r;
+	struct ref *list = NULL;
+	struct ref **tail = &list;
 
-	/* If there's no HEAD value at all, never mind. */
-	if (!remote_head)
+	if (!head)
 		return NULL;
 
 	/* If refs/heads/master could be right, it is. */
-	if (remote_master && !hashcmp(remote_master->old_sha1,
-				      remote_head->old_sha1))
-		return remote_master;
+	if (!all) {
+		const struct ref *m;
+		m = find_ref_by_name(refs, "refs/heads/master");
+		if (m && !hashcmp(m->old_sha1, head->old_sha1))
+			return copy_ref_with_peer(m);
+	}
 
 	/* 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)) {
-			struct ref *cpy;
-			if (!ret)
-				ret = r;
-			if (!all_matches_p)
+	for (r = refs; r; r = r->next) {
+		if (r != head && !hashcmp(r->old_sha1, head->old_sha1)) {
+			*tail = copy_ref_with_peer(r);
+			tail = &((*tail)->next);
+			if (!all)
 				break;
-			*tail = cpy = copy_ref(r);
-			cpy->peer_ref = NULL;
-			tail = &cpy->next;
 		}
+	}
 
-	return ret;
+	return list;
 }
 
 static int one_local_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
diff --git a/remote.h b/remote.h
index 1f78a0a..903712f 100644
--- a/remote.h
+++ b/remote.h
@@ -139,17 +139,14 @@ int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs);
 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.
+ * Find refs from a list which are likely to be pointed to by the given HEAD
+ * ref. If 'all' is false, returns the most likely ref; otherwise, returns a
+ * list of all candidate refs. If no match is found (or 'head' is NULL),
+ * returns NULL. All returns are newly allocated and should be freed.
  */
-const struct ref *guess_remote_head(const struct ref *refs,
-				    const struct ref *mapped_refs,
-				    const struct ref **remote_head_p,
-				    struct ref **all_matches_p);
+struct ref *guess_remote_head(const struct ref *head,
+			      const struct ref *refs,
+			      int all);
 
 struct ref *get_local_heads(void);
 #endif

-- 
1.6.2.rc1.291.g83eb
Previous: Jay SoffianNext: Jay Soffian
Message 22 of 23 in “builtin-clone: move locate_head() to remote.c so it can be re-used”
  1. 01/23 builtin-clone: move locate_head() to remote.c so it can be re-usedJay Soffian, Feb 24, 2009
  2. 02/23 builtin-remote: move duplicated cleanup code its own functionJay Soffian, Feb 24, 2009
  3. 03/23 builtin-remote: teach show to display remote HEADJay Soffian, Feb 24, 2009
  4. 04/23 builtin-remote: add set-head subcommandJay Soffian, Feb 24, 2009
  5. 05/23 builtin-remote: better handling of multiple remote HEADsJay Soffian, Feb 24, 2009
  6. 06/23 remote.c: make match_refs() copy src ref before assigning to peer_refJay Soffian, Feb 24, 2009
  7. 07/23 remote.c: don't short-circuit match_refs() when error in match_explicit_refs()Jay Soffian, Feb 24, 2009
  8. 08/23 refactor duplicated get_local_heads() to remote.cJay Soffian, Feb 24, 2009
  9. 09/23 refactor duplicated ref_newer() to remote.cJay Soffian, Feb 24, 2009
  10. 10/23 string-list: new for_each_string_list() functionJay Soffian, Feb 24, 2009
  11. 11/23 builtin-remote: rename variable and eliminate redundant function callJay Soffian, Feb 24, 2009
  12. 12/23 builtin-remote: name remote_refs consistentlyJay Soffian, Feb 24, 2009
  13. 13/23 builtin-remote: remove unused code in get_ref_statesJay Soffian, Feb 24, 2009
  14. 14/23 builtin-remote: fix two inconsistencies in the output of "show <remote>"Jay Soffian, Feb 24, 2009
  15. 15/23 builtin-remote: make get_remote_ref_states() always populate states.trackedJay Soffian, Feb 24, 2009
  16. 16/23 builtin-remote: refactor get_remote_ref_states()Jay Soffian, Feb 24, 2009
  17. 17/23 builtin-remote: new show output styleJay Soffian, Feb 24, 2009
  18. 18/23 builtin-remote: new show output style for push refspecsJay Soffian, Feb 24, 2009
  19. 19/23 test scripts: refactor start_httpd helperJay Soffian, Feb 24, 2009
  20. 20/23 add basic http clone/fetch testsJay Soffian, Feb 24, 2009
  21. 21/23 refactor find_refs_by_name to accept const listJay Soffian, Feb 24, 2009
  22. 22/23 remote: refactor guess_remote_headJay Soffian, Feb 24, 2009
  23. 23/23 remote: use exact HEAD lookup if it is availableJay Soffian, Feb 24, 2009

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.