@matheusbsbs / matheus-bagliano / commits / 4d99559147

packfile: convert find_sha1_pack() to use object_id

The find_sha1_pack() function has a few problems: - it's badly named, since it works with any object hash - it takes the hash as a bare pointer rather than an object_id struct We can fix both of these easily, as all callers actually have a real object_id anyway. I also found the existence of this function somewhat confusing, as it is about looking in an arbitrary set of linked packed_git structs. It's good for things like dumb-http which are looking in downloaded remote packs, and not our local packs. But despite the name, it is not a good way to find the pack which contains a local object (it skips the use of the midx, the pack mru list, and so on). So let's also add an explanatory comment above the declaration that may point people in the right direction. I suspect the calls in fast-import.c, which use the packed_git list from the repository struct, could actually just be using find_pack_entry(). But since we'd need to keep it anyway for dumb-http, I didn't dig further there. If we eventually drop dumb-http support, then it might be worth examining them to see if we can get rid of the function entirely. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Taylor Blau <me@ttaylorr.com>

Jeff King committed Oct 25, 2024 at 03:05 UTC 4d995591476f0e0b11c512e4d711541118ea2b79

5 files changed +15 -12

builtin/fast-import.c

+2 -4

@@ -966,8 +966,7 @@ static int store_object(

966 if (e->idx.offset) {

967 duplicate_count_by_type[type]++;

968 return 1;

969 - } else if (find_sha1_pack(oid.hash,

970 - get_all_packs(the_repository))) {

969 + } else if (find_oid_pack(&oid, get_all_packs(the_repository))) {

970 e->type = type;

971 e->pack_id = MAX_PACK_ID;

972 e->idx.offset = 1; /* just not zero! */

@@ -1167,8 +1166,7 @@ static void stream_blob(uintmax_t len, struct object_id *oidout, uintmax_t mark)

1166 duplicate_count_by_type[OBJ_BLOB]++;

1167 truncate_pack(&checkpoint);

1168

1170 - } else if (find_sha1_pack(oid.hash,

1171 - get_all_packs(the_repository))) {

1169 + } else if (find_oid_pack(&oid, get_all_packs(the_repository))) {

1170 e->type = OBJ_BLOB;

1171 e->pack_id = MAX_PACK_ID;

1172 e->idx.offset = 1; /* just not zero! */

http-push.c

+2 -2

@@ -309,7 +309,7 @@ static void start_fetch_packed(struct transfer_request *request)

309 struct transfer_request *check_request = request_queue_head;

310 struct http_pack_request *preq;

311

312 - target = find_sha1_pack(request->obj->oid.hash, repo->packs);

312 + target = find_oid_pack(&request->obj->oid, repo->packs);

313 if (!target) {

314 fprintf(stderr, "Unable to fetch %s, will not be able to update server info refs\n", oid_to_hex(&request->obj->oid));

315 repo->can_update_info_refs = 0;

@@ -681,7 +681,7 @@ static int add_send_request(struct object *obj, struct remote_lock *lock)

681 get_remote_object_list(obj->oid.hash[0]);

682 if (obj->flags & (REMOTE | PUSHING))

683 return 0;

684 - target = find_sha1_pack(obj->oid.hash, repo->packs);

684 + target = find_oid_pack(&obj->oid, repo->packs);

685 if (target) {

686 obj->flags |= REMOTE;

687 return 0;

http-walker.c

+1 -1

@@ -432,7 +432,7 @@ static int http_fetch_pack(struct walker *walker, struct alt_base *repo,

432

433 if (fetch_indices(walker, repo))

434 return -1;

435 - target = find_sha1_pack(oid->hash, repo->packs);

435 + target = find_oid_pack(oid, repo->packs);

436 if (!target)

437 return -1;

438 close_pack_index(target);

packfile.c

+3 -3

@@ -2010,13 +2010,13 @@ int is_pack_valid(struct packed_git *p)

2010 return !open_packed_git(p);

2011 }

2012

2013 -struct packed_git *find_sha1_pack(const unsigned char *sha1,

2014 - struct packed_git *packs)

2013 +struct packed_git *find_oid_pack(const struct object_id *oid,

2014 + struct packed_git *packs)

2015 {

2016 struct packed_git *p;

2017

2018 for (p = packs; p; p = p->next) {

2019 - if (find_pack_entry_one(sha1, p))

2019 + if (find_pack_entry_one(oid->hash, p))

2020 return p;

2021 }

2022 return NULL;

packfile.h

+7 -2

@@ -79,8 +79,13 @@ struct packed_git *get_all_packs(struct repository *r);

79 */

80 unsigned long repo_approximate_object_count(struct repository *r);

82 -struct packed_git *find_sha1_pack(const unsigned char *sha1,

83 - struct packed_git *packs);

82 +/*

83 + * Find the pack within the "packs" list whose index contains the object "oid".

84 + * For general object lookups, you probably don't want this; use

85 + * find_pack_entry() instead.

86 + */

87 +struct packed_git *find_oid_pack(const struct object_id *oid,

88 + struct packed_git *packs);

90 void pack_report(void);