git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Cc: Eric Sunshine <sunshine@sunshineco.com>,
	Junio C Hamano <gitster@pobox.com>
Subject: [PATCH v3 05/14] remote.c: provide per-branch pushremote name
Date: Thu, 21 May 2015 00:45:20 -0400	[thread overview]
Message-ID: <20150521044519.GE23409@peff.net> (raw)
In-Reply-To: <20150521044429.GA5857@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>
---
 remote.c | 40 ++++++++++++++++++++++------------------
 remote.h |  2 ++
 2 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/remote.c b/remote.c
index 0d2976b..a91d063 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.1.528.g00591e3

  parent reply	other threads:[~2015-05-21  4:45 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-21  4:44 [PATCH v3 0/14] implement @{push} shorthand Jeff King
2015-05-21  4:45 ` [PATCH v3 01/14] remote.c: drop default_remote_name variable Jeff King
2015-05-21  4:45 ` [PATCH v3 02/14] remote.c: refactor setup of branch->merge list Jeff King
2015-05-21 17:47   ` Junio C Hamano
2015-05-21  4:45 ` [PATCH v3 03/14] remote.c: drop "remote" pointer from "struct branch" Jeff King
2015-05-21  4:45 ` [PATCH v3 04/14] remote.c: hoist branch.*.remote lookup out of remote_get_1 Jeff King
2015-05-21  4:45 ` Jeff King [this message]
2015-05-21  4:45 ` [PATCH v3 06/14] remote.c: hoist read_config into remote_get_1 Jeff King
2015-05-21  4:45 ` [PATCH v3 07/14] remote.c: introduce branch_get_upstream helper Jeff King
2015-05-21 18:07   ` Junio C Hamano
2015-05-21 18:14     ` Jeff King
2015-05-21 18:35       ` Jeff King
2015-05-21 19:16         ` Junio C Hamano
2015-05-21  4:45 ` [PATCH v3 08/14] remote.c: report specific errors from branch_get_upstream Jeff King
2015-05-21 18:33   ` Junio C Hamano
2015-05-21 18:49     ` Jeff King
2015-05-21 19:25       ` Junio C Hamano
2015-05-22  0:46         ` Jeff King
2015-05-22  0:49           ` Jeff King
2015-05-21  4:45 ` [PATCH v3 09/14] remote.c: add branch_get_push Jeff King
2015-05-21  4:45 ` [PATCH v3 10/14] sha1_name: refactor upstream_mark Jeff King
2015-05-21  4:45 ` [PATCH v3 11/14] sha1_name: refactor interpret_upstream_mark Jeff King
2015-05-21  4:45 ` [PATCH v3 12/14] sha1_name: implement @{push} shorthand Jeff King
2015-05-21  4:45 ` [PATCH v3 13/14] for-each-ref: use skip_prefix instead of starts_with Jeff King
2015-05-21  4:45 ` [PATCH v3 14/14] for-each-ref: accept "%(push)" format Jeff King
2015-05-21  4:52 ` [PATCH v3 0/14] implement @{push} shorthand Jeff King
2015-05-21 18:37   ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150521044519.GE23409@peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).