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

[PATCH v6 2/7] refs: get rid of global list of hidden refs

From
Patrick Steinhardt <ps@pks.im>
Date
Nov 17, 2022, 05:46 UTC
Message-ID
<48913c1493abfd297d3adb7a41dc3dcc1c5817f2.1668663796.git.ps@pks.im>
In-Reply-To
<cover.1668663795.git.ps@pks.im>

We're about to add a new argument to git-rev-list(1) that allows it to add all references that are visible when taking `transfer.hideRefs` et al into account. This will require us to potentially parse multiple sets of hidden refs, which is not easily possible right now as there is only a single, global instance of the list of parsed hidden refs.

Refactor `parse_hide_refs_config()` and `ref_is_hidden()` so that both take the list of hidden references as input and adjust callers to keep a local list, instead. This allows us to easily use multiple hidden-ref lists. Furthermore, it allows us to properly free this list before we exit.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/receive-pack.c |  8 +++++---
 ls-refs.c              | 13 +++++++++----
 refs.c                 | 14 ++++----------
 refs.h                 |  5 +++--
 upload-pack.c          | 30 ++++++++++++++++++------------
 5 files changed, 39 insertions(+), 31 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 44bcea3a5b..1e24b31a0a 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -80,6 +80,7 @@ static struct object_id push_cert_oid;
 static struct signature_check sigcheck;
 static const char *push_cert_nonce;
 static const char *cert_nonce_seed;
+static struct string_list hidden_refs = STRING_LIST_INIT_DUP;
 
 static const char *NONCE_UNSOLICITED = "UNSOLICITED";
 static const char *NONCE_BAD = "BAD";
@@ -130,7 +131,7 @@ static enum deny_action parse_deny_action(const char *var, const char *value)
 
 static int receive_pack_config(const char *var, const char *value, void *cb)
 {
-	int status = parse_hide_refs_config(var, value, "receive");
+	int status = parse_hide_refs_config(var, value, "receive", &hidden_refs);
 
 	if (status)
 		return status;
@@ -296,7 +297,7 @@ static int show_ref_cb(const char *path_full, const struct object_id *oid,
 	struct oidset *seen = data;
 	const char *path = strip_namespace(path_full);
 
-	if (ref_is_hidden(path, path_full))
+	if (ref_is_hidden(path, path_full, &hidden_refs))
 		return 0;
 
 	/*
@@ -1794,7 +1795,7 @@ static void reject_updates_to_hidden(struct command *commands)
 		strbuf_setlen(&refname_full, prefix_len);
 		strbuf_addstr(&refname_full, cmd->ref_name);
 
-		if (!ref_is_hidden(cmd->ref_name, refname_full.buf))
+		if (!ref_is_hidden(cmd->ref_name, refname_full.buf, &hidden_refs))
 			continue;
 		if (is_null_oid(&cmd->new_oid))
 			cmd->error_string = "deny deleting a hidden ref";
@@ -2591,6 +2592,7 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
 		packet_flush(1);
 	oid_array_clear(&shallow);
 	oid_array_clear(&ref);
+	string_list_clear(&hidden_refs, 0);
 	free((void *)push_cert_nonce);
 	return 0;
 }
diff --git a/ls-refs.c b/ls-refs.c
index fa0d01b47c..fb6769742c 100644
--- a/ls-refs.c
+++ b/ls-refs.c
@@ -6,6 +6,7 @@
 #include "ls-refs.h"
 #include "pkt-line.h"
 #include "config.h"
+#include "string-list.h"
 
 static int config_read;
 static int advertise_unborn;
@@ -73,6 +74,7 @@ struct ls_refs_data {
 	unsigned symrefs;
 	struct strvec prefixes;
 	struct strbuf buf;
+	struct string_list hidden_refs;
 	unsigned unborn : 1;
 };
 
@@ -84,7 +86,7 @@ static int send_ref(const char *refname, const struct object_id *oid,
 
 	strbuf_reset(&data->buf);
 
-	if (ref_is_hidden(refname_nons, refname))
+	if (ref_is_hidden(refname_nons, refname, &data->hidden_refs))
 		return 0;
 
 	if (!ref_match(&data->prefixes, refname_nons))
@@ -137,14 +139,15 @@ static void send_possibly_unborn_head(struct ls_refs_data *data)
 }
 
 static int ls_refs_config(const char *var, const char *value,
-			  void *data UNUSED)
+			  void *cb_data)
 {
+	struct ls_refs_data *data = cb_data;
 	/*
 	 * We only serve fetches over v2 for now, so respect only "uploadpack"
 	 * config. This may need to eventually be expanded to "receive", but we
 	 * don't yet know how that information will be passed to ls-refs.
 	 */
-	return parse_hide_refs_config(var, value, "uploadpack");
+	return parse_hide_refs_config(var, value, "uploadpack", &data->hidden_refs);
 }
 
 int ls_refs(struct repository *r, struct packet_reader *request)
@@ -154,9 +157,10 @@ int ls_refs(struct repository *r, struct packet_reader *request)
 	memset(&data, 0, sizeof(data));
 	strvec_init(&data.prefixes);
 	strbuf_init(&data.buf, 0);
+	string_list_init_dup(&data.hidden_refs);
 
 	ensure_config_read();
-	git_config(ls_refs_config, NULL);
+	git_config(ls_refs_config, &data);
 
 	while (packet_reader_read(request) == PACKET_READ_NORMAL) {
 		const char *arg = request->line;
@@ -195,6 +199,7 @@ int ls_refs(struct repository *r, struct packet_reader *request)
 	packet_fflush(stdout);
 	strvec_clear(&data.prefixes);
 	strbuf_release(&data.buf);
+	string_list_clear(&data.hidden_refs, 0);
 	return 0;
 }
 
diff --git a/refs.c b/refs.c
index a4ab264d74..2c7e88b190 100644
--- a/refs.c
+++ b/refs.c
@@ -1414,9 +1414,8 @@ char *shorten_unambiguous_ref(const char *refname, int strict)
 					    refname, strict);
 }
 
-static struct string_list *hide_refs;
-
-int parse_hide_refs_config(const char *var, const char *value, const char *section)
+int parse_hide_refs_config(const char *var, const char *value, const char *section,
+			   struct string_list *hide_refs)
 {
 	const char *key;
 	if (!strcmp("transfer.hiderefs", var) ||
@@ -1431,21 +1430,16 @@ int parse_hide_refs_config(const char *var, const char *value, const char *secti
 		len = strlen(ref);
 		while (len && ref[len - 1] == '/')
 			ref[--len] = '\0';
-		if (!hide_refs) {
-			CALLOC_ARRAY(hide_refs, 1);
-			hide_refs->strdup_strings = 1;
-		}
 		string_list_append_nodup(hide_refs, ref);
 	}
 	return 0;
 }
 
-int ref_is_hidden(const char *refname, const char *refname_full)
+int ref_is_hidden(const char *refname, const char *refname_full,
+		  const struct string_list *hide_refs)
 {
 	int i;
 
-	if (!hide_refs)
-		return 0;
 	for (i = hide_refs->nr - 1; i >= 0; i--) {
 		const char *match = hide_refs->items[i].string;
 		const char *subject;
diff --git a/refs.h b/refs.h
index 8958717a17..3266fd8f57 100644
--- a/refs.h
+++ b/refs.h
@@ -808,7 +808,8 @@ int update_ref(const char *msg, const char *refname,
 	       const struct object_id *new_oid, const struct object_id *old_oid,
 	       unsigned int flags, enum action_on_err onerr);
 
-int parse_hide_refs_config(const char *var, const char *value, const char *);
+int parse_hide_refs_config(const char *var, const char *value, const char *,
+			   struct string_list *);
 
 /*
  * Check whether a ref is hidden. If no namespace is set, both the first and
@@ -818,7 +819,7 @@ int parse_hide_refs_config(const char *var, const char *value, const char *);
  * the ref is outside that namespace, the first parameter is NULL. The second
  * parameter always points to the full ref name.
  */
-int ref_is_hidden(const char *, const char *);
+int ref_is_hidden(const char *, const char *, const struct string_list *);
 
 /* Is this a per-worktree ref living in the refs/ namespace? */
 int is_per_worktree_ref(const char *refname);
diff --git a/upload-pack.c b/upload-pack.c
index 0b8311bd68..551f22ffa5 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -62,6 +62,7 @@ struct upload_pack_data {
 	struct object_array have_obj;
 	struct oid_array haves;					/* v2 only */
 	struct string_list wanted_refs;				/* v2 only */
+	struct string_list hidden_refs;
 
 	struct object_array shallows;
 	struct string_list deepen_not;
@@ -118,6 +119,7 @@ static void upload_pack_data_init(struct upload_pack_data *data)
 {
 	struct string_list symref = STRING_LIST_INIT_DUP;
 	struct string_list wanted_refs = STRING_LIST_INIT_DUP;
+	struct string_list hidden_refs = STRING_LIST_INIT_DUP;
 	struct object_array want_obj = OBJECT_ARRAY_INIT;
 	struct object_array have_obj = OBJECT_ARRAY_INIT;
 	struct oid_array haves = OID_ARRAY_INIT;
@@ -130,6 +132,7 @@ static void upload_pack_data_init(struct upload_pack_data *data)
 	memset(data, 0, sizeof(*data));
 	data->symref = symref;
 	data->wanted_refs = wanted_refs;
+	data->hidden_refs = hidden_refs;
 	data->want_obj = want_obj;
 	data->have_obj = have_obj;
 	data->haves = haves;
@@ -151,6 +154,7 @@ static void upload_pack_data_clear(struct upload_pack_data *data)
 {
 	string_list_clear(&data->symref, 1);
 	string_list_clear(&data->wanted_refs, 1);
+	string_list_clear(&data->hidden_refs, 0);
 	object_array_clear(&data->want_obj);
 	object_array_clear(&data->have_obj);
 	oid_array_clear(&data->haves);
@@ -842,8 +846,8 @@ static void deepen(struct upload_pack_data *data, int depth)
 		 * Checking for reachable shallows requires that our refs be
 		 * marked with OUR_REF.
 		 */
-		head_ref_namespaced(check_ref, NULL);
-		for_each_namespaced_ref(check_ref, NULL);
+		head_ref_namespaced(check_ref, data);
+		for_each_namespaced_ref(check_ref, data);
 
 		get_reachable_list(data, &reachable_shallows);
 		result = get_shallow_commits(&reachable_shallows,
@@ -1158,11 +1162,11 @@ static void receive_needs(struct upload_pack_data *data,
 
 /* return non-zero if the ref is hidden, otherwise 0 */
 static int mark_our_ref(const char *refname, const char *refname_full,
-			const struct object_id *oid)
+			const struct object_id *oid, const struct string_list *hidden_refs)
 {
 	struct object *o = lookup_unknown_object(the_repository, oid);
 
-	if (ref_is_hidden(refname, refname_full)) {
+	if (ref_is_hidden(refname, refname_full, hidden_refs)) {
 		o->flags |= HIDDEN_REF;
 		return 1;
 	}
@@ -1171,11 +1175,12 @@ static int mark_our_ref(const char *refname, const char *refname_full,
 }
 
 static int check_ref(const char *refname_full, const struct object_id *oid,
-		     int flag UNUSED, void *cb_data UNUSED)
+		     int flag UNUSED, void *cb_data)
 {
 	const char *refname = strip_namespace(refname_full);
+	struct upload_pack_data *data = cb_data;
 
-	mark_our_ref(refname, refname_full, oid);
+	mark_our_ref(refname, refname_full, oid, &data->hidden_refs);
 	return 0;
 }
 
@@ -1204,7 +1209,7 @@ static int send_ref(const char *refname, const struct object_id *oid,
 	struct object_id peeled;
 	struct upload_pack_data *data = cb_data;
 
-	if (mark_our_ref(refname_nons, refname, oid))
+	if (mark_our_ref(refname_nons, refname, oid, &data->hidden_refs))
 		return 0;
 
 	if (capabilities) {
@@ -1327,7 +1332,7 @@ static int upload_pack_config(const char *var, const char *value, void *cb_data)
 	if (parse_object_filter_config(var, value, data) < 0)
 		return -1;
 
-	return parse_hide_refs_config(var, value, "uploadpack");
+	return parse_hide_refs_config(var, value, "uploadpack", &data->hidden_refs);
 }
 
 static int upload_pack_protected_config(const char *var, const char *value, void *cb_data)
@@ -1375,8 +1380,8 @@ void upload_pack(const int advertise_refs, const int stateless_rpc,
 		advertise_shallow_grafts(1);
 		packet_flush(1);
 	} else {
-		head_ref_namespaced(check_ref, NULL);
-		for_each_namespaced_ref(check_ref, NULL);
+		head_ref_namespaced(check_ref, &data);
+		for_each_namespaced_ref(check_ref, &data);
 	}
 
 	if (!advertise_refs) {
@@ -1441,6 +1446,7 @@ static int parse_want(struct packet_writer *writer, const char *line,
 
 static int parse_want_ref(struct packet_writer *writer, const char *line,
 			  struct string_list *wanted_refs,
+			  struct string_list *hidden_refs,
 			  struct object_array *want_obj)
 {
 	const char *refname_nons;
@@ -1451,7 +1457,7 @@ static int parse_want_ref(struct packet_writer *writer, const char *line,
 		struct strbuf refname = STRBUF_INIT;
 
 		strbuf_addf(&refname, "%s%s", get_git_namespace(), refname_nons);
-		if (ref_is_hidden(refname_nons, refname.buf) ||
+		if (ref_is_hidden(refname_nons, refname.buf, hidden_refs) ||
 		    read_ref(refname.buf, &oid)) {
 			packet_writer_error(writer, "unknown ref %s", refname_nons);
 			die("unknown ref %s", refname_nons);
@@ -1508,7 +1514,7 @@ static void process_args(struct packet_reader *request,
 			continue;
 		if (data->allow_ref_in_want &&
 		    parse_want_ref(&data->writer, arg, &data->wanted_refs,
-				   &data->want_obj))
+				   &data->hidden_refs, &data->want_obj))
 			continue;
 		/* process have line */
 		if (parse_have(arg, &data->haves))

-- 
2.38.1
Previous: Patrick SteinhardtNext: Patrick Steinhardt
Message 81 of 88 in “receive-pack: use advertised reference tips to inform connectivity check”
  1. 0/2 receive-pack: use advertised reference tips to inform connectivity checkPatrick Steinhardt, Oct 28, 2022
  2. 1/2 connected: allow supplying different view of reachable objectsPatrick Steinhardt, Oct 28, 2022
  3. Ævar Arnfjörð BjarmasonOct 28, 2022
  4. Junio C HamanoOct 28, 2022
  5. Taylor BlauOct 30, 2022
  6. Patrick SteinhardtOct 31, 2022
  7. Taylor BlauNov 1, 2022
  8. 2/2 receive-pack: use advertised reference tips to inform connectivity checkPatrick Steinhardt, Oct 28, 2022
  9. Ævar Arnfjörð BjarmasonOct 28, 2022
  10. Patrick SteinhardtOct 31, 2022
  11. Ævar Arnfjörð BjarmasonOct 31, 2022
  12. Taylor BlauOct 30, 2022
  13. Patrick SteinhardtOct 31, 2022
  14. Taylor BlauNov 1, 2022
  15. Patrick SteinhardtNov 1, 2022
  16. Patrick SteinhardtNov 1, 2022
  17. Taylor BlauNov 2, 2022
  18. Jeff KingNov 1, 2022
  19. Junio C HamanoOct 28, 2022
  20. Taylor BlauNov 1, 2022
  21. Jeff KingNov 1, 2022
  22. Patrick SteinhardtNov 1, 2022
  23. 0/3 receive-pack: only use visible refs for connectivity checkPatrick Steinhardt, Nov 3, 2022
  24. 1/3 refs: get rid of global list of hidden refsPatrick Steinhardt, Nov 3, 2022
  25. 2/3 revision: add new parameter to specify all visible refsPatrick Steinhardt, Nov 3, 2022
  26. Jeff KingNov 5, 2022
  27. Patrick SteinhardtNov 7, 2022
  28. Jeff KingNov 8, 2022
  29. Jeff KingNov 5, 2022
  30. 3/3 receive-pack: only use visible refs for connectivity checkPatrick Steinhardt, Nov 3, 2022
  31. Taylor BlauNov 5, 2022
  32. Jeff KingNov 5, 2022
  33. Jeff KingNov 5, 2022
  34. 0/6 receive-pack: only use visible refs for connectivity checkPatrick Steinhardt, Nov 7, 2022
  35. 2/6 revision: move together exclusion-related functionsPatrick Steinhardt, Nov 7, 2022
  36. 1/6 refs: get rid of global list of hidden refsPatrick Steinhardt, Nov 7, 2022
  37. 6/6 receive-pack: only use visible refs for connectivity checkPatrick Steinhardt, Nov 7, 2022
  38. 3/6 revision: introduce struct to handle exclusionsPatrick Steinhardt, Nov 7, 2022
  39. Ævar Arnfjörð BjarmasonNov 7, 2022
  40. Patrick SteinhardtNov 8, 2022
  41. 5/6 revparse: add `--exclude-hidden=` optionPatrick Steinhardt, Nov 7, 2022
  42. Jeff KingNov 8, 2022
  43. 4/6 revision: add new parameter to exclude hidden refsPatrick Steinhardt, Nov 7, 2022
  44. Ævar Arnfjörð BjarmasonNov 7, 2022
  45. Ævar Arnfjörð BjarmasonNov 7, 2022
  46. Patrick SteinhardtNov 8, 2022
  47. Patrick SteinhardtNov 8, 2022
  48. Taylor BlauNov 8, 2022
  49. Patrick SteinhardtNov 8, 2022
  50. Jeff KingNov 8, 2022
  51. Taylor BlauNov 8, 2022
  52. 0/6 receive-pack: only use visible refs for connectivity checkPatrick Steinhardt, Nov 8, 2022
  53. 1/6 refs: get rid of global list of hidden refsPatrick Steinhardt, Nov 8, 2022
  54. Ævar Arnfjörð BjarmasonNov 8, 2022
  55. Patrick SteinhardtNov 8, 2022
  56. Jeff KingNov 8, 2022
  57. 2/6 revision: move together exclusion-related functionsPatrick Steinhardt, Nov 8, 2022
  58. 5/6 rev-parse: add `--exclude-hidden=` optionPatrick Steinhardt, Nov 8, 2022
  59. 3/6 revision: introduce struct to handle exclusionsPatrick Steinhardt, Nov 8, 2022
  60. 4/6 revision: add new parameter to exclude hidden refsPatrick Steinhardt, Nov 8, 2022
  61. Jeff KingNov 8, 2022
  62. Taylor BlauNov 8, 2022
  63. Patrick SteinhardtNov 11, 2022
  64. 6/6 receive-pack: only use visible refs for connectivity checkPatrick Steinhardt, Nov 8, 2022
  65. 0/7 receive-pack: only use visible refs for connectivity checkPatrick Steinhardt, Nov 11, 2022
  66. 1/7 refs: fix memory leak when parsing hideRefs configPatrick Steinhardt, Nov 11, 2022
  67. 2/7 refs: get rid of global list of hidden refsPatrick Steinhardt, Nov 11, 2022
  68. 3/7 revision: move together exclusion-related functionsPatrick Steinhardt, Nov 11, 2022
  69. 4/7 revision: introduce struct to handle exclusionsPatrick Steinhardt, Nov 11, 2022
  70. 5/7 revision: add new parameter to exclude hidden refsPatrick Steinhardt, Nov 11, 2022
  71. 6/7 rev-parse: add `--exclude-hidden=` optionPatrick Steinhardt, Nov 11, 2022
  72. 7/7 receive-pack: only use visible refs for connectivity checkPatrick Steinhardt, Nov 11, 2022
  73. Taylor BlauNov 11, 2022
  74. Jeff KingNov 15, 2022
  75. Taylor BlauNov 16, 2022
  76. Jeff KingNov 16, 2022
  77. Taylor BlauNov 16, 2022
  78. Patrick SteinhardtNov 17, 2022
  79. 0/7 receive-pack: only use visible refs for connectivity checkPatrick Steinhardt, Nov 17, 2022
  80. 1/7 refs: fix memory leak when parsing hideRefs configPatrick Steinhardt, Nov 17, 2022
  81. 2/7 refs: get rid of global list of hidden refsPatrick Steinhardt, Nov 17, 2022
  82. 3/7 revision: move together exclusion-related functionsPatrick Steinhardt, Nov 17, 2022
  83. 4/7 revision: introduce struct to handle exclusionsPatrick Steinhardt, Nov 17, 2022
  84. 5/7 revision: add new parameter to exclude hidden refsPatrick Steinhardt, Nov 17, 2022
  85. 6/7 rev-parse: add `--exclude-hidden=` optionPatrick Steinhardt, Nov 17, 2022
  86. 7/7 receive-pack: only use visible refs for connectivity checkPatrick Steinhardt, Nov 17, 2022
  87. Jeff KingNov 17, 2022
  88. Taylor BlauNov 17, 2022

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.