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

[PATCH 04/12] remote.c: provide per-branch pushremote name

From
Jeff King <peff@peff.net>
Date
May 1, 2015, 22:46 UTC
Message-ID
<20150501224644.GD1534@peff.net>
In-Reply-To
<20150501224414.GA25551@peff.net>

When remote.c loads its config, it records the branch.*.pushremote for the current branch along with the global remote.pushDefault value, and then binds them into a single value: the default push for the current branch. We then pass this value (which may be NULL) to remote_get_1 when looking up a remote for push.

This has a few downsides:
  1. It's confusing. The early-binding of the "current
     value" led to bugs like the one fixed by 98b406f
     (remote: handle pushremote config in any order,
     2014-02-24). And the fact that pushremotes fall back to
     ordinary remotes is not explicit at all; it happens
     because remote_get_1 cannot tell the difference between
     "we are not asking for the push remote" and "there is
     no push remote configured".
  2. It throws away intermediate data. After read_config()
     finishes, we have no idea what the value of
     remote.pushDefault was, because the string has been
     overwritten by the current branch's
     branch.*.pushremote.
  3. It doesn't record other data. We don't note the
     branch.*.pushremote value for anything but the current
     branch.

Let's make this more like the fetch-remote config. We'll record the pushremote for each branch, and then explicitly compute the correct remote for the current branch at the time of reading.

Signed-off-by: Jeff King <peff@peff.net>
---
Versus v1, I did something a little clever by passing a function pointer
around (versus a flag and letting the caller do a conditional based on
the flag). Too clever?
 remote.c | 40 ++++++++++++++++++++++------------------
 remote.h |  2 ++
 2 files changed, 24 insertions(+), 18 deletions(-)
diff --git a/remote.c b/remote.c
index a27f795..9f84ea3 100644
--- a/remote.c
+++ b/remote.c
@@ -49,7 +49,6 @@ static int branches_alloc;
 static int branches_nr;
 
 static struct branch *current_branch;
-static const char *branch_pushremote_name;
 static const char *pushremote_name;
 
 static struct rewrites rewrites;
@@ -367,9 +366,7 @@ static int handle_config(const char *key, const char *value, void *cb)
 		if (!strcmp(subkey, ".remote")) {
 			return git_config_string(&branch->remote_name, key, value);
 		} else if (!strcmp(subkey, ".pushremote")) {
-			if (branch == current_branch)
-				if (git_config_string(&branch_pushremote_name, key, value))
-					return -1;
+			return git_config_string(&branch->pushremote_name, key, value);
 		} else if (!strcmp(subkey, ".merge")) {
 			if (!value)
 				return config_error_nonbool(key);
@@ -510,10 +507,6 @@ static void read_config(void)
 		current_branch = make_branch(head_ref, 0);
 	}
 	git_config(handle_config, NULL);
-	if (branch_pushremote_name) {
-		free((char *)pushremote_name);
-		pushremote_name = branch_pushremote_name;
-	}
 	alias_all_urls();
 }
 
@@ -704,20 +697,31 @@ const char *remote_for_branch(struct branch *branch, int *explicit)
 	return "origin";
 }
 
-static struct remote *remote_get_1(const char *name, const char *pushremote_name)
+const char *pushremote_for_branch(struct branch *branch, int *explicit)
+{
+	if (branch && branch->pushremote_name) {
+		if (explicit)
+			*explicit = 1;
+		return branch->pushremote_name;
+	}
+	if (pushremote_name) {
+		if (explicit)
+			*explicit = 1;
+		return pushremote_name;
+	}
+	return remote_for_branch(branch, explicit);
+}
+
+static struct remote *remote_get_1(const char *name,
+				   const char *(*get_default)(struct branch *, int *))
 {
 	struct remote *ret;
 	int name_given = 0;
 
 	if (name)
 		name_given = 1;
-	else {
-		if (pushremote_name) {
-			name = pushremote_name;
-			name_given = 1;
-		} else
-			name = remote_for_branch(current_branch, &name_given);
-	}
+	else
+		name = get_default(current_branch, &name_given);
 
 	ret = make_remote(name, 0);
 	if (valid_remote_nick(name)) {
@@ -738,13 +742,13 @@ static struct remote *remote_get_1(const char *name, const char *pushremote_name
 struct remote *remote_get(const char *name)
 {
 	read_config();
-	return remote_get_1(name, NULL);
+	return remote_get_1(name, remote_for_branch);
 }
 
 struct remote *pushremote_get(const char *name)
 {
 	read_config();
-	return remote_get_1(name, pushremote_name);
+	return remote_get_1(name, pushremote_for_branch);
 }
 
 int remote_is_configured(const char *name)
diff --git a/remote.h b/remote.h
index 2a7e7a6..30a11da 100644
--- a/remote.h
+++ b/remote.h
@@ -203,6 +203,7 @@ struct branch {
 	const char *refname;
 
 	const char *remote_name;
+	const char *pushremote_name;
 
 	const char **merge_name;
 	struct refspec **merge;
@@ -212,6 +213,7 @@ struct branch {
 
 struct branch *branch_get(const char *name);
 const char *remote_for_branch(struct branch *branch, int *explicit);
+const char *pushremote_for_branch(struct branch *branch, int *explicit);
 
 int branch_has_merge_config(struct branch *branch);
 int branch_merge_matches(struct branch *, int n, const char *);
-- 
2.4.0.rc3.477.gc25258d
Previous: Jeff KingNext: Eric Sunshine
Message 8 of 21 in “implement @{push} shorthand”
  1. 0/12 implement @{push} shorthandJeff King, May 1, 2015
  2. 01/12 remote.c: drop default_remote_name variableJeff King, May 1, 2015
  3. 02/12 remote.c: drop "remote" pointer from "struct branch"Jeff King, May 1, 2015
  4. Eric SunshineMay 3, 2015
  5. Jeff KingMay 5, 2015
  6. Jeff KingMay 7, 2015
  7. 03/12 remote.c: hoist branch.*.remote lookup out of remote_get_1Jeff King, May 1, 2015
  8. 04/12 remote.c: provide per-branch pushremote nameJeff King, May 1, 2015
  9. Eric SunshineMay 3, 2015
  10. Jeff KingMay 5, 2015
  11. Eric SunshineMay 5, 2015
  12. Jeff KingMay 7, 2015
  13. Eric SunshineMay 8, 2015
  14. 05/12 remote.c: introduce branch_get_upstream helperJeff King, May 1, 2015
  15. 06/12 remote.c: report specific errors from branch_get_upstreamJeff King, May 1, 2015
  16. 07/12 remote.c: add branch_get_pushJeff King, May 1, 2015
  17. 08/12 sha1_name: refactor upstream_markJeff King, May 1, 2015
  18. 09/12 sha1_name: refactor interpret_upstream_markJeff King, May 1, 2015
  19. 10/12 sha1_name: implement @{push} shorthandJeff King, May 1, 2015
  20. 11/12 for-each-ref: use skip_prefix instead of starts_withJeff King, May 1, 2015
  21. 12/12 for-each-ref: accept "%(push)" formatJeff King, May 1, 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.