From: Patrick Steinhardt <ps@pks.im>
To: git@vger.kernel.org
Cc: Jeff King <peff@peff.net>, Taylor Blau <me@ttaylorr.com>
Subject: [PATCH 3/8] http: refactor subsystem to use `packfile_list`s
Date: Tue, 28 Oct 2025 12:08:33 +0100 [thread overview]
Message-ID: <20251028-pks-packfiles-store-drop-list-v1-3-1a3b82030a7a@pks.im> (raw)
In-Reply-To: <20251028-pks-packfiles-store-drop-list-v1-0-1a3b82030a7a@pks.im>
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 <ps@pks.im>
---
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 a1c01e3b9b9..d86ce771198 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 0f7ae46d7f1..e886e648664 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 17130823f00..41f850db16d 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 553e16205ce..f9d45934047 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 4d2d3b674f3..6aa2ca8ac9e 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 39ed1073e4a..a53336d722a 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.
*/
--
2.51.2.997.g839fc31de9.dirty
next prev parent reply other threads:[~2025-10-28 11:11 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-10-28 11:08 [PATCH 0/8] packfiles: track pack lists via the packfile store Patrick Steinhardt
2025-10-28 11:08 ` [PATCH 1/8] packfile: use a `strmap` to store packs by name Patrick Steinhardt
2025-10-29 22:16 ` Taylor Blau
2025-10-28 11:08 ` [PATCH 2/8] packfile: move the MRU list into the packfile store Patrick Steinhardt
2025-10-29 22:39 ` Taylor Blau
2025-10-30 8:59 ` Patrick Steinhardt
2025-10-28 11:08 ` Patrick Steinhardt [this message]
2025-10-29 14:24 ` [PATCH 3/8] http: refactor subsystem to use `packfile_list`s Toon Claes
2025-10-30 8:58 ` Patrick Steinhardt
2025-10-28 11:08 ` [PATCH 4/8] packfile: fix approximation of object counts Patrick Steinhardt
2025-10-29 22:49 ` Taylor Blau
2025-10-30 8:58 ` Patrick Steinhardt
2025-10-28 11:08 ` [PATCH 5/8] builtin/pack-objects: simplify logic to find kept or nonlocal objects Patrick Steinhardt
2025-10-29 14:55 ` Toon Claes
2025-10-29 23:15 ` Taylor Blau
2025-10-30 8:59 ` Patrick Steinhardt
2025-10-29 23:13 ` Taylor Blau
2025-10-30 8:58 ` Patrick Steinhardt
2025-10-30 9:31 ` Toon Claes
2025-10-30 9:52 ` Patrick Steinhardt
2025-10-28 11:08 ` [PATCH 6/8] packfile: move list of packs into the packfile store Patrick Steinhardt
2025-10-28 11:08 ` [PATCH 7/8] packfile: always add packfiles to MRU when adding a pack Patrick Steinhardt
2025-10-29 23:25 ` Taylor Blau
2025-10-30 8:58 ` Patrick Steinhardt
2025-10-28 11:08 ` [PATCH 8/8] packfile: track packs via the MRU list exclusively Patrick Steinhardt
2025-10-30 10:38 ` [PATCH v2 0/8] packfiles: track pack lists via the packfile store Patrick Steinhardt
2025-10-30 10:38 ` [PATCH v2 1/8] packfile: use a `strmap` to store packs by name Patrick Steinhardt
2025-10-30 10:38 ` [PATCH v2 2/8] packfile: move the MRU list into the packfile store Patrick Steinhardt
2025-10-30 10:38 ` [PATCH v2 3/8] http: refactor subsystem to use `packfile_list`s Patrick Steinhardt
2025-10-30 10:38 ` [PATCH v2 4/8] packfile: fix approximation of object counts Patrick Steinhardt
2025-10-30 10:38 ` [PATCH v2 5/8] builtin/pack-objects: simplify logic to find kept or nonlocal objects Patrick Steinhardt
2025-10-30 10:38 ` [PATCH v2 6/8] packfile: move list of packs into the packfile store Patrick Steinhardt
2025-10-30 10:38 ` [PATCH v2 7/8] packfile: always add packfiles to MRU when adding a pack Patrick Steinhardt
2025-10-30 10:38 ` [PATCH v2 8/8] packfile: track packs via the MRU list exclusively Patrick Steinhardt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20251028-pks-packfiles-store-drop-list-v1-3-1a3b82030a7a@pks.im \
--to=ps@pks.im \
--cc=git@vger.kernel.org \
--cc=me@ttaylorr.com \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).