From e78ab370545d81a950fa3b2701dd7c72015ee802 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 30 Oct 2025 11:38:38 +0100 Subject: [PATCH 01/29] packfile: use a `strmap` to store packs by name To allow fast lookups of a packfile by name we use a hashmap that has the packfile name as key and the pack itself as value. But while this is the perfect use case for a `strmap`, we instead use `struct hashmap` and store the hashmap entry in the packfile itself. Simplify the code by using a `strmap` instead. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- packfile.c | 24 ++++-------------------- packfile.h | 4 ++-- 2 files changed, 6 insertions(+), 22 deletions(-) diff --git a/packfile.c b/packfile.c index 1ae2b2fe1eda77..04649e52920a1f 100644 --- a/packfile.c +++ b/packfile.c @@ -788,8 +788,7 @@ void packfile_store_add_pack(struct packfile_store *store, pack->next = store->packs; store->packs = pack; - hashmap_entry_init(&pack->packmap_ent, strhash(pack->pack_name)); - hashmap_add(&store->map, &pack->packmap_ent); + strmap_put(&store->packs_by_path, pack->pack_name, pack); } struct packed_git *packfile_store_load_pack(struct packfile_store *store, @@ -806,8 +805,7 @@ struct packed_git *packfile_store_load_pack(struct packfile_store *store, strbuf_strip_suffix(&key, ".idx"); strbuf_addstr(&key, ".pack"); - p = hashmap_get_entry_from_hash(&store->map, strhash(key.buf), key.buf, - struct packed_git, packmap_ent); + p = strmap_get(&store->packs_by_path, key.buf); if (!p) { p = add_packed_git(store->odb->repo, idx_path, strlen(idx_path), local); @@ -2311,27 +2309,13 @@ int parse_pack_header_option(const char *in, unsigned char *out, unsigned int *l return 0; } -static int pack_map_entry_cmp(const void *cmp_data UNUSED, - const struct hashmap_entry *entry, - const struct hashmap_entry *entry2, - const void *keydata) -{ - const char *key = keydata; - const struct packed_git *pg1, *pg2; - - pg1 = container_of(entry, const struct packed_git, packmap_ent); - pg2 = container_of(entry2, const struct packed_git, packmap_ent); - - return strcmp(pg1->pack_name, key ? key : pg2->pack_name); -} - struct packfile_store *packfile_store_new(struct object_database *odb) { struct packfile_store *store; CALLOC_ARRAY(store, 1); store->odb = odb; INIT_LIST_HEAD(&store->mru); - hashmap_init(&store->map, pack_map_entry_cmp, NULL, 0); + strmap_init(&store->packs_by_path); return store; } @@ -2341,7 +2325,7 @@ void packfile_store_free(struct packfile_store *store) next = p->next; free(p); } - hashmap_clear(&store->map); + strmap_clear(&store->packs_by_path, 0); free(store); } diff --git a/packfile.h b/packfile.h index c9d0b93446b5f5..9da7f14317b02c 100644 --- a/packfile.h +++ b/packfile.h @@ -5,12 +5,12 @@ #include "object.h" #include "odb.h" #include "oidset.h" +#include "strmap.h" /* in odb.h */ struct object_info; struct packed_git { - struct hashmap_entry packmap_ent; struct packed_git *next; struct list_head mru; struct pack_window *windows; @@ -85,7 +85,7 @@ struct packfile_store { * A map of packfile names to packed_git structs for tracking which * packs have been loaded already. */ - struct hashmap map; + struct strmap packs_by_path; /* * Whether packfiles have already been populated with this store's From f905a855b1d1e172ba1e51e03fc5ec531445575e Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 30 Oct 2025 11:38:39 +0100 Subject: [PATCH 02/29] packfile: move the MRU list into the packfile store Packfiles have two lists associated to them: - A list that keeps track of packfiles in the order that they were added to a packfile store. - A list that keeps track of packfiles in most-recently-used order so that packfiles that are more likely to contain a specific object are ordered towards the front. Both of these lists are hosted by `struct packed_git` itself, So to identify all packfiles in a repository you simply need to grab the first packfile and then iterate the `->next` pointers or the MRU list. This pattern has the problem that all packfiles are part of the same list, regardless of whether or not they belong to the same object source. With the upcoming pluggable object database effort this needs to change: packfiles should be contained by a single object source, and reading an object from any such packfile should use that source to look up the object. Consequently, we need to break up the global lists of packfiles into per-object-source lists. A first step towards this goal is to move those lists out of `struct packed_git` and into the packfile store. While the packfile store is currently sitting on the `struct object_database` level, the intent is to push it down one level into the `struct odb_source` in a subsequent patch series. Introduce a new `struct packfile_list` that is used to manage lists of packfiles and use it to store the list of most-recently-used packfiles in `struct packfile_store`. For now, the new list type is only used in a single spot, but we'll expand its usage in subsequent patches. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/pack-objects.c | 9 ++--- midx.c | 2 +- packfile.c | 92 +++++++++++++++++++++++++++++++++++++----- packfile.h | 19 +++++++-- 4 files changed, 104 insertions(+), 18 deletions(-) diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index b5454e5df137b4..5348aebbe9f190 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -1706,8 +1706,8 @@ static int want_object_in_pack_mtime(const struct object_id *oid, uint32_t found_mtime) { int want; + struct packfile_list_entry *e; struct odb_source *source; - struct list_head *pos; if (!exclude && local) { /* @@ -1748,12 +1748,11 @@ static int want_object_in_pack_mtime(const struct object_id *oid, } } - list_for_each(pos, packfile_store_get_packs_mru(the_repository->objects->packfiles)) { - struct packed_git *p = list_entry(pos, struct packed_git, mru); + for (e = the_repository->objects->packfiles->mru.head; e; e = e->next) { + struct packed_git *p = e->pack; want = want_object_in_pack_one(p, oid, exclude, found_pack, found_offset, found_mtime); if (!exclude && want > 0) - list_move(&p->mru, - packfile_store_get_packs_mru(the_repository->objects->packfiles)); + packfile_list_prepend(&the_repository->objects->packfiles->mru, p); if (want != -1) return want; } diff --git a/midx.c b/midx.c index 1d6269f957e781..8022be9a45ecb9 100644 --- a/midx.c +++ b/midx.c @@ -463,7 +463,7 @@ int prepare_midx_pack(struct multi_pack_index *m, p = packfile_store_load_pack(r->objects->packfiles, pack_name.buf, m->source->local); if (p) - list_add_tail(&p->mru, &r->objects->packfiles->mru); + packfile_list_append(&m->source->odb->packfiles->mru, p); strbuf_release(&pack_name); if (!p) { diff --git a/packfile.c b/packfile.c index 04649e52920a1f..4d2d3b674f3fb0 100644 --- a/packfile.c +++ b/packfile.c @@ -47,6 +47,80 @@ static size_t pack_mapped; #define SZ_FMT PRIuMAX static inline uintmax_t sz_fmt(size_t s) { return s; } +void packfile_list_clear(struct packfile_list *list) +{ + struct packfile_list_entry *e, *next; + + for (e = list->head; e; e = next) { + next = e->next; + free(e); + } + + list->head = list->tail = NULL; +} + +static struct packfile_list_entry *packfile_list_remove_internal(struct packfile_list *list, + struct packed_git *pack) +{ + struct packfile_list_entry *e, *prev; + + for (e = list->head, prev = NULL; e; prev = e, e = e->next) { + if (e->pack != pack) + continue; + + if (prev) + prev->next = e->next; + if (list->head == e) + list->head = e->next; + if (list->tail == e) + list->tail = prev; + + return e; + } + + return NULL; +} + +void packfile_list_remove(struct packfile_list *list, struct packed_git *pack) +{ + free(packfile_list_remove_internal(list, pack)); +} + +void packfile_list_prepend(struct packfile_list *list, struct packed_git *pack) +{ + struct packfile_list_entry *entry; + + entry = packfile_list_remove_internal(list, pack); + if (!entry) { + entry = xmalloc(sizeof(*entry)); + entry->pack = pack; + } + entry->next = list->head; + + list->head = entry; + if (!list->tail) + list->tail = entry; +} + +void packfile_list_append(struct packfile_list *list, struct packed_git *pack) +{ + struct packfile_list_entry *entry; + + entry = packfile_list_remove_internal(list, pack); + if (!entry) { + entry = xmalloc(sizeof(*entry)); + entry->pack = pack; + } + entry->next = NULL; + + if (list->tail) { + list->tail->next = entry; + list->tail = entry; + } else { + list->head = list->tail = entry; + } +} + void pack_report(struct repository *repo) { fprintf(stderr, @@ -995,10 +1069,10 @@ static void packfile_store_prepare_mru(struct packfile_store *store) { struct packed_git *p; - INIT_LIST_HEAD(&store->mru); + packfile_list_clear(&store->mru); for (p = store->packs; p; p = p->next) - list_add_tail(&p->mru, &store->mru); + packfile_list_append(&store->mru, p); } void packfile_store_prepare(struct packfile_store *store) @@ -1040,10 +1114,10 @@ struct packed_git *packfile_store_get_packs(struct packfile_store *store) return store->packs; } -struct list_head *packfile_store_get_packs_mru(struct packfile_store *store) +struct packfile_list_entry *packfile_store_get_packs_mru(struct packfile_store *store) { packfile_store_prepare(store); - return &store->mru; + return store->mru.head; } /* @@ -2048,7 +2122,7 @@ static int fill_pack_entry(const struct object_id *oid, int find_pack_entry(struct repository *r, const struct object_id *oid, struct pack_entry *e) { - struct list_head *pos; + struct packfile_list_entry *l; packfile_store_prepare(r->objects->packfiles); @@ -2059,10 +2133,11 @@ int find_pack_entry(struct repository *r, const struct object_id *oid, struct pa if (!r->objects->packfiles->packs) return 0; - list_for_each(pos, &r->objects->packfiles->mru) { - struct packed_git *p = list_entry(pos, struct packed_git, mru); + for (l = r->objects->packfiles->mru.head; l; l = l->next) { + struct packed_git *p = l->pack; + if (!p->multi_pack_index && fill_pack_entry(oid, e, p)) { - list_move(&p->mru, &r->objects->packfiles->mru); + packfile_list_prepend(&r->objects->packfiles->mru, p); return 1; } } @@ -2314,7 +2389,6 @@ struct packfile_store *packfile_store_new(struct object_database *odb) struct packfile_store *store; CALLOC_ARRAY(store, 1); store->odb = odb; - INIT_LIST_HEAD(&store->mru); strmap_init(&store->packs_by_path); return store; } diff --git a/packfile.h b/packfile.h index 9da7f14317b02c..39ed1073e4ad79 100644 --- a/packfile.h +++ b/packfile.h @@ -12,7 +12,6 @@ struct object_info; struct packed_git { struct packed_git *next; - struct list_head mru; struct pack_window *windows; off_t pack_size; const void *index_data; @@ -52,6 +51,20 @@ struct packed_git { char pack_name[FLEX_ARRAY]; /* more */ }; +struct packfile_list { + struct packfile_list_entry *head, *tail; +}; + +struct packfile_list_entry { + struct packfile_list_entry *next; + struct packed_git *pack; +}; + +void packfile_list_clear(struct packfile_list *list); +void packfile_list_remove(struct packfile_list *list, struct packed_git *pack); +void packfile_list_prepend(struct packfile_list *list, struct packed_git *pack); +void packfile_list_append(struct packfile_list *list, struct packed_git *pack); + /* * A store that manages packfiles for a given object database. */ @@ -79,7 +92,7 @@ struct packfile_store { } kept_cache; /* A most-recently-used ordered version of the packs list. */ - struct list_head mru; + struct packfile_list mru; /* * A map of packfile names to packed_git structs for tracking which @@ -153,7 +166,7 @@ struct packed_git *packfile_store_get_packs(struct packfile_store *store); /* * Get all packs in most-recently-used order. */ -struct list_head *packfile_store_get_packs_mru(struct packfile_store *store); +struct packfile_list_entry *packfile_store_get_packs_mru(struct packfile_store *store); /* * Open the packfile and add it to the store if it isn't yet known. Returns From 89219bc0cd09ada8a204e0ace0bd15decaea7d31 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 30 Oct 2025 11:38:40 +0100 Subject: [PATCH 03/29] http: refactor subsystem to use `packfile_list`s The dumb HTTP protocol directly fetches packfiles from the remote server and temporarily stores them in a list of packfiles. Those packfiles are not yet added to the repository's packfile store until we finalize the whole fetch. Refactor the code to instead use a `struct packfile_list` to store those packs. This prepares us for a subsequent change where the `->next` pointer of `struct packed_git` will go away. Note that this refactoring creates some temporary duplication of code, as we now have both `packfile_list_find_oid()` and `find_oid_pack()`. The latter function will be removed in a subsequent commit though. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- http-push.c | 6 +++--- http-walker.c | 26 +++++++++----------------- http.c | 21 ++++++++------------- http.h | 5 +++-- packfile.c | 9 +++++++++ packfile.h | 8 ++++++++ 6 files changed, 40 insertions(+), 35 deletions(-) diff --git a/http-push.c b/http-push.c index a1c01e3b9b93a3..d86ce771198206 100644 --- a/http-push.c +++ b/http-push.c @@ -104,7 +104,7 @@ struct repo { int has_info_refs; int can_update_info_refs; int has_info_packs; - struct packed_git *packs; + struct packfile_list packs; struct remote_lock *locks; }; @@ -311,7 +311,7 @@ static void start_fetch_packed(struct transfer_request *request) struct transfer_request *check_request = request_queue_head; struct http_pack_request *preq; - target = find_oid_pack(&request->obj->oid, repo->packs); + target = packfile_list_find_oid(repo->packs.head, &request->obj->oid); if (!target) { fprintf(stderr, "Unable to fetch %s, will not be able to update server info refs\n", oid_to_hex(&request->obj->oid)); repo->can_update_info_refs = 0; @@ -683,7 +683,7 @@ static int add_send_request(struct object *obj, struct remote_lock *lock) get_remote_object_list(obj->oid.hash[0]); if (obj->flags & (REMOTE | PUSHING)) return 0; - target = find_oid_pack(&obj->oid, repo->packs); + target = packfile_list_find_oid(repo->packs.head, &obj->oid); if (target) { obj->flags |= REMOTE; return 0; diff --git a/http-walker.c b/http-walker.c index 0f7ae46d7f12c0..e886e6486646d1 100644 --- a/http-walker.c +++ b/http-walker.c @@ -15,7 +15,7 @@ struct alt_base { char *base; int got_indices; - struct packed_git *packs; + struct packfile_list packs; struct alt_base *next; }; @@ -324,11 +324,8 @@ static void process_alternates_response(void *callback_data) } else if (is_alternate_allowed(target.buf)) { warning("adding alternate object store: %s", target.buf); - newalt = xmalloc(sizeof(*newalt)); - newalt->next = NULL; + CALLOC_ARRAY(newalt, 1); newalt->base = strbuf_detach(&target, NULL); - newalt->got_indices = 0; - newalt->packs = NULL; while (tail->next != NULL) tail = tail->next; @@ -435,7 +432,7 @@ static int http_fetch_pack(struct walker *walker, struct alt_base *repo, if (fetch_indices(walker, repo)) return -1; - target = find_oid_pack(oid, repo->packs); + target = packfile_list_find_oid(repo->packs.head, oid); if (!target) return -1; close_pack_index(target); @@ -584,17 +581,15 @@ static void cleanup(struct walker *walker) if (data) { alt = data->alt; while (alt) { - struct packed_git *pack; + struct packfile_list_entry *e; alt_next = alt->next; - pack = alt->packs; - while (pack) { - struct packed_git *pack_next = pack->next; - close_pack(pack); - free(pack); - pack = pack_next; + for (e = alt->packs.head; e; e = e->next) { + close_pack(e->pack); + free(e->pack); } + packfile_list_clear(&alt->packs); free(alt->base); free(alt); @@ -612,14 +607,11 @@ struct walker *get_http_walker(const char *url) struct walker_data *data = xmalloc(sizeof(struct walker_data)); struct walker *walker = xmalloc(sizeof(struct walker)); - data->alt = xmalloc(sizeof(*data->alt)); + CALLOC_ARRAY(data->alt, 1); data->alt->base = xstrdup(url); for (s = data->alt->base + strlen(data->alt->base) - 1; *s == '/'; --s) *s = 0; - data->alt->got_indices = 0; - data->alt->packs = NULL; - data->alt->next = NULL; data->got_alternates = -1; walker->corrupt_object_found = 0; diff --git a/http.c b/http.c index 17130823f006f2..41f850db16d19f 100644 --- a/http.c +++ b/http.c @@ -2413,8 +2413,9 @@ static char *fetch_pack_index(unsigned char *hash, const char *base_url) return tmp; } -static int fetch_and_setup_pack_index(struct packed_git **packs_head, - unsigned char *sha1, const char *base_url) +static int fetch_and_setup_pack_index(struct packfile_list *packs, + unsigned char *sha1, + const char *base_url) { struct packed_git *new_pack, *p; char *tmp_idx = NULL; @@ -2448,12 +2449,11 @@ static int fetch_and_setup_pack_index(struct packed_git **packs_head, if (ret) return -1; - new_pack->next = *packs_head; - *packs_head = new_pack; + packfile_list_prepend(packs, new_pack); return 0; } -int http_get_info_packs(const char *base_url, struct packed_git **packs_head) +int http_get_info_packs(const char *base_url, struct packfile_list *packs) { struct http_get_options options = {0}; int ret = 0; @@ -2477,7 +2477,7 @@ int http_get_info_packs(const char *base_url, struct packed_git **packs_head) !parse_oid_hex(data, &oid, &data) && skip_prefix(data, ".pack", &data) && (*data == '\n' || *data == '\0')) { - fetch_and_setup_pack_index(packs_head, oid.hash, base_url); + fetch_and_setup_pack_index(packs, oid.hash, base_url); } else { data = strchrnul(data, '\n'); } @@ -2541,14 +2541,9 @@ int finish_http_pack_request(struct http_pack_request *preq) } void http_install_packfile(struct packed_git *p, - struct packed_git **list_to_remove_from) + struct packfile_list *list_to_remove_from) { - struct packed_git **lst = list_to_remove_from; - - while (*lst != p) - lst = &((*lst)->next); - *lst = (*lst)->next; - + packfile_list_remove(list_to_remove_from, p); packfile_store_add_pack(the_repository->objects->packfiles, p); } diff --git a/http.h b/http.h index 553e16205ce201..f9d459340476e4 100644 --- a/http.h +++ b/http.h @@ -2,6 +2,7 @@ #define HTTP_H struct packed_git; +struct packfile_list; #include "git-zlib.h" @@ -190,7 +191,7 @@ struct curl_slist *http_append_auth_header(const struct credential *c, /* Helpers for fetching packs */ int http_get_info_packs(const char *base_url, - struct packed_git **packs_head); + struct packfile_list *packs); /* Helper for getting Accept-Language header */ const char *http_get_accept_language_header(void); @@ -226,7 +227,7 @@ void release_http_pack_request(struct http_pack_request *preq); * from http_get_info_packs() and have chosen a specific pack to fetch. */ void http_install_packfile(struct packed_git *p, - struct packed_git **list_to_remove_from); + struct packfile_list *list_to_remove_from); /* Helpers for fetching object */ struct http_object_request { diff --git a/packfile.c b/packfile.c index 4d2d3b674f3fb0..6aa2ca8ac9ee43 100644 --- a/packfile.c +++ b/packfile.c @@ -121,6 +121,15 @@ void packfile_list_append(struct packfile_list *list, struct packed_git *pack) } } +struct packed_git *packfile_list_find_oid(struct packfile_list_entry *packs, + const struct object_id *oid) +{ + for (; packs; packs = packs->next) + if (find_pack_entry_one(oid, packs->pack)) + return packs->pack; + return NULL; +} + void pack_report(struct repository *repo) { fprintf(stderr, diff --git a/packfile.h b/packfile.h index 39ed1073e4ad79..a53336d722a42b 100644 --- a/packfile.h +++ b/packfile.h @@ -65,6 +65,14 @@ void packfile_list_remove(struct packfile_list *list, struct packed_git *pack); void packfile_list_prepend(struct packfile_list *list, struct packed_git *pack); void packfile_list_append(struct packfile_list *list, struct packed_git *pack); +/* + * Find the pack within the "packs" list whose index contains the object + * "oid". For general object lookups, you probably don't want this; use + * find_pack_entry() instead. + */ +struct packed_git *packfile_list_find_oid(struct packfile_list_entry *packs, + const struct object_id *oid); + /* * A store that manages packfiles for a given object database. */ From 02a7f6ffab9ec7641f88032f30998976bca07820 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 30 Oct 2025 11:38:41 +0100 Subject: [PATCH 04/29] packfile: fix approximation of object counts When approximating the number of objects in a repository we only take into account two data sources, the multi-pack index and the packfile indices, as both of these data structures allow us to easily figure out how many objects they contain. But the way we currently approximate the number of objects is broken in presence of a multi-pack index. This is due to two separate reasons: - We have recently introduced initial infrastructure for incremental multi-pack indices. Starting with that series, `num_objects` only counts the number of objects of a specific layer of the MIDX chain, so we do not take into account objects from parent layers. This issue is fixed by adding `num_objects_in_base`, which contains the sum of all objects in previous layers. - When using the multi-pack index we may count objects contained in packfiles twice: once via the multi-pack index, but then we again count them via the packfile itself. This issue is fixed by skipping any packfiles that have an MIDX. Overall, given that we _always_ count the packs, we can only end up overestimating the number of objects, and the overestimation is limited to a factor of two at most. The consequences of those issues are very limited though, as we only approximate object counts in a small number of cases: - When writing a commit-graph we use the approximate object count to display the upper limit of a progress display. - In `repo_find_unique_abbrev_r()` we use it to specify a lower limit of how many hex digits we want to abbreviate to. Given that we use power-of-two here to derive the lower limit we may end up with an abbreviated hash that is one digit longer than required. - In `estimate_repack_memory()` we may end up overestimating how much memory a repack needs to pack objects. Conseuqently, we may end up dropping some packfiles from a repack. None of these are really game-changing. But it's nice to fix those issues regardless. While at it, convert the code to use `repo_for_each_pack()`. Furthermore, use `odb_prepare_alternates()` instead of explicitly preparing the packfile store. We really only want to prepare the object database sources, and `get_multi_pack_index()` already knows to prepare the packfile store for us. Helped-by: Taylor Blau Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- packfile.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packfile.c b/packfile.c index 6aa2ca8ac9ee43..b07509b69bd7cc 100644 --- a/packfile.c +++ b/packfile.c @@ -1143,16 +1143,16 @@ unsigned long repo_approximate_object_count(struct repository *r) unsigned long count = 0; struct packed_git *p; - packfile_store_prepare(r->objects->packfiles); + odb_prepare_alternates(r->objects); for (source = r->objects->sources; source; source = source->next) { struct multi_pack_index *m = get_multi_pack_index(source); if (m) - count += m->num_objects; + count += m->num_objects + m->num_objects_in_base; } - for (p = r->objects->packfiles->packs; p; p = p->next) { - if (open_pack_index(p)) + repo_for_each_pack(r, p) { + if (p->multi_pack_index || open_pack_index(p)) continue; count += p->num_objects; } From 0d0e4b5954e97fcdfd0a4120e17e2c570497981a Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 30 Oct 2025 11:38:42 +0100 Subject: [PATCH 05/29] builtin/pack-objects: simplify logic to find kept or nonlocal objects The function `has_sha1_pack_kept_or_nonlocal()` takes an object ID and then searches through packed objects to figure out whether the object exists in a kept or non-local pack. As a performance optimization we remember the packfile that contains a given object ID so that the next call to the function first checks that same packfile again. The way this is written is rather hard to follow though, as the caching mechanism is intertwined with the loop that iterates through the packs. Consequently, we need to do some gymnastics to re-start the iteration if the cached pack does not contain the objects. Refactor this so that we check the cached packfile at the beginning. We don't have to re-verify whether the packfile meets the properties as we have already verified those when storing the pack in `last_found` in the first place. So all we need to do is to use `find_pack_entry_one()` to check whether the pack contains the object ID, and to skip the cached pack in the loop so that we don't search it twice. Furthermore, stop using the `(void *)1` sentinel value and instead use a simple `NULL` pointer to indicate that we don't have a last-found pack yet. This refactoring significantly simplifies the logic and makes it much easier to follow. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/pack-objects.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index 5348aebbe9f190..b83eb8ead14139 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -4388,27 +4388,27 @@ static void add_unreachable_loose_objects(struct rev_info *revs) static int has_sha1_pack_kept_or_nonlocal(const struct object_id *oid) { - struct packfile_store *packs = the_repository->objects->packfiles; - static struct packed_git *last_found = (void *)1; + static struct packed_git *last_found = NULL; struct packed_git *p; - p = (last_found != (void *)1) ? last_found : - packfile_store_get_packs(packs); + if (last_found && find_pack_entry_one(oid, last_found)) + return 1; - while (p) { - if ((!p->pack_local || p->pack_keep || - p->pack_keep_in_core) && - find_pack_entry_one(oid, p)) { + repo_for_each_pack(the_repository, p) { + /* + * We have already checked `last_found`, so there is no need to + * re-check here. + */ + if (p == last_found) + continue; + + if ((!p->pack_local || p->pack_keep || p->pack_keep_in_core) && + find_pack_entry_one(oid, p)) { last_found = p; return 1; } - if (p == last_found) - p = packfile_store_get_packs(packs); - else - p = p->next; - if (p == last_found) - p = p->next; } + return 0; } From 589127caa73090040200989ff4d24c3d54f473f2 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 30 Oct 2025 11:38:43 +0100 Subject: [PATCH 06/29] packfile: move list of packs into the packfile store Move the list of packs into the packfile store. This follows the same logic as in a previous commit, where we moved the most-recently-used list of packs, as well. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/fast-import.c | 4 +-- packfile.c | 83 +++++++++++++++++++------------------------ packfile.h | 16 +++------ 3 files changed, 43 insertions(+), 60 deletions(-) diff --git a/builtin/fast-import.c b/builtin/fast-import.c index 215295c1561f57..6fe6e9bc61d81b 100644 --- a/builtin/fast-import.c +++ b/builtin/fast-import.c @@ -978,7 +978,7 @@ static int store_object( if (e->idx.offset) { duplicate_count_by_type[type]++; return 1; - } else if (find_oid_pack(&oid, packfile_store_get_packs(packs))) { + } else if (packfile_list_find_oid(packfile_store_get_packs(packs), &oid)) { e->type = type; e->pack_id = MAX_PACK_ID; e->idx.offset = 1; /* just not zero! */ @@ -1179,7 +1179,7 @@ static void stream_blob(uintmax_t len, struct object_id *oidout, uintmax_t mark) duplicate_count_by_type[OBJ_BLOB]++; truncate_pack(&checkpoint); - } else if (find_oid_pack(&oid, packfile_store_get_packs(packs))) { + } else if (packfile_list_find_oid(packfile_store_get_packs(packs), &oid)) { e->type = OBJ_BLOB; e->pack_id = MAX_PACK_ID; e->idx.offset = 1; /* just not zero! */ diff --git a/packfile.c b/packfile.c index b07509b69bd7cc..71e95ae11c56d2 100644 --- a/packfile.c +++ b/packfile.c @@ -356,13 +356,14 @@ static void scan_windows(struct packed_git *p, static int unuse_one_window(struct packed_git *current) { - struct packed_git *p, *lru_p = NULL; + struct packfile_list_entry *e; + struct packed_git *lru_p = NULL; struct pack_window *lru_w = NULL, *lru_l = NULL; if (current) scan_windows(current, &lru_p, &lru_w, &lru_l); - for (p = current->repo->objects->packfiles->packs; p; p = p->next) - scan_windows(p, &lru_p, &lru_w, &lru_l); + for (e = current->repo->objects->packfiles->packs.head; e; e = e->next) + scan_windows(e->pack, &lru_p, &lru_w, &lru_l); if (lru_p) { munmap(lru_w->base, lru_w->len); pack_mapped -= lru_w->len; @@ -542,14 +543,15 @@ static void find_lru_pack(struct packed_git *p, struct packed_git **lru_p, struc static int close_one_pack(struct repository *r) { - struct packed_git *p, *lru_p = NULL; + struct packfile_list_entry *e; + struct packed_git *lru_p = NULL; struct pack_window *mru_w = NULL; int accept_windows_inuse = 1; - for (p = r->objects->packfiles->packs; p; p = p->next) { - if (p->pack_fd == -1) + for (e = r->objects->packfiles->packs.head; e; e = e->next) { + if (e->pack->pack_fd == -1) continue; - find_lru_pack(p, &lru_p, &mru_w, &accept_windows_inuse); + find_lru_pack(e->pack, &lru_p, &mru_w, &accept_windows_inuse); } if (lru_p) @@ -868,8 +870,7 @@ void packfile_store_add_pack(struct packfile_store *store, if (pack->pack_fd != -1) pack_open_fds++; - pack->next = store->packs; - store->packs = pack; + packfile_list_prepend(&store->packs, pack); strmap_put(&store->packs_by_path, pack->pack_name, pack); } @@ -1046,9 +1047,10 @@ static void prepare_packed_git_one(struct odb_source *source) string_list_clear(data.garbage, 0); } -DEFINE_LIST_SORT(static, sort_packs, struct packed_git, next); +DEFINE_LIST_SORT(static, sort_packs, struct packfile_list_entry, next); -static int sort_pack(const struct packed_git *a, const struct packed_git *b) +static int sort_pack(const struct packfile_list_entry *a, + const struct packfile_list_entry *b) { int st; @@ -1058,7 +1060,7 @@ static int sort_pack(const struct packed_git *a, const struct packed_git *b) * remote ones could be on a network mounted filesystem. * Favor local ones for these reasons. */ - st = a->pack_local - b->pack_local; + st = a->pack->pack_local - b->pack->pack_local; if (st) return -st; @@ -1067,21 +1069,19 @@ static int sort_pack(const struct packed_git *a, const struct packed_git *b) * and more recent objects tend to get accessed more * often. */ - if (a->mtime < b->mtime) + if (a->pack->mtime < b->pack->mtime) return 1; - else if (a->mtime == b->mtime) + else if (a->pack->mtime == b->pack->mtime) return 0; return -1; } static void packfile_store_prepare_mru(struct packfile_store *store) { - struct packed_git *p; - packfile_list_clear(&store->mru); - for (p = store->packs; p; p = p->next) - packfile_list_append(&store->mru, p); + for (struct packfile_list_entry *e = store->packs.head; e; e = e->next) + packfile_list_append(&store->mru, e->pack); } void packfile_store_prepare(struct packfile_store *store) @@ -1096,7 +1096,11 @@ void packfile_store_prepare(struct packfile_store *store) prepare_multi_pack_index_one(source); prepare_packed_git_one(source); } - sort_packs(&store->packs, sort_pack); + + sort_packs(&store->packs.head, sort_pack); + for (struct packfile_list_entry *e = store->packs.head; e; e = e->next) + if (!e->next) + store->packs.tail = e; packfile_store_prepare_mru(store); store->initialized = true; @@ -1108,7 +1112,7 @@ void packfile_store_reprepare(struct packfile_store *store) packfile_store_prepare(store); } -struct packed_git *packfile_store_get_packs(struct packfile_store *store) +struct packfile_list_entry *packfile_store_get_packs(struct packfile_store *store) { packfile_store_prepare(store); @@ -1120,7 +1124,7 @@ struct packed_git *packfile_store_get_packs(struct packfile_store *store) prepare_midx_pack(m, i); } - return store->packs; + return store->packs.head; } struct packfile_list_entry *packfile_store_get_packs_mru(struct packfile_store *store) @@ -1276,11 +1280,11 @@ void mark_bad_packed_object(struct packed_git *p, const struct object_id *oid) const struct packed_git *has_packed_and_bad(struct repository *r, const struct object_id *oid) { - struct packed_git *p; + struct packfile_list_entry *e; - for (p = r->objects->packfiles->packs; p; p = p->next) - if (oidset_contains(&p->bad_objects, oid)) - return p; + for (e = r->objects->packfiles->packs.head; e; e = e->next) + if (oidset_contains(&e->pack->bad_objects, oid)) + return e->pack; return NULL; } @@ -2088,19 +2092,6 @@ int is_pack_valid(struct packed_git *p) return !open_packed_git(p); } -struct packed_git *find_oid_pack(const struct object_id *oid, - struct packed_git *packs) -{ - struct packed_git *p; - - for (p = packs; p; p = p->next) { - if (find_pack_entry_one(oid, p)) - return p; - } - return NULL; - -} - static int fill_pack_entry(const struct object_id *oid, struct pack_entry *e, struct packed_git *p) @@ -2139,7 +2130,7 @@ int find_pack_entry(struct repository *r, const struct object_id *oid, struct pa if (source->midx && fill_midx_entry(source->midx, oid, e)) return 1; - if (!r->objects->packfiles->packs) + if (!r->objects->packfiles->packs.head) return 0; for (l = r->objects->packfiles->mru.head; l; l = l->next) { @@ -2404,19 +2395,19 @@ struct packfile_store *packfile_store_new(struct object_database *odb) void packfile_store_free(struct packfile_store *store) { - for (struct packed_git *p = store->packs, *next; p; p = next) { - next = p->next; - free(p); - } + for (struct packfile_list_entry *e = store->packs.head; e; e = e->next) + free(e->pack); + packfile_list_clear(&store->packs); + strmap_clear(&store->packs_by_path, 0); free(store); } void packfile_store_close(struct packfile_store *store) { - for (struct packed_git *p = store->packs; p; p = p->next) { - if (p->do_not_close) + for (struct packfile_list_entry *e = store->packs.head; e; e = e->next) { + if (e->pack->do_not_close) BUG("want to close pack marked 'do-not-close'"); - close_pack(p); + close_pack(e->pack); } } diff --git a/packfile.h b/packfile.h index a53336d722a42b..d95275e666c95e 100644 --- a/packfile.h +++ b/packfile.h @@ -11,7 +11,6 @@ struct object_info; struct packed_git { - struct packed_git *next; struct pack_window *windows; off_t pack_size; const void *index_data; @@ -83,7 +82,7 @@ struct packfile_store { * The list of packfiles in the order in which they are being added to * the store. */ - struct packed_git *packs; + struct packfile_list packs; /* * Cache of packfiles which are marked as "kept", either because there @@ -163,13 +162,14 @@ void packfile_store_add_pack(struct packfile_store *store, * repository. */ #define repo_for_each_pack(repo, p) \ - for (p = packfile_store_get_packs(repo->objects->packfiles); p; p = p->next) + for (struct packfile_list_entry *e = packfile_store_get_packs(repo->objects->packfiles); \ + ((p) = (e ? e->pack : NULL)); e = e->next) /* * Get all packs managed by the given store, including packfiles that are * referenced by multi-pack indices. */ -struct packed_git *packfile_store_get_packs(struct packfile_store *store); +struct packfile_list_entry *packfile_store_get_packs(struct packfile_store *store); /* * Get all packs in most-recently-used order. @@ -266,14 +266,6 @@ extern void (*report_garbage)(unsigned seen_bits, const char *path); */ unsigned long repo_approximate_object_count(struct repository *r); -/* - * Find the pack within the "packs" list whose index contains the object "oid". - * For general object lookups, you probably don't want this; use - * find_pack_entry() instead. - */ -struct packed_git *find_oid_pack(const struct object_id *oid, - struct packed_git *packs); - void pack_report(struct repository *repo); /* From 6aff1f25a046f3dcd8a78b0c61414fa2d1c9a93c Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 30 Oct 2025 11:38:44 +0100 Subject: [PATCH 07/29] packfile: always add packfiles to MRU when adding a pack When preparing the packfile store we know to also prepare the MRU list of packfiles with all packs that are currently loaded in the store via `packfile_store_prepare_mru()`. So we know that the list of packs in the MRU list should match the list of packs in the non-MRU list. But there are some direct or indirect callsites that add a packfile to the store via `packfile_store_add_pack()` without adding the pack to the MRU. And while functions that access the MRU (e.g. `find_pack_entry()`) know to call `packfile_store_prepare()`, which knows to prepare the MRU via `packfile_store_prepare_mru()`, that operation will be turned into a no-op because the packfile store is already prepared. So this will not cause us to add the packfile to the MRU, and consequently we won't be able to find the packfile in our MRU list. There are only a handful of callers outside of "packfile.c" that add a packfile to the store: - "builtin/fast-import.c" adds multiple packs of imported objects, but it knows to look up objects via `packfile_store_get_packs()`. This function does not use the MRU, so we're good. - "builtin/index-pack.c" adds the indexed pack to the store in case it needs to perform consistency checks on its objects. - "http.c" adds the fetched pack to the store so that we can access its objects. In all of these cases we actually want to access the contained objects. And luckily, reading these objects works as expected: 1. We eventually end up in `do_oid_object_info_extended()`. 2. Calling `find_pack_entry()` fails because the MRU list doesn't contain the newly added packfile. 3. The callers don't pass `OBJECT_INFO_QUICK`, so we end up repreparing the object database. This will also cause us to reprepare the MRU list. 4. We now retry reading the object via `find_pack_entry()`, and now we succeed because the MRU list got populated. This logic feels quite fragile: we intentionally add the packfile to the store, but we then ultimately rely on repreparing the entire store only to make the packfile accessible. While we do the correct thing in `do_oid_object_info_extended()`, other sites that access the MRU may not know to reprepare. But besides being fragile it's also a waste of resources: repreparing the object database requires us to re-read the alternates file and discard any caches. Refactor the code so that we unconditionally add packfiles to the MRU when adding them to a packfile store. This makes the logic less fragile and ensures that we don't have to reprepare the store to make the pack accessible. Note that this does not allow us to drop `packfile_store_prepare_mru()` just yet: while the MRU list is already populated with all packs now, the order in which we add these packs is indeterministic for most of the part. So by first calling `sort_pack()` on the other packfile list and then re-preparing the MRU list we inherit its sorting. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- midx.c | 2 -- packfile.c | 1 + 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/midx.c b/midx.c index 8022be9a45ecb9..24e1e721754d0c 100644 --- a/midx.c +++ b/midx.c @@ -462,8 +462,6 @@ int prepare_midx_pack(struct multi_pack_index *m, m->pack_names[pack_int_id]); p = packfile_store_load_pack(r->objects->packfiles, pack_name.buf, m->source->local); - if (p) - packfile_list_append(&m->source->odb->packfiles->mru, p); strbuf_release(&pack_name); if (!p) { diff --git a/packfile.c b/packfile.c index 71e95ae11c56d2..60f2e42876a823 100644 --- a/packfile.c +++ b/packfile.c @@ -871,6 +871,7 @@ void packfile_store_add_pack(struct packfile_store *store, pack_open_fds++; packfile_list_prepend(&store->packs, pack); + packfile_list_append(&store->mru, pack); strmap_put(&store->packs_by_path, pack->pack_name, pack); } From c31bad4f7dcf3e04ae22e7d4a1059fd628acf1a2 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 30 Oct 2025 11:38:45 +0100 Subject: [PATCH 08/29] packfile: track packs via the MRU list exclusively We track packfiles via two different lists: - `struct packfile_store::packs` is a list that sorts local packs first. In addition, these packs are sorted so that younger packs are sorted towards the front. - `struct packfile_store::mru` is a list that sorts packs so that most-recently used packs are at the front. The reasoning behind the ordering in the `packs` list is that younger objects stored in the local object store tend to be accessed more frequently, and that is certainly true for some cases. But there are going to be lots of cases where that isn't true. Especially when traversing history it is likely that one needs to access many older objects, and due to our housekeeping it is very likely that almost all of those older objects will be contained in one large pack that is oldest. So whether or not the ordering makes sense really depends on the use case at hand. A flexible approach like our MRU list addresses that need, as it will sort packs towards the front that are accessed all the time. Intuitively, this approach is thus able to satisfy more use cases more efficiently. This reasoning casts some doubt on whether or not it really makes sense to track packs via two different lists. It causes confusion, and it is not clear whether there are use cases where the `packs` list really is such an obvious choice. Merge these two lists into one most-recently-used list. Note that there is one important edge case: `for_each_packed_object()` uses the MRU list to iterate through packs, and then it lists each object in those packs. This would have the effect that we now sort the current pack towards the front, thus modifying the list of packfiles we are iterating over, with the consequence that we'll see an infinite loop. This edge case is worked around by introducing a new field that allows us to skip updating the MRU. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/pack-objects.c | 4 ++-- packfile.c | 27 +++++++-------------------- packfile.h | 27 +++++++++++++++++---------- 3 files changed, 26 insertions(+), 32 deletions(-) diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index b83eb8ead14139..0e4e9f80682fd6 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -1748,11 +1748,11 @@ static int want_object_in_pack_mtime(const struct object_id *oid, } } - for (e = the_repository->objects->packfiles->mru.head; e; e = e->next) { + for (e = the_repository->objects->packfiles->packs.head; e; e = e->next) { struct packed_git *p = e->pack; want = want_object_in_pack_one(p, oid, exclude, found_pack, found_offset, found_mtime); if (!exclude && want > 0) - packfile_list_prepend(&the_repository->objects->packfiles->mru, p); + packfile_list_prepend(&the_repository->objects->packfiles->packs, p); if (want != -1) return want; } diff --git a/packfile.c b/packfile.c index 60f2e42876a823..378b0b1920d493 100644 --- a/packfile.c +++ b/packfile.c @@ -870,9 +870,7 @@ void packfile_store_add_pack(struct packfile_store *store, if (pack->pack_fd != -1) pack_open_fds++; - packfile_list_prepend(&store->packs, pack); - packfile_list_append(&store->mru, pack); - + packfile_list_append(&store->packs, pack); strmap_put(&store->packs_by_path, pack->pack_name, pack); } @@ -1077,14 +1075,6 @@ static int sort_pack(const struct packfile_list_entry *a, return -1; } -static void packfile_store_prepare_mru(struct packfile_store *store) -{ - packfile_list_clear(&store->mru); - - for (struct packfile_list_entry *e = store->packs.head; e; e = e->next) - packfile_list_append(&store->mru, e->pack); -} - void packfile_store_prepare(struct packfile_store *store) { struct odb_source *source; @@ -1103,7 +1093,6 @@ void packfile_store_prepare(struct packfile_store *store) if (!e->next) store->packs.tail = e; - packfile_store_prepare_mru(store); store->initialized = true; } @@ -1128,12 +1117,6 @@ struct packfile_list_entry *packfile_store_get_packs(struct packfile_store *stor return store->packs.head; } -struct packfile_list_entry *packfile_store_get_packs_mru(struct packfile_store *store) -{ - packfile_store_prepare(store); - return store->mru.head; -} - /* * Give a fast, rough count of the number of objects in the repository. This * ignores loose objects completely. If you have a lot of them, then either @@ -2134,11 +2117,12 @@ int find_pack_entry(struct repository *r, const struct object_id *oid, struct pa if (!r->objects->packfiles->packs.head) return 0; - for (l = r->objects->packfiles->mru.head; l; l = l->next) { + for (l = r->objects->packfiles->packs.head; l; l = l->next) { struct packed_git *p = l->pack; if (!p->multi_pack_index && fill_pack_entry(oid, e, p)) { - packfile_list_prepend(&r->objects->packfiles->mru, p); + if (!r->objects->packfiles->skip_mru_updates) + packfile_list_prepend(&r->objects->packfiles->packs, p); return 1; } } @@ -2270,6 +2254,7 @@ int for_each_packed_object(struct repository *repo, each_packed_object_fn cb, int r = 0; int pack_errors = 0; + repo->objects->packfiles->skip_mru_updates = true; repo_for_each_pack(repo, p) { if ((flags & FOR_EACH_OBJECT_LOCAL_ONLY) && !p->pack_local) continue; @@ -2290,6 +2275,8 @@ int for_each_packed_object(struct repository *repo, each_packed_object_fn cb, if (r) break; } + repo->objects->packfiles->skip_mru_updates = false; + return r ? r : pack_errors; } diff --git a/packfile.h b/packfile.h index d95275e666c95e..27ba607e7c5eae 100644 --- a/packfile.h +++ b/packfile.h @@ -79,8 +79,8 @@ struct packfile_store { struct object_database *odb; /* - * The list of packfiles in the order in which they are being added to - * the store. + * The list of packfiles in the order in which they have been most + * recently used. */ struct packfile_list packs; @@ -98,9 +98,6 @@ struct packfile_store { unsigned flags; } kept_cache; - /* A most-recently-used ordered version of the packs list. */ - struct packfile_list mru; - /* * A map of packfile names to packed_git structs for tracking which * packs have been loaded already. @@ -112,6 +109,21 @@ struct packfile_store { * packs. */ bool initialized; + + /* + * Usually, packfiles will be reordered to the front of the `packs` + * list whenever an object is looked up via them. This has the effect + * that packs that contain a lot of accessed objects will be located + * towards the front. + * + * This is usually desireable, but there are exceptions. One exception + * is when the looking up multiple objects in a loop for each packfile. + * In that case, we may easily end up with an infinite loop as the + * packfiles get reordered to the front repeatedly. + * + * Setting this field to `true` thus disables these reorderings. + */ + bool skip_mru_updates; }; /* @@ -171,11 +183,6 @@ void packfile_store_add_pack(struct packfile_store *store, */ struct packfile_list_entry *packfile_store_get_packs(struct packfile_store *store); -/* - * Get all packs in most-recently-used order. - */ -struct packfile_list_entry *packfile_store_get_packs_mru(struct packfile_store *store); - /* * Open the packfile and add it to the store if it isn't yet known. Returns * either the newly opened packfile or the preexisting packfile. Returns a From bdbebe5714b25dc9d215b48efbb80f410925d7dd Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 23 Oct 2025 09:16:10 +0200 Subject: [PATCH 09/29] 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 --- bisect.c | 24 ++++++------- builtin/bisect.c | 17 +++------- builtin/checkout.c | 6 ++-- builtin/describe.c | 18 +++++----- builtin/fetch.c | 13 +++---- builtin/fsck.c | 33 ++++++++++-------- builtin/gc.c | 15 ++++----- builtin/name-rev.c | 17 +++++----- builtin/pack-objects.c | 27 ++++++--------- builtin/receive-pack.c | 13 ++++--- builtin/remote.c | 44 +++++++++++------------- builtin/replace.c | 21 +++++------- builtin/repo.c | 9 ++--- builtin/rev-parse.c | 12 +++---- builtin/show-branch.c | 35 +++++++++---------- builtin/show-ref.c | 20 +++++------ builtin/submodule--helper.c | 10 ++---- builtin/worktree.c | 6 +--- commit-graph.c | 14 ++++---- delta-islands.c | 9 +++-- fetch-pack.c | 16 +++------ help.c | 10 +++--- http-backend.c | 20 +++++------ log-tree.c | 24 ++++++------- ls-refs.c | 36 ++++++++++++-------- midx-write.c | 17 +++++----- negotiator/default.c | 7 ++-- negotiator/skipping.c | 7 ++-- notes.c | 8 ++--- object-name.c | 10 +++--- pseudo-merge.c | 21 +++++------- reachable.c | 9 +++-- ref-filter.c | 24 +++++++------ reflog.c | 9 ++--- refs.c | 67 +++++++++++++++++++++---------------- refs.h | 26 +++++++++++--- refs/files-backend.c | 7 ++-- refs/iterator.c | 9 ++++- remote.c | 27 +++++++-------- repack-midx.c | 16 ++++----- replace-object.c | 16 ++++----- revision.c | 12 +++---- server-info.c | 12 +++---- shallow.c | 16 +++------ submodule.c | 12 ++----- t/helper/test-ref-store.c | 5 ++- upload-pack.c | 29 +++++++--------- walker.c | 8 ++--- worktree.c | 11 ++++-- 49 files changed, 392 insertions(+), 462 deletions(-) diff --git a/bisect.c b/bisect.c index a6dc76b15c910b..326b59c0dc70e7 100644 --- a/bisect.c +++ b/bisect.c @@ -450,21 +450,20 @@ void find_bisection(struct commit_list **commit_list, int *reaches, clear_commit_weight(&commit_weight); } -static int register_ref(const char *refname, const char *referent UNUSED, const struct object_id *oid, - int flags UNUSED, void *cb_data UNUSED) +static int register_ref(const struct reference *ref, void *cb_data UNUSED) { struct strbuf good_prefix = STRBUF_INIT; strbuf_addstr(&good_prefix, term_good); strbuf_addstr(&good_prefix, "-"); - if (!strcmp(refname, term_bad)) { + if (!strcmp(ref->name, term_bad)) { free(current_bad_oid); current_bad_oid = xmalloc(sizeof(*current_bad_oid)); - oidcpy(current_bad_oid, oid); - } else if (starts_with(refname, good_prefix.buf)) { - oid_array_append(&good_revs, oid); - } else if (starts_with(refname, "skip-")) { - oid_array_append(&skipped_revs, oid); + oidcpy(current_bad_oid, ref->oid); + } else if (starts_with(ref->name, good_prefix.buf)) { + oid_array_append(&good_revs, ref->oid); + } else if (starts_with(ref->name, "skip-")) { + oid_array_append(&skipped_revs, ref->oid); } strbuf_release(&good_prefix); @@ -1178,14 +1177,11 @@ int estimate_bisect_steps(int all) return (e < 3 * x) ? n : n - 1; } -static int mark_for_removal(const char *refname, - const char *referent UNUSED, - const struct object_id *oid UNUSED, - int flag UNUSED, void *cb_data) +static int mark_for_removal(const struct reference *ref, void *cb_data) { struct string_list *refs = cb_data; - char *ref = xstrfmt("refs/bisect%s", refname); - string_list_append(refs, ref); + char *bisect_ref = xstrfmt("refs/bisect%s", ref->name); + string_list_append(refs, bisect_ref); return 0; } diff --git a/builtin/bisect.c b/builtin/bisect.c index 8b8d870cd1ef08..5b2024be62dacd 100644 --- a/builtin/bisect.c +++ b/builtin/bisect.c @@ -358,10 +358,7 @@ static int check_and_set_terms(struct bisect_terms *terms, const char *cmd) return 0; } -static int inc_nr(const char *refname UNUSED, - const char *referent UNUSED, - const struct object_id *oid UNUSED, - int flag UNUSED, void *cb_data) +static int inc_nr(const struct reference *ref UNUSED, void *cb_data) { unsigned int *nr = (unsigned int *)cb_data; (*nr)++; @@ -549,12 +546,11 @@ static int bisect_append_log_quoted(const char **argv) return res; } -static int add_bisect_ref(const char *refname, const char *referent UNUSED, const struct object_id *oid, - int flags UNUSED, void *cb) +static int add_bisect_ref(const struct reference *ref, void *cb) { struct add_bisect_ref_data *data = cb; - add_pending_oid(data->revs, refname, oid, data->object_flags); + add_pending_oid(data->revs, ref->name, ref->oid, data->object_flags); return 0; } @@ -1165,12 +1161,9 @@ static int bisect_visualize(struct bisect_terms *terms, int argc, return run_command(&cmd); } -static int get_first_good(const char *refname UNUSED, - const char *referent UNUSED, - const struct object_id *oid, - int flag UNUSED, void *cb_data) +static int get_first_good(const struct reference *ref, void *cb_data) { - oidcpy(cb_data, oid); + oidcpy(cb_data, ref->oid); return 1; } diff --git a/builtin/checkout.c b/builtin/checkout.c index f9453473fe2a20..66b69df6e67234 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -1063,11 +1063,9 @@ static void update_refs_for_switch(const struct checkout_opts *opts, report_tracking(new_branch_info); } -static int add_pending_uninteresting_ref(const char *refname, const char *referent UNUSED, - const struct object_id *oid, - int flags UNUSED, void *cb_data) +static int add_pending_uninteresting_ref(const struct reference *ref, void *cb_data) { - add_pending_oid(cb_data, refname, oid, UNINTERESTING); + add_pending_oid(cb_data, ref->name, ref->oid, UNINTERESTING); return 0; } diff --git a/builtin/describe.c b/builtin/describe.c index ffaf8d9f0aa6ea..79545350443c6c 100644 --- a/builtin/describe.c +++ b/builtin/describe.c @@ -154,20 +154,19 @@ static void add_to_known_names(const char *path, } } -static int get_name(const char *path, const char *referent UNUSED, const struct object_id *oid, - int flag UNUSED, void *cb_data UNUSED) +static int get_name(const struct reference *ref, void *cb_data UNUSED) { int is_tag = 0; struct object_id peeled; int is_annotated, prio; const char *path_to_match = NULL; - if (skip_prefix(path, "refs/tags/", &path_to_match)) { + if (skip_prefix(ref->name, "refs/tags/", &path_to_match)) { is_tag = 1; } else if (all) { if ((exclude_patterns.nr || patterns.nr) && - !skip_prefix(path, "refs/heads/", &path_to_match) && - !skip_prefix(path, "refs/remotes/", &path_to_match)) { + !skip_prefix(ref->name, "refs/heads/", &path_to_match) && + !skip_prefix(ref->name, "refs/remotes/", &path_to_match)) { /* Only accept reference of known type if there are match/exclude patterns */ return 0; } @@ -209,10 +208,10 @@ static int get_name(const char *path, const char *referent UNUSED, const struct } /* Is it annotated? */ - if (!peel_iterated_oid(the_repository, oid, &peeled)) { - is_annotated = !oideq(oid, &peeled); + if (!peel_iterated_oid(the_repository, ref->oid, &peeled)) { + is_annotated = !oideq(ref->oid, &peeled); } else { - oidcpy(&peeled, oid); + oidcpy(&peeled, ref->oid); is_annotated = 0; } @@ -229,7 +228,8 @@ static int get_name(const char *path, const char *referent UNUSED, const struct else prio = 0; - add_to_known_names(all ? path + 5 : path + 10, &peeled, prio, oid); + add_to_known_names(all ? ref->name + 5 : ref->name + 10, + &peeled, prio, ref->oid); return 0; } diff --git a/builtin/fetch.c b/builtin/fetch.c index c7ff3480fb1827..7052e6ff215ead 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -289,13 +289,11 @@ static struct refname_hash_entry *refname_hash_add(struct hashmap *map, return ent; } -static int add_one_refname(const char *refname, const char *referent UNUSED, - const struct object_id *oid, - int flag UNUSED, void *cbdata) +static int add_one_refname(const struct reference *ref, void *cbdata) { struct hashmap *refname_map = cbdata; - (void) refname_hash_add(refname_map, refname, oid); + (void) refname_hash_add(refname_map, ref->name, ref->oid); return 0; } @@ -1416,14 +1414,11 @@ static void set_option(struct transport *transport, const char *name, const char } -static int add_oid(const char *refname UNUSED, - const char *referent UNUSED, - const struct object_id *oid, - int flags UNUSED, void *cb_data) +static int add_oid(const struct reference *ref, void *cb_data) { struct oid_array *oids = cb_data; - oid_array_append(oids, oid); + oid_array_append(oids, ref->oid); return 0; } diff --git a/builtin/fsck.c b/builtin/fsck.c index 8ee95e0d67cf37..ed4eea16803349 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -530,14 +530,13 @@ static int fsck_handle_reflog(const char *logname, void *cb_data) return 0; } -static int fsck_handle_ref(const char *refname, const char *referent UNUSED, const struct object_id *oid, - int flag UNUSED, void *cb_data UNUSED) +static int fsck_handle_ref(const struct reference *ref, void *cb_data UNUSED) { struct object *obj; - obj = parse_object(the_repository, oid); + obj = parse_object(the_repository, ref->oid); if (!obj) { - if (is_promisor_object(the_repository, oid)) { + if (is_promisor_object(the_repository, ref->oid)) { /* * Increment default_refs anyway, because this is a * valid ref. @@ -546,19 +545,19 @@ static int fsck_handle_ref(const char *refname, const char *referent UNUSED, con return 0; } error(_("%s: invalid sha1 pointer %s"), - refname, oid_to_hex(oid)); + ref->name, oid_to_hex(ref->oid)); errors_found |= ERROR_REACHABLE; /* We'll continue with the rest despite the error.. */ return 0; } - if (obj->type != OBJ_COMMIT && is_branch(refname)) { - error(_("%s: not a commit"), refname); + if (obj->type != OBJ_COMMIT && is_branch(ref->name)) { + error(_("%s: not a commit"), ref->name); errors_found |= ERROR_REFS; } default_refs++; obj->flags |= USED; fsck_put_object_name(&fsck_walk_options, - oid, "%s", refname); + ref->oid, "%s", ref->name); mark_object_reachable(obj); return 0; @@ -580,13 +579,19 @@ static void get_default_heads(void) worktrees = get_worktrees(); for (p = worktrees; *p; p++) { struct worktree *wt = *p; - struct strbuf ref = STRBUF_INIT; + struct strbuf refname = STRBUF_INIT; - strbuf_worktree_ref(wt, &ref, "HEAD"); - fsck_head_link(ref.buf, &head_points_at, &head_oid); - if (head_points_at && !is_null_oid(&head_oid)) - fsck_handle_ref(ref.buf, NULL, &head_oid, 0, NULL); - strbuf_release(&ref); + strbuf_worktree_ref(wt, &refname, "HEAD"); + fsck_head_link(refname.buf, &head_points_at, &head_oid); + if (head_points_at && !is_null_oid(&head_oid)) { + struct reference ref = { + .name = refname.buf, + .oid = &head_oid, + }; + + fsck_handle_ref(&ref, NULL); + } + strbuf_release(&refname); if (include_reflogs) refs_for_each_reflog(get_worktree_ref_store(wt), diff --git a/builtin/gc.c b/builtin/gc.c index e19e13d9788076..9de5de175f6a40 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1100,24 +1100,21 @@ struct cg_auto_data { int limit; }; -static int dfs_on_ref(const char *refname UNUSED, - const char *referent UNUSED, - const struct object_id *oid, - int flags UNUSED, - void *cb_data) +static int dfs_on_ref(const struct reference *ref, void *cb_data) { struct cg_auto_data *data = (struct cg_auto_data *)cb_data; int result = 0; + const struct object_id *maybe_peeled = ref->oid; struct object_id peeled; struct commit_list *stack = NULL; struct commit *commit; - if (!peel_iterated_oid(the_repository, oid, &peeled)) - oid = &peeled; - if (odb_read_object_info(the_repository->objects, oid, NULL) != OBJ_COMMIT) + if (!peel_iterated_oid(the_repository, ref->oid, &peeled)) + maybe_peeled = &peeled; + if (odb_read_object_info(the_repository->objects, maybe_peeled, NULL) != OBJ_COMMIT) return 0; - commit = lookup_commit(the_repository, oid); + commit = lookup_commit(the_repository, maybe_peeled); if (!commit) return 0; if (repo_parse_commit(the_repository, commit) || diff --git a/builtin/name-rev.c b/builtin/name-rev.c index 74512e54a38c45..615f7d1aae4987 100644 --- a/builtin/name-rev.c +++ b/builtin/name-rev.c @@ -339,10 +339,9 @@ static int cmp_by_tag_and_age(const void *a_, const void *b_) return a->taggerdate != b->taggerdate; } -static int name_ref(const char *path, const char *referent UNUSED, const struct object_id *oid, - int flags UNUSED, void *cb_data) +static int name_ref(const struct reference *ref, void *cb_data) { - struct object *o = parse_object(the_repository, oid); + struct object *o = parse_object(the_repository, ref->oid); struct name_ref_data *data = cb_data; int can_abbreviate_output = data->tags_only && data->name_only; int deref = 0; @@ -350,14 +349,14 @@ static int name_ref(const char *path, const char *referent UNUSED, const struct struct commit *commit = NULL; timestamp_t taggerdate = TIME_MAX; - if (data->tags_only && !starts_with(path, "refs/tags/")) + if (data->tags_only && !starts_with(ref->name, "refs/tags/")) return 0; if (data->exclude_filters.nr) { struct string_list_item *item; for_each_string_list_item(item, &data->exclude_filters) { - if (subpath_matches(path, item->string) >= 0) + if (subpath_matches(ref->name, item->string) >= 0) return 0; } } @@ -378,7 +377,7 @@ static int name_ref(const char *path, const char *referent UNUSED, const struct * shouldn't stop when seeing 'refs/tags/v1.4' matches * 'refs/tags/v*'. We should show it as 'v1.4'. */ - switch (subpath_matches(path, item->string)) { + switch (subpath_matches(ref->name, item->string)) { case -1: /* did not match */ break; case 0: /* matched fully */ @@ -406,13 +405,13 @@ static int name_ref(const char *path, const char *referent UNUSED, const struct } if (o && o->type == OBJ_COMMIT) { commit = (struct commit *)o; - from_tag = starts_with(path, "refs/tags/"); + from_tag = starts_with(ref->name, "refs/tags/"); if (taggerdate == TIME_MAX) taggerdate = commit->date; } - add_to_tip_table(oid, path, can_abbreviate_output, commit, taggerdate, - from_tag, deref); + add_to_tip_table(ref->oid, ref->name, can_abbreviate_output, + commit, taggerdate, from_tag, deref); return 0; } diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index 5bdc44fb2de1fa..39633a0158e095 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -831,15 +831,14 @@ static enum write_one_status write_one(struct hashfile *f, return WRITE_ONE_WRITTEN; } -static int mark_tagged(const char *path UNUSED, const char *referent UNUSED, const struct object_id *oid, - int flag UNUSED, void *cb_data UNUSED) +static int mark_tagged(const struct reference *ref, void *cb_data UNUSED) { struct object_id peeled; - struct object_entry *entry = packlist_find(&to_pack, oid); + struct object_entry *entry = packlist_find(&to_pack, ref->oid); if (entry) entry->tagged = 1; - if (!peel_iterated_oid(the_repository, oid, &peeled)) { + if (!peel_iterated_oid(the_repository, ref->oid, &peeled)) { entry = packlist_find(&to_pack, &peeled); if (entry) entry->tagged = 1; @@ -3306,13 +3305,12 @@ static void add_tag_chain(const struct object_id *oid) } } -static int add_ref_tag(const char *tag UNUSED, const char *referent UNUSED, const struct object_id *oid, - int flag UNUSED, void *cb_data UNUSED) +static int add_ref_tag(const struct reference *ref, void *cb_data UNUSED) { struct object_id peeled; - if (!peel_iterated_oid(the_repository, oid, &peeled) && obj_is_packed(&peeled)) - add_tag_chain(oid); + if (!peel_iterated_oid(the_repository, ref->oid, &peeled) && obj_is_packed(&peeled)) + add_tag_chain(ref->oid); return 0; } @@ -4533,19 +4531,16 @@ static void record_recent_commit(struct commit *commit, void *data UNUSED) oid_array_append(&recent_objects, &commit->object.oid); } -static int mark_bitmap_preferred_tip(const char *refname, - const char *referent UNUSED, - const struct object_id *oid, - int flags UNUSED, - void *data UNUSED) +static int mark_bitmap_preferred_tip(const struct reference *ref, void *data UNUSED) { + const struct object_id *maybe_peeled = ref->oid; struct object_id peeled; struct object *object; - if (!peel_iterated_oid(the_repository, oid, &peeled)) - oid = &peeled; + if (!peel_iterated_oid(the_repository, ref->oid, &peeled)) + maybe_peeled = &peeled; - object = parse_object_or_die(the_repository, oid, refname); + object = parse_object_or_die(the_repository, maybe_peeled, ref->name); if (object->type == OBJ_COMMIT) object->flags |= NEEDS_BITMAP; diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index c9288a9c7e382b..e8ee0e73217aae 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -305,13 +305,12 @@ static void show_ref(const char *path, const struct object_id *oid) } } -static int show_ref_cb(const char *path_full, const char *referent UNUSED, const struct object_id *oid, - int flag UNUSED, void *data) +static int show_ref_cb(const struct reference *ref, void *data) { struct oidset *seen = data; - const char *path = strip_namespace(path_full); + const char *path = strip_namespace(ref->name); - if (ref_is_hidden(path, path_full, &hidden_refs)) + if (ref_is_hidden(path, ref->name, &hidden_refs)) return 0; /* @@ -320,13 +319,13 @@ static int show_ref_cb(const char *path_full, const char *referent UNUSED, const * transfer but will otherwise ignore them. */ if (!path) { - if (oidset_insert(seen, oid)) + if (oidset_insert(seen, ref->oid)) return 0; path = ".have"; } else { - oidset_insert(seen, oid); + oidset_insert(seen, ref->oid); } - show_ref(path, oid); + show_ref(path, ref->oid); return 0; } diff --git a/builtin/remote.c b/builtin/remote.c index 8a7ed4299a4b51..7ffc14ba15743a 100644 --- a/builtin/remote.c +++ b/builtin/remote.c @@ -570,17 +570,14 @@ struct branches_for_remote { struct known_remotes *keep; }; -static int add_branch_for_removal(const char *refname, - const char *referent UNUSED, - const struct object_id *oid UNUSED, - int flags UNUSED, void *cb_data) +static int add_branch_for_removal(const struct reference *ref, void *cb_data) { struct branches_for_remote *branches = cb_data; struct refspec_item refspec; struct known_remote *kr; memset(&refspec, 0, sizeof(refspec)); - refspec.dst = (char *)refname; + refspec.dst = (char *)ref->name; if (remote_find_tracking(branches->remote, &refspec)) return 0; free(refspec.src); @@ -588,7 +585,7 @@ static int add_branch_for_removal(const char *refname, /* don't delete a branch if another remote also uses it */ for (kr = branches->keep->list; kr; kr = kr->next) { memset(&refspec, 0, sizeof(refspec)); - refspec.dst = (char *)refname; + refspec.dst = (char *)ref->name; if (!remote_find_tracking(kr->remote, &refspec)) { free(refspec.src); return 0; @@ -596,16 +593,16 @@ static int add_branch_for_removal(const char *refname, } /* don't delete non-remote-tracking refs */ - if (!starts_with(refname, "refs/remotes/")) { + if (!starts_with(ref->name, "refs/remotes/")) { /* advise user how to delete local branches */ - if (starts_with(refname, "refs/heads/")) + if (starts_with(ref->name, "refs/heads/")) string_list_append(branches->skipped, - abbrev_branch(refname)); + abbrev_branch(ref->name)); /* silently skip over other non-remote refs */ return 0; } - string_list_append(branches->branches, refname); + string_list_append(branches->branches, ref->name); return 0; } @@ -713,18 +710,18 @@ static int rename_one_reflog(const char *old_refname, return error; } -static int rename_one_ref(const char *old_refname, const char *referent, - const struct object_id *oid, - int flags, void *cb_data) +static int rename_one_ref(const struct reference *ref, void *cb_data) { struct strbuf new_referent = STRBUF_INIT; struct strbuf new_refname = STRBUF_INIT; struct rename_info *rename = cb_data; + const struct object_id *oid = ref->oid; + const char *referent = ref->target; int error; - compute_renamed_ref(rename, old_refname, &new_refname); + compute_renamed_ref(rename, ref->name, &new_refname); - if (flags & REF_ISSYMREF) { + if (ref->flags & REF_ISSYMREF) { /* * Stupidly enough `referent` is not pointing to the immediate * target of a symref, but it's the recursively resolved value. @@ -732,25 +729,25 @@ static int rename_one_ref(const char *old_refname, const char *referent, * unborn symrefs don't have any value for the `referent` at all. */ referent = refs_resolve_ref_unsafe(get_main_ref_store(the_repository), - old_refname, RESOLVE_REF_NO_RECURSE, + ref->name, RESOLVE_REF_NO_RECURSE, NULL, NULL); compute_renamed_ref(rename, referent, &new_referent); oid = NULL; } - error = ref_transaction_delete(rename->transaction, old_refname, + error = ref_transaction_delete(rename->transaction, ref->name, oid, referent, REF_NO_DEREF, NULL, rename->err); if (error < 0) goto out; error = ref_transaction_update(rename->transaction, new_refname.buf, oid, null_oid(the_hash_algo), - (flags & REF_ISSYMREF) ? new_referent.buf : NULL, NULL, + (ref->flags & REF_ISSYMREF) ? new_referent.buf : NULL, NULL, REF_SKIP_CREATE_REFLOG | REF_NO_DEREF | REF_SKIP_OID_VERIFICATION, NULL, rename->err); if (error < 0) goto out; - error = rename_one_reflog(old_refname, oid, rename); + error = rename_one_reflog(ref->name, oid, rename); if (error < 0) goto out; @@ -1125,19 +1122,16 @@ static void free_remote_ref_states(struct ref_states *states) string_list_clear_func(&states->push, clear_push_info); } -static int append_ref_to_tracked_list(const char *refname, - const char *referent UNUSED, - const struct object_id *oid UNUSED, - int flags, void *cb_data) +static int append_ref_to_tracked_list(const struct reference *ref, void *cb_data) { struct ref_states *states = cb_data; struct refspec_item refspec; - if (flags & REF_ISSYMREF) + if (ref->flags & REF_ISSYMREF) return 0; memset(&refspec, 0, sizeof(refspec)); - refspec.dst = (char *)refname; + refspec.dst = (char *)ref->name; if (!remote_find_tracking(states->remote, &refspec)) { string_list_append(&states->tracked, abbrev_branch(refspec.src)); free(refspec.src); diff --git a/builtin/replace.c b/builtin/replace.c index 900b560a77d9d7..4c62c5ab58bd0a 100644 --- a/builtin/replace.c +++ b/builtin/replace.c @@ -47,30 +47,27 @@ struct show_data { enum replace_format format; }; -static int show_reference(const char *refname, - const char *referent UNUSED, - const struct object_id *oid, - int flag UNUSED, void *cb_data) +static int show_reference(const struct reference *ref, void *cb_data) { struct show_data *data = cb_data; - if (!wildmatch(data->pattern, refname, 0)) { + if (!wildmatch(data->pattern, ref->name, 0)) { if (data->format == REPLACE_FORMAT_SHORT) - printf("%s\n", refname); + printf("%s\n", ref->name); else if (data->format == REPLACE_FORMAT_MEDIUM) - printf("%s -> %s\n", refname, oid_to_hex(oid)); + printf("%s -> %s\n", ref->name, oid_to_hex(ref->oid)); else { /* data->format == REPLACE_FORMAT_LONG */ struct object_id object; enum object_type obj_type, repl_type; - if (repo_get_oid(data->repo, refname, &object)) - return error(_("failed to resolve '%s' as a valid ref"), refname); + if (repo_get_oid(data->repo, ref->name, &object)) + return error(_("failed to resolve '%s' as a valid ref"), ref->name); obj_type = odb_read_object_info(data->repo->objects, &object, NULL); - repl_type = odb_read_object_info(data->repo->objects, oid, NULL); + repl_type = odb_read_object_info(data->repo->objects, ref->oid, NULL); - printf("%s (%s) -> %s (%s)\n", refname, type_name(obj_type), - oid_to_hex(oid), type_name(repl_type)); + printf("%s (%s) -> %s (%s)\n", ref->name, type_name(obj_type), + oid_to_hex(ref->oid), type_name(repl_type)); } } diff --git a/builtin/repo.c b/builtin/repo.c index 9d4749f79befa8..f26640bd6ea1e7 100644 --- a/builtin/repo.c +++ b/builtin/repo.c @@ -366,16 +366,13 @@ struct count_references_data { struct progress *progress; }; -static int count_references(const char *refname, - const char *referent UNUSED, - const struct object_id *oid, - int flags UNUSED, void *cb_data) +static int count_references(const struct reference *ref, void *cb_data) { struct count_references_data *data = cb_data; struct ref_stats *stats = data->stats; size_t ref_count; - switch (ref_kind_from_refname(refname)) { + switch (ref_kind_from_refname(ref->name)) { case FILTER_REFS_BRANCHES: stats->branches++; break; @@ -396,7 +393,7 @@ static int count_references(const char *refname, * While iterating through references for counting, also add OIDs in * preparation for the path walk. */ - add_pending_oid(data->revs, NULL, oid, 0); + add_pending_oid(data->revs, NULL, ref->oid, 0); ref_count = get_total_reference_count(stats); display_progress(data->progress, ref_count); diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c index 9da92b990d074b..3578591b4f2a38 100644 --- a/builtin/rev-parse.c +++ b/builtin/rev-parse.c @@ -217,19 +217,17 @@ static int show_default(void) return 0; } -static int show_reference(const char *refname, const char *referent UNUSED, const struct object_id *oid, - int flag UNUSED, void *cb_data UNUSED) +static int show_reference(const struct reference *ref, void *cb_data UNUSED) { - if (ref_excluded(&ref_excludes, refname)) + if (ref_excluded(&ref_excludes, ref->name)) return 0; - show_rev(NORMAL, oid, refname); + show_rev(NORMAL, ref->oid, ref->name); return 0; } -static int anti_reference(const char *refname, const char *referent UNUSED, const struct object_id *oid, - int flag UNUSED, void *cb_data UNUSED) +static int anti_reference(const struct reference *ref, void *cb_data UNUSED) { - show_rev(REVERSED, oid, refname); + show_rev(REVERSED, ref->oid, ref->name); return 0; } diff --git a/builtin/show-branch.c b/builtin/show-branch.c index 441babf2e350f9..10475a6b5edb2b 100644 --- a/builtin/show-branch.c +++ b/builtin/show-branch.c @@ -413,34 +413,32 @@ static int append_ref(const char *refname, const struct object_id *oid, return 0; } -static int append_head_ref(const char *refname, const char *referent UNUSED, const struct object_id *oid, - int flag UNUSED, void *cb_data UNUSED) +static int append_head_ref(const struct reference *ref, void *cb_data UNUSED) { struct object_id tmp; int ofs = 11; - if (!starts_with(refname, "refs/heads/")) + if (!starts_with(ref->name, "refs/heads/")) return 0; /* If both heads/foo and tags/foo exists, get_sha1 would * get confused. */ - if (repo_get_oid(the_repository, refname + ofs, &tmp) || !oideq(&tmp, oid)) + if (repo_get_oid(the_repository, ref->name + ofs, &tmp) || !oideq(&tmp, ref->oid)) ofs = 5; - return append_ref(refname + ofs, oid, 0); + return append_ref(ref->name + ofs, ref->oid, 0); } -static int append_remote_ref(const char *refname, const char *referent UNUSED, const struct object_id *oid, - int flag UNUSED, void *cb_data UNUSED) +static int append_remote_ref(const struct reference *ref, void *cb_data UNUSED) { struct object_id tmp; int ofs = 13; - if (!starts_with(refname, "refs/remotes/")) + if (!starts_with(ref->name, "refs/remotes/")) return 0; /* If both heads/foo and tags/foo exists, get_sha1 would * get confused. */ - if (repo_get_oid(the_repository, refname + ofs, &tmp) || !oideq(&tmp, oid)) + if (repo_get_oid(the_repository, ref->name + ofs, &tmp) || !oideq(&tmp, ref->oid)) ofs = 5; - return append_ref(refname + ofs, oid, 0); + return append_ref(ref->name + ofs, ref->oid, 0); } static int append_tag_ref(const char *refname, const struct object_id *oid, @@ -454,27 +452,26 @@ static int append_tag_ref(const char *refname, const struct object_id *oid, static const char *match_ref_pattern = NULL; static int match_ref_slash = 0; -static int append_matching_ref(const char *refname, const char *referent UNUSED, const struct object_id *oid, - int flag, void *cb_data) +static int append_matching_ref(const struct reference *ref, void *cb_data) { /* we want to allow pattern hold/ to show all * branches under refs/heads/hold/, and v0.99.9? to show * refs/tags/v0.99.9a and friends. */ const char *tail; - int slash = count_slashes(refname); - for (tail = refname; *tail && match_ref_slash < slash; ) + int slash = count_slashes(ref->name); + for (tail = ref->name; *tail && match_ref_slash < slash; ) if (*tail++ == '/') slash--; if (!*tail) return 0; if (wildmatch(match_ref_pattern, tail, 0)) return 0; - if (starts_with(refname, "refs/heads/")) - return append_head_ref(refname, NULL, oid, flag, cb_data); - if (starts_with(refname, "refs/tags/")) - return append_tag_ref(refname, oid, flag, cb_data); - return append_ref(refname, oid, 0); + if (starts_with(ref->name, "refs/heads/")) + return append_head_ref(ref, cb_data); + if (starts_with(ref->name, "refs/tags/")) + return append_tag_ref(ref->name, ref->oid, ref->flags, cb_data); + return append_ref(ref->name, ref->oid, 0); } static void snarf_refs(int head, int remotes) diff --git a/builtin/show-ref.c b/builtin/show-ref.c index 0b6f9edf86c24f..4803b5e59865f6 100644 --- a/builtin/show-ref.c +++ b/builtin/show-ref.c @@ -66,26 +66,25 @@ struct show_ref_data { int show_head; }; -static int show_ref(const char *refname, const char *referent UNUSED, const struct object_id *oid, - int flag UNUSED, void *cbdata) +static int show_ref(const struct reference *ref, void *cbdata) { struct show_ref_data *data = cbdata; - if (data->show_head && !strcmp(refname, "HEAD")) + if (data->show_head && !strcmp(ref->name, "HEAD")) goto match; if (data->patterns) { - int reflen = strlen(refname); + int reflen = strlen(ref->name); const char **p = data->patterns, *m; while ((m = *p++) != NULL) { int len = strlen(m); if (len > reflen) continue; - if (memcmp(m, refname + reflen - len, len)) + if (memcmp(m, ref->name + reflen - len, len)) continue; if (len == reflen) goto match; - if (refname[reflen - len - 1] == '/') + if (ref->name[reflen - len - 1] == '/') goto match; } return 0; @@ -94,18 +93,15 @@ static int show_ref(const char *refname, const char *referent UNUSED, const stru match: data->found_match++; - show_one(data->show_one_opts, refname, oid); + show_one(data->show_one_opts, ref->name, ref->oid); return 0; } -static int add_existing(const char *refname, - const char *referent UNUSED, - const struct object_id *oid UNUSED, - int flag UNUSED, void *cbdata) +static int add_existing(const struct reference *ref, void *cbdata) { struct string_list *list = (struct string_list *)cbdata; - string_list_insert(list, refname); + string_list_insert(list, ref->name); return 0; } diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index fcd73abe5336a9..35f6cf735e51cd 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -593,16 +593,12 @@ static void print_status(unsigned int flags, char state, const char *path, printf("\n"); } -static int handle_submodule_head_ref(const char *refname UNUSED, - const char *referent UNUSED, - const struct object_id *oid, - int flags UNUSED, - void *cb_data) +static int handle_submodule_head_ref(const struct reference *ref, void *cb_data) { struct object_id *output = cb_data; - if (oid) - oidcpy(output, oid); + if (ref->oid) + oidcpy(output, ref->oid); return 0; } diff --git a/builtin/worktree.c b/builtin/worktree.c index 812774a5ca992c..b7f323b5e4d73e 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -635,11 +635,7 @@ static void print_preparing_worktree_line(int detach, * * Returns 0 on failure and non-zero on success. */ -static int first_valid_ref(const char *refname UNUSED, - const char *referent UNUSED, - const struct object_id *oid UNUSED, - int flags UNUSED, - void *cb_data UNUSED) +static int first_valid_ref(const struct reference *ref UNUSED, void *cb_data UNUSED) { return 1; } diff --git a/commit-graph.c b/commit-graph.c index 474454db73d094..f91af416259c84 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -1851,18 +1851,16 @@ struct refs_cb_data { struct progress *progress; }; -static int add_ref_to_set(const char *refname UNUSED, - const char *referent UNUSED, - const struct object_id *oid, - int flags UNUSED, void *cb_data) +static int add_ref_to_set(const struct reference *ref, void *cb_data) { + const struct object_id *maybe_peeled = ref->oid; struct object_id peeled; struct refs_cb_data *data = (struct refs_cb_data *)cb_data; - if (!peel_iterated_oid(data->repo, oid, &peeled)) - oid = &peeled; - if (odb_read_object_info(data->repo->objects, oid, NULL) == OBJ_COMMIT) - oidset_insert(data->commits, oid); + if (!peel_iterated_oid(data->repo, ref->oid, &peeled)) + maybe_peeled = &peeled; + if (odb_read_object_info(data->repo->objects, maybe_peeled, NULL) == OBJ_COMMIT) + oidset_insert(data->commits, maybe_peeled); display_progress(data->progress, oidset_size(data->commits)); diff --git a/delta-islands.c b/delta-islands.c index 36c94799d69d7a..7cfebc4162b0e0 100644 --- a/delta-islands.c +++ b/delta-islands.c @@ -390,8 +390,7 @@ static void add_ref_to_island(kh_str_t *remote_islands, const char *island_name, rl->hash += sha_core; } -static int find_island_for_ref(const char *refname, const char *referent UNUSED, const struct object_id *oid, - int flags UNUSED, void *cb) +static int find_island_for_ref(const struct reference *ref, void *cb) { struct island_load_data *ild = cb; @@ -406,7 +405,7 @@ static int find_island_for_ref(const char *refname, const char *referent UNUSED, /* walk backwards to get last-one-wins ordering */ for (i = ild->nr - 1; i >= 0; i--) { - if (!regexec(&ild->rx[i], refname, + if (!regexec(&ild->rx[i], ref->name, ARRAY_SIZE(matches), matches, 0)) break; } @@ -428,10 +427,10 @@ static int find_island_for_ref(const char *refname, const char *referent UNUSED, if (island_name.len) strbuf_addch(&island_name, '-'); - strbuf_add(&island_name, refname + match->rm_so, match->rm_eo - match->rm_so); + strbuf_add(&island_name, ref->name + match->rm_so, match->rm_eo - match->rm_so); } - add_ref_to_island(ild->remote_islands, island_name.buf, oid); + add_ref_to_island(ild->remote_islands, island_name.buf, ref->oid); strbuf_release(&island_name); return 0; } diff --git a/fetch-pack.c b/fetch-pack.c index fe7a84bf2f97fa..78c45d4a155c89 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -188,13 +188,9 @@ static int rev_list_insert_ref(struct fetch_negotiator *negotiator, return 0; } -static int rev_list_insert_ref_oid(const char *refname UNUSED, - const char *referent UNUSED, - const struct object_id *oid, - int flag UNUSED, - void *cb_data) +static int rev_list_insert_ref_oid(const struct reference *ref, void *cb_data) { - return rev_list_insert_ref(cb_data, oid); + return rev_list_insert_ref(cb_data, ref->oid); } enum ack_type { @@ -616,13 +612,9 @@ static int mark_complete(const struct object_id *oid) return 0; } -static int mark_complete_oid(const char *refname UNUSED, - const char *referent UNUSED, - const struct object_id *oid, - int flag UNUSED, - void *cb_data UNUSED) +static int mark_complete_oid(const struct reference *ref, void *cb_data UNUSED) { - return mark_complete(oid); + return mark_complete(ref->oid); } static void mark_recent_complete_commits(struct fetch_pack_args *args, diff --git a/help.c b/help.c index 5854dd4a7e468b..20e114432d7f85 100644 --- a/help.c +++ b/help.c @@ -851,18 +851,16 @@ struct similar_ref_cb { struct string_list *similar_refs; }; -static int append_similar_ref(const char *refname, const char *referent UNUSED, - const struct object_id *oid UNUSED, - int flags UNUSED, void *cb_data) +static int append_similar_ref(const struct reference *ref, void *cb_data) { struct similar_ref_cb *cb = (struct similar_ref_cb *)(cb_data); - char *branch = strrchr(refname, '/') + 1; + char *branch = strrchr(ref->name, '/') + 1; /* A remote branch of the same name is deemed similar */ - if (starts_with(refname, "refs/remotes/") && + if (starts_with(ref->name, "refs/remotes/") && !strcmp(branch, cb->base_ref)) string_list_append_nodup(cb->similar_refs, - refs_shorten_unambiguous_ref(get_main_ref_store(the_repository), refname, 1)); + refs_shorten_unambiguous_ref(get_main_ref_store(the_repository), ref->name, 1)); return 0; } diff --git a/http-backend.c b/http-backend.c index 9084058f1e9f13..92e1733f14042c 100644 --- a/http-backend.c +++ b/http-backend.c @@ -513,18 +513,17 @@ static void run_service(const char **argv, int buffer_input) exit(1); } -static int show_text_ref(const char *name, const char *referent UNUSED, const struct object_id *oid, - int flag UNUSED, void *cb_data) +static int show_text_ref(const struct reference *ref, void *cb_data) { - const char *name_nons = strip_namespace(name); + const char *name_nons = strip_namespace(ref->name); struct strbuf *buf = cb_data; - struct object *o = parse_object(the_repository, oid); + struct object *o = parse_object(the_repository, ref->oid); if (!o) return 0; - strbuf_addf(buf, "%s\t%s\n", oid_to_hex(oid), name_nons); + strbuf_addf(buf, "%s\t%s\n", oid_to_hex(ref->oid), name_nons); if (o->type == OBJ_TAG) { - o = deref_tag(the_repository, o, name, 0); + o = deref_tag(the_repository, o, ref->name, 0); if (!o) return 0; strbuf_addf(buf, "%s\t%s^{}\n", oid_to_hex(&o->oid), @@ -569,21 +568,20 @@ static void get_info_refs(struct strbuf *hdr, char *arg UNUSED) strbuf_release(&buf); } -static int show_head_ref(const char *refname, const char *referent UNUSED, const struct object_id *oid, - int flag, void *cb_data) +static int show_head_ref(const struct reference *ref, void *cb_data) { struct strbuf *buf = cb_data; - if (flag & REF_ISSYMREF) { + if (ref->flags & REF_ISSYMREF) { const char *target = refs_resolve_ref_unsafe(get_main_ref_store(the_repository), - refname, + ref->name, RESOLVE_REF_READING, NULL, NULL); if (target) strbuf_addf(buf, "ref: %s\n", strip_namespace(target)); } else { - strbuf_addf(buf, "%s\n", oid_to_hex(oid)); + strbuf_addf(buf, "%s\n", oid_to_hex(ref->oid)); } return 0; diff --git a/log-tree.c b/log-tree.c index 7d917f2a83d5ae..1729b0c201271b 100644 --- a/log-tree.c +++ b/log-tree.c @@ -147,9 +147,7 @@ static int ref_filter_match(const char *refname, return 1; } -static int add_ref_decoration(const char *refname, const char *referent UNUSED, const struct object_id *oid, - int flags UNUSED, - void *cb_data) +static int add_ref_decoration(const struct reference *ref, void *cb_data) { int i; struct object *obj; @@ -158,16 +156,16 @@ static int add_ref_decoration(const char *refname, const char *referent UNUSED, struct decoration_filter *filter = (struct decoration_filter *)cb_data; const char *git_replace_ref_base = ref_namespace[NAMESPACE_REPLACE].ref; - if (filter && !ref_filter_match(refname, filter)) + if (filter && !ref_filter_match(ref->name, filter)) return 0; - if (starts_with(refname, git_replace_ref_base)) { + if (starts_with(ref->name, git_replace_ref_base)) { struct object_id original_oid; if (!replace_refs_enabled(the_repository)) return 0; - if (get_oid_hex(refname + strlen(git_replace_ref_base), + if (get_oid_hex(ref->name + strlen(git_replace_ref_base), &original_oid)) { - warning("invalid replace ref %s", refname); + warning("invalid replace ref %s", ref->name); return 0; } obj = parse_object(the_repository, &original_oid); @@ -176,10 +174,10 @@ static int add_ref_decoration(const char *refname, const char *referent UNUSED, return 0; } - objtype = odb_read_object_info(the_repository->objects, oid, NULL); + objtype = odb_read_object_info(the_repository->objects, ref->oid, NULL); if (objtype < 0) return 0; - obj = lookup_object_by_type(the_repository, oid, objtype); + obj = lookup_object_by_type(the_repository, ref->oid, objtype); for (i = 0; i < ARRAY_SIZE(ref_namespace); i++) { struct ref_namespace_info *info = &ref_namespace[i]; @@ -187,24 +185,24 @@ static int add_ref_decoration(const char *refname, const char *referent UNUSED, if (!info->decoration) continue; if (info->exact) { - if (!strcmp(refname, info->ref)) { + if (!strcmp(ref->name, info->ref)) { deco_type = info->decoration; break; } - } else if (starts_with(refname, info->ref)) { + } else if (starts_with(ref->name, info->ref)) { deco_type = info->decoration; break; } } - add_name_decoration(deco_type, refname, obj); + add_name_decoration(deco_type, ref->name, obj); while (obj->type == OBJ_TAG) { if (!obj->parsed) parse_object(the_repository, &obj->oid); obj = ((struct tag *)obj)->tagged; if (!obj) break; - add_name_decoration(DECORATION_REF_TAG, refname, obj); + add_name_decoration(DECORATION_REF_TAG, ref->name, obj); } return 0; } diff --git a/ls-refs.c b/ls-refs.c index c47acde07f335b..64d02723691466 100644 --- a/ls-refs.c +++ b/ls-refs.c @@ -75,42 +75,42 @@ struct ls_refs_data { unsigned unborn : 1; }; -static int send_ref(const char *refname, const char *referent UNUSED, const struct object_id *oid, - int flag, void *cb_data) +static int send_ref(const struct reference *ref, void *cb_data) { struct ls_refs_data *data = cb_data; - const char *refname_nons = strip_namespace(refname); + const char *refname_nons = strip_namespace(ref->name); strbuf_reset(&data->buf); - if (ref_is_hidden(refname_nons, refname, &data->hidden_refs)) + if (ref_is_hidden(refname_nons, ref->name, &data->hidden_refs)) return 0; if (!ref_match(&data->prefixes, refname_nons)) return 0; - if (oid) - strbuf_addf(&data->buf, "%s %s", oid_to_hex(oid), refname_nons); + if (ref->oid) + strbuf_addf(&data->buf, "%s %s", oid_to_hex(ref->oid), refname_nons); else strbuf_addf(&data->buf, "unborn %s", refname_nons); - if (data->symrefs && flag & REF_ISSYMREF) { + if (data->symrefs && ref->flags & REF_ISSYMREF) { + int unused_flag; struct object_id unused; const char *symref_target = refs_resolve_ref_unsafe(get_main_ref_store(the_repository), - refname, + ref->name, 0, &unused, - &flag); + &unused_flag); if (!symref_target) - die("'%s' is a symref but it is not?", refname); + die("'%s' is a symref but it is not?", ref->name); strbuf_addf(&data->buf, " symref-target:%s", strip_namespace(symref_target)); } - if (data->peel && oid) { + if (data->peel && ref->oid) { struct object_id peeled; - if (!peel_iterated_oid(the_repository, oid, &peeled)) + if (!peel_iterated_oid(the_repository, ref->oid, &peeled)) strbuf_addf(&data->buf, " peeled:%s", oid_to_hex(&peeled)); } @@ -131,9 +131,17 @@ static void send_possibly_unborn_head(struct ls_refs_data *data) if (!refs_resolve_ref_unsafe(get_main_ref_store(the_repository), namespaced.buf, 0, &oid, &flag)) return; /* bad ref */ oid_is_null = is_null_oid(&oid); + if (!oid_is_null || - (data->unborn && data->symrefs && (flag & REF_ISSYMREF))) - send_ref(namespaced.buf, NULL, oid_is_null ? NULL : &oid, flag, data); + (data->unborn && data->symrefs && (flag & REF_ISSYMREF))) { + struct reference ref = { + .name = namespaced.buf, + .oid = oid_is_null ? NULL : &oid, + .flags = flag, + }; + + send_ref(&ref, data); + } strbuf_release(&namespaced); } diff --git a/midx-write.c b/midx-write.c index c73010df6d3a4f..f4dd875747a4b6 100644 --- a/midx-write.c +++ b/midx-write.c @@ -697,28 +697,27 @@ static void prepare_midx_packing_data(struct packing_data *pdata, trace2_region_leave("midx", "prepare_midx_packing_data", ctx->repo); } -static int add_ref_to_pending(const char *refname, const char *referent UNUSED, - const struct object_id *oid, - int flag, void *cb_data) +static int add_ref_to_pending(const struct reference *ref, void *cb_data) { struct rev_info *revs = (struct rev_info*)cb_data; + const struct object_id *maybe_peeled = ref->oid; struct object_id peeled; struct object *object; - if ((flag & REF_ISSYMREF) && (flag & REF_ISBROKEN)) { - warning("symbolic ref is dangling: %s", refname); + if ((ref->flags & REF_ISSYMREF) && (ref->flags & REF_ISBROKEN)) { + warning("symbolic ref is dangling: %s", ref->name); return 0; } - if (!peel_iterated_oid(revs->repo, oid, &peeled)) - oid = &peeled; + if (!peel_iterated_oid(revs->repo, ref->oid, &peeled)) + maybe_peeled = &peeled; - object = parse_object_or_die(revs->repo, oid, refname); + object = parse_object_or_die(revs->repo, maybe_peeled, ref->name); if (object->type != OBJ_COMMIT) return 0; add_pending_object(revs, object, ""); - if (bitmap_is_preferred_refname(revs->repo, refname)) + if (bitmap_is_preferred_refname(revs->repo, ref->name)) object->flags |= NEEDS_BITMAP; return 0; } diff --git a/negotiator/default.c b/negotiator/default.c index c479da9b091570..116dedcf83035d 100644 --- a/negotiator/default.c +++ b/negotiator/default.c @@ -38,11 +38,10 @@ static void rev_list_push(struct negotiation_state *ns, } } -static int clear_marks(const char *refname, const char *referent UNUSED, const struct object_id *oid, - int flag UNUSED, - void *cb_data UNUSED) +static int clear_marks(const struct reference *ref, void *cb_data UNUSED) { - struct object *o = deref_tag(the_repository, parse_object(the_repository, oid), refname, 0); + struct object *o = deref_tag(the_repository, parse_object(the_repository, ref->oid), + ref->name, 0); if (o && o->type == OBJ_COMMIT) clear_commit_marks((struct commit *)o, diff --git a/negotiator/skipping.c b/negotiator/skipping.c index 616df6bf3af51c..0a272130fb1b6d 100644 --- a/negotiator/skipping.c +++ b/negotiator/skipping.c @@ -75,11 +75,10 @@ static struct entry *rev_list_push(struct data *data, struct commit *commit, int return entry; } -static int clear_marks(const char *refname, const char *referent UNUSED, const struct object_id *oid, - int flag UNUSED, - void *cb_data UNUSED) +static int clear_marks(const struct reference *ref, void *cb_data UNUSED) { - struct object *o = deref_tag(the_repository, parse_object(the_repository, oid), refname, 0); + struct object *o = deref_tag(the_repository, parse_object(the_repository, ref->oid), + ref->name, 0); if (o && o->type == OBJ_COMMIT) clear_commit_marks((struct commit *)o, diff --git a/notes.c b/notes.c index 9a2e9181fe67d8..8e00fd8c470dd2 100644 --- a/notes.c +++ b/notes.c @@ -938,13 +938,11 @@ int combine_notes_cat_sort_uniq(struct object_id *cur_oid, return ret; } -static int string_list_add_one_ref(const char *refname, const char *referent UNUSED, - const struct object_id *oid UNUSED, - int flag UNUSED, void *cb) +static int string_list_add_one_ref(const struct reference *ref, void *cb) { struct string_list *refs = cb; - if (!unsorted_string_list_has_string(refs, refname)) - string_list_append(refs, refname); + if (!unsorted_string_list_has_string(refs, ref->name)) + string_list_append(refs, ref->name); return 0; } diff --git a/object-name.c b/object-name.c index f6902e140dd43e..7e8109f25fb839 100644 --- a/object-name.c +++ b/object-name.c @@ -1444,18 +1444,16 @@ struct handle_one_ref_cb { struct commit_list **list; }; -static int handle_one_ref(const char *path, const char *referent UNUSED, const struct object_id *oid, - int flag UNUSED, - void *cb_data) +static int handle_one_ref(const struct reference *ref, void *cb_data) { struct handle_one_ref_cb *cb = cb_data; struct commit_list **list = cb->list; - struct object *object = parse_object(cb->repo, oid); + struct object *object = parse_object(cb->repo, ref->oid); if (!object) return 0; if (object->type == OBJ_TAG) { - object = deref_tag(cb->repo, object, path, - strlen(path)); + object = deref_tag(cb->repo, object, ref->name, + strlen(ref->name)); if (!object) return 0; } diff --git a/pseudo-merge.c b/pseudo-merge.c index 893b763fe45490..0abd51b42c185a 100644 --- a/pseudo-merge.c +++ b/pseudo-merge.c @@ -221,28 +221,25 @@ void load_pseudo_merges_from_config(struct repository *r, } } -static int find_pseudo_merge_group_for_ref(const char *refname, - const char *referent UNUSED, - const struct object_id *oid, - int flags UNUSED, - void *_data) +static int find_pseudo_merge_group_for_ref(const struct reference *ref, void *_data) { struct bitmap_writer *writer = _data; + const struct object_id *maybe_peeled = ref->oid; struct object_id peeled; struct commit *c; uint32_t i; int has_bitmap; - if (!peel_iterated_oid(the_repository, oid, &peeled)) - oid = &peeled; + if (!peel_iterated_oid(the_repository, ref->oid, &peeled)) + maybe_peeled = &peeled; - c = lookup_commit(the_repository, oid); + c = lookup_commit(the_repository, maybe_peeled); if (!c) return 0; - if (!packlist_find(writer->to_pack, oid)) + if (!packlist_find(writer->to_pack, maybe_peeled)) return 0; - has_bitmap = bitmap_writer_has_bitmapped_object_id(writer, oid); + has_bitmap = bitmap_writer_has_bitmapped_object_id(writer, maybe_peeled); for (i = 0; i < writer->pseudo_merge_groups.nr; i++) { struct pseudo_merge_group *group; @@ -252,7 +249,7 @@ static int find_pseudo_merge_group_for_ref(const char *refname, size_t j; group = writer->pseudo_merge_groups.items[i].util; - if (regexec(group->pattern, refname, ARRAY_SIZE(captures), + if (regexec(group->pattern, ref->name, ARRAY_SIZE(captures), captures, 0)) continue; @@ -269,7 +266,7 @@ static int find_pseudo_merge_group_for_ref(const char *refname, if (group_name.len) strbuf_addch(&group_name, '-'); - strbuf_add(&group_name, refname + match->rm_so, + strbuf_add(&group_name, ref->name + match->rm_so, match->rm_eo - match->rm_so); } diff --git a/reachable.c b/reachable.c index 22266db5233df7..b753c395530b6d 100644 --- a/reachable.c +++ b/reachable.c @@ -83,18 +83,17 @@ static void add_rebase_files(struct rev_info *revs) free_worktrees(worktrees); } -static int add_one_ref(const char *path, const char *referent UNUSED, const struct object_id *oid, - int flag, void *cb_data) +static int add_one_ref(const struct reference *ref, void *cb_data) { struct rev_info *revs = (struct rev_info *)cb_data; struct object *object; - if ((flag & REF_ISSYMREF) && (flag & REF_ISBROKEN)) { - warning("symbolic ref is dangling: %s", path); + if ((ref->flags & REF_ISSYMREF) && (ref->flags & REF_ISBROKEN)) { + warning("symbolic ref is dangling: %s", ref->name); return 0; } - object = parse_object_or_die(the_repository, oid, path); + object = parse_object_or_die(the_repository, ref->oid, ref->name); add_pending_object(revs, object, ""); return 0; diff --git a/ref-filter.c b/ref-filter.c index 30cc488d8ab659..6837fa60a9b2b5 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -2954,14 +2954,15 @@ struct ref_filter_cbdata { * A call-back given to for_each_ref(). Filter refs and keep them for * later object processing. */ -static int filter_one(const char *refname, const char *referent, const struct object_id *oid, int flag, void *cb_data) +static int filter_one(const struct reference *ref, void *cb_data) { struct ref_filter_cbdata *ref_cbdata = cb_data; - struct ref_array_item *ref; + struct ref_array_item *item; - ref = apply_ref_filter(refname, referent, oid, flag, ref_cbdata->filter); - if (ref) - ref_array_append(ref_cbdata->array, ref); + item = apply_ref_filter(ref->name, ref->target, ref->oid, + ref->flags, ref_cbdata->filter); + if (item) + ref_array_append(ref_cbdata->array, item); return 0; } @@ -2990,17 +2991,18 @@ struct ref_filter_and_format_cbdata { } internal; }; -static int filter_and_format_one(const char *refname, const char *referent, const struct object_id *oid, int flag, void *cb_data) +static int filter_and_format_one(const struct reference *ref, void *cb_data) { struct ref_filter_and_format_cbdata *ref_cbdata = cb_data; - struct ref_array_item *ref; + struct ref_array_item *item; struct strbuf output = STRBUF_INIT, err = STRBUF_INIT; - ref = apply_ref_filter(refname, referent, oid, flag, ref_cbdata->filter); - if (!ref) + item = apply_ref_filter(ref->name, ref->target, ref->oid, + ref->flags, ref_cbdata->filter); + if (!item) return 0; - if (format_ref_array_item(ref, ref_cbdata->format, &output, &err)) + if (format_ref_array_item(item, ref_cbdata->format, &output, &err)) die("%s", err.buf); if (output.len || !ref_cbdata->format->array_opts.omit_empty) { @@ -3010,7 +3012,7 @@ static int filter_and_format_one(const char *refname, const char *referent, cons strbuf_release(&output); strbuf_release(&err); - free_array_item(ref); + free_array_item(item); /* * Increment the running count of refs that match the filter. If diff --git a/reflog.c b/reflog.c index 65ef259b4f5e1e..ac87e20c4f97ff 100644 --- a/reflog.c +++ b/reflog.c @@ -423,16 +423,13 @@ int should_expire_reflog_ent_verbose(struct object_id *ooid, return expire; } -static int push_tip_to_list(const char *refname UNUSED, - const char *referent UNUSED, - const struct object_id *oid, - int flags, void *cb_data) +static int push_tip_to_list(const struct reference *ref, void *cb_data) { struct commit_list **list = cb_data; struct commit *tip_commit; - if (flags & REF_ISSYMREF) + if (ref->flags & REF_ISSYMREF) return 0; - tip_commit = lookup_commit_reference_gently(the_repository, oid, 1); + tip_commit = lookup_commit_reference_gently(the_repository, ref->oid, 1); if (!tip_commit) return 0; commit_list_insert(tip_commit, list); diff --git a/refs.c b/refs.c index 965381367e0e53..25f0579d610ddc 100644 --- a/refs.c +++ b/refs.c @@ -426,17 +426,19 @@ int refs_ref_exists(struct ref_store *refs, const char *refname) NULL, NULL); } -static int for_each_filter_refs(const char *refname, const char *referent, - const struct object_id *oid, - int flags, void *data) +static int for_each_filter_refs(const struct reference *ref, void *data) { struct for_each_ref_filter *filter = data; - if (wildmatch(filter->pattern, refname, 0)) + if (wildmatch(filter->pattern, ref->name, 0)) return 0; - if (filter->prefix) - skip_prefix(refname, filter->prefix, &refname); - return filter->fn(refname, referent, oid, flags, filter->cb_data); + if (filter->prefix) { + struct reference skipped = *ref; + skip_prefix(skipped.name, filter->prefix, &skipped.name); + return filter->fn(&skipped, filter->cb_data); + } else { + return filter->fn(ref, filter->cb_data); + } } struct warn_if_dangling_data { @@ -447,17 +449,15 @@ struct warn_if_dangling_data { int dry_run; }; -static int warn_if_dangling_symref(const char *refname, const char *referent UNUSED, - const struct object_id *oid UNUSED, - int flags, void *cb_data) +static int warn_if_dangling_symref(const struct reference *ref, void *cb_data) { struct warn_if_dangling_data *d = cb_data; const char *resolves_to, *msg; - if (!(flags & REF_ISSYMREF)) + if (!(ref->flags & REF_ISSYMREF)) return 0; - resolves_to = refs_resolve_ref_unsafe(d->refs, refname, 0, NULL, NULL); + resolves_to = refs_resolve_ref_unsafe(d->refs, ref->name, 0, NULL, NULL); if (!resolves_to || !string_list_has_string(d->refnames, resolves_to)) { return 0; @@ -466,7 +466,7 @@ static int warn_if_dangling_symref(const char *refname, const char *referent UNU msg = d->dry_run ? _("%s%s will become dangling after %s is deleted\n") : _("%s%s has become dangling after %s was deleted\n"); - fprintf(d->fp, msg, d->indent, refname, resolves_to); + fprintf(d->fp, msg, d->indent, ref->name, resolves_to); return 0; } @@ -507,8 +507,15 @@ int refs_head_ref_namespaced(struct ref_store *refs, each_ref_fn fn, void *cb_da int flag; strbuf_addf(&buf, "%sHEAD", get_git_namespace()); - if (!refs_read_ref_full(refs, buf.buf, RESOLVE_REF_READING, &oid, &flag)) - ret = fn(buf.buf, NULL, &oid, flag, cb_data); + if (!refs_read_ref_full(refs, buf.buf, RESOLVE_REF_READING, &oid, &flag)) { + struct reference ref = { + .name = buf.buf, + .oid = &oid, + .flags = flag, + }; + + ret = fn(&ref, cb_data); + } strbuf_release(&buf); return ret; @@ -1741,8 +1748,15 @@ int refs_head_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data) int flag; if (refs_resolve_ref_unsafe(refs, "HEAD", RESOLVE_REF_READING, - &oid, &flag)) - return fn("HEAD", NULL, &oid, flag, cb_data); + &oid, &flag)) { + struct reference ref = { + .name = "HEAD", + .oid = &oid, + .flags = flag, + }; + + return fn(&ref, cb_data); + } return 0; } @@ -2753,14 +2767,10 @@ struct do_for_each_reflog_help { void *cb_data; }; -static int do_for_each_reflog_helper(const char *refname, - const char *referent UNUSED, - const struct object_id *oid UNUSED, - int flags UNUSED, - void *cb_data) +static int do_for_each_reflog_helper(const struct reference *ref, void *cb_data) { struct do_for_each_reflog_help *hp = cb_data; - return hp->fn(refname, hp->cb_data); + return hp->fn(ref->name, hp->cb_data); } int refs_for_each_reflog(struct ref_store *refs, each_reflog_fn fn, void *cb_data) @@ -2976,25 +2986,24 @@ struct migration_data { uint64_t index; }; -static int migrate_one_ref(const char *refname, const char *referent UNUSED, const struct object_id *oid, - int flags, void *cb_data) +static int migrate_one_ref(const struct reference *ref, void *cb_data) { struct migration_data *data = cb_data; struct strbuf symref_target = STRBUF_INIT; int ret; - if (flags & REF_ISSYMREF) { - ret = refs_read_symbolic_ref(data->old_refs, refname, &symref_target); + if (ref->flags & REF_ISSYMREF) { + ret = refs_read_symbolic_ref(data->old_refs, ref->name, &symref_target); if (ret < 0) goto done; - ret = ref_transaction_update(data->transaction, refname, NULL, null_oid(the_hash_algo), + ret = ref_transaction_update(data->transaction, ref->name, NULL, null_oid(the_hash_algo), symref_target.buf, NULL, REF_SKIP_CREATE_REFLOG | REF_NO_DEREF, NULL, data->errbuf); if (ret < 0) goto done; } else { - ret = ref_transaction_create(data->transaction, refname, oid, NULL, + ret = ref_transaction_create(data->transaction, ref->name, ref->oid, NULL, REF_SKIP_CREATE_REFLOG | REF_SKIP_OID_VERIFICATION, NULL, data->errbuf); if (ret < 0) diff --git a/refs.h b/refs.h index 4e6bd63aa86c54..68d235438c2b32 100644 --- a/refs.h +++ b/refs.h @@ -355,14 +355,32 @@ struct ref_transaction; */ #define REF_BAD_NAME 0x08 +/* A reference passed to `for_each_ref()`-style callbacks. */ +struct reference { + /* The fully-qualified name of the reference. */ + const char *name; + + /* The target of a symbolic ref. `NULL` for direct references. */ + const char *target; + + /* + * The object ID of a reference. Either the direct object ID or the + * resolved object ID in the case of a symbolic ref. May be the zero + * object ID in case the symbolic ref cannot be resolved. + */ + const struct object_id *oid; + + /* A bitfield of `REF_` flags. */ + int flags; +}; + /* * The signature for the callback function for the for_each_*() - * functions below. The memory pointed to by the refname and oid - * arguments is only guaranteed to be valid for the duration of a + * functions below. The memory pointed to by the `struct reference` + * argument is only guaranteed to be valid for the duration of a * single callback invocation. */ -typedef int each_ref_fn(const char *refname, const char *referent, - const struct object_id *oid, int flags, void *cb_data); +typedef int each_ref_fn(const struct reference *ref, void *cb_data); /* * The following functions invoke the specified callback function for diff --git a/refs/files-backend.c b/refs/files-backend.c index 8d7007f4aaa9da..eb3142f8f2dd32 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -3150,14 +3150,11 @@ static int parse_and_write_reflog(struct files_ref_store *refs, return 0; } -static int ref_present(const char *refname, const char *referent UNUSED, - const struct object_id *oid UNUSED, - int flags UNUSED, - void *cb_data) +static int ref_present(const struct reference *ref, void *cb_data) { struct string_list *affected_refnames = cb_data; - return string_list_has_string(affected_refnames, refname); + return string_list_has_string(affected_refnames, ref->name); } static int files_transaction_finish_initial(struct files_ref_store *refs, diff --git a/refs/iterator.c b/refs/iterator.c index 17ef841d8a3013..7f2e718f1c9107 100644 --- a/refs/iterator.c +++ b/refs/iterator.c @@ -476,7 +476,14 @@ int do_for_each_ref_iterator(struct ref_iterator *iter, current_ref_iter = iter; while ((ok = ref_iterator_advance(iter)) == ITER_OK) { - retval = fn(iter->refname, iter->referent, iter->oid, iter->flags, cb_data); + struct reference ref = { + .name = iter->refname, + .target = iter->referent, + .oid = iter->oid, + .flags = iter->flags, + }; + + retval = fn(&ref, cb_data); if (retval) goto out; } diff --git a/remote.c b/remote.c index df9675cd330ed1..59b371512084eb 100644 --- a/remote.c +++ b/remote.c @@ -2315,21 +2315,19 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb, return 1; } -static int one_local_ref(const char *refname, const char *referent UNUSED, const struct object_id *oid, - int flag UNUSED, - void *cb_data) +static int one_local_ref(const struct reference *ref, void *cb_data) { struct ref ***local_tail = cb_data; - struct ref *ref; + struct ref *local_ref; /* we already know it starts with refs/ to get here */ - if (check_refname_format(refname + 5, 0)) + if (check_refname_format(ref->name + 5, 0)) return 0; - ref = alloc_ref(refname); - oidcpy(&ref->new_oid, oid); - **local_tail = ref; - *local_tail = &ref->next; + local_ref = alloc_ref(ref->name); + oidcpy(&local_ref->new_oid, ref->oid); + **local_tail = local_ref; + *local_tail = &local_ref->next; return 0; } @@ -2402,15 +2400,14 @@ struct stale_heads_info { struct refspec *rs; }; -static int get_stale_heads_cb(const char *refname, const char *referent UNUSED, const struct object_id *oid, - int flags, void *cb_data) +static int get_stale_heads_cb(const struct reference *ref, void *cb_data) { struct stale_heads_info *info = cb_data; struct string_list matches = STRING_LIST_INIT_DUP; struct refspec_item query; int i, stale = 1; memset(&query, 0, sizeof(struct refspec_item)); - query.dst = (char *)refname; + query.dst = (char *)ref->name; refspec_find_all_matches(info->rs, &query, &matches); if (matches.nr == 0) @@ -2423,7 +2420,7 @@ static int get_stale_heads_cb(const char *refname, const char *referent UNUSED, * overlapping refspecs, we need to go over all of the * matching refs. */ - if (flags & REF_ISSYMREF) + if (ref->flags & REF_ISSYMREF) goto clean_exit; for (i = 0; stale && i < matches.nr; i++) @@ -2431,8 +2428,8 @@ static int get_stale_heads_cb(const char *refname, const char *referent UNUSED, stale = 0; if (stale) { - struct ref *ref = make_linked_ref(refname, &info->stale_refs_tail); - oidcpy(&ref->new_oid, oid); + struct ref *linked_ref = make_linked_ref(ref->name, &info->stale_refs_tail); + oidcpy(&linked_ref->new_oid, ref->oid); } clean_exit: diff --git a/repack-midx.c b/repack-midx.c index 6f6202c5bccd89..349f7e20b53f25 100644 --- a/repack-midx.c +++ b/repack-midx.c @@ -16,25 +16,23 @@ struct midx_snapshot_ref_data { int preferred; }; -static int midx_snapshot_ref_one(const char *refname UNUSED, - const char *referent UNUSED, - const struct object_id *oid, - int flag UNUSED, void *_data) +static int midx_snapshot_ref_one(const struct reference *ref, void *_data) { struct midx_snapshot_ref_data *data = _data; + const struct object_id *maybe_peeled = ref->oid; struct object_id peeled; - if (!peel_iterated_oid(data->repo, oid, &peeled)) - oid = &peeled; + if (!peel_iterated_oid(data->repo, ref->oid, &peeled)) + maybe_peeled = &peeled; - if (oidset_insert(&data->seen, oid)) + if (oidset_insert(&data->seen, maybe_peeled)) return 0; /* already seen */ - if (odb_read_object_info(data->repo->objects, oid, NULL) != OBJ_COMMIT) + if (odb_read_object_info(data->repo->objects, maybe_peeled, NULL) != OBJ_COMMIT) return 0; fprintf(data->f->fp, "%s%s\n", data->preferred ? "+" : "", - oid_to_hex(oid)); + oid_to_hex(maybe_peeled)); return 0; } diff --git a/replace-object.c b/replace-object.c index 3eae0510745eae..03d0f1f083bed9 100644 --- a/replace-object.c +++ b/replace-object.c @@ -8,31 +8,27 @@ #include "repository.h" #include "commit.h" -static int register_replace_ref(const char *refname, - const char *referent UNUSED, - const struct object_id *oid, - int flag UNUSED, - void *cb_data) +static int register_replace_ref(const struct reference *ref, void *cb_data) { struct repository *r = cb_data; /* Get sha1 from refname */ - const char *slash = strrchr(refname, '/'); - const char *hash = slash ? slash + 1 : refname; + const char *slash = strrchr(ref->name, '/'); + const char *hash = slash ? slash + 1 : ref->name; struct replace_object *repl_obj = xmalloc(sizeof(*repl_obj)); if (get_oid_hex_algop(hash, &repl_obj->original.oid, r->hash_algo)) { free(repl_obj); - warning(_("bad replace ref name: %s"), refname); + warning(_("bad replace ref name: %s"), ref->name); return 0; } /* Copy sha1 from the read ref */ - oidcpy(&repl_obj->replacement, oid); + oidcpy(&repl_obj->replacement, ref->oid); /* Register new object */ if (oidmap_put(&r->objects->replace_map, repl_obj)) - die(_("duplicate replace ref: %s"), refname); + die(_("duplicate replace ref: %s"), ref->name); return 0; } diff --git a/revision.c b/revision.c index cf5e6c1ec9e4e1..5f0850ae5c9c1a 100644 --- a/revision.c +++ b/revision.c @@ -1644,19 +1644,17 @@ struct all_refs_cb { struct worktree *wt; }; -static int handle_one_ref(const char *path, const char *referent UNUSED, const struct object_id *oid, - int flag UNUSED, - void *cb_data) +static int handle_one_ref(const struct reference *ref, void *cb_data) { struct all_refs_cb *cb = cb_data; struct object *object; - if (ref_excluded(&cb->all_revs->ref_excludes, path)) + if (ref_excluded(&cb->all_revs->ref_excludes, ref->name)) return 0; - object = get_reference(cb->all_revs, path, oid, cb->all_flags); - add_rev_cmdline(cb->all_revs, object, path, REV_CMD_REF, cb->all_flags); - add_pending_object(cb->all_revs, object, path); + object = get_reference(cb->all_revs, ref->name, ref->oid, cb->all_flags); + add_rev_cmdline(cb->all_revs, object, ref->name, REV_CMD_REF, cb->all_flags); + add_pending_object(cb->all_revs, object, ref->name); return 0; } diff --git a/server-info.c b/server-info.c index 1d33de821e9f5e..0a07c722e8bfe5 100644 --- a/server-info.c +++ b/server-info.c @@ -148,23 +148,21 @@ static int update_info_file(struct repository *r, char *path, return ret; } -static int add_info_ref(const char *path, const char *referent UNUSED, const struct object_id *oid, - int flag UNUSED, - void *cb_data) +static int add_info_ref(const struct reference *ref, void *cb_data) { struct update_info_ctx *uic = cb_data; - struct object *o = parse_object(uic->repo, oid); + struct object *o = parse_object(uic->repo, ref->oid); if (!o) return -1; - if (uic_printf(uic, "%s %s\n", oid_to_hex(oid), path) < 0) + if (uic_printf(uic, "%s %s\n", oid_to_hex(ref->oid), ref->name) < 0) return -1; if (o->type == OBJ_TAG) { - o = deref_tag(uic->repo, o, path, 0); + o = deref_tag(uic->repo, o, ref->name, 0); if (o) if (uic_printf(uic, "%s %s^{}\n", - oid_to_hex(&o->oid), path) < 0) + oid_to_hex(&o->oid), ref->name) < 0) return -1; } return 0; diff --git a/shallow.c b/shallow.c index d9cd4e219cb07d..55b9cd9d3f29f1 100644 --- a/shallow.c +++ b/shallow.c @@ -626,14 +626,10 @@ static void paint_down(struct paint_info *info, const struct object_id *oid, free(tmp); } -static int mark_uninteresting(const char *refname UNUSED, - const char *referent UNUSED, - const struct object_id *oid, - int flags UNUSED, - void *cb_data UNUSED) +static int mark_uninteresting(const struct reference *ref, void *cb_data UNUSED) { struct commit *commit = lookup_commit_reference_gently(the_repository, - oid, 1); + ref->oid, 1); if (!commit) return 0; commit->object.flags |= UNINTERESTING; @@ -742,16 +738,12 @@ struct commit_array { size_t nr, alloc; }; -static int add_ref(const char *refname UNUSED, - const char *referent UNUSED, - const struct object_id *oid, - int flags UNUSED, - void *cb_data) +static int add_ref(const struct reference *ref, void *cb_data) { struct commit_array *ca = cb_data; ALLOC_GROW(ca->commits, ca->nr + 1, ca->alloc); ca->commits[ca->nr] = lookup_commit_reference_gently(the_repository, - oid, 1); + ref->oid, 1); if (ca->commits[ca->nr]) ca->nr++; return 0; diff --git a/submodule.c b/submodule.c index 35c55155f7bf83..40a5c6fb9d1545 100644 --- a/submodule.c +++ b/submodule.c @@ -934,10 +934,7 @@ static void free_submodules_data(struct string_list *submodules) string_list_clear(submodules, 1); } -static int has_remote(const char *refname UNUSED, - const char *referent UNUSED, - const struct object_id *oid UNUSED, - int flags UNUSED, void *cb_data UNUSED) +static int has_remote(const struct reference *ref UNUSED, void *cb_data UNUSED) { return 1; } @@ -1255,13 +1252,10 @@ int push_unpushed_submodules(struct repository *r, return ret; } -static int append_oid_to_array(const char *ref UNUSED, - const char *referent UNUSED, - const struct object_id *oid, - int flags UNUSED, void *data) +static int append_oid_to_array(const struct reference *ref, void *data) { struct oid_array *array = data; - oid_array_append(array, oid); + oid_array_append(array, ref->oid); return 0; } diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c index 83b06d39a36235..b1215947c5e67a 100644 --- a/t/helper/test-ref-store.c +++ b/t/helper/test-ref-store.c @@ -154,10 +154,9 @@ static int cmd_rename_ref(struct ref_store *refs, const char **argv) return refs_rename_ref(refs, oldref, newref, logmsg); } -static int each_ref(const char *refname, const char *referent UNUSED, const struct object_id *oid, - int flags, void *cb_data UNUSED) +static int each_ref(const struct reference *ref, void *cb_data UNUSED) { - printf("%s %s 0x%x\n", oid_to_hex(oid), refname, flags); + printf("%s %s 0x%x\n", oid_to_hex(ref->oid), ref->name, ref->flags); return 0; } diff --git a/upload-pack.c b/upload-pack.c index 1e87ae95593063..0d563ae74e92be 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; } diff --git a/walker.c b/walker.c index 80737545172bbd..409b646578a3d4 100644 --- a/walker.c +++ b/walker.c @@ -226,14 +226,10 @@ static int interpret_target(struct walker *walker, char *target, struct object_i return -1; } -static int mark_complete(const char *path UNUSED, - const char *referent UNUSED, - const struct object_id *oid, - int flag UNUSED, - void *cb_data UNUSED) +static int mark_complete(const struct reference *ref, void *cb_data UNUSED) { struct commit *commit = lookup_commit_reference_gently(the_repository, - oid, 1); + ref->oid, 1); if (commit) { commit->object.flags |= COMPLETE; diff --git a/worktree.c b/worktree.c index a2a5f51f29fca2..9308389cb6f029 100644 --- a/worktree.c +++ b/worktree.c @@ -595,8 +595,15 @@ int other_head_refs(each_ref_fn fn, void *cb_data) if (refs_resolve_ref_unsafe(get_main_ref_store(the_repository), refname.buf, RESOLVE_REF_READING, - &oid, &flag)) - ret = fn(refname.buf, NULL, &oid, flag, cb_data); + &oid, &flag)) { + struct reference ref = { + .name = refname.buf, + .oid = &oid, + .flags = flag, + }; + + ret = fn(&ref, cb_data); + } if (ret) break; } From 89baa52da612dde6da031acfa2cb957d4297d544 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 23 Oct 2025 09:16:11 +0200 Subject: [PATCH 10/29] refs: introduce `.ref` field for the base iterator The base iterator has a couple of fields that tracks the name, target, object ID and flags for the current reference. Due to this design we have to create a new `struct reference` whenever we want to hand over that reference to the callback function, which is tedious and not very efficient. Convert the structure to instead contain a `struct reference` as member. This member is expected to be populated by the implementations of the iterator and is handed over to the callback directly. While at it, simplify `should_pack_ref()` to take a `struct reference` directly instead of passing its respective fields. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- refs.c | 8 +++---- refs/debug.c | 8 +++---- refs/files-backend.c | 47 ++++++++++++++++++----------------------- refs/iterator.c | 39 +++++++++++----------------------- refs/packed-backend.c | 46 ++++++++++++++++++++-------------------- refs/ref-cache.c | 10 ++++----- refs/refs-internal.h | 5 +---- refs/reftable-backend.c | 12 +++++------ 8 files changed, 75 insertions(+), 100 deletions(-) diff --git a/refs.c b/refs.c index 25f0579d610ddc..f96cf43b128a27 100644 --- a/refs.c +++ b/refs.c @@ -2327,8 +2327,8 @@ int refs_optimize(struct ref_store *refs, struct pack_refs_opts *opts) int peel_iterated_oid(struct repository *r, const struct object_id *base, struct object_id *peeled) { if (current_ref_iter && - (current_ref_iter->oid == base || - oideq(current_ref_iter->oid, base))) + (current_ref_iter->ref.oid == base || + oideq(current_ref_iter->ref.oid, base))) return ref_iterator_peel(current_ref_iter, peeled); return peel_object(r, base, peeled) ? -1 : 0; @@ -2703,7 +2703,7 @@ enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs while ((ok = ref_iterator_advance(iter)) == ITER_OK) { if (skip && - string_list_has_string(skip, iter->refname)) + string_list_has_string(skip, iter->ref.name)) continue; if (transaction && ref_transaction_maybe_set_rejected( @@ -2712,7 +2712,7 @@ enum ref_transaction_error refs_verify_refnames_available(struct ref_store *refs continue; strbuf_addf(err, _("'%s' exists; cannot create '%s'"), - iter->refname, refname); + iter->ref.name, refname); goto cleanup; } diff --git a/refs/debug.c b/refs/debug.c index 697adbd0dc3f65..67718bd1f49f1f 100644 --- a/refs/debug.c +++ b/refs/debug.c @@ -160,11 +160,9 @@ static int debug_ref_iterator_advance(struct ref_iterator *ref_iterator) trace_printf_key(&trace_refs, "iterator_advance: (%d)\n", res); else trace_printf_key(&trace_refs, "iterator_advance: %s (0)\n", - diter->iter->refname); + diter->iter->ref.name); - diter->base.refname = diter->iter->refname; - diter->base.oid = diter->iter->oid; - diter->base.flags = diter->iter->flags; + diter->base.ref = diter->iter->ref; return res; } @@ -185,7 +183,7 @@ static int debug_ref_iterator_peel(struct ref_iterator *ref_iterator, struct debug_ref_iterator *diter = (struct debug_ref_iterator *)ref_iterator; int res = diter->iter->vtable->peel(diter->iter, peeled); - trace_printf_key(&trace_refs, "iterator_peel: %s: %d\n", diter->iter->refname, res); + trace_printf_key(&trace_refs, "iterator_peel: %s: %d\n", diter->iter->ref.name, res); return res; } diff --git a/refs/files-backend.c b/refs/files-backend.c index eb3142f8f2dd32..fac53fa052dd22 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -961,26 +961,23 @@ static int files_ref_iterator_advance(struct ref_iterator *ref_iterator) while ((ok = ref_iterator_advance(iter->iter0)) == ITER_OK) { if (iter->flags & DO_FOR_EACH_PER_WORKTREE_ONLY && - parse_worktree_ref(iter->iter0->refname, NULL, NULL, + parse_worktree_ref(iter->iter0->ref.name, NULL, NULL, NULL) != REF_WORKTREE_CURRENT) continue; if ((iter->flags & DO_FOR_EACH_OMIT_DANGLING_SYMREFS) && - (iter->iter0->flags & REF_ISSYMREF) && - (iter->iter0->flags & REF_ISBROKEN)) + (iter->iter0->ref.flags & REF_ISSYMREF) && + (iter->iter0->ref.flags & REF_ISBROKEN)) continue; if (!(iter->flags & DO_FOR_EACH_INCLUDE_BROKEN) && - !ref_resolves_to_object(iter->iter0->refname, + !ref_resolves_to_object(iter->iter0->ref.name, iter->repo, - iter->iter0->oid, - iter->iter0->flags)) + iter->iter0->ref.oid, + iter->iter0->ref.flags)) continue; - iter->base.refname = iter->iter0->refname; - iter->base.oid = iter->iter0->oid; - iter->base.flags = iter->iter0->flags; - iter->base.referent = iter->iter0->referent; + iter->base.ref = iter->iter0->ref; return ITER_OK; } @@ -1367,30 +1364,29 @@ static void prune_refs(struct files_ref_store *refs, struct ref_to_prune **refs_ * Return true if the specified reference should be packed. */ static int should_pack_ref(struct files_ref_store *refs, - const char *refname, - const struct object_id *oid, unsigned int ref_flags, + const struct reference *ref, struct pack_refs_opts *opts) { struct string_list_item *item; /* Do not pack per-worktree refs: */ - if (parse_worktree_ref(refname, NULL, NULL, NULL) != + if (parse_worktree_ref(ref->name, NULL, NULL, NULL) != REF_WORKTREE_SHARED) return 0; /* Do not pack symbolic refs: */ - if (ref_flags & REF_ISSYMREF) + if (ref->flags & REF_ISSYMREF) return 0; /* Do not pack broken refs: */ - if (!ref_resolves_to_object(refname, refs->base.repo, oid, ref_flags)) + if (!ref_resolves_to_object(ref->name, refs->base.repo, ref->oid, ref->flags)) return 0; - if (ref_excluded(opts->exclusions, refname)) + if (ref_excluded(opts->exclusions, ref->name)) return 0; for_each_string_list_item(item, opts->includes) - if (!wildmatch(item->string, refname, 0)) + if (!wildmatch(item->string, ref->name, 0)) return 1; return 0; @@ -1443,8 +1439,7 @@ static int should_pack_refs(struct files_ref_store *refs, iter = cache_ref_iterator_begin(get_loose_ref_cache(refs, 0), NULL, refs->base.repo, 0); while ((ret = ref_iterator_advance(iter)) == ITER_OK) { - if (should_pack_ref(refs, iter->refname, iter->oid, - iter->flags, opts)) + if (should_pack_ref(refs, &iter->ref, opts)) refcount++; if (refcount >= limit) { ref_iterator_free(iter); @@ -1489,24 +1484,24 @@ static int files_pack_refs(struct ref_store *ref_store, * in the packed ref cache. If the reference should be * pruned, also add it to refs_to_prune. */ - if (!should_pack_ref(refs, iter->refname, iter->oid, iter->flags, opts)) + if (!should_pack_ref(refs, &iter->ref, opts)) continue; /* * Add a reference creation for this reference to the * packed-refs transaction: */ - if (ref_transaction_update(transaction, iter->refname, - iter->oid, NULL, NULL, NULL, + if (ref_transaction_update(transaction, iter->ref.name, + iter->ref.oid, NULL, NULL, NULL, REF_NO_DEREF, NULL, &err)) die("failure preparing to create packed reference %s: %s", - iter->refname, err.buf); + iter->ref.name, err.buf); /* Schedule the loose reference for pruning if requested. */ if ((opts->flags & PACK_REFS_PRUNE)) { struct ref_to_prune *n; - FLEX_ALLOC_STR(n, name, iter->refname); - oidcpy(&n->oid, iter->oid); + FLEX_ALLOC_STR(n, name, iter->ref.name); + oidcpy(&n->oid, iter->ref.oid); n->next = refs_to_prune; refs_to_prune = n; } @@ -2379,7 +2374,7 @@ static int files_reflog_iterator_advance(struct ref_iterator *ref_iterator) REFNAME_ALLOW_ONELEVEL)) continue; - iter->base.refname = diter->relative_path; + iter->base.ref.name = diter->relative_path; return ITER_OK; } diff --git a/refs/iterator.c b/refs/iterator.c index 7f2e718f1c9107..fe5980e1b6c96b 100644 --- a/refs/iterator.c +++ b/refs/iterator.c @@ -41,10 +41,7 @@ void base_ref_iterator_init(struct ref_iterator *iter, struct ref_iterator_vtable *vtable) { iter->vtable = vtable; - iter->refname = NULL; - iter->referent = NULL; - iter->oid = NULL; - iter->flags = 0; + memset(&iter->ref, 0, sizeof(iter->ref)); } struct empty_ref_iterator { @@ -127,8 +124,8 @@ enum iterator_selection ref_iterator_select(struct ref_iterator *iter_worktree, * latter. */ if (iter_worktree) { - int cmp = strcmp(iter_worktree->refname, - iter_common->refname); + int cmp = strcmp(iter_worktree->ref.name, + iter_common->ref.name); if (cmp < 0) return ITER_SELECT_0; else if (!cmp) @@ -139,7 +136,7 @@ enum iterator_selection ref_iterator_select(struct ref_iterator *iter_worktree, * We now know that the lexicographically-next ref is a common * ref. When the common ref is a shared one we return it. */ - if (parse_worktree_ref(iter_common->refname, NULL, NULL, + if (parse_worktree_ref(iter_common->ref.name, NULL, NULL, NULL) == REF_WORKTREE_SHARED) return ITER_SELECT_1; @@ -212,10 +209,7 @@ static int merge_ref_iterator_advance(struct ref_iterator *ref_iterator) } if (selection & ITER_YIELD_CURRENT) { - iter->base.referent = (*iter->current)->referent; - iter->base.refname = (*iter->current)->refname; - iter->base.oid = (*iter->current)->oid; - iter->base.flags = (*iter->current)->flags; + iter->base.ref = (*iter->current)->ref; return ITER_OK; } } @@ -313,7 +307,7 @@ static enum iterator_selection overlay_iterator_select( else if (!front) return ITER_SELECT_1; - cmp = strcmp(front->refname, back->refname); + cmp = strcmp(front->ref.name, back->ref.name); if (cmp < 0) return ITER_SELECT_0; @@ -371,7 +365,7 @@ static int prefix_ref_iterator_advance(struct ref_iterator *ref_iterator) int ok; while ((ok = ref_iterator_advance(iter->iter0)) == ITER_OK) { - int cmp = compare_prefix(iter->iter0->refname, iter->prefix); + int cmp = compare_prefix(iter->iter0->ref.name, iter->prefix); if (cmp < 0) continue; /* @@ -382,6 +376,8 @@ static int prefix_ref_iterator_advance(struct ref_iterator *ref_iterator) if (cmp > 0) return ITER_DONE; + iter->base.ref = iter->iter0->ref; + if (iter->trim) { /* * It is nonsense to trim off characters that @@ -392,15 +388,11 @@ static int prefix_ref_iterator_advance(struct ref_iterator *ref_iterator) * one character left in the refname after * trimming, report it as a bug: */ - if (strlen(iter->iter0->refname) <= iter->trim) + if (strlen(iter->base.ref.name) <= iter->trim) BUG("attempt to trim too many characters"); - iter->base.refname = iter->iter0->refname + iter->trim; - } else { - iter->base.refname = iter->iter0->refname; + iter->base.ref.name += iter->trim; } - iter->base.oid = iter->iter0->oid; - iter->base.flags = iter->iter0->flags; return ITER_OK; } @@ -476,14 +468,7 @@ int do_for_each_ref_iterator(struct ref_iterator *iter, current_ref_iter = iter; while ((ok = ref_iterator_advance(iter)) == ITER_OK) { - struct reference ref = { - .name = iter->refname, - .target = iter->referent, - .oid = iter->oid, - .flags = iter->flags, - }; - - retval = fn(&ref, cb_data); + retval = fn(&iter->ref, cb_data); if (retval) goto out; } diff --git a/refs/packed-backend.c b/refs/packed-backend.c index a8c22a0a7ff8cd..7987acdc96a14b 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -908,7 +908,7 @@ static int next_record(struct packed_ref_iterator *iter) if (iter->pos == iter->eof) return ITER_DONE; - iter->base.flags = REF_ISPACKED; + iter->base.ref.flags = REF_ISPACKED; p = iter->pos; if (iter->eof - p < snapshot_hexsz(iter->snapshot) + 2 || @@ -923,22 +923,22 @@ static int next_record(struct packed_ref_iterator *iter) iter->pos, iter->eof - iter->pos); strbuf_add(&iter->refname_buf, p, eol - p); - iter->base.refname = iter->refname_buf.buf; + iter->base.ref.name = iter->refname_buf.buf; if (refname_contains_nul(&iter->refname_buf)) - die("packed refname contains embedded NULL: %s", iter->base.refname); + die("packed refname contains embedded NULL: %s", iter->base.ref.name); - if (check_refname_format(iter->base.refname, REFNAME_ALLOW_ONELEVEL)) { - if (!refname_is_safe(iter->base.refname)) + if (check_refname_format(iter->base.ref.name, REFNAME_ALLOW_ONELEVEL)) { + if (!refname_is_safe(iter->base.ref.name)) die("packed refname is dangerous: %s", - iter->base.refname); + iter->base.ref.name); oidclr(&iter->oid, iter->repo->hash_algo); - iter->base.flags |= REF_BAD_NAME | REF_ISBROKEN; + iter->base.ref.flags |= REF_BAD_NAME | REF_ISBROKEN; } if (iter->snapshot->peeled == PEELED_FULLY || (iter->snapshot->peeled == PEELED_TAGS && - starts_with(iter->base.refname, "refs/tags/"))) - iter->base.flags |= REF_KNOWS_PEELED; + starts_with(iter->base.ref.name, "refs/tags/"))) + iter->base.ref.flags |= REF_KNOWS_PEELED; iter->pos = eol + 1; @@ -956,11 +956,11 @@ static int next_record(struct packed_ref_iterator *iter) * definitely know the value of *this* reference. But * we suppress it if the reference is broken: */ - if ((iter->base.flags & REF_ISBROKEN)) { + if ((iter->base.ref.flags & REF_ISBROKEN)) { oidclr(&iter->peeled, iter->repo->hash_algo); - iter->base.flags &= ~REF_KNOWS_PEELED; + iter->base.ref.flags &= ~REF_KNOWS_PEELED; } else { - iter->base.flags |= REF_KNOWS_PEELED; + iter->base.ref.flags |= REF_KNOWS_PEELED; } } else { oidclr(&iter->peeled, iter->repo->hash_algo); @@ -976,15 +976,15 @@ static int packed_ref_iterator_advance(struct ref_iterator *ref_iterator) int ok; while ((ok = next_record(iter)) == ITER_OK) { - const char *refname = iter->base.refname; + const char *refname = iter->base.ref.name; const char *prefix = iter->prefix; if (iter->flags & DO_FOR_EACH_PER_WORKTREE_ONLY && - !is_per_worktree_ref(iter->base.refname)) + !is_per_worktree_ref(iter->base.ref.name)) continue; if (!(iter->flags & DO_FOR_EACH_INCLUDE_BROKEN) && - !ref_resolves_to_object(iter->base.refname, iter->repo, + !ref_resolves_to_object(iter->base.ref.name, iter->repo, &iter->oid, iter->flags)) continue; @@ -1033,10 +1033,10 @@ static int packed_ref_iterator_peel(struct ref_iterator *ref_iterator, struct packed_ref_iterator *iter = (struct packed_ref_iterator *)ref_iterator; - if ((iter->base.flags & REF_KNOWS_PEELED)) { + if ((iter->base.ref.flags & REF_KNOWS_PEELED)) { oidcpy(peeled, &iter->peeled); return is_null_oid(&iter->peeled) ? -1 : 0; - } else if ((iter->base.flags & (REF_ISBROKEN | REF_ISSYMREF))) { + } else if ((iter->base.ref.flags & (REF_ISBROKEN | REF_ISSYMREF))) { return -1; } else { return peel_object(iter->repo, &iter->oid, peeled) ? -1 : 0; @@ -1194,7 +1194,7 @@ static struct ref_iterator *packed_ref_iterator_begin( iter->snapshot = snapshot; acquire_snapshot(snapshot); strbuf_init(&iter->refname_buf, 0); - iter->base.oid = &iter->oid; + iter->base.ref.oid = &iter->oid; iter->repo = ref_store->repo; iter->flags = flags; @@ -1436,7 +1436,7 @@ static enum ref_transaction_error write_with_updates(struct packed_ref_store *re if (!iter) cmp = +1; else - cmp = strcmp(iter->refname, update->refname); + cmp = strcmp(iter->ref.name, update->refname); } if (!cmp) { @@ -1459,11 +1459,11 @@ static enum ref_transaction_error write_with_updates(struct packed_ref_store *re } goto error; - } else if (!oideq(&update->old_oid, iter->oid)) { + } else if (!oideq(&update->old_oid, iter->ref.oid)) { strbuf_addf(err, "cannot update ref '%s': " "is at %s but expected %s", update->refname, - oid_to_hex(iter->oid), + oid_to_hex(iter->ref.oid), oid_to_hex(&update->old_oid)); ret = REF_TRANSACTION_ERROR_INCORRECT_OLD_VALUE; @@ -1527,8 +1527,8 @@ static enum ref_transaction_error write_with_updates(struct packed_ref_store *re struct object_id peeled; int peel_error = ref_iterator_peel(iter, &peeled); - if (write_packed_entry(out, iter->refname, - iter->oid, + if (write_packed_entry(out, iter->ref.name, + iter->ref.oid, peel_error ? NULL : &peeled)) goto write_error; diff --git a/refs/ref-cache.c b/refs/ref-cache.c index e5e5df16d85e40..f1abc39624166e 100644 --- a/refs/ref-cache.c +++ b/refs/ref-cache.c @@ -425,10 +425,10 @@ static int cache_ref_iterator_advance(struct ref_iterator *ref_iterator) level->prefix_state = entry_prefix_state; level->index = -1; } else { - iter->base.refname = entry->name; - iter->base.referent = entry->u.value.referent; - iter->base.oid = &entry->u.value.oid; - iter->base.flags = entry->flag; + iter->base.ref.name = entry->name; + iter->base.ref.target = entry->u.value.referent; + iter->base.ref.oid = &entry->u.value.oid; + iter->base.ref.flags = entry->flag; return ITER_OK; } } @@ -550,7 +550,7 @@ static int cache_ref_iterator_peel(struct ref_iterator *ref_iterator, { struct cache_ref_iterator *iter = (struct cache_ref_iterator *)ref_iterator; - return peel_object(iter->repo, ref_iterator->oid, peeled) ? -1 : 0; + return peel_object(iter->repo, ref_iterator->ref.oid, peeled) ? -1 : 0; } static void cache_ref_iterator_release(struct ref_iterator *ref_iterator) diff --git a/refs/refs-internal.h b/refs/refs-internal.h index 4ef3bd75c6ae55..ed749d16572dac 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -249,10 +249,7 @@ const char *find_descendant_ref(const char *dirname, */ struct ref_iterator { struct ref_iterator_vtable *vtable; - const char *refname; - const char *referent; - const struct object_id *oid; - unsigned int flags; + struct reference ref; }; /* diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index d4b792862024fc..0e47986cb5b699 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -704,10 +704,10 @@ static int reftable_ref_iterator_advance(struct ref_iterator *ref_iterator) &iter->oid, flags)) continue; - iter->base.refname = iter->ref.refname; - iter->base.referent = referent; - iter->base.oid = &iter->oid; - iter->base.flags = flags; + iter->base.ref.name = iter->ref.refname; + iter->base.ref.target = referent; + iter->base.ref.oid = &iter->oid; + iter->base.ref.flags = flags; break; } @@ -828,7 +828,7 @@ static struct reftable_ref_iterator *ref_iterator_for_stack(struct reftable_ref_ iter = xcalloc(1, sizeof(*iter)); base_ref_iterator_init(&iter->base, &reftable_ref_iterator_vtable); - iter->base.oid = &iter->oid; + iter->base.ref.oid = &iter->oid; iter->flags = flags; iter->refs = refs; iter->exclude_patterns = filter_exclude_patterns(exclude_patterns); @@ -2072,7 +2072,7 @@ static int reftable_reflog_iterator_advance(struct ref_iterator *ref_iterator) strbuf_reset(&iter->last_name); strbuf_addstr(&iter->last_name, iter->log.refname); - iter->base.refname = iter->log.refname; + iter->base.ref.name = iter->log.refname; break; } From 4cea0422879f6a64c0f7ad0ddac6d43897a53e94 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 23 Oct 2025 09:16:12 +0200 Subject: [PATCH 11/29] refs: fully reset `struct ref_iterator::ref` on iteration With the introduction of the `struct ref_iterator::ref` field it now is a whole lot easier to introduce new fields that become accessible to the caller without having to adapt every single callsite. But there's a downside: when a new field is introduced we always have to adapt all backends to set that field. This isn't something we can avoid in the general case: when the new field is expected to be populated by all backends we of course cannot avoid doing so. But new fields may be entirely optional, in which case we'd still have such churn. And furthermore, it is very easy right now to leak state from a previous iteration into the next iteration. Address this issue by ensuring that the reference backends all fully reset the field on every single iteration. This ensures that no state from previous iterations can leak into the next one. And it ensures that any newly introduced fields will be zeroed out by default. Note that we don't have to explicitly adapt the "files" backend, as it uses the `cache_ref_iterator` internally. Furthermore, other "wrapping" iterators like for example the `prefix_ref_iterator` copy around the whole reference, so these don't need to be adapted either. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- refs/packed-backend.c | 3 ++- refs/ref-cache.c | 1 + refs/reftable-backend.c | 1 + 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 7987acdc96a14b..711e07f8326c6b 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -882,6 +882,7 @@ static int next_record(struct packed_ref_iterator *iter) { const char *p, *eol; + memset(&iter->base.ref, 0, sizeof(iter->base.ref)); strbuf_reset(&iter->refname_buf); /* @@ -916,6 +917,7 @@ static int next_record(struct packed_ref_iterator *iter) !isspace(*p++)) die_invalid_line(iter->snapshot->refs->path, iter->pos, iter->eof - iter->pos); + iter->base.ref.oid = &iter->oid; eol = memchr(p, '\n', iter->eof - p); if (!eol) @@ -1194,7 +1196,6 @@ static struct ref_iterator *packed_ref_iterator_begin( iter->snapshot = snapshot; acquire_snapshot(snapshot); strbuf_init(&iter->refname_buf, 0); - iter->base.ref.oid = &iter->oid; iter->repo = ref_store->repo; iter->flags = flags; diff --git a/refs/ref-cache.c b/refs/ref-cache.c index f1abc39624166e..e427848879d61b 100644 --- a/refs/ref-cache.c +++ b/refs/ref-cache.c @@ -425,6 +425,7 @@ static int cache_ref_iterator_advance(struct ref_iterator *ref_iterator) level->prefix_state = entry_prefix_state; level->index = -1; } else { + memset(&iter->base.ref, 0, sizeof(iter->base.ref)); iter->base.ref.name = entry->name; iter->base.ref.target = entry->u.value.referent; iter->base.ref.oid = &entry->u.value.oid; diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index 0e47986cb5b699..728886eafd33bd 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -704,6 +704,7 @@ static int reftable_ref_iterator_advance(struct ref_iterator *ref_iterator) &iter->oid, flags)) continue; + memset(&iter->base.ref, 0, sizeof(iter->base.ref)); iter->base.ref.name = iter->ref.refname; iter->base.ref.target = referent; iter->base.ref.oid = &iter->oid; From eb2934d94b43388642ce4840994800310a6d3456 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 23 Oct 2025 09:16:13 +0200 Subject: [PATCH 12/29] refs: refactor reference status flags The reference flags encode information like whether or not a reference is a symbolic reference or whether it may be broken. This information is stored in a `int flags` bitfield, which is in conflict with our modern best practices; we tend to use an unsigned integer to store flags. Change the type of the field to be `unsigned`. While at it, refactor the individual flags to be part of an `enum` instead of using preprocessor defines. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- refs.h | 41 +++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/refs.h b/refs.h index 68d235438c2b32..4f0a685714fa3a 100644 --- a/refs.h +++ b/refs.h @@ -333,27 +333,28 @@ struct ref_transaction; * stored in ref_iterator::flags. Other bits are for internal use * only: */ +enum reference_status { + /* Reference is a symbolic reference. */ + REF_ISSYMREF = (1 << 0), -/* Reference is a symbolic reference. */ -#define REF_ISSYMREF 0x01 + /* Reference is a packed reference. */ + REF_ISPACKED = (1 << 1), -/* Reference is a packed reference. */ -#define REF_ISPACKED 0x02 - -/* - * Reference cannot be resolved to an object name: dangling symbolic - * reference (directly or indirectly), corrupt reference file, - * reference exists but name is bad, or symbolic reference refers to - * ill-formatted reference name. - */ -#define REF_ISBROKEN 0x04 + /* + * Reference cannot be resolved to an object name: dangling symbolic + * reference (directly or indirectly), corrupt reference file, + * reference exists but name is bad, or symbolic reference refers to + * ill-formatted reference name. + */ + REF_ISBROKEN = (1 << 2), -/* - * Reference name is not well formed. - * - * See git-check-ref-format(1) for the definition of well formed ref names. - */ -#define REF_BAD_NAME 0x08 + /* + * Reference name is not well formed. + * + * See git-check-ref-format(1) for the definition of well formed ref names. + */ + REF_BAD_NAME = (1 << 3), +}; /* A reference passed to `for_each_ref()`-style callbacks. */ struct reference { @@ -370,8 +371,8 @@ struct reference { */ const struct object_id *oid; - /* A bitfield of `REF_` flags. */ - int flags; + /* A bitfield of `enum reference_status` flags. */ + unsigned flags; }; /* From f89866163704528f1a6570e134853dbb99120e7c Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 23 Oct 2025 09:16:14 +0200 Subject: [PATCH 13/29] refs: expose peeled object ID via the iterator Both the "files" and "reftable" backend are able to store peeled values for tags in the respective formats. This allows for a more efficient lookup of the target object of such a tag without having to manually peel via the object database. The infrastructure to access these peeled object IDs is somewhat funky though. When iterating through objects, we store a pointer reference to the current iterator in a global variable. The callbacks invoked by that iterator are then expected to call `peel_iterated_oid()`, which checks whether the globally-stored iterator's current reference refers to the one handed into that function. If so, we ask the iterator to peel the object, otherwise we manually peel the object via the object database. Depending on global state like this is somewhat weird and also quite fragile. Introduce a new `struct reference::peeled_oid` field that can be populated by the reference backends. This field can be accessed via a new function `reference_get_peeled_oid()` that either uses that value, if set, or alternatively peels via the ODB. With this change we don't have to rely on global state anymore, but make the peeled object ID available to the callback functions directly. Adjust trivial callers that already have a `struct reference` available. Remaining callers will be adjusted in subsequent commits. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/describe.c | 2 +- builtin/gc.c | 2 +- builtin/pack-objects.c | 7 ++++--- commit-graph.c | 2 +- ls-refs.c | 2 +- midx-write.c | 2 +- pseudo-merge.c | 2 +- refs.c | 12 ++++++++++++ refs.h | 19 +++++++++++++++++++ refs/packed-backend.c | 1 + refs/reftable-backend.c | 5 +++++ repack-midx.c | 2 +- 12 files changed, 48 insertions(+), 10 deletions(-) diff --git a/builtin/describe.c b/builtin/describe.c index 79545350443c6c..443546aaac96f0 100644 --- a/builtin/describe.c +++ b/builtin/describe.c @@ -208,7 +208,7 @@ static int get_name(const struct reference *ref, void *cb_data UNUSED) } /* Is it annotated? */ - if (!peel_iterated_oid(the_repository, ref->oid, &peeled)) { + if (!reference_get_peeled_oid(the_repository, ref, &peeled)) { is_annotated = !oideq(ref->oid, &peeled); } else { oidcpy(&peeled, ref->oid); diff --git a/builtin/gc.c b/builtin/gc.c index 9de5de175f6a40..f0cf20d42389fe 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1109,7 +1109,7 @@ static int dfs_on_ref(const struct reference *ref, void *cb_data) struct commit_list *stack = NULL; struct commit *commit; - if (!peel_iterated_oid(the_repository, ref->oid, &peeled)) + if (!reference_get_peeled_oid(the_repository, ref, &peeled)) maybe_peeled = &peeled; if (odb_read_object_info(the_repository->objects, maybe_peeled, NULL) != OBJ_COMMIT) return 0; diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index 39633a0158e095..1613fecb669830 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -838,7 +838,7 @@ static int mark_tagged(const struct reference *ref, void *cb_data UNUSED) if (entry) entry->tagged = 1; - if (!peel_iterated_oid(the_repository, ref->oid, &peeled)) { + if (!reference_get_peeled_oid(the_repository, ref, &peeled)) { entry = packlist_find(&to_pack, &peeled); if (entry) entry->tagged = 1; @@ -3309,7 +3309,8 @@ static int add_ref_tag(const struct reference *ref, void *cb_data UNUSED) { struct object_id peeled; - if (!peel_iterated_oid(the_repository, ref->oid, &peeled) && obj_is_packed(&peeled)) + if (!reference_get_peeled_oid(the_repository, ref, &peeled) && + obj_is_packed(&peeled)) add_tag_chain(ref->oid); return 0; } @@ -4537,7 +4538,7 @@ static int mark_bitmap_preferred_tip(const struct reference *ref, void *data UNU struct object_id peeled; struct object *object; - if (!peel_iterated_oid(the_repository, ref->oid, &peeled)) + if (!reference_get_peeled_oid(the_repository, ref, &peeled)) maybe_peeled = &peeled; object = parse_object_or_die(the_repository, maybe_peeled, ref->name); diff --git a/commit-graph.c b/commit-graph.c index f91af416259c84..80be2ff2c39842 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -1857,7 +1857,7 @@ static int add_ref_to_set(const struct reference *ref, void *cb_data) struct object_id peeled; struct refs_cb_data *data = (struct refs_cb_data *)cb_data; - if (!peel_iterated_oid(data->repo, ref->oid, &peeled)) + if (!reference_get_peeled_oid(data->repo, ref, &peeled)) maybe_peeled = &peeled; if (odb_read_object_info(data->repo->objects, maybe_peeled, NULL) == OBJ_COMMIT) oidset_insert(data->commits, maybe_peeled); diff --git a/ls-refs.c b/ls-refs.c index 64d02723691466..8641281b86c55a 100644 --- a/ls-refs.c +++ b/ls-refs.c @@ -110,7 +110,7 @@ static int send_ref(const struct reference *ref, void *cb_data) if (data->peel && ref->oid) { struct object_id peeled; - if (!peel_iterated_oid(the_repository, ref->oid, &peeled)) + if (!reference_get_peeled_oid(the_repository, ref, &peeled)) strbuf_addf(&data->buf, " peeled:%s", oid_to_hex(&peeled)); } diff --git a/midx-write.c b/midx-write.c index f4dd875747a4b6..23e61cb0001428 100644 --- a/midx-write.c +++ b/midx-write.c @@ -709,7 +709,7 @@ static int add_ref_to_pending(const struct reference *ref, void *cb_data) return 0; } - if (!peel_iterated_oid(revs->repo, ref->oid, &peeled)) + if (!reference_get_peeled_oid(revs->repo, ref, &peeled)) maybe_peeled = &peeled; object = parse_object_or_die(revs->repo, maybe_peeled, ref->name); diff --git a/pseudo-merge.c b/pseudo-merge.c index 0abd51b42c185a..a2d5bd85f95ebf 100644 --- a/pseudo-merge.c +++ b/pseudo-merge.c @@ -230,7 +230,7 @@ static int find_pseudo_merge_group_for_ref(const struct reference *ref, void *_d uint32_t i; int has_bitmap; - if (!peel_iterated_oid(the_repository, ref->oid, &peeled)) + if (!reference_get_peeled_oid(the_repository, ref, &peeled)) maybe_peeled = &peeled; c = lookup_commit(the_repository, maybe_peeled); diff --git a/refs.c b/refs.c index f96cf43b128a27..1b1551f9814394 100644 --- a/refs.c +++ b/refs.c @@ -2334,6 +2334,18 @@ int peel_iterated_oid(struct repository *r, const struct object_id *base, struct return peel_object(r, base, peeled) ? -1 : 0; } +int reference_get_peeled_oid(struct repository *repo, + const struct reference *ref, + struct object_id *peeled_oid) +{ + if (ref->peeled_oid) { + oidcpy(peeled_oid, ref->peeled_oid); + return 0; + } + + return peel_object(repo, ref->oid, peeled_oid) ? -1 : 0; +} + int refs_update_symref(struct ref_store *refs, const char *ref, const char *target, const char *logmsg) { diff --git a/refs.h b/refs.h index 4f0a685714fa3a..886ed2c0f43b04 100644 --- a/refs.h +++ b/refs.h @@ -371,10 +371,29 @@ struct reference { */ const struct object_id *oid; + /* + * An optional peeled object ID. This field _may_ be set for tags in + * case the peeled value is present in the backend. Please refer to + * `reference_get_peeled_oid()`. + */ + const struct object_id *peeled_oid; + /* A bitfield of `enum reference_status` flags. */ unsigned flags; }; +/* + * Peel the tag to a non-tag commit. If present, this uses the peeled object ID + * exposed by the reference backend. Otherwise, the object is peeled via the + * object database, which is less efficient. + * + * Return `0` if the reference could be peeled, a negative error code + * otherwise. + */ +int reference_get_peeled_oid(struct repository *repo, + const struct reference *ref, + struct object_id *peeled_oid); + /* * The signature for the callback function for the for_each_*() * functions below. The memory pointed to by the `struct reference` diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 711e07f8326c6b..1fefefd54ed0e7 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -963,6 +963,7 @@ static int next_record(struct packed_ref_iterator *iter) iter->base.ref.flags &= ~REF_KNOWS_PEELED; } else { iter->base.ref.flags |= REF_KNOWS_PEELED; + iter->base.ref.peeled_oid = &iter->peeled; } } else { oidclr(&iter->peeled, iter->repo->hash_algo); diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index 728886eafd33bd..e214e120d77a5c 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -547,6 +547,7 @@ struct reftable_ref_iterator { struct reftable_iterator iter; struct reftable_ref_record ref; struct object_id oid; + struct object_id peeled_oid; char *prefix; size_t prefix_len; @@ -671,6 +672,8 @@ static int reftable_ref_iterator_advance(struct ref_iterator *ref_iterator) case REFTABLE_REF_VAL2: oidread(&iter->oid, iter->ref.value.val2.value, refs->base.repo->hash_algo); + oidread(&iter->peeled_oid, iter->ref.value.val2.target_value, + refs->base.repo->hash_algo); break; case REFTABLE_REF_SYMREF: referent = refs_resolve_ref_unsafe(&iter->refs->base, @@ -708,6 +711,8 @@ static int reftable_ref_iterator_advance(struct ref_iterator *ref_iterator) iter->base.ref.name = iter->ref.refname; iter->base.ref.target = referent; iter->base.ref.oid = &iter->oid; + if (iter->ref.value_type == REFTABLE_REF_VAL2) + iter->base.ref.peeled_oid = &iter->peeled_oid; iter->base.ref.flags = flags; break; diff --git a/repack-midx.c b/repack-midx.c index 349f7e20b53f25..74bdfa3a6e913f 100644 --- a/repack-midx.c +++ b/repack-midx.c @@ -22,7 +22,7 @@ static int midx_snapshot_ref_one(const struct reference *ref, void *_data) const struct object_id *maybe_peeled = ref->oid; struct object_id peeled; - if (!peel_iterated_oid(data->repo, ref->oid, &peeled)) + if (!reference_get_peeled_oid(data->repo, ref, &peeled)) maybe_peeled = &peeled; if (oidset_insert(&data->seen, maybe_peeled)) From adecd5f0b6fdd40219d5503fdaf46aa8d36a4ff7 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 23 Oct 2025 09:16:15 +0200 Subject: [PATCH 14/29] 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(-) diff --git a/upload-pack.c b/upload-pack.c index 0d563ae74e92be..2d2b70cbf2dd0b 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 From 70b783c3a194746d8b747677615f33b94454146f Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 23 Oct 2025 09:16:16 +0200 Subject: [PATCH 15/29] ref-filter: propagate peeled object ID When queueing a reference in the "ref-filter" subsystem we end up creating a new ref array item that contains the reference's info. One bit of info that we always discard though is the peeled object ID, and because of that we are forced to use `peel_iterated_oid()`. Refactor the code to propagate the peeled object ID via the ref array, if available. This allows us to manually peel tags without having to go through the object database. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/ls-remote.c | 2 +- builtin/tag.c | 2 +- builtin/verify-tag.c | 2 +- ref-filter.c | 66 +++++++++++++++++++++++++------------------- ref-filter.h | 5 +++- 5 files changed, 45 insertions(+), 32 deletions(-) diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c index df09000b30de50..fe77829557f252 100644 --- a/builtin/ls-remote.c +++ b/builtin/ls-remote.c @@ -156,7 +156,7 @@ int cmd_ls_remote(int argc, continue; if (!tail_match(&pattern, ref->name)) continue; - item = ref_array_push(&ref_array, ref->name, &ref->old_oid); + item = ref_array_push(&ref_array, ref->name, &ref->old_oid, NULL); item->symref = xstrdup_or_null(ref->symref); } diff --git a/builtin/tag.c b/builtin/tag.c index f0665af3acdf6c..01eba90c5c7bb2 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -153,7 +153,7 @@ static int verify_tag(const char *name, const char *ref UNUSED, return -1; if (format->format) - pretty_print_ref(name, oid, format); + pretty_print_ref(name, oid, NULL, format); return 0; } diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c index cd6bc11095d01a..558121eaa1688e 100644 --- a/builtin/verify-tag.c +++ b/builtin/verify-tag.c @@ -67,7 +67,7 @@ int cmd_verify_tag(int argc, } if (format.format) - pretty_print_ref(name, &oid, &format); + pretty_print_ref(name, &oid, NULL, &format); } return had_error; } diff --git a/ref-filter.c b/ref-filter.c index 6837fa60a9b2b5..7fd8babec8f5bd 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -2578,8 +2578,15 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) * If it is a tag object, see if we use the peeled value. If we do, * grab the peeled OID. */ - if (need_tagged && peel_iterated_oid(the_repository, &obj->oid, &oi_deref.oid)) - die("bad tag"); + if (need_tagged) { + if (!is_null_oid(&ref->peeled_oid)) { + oidcpy(&oi_deref.oid, &ref->peeled_oid); + } else if (!peel_object(the_repository, &obj->oid, &oi_deref.oid)) { + /* We managed to peel the object ourselves. */ + } else { + die("bad tag"); + } + } return get_object(ref, 1, &obj, &oi_deref, err); } @@ -2807,12 +2814,15 @@ static int match_points_at(struct oid_array *points_at, * Callers can then fill in other struct members at their leisure. */ static struct ref_array_item *new_ref_array_item(const char *refname, - const struct object_id *oid) + const struct object_id *oid, + const struct object_id *peeled_oid) { struct ref_array_item *ref; FLEX_ALLOC_STR(ref, refname, refname); oidcpy(&ref->objectname, oid); + if (peeled_oid) + oidcpy(&ref->peeled_oid, peeled_oid); ref->rest = NULL; return ref; @@ -2826,9 +2836,10 @@ static void ref_array_append(struct ref_array *array, struct ref_array_item *ref struct ref_array_item *ref_array_push(struct ref_array *array, const char *refname, - const struct object_id *oid) + const struct object_id *oid, + const struct object_id *peeled_oid) { - struct ref_array_item *ref = new_ref_array_item(refname, oid); + struct ref_array_item *ref = new_ref_array_item(refname, oid, peeled_oid); ref_array_append(array, ref); return ref; } @@ -2871,25 +2882,25 @@ static int filter_ref_kind(struct ref_filter *filter, const char *refname) return ref_kind_from_refname(refname); } -static struct ref_array_item *apply_ref_filter(const char *refname, const char *referent, const struct object_id *oid, - int flag, struct ref_filter *filter) +static struct ref_array_item *apply_ref_filter(const struct reference *ref, + struct ref_filter *filter) { - struct ref_array_item *ref; + struct ref_array_item *item; struct commit *commit = NULL; unsigned int kind; - if (flag & REF_BAD_NAME) { - warning(_("ignoring ref with broken name %s"), refname); + if (ref->flags & REF_BAD_NAME) { + warning(_("ignoring ref with broken name %s"), ref->name); return NULL; } - if (flag & REF_ISBROKEN) { - warning(_("ignoring broken ref %s"), refname); + if (ref->flags & REF_ISBROKEN) { + warning(_("ignoring broken ref %s"), ref->name); return NULL; } /* Obtain the current ref kind from filter_ref_kind() and ignore unwanted refs. */ - kind = filter_ref_kind(filter, refname); + kind = filter_ref_kind(filter, ref->name); /* * Generally HEAD refs are printed with special description denoting a rebase, @@ -2902,13 +2913,13 @@ static struct ref_array_item *apply_ref_filter(const char *refname, const char * else if (!(kind & filter->kind)) return NULL; - if (!filter_pattern_match(filter, refname)) + if (!filter_pattern_match(filter, ref->name)) return NULL; - if (filter_exclude_match(filter, refname)) + if (filter_exclude_match(filter, ref->name)) return NULL; - if (filter->points_at.nr && !match_points_at(&filter->points_at, oid, refname)) + if (filter->points_at.nr && !match_points_at(&filter->points_at, ref->oid, ref->name)) return NULL; /* @@ -2918,7 +2929,7 @@ static struct ref_array_item *apply_ref_filter(const char *refname, const char * */ if (filter->reachable_from || filter->unreachable_from || filter->with_commit || filter->no_commit || filter->verbose) { - commit = lookup_commit_reference_gently(the_repository, oid, 1); + commit = lookup_commit_reference_gently(the_repository, ref->oid, 1); if (!commit) return NULL; /* We perform the filtering for the '--contains' option... */ @@ -2936,13 +2947,13 @@ static struct ref_array_item *apply_ref_filter(const char *refname, const char * * to do its job and the resulting list may yet to be pruned * by maxcount logic. */ - ref = new_ref_array_item(refname, oid); - ref->commit = commit; - ref->flag = flag; - ref->kind = kind; - ref->symref = xstrdup_or_null(referent); + item = new_ref_array_item(ref->name, ref->oid, ref->peeled_oid); + item->commit = commit; + item->flag = ref->flags; + item->kind = kind; + item->symref = xstrdup_or_null(ref->target); - return ref; + return item; } struct ref_filter_cbdata { @@ -2959,8 +2970,7 @@ static int filter_one(const struct reference *ref, void *cb_data) struct ref_filter_cbdata *ref_cbdata = cb_data; struct ref_array_item *item; - item = apply_ref_filter(ref->name, ref->target, ref->oid, - ref->flags, ref_cbdata->filter); + item = apply_ref_filter(ref, ref_cbdata->filter); if (item) ref_array_append(ref_cbdata->array, item); @@ -2997,8 +3007,7 @@ static int filter_and_format_one(const struct reference *ref, void *cb_data) struct ref_array_item *item; struct strbuf output = STRBUF_INIT, err = STRBUF_INIT; - item = apply_ref_filter(ref->name, ref->target, ref->oid, - ref->flags, ref_cbdata->filter); + item = apply_ref_filter(ref, ref_cbdata->filter); if (!item) return 0; @@ -3585,13 +3594,14 @@ void print_formatted_ref_array(struct ref_array *array, struct ref_format *forma } void pretty_print_ref(const char *name, const struct object_id *oid, + const struct object_id *peeled_oid, struct ref_format *format) { struct ref_array_item *ref_item; struct strbuf output = STRBUF_INIT; struct strbuf err = STRBUF_INIT; - ref_item = new_ref_array_item(name, oid); + ref_item = new_ref_array_item(name, oid, peeled_oid); ref_item->kind = ref_kind_from_refname(name); if (format_ref_array_item(ref_item, format, &output, &err)) die("%s", err.buf); diff --git a/ref-filter.h b/ref-filter.h index 235c60f79c9a0f..120221b47fa30d 100644 --- a/ref-filter.h +++ b/ref-filter.h @@ -41,6 +41,7 @@ enum ref_sorting_order { struct ref_array_item { struct object_id objectname; + struct object_id peeled_oid; const char *rest; int flag; unsigned int kind; @@ -187,6 +188,7 @@ void print_formatted_ref_array(struct ref_array *array, struct ref_format *forma * name must be a fully qualified refname. */ void pretty_print_ref(const char *name, const struct object_id *oid, + const struct object_id *peeled_oid, struct ref_format *format); /* @@ -195,7 +197,8 @@ void pretty_print_ref(const char *name, const struct object_id *oid, */ struct ref_array_item *ref_array_push(struct ref_array *array, const char *refname, - const struct object_id *oid); + const struct object_id *oid, + const struct object_id *peeled_oid); /* * If the provided format includes ahead-behind atoms, then compute the From feaaea4c123e6b94ebbdc2135278946ee9cc8eed Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 23 Oct 2025 09:16:17 +0200 Subject: [PATCH 16/29] builtin/show-ref: convert to use `reference_get_peeled_oid()` The git-show-ref(1) command has multiple different modes: - It knows to show all references matching a pattern. - It knows to list all references that are an exact match to whatever the user has provided. - It knows to check for reference existence. The first two commands use mostly the same infrastructure to print the references via `show_one()`. But while the former mode uses a proper iterator and thus has a `struct reference` available in its context, the latter calls `refs_read_ref()` and thus doesn't. Consequently, we cannot easily use `reference_get_peeled_oid()` to print the peeled value. Adapt the code so that we manually construct a `struct reference` when verifying refs. We wouldn't ever have the peeled value available anyway as we're not using an iterator here, so we can simply plug in the values we _do_ have. With this change we now have a `struct reference` available at both callsites of `show_one()` and can thus pass it, which allows us to use `reference_get_peeled_oid()` instead of `peel_iterated_oid()`. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/show-ref.c | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/builtin/show-ref.c b/builtin/show-ref.c index 4803b5e59865f6..4d4984e4e0c244 100644 --- a/builtin/show-ref.c +++ b/builtin/show-ref.c @@ -31,31 +31,31 @@ struct show_one_options { }; static void show_one(const struct show_one_options *opts, - const char *refname, const struct object_id *oid) + const struct reference *ref) { const char *hex; struct object_id peeled; - if (!odb_has_object(the_repository->objects, oid, + if (!odb_has_object(the_repository->objects, ref->oid, HAS_OBJECT_RECHECK_PACKED | HAS_OBJECT_FETCH_PROMISOR)) - die("git show-ref: bad ref %s (%s)", refname, - oid_to_hex(oid)); + die("git show-ref: bad ref %s (%s)", ref->name, + oid_to_hex(ref->oid)); if (opts->quiet) return; - hex = repo_find_unique_abbrev(the_repository, oid, opts->abbrev); + hex = repo_find_unique_abbrev(the_repository, ref->oid, opts->abbrev); if (opts->hash_only) printf("%s\n", hex); else - printf("%s %s\n", hex, refname); + printf("%s %s\n", hex, ref->name); if (!opts->deref_tags) return; - if (!peel_iterated_oid(the_repository, oid, &peeled)) { + if (!reference_get_peeled_oid(the_repository, ref, &peeled)) { hex = repo_find_unique_abbrev(the_repository, &peeled, opts->abbrev); - printf("%s %s^{}\n", hex, refname); + printf("%s %s^{}\n", hex, ref->name); } } @@ -93,7 +93,7 @@ static int show_ref(const struct reference *ref, void *cbdata) match: data->found_match++; - show_one(data->show_one_opts, ref->name, ref->oid); + show_one(data->show_one_opts, ref); return 0; } @@ -175,12 +175,18 @@ static int cmd_show_ref__verify(const struct show_one_options *show_one_opts, if ((starts_with(*refs, "refs/") || refname_is_safe(*refs)) && !refs_read_ref(get_main_ref_store(the_repository), *refs, &oid)) { - show_one(show_one_opts, *refs, &oid); - } - else if (!show_one_opts->quiet) + struct reference ref = { + .name = *refs, + .oid = &oid, + }; + + show_one(show_one_opts, &ref); + } else if (!show_one_opts->quiet) { die("'%s' - not a valid ref", *refs); - else + } else { return 1; + } + refs++; } From 5a5c7359f77ecd1bc4b0e172563161d602f131d3 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 23 Oct 2025 09:16:18 +0200 Subject: [PATCH 17/29] refs: drop `current_ref_iter` hack In preceding commits we have refactored all callers of `peel_iterated_oid()` to instead use `reference_get_peeled_oid()`. This allows us to thus get rid of the former function. Getting rid of that function is nice, but even nicer is that this also allows us to get rid of the `current_ref_iter` hack. This global variable tracked the currently-active ref iterator so that we can use it to peel an object ID. Now that the peeled object ID is propagated via `struct reference` though we don't have to depend on this hack anymore, which makes for a more robust and easier-to-understand infrastructure. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- refs.c | 10 ---------- refs/iterator.c | 5 ----- refs/refs-internal.h | 13 ------------- 3 files changed, 28 deletions(-) diff --git a/refs.c b/refs.c index 1b1551f9814394..9d8f0a9ca4a3a6 100644 --- a/refs.c +++ b/refs.c @@ -2324,16 +2324,6 @@ int refs_optimize(struct ref_store *refs, struct pack_refs_opts *opts) return refs->be->optimize(refs, opts); } -int peel_iterated_oid(struct repository *r, const struct object_id *base, struct object_id *peeled) -{ - if (current_ref_iter && - (current_ref_iter->ref.oid == base || - oideq(current_ref_iter->ref.oid, base))) - return ref_iterator_peel(current_ref_iter, peeled); - - return peel_object(r, base, peeled) ? -1 : 0; -} - int reference_get_peeled_oid(struct repository *repo, const struct reference *ref, struct object_id *peeled_oid) diff --git a/refs/iterator.c b/refs/iterator.c index fe5980e1b6c96b..072c6aacdb0341 100644 --- a/refs/iterator.c +++ b/refs/iterator.c @@ -458,15 +458,11 @@ struct ref_iterator *prefix_ref_iterator_begin(struct ref_iterator *iter0, return ref_iterator; } -struct ref_iterator *current_ref_iter = NULL; - int do_for_each_ref_iterator(struct ref_iterator *iter, each_ref_fn fn, void *cb_data) { int retval = 0, ok; - struct ref_iterator *old_ref_iter = current_ref_iter; - current_ref_iter = iter; while ((ok = ref_iterator_advance(iter)) == ITER_OK) { retval = fn(&iter->ref, cb_data); if (retval) @@ -474,7 +470,6 @@ int do_for_each_ref_iterator(struct ref_iterator *iter, } out: - current_ref_iter = old_ref_iter; if (ok == ITER_ERROR) retval = -1; ref_iterator_free(iter); diff --git a/refs/refs-internal.h b/refs/refs-internal.h index ed749d16572dac..f4f845bbeaf673 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -376,19 +376,6 @@ struct ref_iterator_vtable { ref_iterator_release_fn *release; }; -/* - * current_ref_iter is a performance hack: when iterating over - * references using the for_each_ref*() functions, current_ref_iter is - * set to the reference iterator before calling the callback function. - * If the callback function calls peel_ref(), then peel_ref() first - * checks whether the reference to be peeled is the one referred to by - * the iterator (it usually is) and if so, asks the iterator for the - * peeled version of the reference if it is available. This avoids a - * refname lookup in a common case. current_ref_iter is set to NULL - * when the iteration is over. - */ -extern struct ref_iterator *current_ref_iter; - struct ref_store; /* refs backends */ From 705114772e0a0741c3288329bd9ac4e11e38db9a Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 23 Oct 2025 09:16:19 +0200 Subject: [PATCH 18/29] refs: drop infrastructure to peel via iterators Now that the peeled object ID gets propagated via the `struct reference` there is no need anymore to call into the reference iterator itself to dereference an object. Remove this infrastructure. Most of the changes are straight-forward deletions of code. There is one exception though in `refs/packed-backend.c::write_with_updates()`. Here we stop peeling the iterator and instead just pass the peeled object ID of that iterator directly. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- refs.h | 14 -------------- refs/debug.c | 11 ----------- refs/files-backend.c | 17 ----------------- refs/iterator.c | 36 ------------------------------------ refs/packed-backend.c | 24 +----------------------- refs/ref-cache.c | 9 --------- refs/refs-internal.h | 7 ------- refs/reftable-backend.c | 24 ------------------------ 8 files changed, 1 insertion(+), 141 deletions(-) diff --git a/refs.h b/refs.h index 886ed2c0f43b04..2dd7ac1a16aee9 100644 --- a/refs.h +++ b/refs.h @@ -1289,10 +1289,6 @@ int repo_migrate_ref_storage_format(struct repository *repo, * to the next entry, ref_iterator_advance() aborts the iteration, * frees the ref_iterator, and returns ITER_ERROR. * - * The reference currently being looked at can be peeled by calling - * ref_iterator_peel(). This function is often faster than peel_ref(), - * so it should be preferred when iterating over references. - * * Putting it all together, a typical iteration looks like this: * * int ok; @@ -1307,9 +1303,6 @@ int repo_migrate_ref_storage_format(struct repository *repo, * // Access information about the current reference: * if (!(iter->flags & REF_ISSYMREF)) * printf("%s is %s\n", iter->refname, oid_to_hex(iter->oid)); - * - * // If you need to peel the reference: - * ref_iterator_peel(iter, &oid); * } * * if (ok != ITER_DONE) @@ -1400,13 +1393,6 @@ enum ref_iterator_seek_flag { int ref_iterator_seek(struct ref_iterator *ref_iterator, const char *refname, unsigned int flags); -/* - * If possible, peel the reference currently being viewed by the - * iterator. Return 0 on success. - */ -int ref_iterator_peel(struct ref_iterator *ref_iterator, - struct object_id *peeled); - /* Free the reference iterator and any associated resources. */ void ref_iterator_free(struct ref_iterator *ref_iterator); diff --git a/refs/debug.c b/refs/debug.c index 67718bd1f49f1f..01499b9033ca3c 100644 --- a/refs/debug.c +++ b/refs/debug.c @@ -177,16 +177,6 @@ static int debug_ref_iterator_seek(struct ref_iterator *ref_iterator, return res; } -static int debug_ref_iterator_peel(struct ref_iterator *ref_iterator, - struct object_id *peeled) -{ - struct debug_ref_iterator *diter = - (struct debug_ref_iterator *)ref_iterator; - int res = diter->iter->vtable->peel(diter->iter, peeled); - trace_printf_key(&trace_refs, "iterator_peel: %s: %d\n", diter->iter->ref.name, res); - return res; -} - static void debug_ref_iterator_release(struct ref_iterator *ref_iterator) { struct debug_ref_iterator *diter = @@ -198,7 +188,6 @@ static void debug_ref_iterator_release(struct ref_iterator *ref_iterator) static struct ref_iterator_vtable debug_ref_iterator_vtable = { .advance = debug_ref_iterator_advance, .seek = debug_ref_iterator_seek, - .peel = debug_ref_iterator_peel, .release = debug_ref_iterator_release, }; diff --git a/refs/files-backend.c b/refs/files-backend.c index fac53fa052dd22..5aeb454fb47684 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -993,15 +993,6 @@ static int files_ref_iterator_seek(struct ref_iterator *ref_iterator, return ref_iterator_seek(iter->iter0, refname, flags); } -static int files_ref_iterator_peel(struct ref_iterator *ref_iterator, - struct object_id *peeled) -{ - struct files_ref_iterator *iter = - (struct files_ref_iterator *)ref_iterator; - - return ref_iterator_peel(iter->iter0, peeled); -} - static void files_ref_iterator_release(struct ref_iterator *ref_iterator) { struct files_ref_iterator *iter = @@ -1012,7 +1003,6 @@ static void files_ref_iterator_release(struct ref_iterator *ref_iterator) static struct ref_iterator_vtable files_ref_iterator_vtable = { .advance = files_ref_iterator_advance, .seek = files_ref_iterator_seek, - .peel = files_ref_iterator_peel, .release = files_ref_iterator_release, }; @@ -2388,12 +2378,6 @@ static int files_reflog_iterator_seek(struct ref_iterator *ref_iterator UNUSED, BUG("ref_iterator_seek() called for reflog_iterator"); } -static int files_reflog_iterator_peel(struct ref_iterator *ref_iterator UNUSED, - struct object_id *peeled UNUSED) -{ - BUG("ref_iterator_peel() called for reflog_iterator"); -} - static void files_reflog_iterator_release(struct ref_iterator *ref_iterator) { struct files_reflog_iterator *iter = @@ -2404,7 +2388,6 @@ static void files_reflog_iterator_release(struct ref_iterator *ref_iterator) static struct ref_iterator_vtable files_reflog_iterator_vtable = { .advance = files_reflog_iterator_advance, .seek = files_reflog_iterator_seek, - .peel = files_reflog_iterator_peel, .release = files_reflog_iterator_release, }; diff --git a/refs/iterator.c b/refs/iterator.c index 072c6aacdb0341..d79aa5ec82dc6f 100644 --- a/refs/iterator.c +++ b/refs/iterator.c @@ -21,12 +21,6 @@ int ref_iterator_seek(struct ref_iterator *ref_iterator, const char *refname, return ref_iterator->vtable->seek(ref_iterator, refname, flags); } -int ref_iterator_peel(struct ref_iterator *ref_iterator, - struct object_id *peeled) -{ - return ref_iterator->vtable->peel(ref_iterator, peeled); -} - void ref_iterator_free(struct ref_iterator *ref_iterator) { if (ref_iterator) { @@ -60,12 +54,6 @@ static int empty_ref_iterator_seek(struct ref_iterator *ref_iterator UNUSED, return 0; } -static int empty_ref_iterator_peel(struct ref_iterator *ref_iterator UNUSED, - struct object_id *peeled UNUSED) -{ - BUG("peel called for empty iterator"); -} - static void empty_ref_iterator_release(struct ref_iterator *ref_iterator UNUSED) { } @@ -73,7 +61,6 @@ static void empty_ref_iterator_release(struct ref_iterator *ref_iterator UNUSED) static struct ref_iterator_vtable empty_ref_iterator_vtable = { .advance = empty_ref_iterator_advance, .seek = empty_ref_iterator_seek, - .peel = empty_ref_iterator_peel, .release = empty_ref_iterator_release, }; @@ -240,18 +227,6 @@ static int merge_ref_iterator_seek(struct ref_iterator *ref_iterator, return 0; } -static int merge_ref_iterator_peel(struct ref_iterator *ref_iterator, - struct object_id *peeled) -{ - struct merge_ref_iterator *iter = - (struct merge_ref_iterator *)ref_iterator; - - if (!iter->current) { - BUG("peel called before advance for merge iterator"); - } - return ref_iterator_peel(*iter->current, peeled); -} - static void merge_ref_iterator_release(struct ref_iterator *ref_iterator) { struct merge_ref_iterator *iter = @@ -263,7 +238,6 @@ static void merge_ref_iterator_release(struct ref_iterator *ref_iterator) static struct ref_iterator_vtable merge_ref_iterator_vtable = { .advance = merge_ref_iterator_advance, .seek = merge_ref_iterator_seek, - .peel = merge_ref_iterator_peel, .release = merge_ref_iterator_release, }; @@ -412,15 +386,6 @@ static int prefix_ref_iterator_seek(struct ref_iterator *ref_iterator, return ref_iterator_seek(iter->iter0, refname, flags); } -static int prefix_ref_iterator_peel(struct ref_iterator *ref_iterator, - struct object_id *peeled) -{ - struct prefix_ref_iterator *iter = - (struct prefix_ref_iterator *)ref_iterator; - - return ref_iterator_peel(iter->iter0, peeled); -} - static void prefix_ref_iterator_release(struct ref_iterator *ref_iterator) { struct prefix_ref_iterator *iter = @@ -432,7 +397,6 @@ static void prefix_ref_iterator_release(struct ref_iterator *ref_iterator) static struct ref_iterator_vtable prefix_ref_iterator_vtable = { .advance = prefix_ref_iterator_advance, .seek = prefix_ref_iterator_seek, - .peel = prefix_ref_iterator_peel, .release = prefix_ref_iterator_release, }; diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 1fefefd54ed0e7..6fa229edd0ffad 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -1030,22 +1030,6 @@ static int packed_ref_iterator_seek(struct ref_iterator *ref_iterator, return 0; } -static int packed_ref_iterator_peel(struct ref_iterator *ref_iterator, - struct object_id *peeled) -{ - struct packed_ref_iterator *iter = - (struct packed_ref_iterator *)ref_iterator; - - if ((iter->base.ref.flags & REF_KNOWS_PEELED)) { - oidcpy(peeled, &iter->peeled); - return is_null_oid(&iter->peeled) ? -1 : 0; - } else if ((iter->base.ref.flags & (REF_ISBROKEN | REF_ISSYMREF))) { - return -1; - } else { - return peel_object(iter->repo, &iter->oid, peeled) ? -1 : 0; - } -} - static void packed_ref_iterator_release(struct ref_iterator *ref_iterator) { struct packed_ref_iterator *iter = @@ -1059,7 +1043,6 @@ static void packed_ref_iterator_release(struct ref_iterator *ref_iterator) static struct ref_iterator_vtable packed_ref_iterator_vtable = { .advance = packed_ref_iterator_advance, .seek = packed_ref_iterator_seek, - .peel = packed_ref_iterator_peel, .release = packed_ref_iterator_release, }; @@ -1525,13 +1508,8 @@ static enum ref_transaction_error write_with_updates(struct packed_ref_store *re if (cmp < 0) { /* Pass the old reference through. */ - - struct object_id peeled; - int peel_error = ref_iterator_peel(iter, &peeled); - if (write_packed_entry(out, iter->ref.name, - iter->ref.oid, - peel_error ? NULL : &peeled)) + iter->ref.oid, iter->ref.peeled_oid)) goto write_error; if ((ok = ref_iterator_advance(iter)) != ITER_OK) { diff --git a/refs/ref-cache.c b/refs/ref-cache.c index e427848879d61b..ffef01a597579e 100644 --- a/refs/ref-cache.c +++ b/refs/ref-cache.c @@ -546,14 +546,6 @@ static int cache_ref_iterator_seek(struct ref_iterator *ref_iterator, return 0; } -static int cache_ref_iterator_peel(struct ref_iterator *ref_iterator, - struct object_id *peeled) -{ - struct cache_ref_iterator *iter = - (struct cache_ref_iterator *)ref_iterator; - return peel_object(iter->repo, ref_iterator->ref.oid, peeled) ? -1 : 0; -} - static void cache_ref_iterator_release(struct ref_iterator *ref_iterator) { struct cache_ref_iterator *iter = @@ -565,7 +557,6 @@ static void cache_ref_iterator_release(struct ref_iterator *ref_iterator) static struct ref_iterator_vtable cache_ref_iterator_vtable = { .advance = cache_ref_iterator_advance, .seek = cache_ref_iterator_seek, - .peel = cache_ref_iterator_peel, .release = cache_ref_iterator_release, }; diff --git a/refs/refs-internal.h b/refs/refs-internal.h index f4f845bbeaf673..4671517dade968 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -357,12 +357,6 @@ typedef int ref_iterator_advance_fn(struct ref_iterator *ref_iterator); typedef int ref_iterator_seek_fn(struct ref_iterator *ref_iterator, const char *refname, unsigned int flags); -/* - * Peels the current ref, returning 0 for success or -1 for failure. - */ -typedef int ref_iterator_peel_fn(struct ref_iterator *ref_iterator, - struct object_id *peeled); - /* * Implementations of this function should free any resources specific * to the derived class. @@ -372,7 +366,6 @@ typedef void ref_iterator_release_fn(struct ref_iterator *ref_iterator); struct ref_iterator_vtable { ref_iterator_advance_fn *advance; ref_iterator_seek_fn *seek; - ref_iterator_peel_fn *peel; ref_iterator_release_fn *release; }; diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index e214e120d77a5c..e329d4a423abdb 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -744,21 +744,6 @@ static int reftable_ref_iterator_seek(struct ref_iterator *ref_iterator, return iter->err; } -static int reftable_ref_iterator_peel(struct ref_iterator *ref_iterator, - struct object_id *peeled) -{ - struct reftable_ref_iterator *iter = - (struct reftable_ref_iterator *)ref_iterator; - - if (iter->ref.value_type == REFTABLE_REF_VAL2) { - oidread(peeled, iter->ref.value.val2.target_value, - iter->refs->base.repo->hash_algo); - return 0; - } - - return -1; -} - static void reftable_ref_iterator_release(struct ref_iterator *ref_iterator) { struct reftable_ref_iterator *iter = @@ -776,7 +761,6 @@ static void reftable_ref_iterator_release(struct ref_iterator *ref_iterator) static struct ref_iterator_vtable reftable_ref_iterator_vtable = { .advance = reftable_ref_iterator_advance, .seek = reftable_ref_iterator_seek, - .peel = reftable_ref_iterator_peel, .release = reftable_ref_iterator_release, }; @@ -2098,13 +2082,6 @@ static int reftable_reflog_iterator_seek(struct ref_iterator *ref_iterator UNUSE return -1; } -static int reftable_reflog_iterator_peel(struct ref_iterator *ref_iterator UNUSED, - struct object_id *peeled UNUSED) -{ - BUG("reftable reflog iterator cannot be peeled"); - return -1; -} - static void reftable_reflog_iterator_release(struct ref_iterator *ref_iterator) { struct reftable_reflog_iterator *iter = @@ -2117,7 +2094,6 @@ static void reftable_reflog_iterator_release(struct ref_iterator *ref_iterator) static struct ref_iterator_vtable reftable_reflog_iterator_vtable = { .advance = reftable_reflog_iterator_advance, .seek = reftable_reflog_iterator_seek, - .peel = reftable_reflog_iterator_peel, .release = reftable_reflog_iterator_release, }; From 7ec85185b197ce1cd28721a6f4415fb9db5cd42f Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 23 Oct 2025 09:16:20 +0200 Subject: [PATCH 19/29] object: add flag to `peel_object()` to verify object type When peeling a tag to a non-tag object we repeatedly call `parse_object()` on the tagged object until we find the first object that isn't a tag. While this feels sensible at first, there is a big catch here: `parse_object()` doesn't actually verify the type of the tagged object. The relevant code path here eventually ends up in `parse_tag_buffer()`. Here, we parse the various fields of the tag, including the "type". Once we've figured out the type and the tagged object ID, we call one of the `lookup_${type}()` functions for whatever type we have found. There is two possible outcomes in the successful case: 1. The object is already part of our cached objects. In that case we double-check whether the type we're trying to look up matches the type that was cached. 2. The object is _not_ part of our cached objects. In that case, we simply create a new object with the expected type, but we don't parse that object. In the first case we might notice type mismatches, but only in the case where our cache has the object with the correct type. In the second case, we'll blindly assume that the type is correct and then go with it. We'll only notice that the type might be wrong when we try to parse the object at a later point. Now arguably, we could change `parse_tag_buffer()` to verify the tagged object's type for us. But that would have the effect that such a tag cannot be parsed at all anymore, and we have a small bunch of tests for exactly this case that assert we still can open such tags. So this change does not feel like something we can retroactively tighten, even though one shouldn't ever hit such corrupted tags. Instead, add a new `flags` field to `peel_object()` that allows the caller to opt in to strict object verification. This will be wired up at a subset of callsites over the next few commits. Note that this change also inlines `deref_tag_noverify()`. There's only been two callsites of that function, the one we're changing and one in our test helpers. The latter callsite can trivially use `deref_tag()` instead, so by inlining the function we avoid having to pass down the flag. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- object.c | 20 +++++++++++++++++--- object.h | 15 ++++++++++++++- ref-filter.c | 2 +- refs.c | 2 +- refs/packed-backend.c | 5 ++--- refs/reftable-backend.c | 4 ++-- t/helper/test-reach.c | 2 +- tag.c | 12 ------------ tag.h | 1 - 9 files changed, 38 insertions(+), 25 deletions(-) diff --git a/object.c b/object.c index 986114a6dba843..e72b0ed4360e67 100644 --- a/object.c +++ b/object.c @@ -209,11 +209,12 @@ struct object *lookup_object_by_type(struct repository *r, enum peel_status peel_object(struct repository *r, const struct object_id *name, - struct object_id *oid) + struct object_id *oid, + unsigned flags) { struct object *o = lookup_unknown_object(r, name); - if (o->type == OBJ_NONE) { + if (o->type == OBJ_NONE || flags & PEEL_OBJECT_VERIFY_OBJECT_TYPE) { int type = odb_read_object_info(r->objects, name, NULL); if (type < 0 || !object_as_type(o, type, 0)) return PEEL_INVALID; @@ -222,7 +223,20 @@ enum peel_status peel_object(struct repository *r, if (o->type != OBJ_TAG) return PEEL_NON_TAG; - o = deref_tag_noverify(r, o); + while (o && o->type == OBJ_TAG) { + o = parse_object(r, &o->oid); + if (o && o->type == OBJ_TAG && ((struct tag *)o)->tagged) { + o = ((struct tag *)o)->tagged; + + if (flags & PEEL_OBJECT_VERIFY_OBJECT_TYPE) { + int type = odb_read_object_info(r->objects, &o->oid, NULL); + if (type < 0 || !object_as_type(o, type, 0)) + return PEEL_INVALID; + } + } else { + o = NULL; + } + } if (!o) return PEEL_INVALID; diff --git a/object.h b/object.h index 8c3c1c46e1bf04..1499f63d507c32 100644 --- a/object.h +++ b/object.h @@ -287,6 +287,17 @@ enum peel_status { PEEL_BROKEN = -4 }; +enum peel_object_flags { + /* + * Always verify the object type, even in the case where the looked-up + * object already has an object type. This can be useful when the + * stored object type may be invalid. One such case is when looking up + * objects via tags, where we blindly trust the object type declared by + * the tag. + */ + PEEL_OBJECT_VERIFY_OBJECT_TYPE = (1 << 0), +}; + /* * Peel the named object; i.e., if the object is a tag, resolve the * tag recursively until a non-tag is found. If successful, store the @@ -295,7 +306,9 @@ enum peel_status { * and leave oid unchanged. */ enum peel_status peel_object(struct repository *r, - const struct object_id *name, struct object_id *oid); + const struct object_id *name, + struct object_id *oid, + unsigned flags); struct object_list *object_list_insert(struct object *item, struct object_list **list_p); diff --git a/ref-filter.c b/ref-filter.c index 7fd8babec8f5bd..9a8ed8c8fc1f3b 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -2581,7 +2581,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) if (need_tagged) { if (!is_null_oid(&ref->peeled_oid)) { oidcpy(&oi_deref.oid, &ref->peeled_oid); - } else if (!peel_object(the_repository, &obj->oid, &oi_deref.oid)) { + } else if (!peel_object(the_repository, &oi.oid, &oi_deref.oid, 0)) { /* We managed to peel the object ourselves. */ } else { die("bad tag"); diff --git a/refs.c b/refs.c index 9d8f0a9ca4a3a6..a41a94ae55bb43 100644 --- a/refs.c +++ b/refs.c @@ -2333,7 +2333,7 @@ int reference_get_peeled_oid(struct repository *repo, return 0; } - return peel_object(repo, ref->oid, peeled_oid) ? -1 : 0; + return peel_object(repo, ref->oid, peeled_oid, 0) ? -1 : 0; } int refs_update_symref(struct ref_store *refs, const char *ref, diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 6fa229edd0ffad..4752d3f3981fe3 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -1527,9 +1527,8 @@ static enum ref_transaction_error write_with_updates(struct packed_ref_store *re i++; } else { struct object_id peeled; - int peel_error = peel_object(refs->base.repo, - &update->new_oid, - &peeled); + int peel_error = peel_object(refs->base.repo, &update->new_oid, + &peeled, 0); if (write_packed_entry(out, update->refname, &update->new_oid, diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index e329d4a423abdb..9febb2322c3b24 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -1632,7 +1632,7 @@ static int write_transaction_table(struct reftable_writer *writer, void *cb_data ref.refname = (char *)u->refname; ref.update_index = ts; - peel_error = peel_object(arg->refs->base.repo, &u->new_oid, &peeled); + peel_error = peel_object(arg->refs->base.repo, &u->new_oid, &peeled, 0); if (!peel_error) { ref.value_type = REFTABLE_REF_VAL2; memcpy(ref.value.val2.target_value, peeled.hash, GIT_MAX_RAWSZ); @@ -2497,7 +2497,7 @@ static int write_reflog_expiry_table(struct reftable_writer *writer, void *cb_da ref.refname = (char *)arg->refname; ref.update_index = ts; - if (!peel_object(arg->refs->base.repo, &arg->update_oid, &peeled)) { + if (!peel_object(arg->refs->base.repo, &arg->update_oid, &peeled, 0)) { ref.value_type = REFTABLE_REF_VAL2; memcpy(ref.value.val2.target_value, peeled.hash, GIT_MAX_RAWSZ); memcpy(ref.value.val2.value, arg->update_oid.hash, GIT_MAX_RAWSZ); diff --git a/t/helper/test-reach.c b/t/helper/test-reach.c index 028ec003067828..c58c93800f3232 100644 --- a/t/helper/test-reach.c +++ b/t/helper/test-reach.c @@ -63,7 +63,7 @@ int cmd__reach(int ac, const char **av) die("failed to resolve %s", buf.buf + 2); orig = parse_object(r, &oid); - peeled = deref_tag_noverify(the_repository, orig); + peeled = deref_tag(the_repository, orig, NULL, 0); if (!peeled) die("failed to load commit for input %s resulting in oid %s", diff --git a/tag.c b/tag.c index 1d52686ee105f2..f5c232d2f1f36c 100644 --- a/tag.c +++ b/tag.c @@ -94,18 +94,6 @@ struct object *deref_tag(struct repository *r, struct object *o, const char *war return o; } -struct object *deref_tag_noverify(struct repository *r, struct object *o) -{ - while (o && o->type == OBJ_TAG) { - o = parse_object(r, &o->oid); - if (o && o->type == OBJ_TAG && ((struct tag *)o)->tagged) - o = ((struct tag *)o)->tagged; - else - o = NULL; - } - return o; -} - struct tag *lookup_tag(struct repository *r, const struct object_id *oid) { struct object *obj = lookup_object(r, oid); diff --git a/tag.h b/tag.h index c49d7c19ad3c90..ef12a610372063 100644 --- a/tag.h +++ b/tag.h @@ -16,7 +16,6 @@ int parse_tag_buffer(struct repository *r, struct tag *item, const void *data, u int parse_tag(struct tag *item); void release_tag_memory(struct tag *t); struct object *deref_tag(struct repository *r, struct object *, const char *, int); -struct object *deref_tag_noverify(struct repository *r, struct object *); int gpg_verify_tag(const struct object_id *oid, const char *name_to_report, unsigned flags); struct object_id *get_tagged_oid(struct tag *tag); From 6ec4c0b45ba916770c6fbc03df94018ca06fb77e Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 23 Oct 2025 09:16:21 +0200 Subject: [PATCH 20/29] refs: don't store peeled object IDs for invalid tags Both the "files" and "reftable" backend store peeled object IDs for references that point to tags: - The "files" backend stores the value when packing refs, where each peeled object ID is prefixed with "^". - The "reftable" backend stores the value whenever writing a new reference that points to a tag via a special ref record type. Both of these backends use `peel_object()` to find the peeled object ID. But as explained in the preceding commit, that function does not detect the case where the tag's tagged object and its claimed type mismatch. The consequence of storing these bogus peeled object IDs is that we're less likely to detect such corruption in other parts of Git. git-for-each-ref(1) for example does not notice anymore that the tag is broken when using "--format=%(*objectname)" to dereference tags. One could claim that this is good, because it still allows us to mostly use the tag as intended. But the biggest problem here is that we now have different behaviour for such a broken tag depending on whether or not we have its peeled value in the refdb. Fix the issue by verifying the object type when peeling the object. If that verification fails we simply skip storing the peeled value in either of the reference formats. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- refs/packed-backend.c | 2 +- refs/reftable-backend.c | 3 ++- t/pack-refs-tests.sh | 32 ++++++++++++++++++++++++++++++++ t/t0610-reftable-basics.sh | 28 ++++++++++++++++++++++++++++ 4 files changed, 63 insertions(+), 2 deletions(-) diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 4752d3f3981fe3..1ab0c503930164 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -1528,7 +1528,7 @@ static enum ref_transaction_error write_with_updates(struct packed_ref_store *re } else { struct object_id peeled; int peel_error = peel_object(refs->base.repo, &update->new_oid, - &peeled, 0); + &peeled, PEEL_OBJECT_VERIFY_OBJECT_TYPE); if (write_packed_entry(out, update->refname, &update->new_oid, diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index 9febb2322c3b24..6bbfd5618dac16 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -1632,7 +1632,8 @@ static int write_transaction_table(struct reftable_writer *writer, void *cb_data ref.refname = (char *)u->refname; ref.update_index = ts; - peel_error = peel_object(arg->refs->base.repo, &u->new_oid, &peeled, 0); + peel_error = peel_object(arg->refs->base.repo, &u->new_oid, &peeled, + PEEL_OBJECT_VERIFY_OBJECT_TYPE); if (!peel_error) { ref.value_type = REFTABLE_REF_VAL2; memcpy(ref.value.val2.target_value, peeled.hash, GIT_MAX_RAWSZ); diff --git a/t/pack-refs-tests.sh b/t/pack-refs-tests.sh index 3dbcc01718e157..095823d915fd63 100644 --- a/t/pack-refs-tests.sh +++ b/t/pack-refs-tests.sh @@ -428,4 +428,36 @@ do ' done +test_expect_success 'pack-refs does not store invalid peeled tag value' ' + test_when_finished rm -rf repo && + git init repo && + ( + cd repo && + git commit --allow-empty --message initial && + + echo garbage >blob-content && + blob_id=$(git hash-object -w -t blob blob-content) && + + # Write an invalid tag into the object database. The tag itself + # is well-formed, but the tagged object is a blob while we + # claim that it is a commit. + cat >tag-content <<-EOF && + object $blob_id + type commit + tag bad-tag + tagger C O Mitter 1112354055 +0200 + + annotated + EOF + tag_id=$(git hash-object -w -t tag tag-content) && + git update-ref refs/tags/bad-tag "$tag_id" && + + # The packed-refs file should not contain the peeled object ID. + # If it did this would cause commands that use the peeled value + # to not notice this corrupted tag. + git pack-refs --all && + test_grep ! "^\^" .git/packed-refs + ) +' + test_done diff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh index 3ea5d51532a8b8..6575528f212716 100755 --- a/t/t0610-reftable-basics.sh +++ b/t/t0610-reftable-basics.sh @@ -1135,4 +1135,32 @@ test_expect_success 'fetch: accessing FETCH_HEAD special ref works' ' test_cmp expect actual ' +test_expect_success 'writes do not persist peeled value for invalid tags' ' + test_when_finished rm -rf repo && + git init repo && + ( + cd repo && + git commit --allow-empty --message initial && + + # We cannot easily verify that the peeled value is not stored + # in the tables. Instead, we test this indirectly: we create + # two tags that both point to the same object, but they claim + # different object types. If we parse both tags we notice that + # the parsed tagged object has a mismatch between the two tags + # and bail out. + # + # If we instead use the persisted peeled value we would not + # even parse the tags. As such, we would not notice the + # discrepancy either and thus listing these tags would succeed. + git tag tag-1 -m "tag 1" && + git cat-file tag tag-1 >raw-tag && + sed "s/^type commit$/type blob/" broken-tag && + broken_tag_id=$(git hash-object -w -t tag broken-tag) && + git update-ref refs/tags/tag-2 $broken_tag_id && + + test_must_fail git for-each-ref --format="%(*objectname)" refs/tags/ 2>err && + test_grep "bad tag pointer" err + ) +' + test_done From e66077ae45813a5ca269a7d676310a243bdcc1c2 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 23 Oct 2025 09:16:22 +0200 Subject: [PATCH 21/29] ref-filter: detect broken tags when dereferencing them Users can ask git-for-each-ref(1) to peel tags and return information of the tagged object by adding an asterisk to the format, like for example "%(*$objectname)". If so, git-for-each-ref(1) peels that object to the first non-tag object and then returns its values. As mentioned in preceding commits, it can happen that the tagged object type and the claimed object type differ, effectively resulting in a corrupt tag. git-for-each-ref(1) would notice this mismatch, print an error and then bail out when trying to peel the tag. But we only notice this corruption in some very specific edge cases! While we have a test in "t/for-each-ref-tests.sh" that verifies the above scenario, this test is specifically crafted to detect the issue at hand. Namely, we create two tags: - One tag points to a specific object with the correct type. - The other tag points to the *same* object with a different type. The fact that both tags point to the same object is important here: `peel_object()` wouldn't notice the corruption if the tagged objects were different. The root cause is that `peel_object()` calls `lookup_${type}()` eventually, where the type is the same type declared in the tag object. Consequently, when we have two tags pointing to the same object but with different declared types we'll call two different lookup functions. The first lookup will store the object with an unverified type A, whereas the second lookup will try to look up the object with a different unverified type B. And it is only now that we notice the discrepancy in object types, even though type A could've already been the wrong type. Fix the issue by verifying the object type in `populate_value()`. With this change we'll also notice type mismatches when only dereferencing a tag once. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- ref-filter.c | 3 ++- t/for-each-ref-tests.sh | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index 9a8ed8c8fc1f3b..c54025d6b4c4cd 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -2581,7 +2581,8 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) if (need_tagged) { if (!is_null_oid(&ref->peeled_oid)) { oidcpy(&oi_deref.oid, &ref->peeled_oid); - } else if (!peel_object(the_repository, &oi.oid, &oi_deref.oid, 0)) { + } else if (!peel_object(the_repository, &oi.oid, &oi_deref.oid, + PEEL_OBJECT_VERIFY_OBJECT_TYPE)) { /* We managed to peel the object ourselves. */ } else { die("bad tag"); diff --git a/t/for-each-ref-tests.sh b/t/for-each-ref-tests.sh index e3ad19298accde..4593be5fd544a8 100644 --- a/t/for-each-ref-tests.sh +++ b/t/for-each-ref-tests.sh @@ -1809,7 +1809,9 @@ test_expect_success "${git_for_each_ref} reports broken tags" ' bad=$(git hash-object -w -t tag bad) && git update-ref refs/tags/broken-tag-bad $bad && test_must_fail ${git_for_each_ref} --format="%(*objectname)" \ - refs/tags/broken-tag-* + refs/tags/broken-tag-* && + test_must_fail ${git_for_each_ref} --format="%(*objectname)" \ + refs/tags/broken-tag-bad ' test_expect_success 'set up tag with signature and no blank lines' ' From a29e2e8fe7e3935e23d2a03dc429cc9c2e68bfbe Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 23 Oct 2025 09:16:23 +0200 Subject: [PATCH 22/29] ref-filter: parse objects on demand MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When formatting an arbitrary object we parse that object regardless of whether or not we actually need any parsed data. In fact, many of the atoms we have don't require any. Refactor the code so that we parse the data on demand when we see an atom that wants to access the objects. This leads to a small speedup, for example in the Chromium repository with around 40000 refs: Benchmark 1: for-each-ref --format='%(raw)' (HEAD~) Time (mean ± σ): 388.7 ms ± 1.1 ms [User: 322.2 ms, System: 65.0 ms] Range (min … max): 387.3 ms … 390.8 ms 10 runs Benchmark 2: for-each-ref --format='%(raw)' (HEAD) Time (mean ± σ): 344.7 ms ± 0.7 ms [User: 287.8 ms, System: 55.1 ms] Range (min … max): 343.9 ms … 345.7 ms 10 runs Summary for-each-ref --format='%(raw)' (HEAD) ran 1.13 ± 0.00 times faster than for-each-ref --format='%(raw)' (HEAD~) With this change, we now spend ~90% of the time decompressing objects, which is almost as good as it gets regarding git-for-each-ref(1)'s own infrastructure. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- ref-filter.c | 142 ++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 106 insertions(+), 36 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index c54025d6b4c4cd..7cfcd5c3554930 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -91,6 +91,7 @@ static struct expand_data { struct object_id delta_base_oid; void *content; + struct object *maybe_object; struct object_info info; } oi, oi_deref; @@ -1475,11 +1476,29 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_ } } +static struct object *get_or_parse_object(struct expand_data *data, const char *refname, + struct strbuf *err, int *eaten) +{ + if (!data->maybe_object) { + data->maybe_object = parse_object_buffer(the_repository, &data->oid, data->type, + data->size, data->content, eaten); + if (!data->maybe_object) { + strbuf_addf(err, _("parse_object_buffer failed on %s for %s"), + oid_to_hex(&data->oid), refname); + return NULL; + } + } + + return data->maybe_object; +} + /* See grab_values */ -static void grab_tag_values(struct atom_value *val, int deref, struct object *obj) +static int grab_tag_values(struct atom_value *val, int deref, + struct expand_data *data, const char *refname, + struct strbuf *err, int *eaten) { + struct tag *tag = NULL; int i; - struct tag *tag = (struct tag *) obj; for (i = 0; i < used_atom_cnt; i++) { const char *name = used_atom[i].name; @@ -1487,6 +1506,14 @@ static void grab_tag_values(struct atom_value *val, int deref, struct object *ob struct atom_value *v = &val[i]; if (!!deref != (*name == '*')) continue; + + if (!tag) { + tag = (struct tag *) get_or_parse_object(data, refname, + err, eaten); + if (!tag) + return -1; + } + if (deref) name++; if (atom_type == ATOM_TAG) @@ -1496,22 +1523,35 @@ static void grab_tag_values(struct atom_value *val, int deref, struct object *ob else if (atom_type == ATOM_OBJECT && tag->tagged) v->s = xstrdup(oid_to_hex(&tag->tagged->oid)); } + + return 0; } /* See grab_values */ -static void grab_commit_values(struct atom_value *val, int deref, struct object *obj) +static int grab_commit_values(struct atom_value *val, int deref, + struct expand_data *data, const char *refname, + struct strbuf *err, int *eaten) { int i; - struct commit *commit = (struct commit *) obj; + struct commit *commit = NULL; for (i = 0; i < used_atom_cnt; i++) { const char *name = used_atom[i].name; enum atom_type atom_type = used_atom[i].atom_type; struct atom_value *v = &val[i]; + if (!!deref != (*name == '*')) continue; if (deref) name++; + + if (!commit) { + commit = (struct commit *) get_or_parse_object(data, refname, + err, eaten); + if (!commit) + return -1; + } + if (atom_type == ATOM_TREE && grab_oid(name, "tree", get_commit_tree_oid(commit), v, &used_atom[i])) continue; @@ -1531,6 +1571,8 @@ static void grab_commit_values(struct atom_value *val, int deref, struct object v->s = strbuf_detach(&s, NULL); } } + + return 0; } static const char *find_wholine(const char *who, int wholen, const char *buf) @@ -1759,10 +1801,12 @@ static void grab_person(const char *who, struct atom_value *val, int deref, void } } -static void grab_signature(struct atom_value *val, int deref, struct object *obj) +static int grab_signature(struct atom_value *val, int deref, + struct expand_data *data, const char *refname, + struct strbuf *err, int *eaten) { int i; - struct commit *commit = (struct commit *) obj; + struct commit *commit = NULL; struct signature_check sigc = { 0 }; int signature_checked = 0; @@ -1790,6 +1834,13 @@ static void grab_signature(struct atom_value *val, int deref, struct object *obj continue; if (!signature_checked) { + if (!commit) { + commit = (struct commit *) get_or_parse_object(data, refname, + err, eaten); + if (!commit) + return -1; + } + check_commit_signature(commit, &sigc); signature_checked = 1; } @@ -1843,6 +1894,8 @@ static void grab_signature(struct atom_value *val, int deref, struct object *obj if (signature_checked) signature_check_clear(&sigc); + + return 0; } static void find_subpos(const char *buf, @@ -1920,9 +1973,8 @@ static void append_lines(struct strbuf *out, const char *buf, unsigned long size } static void grab_describe_values(struct atom_value *val, int deref, - struct object *obj) + struct expand_data *data) { - struct commit *commit = (struct commit *)obj; int i; for (i = 0; i < used_atom_cnt; i++) { @@ -1944,7 +1996,7 @@ static void grab_describe_values(struct atom_value *val, int deref, cmd.git_cmd = 1; strvec_push(&cmd.args, "describe"); strvec_pushv(&cmd.args, atom->u.describe_args.v); - strvec_push(&cmd.args, oid_to_hex(&commit->object.oid)); + strvec_push(&cmd.args, oid_to_hex(&data->oid)); if (pipe_command(&cmd, NULL, 0, &out, 0, &err, 0) < 0) { error(_("failed to run 'describe'")); v->s = xstrdup(""); @@ -2066,24 +2118,36 @@ static void fill_missing_values(struct atom_value *val) * pointed at by the ref itself; otherwise it is the object the * ref (which is a tag) refers to. */ -static void grab_values(struct atom_value *val, int deref, struct object *obj, struct expand_data *data) +static int grab_values(struct atom_value *val, int deref, struct expand_data *data, + const char *refname, struct strbuf *err, int *eaten) { void *buf = data->content; + int ret; - switch (obj->type) { + switch (data->type) { case OBJ_TAG: - grab_tag_values(val, deref, obj); + ret = grab_tag_values(val, deref, data, refname, err, eaten); + if (ret < 0) + goto out; + grab_sub_body_contents(val, deref, data); grab_person("tagger", val, deref, buf); - grab_describe_values(val, deref, obj); + grab_describe_values(val, deref, data); break; case OBJ_COMMIT: - grab_commit_values(val, deref, obj); + ret = grab_commit_values(val, deref, data, refname, err, eaten); + if (ret < 0) + goto out; + grab_sub_body_contents(val, deref, data); grab_person("author", val, deref, buf); grab_person("committer", val, deref, buf); - grab_signature(val, deref, obj); - grab_describe_values(val, deref, obj); + + ret = grab_signature(val, deref, data, refname, err, eaten); + if (ret < 0) + goto out; + + grab_describe_values(val, deref, data); break; case OBJ_TREE: /* grab_tree_values(val, deref, obj, buf, sz); */ @@ -2094,8 +2158,12 @@ static void grab_values(struct atom_value *val, int deref, struct object *obj, s grab_sub_body_contents(val, deref, data); break; default: - die("Eh? Object of type %d?", obj->type); + die("Eh? Object of type %d?", data->type); } + + ret = 0; +out: + return ret; } static inline char *copy_advance(char *dst, const char *src) @@ -2292,38 +2360,41 @@ static const char *get_refname(struct used_atom *atom, struct ref_array_item *re return show_ref(&atom->u.refname, ref->refname); } -static int get_object(struct ref_array_item *ref, int deref, struct object **obj, +static int get_object(struct ref_array_item *ref, int deref, struct expand_data *oi, struct strbuf *err) { - /* parse_object_buffer() will set eaten to 0 if free() will be needed */ - int eaten = 1; + /* parse_object_buffer() will set eaten to 1 if free() will be needed */ + int eaten = 0; + int ret; + if (oi->info.contentp) { /* We need to know that to use parse_object_buffer properly */ oi->info.sizep = &oi->size; oi->info.typep = &oi->type; } + if (odb_read_object_info_extended(the_repository->objects, &oi->oid, &oi->info, - OBJECT_INFO_LOOKUP_REPLACE)) - return strbuf_addf_ret(err, -1, _("missing object %s for %s"), - oid_to_hex(&oi->oid), ref->refname); + OBJECT_INFO_LOOKUP_REPLACE)) { + ret = strbuf_addf_ret(err, -1, _("missing object %s for %s"), + oid_to_hex(&oi->oid), ref->refname); + goto out; + } if (oi->info.disk_sizep && oi->disk_size < 0) BUG("Object size is less than zero."); if (oi->info.contentp) { - *obj = parse_object_buffer(the_repository, &oi->oid, oi->type, oi->size, oi->content, &eaten); - if (!*obj) { - if (!eaten) - free(oi->content); - return strbuf_addf_ret(err, -1, _("parse_object_buffer failed on %s for %s"), - oid_to_hex(&oi->oid), ref->refname); - } - grab_values(ref->value, deref, *obj, oi); + ret = grab_values(ref->value, deref, oi, ref->refname, err, &eaten); + if (ret < 0) + goto out; } grab_common_values(ref->value, deref, oi); + ret = 0; + +out: if (!eaten) free(oi->content); - return 0; + return ret; } static void populate_worktree_map(struct hashmap *map, struct worktree **worktrees) @@ -2376,7 +2447,6 @@ static char *get_worktree_path(const struct ref_array_item *ref) */ static int populate_value(struct ref_array_item *ref, struct strbuf *err) { - struct object *obj; int i; struct object_info empty = OBJECT_INFO_INIT; int ahead_behind_atoms = 0; @@ -2564,14 +2634,14 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) oi.oid = ref->objectname; - if (get_object(ref, 0, &obj, &oi, err)) + if (get_object(ref, 0, &oi, err)) return -1; /* * If there is no atom that wants to know about tagged * object, we are done. */ - if (!need_tagged || (obj->type != OBJ_TAG)) + if (!need_tagged || (oi.type != OBJ_TAG)) return 0; /* @@ -2589,7 +2659,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) } } - return get_object(ref, 1, &obj, &oi_deref, err); + return get_object(ref, 1, &oi_deref, err); } /* From bea37f1d647c6b17896eb3f0c210ac8dfc27b6d7 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 4 Nov 2025 15:36:13 +0100 Subject: [PATCH 23/29] ref-filter: fix stale parsed objects In 054f5f457e (ref-filter: parse objects on demand, 2025-10-23) we have started to skip parsing some objects in case we don't need to access their values in the first place. This was done by introducing a new member `struct expand_data::maybe_object` that gets populated on demand via `get_or_parse_object()`. This has led to a regression though where the object now gets reused because we don't reset it properly. The `oi` structure is declared in global scope, and there is no single place where we reset it before invoking `get_object()`. The consequence is that the `maybe_object` member doesn't get reset across calls, so subsequent calls will end up reusing the same object. This is only an issue for a subset of retrieved values, as not all of the infrastructure ends up calling `get_or_parse_object()`. So the effect is limited, which is probably why the issue wasn't detected earlier. Fix the issue by resetting `maybe_object` in `get_object()`. Reported-by: Junio C Hamano Based-on-patch-by: Jeff King Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- ref-filter.c | 2 ++ t/t7004-tag.sh | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/ref-filter.c b/ref-filter.c index 7cfcd5c3554930..d8667c569a18f1 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -2367,6 +2367,8 @@ static int get_object(struct ref_array_item *ref, int deref, int eaten = 0; int ret; + oi->maybe_object = NULL; + if (oi->info.contentp) { /* We need to know that to use parse_object_buffer properly */ oi->info.sizep = &oi->size; diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh index 10835631ca7644..d1388cfdf45b02 100755 --- a/t/t7004-tag.sh +++ b/t/t7004-tag.sh @@ -2332,4 +2332,24 @@ test_expect_success 'If tag cannot be created then tag message file is not unlin test_path_exists .git/TAG_EDITMSG ' +test_expect_success 'annotated tag version sort' ' + git tag -a -m "sample 1.0" vsample-1.0 && + git tag -a -m "sample 2.0" vsample-2.0 && + git tag -a -m "sample 10.0" vsample-10.0 && + cat >expect <<-EOF && + vsample-1.0 + vsample-2.0 + vsample-10.0 + EOF + + git tag --list --sort=version:tag vsample-\* >actual && + test_cmp expect actual && + + # Ensure that we also handle this case alright in the case we have the + # peeled values cached e.g. via the packed-refs file. + git pack-refs --all && + git tag --list --sort=version:tag vsample-\* && + test_cmp expect actual +' + test_done From 61ac8ba0f034b61d7353a799fecae9fe45137a72 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 4 Nov 2025 07:28:59 -0800 Subject: [PATCH 24/29] t7004: do not chdir around in the main process Move down to no-contains subdirectory inside a subshell, just like the previous step that created and used it does. Signed-off-by: Junio C Hamano --- t/t7004-tag.sh | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh index d1388cfdf45b02..ce2ff2a28abaa3 100755 --- a/t/t7004-tag.sh +++ b/t/t7004-tag.sh @@ -2293,24 +2293,26 @@ test_expect_success '--contains combined with --no-contains' ' # don't recurse down to tags for trees or blobs pointed to by *those* # commits. test_expect_success 'Does --[no-]contains stop at commits? Yes!' ' - cd no-contains && - blob=$(git rev-parse v0.3:v0.3.t) && - tree=$(git rev-parse v0.3^{tree}) && - git tag tag-blob $blob && - git tag tag-tree $tree && - git tag --contains v0.3 >actual && - cat >expected <<-\EOF && - v0.3 - v0.4 - v0.5 - EOF - test_cmp expected actual && - git tag --no-contains v0.3 >actual && - cat >expected <<-\EOF && - v0.1 - v0.2 - EOF - test_cmp expected actual + ( + cd no-contains && + blob=$(git rev-parse v0.3:v0.3.t) && + tree=$(git rev-parse v0.3^{tree}) && + git tag tag-blob $blob && + git tag tag-tree $tree && + git tag --contains v0.3 >actual && + cat >expected <<-\EOF && + v0.3 + v0.4 + v0.5 + EOF + test_cmp expected actual && + git tag --no-contains v0.3 >actual && + cat >expected <<-\EOF && + v0.1 + v0.2 + EOF + test_cmp expected actual + ) ' test_expect_success 'If tag is created then tag message file is unlinked' ' From 9b93ab8a9c61c53b3b9b2b3ba60c3e5d66b8ff56 Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Mon, 20 Oct 2025 10:18:29 +0200 Subject: [PATCH 25/29] refs: move to using the '.optimize' functions The `struct ref_store` variable exposes two ways to optimize a reftable backend: 1. pack_refs 2. optimize The former was specific to the 'files' + 'packed' refs backend. The latter is more generic and covers all backends. While the naming is different, both of these functions perform the same functionality. Consolidate this code to only maintain the 'optimize' functions. Do this by modifying the backends so that they exclusively implement the `optimize` callback, only. All users of the refs subsystem already use the 'optimize' function so there is no changes needed on the callee side. Finally, cleanup all references to the 'pack_refs' field of the structure and code around it. Signed-off-by: Karthik Nayak Signed-off-by: Junio C Hamano --- refs.c | 6 ------ refs.h | 6 ------ refs/debug.c | 8 ++++---- refs/files-backend.c | 14 ++------------ refs/packed-backend.c | 6 +++--- refs/refs-internal.h | 3 --- refs/reftable-backend.c | 13 +++---------- 7 files changed, 12 insertions(+), 44 deletions(-) diff --git a/refs.c b/refs.c index a41a94ae55bb43..b9a4a606462d71 100644 --- a/refs.c +++ b/refs.c @@ -2313,12 +2313,6 @@ void base_ref_store_init(struct ref_store *refs, struct repository *repo, refs->gitdir = xstrdup(path); } -/* backend functions */ -int refs_pack_refs(struct ref_store *refs, struct pack_refs_opts *opts) -{ - return refs->be->pack_refs(refs, opts); -} - int refs_optimize(struct ref_store *refs, struct pack_refs_opts *opts) { return refs->be->optimize(refs, opts); diff --git a/refs.h b/refs.h index 2dd7ac1a16aee9..8ff591ea95c7d9 100644 --- a/refs.h +++ b/refs.h @@ -514,12 +514,6 @@ struct pack_refs_opts { struct string_list *includes; }; -/* - * Write a packed-refs file for the current repository. - * flags: Combination of the above PACK_REFS_* flags. - */ -int refs_pack_refs(struct ref_store *refs, struct pack_refs_opts *opts); - /* * Optimize the ref store. The exact behavior is up to the backend. * For the files backend, this is equivalent to packing refs. diff --git a/refs/debug.c b/refs/debug.c index 01499b9033ca3c..40cd1d9c1540c6 100644 --- a/refs/debug.c +++ b/refs/debug.c @@ -116,11 +116,11 @@ static int debug_transaction_abort(struct ref_store *refs, return res; } -static int debug_pack_refs(struct ref_store *ref_store, struct pack_refs_opts *opts) +static int debug_optimize(struct ref_store *ref_store, struct pack_refs_opts *opts) { struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store; - int res = drefs->refs->be->pack_refs(drefs->refs, opts); - trace_printf_key(&trace_refs, "pack_refs: %d\n", res); + int res = drefs->refs->be->optimize(drefs->refs, opts); + trace_printf_key(&trace_refs, "optimize: %d\n", res); return res; } @@ -430,7 +430,7 @@ struct ref_storage_be refs_be_debug = { .transaction_finish = debug_transaction_finish, .transaction_abort = debug_transaction_abort, - .pack_refs = debug_pack_refs, + .optimize = debug_optimize, .rename_ref = debug_rename_ref, .copy_ref = debug_copy_ref, diff --git a/refs/files-backend.c b/refs/files-backend.c index 5aeb454fb47684..d13b87e056cf5e 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -1444,8 +1444,8 @@ static int should_pack_refs(struct files_ref_store *refs, return 0; } -static int files_pack_refs(struct ref_store *ref_store, - struct pack_refs_opts *opts) +static int files_optimize(struct ref_store *ref_store, + struct pack_refs_opts *opts) { struct files_ref_store *refs = files_downcast(ref_store, REF_STORE_WRITE | REF_STORE_ODB, @@ -1512,15 +1512,6 @@ static int files_pack_refs(struct ref_store *ref_store, return 0; } -static int files_optimize(struct ref_store *ref_store, struct pack_refs_opts *opts) -{ - /* - * For the "files" backend, "optimizing" is the same as "packing". - * So, we just call the existing worker function for packing. - */ - return files_pack_refs(ref_store, opts); -} - /* * People using contrib's git-new-workdir have .git/logs/refs -> * /some/other/path/.git/logs/refs, and that may live on another device. @@ -3975,7 +3966,6 @@ struct ref_storage_be refs_be_files = { .transaction_finish = files_transaction_finish, .transaction_abort = files_transaction_abort, - .pack_refs = files_pack_refs, .optimize = files_optimize, .rename_ref = files_rename_ref, .copy_ref = files_copy_ref, diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 1ab0c503930164..20cf9fab18e2e9 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -1773,8 +1773,8 @@ static int packed_transaction_finish(struct ref_store *ref_store, return ret; } -static int packed_pack_refs(struct ref_store *ref_store UNUSED, - struct pack_refs_opts *pack_opts UNUSED) +static int packed_optimize(struct ref_store *ref_store UNUSED, + struct pack_refs_opts *pack_opts UNUSED) { /* * Packed refs are already packed. It might be that loose refs @@ -2129,7 +2129,7 @@ struct ref_storage_be refs_be_packed = { .transaction_finish = packed_transaction_finish, .transaction_abort = packed_transaction_abort, - .pack_refs = packed_pack_refs, + .optimize = packed_optimize, .rename_ref = NULL, .copy_ref = NULL, diff --git a/refs/refs-internal.h b/refs/refs-internal.h index 4671517dade968..fc5149df5b3c5c 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -422,8 +422,6 @@ typedef int ref_transaction_commit_fn(struct ref_store *refs, struct ref_transaction *transaction, struct strbuf *err); -typedef int pack_refs_fn(struct ref_store *ref_store, - struct pack_refs_opts *opts); typedef int optimize_fn(struct ref_store *ref_store, struct pack_refs_opts *opts); typedef int rename_ref_fn(struct ref_store *ref_store, @@ -550,7 +548,6 @@ struct ref_storage_be { ref_transaction_finish_fn *transaction_finish; ref_transaction_abort_fn *transaction_abort; - pack_refs_fn *pack_refs; optimize_fn *optimize; rename_ref_fn *rename_ref; copy_ref_fn *copy_ref; diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index 6bbfd5618dac16..43cc66a48e9143 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -1700,11 +1700,11 @@ static int reftable_be_transaction_finish(struct ref_store *ref_store UNUSED, return ret; } -static int reftable_be_pack_refs(struct ref_store *ref_store, - struct pack_refs_opts *opts) +static int reftable_be_optimize(struct ref_store *ref_store, + struct pack_refs_opts *opts) { struct reftable_ref_store *refs = - reftable_be_downcast(ref_store, REF_STORE_WRITE | REF_STORE_ODB, "pack_refs"); + reftable_be_downcast(ref_store, REF_STORE_WRITE | REF_STORE_ODB, "optimize_refs"); struct reftable_stack *stack; int ret; @@ -1733,12 +1733,6 @@ static int reftable_be_pack_refs(struct ref_store *ref_store, return ret; } -static int reftable_be_optimize(struct ref_store *ref_store, - struct pack_refs_opts *opts) -{ - return reftable_be_pack_refs(ref_store, opts); -} - struct write_create_symref_arg { struct reftable_ref_store *refs; struct reftable_stack *stack; @@ -2761,7 +2755,6 @@ struct ref_storage_be refs_be_reftable = { .transaction_finish = reftable_be_transaction_finish, .transaction_abort = reftable_be_transaction_abort, - .pack_refs = reftable_be_pack_refs, .optimize = reftable_be_optimize, .rename_ref = reftable_be_rename_ref, .copy_ref = reftable_be_copy_ref, From 2cd99d984122f7f1cd7c3b153ee0a0d566831b30 Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Mon, 20 Oct 2025 10:18:30 +0200 Subject: [PATCH 26/29] refs: rename 'pack_refs_opts' to 'refs_optimize_opts' The previous commit removed all references to 'pack_refs()' within the refs subsystem. Continue this cleanup by also renaming 'pack_refs_opts' to 'refs_optimize_opts' and the respective flags accordingly. Keeping the naming consistent will make the code easier to maintain. Signed-off-by: Karthik Nayak Signed-off-by: Junio C Hamano --- pack-refs.c | 20 ++++++++++---------- refs.c | 2 +- refs.h | 18 +++++++++--------- refs/debug.c | 2 +- refs/files-backend.c | 10 +++++----- refs/packed-backend.c | 2 +- refs/refs-internal.h | 2 +- refs/reftable-backend.c | 4 ++-- 8 files changed, 30 insertions(+), 30 deletions(-) diff --git a/pack-refs.c b/pack-refs.c index 1a5e07d8b888ab..eb6b2ba2c2c2b5 100644 --- a/pack-refs.c +++ b/pack-refs.c @@ -14,10 +14,10 @@ int pack_refs_core(int argc, { struct ref_exclusions excludes = REF_EXCLUSIONS_INIT; struct string_list included_refs = STRING_LIST_INIT_NODUP; - struct pack_refs_opts pack_refs_opts = { + struct refs_optimize_opts optimize_opts = { .exclusions = &excludes, .includes = &included_refs, - .flags = PACK_REFS_PRUNE, + .flags = REFS_OPTIMIZE_PRUNE, }; struct string_list option_excluded_refs = STRING_LIST_INIT_NODUP; struct string_list_item *item; @@ -26,9 +26,9 @@ int pack_refs_core(int argc, struct option opts[] = { OPT_BOOL(0, "all", &pack_all, N_("pack everything")), - OPT_BIT(0, "prune", &pack_refs_opts.flags, N_("prune loose refs (default)"), PACK_REFS_PRUNE), - OPT_BIT(0, "auto", &pack_refs_opts.flags, N_("auto-pack refs as needed"), PACK_REFS_AUTO), - OPT_STRING_LIST(0, "include", pack_refs_opts.includes, N_("pattern"), + OPT_BIT(0, "prune", &optimize_opts.flags, N_("prune loose refs (default)"), REFS_OPTIMIZE_PRUNE), + OPT_BIT(0, "auto", &optimize_opts.flags, N_("auto-pack refs as needed"), REFS_OPTIMIZE_AUTO), + OPT_STRING_LIST(0, "include", optimize_opts.includes, N_("pattern"), N_("references to include")), OPT_STRING_LIST(0, "exclude", &option_excluded_refs, N_("pattern"), N_("references to exclude")), @@ -39,15 +39,15 @@ int pack_refs_core(int argc, usage_with_options(usage_opts, opts); for_each_string_list_item(item, &option_excluded_refs) - add_ref_exclusion(pack_refs_opts.exclusions, item->string); + add_ref_exclusion(optimize_opts.exclusions, item->string); if (pack_all) - string_list_append(pack_refs_opts.includes, "*"); + string_list_append(optimize_opts.includes, "*"); - if (!pack_refs_opts.includes->nr) - string_list_append(pack_refs_opts.includes, "refs/tags/*"); + if (!optimize_opts.includes->nr) + string_list_append(optimize_opts.includes, "refs/tags/*"); - ret = refs_optimize(get_main_ref_store(repo), &pack_refs_opts); + ret = refs_optimize(get_main_ref_store(repo), &optimize_opts); clear_ref_exclusions(&excludes); string_list_clear(&included_refs, 0); diff --git a/refs.c b/refs.c index b9a4a606462d71..0d0831f29ba25b 100644 --- a/refs.c +++ b/refs.c @@ -2313,7 +2313,7 @@ void base_ref_store_init(struct ref_store *refs, struct repository *repo, refs->gitdir = xstrdup(path); } -int refs_optimize(struct ref_store *refs, struct pack_refs_opts *opts) +int refs_optimize(struct ref_store *refs, struct refs_optimize_opts *opts) { return refs->be->optimize(refs, opts); } diff --git a/refs.h b/refs.h index 8ff591ea95c7d9..6b05bba527ffca 100644 --- a/refs.h +++ b/refs.h @@ -499,16 +499,16 @@ void refs_warn_dangling_symrefs(struct ref_store *refs, FILE *fp, const struct string_list *refnames); /* - * Flags for controlling behaviour of pack_refs() - * PACK_REFS_PRUNE: Prune loose refs after packing - * PACK_REFS_AUTO: Pack refs on a best effort basis. The heuristics and end - * result are decided by the ref backend. Backends may ignore - * this flag and fall back to a normal repack. + * Flags for controlling behaviour of refs_optimize() + * REFS_OPTIMIZE_PRUNE: Prune loose refs after packing + * REFS_OPTIMIZE_AUTO: Pack refs on a best effort basis. The heuristics and end + * result are decided by the ref backend. Backends may ignore + * this flag and fall back to a normal repack. */ -#define PACK_REFS_PRUNE (1 << 0) -#define PACK_REFS_AUTO (1 << 1) +#define REFS_OPTIMIZE_PRUNE (1 << 0) +#define REFS_OPTIMIZE_AUTO (1 << 1) -struct pack_refs_opts { +struct refs_optimize_opts { unsigned int flags; struct ref_exclusions *exclusions; struct string_list *includes; @@ -518,7 +518,7 @@ struct pack_refs_opts { * Optimize the ref store. The exact behavior is up to the backend. * For the files backend, this is equivalent to packing refs. */ -int refs_optimize(struct ref_store *refs, struct pack_refs_opts *opts); +int refs_optimize(struct ref_store *refs, struct refs_optimize_opts *opts); /* * Setup reflog before using. Fill in err and return -1 on failure. diff --git a/refs/debug.c b/refs/debug.c index 40cd1d9c1540c6..2defd2d465712e 100644 --- a/refs/debug.c +++ b/refs/debug.c @@ -116,7 +116,7 @@ static int debug_transaction_abort(struct ref_store *refs, return res; } -static int debug_optimize(struct ref_store *ref_store, struct pack_refs_opts *opts) +static int debug_optimize(struct ref_store *ref_store, struct refs_optimize_opts *opts) { struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store; int res = drefs->refs->be->optimize(drefs->refs, opts); diff --git a/refs/files-backend.c b/refs/files-backend.c index d13b87e056cf5e..23bb641f2c84ee 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -1355,7 +1355,7 @@ static void prune_refs(struct files_ref_store *refs, struct ref_to_prune **refs_ */ static int should_pack_ref(struct files_ref_store *refs, const struct reference *ref, - struct pack_refs_opts *opts) + struct refs_optimize_opts *opts) { struct string_list_item *item; @@ -1383,7 +1383,7 @@ static int should_pack_ref(struct files_ref_store *refs, } static int should_pack_refs(struct files_ref_store *refs, - struct pack_refs_opts *opts) + struct refs_optimize_opts *opts) { struct ref_iterator *iter; size_t packed_size; @@ -1391,7 +1391,7 @@ static int should_pack_refs(struct files_ref_store *refs, size_t limit; int ret; - if (!(opts->flags & PACK_REFS_AUTO)) + if (!(opts->flags & REFS_OPTIMIZE_AUTO)) return 1; ret = packed_refs_size(refs->packed_ref_store, &packed_size); @@ -1445,7 +1445,7 @@ static int should_pack_refs(struct files_ref_store *refs, } static int files_optimize(struct ref_store *ref_store, - struct pack_refs_opts *opts) + struct refs_optimize_opts *opts) { struct files_ref_store *refs = files_downcast(ref_store, REF_STORE_WRITE | REF_STORE_ODB, @@ -1488,7 +1488,7 @@ static int files_optimize(struct ref_store *ref_store, iter->ref.name, err.buf); /* Schedule the loose reference for pruning if requested. */ - if ((opts->flags & PACK_REFS_PRUNE)) { + if ((opts->flags & REFS_OPTIMIZE_PRUNE)) { struct ref_to_prune *n; FLEX_ALLOC_STR(n, name, iter->ref.name); oidcpy(&n->oid, iter->ref.oid); diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 20cf9fab18e2e9..10062fd8b63063 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -1774,7 +1774,7 @@ static int packed_transaction_finish(struct ref_store *ref_store, } static int packed_optimize(struct ref_store *ref_store UNUSED, - struct pack_refs_opts *pack_opts UNUSED) + struct refs_optimize_opts *opts UNUSED) { /* * Packed refs are already packed. It might be that loose refs diff --git a/refs/refs-internal.h b/refs/refs-internal.h index fc5149df5b3c5c..dee42f231dbd0a 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -423,7 +423,7 @@ typedef int ref_transaction_commit_fn(struct ref_store *refs, struct strbuf *err); typedef int optimize_fn(struct ref_store *ref_store, - struct pack_refs_opts *opts); + struct refs_optimize_opts *opts); typedef int rename_ref_fn(struct ref_store *ref_store, const char *oldref, const char *newref, const char *logmsg); diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index 43cc66a48e9143..c23c45f3bf4639 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -1701,7 +1701,7 @@ static int reftable_be_transaction_finish(struct ref_store *ref_store UNUSED, } static int reftable_be_optimize(struct ref_store *ref_store, - struct pack_refs_opts *opts) + struct refs_optimize_opts *opts) { struct reftable_ref_store *refs = reftable_be_downcast(ref_store, REF_STORE_WRITE | REF_STORE_ODB, "optimize_refs"); @@ -1715,7 +1715,7 @@ static int reftable_be_optimize(struct ref_store *ref_store, if (!stack) stack = refs->main_backend.stack; - if (opts->flags & PACK_REFS_AUTO) + if (opts->flags & REFS_OPTIMIZE_AUTO) ret = reftable_stack_auto_compact(stack); else ret = reftable_stack_compact_all(stack, NULL); From c113f4ca4dbba5529af645f7d7837c7dae12b403 Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Mon, 20 Oct 2025 10:18:31 +0200 Subject: [PATCH 27/29] t/pack-refs-tests: move the 'test_done' to callees In ac0bad0af4 (t0601: refactor tests to be shareable, 2025-09-19), we refactored 't/t0601-reffiles-pack-refs.sh' to move all of the tests to 't/pack-refs-tests.sh', which became a common test suite which was also used by 't/t1463-refs-optimize.sh'. This also moved the 'test_done' directive to 't/pack-refs-tests.sh'. Which inhibits additional tests from being added to either of the tests. Let's move the directive out to both the tests, so that we can add additional specific tests to them. Also the test flow logic shouldn't be part of tests which can be embedded in other test scripts. Signed-off-by: Karthik Nayak Signed-off-by: Junio C Hamano --- t/pack-refs-tests.sh | 2 -- t/t0601-reffiles-pack-refs.sh | 2 ++ t/t1463-refs-optimize.sh | 2 ++ 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/t/pack-refs-tests.sh b/t/pack-refs-tests.sh index 095823d915fd63..81086c369089b9 100644 --- a/t/pack-refs-tests.sh +++ b/t/pack-refs-tests.sh @@ -459,5 +459,3 @@ test_expect_success 'pack-refs does not store invalid peeled tag value' ' test_grep ! "^\^" .git/packed-refs ) ' - -test_done diff --git a/t/t0601-reffiles-pack-refs.sh b/t/t0601-reffiles-pack-refs.sh index 12cf5d1dcba814..3c706978efc219 100755 --- a/t/t0601-reffiles-pack-refs.sh +++ b/t/t0601-reffiles-pack-refs.sh @@ -18,3 +18,5 @@ export GIT_TEST_DEFAULT_REF_FORMAT . ./test-lib.sh . "$TEST_DIRECTORY"/pack-refs-tests.sh + +test_done diff --git a/t/t1463-refs-optimize.sh b/t/t1463-refs-optimize.sh index c11c905d795d26..9afe3c1ed7e33a 100755 --- a/t/t1463-refs-optimize.sh +++ b/t/t1463-refs-optimize.sh @@ -15,3 +15,5 @@ export GIT_TEST_DEFAULT_REF_FORMAT pack_refs='refs optimize' . "$TEST_DIRECTORY"/pack-refs-tests.sh + +test_done From 7048e74609fbef2c91bfa3a80e3a9c4fc0ac04c9 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 6 Nov 2025 09:52:54 +0100 Subject: [PATCH 28/29] object: fix performance regression when peeling tags MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Our Bencher dashboards [1] have recently alerted us about a bunch of performance regressions when writing references, specifically with the reftable backend. There is a 3x regression when writing many refs with preexisting refs in the reftable format, and a 10x regression when migrating refs between backends in either of the formats. Bisecting the issue lands us at 6ec4c0b45b (refs: don't store peeled object IDs for invalid tags, 2025-10-23). The gist of the commit is that we may end up storing peeled objects in both reftables and packed-refs for corrupted tags, where the claimed tagged object type is different than the actual tagged object type. This will then cause us to create the `struct object *` with a wrong type, as well, and obviously nothing good comes out of that. The fix for this issue was to introduce a new flag to `peel_object()` that causes us to verify the tagged object's type before writing it into the refdb -- if the tag is corrupt, we skip writing the peeled value. To verify whether the peeled value is correct we have to look up the object type via the ODB and compare the actual type with the claimed type, and that additional object lookup is costly. This also explains why we see the regression only when writing refs with the reftable backend, but we see the regression with both backends when migrating refs: - The reftable backend knows to store peeled values in the new table immediately, so it has to try and peel each ref it's about to write to the transaction. So the performance regression is visible for all writes. - The files backend only stores peeled values when writing the packed-refs file, so it wouldn't hit the performance regression for normal writes. But on ref migrations we know to write all new values into the packed-refs file immediately, and that's why we see the regression for both backends there. Taking a step back though reveals an oddity in the new verification logic: we not only verify the _tagged_ object's type, but we also verify the type of the tag itself. But this isn't really needed, as we wouldn't hit the bug in such a case anyway, as we only hit the issue with corrupt tags claiming an invalid type for the tagged object. The consequence of this is that we now started to look up the target object of every single reference we're about to write, regardless of whether it even is a tag or not. And that is of course quite costly. Fix the issue by only verifying the type of the tagged objects. This means that we of course still have a performance hit for actual tags. But this only happens for writes anyway, and I'd claim it's preferable to not store corrupted data in the refdb than to be fast here. Rename the flag accordingly to clarify that we only verify the tagged object's type. This fix brings performance back to previous levels: Benchmark 1: baseline Time (mean ± σ): 46.0 ms ± 0.4 ms [User: 40.0 ms, System: 5.7 ms] Range (min … max): 45.0 ms … 47.1 ms 54 runs Benchmark 2: regression Time (mean ± σ): 140.2 ms ± 1.3 ms [User: 77.5 ms, System: 60.5 ms] Range (min … max): 138.0 ms … 142.7 ms 20 runs Benchmark 3: fix Time (mean ± σ): 46.2 ms ± 0.4 ms [User: 40.2 ms, System: 5.7 ms] Range (min … max): 45.0 ms … 47.3 ms 55 runs Summary update-ref: baseline 1.00 ± 0.01 times faster than fix 3.05 ± 0.04 times faster than regression [1]: https://bencher.dev/perf/git/plots Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- object.c | 4 ++-- object.h | 12 ++++++------ ref-filter.c | 2 +- refs/packed-backend.c | 2 +- refs/reftable-backend.c | 2 +- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/object.c b/object.c index e72b0ed4360e67..b08fc7a163ae69 100644 --- a/object.c +++ b/object.c @@ -214,7 +214,7 @@ enum peel_status peel_object(struct repository *r, { struct object *o = lookup_unknown_object(r, name); - if (o->type == OBJ_NONE || flags & PEEL_OBJECT_VERIFY_OBJECT_TYPE) { + if (o->type == OBJ_NONE) { int type = odb_read_object_info(r->objects, name, NULL); if (type < 0 || !object_as_type(o, type, 0)) return PEEL_INVALID; @@ -228,7 +228,7 @@ enum peel_status peel_object(struct repository *r, if (o && o->type == OBJ_TAG && ((struct tag *)o)->tagged) { o = ((struct tag *)o)->tagged; - if (flags & PEEL_OBJECT_VERIFY_OBJECT_TYPE) { + if (flags & PEEL_OBJECT_VERIFY_TAGGED_OBJECT_TYPE) { int type = odb_read_object_info(r->objects, &o->oid, NULL); if (type < 0 || !object_as_type(o, type, 0)) return PEEL_INVALID; diff --git a/object.h b/object.h index 1499f63d507c32..e9baade1e0ecab 100644 --- a/object.h +++ b/object.h @@ -289,13 +289,13 @@ enum peel_status { enum peel_object_flags { /* - * Always verify the object type, even in the case where the looked-up - * object already has an object type. This can be useful when the - * stored object type may be invalid. One such case is when looking up - * objects via tags, where we blindly trust the object type declared by - * the tag. + * Always verify the object type of the tagged object, even in the case + * where the looked-up object already has an object type. This can be + * useful when the tagged object type may be invalid. One such case is + * when looking up objects via tags, where we blindly trust the object + * type declared by the tag. */ - PEEL_OBJECT_VERIFY_OBJECT_TYPE = (1 << 0), + PEEL_OBJECT_VERIFY_TAGGED_OBJECT_TYPE = (1 << 0), }; /* diff --git a/ref-filter.c b/ref-filter.c index d8667c569a18f1..d7454269e87cd3 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -2654,7 +2654,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) if (!is_null_oid(&ref->peeled_oid)) { oidcpy(&oi_deref.oid, &ref->peeled_oid); } else if (!peel_object(the_repository, &oi.oid, &oi_deref.oid, - PEEL_OBJECT_VERIFY_OBJECT_TYPE)) { + PEEL_OBJECT_VERIFY_TAGGED_OBJECT_TYPE)) { /* We managed to peel the object ourselves. */ } else { die("bad tag"); diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 1ab0c503930164..5aa615011a6db3 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -1528,7 +1528,7 @@ static enum ref_transaction_error write_with_updates(struct packed_ref_store *re } else { struct object_id peeled; int peel_error = peel_object(refs->base.repo, &update->new_oid, - &peeled, PEEL_OBJECT_VERIFY_OBJECT_TYPE); + &peeled, PEEL_OBJECT_VERIFY_TAGGED_OBJECT_TYPE); if (write_packed_entry(out, update->refname, &update->new_oid, diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index 6bbfd5618dac16..1ac1f6156f256d 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -1633,7 +1633,7 @@ static int write_transaction_table(struct reftable_writer *writer, void *cb_data ref.update_index = ts; peel_error = peel_object(arg->refs->base.repo, &u->new_oid, &peeled, - PEEL_OBJECT_VERIFY_OBJECT_TYPE); + PEEL_OBJECT_VERIFY_TAGGED_OBJECT_TYPE); if (!peel_error) { ref.value_type = REFTABLE_REF_VAL2; memcpy(ref.value.val2.target_value, peeled.hash, GIT_MAX_RAWSZ); From 5e6e4854e086ba0025bc7dc11e6b475c92a2f556 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 19 Nov 2025 10:55:15 -0800 Subject: [PATCH 29/29] Start 2.53 cycle Signed-off-by: Junio C Hamano --- Documentation/RelNotes/2.53.0.adoc | 12 ++++++++++++ GIT-VERSION-GEN | 2 +- RelNotes | 2 +- 3 files changed, 14 insertions(+), 2 deletions(-) create mode 100644 Documentation/RelNotes/2.53.0.adoc diff --git a/Documentation/RelNotes/2.53.0.adoc b/Documentation/RelNotes/2.53.0.adoc new file mode 100644 index 00000000000000..b0b3dc9b3dcc26 --- /dev/null +++ b/Documentation/RelNotes/2.53.0.adoc @@ -0,0 +1,12 @@ +Git v2.53 Release Notes +======================= + +Performance, Internal Implementation, Development Support etc. +-------------------------------------------------------------- + + * The list of packfiles used in a running Git process is moved from + the packed_git structure into the packfile store. + + * Some ref backend storage can hold not just the object name of an + annotated tag, but the object name of the object the tag points at. + The code to handle this information has been streamlined. diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN index 8d5bbf7b6d3efd..1f7af0328a0461 100755 --- a/GIT-VERSION-GEN +++ b/GIT-VERSION-GEN @@ -1,6 +1,6 @@ #!/bin/sh -DEF_VER=v2.52.0 +DEF_VER=v2.52.GIT LF=' ' diff --git a/RelNotes b/RelNotes index 6d16c0077a11cb..6dfd3d1bcf2f33 120000 --- a/RelNotes +++ b/RelNotes @@ -1 +1 @@ -Documentation/RelNotes/2.52.0.adoc \ No newline at end of file +Documentation/RelNotes/2.53.0.adoc \ No newline at end of file