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

[PATCH v3 2/4] remote: use remote_state parameter internally

From
Glen Choo <chooglen@google.com>
Date
Oct 19, 2021, 22:43 UTC
Message-ID
<20211019224339.61881-3-chooglen@google.com>
In-Reply-To
<20211019224339.61881-1-chooglen@google.com>

Introduce a struct remote_state member to structs that need to 'remember' their remote_state. Without changing external-facing functions, replace the_repository->remote_state internally by using the remote_state member where it is applicable i.e. when a function accepts a struct that depends on the remote_state. If it is not applicable, add a struct remote_state parameter instead.

As a result, external-facing functions are still tied to the_repository, but most static functions no longer reference the_repository->remote_state. The exceptions are those that are used in a way that depends on external-facing functions e.g. the callbacks to remote_get_1().

Signed-off-by: Glen Choo <chooglen@google.com>
---
 remote.c | 153 ++++++++++++++++++++++++++-----------------------------
 remote.h |  10 ++++
 2 files changed, 81 insertions(+), 82 deletions(-)
diff --git a/remote.c b/remote.c
index 29c29fcc3b..e3ca44f735 100644
--- a/remote.c
+++ b/remote.c
@@ -68,15 +68,14 @@ static void add_pushurl(struct remote *remote, const char *pushurl)
 static void add_pushurl_alias(struct remote *remote, const char *url)
 {
 	const char *pushurl =
-		alias_url(url, &the_repository->remote_state->rewrites_push);
+		alias_url(url, &remote->remote_state->rewrites_push);
 	if (pushurl != url)
 		add_pushurl(remote, pushurl);
 }
 
 static void add_url_alias(struct remote *remote, const char *url)
 {
-	add_url(remote,
-		alias_url(url, &the_repository->remote_state->rewrites));
+	add_url(remote, alias_url(url, &remote->remote_state->rewrites));
 	add_pushurl_alias(remote, url);
 }
 
@@ -102,14 +101,19 @@ static int remotes_hash_cmp(const void *unused_cmp_data,
 		return strcmp(a->name, b->name);
 }
 
-static inline void init_remotes_hash(void)
+/**
+ * NEEDSWORK: Now that the hashmap is in a struct, this should probably
+ * just be moved into remote_state_new().
+ */
+static inline void init_remotes_hash(struct remote_state *remote_state)
 {
-	if (!the_repository->remote_state->remotes_hash.cmpfn)
-		hashmap_init(&the_repository->remote_state->remotes_hash,
-			     remotes_hash_cmp, NULL, 0);
+	if (!remote_state->remotes_hash.cmpfn)
+		hashmap_init(&remote_state->remotes_hash, remotes_hash_cmp,
+			     NULL, 0);
 }
 
-static struct remote *make_remote(const char *name, int len)
+static struct remote *make_remote(struct remote_state *remote_state,
+				  const char *name, int len)
 {
 	struct remote *ret;
 	struct remotes_hash_key lookup;
@@ -118,13 +122,12 @@ static struct remote *make_remote(const char *name, int len)
 	if (!len)
 		len = strlen(name);
 
-	init_remotes_hash();
+	init_remotes_hash(remote_state);
 	lookup.str = name;
 	lookup.len = len;
 	hashmap_entry_init(&lookup_entry, memhash(name, len));
 
-	e = hashmap_get(&the_repository->remote_state->remotes_hash,
-			&lookup_entry, &lookup);
+	e = hashmap_get(&remote_state->remotes_hash, &lookup_entry, &lookup);
 	if (e)
 		return container_of(e, struct remote, ent);
 
@@ -132,18 +135,16 @@ static struct remote *make_remote(const char *name, int len)
 	ret->prune = -1;  /* unspecified */
 	ret->prune_tags = -1;  /* unspecified */
 	ret->name = xstrndup(name, len);
+	ret->remote_state = remote_state;
 	refspec_init(&ret->push, REFSPEC_PUSH);
 	refspec_init(&ret->fetch, REFSPEC_FETCH);
 
-	ALLOC_GROW(the_repository->remote_state->remotes,
-		   the_repository->remote_state->remotes_nr + 1,
-		   the_repository->remote_state->remotes_alloc);
-	the_repository->remote_state
-		->remotes[the_repository->remote_state->remotes_nr++] = ret;
+	ALLOC_GROW(remote_state->remotes, remote_state->remotes_nr + 1,
+		   remote_state->remotes_alloc);
+	remote_state->remotes[remote_state->remotes_nr++] = ret;
 
 	hashmap_entry_init(&ret->ent, lookup_entry.hash);
-	if (hashmap_put_entry(&the_repository->remote_state->remotes_hash, ret,
-			      ent))
+	if (hashmap_put_entry(&remote_state->remotes_hash, ret, ent))
 		BUG("hashmap_put overwrote entry after hashmap_get returned NULL");
 	return ret;
 }
@@ -177,27 +178,25 @@ static void add_merge(struct branch *branch, const char *name)
 	branch->merge_name[branch->merge_nr++] = name;
 }
 
-static struct branch *make_branch(const char *name, size_t len)
+static struct branch *make_branch(struct remote_state *remote_state,
+				  const char *name, size_t len)
 {
 	struct branch *ret;
 	int i;
 
-	for (i = 0; i < the_repository->remote_state->branches_nr; i++) {
-		if (!strncmp(name,
-			     the_repository->remote_state->branches[i]->name,
-			     len) &&
-		    !the_repository->remote_state->branches[i]->name[len])
-			return the_repository->remote_state->branches[i];
+	for (i = 0; i < remote_state->branches_nr; i++) {
+		if (!strncmp(name, remote_state->branches[i]->name, len) &&
+		    !remote_state->branches[i]->name[len])
+			return remote_state->branches[i];
 	}
 
-	ALLOC_GROW(the_repository->remote_state->branches,
-		   the_repository->remote_state->branches_nr + 1,
-		   the_repository->remote_state->branches_alloc);
+	ALLOC_GROW(remote_state->branches, remote_state->branches_nr + 1,
+		   remote_state->branches_alloc);
 	CALLOC_ARRAY(ret, 1);
-	the_repository->remote_state
-		->branches[the_repository->remote_state->branches_nr++] = ret;
+	remote_state->branches[remote_state->branches_nr++] = ret;
 	ret->name = xstrndup(name, len);
 	ret->refname = xstrfmt("refs/heads/%s", ret->name);
+	ret->remote_state = remote_state;
 
 	return ret;
 }
@@ -313,10 +312,12 @@ static int handle_config(const char *key, const char *value, void *cb)
 	const char *subkey;
 	struct remote *remote;
 	struct branch *branch;
+	struct remote_state *remote_state = cb;
+
 	if (parse_config_key(key, "branch", &name, &namelen, &subkey) >= 0) {
 		if (!name)
 			return 0;
-		branch = make_branch(name, namelen);
+		branch = make_branch(remote_state, name, namelen);
 		if (!strcmp(subkey, "remote")) {
 			return git_config_string(&branch->remote_name, key, value);
 		} else if (!strcmp(subkey, "pushremote")) {
@@ -335,16 +336,14 @@ static int handle_config(const char *key, const char *value, void *cb)
 		if (!strcmp(subkey, "insteadof")) {
 			if (!value)
 				return config_error_nonbool(key);
-			rewrite = make_rewrite(
-				&the_repository->remote_state->rewrites, name,
-				namelen);
+			rewrite = make_rewrite(&remote_state->rewrites, name,
+					       namelen);
 			add_instead_of(rewrite, xstrdup(value));
 		} else if (!strcmp(subkey, "pushinsteadof")) {
 			if (!value)
 				return config_error_nonbool(key);
-			rewrite = make_rewrite(
-				&the_repository->remote_state->rewrites_push,
-				name, namelen);
+			rewrite = make_rewrite(&remote_state->rewrites_push,
+					       name, namelen);
 			add_instead_of(rewrite, xstrdup(value));
 		}
 	}
@@ -354,9 +353,8 @@ static int handle_config(const char *key, const char *value, void *cb)
 
 	/* Handle remote.* variables */
 	if (!name && !strcmp(subkey, "pushdefault"))
-		return git_config_string(
-			&the_repository->remote_state->pushremote_name, key,
-			value);
+		return git_config_string(&remote_state->pushremote_name, key,
+					 value);
 
 	if (!name)
 		return 0;
@@ -366,7 +364,7 @@ static int handle_config(const char *key, const char *value, void *cb)
 			name);
 		return 0;
 	}
-	remote = make_remote(name, namelen);
+	remote = make_remote(remote_state, name, namelen);
 	remote->origin = REMOTE_CONFIG;
 	if (current_config_scope() == CONFIG_SCOPE_LOCAL ||
 	    current_config_scope() == CONFIG_SCOPE_WORKTREE)
@@ -436,61 +434,51 @@ static int handle_config(const char *key, const char *value, void *cb)
 	return 0;
 }
 
-static void alias_all_urls(void)
+static void alias_all_urls(struct remote_state *remote_state)
 {
 	int i, j;
-	for (i = 0; i < the_repository->remote_state->remotes_nr; i++) {
+	for (i = 0; i < remote_state->remotes_nr; i++) {
 		int add_pushurl_aliases;
-		if (!the_repository->remote_state->remotes[i])
+		if (!remote_state->remotes[i])
 			continue;
-		for (j = 0;
-		     j < the_repository->remote_state->remotes[i]->pushurl_nr;
-		     j++) {
-			the_repository->remote_state->remotes[i]->pushurl[j] =
-				alias_url(
-					the_repository->remote_state->remotes[i]
-						->pushurl[j],
-					&the_repository->remote_state->rewrites);
+		for (j = 0; j < remote_state->remotes[i]->pushurl_nr; j++) {
+			remote_state->remotes[i]->pushurl[j] =
+				alias_url(remote_state->remotes[i]->pushurl[j],
+					  &remote_state->rewrites);
 		}
-		add_pushurl_aliases =
-			the_repository->remote_state->remotes[i]->pushurl_nr ==
-			0;
-		for (j = 0;
-		     j < the_repository->remote_state->remotes[i]->url_nr;
-		     j++) {
+		add_pushurl_aliases = remote_state->remotes[i]->pushurl_nr == 0;
+		for (j = 0; j < remote_state->remotes[i]->url_nr; j++) {
 			if (add_pushurl_aliases)
 				add_pushurl_alias(
-					the_repository->remote_state->remotes[i],
-					the_repository->remote_state->remotes[i]
-						->url[j]);
-			the_repository->remote_state->remotes[i]
-				->url[j] = alias_url(
-				the_repository->remote_state->remotes[i]->url[j],
-				&the_repository->remote_state->rewrites);
+					remote_state->remotes[i],
+					remote_state->remotes[i]->url[j]);
+			remote_state->remotes[i]->url[j] =
+				alias_url(remote_state->remotes[i]->url[j],
+					  &remote_state->rewrites);
 		}
 	}
 }
 
-static void read_config(void)
+static void read_config(struct repository *repo)
 {
-	static int loaded;
 	int flag;
 
-	if (loaded)
+	if (repo->remote_state->initialized)
 		return;
-	loaded = 1;
+	repo->remote_state->initialized = 1;
 
-	the_repository->remote_state->current_branch = NULL;
+	repo->remote_state->current_branch = NULL;
 	if (startup_info->have_repository) {
-		const char *head_ref = resolve_ref_unsafe("HEAD", 0, NULL, &flag);
+		const char *head_ref = refs_resolve_ref_unsafe(
+			get_main_ref_store(repo), "HEAD", 0, NULL, &flag);
 		if (head_ref && (flag & REF_ISSYMREF) &&
 		    skip_prefix(head_ref, "refs/heads/", &head_ref)) {
-			the_repository->remote_state->current_branch =
-				make_branch(head_ref, strlen(head_ref));
+			repo->remote_state->current_branch = make_branch(
+				repo->remote_state, head_ref, strlen(head_ref));
 		}
 	}
-	git_config(handle_config, NULL);
-	alias_all_urls();
+	repo_config(repo, handle_config, repo->remote_state);
+	alias_all_urls(repo->remote_state);
 }
 
 static int valid_remote_nick(const char *name)
@@ -524,10 +512,10 @@ const char *pushremote_for_branch(struct branch *branch, int *explicit)
 			*explicit = 1;
 		return branch->pushremote_name;
 	}
-	if (the_repository->remote_state->pushremote_name) {
+	if (branch->remote_state->pushremote_name) {
 		if (explicit)
 			*explicit = 1;
-		return the_repository->remote_state->pushremote_name;
+		return branch->remote_state->pushremote_name;
 	}
 	return remote_for_branch(branch, explicit);
 }
@@ -560,7 +548,7 @@ static struct remote *remote_get_1(const char *name,
 	struct remote *ret;
 	int name_given = 0;
 
-	read_config();
+	read_config(the_repository);
 
 	if (name)
 		name_given = 1;
@@ -568,7 +556,7 @@ static struct remote *remote_get_1(const char *name,
 		name = get_default(the_repository->remote_state->current_branch,
 				   &name_given);
 
-	ret = make_remote(name, 0);
+	ret = make_remote(the_repository->remote_state, name, 0);
 	if (valid_remote_nick(name) && have_git_dir()) {
 		if (!valid_remote(ret))
 			read_remotes_file(ret);
@@ -604,7 +592,7 @@ int remote_is_configured(struct remote *remote, int in_repo)
 int for_each_remote(each_remote_fn fn, void *priv)
 {
 	int i, result = 0;
-	read_config();
+	read_config(the_repository);
 	for (i = 0; i < the_repository->remote_state->remotes_nr && !result;
 	     i++) {
 		struct remote *remote =
@@ -1717,11 +1705,12 @@ struct branch *branch_get(const char *name)
 {
 	struct branch *ret;
 
-	read_config();
+	read_config(the_repository);
 	if (!name || !*name || !strcmp(name, "HEAD"))
 		ret = the_repository->remote_state->current_branch;
 	else
-		ret = make_branch(name, strlen(name));
+		ret = make_branch(the_repository->remote_state, name,
+				  strlen(name));
 	set_merge(ret);
 	return ret;
 }
diff --git a/remote.h b/remote.h
index d21c035f1b..d268a92ebb 100644
--- a/remote.h
+++ b/remote.h
@@ -52,6 +52,8 @@ struct remote_state {
 
 	struct rewrites rewrites;
 	struct rewrites rewrites_push;
+
+	int initialized;
 };
 
 void remote_state_clear(struct remote_state *remote_state);
@@ -110,6 +112,10 @@ struct remote {
 
 	/* The method used for authenticating against `http_proxy`. */
 	char *http_proxy_authmethod;
+
+	/** The remote_state that this remote belongs to. This is only meant to
+	 * be used by remote_* functions. */
+	struct remote_state *remote_state;
 };
 
 /**
@@ -318,6 +324,10 @@ struct branch {
 	int merge_alloc;
 
 	const char *push_tracking_ref;
+
+	/** The remote_state that this branch belongs to. This is only meant to
+	 * be used by branch_* functions. */
+	struct remote_state *remote_state;
 };
 
 struct branch *branch_get(const char *name);

-- 
2.33.GIT
Previous: Glen ChooNext: Junio C Hamano
Message 24 of 56 in “remote: replace static variables with struct remote_state”
  1. 0/2 remote: replace static variables with struct remote_stateGlen Choo via GitGitGadget, Oct 7, 2021
  2. 1/2 remote: move static variables into structGlen Choo via GitGitGadget, Oct 7, 2021
  3. Junio C HamanoOct 7, 2021
  4. 2/2 remote: add remote_state to struct repositoryGlen Choo via GitGitGadget, Oct 7, 2021
  5. Junio C HamanoOct 7, 2021
  6. Glen ChooOct 8, 2021
  7. 0/3 remote: replace static variables with struct remote_stateGlen Choo, Oct 13, 2021
  8. 1/3 remote: move static variables into per-repository structGlen Choo, Oct 13, 2021
  9. Junio C HamanoOct 13, 2021
  10. Glen ChooOct 14, 2021
  11. Junio C HamanoOct 14, 2021
  12. 2/3 remote: use remote_state parameter internallyGlen Choo, Oct 13, 2021
  13. Junio C HamanoOct 13, 2021
  14. 3/3 remote: add struct repository parameter to external functionsGlen Choo, Oct 13, 2021
  15. Junio C HamanoOct 13, 2021
  16. Junio C HamanoOct 13, 2021
  17. Junio C HamanoOct 13, 2021
  18. Glen ChooOct 13, 2021
  19. Glen ChooOct 13, 2021
  20. Junio C HamanoOct 13, 2021
  21. Glen ChooOct 14, 2021
  22. 0/4 remote: replace static variables with struct remote_stateGlen Choo, Oct 19, 2021
  23. 1/4 remote: move static variables into per-repository structGlen Choo, Oct 19, 2021
  24. 2/4 remote: use remote_state parameter internallyGlen Choo, Oct 19, 2021
  25. Junio C HamanoOct 20, 2021
  26. Junio C HamanoOct 20, 2021
  27. Junio C HamanoOct 20, 2021
  28. Glen ChooOct 25, 2021
  29. Glen ChooOct 25, 2021
  30. Junio C HamanoOct 25, 2021
  31. Glen ChooOct 25, 2021
  32. Junio C HamanoOct 26, 2021
  33. Junio C HamanoOct 26, 2021
  34. Glen ChooOct 26, 2021
  35. Junio C HamanoOct 27, 2021
  36. Glen ChooOct 27, 2021
  37. Junio C HamanoOct 27, 2021
  38. 3/4 remote: remove the_repository->remote_state from static methodsGlen Choo, Oct 19, 2021
  39. 4/4 remote: add struct repository parameter to external functionsGlen Choo, Oct 19, 2021
  40. 0/6 remote: replace static variables with struct remote_stateGlen Choo, Oct 28, 2021
  41. 1/6 t5516: add test case for pushing remote refspecsGlen Choo, Oct 28, 2021
  42. Junio C HamanoOct 28, 2021
  43. Jonathan TanNov 15, 2021
  44. Glen ChooNov 15, 2021
  45. 2/6 remote: move static variables into per-repository structGlen Choo, Oct 28, 2021
  46. 3/6 remote: use remote_state parameter internallyGlen Choo, Oct 28, 2021
  47. 4/6 remote: remove the_repository->remote_state from static methodsGlen Choo, Oct 28, 2021
  48. Jonathan TanNov 15, 2021
  49. 5/6 remote: die if branch is not found in repositoryGlen Choo, Oct 28, 2021
  50. Jonathan TanNov 15, 2021
  51. Glen ChooNov 15, 2021
  52. Jonathan TanNov 16, 2021
  53. 6/6 remote: add struct repository parameter to external functionsGlen Choo, Oct 28, 2021
  54. Jonathan TanNov 15, 2021
  55. Glen ChooNov 15, 2021
  56. Glen ChooNov 12, 2021

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.