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

[PATCH v3 03/14] remote.c: drop "remote" pointer from "struct branch"

From
Jeff King <peff@peff.net>
Date
May 21, 2015, 04:45 UTC
Message-ID
<20150521044512.GC23409@peff.net>
In-Reply-To
<20150521044429.GA5857@peff.net>

When we create each branch struct, we fill in the "remote_name" field from the config, and then fill in the actual "remote" field (with a "struct remote") based on that name. However, it turns out that nobody really cares about the latter field. The only two sites that access it at all are:

  1. git-merge, which uses it to notice when the branch does
     not have a remote defined. But we can easily replace this
     with looking at remote_name instead.
  2. remote.c itself, when setting up the @{upstream} merge
     config. But we don't need to save the "remote" in the
     "struct branch" for that; we can just look it up for
     the duration of the operation.

So there is no need to have both fields; they are redundant with each other (the struct remote contains the name, or you can look up the struct from the name). It would be nice to simplify this, especially as we are going to add matching pushremote config in a future patch (and it would be nice to keep them consistent).

So which one do we keep and which one do we get rid of?

If we had a lot of callers accessing the struct, it would be more efficient to keep it (since you have to do a lookup to go from the name to the struct, but not vice versa). But we don't have a lot of callers; we have exactly one, so efficiency doesn't matter. We can decide this based on simplicity and readability.

And the meaning of the struct value is somewhat unclear. Is it always the remote matching remote_name? If remote_name is NULL (i.e., no per-branch config), does the struct fall back to the "origin" remote, or is it also NULL? These questions will get even more tricky with pushremotes, whose fallback behavior is more complicated. So let's just store the name, which pretty clearly represents the branch.*.remote config. Any lookup or fallback behavior can then be implemented in helper functions.

Signed-off-by: Jeff King <peff@peff.net>
---
 Documentation/technical/api-remote.txt | 4 ----
 builtin/merge.c                        | 2 +-
 remote.c                               | 7 ++++---
 remote.h                               | 1 -
 4 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/Documentation/technical/api-remote.txt b/Documentation/technical/api-remote.txt
index 5d245aa..2cfdd22 100644
--- a/Documentation/technical/api-remote.txt
+++ b/Documentation/technical/api-remote.txt
@@ -97,10 +97,6 @@ It contains:
 
 	The name of the remote listed in the configuration.
 
-`remote`::
-
-	The struct remote for that remote.
-
 `merge_name`::
 
 	An array of the "merge" lines in the configuration.
diff --git a/builtin/merge.c b/builtin/merge.c
index f89f60e..85c54dc 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -933,7 +933,7 @@ static int setup_with_upstream(const char ***argv)
 
 	if (!branch)
 		die(_("No current branch."));
-	if (!branch->remote)
+	if (!branch->remote_name)
 		die(_("No remote for the current branch."));
 	if (!branch->merge_nr)
 		die(_("No default upstream defined for the current branch."));
diff --git a/remote.c b/remote.c
index ac17e66..c298a43 100644
--- a/remote.c
+++ b/remote.c
@@ -1632,6 +1632,7 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
 
 static void set_merge(struct branch *ret)
 {
+	struct remote *remote;
 	char *ref;
 	unsigned char sha1[20];
 	int i;
@@ -1649,11 +1650,13 @@ static void set_merge(struct branch *ret)
 		return;
 	}
 
+	remote = remote_get(ret->remote_name);
+
 	ret->merge = xcalloc(ret->merge_nr, sizeof(*ret->merge));
 	for (i = 0; i < ret->merge_nr; i++) {
 		ret->merge[i] = xcalloc(1, sizeof(**ret->merge));
 		ret->merge[i]->src = xstrdup(ret->merge_name[i]);
-		if (!remote_find_tracking(ret->remote, ret->merge[i]) ||
+		if (!remote_find_tracking(remote, ret->merge[i]) ||
 		    strcmp(ret->remote_name, "."))
 			continue;
 		if (dwim_ref(ret->merge_name[i], strlen(ret->merge_name[i]),
@@ -1673,8 +1676,6 @@ struct branch *branch_get(const char *name)
 		ret = current_branch;
 	else
 		ret = make_branch(name, 0);
-	if (ret && ret->remote_name)
-		ret->remote = remote_get(ret->remote_name);
 	set_merge(ret);
 	return ret;
 }
diff --git a/remote.h b/remote.h
index 02d66ce..4bb6672 100644
--- a/remote.h
+++ b/remote.h
@@ -203,7 +203,6 @@ struct branch {
 	const char *refname;
 
 	const char *remote_name;
-	struct remote *remote;
 
 	const char **merge_name;
 	struct refspec **merge;

-- 
2.4.1.528.g00591e3
Previous: Junio C HamanoNext: Jeff King
Message 5 of 27 in “implement @{push} shorthand”
  1. 0/14 implement @{push} shorthandJeff King, May 21, 2015
  2. 01/14 remote.c: drop default_remote_name variableJeff King, May 21, 2015
  3. 02/14 remote.c: refactor setup of branch->merge listJeff King, May 21, 2015
  4. Junio C HamanoMay 21, 2015
  5. 03/14 remote.c: drop "remote" pointer from "struct branch"Jeff King, May 21, 2015
  6. 04/14 remote.c: hoist branch.*.remote lookup out of remote_get_1Jeff King, May 21, 2015
  7. 05/14 remote.c: provide per-branch pushremote nameJeff King, May 21, 2015
  8. 06/14 remote.c: hoist read_config into remote_get_1Jeff King, May 21, 2015
  9. 07/14 remote.c: introduce branch_get_upstream helperJeff King, May 21, 2015
  10. Junio C HamanoMay 21, 2015
  11. Jeff KingMay 21, 2015
  12. Jeff KingMay 21, 2015
  13. Junio C HamanoMay 21, 2015
  14. 08/14 remote.c: report specific errors from branch_get_upstreamJeff King, May 21, 2015
  15. Junio C HamanoMay 21, 2015
  16. Jeff KingMay 21, 2015
  17. Junio C HamanoMay 21, 2015
  18. Jeff KingMay 22, 2015
  19. Jeff KingMay 22, 2015
  20. 09/14 remote.c: add branch_get_pushJeff King, May 21, 2015
  21. 10/14 sha1_name: refactor upstream_markJeff King, May 21, 2015
  22. 11/14 sha1_name: refactor interpret_upstream_markJeff King, May 21, 2015
  23. 12/14 sha1_name: implement @{push} shorthandJeff King, May 21, 2015
  24. 13/14 for-each-ref: use skip_prefix instead of starts_withJeff King, May 21, 2015
  25. 14/14 for-each-ref: accept "%(push)" formatJeff King, May 21, 2015
  26. Jeff KingMay 21, 2015
  27. Junio C HamanoMay 21, 2015

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.