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 03/14] remote.c: drop "remote" pointer from "struct branch"
Date: Thu, 21 May 2015 00:45:13 -0400	[thread overview]
Message-ID: <20150521044512.GC23409@peff.net> (raw)
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

  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 ` Jeff King [this message]
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 ` [PATCH v3 05/14] remote.c: provide per-branch pushremote name Jeff King
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=20150521044512.GC23409@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).