From bdbebe5714b25dc9d215b48efbb80f410925d7dd Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 23 Oct 2025 09:16:10 +0200 Subject: refs: introduce wrapper struct for `each_ref_fn` The `each_ref_fn` callback function type is used across our code base for several different functions that iterate through reference. There's a bunch of callbacks implementing this type, which makes any changes to the callback signature extremely noisy. An example of the required churn is e8207717f1 (refs: add referent to each_ref_fn, 2024-08-09): adding a single argument required us to change 48 files. It was already proposed back then [1] that we might want to introduce a wrapper structure to alleviate the pain going forward. While this of course requires the same kind of global refactoring as just introducing a new parameter, it at least allows us to more change the callback type afterwards by just extending the wrapper structure. One counterargument to this refactoring is that it makes the structure more opaque. While it is obvious which callsites need to be fixed up when we change the function type, it's not obvious anymore once we use a structure. That being said, we only have a handful of sites that actually need to populate this wrapper structure: our ref backends, "refs/iterator.c" as well as very few sites that invoke the iterator callback functions directly. Introduce this wrapper structure so that we can adapt the iterator interfaces more readily. [1]: Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- upload-pack.c | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) (limited to 'upload-pack.c') diff --git a/upload-pack.c b/upload-pack.c index 1e87ae9559..0d563ae74e 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -870,8 +870,8 @@ static void send_unshallow(struct upload_pack_data *data) } } -static int check_ref(const char *refname_full, const char *referent UNUSED, const struct object_id *oid, - int flag, void *cb_data); +static int check_ref(const struct reference *ref, void *cb_data); + static void deepen(struct upload_pack_data *data, int depth) { if (depth == INFINITE_DEPTH && !is_repository_shallow(the_repository)) { @@ -1224,13 +1224,12 @@ static int mark_our_ref(const char *refname, const char *refname_full, return 0; } -static int check_ref(const char *refname_full, const char *referent UNUSED,const struct object_id *oid, - int flag UNUSED, void *cb_data) +static int check_ref(const struct reference *ref, void *cb_data) { - const char *refname = strip_namespace(refname_full); + const char *refname = strip_namespace(ref->name); struct upload_pack_data *data = cb_data; - mark_our_ref(refname, refname_full, oid, &data->hidden_refs); + mark_our_ref(refname, ref->name, ref->oid, &data->hidden_refs); return 0; } @@ -1292,27 +1291,25 @@ static void write_v0_ref(struct upload_pack_data *data, return; } -static int send_ref(const char *refname, const char *referent UNUSED, const struct object_id *oid, - int flag UNUSED, void *cb_data) +static int send_ref(const struct reference *ref, void *cb_data) { - write_v0_ref(cb_data, refname, strip_namespace(refname), oid); + write_v0_ref(cb_data, ref->name, strip_namespace(ref->name), ref->oid); return 0; } -static int find_symref(const char *refname, const char *referent UNUSED, - const struct object_id *oid UNUSED, - int flag, void *cb_data) +static int find_symref(const struct reference *ref, void *cb_data) { const char *symref_target; struct string_list_item *item; + int flag; - if ((flag & REF_ISSYMREF) == 0) + if ((ref->flags & REF_ISSYMREF) == 0) return 0; symref_target = refs_resolve_ref_unsafe(get_main_ref_store(the_repository), - refname, 0, NULL, &flag); + ref->name, 0, NULL, &flag); if (!symref_target || (flag & REF_ISSYMREF) == 0) - die("'%s' is a symref but it is not?", refname); - item = string_list_append(cb_data, strip_namespace(refname)); + die("'%s' is a symref but it is not?", ref->name); + item = string_list_append(cb_data, strip_namespace(ref->name)); item->util = xstrdup(strip_namespace(symref_target)); return 0; } -- cgit v1.3 From adecd5f0b6fdd40219d5503fdaf46aa8d36a4ff7 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 23 Oct 2025 09:16:15 +0200 Subject: upload-pack: convert to use `reference_get_peeled_oid()` The `write_v0_ref()` callback is invoked from two callsites: - Once via `send_ref()` which is a callback passed to `for_each_namespaced_ref_1()` and `refs_head_ref_namespaced()`. - Once manually to announce capabilities. When sending references to the client we also send the peeled value of tags. As we don't have a `struct reference` available in the second case, we cannot easily peel by calling `reference_get_peeled_oid()`, but we instead have to depend on on global state via `peel_iterated_oid()`. We do have a reference available though in the first case, it's only the second case that keeps us from using `reference_get_peeled_oid()`. But that second case only announces capabilities anyway, so we're not really handling a reference at all here. Adapt that case to construct a reference manually and pass that to `write_v0_ref()`. Start to use `reference_get_peeled_oid()` now that we always have a `struct reference` available. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- upload-pack.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) (limited to 'upload-pack.c') diff --git a/upload-pack.c b/upload-pack.c index 0d563ae74e..2d2b70cbf2 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -1249,15 +1249,15 @@ static void format_session_id(struct strbuf *buf, struct upload_pack_data *d) { } static void write_v0_ref(struct upload_pack_data *data, - const char *refname, const char *refname_nons, - const struct object_id *oid) + const struct reference *ref, + const char *refname_nons) { static const char *capabilities = "multi_ack thin-pack side-band" " side-band-64k ofs-delta shallow deepen-since deepen-not" " deepen-relative no-progress include-tag multi_ack_detailed"; struct object_id peeled; - if (mark_our_ref(refname_nons, refname, oid, &data->hidden_refs)) + if (mark_our_ref(refname_nons, ref->name, ref->oid, &data->hidden_refs)) return; if (capabilities) { @@ -1267,7 +1267,7 @@ static void write_v0_ref(struct upload_pack_data *data, format_symref_info(&symref_info, &data->symref); format_session_id(&session_id, data); packet_fwrite_fmt(stdout, "%s %s%c%s%s%s%s%s%s%s object-format=%s agent=%s\n", - oid_to_hex(oid), refname_nons, + oid_to_hex(ref->oid), refname_nons, 0, capabilities, (data->allow_uor & ALLOW_TIP_SHA1) ? " allow-tip-sha1-in-want" : "", @@ -1283,17 +1283,17 @@ static void write_v0_ref(struct upload_pack_data *data, strbuf_release(&session_id); data->sent_capabilities = 1; } else { - packet_fwrite_fmt(stdout, "%s %s\n", oid_to_hex(oid), refname_nons); + packet_fwrite_fmt(stdout, "%s %s\n", oid_to_hex(ref->oid), refname_nons); } capabilities = NULL; - if (!peel_iterated_oid(the_repository, oid, &peeled)) + if (!reference_get_peeled_oid(the_repository, ref, &peeled)) packet_fwrite_fmt(stdout, "%s %s^{}\n", oid_to_hex(&peeled), refname_nons); return; } static int send_ref(const struct reference *ref, void *cb_data) { - write_v0_ref(cb_data, ref->name, strip_namespace(ref->name), ref->oid); + write_v0_ref(cb_data, ref, strip_namespace(ref->name)); return 0; } @@ -1442,8 +1442,12 @@ void upload_pack(const int advertise_refs, const int stateless_rpc, send_ref, &data); for_each_namespaced_ref_1(send_ref, &data); if (!data.sent_capabilities) { - const char *refname = "capabilities^{}"; - write_v0_ref(&data, refname, refname, null_oid(the_hash_algo)); + struct reference ref = { + .name = "capabilities^{}", + .oid = null_oid(the_hash_algo), + }; + + write_v0_ref(&data, &ref, ref.name); } /* * fflush stdout before calling advertise_shallow_grafts because send_ref -- cgit v1.3