{"thread":{"id":"64646","subject":"[PATCH 0/8] Improvements for reading object info","startedAt":"2025-12-18T06:28:24Z","lastAt":"2026-01-12T14:54:10Z","messageCount":58,"participants":["Patrick Steinhardt","Junio C Hamano","Kristoffer Haugsbakk","Toon Claes","Karthik Nayak"],"isPatch":true,"patchVersion":1,"patchTotal":8},"messages":[{"id":"532413","messageId":"20251218-b4-pks-odb-read-object-info-improvements-v1-0-81c8368492be@pks.im","threadId":"64646","inReplyTo":null,"subject":"[PATCH 0/8] Improvements for reading object info","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-18T06:28:10Z","receivedAt":"2025-12-18T06:28:24Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nthis patch series contains various small improvements for reading object\ninfo for either loose or packed objects. These improvements were split\nout of a larger patch series where I'm about to introduce a new generic\n`odb_for_each_object()` function.\n\nThis series has a conflict with ps/packfile-store-in-odb-source. I\ndecided to not make this a dependency though because those two topics\nare independent from one another, and I expect that this series here\nwill be merged down faster than the conflicting one. Furthermore, the\nconflict itself is quite minor:\n\ndiff --cc packfile.c\nindex 8daa5a5ee7,ce6716fbea..0000000000\n--- a/packfile.c\n+++ b/packfile.c\n@@@ -2157,10 -2132,11 +2151,10 @@@ int packfile_store_read_object_info(str\n \t\t\t\t struct object_info *oi,\n \t\t\t\t unsigned flags UNUSED)\n {\n -\tstatic struct object_info blank_oi = OBJECT_INFO_INIT;\n \tstruct pack_entry e;\n -\tint rtype;\n +\tint ret;\n \n- \tif (!find_pack_entry(store->odb->repo, oid, &e))\n+ \tif (!find_pack_entry(store, oid, &e))\n \t\treturn 1;\n \n \t/*\n@@@ -2549,9 -2555,8 +2571,9 @@@ int packfile_store_read_object_stream(s\n \toi.sizep = &size;\n \n \tif (packfile_store_read_object_info(store, oid, &oi, 0) ||\n -\t oi.u.packed.is_delta ||\n +\t oi.u.packed.type == PACKED_OBJECT_TYPE_REF_DELTA ||\n +\t oi.u.packed.type == PACKED_OBJECT_TYPE_OFS_DELTA ||\n- \t repo_settings_get_big_file_threshold(store->odb->repo) >= size)\n+ \t repo_settings_get_big_file_threshold(store->source->odb->repo) >= size)\n \t\treturn -1;\n \n \tin_pack_type = unpack_object_header(oi.u.packed.pack,\n\nI'd thus propose to merge this series via an evil merge, but if this\nproves to be burdensome I'm happy to defer it to a later point. Just let\nme know and I'll adapt accordingly, thanks!\n\nThis also fixes the issue reported in .\n\nPatrick\n\n---\nPatrick Steinhardt (8):\n object-file: always set OI_LOOSE when reading object info\n packfile: always declare object info to be OI_PACKED\n packfile: extend `is_delta` field to allow for \"unknown\" state\n packfile: always populate pack-specific info when reading object info\n packfile: disentangle return value of `packed_object_info()`\n packfile: skip unpacking object header for disk size requests\n packfile: fix short-circuiting of empty requests\n packfile: drop repository parameter from `packed_object_info()`\n\n builtin/cat-file.c | 3 +--\n builtin/pack-objects.c | 4 +--\n commit-graph.c | 2 +-\n object-file.c | 13 ++++++++--\n odb.h | 18 +++++++++++--\n pack-bitmap.c | 3 +--\n packfile.c | 69 +++++++++++++++++++++++++++++++-------------------\n packfile.h | 7 +++--\n 8 files changed, 80 insertions(+), 39 deletions(-)\n\n\n---\nbase-commit: c4a0c8845e2426375ad257b6c221a3a7d92ecfda\nchange-id: 20251215-b4-pks-odb-read-object-info-improvements-0e031ef827d2\n\n"},{"id":"532414","messageId":"20251218-b4-pks-odb-read-object-info-improvements-v1-1-81c8368492be@pks.im","threadId":"64646","inReplyTo":"20251218-b4-pks-odb-read-object-info-improvements-v1-0-81c8368492be@pks.im","subject":"[PATCH 1/8] object-file: always set OI_LOOSE when reading object info","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-18T06:28:11Z","receivedAt":"2025-12-18T06:28:26Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"There are some early returns in ``odb_source_loose_read_object_info()`\nin cases where we don't have to open the loose object. These return\npaths do not set `struct object_info::whence` to `OI_LOOSE` though, so\nit becomes impossible for the caller to tell the format of such an\nobject.\n\nNobody seems to care about this right now, but it's a bug waiting to\nhappen. Fix this by always setting `whence` on success.\n\nSigned-off-by: Patrick Steinhardt \n---\n object-file.c | 13 +++++++++++--\n 1 file changed, 11 insertions(+), 2 deletions(-)\n\ndiff --git a/object-file.c b/object-file.c\nindex af1c3f972d..716b325669 100644\n--- a/object-file.c\n+++ b/object-file.c\n@@ -439,12 +439,21 @@ int odb_source_loose_read_object_info(struct odb_source *source,\n \t */\n \tif (!oi->typep && !oi->sizep && !oi->contentp) {\n \t\tstruct stat st;\n-\t\tif (!oi->disk_sizep && (flags & OBJECT_INFO_QUICK))\n-\t\t\treturn quick_has_loose(source->loose, oid) ? 0 : -1;\n+\n+\t\tif (!oi->disk_sizep && (flags & OBJECT_INFO_QUICK)) {\n+\t\t\tstatus = quick_has_loose(source->loose, oid) ? 0 : -1;\n+\t\t\tif (!status)\n+\t\t\t\toi->whence = OI_LOOSE;\n+\t\t\treturn status;\n+\t\t}\n+\n \t\tif (stat_loose_object(source->loose, oid, &st, &path) < 0)\n \t\t\treturn -1;\n+\n \t\tif (oi->disk_sizep)\n \t\t\t*oi->disk_sizep = st.st_size;\n+\n+\t\toi->whence = OI_LOOSE;\n \t\treturn 0;\n \t}\n \n\n-- \n2.52.0.351.gbe84eed79e.dirty\n\n"},{"id":"532415","messageId":"20251218-b4-pks-odb-read-object-info-improvements-v1-2-81c8368492be@pks.im","threadId":"64646","inReplyTo":"20251218-b4-pks-odb-read-object-info-improvements-v1-0-81c8368492be@pks.im","subject":"[PATCH 2/8] packfile: always declare object info to be OI_PACKED","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-18T06:28:12Z","receivedAt":"2025-12-18T06:28:30Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When reading object info via a packfile we yield one of two types:\n\n - The object can either be OI_PACKED, which is what a caller would\n typically expect.\n\n - Or it can be OI_DBCACHED if it is stored in the delta base cache.\n\nThe latter really is an implementation detail though, and callers\ntypically don't care at all about the difference. Furthermore, the\ninformation whether or not it is part of the delta base cache can\nalready be derived via the `is_delta` field, so the fact that we discern\nbetween OI_PACKED and OI_DBCACHED only further complicates the\ninterface.\n\nDrop the OI_DBCACHED enum completely. There don't seem to be any callers\nthat care about the distinction.\n\nSigned-off-by: Patrick Steinhardt \n---\n odb.h | 1 -\n packfile.c | 3 +--\n 2 files changed, 1 insertion(+), 3 deletions(-)\n\ndiff --git a/odb.h b/odb.h\nindex 014cd9585a..73b0b87ad5 100644\n--- a/odb.h\n+++ b/odb.h\n@@ -330,7 +330,6 @@ struct object_info {\n \t\tOI_CACHED,\n \t\tOI_LOOSE,\n \t\tOI_PACKED,\n-\t\tOI_DBCACHED\n \t} whence;\n \tunion {\n \t\t/*\ndiff --git a/packfile.c b/packfile.c\nindex c88bd92619..79ad9d7179 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -1656,8 +1656,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \t\t\toidclr(oi->delta_base_oid, p->repo->hash_algo);\n \t}\n \n-\toi->whence = in_delta_base_cache(p, obj_offset) ? OI_DBCACHED :\n-\t\t\t\t\t\t\t OI_PACKED;\n+\toi->whence = OI_PACKED;\n \n out:\n \tunuse_pack(&w_curs);\n\n-- \n2.52.0.351.gbe84eed79e.dirty\n\n"},{"id":"532416","messageId":"20251218-b4-pks-odb-read-object-info-improvements-v1-3-81c8368492be@pks.im","threadId":"64646","inReplyTo":"20251218-b4-pks-odb-read-object-info-improvements-v1-0-81c8368492be@pks.im","subject":"[PATCH 3/8] packfile: extend `is_delta` field to allow for \"unknown\" state","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-18T06:28:13Z","receivedAt":"2025-12-18T06:28:32Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The `struct object_info::u::packed::is_delta` field determines whether\nor not a specific object is stored as a delta. It only stores whether or\nnot the object is stored as delta, so it is treated as a boolean value.\n\nThis boolean is insufficient though: when reading a packed object via\n`packfile_store_read_object_info()` we know to skip parsing the actual\nobject when the user didn't request any object-specific data. In that\ncase we won't read the object itself, but will only look up its position\nin the packfile. Consequently, we do not know whether it is a delta or\nnot.\n\nThis isn't really an issue right now, as the check for an empty request\nis broken. But a subsequent commit will fix it, and once we do we will\nhave the need to also represent an \"unknown\" delta state.\n\nPrepare for this change by introducing a new enum that encodes the\nobject type. We don't use the \"unknown\" state just yet, but will start\nto do so in the next commit.\n\nSigned-off-by: Patrick Steinhardt \n---\n odb.h | 7 ++++++-\n packfile.c | 17 ++++++++++++++---\n 2 files changed, 20 insertions(+), 4 deletions(-)\n\ndiff --git a/odb.h b/odb.h\nindex 73b0b87ad5..afae5e5c01 100644\n--- a/odb.h\n+++ b/odb.h\n@@ -343,7 +343,12 @@ struct object_info {\n \t\tstruct {\n \t\t\tstruct packed_git *pack;\n \t\t\toff_t offset;\n-\t\t\tunsigned int is_delta;\n+\t\t\tenum packed_object_type {\n+\t\t\t\tPACKED_OBJECT_TYPE_UNKNOWN,\n+\t\t\t\tPACKED_OBJECT_TYPE_FULL,\n+\t\t\t\tPACKED_OBJECT_TYPE_OFS_DELTA,\n+\t\t\t\tPACKED_OBJECT_TYPE_REF_DELTA,\n+\t\t\t} type;\n \t\t} packed;\n \t} u;\n };\ndiff --git a/packfile.c b/packfile.c\nindex 79ad9d7179..9bce52f912 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -2160,8 +2160,18 @@ int packfile_store_read_object_info(struct packfile_store *store,\n \tif (oi->whence == OI_PACKED) {\n \t\toi->u.packed.offset = e.offset;\n \t\toi->u.packed.pack = e.p;\n-\t\toi->u.packed.is_delta = (rtype == OBJ_REF_DELTA ||\n-\t\t\t\t\t rtype == OBJ_OFS_DELTA);\n+\n+\t\tswitch (rtype) {\n+\t\tcase OBJ_REF_DELTA:\n+\t\t\toi->u.packed.type = PACKED_OBJECT_TYPE_REF_DELTA;\n+\t\t\tbreak;\n+\t\tcase OBJ_OFS_DELTA:\n+\t\t\toi->u.packed.type = PACKED_OBJECT_TYPE_OFS_DELTA;\n+\t\t\tbreak;\n+\t\tdefault:\n+\t\t\toi->u.packed.type = PACKED_OBJECT_TYPE_FULL;\n+\t\t\tbreak;\n+\t\t}\n \t}\n \n \treturn 0;\n@@ -2532,7 +2542,8 @@ int packfile_store_read_object_stream(struct odb_read_stream **out,\n \toi.sizep = &size;\n \n \tif (packfile_store_read_object_info(store, oid, &oi, 0) ||\n-\t oi.u.packed.is_delta ||\n+\t oi.u.packed.type == PACKED_OBJECT_TYPE_REF_DELTA ||\n+\t oi.u.packed.type == PACKED_OBJECT_TYPE_OFS_DELTA ||\n \t repo_settings_get_big_file_threshold(store->odb->repo) >= size)\n \t\treturn -1;\n \n\n-- \n2.52.0.351.gbe84eed79e.dirty\n\n"},{"id":"532417","messageId":"20251218-b4-pks-odb-read-object-info-improvements-v1-4-81c8368492be@pks.im","threadId":"64646","inReplyTo":"20251218-b4-pks-odb-read-object-info-improvements-v1-0-81c8368492be@pks.im","subject":"[PATCH 4/8] packfile: always populate pack-specific info when reading object info","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-18T06:28:14Z","receivedAt":"2025-12-18T06:28:35Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When reading object information from a packfile we are not always\npopulating the pack-specific information. This happens in two cases:\n\n - When calling `packed_object_info()` directly instead of\n `packfile_store_read_object_info()`.\n\n - When we've got the empty request.\n\nFix both of these issues so that we can always assume the pack info to\nbe populated when reading object info from a pack.\n\nNote that we don't really care about the second case right now, as the\ncondition will always evaluate to false anyway. This will be fixed in\nthe next commit.\n\nSigned-off-by: Patrick Steinhardt \n---\n packfile.c | 38 ++++++++++++++++++++------------------\n 1 file changed, 20 insertions(+), 18 deletions(-)\n\ndiff --git a/packfile.c b/packfile.c\nindex 9bce52f912..6e66c90c46 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -1657,6 +1657,20 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \t}\n \n \toi->whence = OI_PACKED;\n+\toi->u.packed.offset = obj_offset;\n+\toi->u.packed.pack = p;\n+\n+\tswitch (type) {\n+\tcase OBJ_REF_DELTA:\n+\t\toi->u.packed.type = PACKED_OBJECT_TYPE_REF_DELTA;\n+\t\tbreak;\n+\tcase OBJ_OFS_DELTA:\n+\t\toi->u.packed.type = PACKED_OBJECT_TYPE_OFS_DELTA;\n+\t\tbreak;\n+\tdefault:\n+\t\toi->u.packed.type = PACKED_OBJECT_TYPE_FULL;\n+\t\tbreak;\n+\t}\n \n out:\n \tunuse_pack(&w_curs);\n@@ -2148,8 +2162,13 @@ int packfile_store_read_object_info(struct packfile_store *store,\n \t * We know that the caller doesn't actually need the\n \t * information below, so return early.\n \t */\n-\tif (oi == &blank_oi)\n+\tif (oi == &blank_oi) {\n+\t\toi->whence = OI_PACKED;\n+\t\toi->u.packed.offset = e.offset;\n+\t\toi->u.packed.pack = e.p;\n+\t\toi->u.packed.type = PACKED_OBJECT_TYPE_UNKNOWN;\n \t\treturn 0;\n+\t}\n \n \trtype = packed_object_info(store->odb->repo, e.p, e.offset, oi);\n \tif (rtype < 0) {\n@@ -2157,23 +2176,6 @@ int packfile_store_read_object_info(struct packfile_store *store,\n \t\treturn -1;\n \t}\n \n-\tif (oi->whence == OI_PACKED) {\n-\t\toi->u.packed.offset = e.offset;\n-\t\toi->u.packed.pack = e.p;\n-\n-\t\tswitch (rtype) {\n-\t\tcase OBJ_REF_DELTA:\n-\t\t\toi->u.packed.type = PACKED_OBJECT_TYPE_REF_DELTA;\n-\t\t\tbreak;\n-\t\tcase OBJ_OFS_DELTA:\n-\t\t\toi->u.packed.type = PACKED_OBJECT_TYPE_OFS_DELTA;\n-\t\t\tbreak;\n-\t\tdefault:\n-\t\t\toi->u.packed.type = PACKED_OBJECT_TYPE_FULL;\n-\t\t\tbreak;\n-\t\t}\n-\t}\n-\n \treturn 0;\n }\n \n\n-- \n2.52.0.351.gbe84eed79e.dirty\n\n"},{"id":"532418","messageId":"20251218-b4-pks-odb-read-object-info-improvements-v1-5-81c8368492be@pks.im","threadId":"64646","inReplyTo":"20251218-b4-pks-odb-read-object-info-improvements-v1-0-81c8368492be@pks.im","subject":"[PATCH 5/8] packfile: disentangle return value of `packed_object_info()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-18T06:28:15Z","receivedAt":"2025-12-18T06:28:39Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The `packed_object_info()` function returns the type of the packed\nobject. While we use an `enum object_type` to store the return value,\nthis type is not to be confused with the actual object type. It _may_\ncontain the object type, but it may just as well encode that the given\npacked object is stored as a delta.\n\nWe have removed the only caller that relied on this returned object type\nin the preceding commit, so let's simplify semantics and return either 0\non success or a negative error code otherwise.\n\nThis unblocks a small optimization where we can skip reading the object\ntype altogether.\n\nSigned-off-by: Patrick Steinhardt \n---\n packfile.c | 21 ++++++++++++---------\n packfile.h | 4 ++++\n 2 files changed, 16 insertions(+), 9 deletions(-)\n\ndiff --git a/packfile.c b/packfile.c\nindex 6e66c90c46..c141b8a7b1 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -1587,6 +1587,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \tunsigned long size;\n \toff_t curpos = obj_offset;\n \tenum object_type type;\n+\tint ret;\n \n \t/*\n \t * We always get the representation type, but only convert it to\n@@ -1607,12 +1608,12 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \t\t\toff_t base_offset = get_delta_base(p, &w_curs, &tmp_pos,\n \t\t\t\t\t\t\t type, obj_offset);\n \t\t\tif (!base_offset) {\n-\t\t\t\ttype = OBJ_BAD;\n+\t\t\t\tret = -1;\n \t\t\t\tgoto out;\n \t\t\t}\n \t\t\t*oi->sizep = get_size_from_delta(p, &w_curs, tmp_pos);\n \t\t\tif (*oi->sizep == 0) {\n-\t\t\t\ttype = OBJ_BAD;\n+\t\t\t\tret = -1;\n \t\t\t\tgoto out;\n \t\t\t}\n \t\t} else {\n@@ -1625,7 +1626,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \t\tif (offset_to_pack_pos(p, obj_offset, &pos) < 0) {\n \t\t\terror(\"could not find object at offset %\"PRIuMAX\" \"\n \t\t\t \"in pack %s\", (uintmax_t)obj_offset, p->pack_name);\n-\t\t\ttype = OBJ_BAD;\n+\t\t\tret = -1;\n \t\t\tgoto out;\n \t\t}\n \n@@ -1639,7 +1640,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \t\tif (oi->typep)\n \t\t\t*oi->typep = ptot;\n \t\tif (ptot < 0) {\n-\t\t\ttype = OBJ_BAD;\n+\t\t\tret = -1;\n \t\t\tgoto out;\n \t\t}\n \t}\n@@ -1649,7 +1650,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \t\t\tif (get_delta_base_oid(p, &w_curs, curpos,\n \t\t\t\t\t oi->delta_base_oid,\n \t\t\t\t\t type, obj_offset) < 0) {\n-\t\t\t\ttype = OBJ_BAD;\n+\t\t\t\tret = -1;\n \t\t\t\tgoto out;\n \t\t\t}\n \t\t} else\n@@ -1672,9 +1673,11 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \t\tbreak;\n \t}\n \n+\tret = 0;\n+\n out:\n \tunuse_pack(&w_curs);\n-\treturn type;\n+\treturn ret;\n }\n \n static void *unpack_compressed_entry(struct packed_git *p,\n@@ -2153,7 +2156,7 @@ int packfile_store_read_object_info(struct packfile_store *store,\n {\n \tstatic struct object_info blank_oi = OBJECT_INFO_INIT;\n \tstruct pack_entry e;\n-\tint rtype;\n+\tint ret;\n \n \tif (!find_pack_entry(store->odb->repo, oid, &e))\n \t\treturn 1;\n@@ -2170,8 +2173,8 @@ int packfile_store_read_object_info(struct packfile_store *store,\n \t\treturn 0;\n \t}\n \n-\trtype = packed_object_info(store->odb->repo, e.p, e.offset, oi);\n-\tif (rtype < 0) {\n+\tret = packed_object_info(store->odb->repo, e.p, e.offset, oi);\n+\tif (ret < 0) {\n \t\tmark_bad_packed_object(e.p, oid);\n \t\treturn -1;\n \t}\ndiff --git a/packfile.h b/packfile.h\nindex 59d162a3f4..07f5bfbc4f 100644\n--- a/packfile.h\n+++ b/packfile.h\n@@ -378,6 +378,10 @@ void release_pack_memory(size_t);\n /* global flag to enable extra checks when accessing packed objects */\n extern int do_check_packed_object_crc;\n \n+/*\n+ * Look up the object info for a specific offset in the packfile.\n+ * success, a negative error code otherwise.\n+ */\n int packed_object_info(struct repository *r,\n \t\t struct packed_git *pack,\n \t\t off_t offset, struct object_info *);\n\n-- \n2.52.0.351.gbe84eed79e.dirty\n\n"},{"id":"532419","messageId":"20251218-b4-pks-odb-read-object-info-improvements-v1-6-81c8368492be@pks.im","threadId":"64646","inReplyTo":"20251218-b4-pks-odb-read-object-info-improvements-v1-0-81c8368492be@pks.im","subject":"[PATCH 6/8] packfile: skip unpacking object header for disk size requests","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-18T06:28:16Z","receivedAt":"2025-12-18T06:28:42Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"While most of the object info requests for a packed object require us to\nunpack its headers, reading its disk size doesn't. We still unpack the\nobject header in that case though, which is unnecessary work.\n\nSkip reading the header if only the disk size is requested. This leads\nto a small speedup when reading disk size, only. The following benchmark\nwas done in the Git repository:\n\n Benchmark 1: ./git rev-list --disk-usage HEAD (rev = HEAD~)\n Time (mean ± σ): 105.2 ms ± 0.6 ms [User: 91.4 ms, System: 13.3 ms]\n Range (min … max): 103.7 ms … 106.0 ms 27 runs\n\n Benchmark 2: ./git rev-list --disk-usage HEAD (rev = HEAD)\n Time (mean ± σ): 96.7 ms ± 0.4 ms [User: 86.2 ms, System: 10.0 ms]\n Range (min … max): 96.2 ms … 98.1 ms 30 runs\n\n Summary\n ./git rev-list --disk-usage HEAD (rev = HEAD) ran\n 1.09 ± 0.01 times faster than ./git rev-list --disk-usage HEAD (rev = HEAD~)\n\nSigned-off-by: Patrick Steinhardt \n---\n packfile.c | 7 +++++--\n 1 file changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/packfile.c b/packfile.c\nindex c141b8a7b1..d2ae2432eb 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -1586,7 +1586,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \tstruct pack_window *w_curs = NULL;\n \tunsigned long size;\n \toff_t curpos = obj_offset;\n-\tenum object_type type;\n+\tenum object_type type = OBJ_NONE;\n \tint ret;\n \n \t/*\n@@ -1598,7 +1598,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \t\t\t\t\t\t &type);\n \t\tif (!*oi->contentp)\n \t\t\ttype = OBJ_BAD;\n-\t} else {\n+\t} else if (oi->sizep || oi->typep || oi->delta_base_oid) {\n \t\ttype = unpack_object_header(p, &w_curs, &curpos, &size);\n \t}\n \n@@ -1662,6 +1662,9 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \toi->u.packed.pack = p;\n \n \tswitch (type) {\n+\tcase OBJ_NONE:\n+\t\toi->u.packed.type = PACKED_OBJECT_TYPE_UNKNOWN;\n+\t\tbreak;\n \tcase OBJ_REF_DELTA:\n \t\toi->u.packed.type = PACKED_OBJECT_TYPE_REF_DELTA;\n \t\tbreak;\n\n-- \n2.52.0.351.gbe84eed79e.dirty\n\n"},{"id":"532420","messageId":"20251218-b4-pks-odb-read-object-info-improvements-v1-7-81c8368492be@pks.im","threadId":"64646","inReplyTo":"20251218-b4-pks-odb-read-object-info-improvements-v1-0-81c8368492be@pks.im","subject":"[PATCH 7/8] packfile: fix short-circuiting of empty requests","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-18T06:28:17Z","receivedAt":"2025-12-18T06:28:45Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When reading object information from the packfile store we have logic\nthat tries to bail out early on empty requests. This is supposed to be a\nperformance optimization so that we don't even have to unpack the object\nheader stored in the packfile.\n\nThis optimization doesn't work though: we compare the passed-in object\ninfo pointer with the pointer of an on-stack variable, which of course\ncannot ever become true. This issue was introduced via d9f517d051\n(object-file: split out functions relating to object store subsystem,\n2025-04-15): before this commit, we checked whether the passed-in object\ninfo was a `NULL` pointer, and if so, we set it to point to `blank_oi`\ninstead. The commit then split up these the logic so that we continue to\nset up `blank_oi` in `do_oid_object_info_extended()`, but then do the\ncheck in `packfile_store_read_object_info()`. But even before that\ncommit the logic was only partially working, as it could very well be\nthat callers pass a blank object info themselves.\n\nFix this bug by introducing a new `object_info_is_blank_request()`\nhelper, which simply verifies that none of the contained request\npointers are populated.\n\nReported-by: Aaron Plattner \nHelped-by: Junio C Hamano \nSigned-off-by: Patrick Steinhardt \n---\n odb.h | 10 ++++++++++\n packfile.c | 3 +--\n 2 files changed, 11 insertions(+), 2 deletions(-)\n\ndiff --git a/odb.h b/odb.h\nindex afae5e5c01..b869c054c1 100644\n--- a/odb.h\n+++ b/odb.h\n@@ -353,6 +353,16 @@ struct object_info {\n \t} u;\n };\n \n+/*\n+ * Given an object info structure, figure out whether any of its request\n+ * pointers are populated.\n+ */\n+static inline bool object_info_is_blank_request(struct object_info *oi)\n+{\n+\treturn !oi->typep && !oi->sizep && !oi->disk_sizep &&\n+\t\t!oi->delta_base_oid && !oi->contentp;\n+}\n+\n /*\n * Initializer for a \"struct object_info\" that wants no items. You may\n * also memset() the memory to all-zeroes.\ndiff --git a/packfile.c b/packfile.c\nindex d2ae2432eb..ce83e77899 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -2157,7 +2157,6 @@ int packfile_store_read_object_info(struct packfile_store *store,\n \t\t\t\t struct object_info *oi,\n \t\t\t\t unsigned flags UNUSED)\n {\n-\tstatic struct object_info blank_oi = OBJECT_INFO_INIT;\n \tstruct pack_entry e;\n \tint ret;\n \n@@ -2168,7 +2167,7 @@ int packfile_store_read_object_info(struct packfile_store *store,\n \t * We know that the caller doesn't actually need the\n \t * information below, so return early.\n \t */\n-\tif (oi == &blank_oi) {\n+\tif (object_info_is_blank_request(oi)) {\n \t\toi->whence = OI_PACKED;\n \t\toi->u.packed.offset = e.offset;\n \t\toi->u.packed.pack = e.p;\n\n-- \n2.52.0.351.gbe84eed79e.dirty\n\n"},{"id":"532421","messageId":"20251218-b4-pks-odb-read-object-info-improvements-v1-8-81c8368492be@pks.im","threadId":"64646","inReplyTo":"20251218-b4-pks-odb-read-object-info-improvements-v1-0-81c8368492be@pks.im","subject":"[PATCH 8/8] packfile: drop repository parameter from `packed_object_info()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-18T06:28:18Z","receivedAt":"2025-12-18T06:28:47Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The function `packed_object_info()` takes a packfile and offset and\nreturns the object info for the corresponding object. Despite these two\nparameters though it also takes a repository pointer. This is redundant\ninformation though, as `struct packed_git` already has a repository\npointer that is always populated.\n\nDrop the redundant parameter.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/cat-file.c | 3 +--\n builtin/pack-objects.c | 4 ++--\n commit-graph.c | 2 +-\n pack-bitmap.c | 3 +--\n packfile.c | 8 ++++----\n packfile.h | 3 +--\n 6 files changed, 10 insertions(+), 13 deletions(-)\n\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex 505ddaa12f..2ad712e9f8 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -487,8 +487,7 @@ static void batch_object_write(const char *obj_name,\n \t\t\tdata->info.sizep = &data->size;\n \n \t\tif (pack)\n-\t\t\tret = packed_object_info(the_repository, pack,\n-\t\t\t\t\t\t offset, &data->info);\n+\t\t\tret = packed_object_info(pack, offset, &data->info);\n \t\telse\n \t\t\tret = odb_read_object_info_extended(the_repository->objects,\n \t\t\t\t\t\t\t &data->oid, &data->info,\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 1ce8d6ee21..85762f8c4f 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -2411,7 +2411,7 @@ static void drop_reused_delta(struct object_entry *entry)\n \n \toi.sizep = &size;\n \toi.typep = &type;\n-\tif (packed_object_info(the_repository, IN_PACK(entry), entry->in_pack_offset, &oi) < 0) {\n+\tif (packed_object_info(IN_PACK(entry), entry->in_pack_offset, &oi) < 0) {\n \t\t/*\n \t\t * We failed to get the info from this pack for some reason;\n \t\t * fall back to odb_read_object_info, which may find another copy.\n@@ -3748,7 +3748,7 @@ static int add_object_entry_from_pack(const struct object_id *oid,\n \t\tstruct object_info oi = OBJECT_INFO_INIT;\n \n \t\toi.typep = &type;\n-\t\tif (packed_object_info(the_repository, p, ofs, &oi) < 0) {\n+\t\tif (packed_object_info(p, ofs, &oi) < 0) {\n \t\t\tdie(_(\"could not get type of object %s in pack %s\"),\n \t\t\t oid_to_hex(oid), p->pack_name);\n \t\t} else if (type == OBJ_COMMIT) {\ndiff --git a/commit-graph.c b/commit-graph.c\nindex 80be2ff2c3..f572670bd0 100644\n--- a/commit-graph.c\n+++ b/commit-graph.c\n@@ -1499,7 +1499,7 @@ static int add_packed_commits(const struct object_id *oid,\n \t\tdisplay_progress(ctx->progress, ++ctx->progress_done);\n \n \toi.typep = &type;\n-\tif (packed_object_info(ctx->r, pack, offset, &oi) < 0)\n+\tif (packed_object_info(pack, offset, &oi) < 0)\n \t\tdie(_(\"unable to get type of object %s\"), oid_to_hex(oid));\n \n \tif (type != OBJ_COMMIT)\ndiff --git a/pack-bitmap.c b/pack-bitmap.c\nindex 8ca79725b1..972203f12b 100644\n--- a/pack-bitmap.c\n+++ b/pack-bitmap.c\n@@ -1876,8 +1876,7 @@ static unsigned long get_size_by_pos(struct bitmap_index *bitmap_git,\n \t\t\tofs = pack_pos_to_offset(pack, pos);\n \t\t}\n \n-\t\tif (packed_object_info(bitmap_repo(bitmap_git), pack, ofs,\n-\t\t\t\t &oi) < 0) {\n+\t\tif (packed_object_info(pack, ofs, &oi) < 0) {\n \t\t\tstruct object_id oid;\n \t\t\tnth_bitmap_object_oid(bitmap_git, &oid,\n \t\t\t\t\t pack_pos_to_index(pack, pos));\ndiff --git a/packfile.c b/packfile.c\nindex ce83e77899..8daa5a5ee7 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -1580,7 +1580,7 @@ static void add_delta_base_cache(struct packed_git *p, off_t base_offset,\n \thashmap_add(&delta_base_cache, &ent->ent);\n }\n \n-int packed_object_info(struct repository *r, struct packed_git *p,\n+int packed_object_info(struct packed_git *p,\n \t\t off_t obj_offset, struct object_info *oi)\n {\n \tstruct pack_window *w_curs = NULL;\n@@ -1594,7 +1594,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \t * a \"real\" type later if the caller is interested.\n \t */\n \tif (oi->contentp) {\n-\t\t*oi->contentp = cache_or_unpack_entry(r, p, obj_offset, oi->sizep,\n+\t\t*oi->contentp = cache_or_unpack_entry(p->repo, p, obj_offset, oi->sizep,\n \t\t\t\t\t\t &type);\n \t\tif (!*oi->contentp)\n \t\t\ttype = OBJ_BAD;\n@@ -1635,7 +1635,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \n \tif (oi->typep) {\n \t\tenum object_type ptot;\n-\t\tptot = packed_to_object_type(r, p, obj_offset,\n+\t\tptot = packed_to_object_type(p->repo, p, obj_offset,\n \t\t\t\t\t type, &w_curs, curpos);\n \t\tif (oi->typep)\n \t\t\t*oi->typep = ptot;\n@@ -2175,7 +2175,7 @@ int packfile_store_read_object_info(struct packfile_store *store,\n \t\treturn 0;\n \t}\n \n-\tret = packed_object_info(store->odb->repo, e.p, e.offset, oi);\n+\tret = packed_object_info(e.p, e.offset, oi);\n \tif (ret < 0) {\n \t\tmark_bad_packed_object(e.p, oid);\n \t\treturn -1;\ndiff --git a/packfile.h b/packfile.h\nindex 07f5bfbc4f..573d06f6ba 100644\n--- a/packfile.h\n+++ b/packfile.h\n@@ -382,8 +382,7 @@ extern int do_check_packed_object_crc;\n * Look up the object info for a specific offset in the packfile.\n * success, a negative error code otherwise.\n */\n-int packed_object_info(struct repository *r,\n-\t\t struct packed_git *pack,\n+int packed_object_info(struct packed_git *pack,\n \t\t off_t offset, struct object_info *);\n \n void mark_bad_packed_object(struct packed_git *, const struct object_id *);\n\n-- \n2.52.0.351.gbe84eed79e.dirty\n\n"},{"id":"532438","messageId":"xmqqh5toxnhk.fsf@gitster.g","threadId":"64646","inReplyTo":"20251218-b4-pks-odb-read-object-info-improvements-v1-2-81c8368492be@pks.im","subject":"Re: [PATCH 2/8] packfile: always declare object info to be OI_PACKED","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-12-18T07:23:03Z","receivedAt":"2025-12-18T07:23:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> When reading object info via a packfile we yield one of two types:\n>\n> - The object can either be OI_PACKED, which is what a caller would\n> typically expect.\n>\n> - Or it can be OI_DBCACHED if it is stored in the delta base cache.\n>\n> The latter really is an implementation detail though, and callers\n> typically don't care at all about the difference. Furthermore, the\n> information whether or not it is part of the delta base cache can\n> already be derived via the `is_delta` field, so the fact that we discern\n> between OI_PACKED and OI_DBCACHED only further complicates the\n> interface.\n\nIf this were \"and no existing callers check at all\", it would be\ntrivial to decide for this change. In fact you do say that but in a\nweaker form just below.\n\n> Drop the OI_DBCACHED enum completely. There don't seem to be any callers\n> that care about the distinction.\n\n\"git grep OI_DBCACHED\" shows only a single hit, which is what you\nare getting rid of in this patch, but I cannot claim that we did a\nsufficient audit, as this change will break code paths that check if\nthey got OI_PACKED and do something differently (or if what they got\nis different from OI_PACKED, for that matter).\n\n>\n> Signed-off-by: Patrick Steinhardt \n> ---\n> odb.h | 1 -\n> packfile.c | 3 +--\n> 2 files changed, 1 insertion(+), 3 deletions(-)\n>\n> diff --git a/odb.h b/odb.h\n> index 014cd9585a..73b0b87ad5 100644\n> --- a/odb.h\n> +++ b/odb.h\n> @@ -330,7 +330,6 @@ struct object_info {\n> \t\tOI_CACHED,\n> \t\tOI_LOOSE,\n> \t\tOI_PACKED,\n> -\t\tOI_DBCACHED\n> \t} whence;\n> \tunion {\n> \t\t/*\n> diff --git a/packfile.c b/packfile.c\n> index c88bd92619..79ad9d7179 100644\n> --- a/packfile.c\n> +++ b/packfile.c\n> @@ -1656,8 +1656,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n> \t\t\toidclr(oi->delta_base_oid, p->repo->hash_algo);\n> \t}\n> \n> -\toi->whence = in_delta_base_cache(p, obj_offset) ? OI_DBCACHED :\n> -\t\t\t\t\t\t\t OI_PACKED;\n> +\toi->whence = OI_PACKED;\n> \n> out:\n> \tunuse_pack(&w_curs);\n"},{"id":"532440","messageId":"xmqqcy4cxn2l.fsf@gitster.g","threadId":"64646","inReplyTo":"20251218-b4-pks-odb-read-object-info-improvements-v1-4-81c8368492be@pks.im","subject":"Re: [PATCH 4/8] packfile: always populate pack-specific info when reading object info","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-12-18T07:32:02Z","receivedAt":"2025-12-18T07:32:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> @@ -2148,8 +2162,13 @@ int packfile_store_read_object_info(struct packfile_store *store,\n> \t * We know that the caller doesn't actually need the\n> \t * information below, so return early.\n> \t */\n> -\tif (oi == &blank_oi)\n> +\tif (oi == &blank_oi) {\n> +\t\toi->whence = OI_PACKED;\n> +\t\toi->u.packed.offset = e.offset;\n> +\t\toi->u.packed.pack = e.p;\n> +\t\toi->u.packed.type = PACKED_OBJECT_TYPE_UNKNOWN;\n\nIt cannot be seen as it is before the precontext, but if blank_oi is\nstill a function scope static that is initialized only once by\nassigning OBJECT_INFO_INIT, this will leave a timg bomb waiting to\ngo off, as it violates the \"blank\"-ness promise for the next caller\nof this function who calls NULL in oi.\n\nI'd prefer we fix this nonsense \"we only declared a function scope\nstatic, but without actually using it for anything, other than to\ncompare its address with the caller supplied parameter\" well before\nthis step.\n"},{"id":"532445","messageId":"xmqq8qf0xlce.fsf@gitster.g","threadId":"64646","inReplyTo":"20251218-b4-pks-odb-read-object-info-improvements-v1-0-81c8368492be@pks.im","subject":"Re: [PATCH 0/8] Improvements for reading object info","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-12-18T08:09:21Z","receivedAt":"2025-12-18T08:09:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> This series has a conflict with ps/packfile-store-in-odb-source. I\n> decided to not make this a dependency though because those two topics\n> are independent from one another, and I expect that this series here\n> will be merged down faster than the conflicting one. Furthermore, the\n> conflict itself is quite minor:\n>\n> diff --cc packfile.c\n> index 8daa5a5ee7,ce6716fbea..0000000000\n> --- a/packfile.c\n> +++ b/packfile.c\n> @@@ -2157,10 -2132,11 +2151,10 @@@ int packfile_store_read_object_info(str\n> \t\t\t\t struct object_info *oi,\n> \t\t\t\t unsigned flags UNUSED)\n> {\n> -\tstatic struct object_info blank_oi = OBJECT_INFO_INIT;\n> \tstruct pack_entry e;\n> -\tint rtype;\n> +\tint ret;\n> \n> - \tif (!find_pack_entry(store->odb->repo, oid, &e))\n> + \tif (!find_pack_entry(store, oid, &e))\n> \t\treturn 1;\n> \n> \t/*\n> @@@ -2549,9 -2555,8 +2571,9 @@@ int packfile_store_read_object_stream(s\n> \toi.sizep = &size;\n> \n> \tif (packfile_store_read_object_info(store, oid, &oi, 0) ||\n> -\t oi.u.packed.is_delta ||\n> +\t oi.u.packed.type == PACKED_OBJECT_TYPE_REF_DELTA ||\n> +\t oi.u.packed.type == PACKED_OBJECT_TYPE_OFS_DELTA ||\n> - \t repo_settings_get_big_file_threshold(store->odb->repo) >= size)\n> + \t repo_settings_get_big_file_threshold(store->source->odb->repo) >= size)\n> \t\treturn -1;\n> \n> \tin_pack_type = unpack_object_header(oi.u.packed.pack,\n>\n> I'd thus propose to merge this series via an evil merge, but if this\n> proves to be burdensome I'm happy to defer it to a later point. Just let\n> me know and I'll adapt accordingly, thanks!\n\nIndeed the conflicts above are miniscule that it does not even need\nany evil merge. The surviving lines are all from either ours or\ntheirs, that changes are close enough to be shown in --cc.\n\nBut let me first concentrate more on fixing performance regression\nthat already made down to 'master'. It is a shame that nobody\ncaught it while it was cooking in 'next'.\n\nThanks.\n"},{"id":"532447","messageId":"aUO7kHwgSkV5uQdX@pks.im","threadId":"64646","inReplyTo":"xmqq8qf0xlce.fsf@gitster.g","subject":"Re: [PATCH 0/8] Improvements for reading object info","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-18T08:30:08Z","receivedAt":"2025-12-18T08:30:16Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Dec 18, 2025 at 05:09:21PM +0900, Junio C Hamano wrote:\n> Patrick Steinhardt writes:\n> > diff --cc packfile.c\n> > index 8daa5a5ee7,ce6716fbea..0000000000\n> > --- a/packfile.c\n> > +++ b/packfile.c\n> > @@@ -2549,9 -2555,8 +2571,9 @@@ int packfile_store_read_object_stream(s\n> > \toi.sizep = &size;\n> > \n> > \tif (packfile_store_read_object_info(store, oid, &oi, 0) ||\n> > -\t oi.u.packed.is_delta ||\n> > +\t oi.u.packed.type == PACKED_OBJECT_TYPE_REF_DELTA ||\n> > +\t oi.u.packed.type == PACKED_OBJECT_TYPE_OFS_DELTA ||\n> > - \t repo_settings_get_big_file_threshold(store->odb->repo) >= size)\n> > + \t repo_settings_get_big_file_threshold(store->source->odb->repo) >= size)\n> > \t\treturn -1;\n> > \n> > \tin_pack_type = unpack_object_header(oi.u.packed.pack,\n> >\n> > I'd thus propose to merge this series via an evil merge, but if this\n> > proves to be burdensome I'm happy to defer it to a later point. Just let\n> > me know and I'll adapt accordingly, thanks!\n> \n> Indeed the conflicts above are miniscule that it does not even need\n> any evil merge. The surviving lines are all from either ours or\n> theirs, that changes are close enough to be shown in --cc.\n> \n> But let me first concentrate more on fixing performance regression\n> that already made down to 'master'. It is a shame that nobody\n> caught it while it was cooking in 'next'.\n\nFair enough, so that means that you'd want to merge your patch down\nfirst, right? If so I'll rebase my series on top of your patch and then\nresend it soonish.\n\nIn any case, I noticed a slight regression in one of the benchmarks that\nprints all objects, but I attributed it to CI flakiness [1]. The uptick\ndidn't seem strong enough to really be a regression, and I'm still not\nsure whether it's related to this patch series or not. Chances are it\nis. I'll investigate and make sure to extend the benchmarking suite\naccordingly so that we have a clearer signal there.\n\nThanks!\n\nPatrick\n\n[1]: https://bencher.dev/perf/git?branches=595859eb-071c-48e9-97cf-195e0a3d6ed1&testbeds=02dcb8ad-6873-494c-aabc-9a6237601308&benchmarks=0da3d87a-ce30-4125-86e9-12d84ec4bc49&measures=63dafffb-98c4-4c27-ba43-7112cae627fc\n"},{"id":"532449","messageId":"aUPE-H6mQQwlOQ1Z@pks.im","threadId":"64646","inReplyTo":"xmqqh5toxnhk.fsf@gitster.g","subject":"Re: [PATCH 2/8] packfile: always declare object info to be OI_PACKED","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-18T09:10:16Z","receivedAt":"2025-12-18T09:10:23Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Dec 18, 2025 at 04:23:03PM +0900, Junio C Hamano wrote:\n> Patrick Steinhardt writes:\n> \n> > When reading object info via a packfile we yield one of two types:\n> >\n> > - The object can either be OI_PACKED, which is what a caller would\n> > typically expect.\n> >\n> > - Or it can be OI_DBCACHED if it is stored in the delta base cache.\n> >\n> > The latter really is an implementation detail though, and callers\n> > typically don't care at all about the difference. Furthermore, the\n> > information whether or not it is part of the delta base cache can\n> > already be derived via the `is_delta` field, so the fact that we discern\n> > between OI_PACKED and OI_DBCACHED only further complicates the\n> > interface.\n> \n> If this were \"and no existing callers check at all\", it would be\n> trivial to decide for this change. In fact you do say that but in a\n> weaker form just below.\n> \n> > Drop the OI_DBCACHED enum completely. There don't seem to be any callers\n> > that care about the distinction.\n> \n> \"git grep OI_DBCACHED\" shows only a single hit, which is what you\n> are getting rid of in this patch, but I cannot claim that we did a\n> sufficient audit, as this change will break code paths that check if\n> they got OI_PACKED and do something differently (or if what they got\n> is different from OI_PACKED, for that matter).\n\nThat's a fair complaint. I'll adapt the commit message to include the\ninvestigation.\n\nThanks!\n\nPatrick\n"},{"id":"532453","messageId":"20251218-b4-pks-odb-read-object-info-improvements-v2-0-62e3e49072bc@pks.im","threadId":"64646","inReplyTo":"20251218-b4-pks-odb-read-object-info-improvements-v1-0-81c8368492be@pks.im","subject":"[PATCH v2 0/7] Improvements for reading object info","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-18T10:54:12Z","receivedAt":"2025-12-18T10:54:24Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nthis patch series contains various small improvements for reading object\ninfo for either loose or packed objects. These improvements were split\nout of a larger patch series where I'm about to introduce a new generic\n`odb_for_each_object()` function.\n\nChanges in v2:\n - Rebase the series on top of master with jc/object-read-stream-fix\n merged into it. I've also evicted the patch that fixes the same\n underlying issue.\n - Improve the commit message that drops OI_DBCACHED to explain why\n this is a safe refactoring.\n - Link to v1: https://lore.kernel.org/r/20251218-b4-pks-odb-read-object-info-improvements-v1-0-81c8368492be@pks.im\n\nThanks!\n\nPatrick\n\n---\nPatrick Steinhardt (7):\n object-file: always set OI_LOOSE when reading object info\n packfile: always declare object info to be OI_PACKED\n packfile: extend `is_delta` field to allow for \"unknown\" state\n packfile: always populate pack-specific info when reading object info\n packfile: disentangle return value of `packed_object_info()`\n packfile: skip unpacking object header for disk size requests\n packfile: drop repository parameter from `packed_object_info()`\n\n builtin/cat-file.c | 3 +--\n builtin/pack-objects.c | 4 ++--\n commit-graph.c | 2 +-\n object-file.c | 19 ++++++++++++----\n odb.h | 8 +++++--\n pack-bitmap.c | 3 +--\n packfile.c | 61 ++++++++++++++++++++++++++++++--------------------\n packfile.h | 7 ++++--\n 8 files changed, 68 insertions(+), 39 deletions(-)\n\nRange-diff versus v1:\n\n1: 0c1a4a4745 < -: ---------- object-file: always set OI_LOOSE when reading object info\n-: ---------- > 1: 2287c0cbd9 object-file: always set OI_LOOSE when reading object info\n2: 98962428cf ! 2: a1cd99af9c packfile: always declare object info to be OI_PACKED\n @@ Commit message\n between OI_PACKED and OI_DBCACHED only further complicates the\n interface.\n \n - Drop the OI_DBCACHED enum completely. There don't seem to be any callers\n - that care about the distinction.\n + There aren't all that many callers that care about the `whence` field in\n + the first place. In fact, there's only three:\n +\n + - `packfile_store_read_object_info()` checks for `whence == OI_PACKED`\n + and then populates the packfile information of the object info\n + structure. We now start to do this also for deltified objects, which\n + gives its callers strictly more information.\n +\n + - `repack_local_links()` wants to determine whether the object is part\n + of a promisor pack and checks for `whence == OI_PACKED`. If so, it\n + verifies that the packfile is a promisor pack. It's arguably wrong\n + to declare that an object is not part of a promisor pack only\n + because it is stored in the delta base cache.\n +\n + - `is_not_in_promisor_pack_obj()` does the same, but checks that a\n + specific object is _not_ part of a promisor pack. The same reasoning\n + as above applies.\n +\n + Drop the OI_DBCACHED enum completely. None of the callers seem to care\n + about the distinction.\n \n Signed-off-by: Patrick Steinhardt \n \n3: 0a5b806934 = 3: 7a043c09ee packfile: extend `is_delta` field to allow for \"unknown\" state\n4: 6a05c85683 ! 4: 448511cb19 packfile: always populate pack-specific info when reading object info\n @@ Metadata\n ## Commit message ##\n packfile: always populate pack-specific info when reading object info\n \n - When reading object information from a packfile we are not always\n - populating the pack-specific information. This happens in two cases:\n + When reading object information via `packed_object_info()` we may not\n + populate the object info's packfile-specific fields. This leads to\n + inconsistent object info depending on whether the info was populated via\n + `packfile_store_read_object_info()` or `packed_object_info()`.\n \n - - When calling `packed_object_info()` directly instead of\n - `packfile_store_read_object_info()`.\n -\n - - When we've got the empty request.\n -\n - Fix both of these issues so that we can always assume the pack info to\n - be populated when reading object info from a pack.\n -\n - Note that we don't really care about the second case right now, as the\n - condition will always evaluate to false anyway. This will be fixed in\n - the next commit.\n + Fix this inconsistecny so that we can always assume the pack info to be\n + populated when reading object info from a pack.\n \n Signed-off-by: Patrick Steinhardt \n \n @@ packfile.c: int packed_object_info(struct repository *r, struct packed_git *p,\n \n out:\n \tunuse_pack(&w_curs);\n -@@ packfile.c: int packfile_store_read_object_info(struct packfile_store *store,\n - \t * We know that the caller doesn't actually need the\n - \t * information below, so return early.\n - \t */\n --\tif (oi == &blank_oi)\n -+\tif (oi == &blank_oi) {\n -+\t\toi->whence = OI_PACKED;\n -+\t\toi->u.packed.offset = e.offset;\n -+\t\toi->u.packed.pack = e.p;\n -+\t\toi->u.packed.type = PACKED_OBJECT_TYPE_UNKNOWN;\n - \t\treturn 0;\n -+\t}\n - \n - \trtype = packed_object_info(store->odb->repo, e.p, e.offset, oi);\n - \tif (rtype < 0) {\n @@ packfile.c: int packfile_store_read_object_info(struct packfile_store *store,\n \t\treturn -1;\n \t}\n5: b09f37400c ! 5: e1ee6c7841 packfile: disentangle return value of `packed_object_info()`\n @@ packfile.c: int packed_object_info(struct repository *r, struct packed_git *p,\n \n static void *unpack_compressed_entry(struct packed_git *p,\n @@ packfile.c: int packfile_store_read_object_info(struct packfile_store *store,\n + \t\t\t\t unsigned flags UNUSED)\n {\n - \tstatic struct object_info blank_oi = OBJECT_INFO_INIT;\n \tstruct pack_entry e;\n -\tint rtype;\n +\tint ret;\n @@ packfile.c: int packfile_store_read_object_info(struct packfile_store *store,\n \tif (!find_pack_entry(store->odb->repo, oid, &e))\n \t\treturn 1;\n @@ packfile.c: int packfile_store_read_object_info(struct packfile_store *store,\n + \tif (!oi)\n \t\treturn 0;\n - \t}\n \n -\trtype = packed_object_info(store->odb->repo, e.p, e.offset, oi);\n -\tif (rtype < 0) {\n6: 253c0d47ab = 6: 77589e84b5 packfile: skip unpacking object header for disk size requests\n7: 4dac51d4be < -: ---------- packfile: fix short-circuiting of empty requests\n8: 2cf441de0d ! 7: 08f4b865e5 packfile: drop repository parameter from `packed_object_info()`\n @@ packfile.c: int packed_object_info(struct repository *r, struct packed_git *p,\n \t\tif (oi->typep)\n \t\t\t*oi->typep = ptot;\n @@ packfile.c: int packfile_store_read_object_info(struct packfile_store *store,\n + \tif (!oi)\n \t\treturn 0;\n - \t}\n \n -\tret = packed_object_info(store->odb->repo, e.p, e.offset, oi);\n +\tret = packed_object_info(e.p, e.offset, oi);\n\n---\nbase-commit: 7df68b50e49b6a1b576abb19b2e5d457749bc28b\nchange-id: 20251215-b4-pks-odb-read-object-info-improvements-0e031ef827d2\n\n"},{"id":"532454","messageId":"20251218-b4-pks-odb-read-object-info-improvements-v2-1-62e3e49072bc@pks.im","threadId":"64646","inReplyTo":"20251218-b4-pks-odb-read-object-info-improvements-v2-0-62e3e49072bc@pks.im","subject":"[PATCH v2 1/7] object-file: always set OI_LOOSE when reading object info","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-18T10:54:13Z","receivedAt":"2025-12-18T10:54:25Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"There are some early returns in ``odb_source_loose_read_object_info()`\nin cases where we don't have to open the loose object. These return\npaths do not set `struct object_info::whence` to `OI_LOOSE` though, so\nit becomes impossible for the caller to tell the format of such an\nobject.\n\nNobody seems to care about this right now, but it's a bug waiting to\nhappen. Fix this by always setting `whence` on success.\n\nSigned-off-by: Patrick Steinhardt \n---\n object-file.c | 19 +++++++++++++++----\n 1 file changed, 15 insertions(+), 4 deletions(-)\n\ndiff --git a/object-file.c b/object-file.c\nindex 6280e42f34..d566df427a 100644\n--- a/object-file.c\n+++ b/object-file.c\n@@ -439,12 +439,23 @@ int odb_source_loose_read_object_info(struct odb_source *source,\n \t */\n \tif (!oi || (!oi->typep && !oi->sizep && !oi->contentp)) {\n \t\tstruct stat st;\n-\t\tif ((!oi || !oi->disk_sizep) && (flags & OBJECT_INFO_QUICK))\n-\t\t\treturn quick_has_loose(source->loose, oid) ? 0 : -1;\n+\n+\t\tif ((!oi || !oi->disk_sizep) && (flags & OBJECT_INFO_QUICK)) {\n+\t\t\tstatus = quick_has_loose(source->loose, oid) ? 0 : -1;\n+\t\t\tif (!status && oi)\n+\t\t\t\toi->whence = OI_LOOSE;\n+\t\t\treturn status;\n+\t\t}\n+\n \t\tif (stat_loose_object(source->loose, oid, &st, &path) < 0)\n \t\t\treturn -1;\n-\t\tif (oi && oi->disk_sizep)\n-\t\t\t*oi->disk_sizep = st.st_size;\n+\n+\t\tif (oi) {\n+\t\t\tif (oi->disk_sizep)\n+\t\t\t\t*oi->disk_sizep = st.st_size;\n+\t\t\toi->whence = OI_LOOSE;\n+\t\t}\n+\n \t\treturn 0;\n \t}\n \n\n-- \n2.52.0.351.gbe84eed79e.dirty\n\n"},{"id":"532455","messageId":"20251218-b4-pks-odb-read-object-info-improvements-v2-2-62e3e49072bc@pks.im","threadId":"64646","inReplyTo":"20251218-b4-pks-odb-read-object-info-improvements-v2-0-62e3e49072bc@pks.im","subject":"[PATCH v2 2/7] packfile: always declare object info to be OI_PACKED","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-18T10:54:14Z","receivedAt":"2025-12-18T10:54:28Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When reading object info via a packfile we yield one of two types:\n\n - The object can either be OI_PACKED, which is what a caller would\n typically expect.\n\n - Or it can be OI_DBCACHED if it is stored in the delta base cache.\n\nThe latter really is an implementation detail though, and callers\ntypically don't care at all about the difference. Furthermore, the\ninformation whether or not it is part of the delta base cache can\nalready be derived via the `is_delta` field, so the fact that we discern\nbetween OI_PACKED and OI_DBCACHED only further complicates the\ninterface.\n\nThere aren't all that many callers that care about the `whence` field in\nthe first place. In fact, there's only three:\n\n - `packfile_store_read_object_info()` checks for `whence == OI_PACKED`\n and then populates the packfile information of the object info\n structure. We now start to do this also for deltified objects, which\n gives its callers strictly more information.\n\n - `repack_local_links()` wants to determine whether the object is part\n of a promisor pack and checks for `whence == OI_PACKED`. If so, it\n verifies that the packfile is a promisor pack. It's arguably wrong\n to declare that an object is not part of a promisor pack only\n because it is stored in the delta base cache.\n\n - `is_not_in_promisor_pack_obj()` does the same, but checks that a\n specific object is _not_ part of a promisor pack. The same reasoning\n as above applies.\n\nDrop the OI_DBCACHED enum completely. None of the callers seem to care\nabout the distinction.\n\nSigned-off-by: Patrick Steinhardt \n---\n odb.h | 1 -\n packfile.c | 3 +--\n 2 files changed, 1 insertion(+), 3 deletions(-)\n\ndiff --git a/odb.h b/odb.h\nindex 014cd9585a..73b0b87ad5 100644\n--- a/odb.h\n+++ b/odb.h\n@@ -330,7 +330,6 @@ struct object_info {\n \t\tOI_CACHED,\n \t\tOI_LOOSE,\n \t\tOI_PACKED,\n-\t\tOI_DBCACHED\n \t} whence;\n \tunion {\n \t\t/*\ndiff --git a/packfile.c b/packfile.c\nindex 08a0863fc3..b0c6665c87 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -1656,8 +1656,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \t\t\toidclr(oi->delta_base_oid, p->repo->hash_algo);\n \t}\n \n-\toi->whence = in_delta_base_cache(p, obj_offset) ? OI_DBCACHED :\n-\t\t\t\t\t\t\t OI_PACKED;\n+\toi->whence = OI_PACKED;\n \n out:\n \tunuse_pack(&w_curs);\n\n-- \n2.52.0.351.gbe84eed79e.dirty\n\n"},{"id":"532456","messageId":"20251218-b4-pks-odb-read-object-info-improvements-v2-3-62e3e49072bc@pks.im","threadId":"64646","inReplyTo":"20251218-b4-pks-odb-read-object-info-improvements-v2-0-62e3e49072bc@pks.im","subject":"[PATCH v2 3/7] packfile: extend `is_delta` field to allow for \"unknown\" state","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-18T10:54:15Z","receivedAt":"2025-12-18T10:54:36Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The `struct object_info::u::packed::is_delta` field determines whether\nor not a specific object is stored as a delta. It only stores whether or\nnot the object is stored as delta, so it is treated as a boolean value.\n\nThis boolean is insufficient though: when reading a packed object via\n`packfile_store_read_object_info()` we know to skip parsing the actual\nobject when the user didn't request any object-specific data. In that\ncase we won't read the object itself, but will only look up its position\nin the packfile. Consequently, we do not know whether it is a delta or\nnot.\n\nThis isn't really an issue right now, as the check for an empty request\nis broken. But a subsequent commit will fix it, and once we do we will\nhave the need to also represent an \"unknown\" delta state.\n\nPrepare for this change by introducing a new enum that encodes the\nobject type. We don't use the \"unknown\" state just yet, but will start\nto do so in the next commit.\n\nSigned-off-by: Patrick Steinhardt \n---\n odb.h | 7 ++++++-\n packfile.c | 17 ++++++++++++++---\n 2 files changed, 20 insertions(+), 4 deletions(-)\n\ndiff --git a/odb.h b/odb.h\nindex 73b0b87ad5..afae5e5c01 100644\n--- a/odb.h\n+++ b/odb.h\n@@ -343,7 +343,12 @@ struct object_info {\n \t\tstruct {\n \t\t\tstruct packed_git *pack;\n \t\t\toff_t offset;\n-\t\t\tunsigned int is_delta;\n+\t\t\tenum packed_object_type {\n+\t\t\t\tPACKED_OBJECT_TYPE_UNKNOWN,\n+\t\t\t\tPACKED_OBJECT_TYPE_FULL,\n+\t\t\t\tPACKED_OBJECT_TYPE_OFS_DELTA,\n+\t\t\t\tPACKED_OBJECT_TYPE_REF_DELTA,\n+\t\t\t} type;\n \t\t} packed;\n \t} u;\n };\ndiff --git a/packfile.c b/packfile.c\nindex b0c6665c87..cc797b2b6a 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -2159,8 +2159,18 @@ int packfile_store_read_object_info(struct packfile_store *store,\n \tif (oi->whence == OI_PACKED) {\n \t\toi->u.packed.offset = e.offset;\n \t\toi->u.packed.pack = e.p;\n-\t\toi->u.packed.is_delta = (rtype == OBJ_REF_DELTA ||\n-\t\t\t\t\t rtype == OBJ_OFS_DELTA);\n+\n+\t\tswitch (rtype) {\n+\t\tcase OBJ_REF_DELTA:\n+\t\t\toi->u.packed.type = PACKED_OBJECT_TYPE_REF_DELTA;\n+\t\t\tbreak;\n+\t\tcase OBJ_OFS_DELTA:\n+\t\t\toi->u.packed.type = PACKED_OBJECT_TYPE_OFS_DELTA;\n+\t\t\tbreak;\n+\t\tdefault:\n+\t\t\toi->u.packed.type = PACKED_OBJECT_TYPE_FULL;\n+\t\t\tbreak;\n+\t\t}\n \t}\n \n \treturn 0;\n@@ -2531,7 +2541,8 @@ int packfile_store_read_object_stream(struct odb_read_stream **out,\n \toi.sizep = &size;\n \n \tif (packfile_store_read_object_info(store, oid, &oi, 0) ||\n-\t oi.u.packed.is_delta ||\n+\t oi.u.packed.type == PACKED_OBJECT_TYPE_REF_DELTA ||\n+\t oi.u.packed.type == PACKED_OBJECT_TYPE_OFS_DELTA ||\n \t repo_settings_get_big_file_threshold(store->odb->repo) >= size)\n \t\treturn -1;\n \n\n-- \n2.52.0.351.gbe84eed79e.dirty\n\n"},{"id":"532457","messageId":"20251218-b4-pks-odb-read-object-info-improvements-v2-4-62e3e49072bc@pks.im","threadId":"64646","inReplyTo":"20251218-b4-pks-odb-read-object-info-improvements-v2-0-62e3e49072bc@pks.im","subject":"[PATCH v2 4/7] packfile: always populate pack-specific info when reading object info","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-18T10:54:16Z","receivedAt":"2025-12-18T10:54:38Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When reading object information via `packed_object_info()` we may not\npopulate the object info's packfile-specific fields. This leads to\ninconsistent object info depending on whether the info was populated via\n`packfile_store_read_object_info()` or `packed_object_info()`.\n\nFix this inconsistecny so that we can always assume the pack info to be\npopulated when reading object info from a pack.\n\nSigned-off-by: Patrick Steinhardt \n---\n packfile.c | 31 ++++++++++++++-----------------\n 1 file changed, 14 insertions(+), 17 deletions(-)\n\ndiff --git a/packfile.c b/packfile.c\nindex cc797b2b6a..f7c33a2f77 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -1657,6 +1657,20 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \t}\n \n \toi->whence = OI_PACKED;\n+\toi->u.packed.offset = obj_offset;\n+\toi->u.packed.pack = p;\n+\n+\tswitch (type) {\n+\tcase OBJ_REF_DELTA:\n+\t\toi->u.packed.type = PACKED_OBJECT_TYPE_REF_DELTA;\n+\t\tbreak;\n+\tcase OBJ_OFS_DELTA:\n+\t\toi->u.packed.type = PACKED_OBJECT_TYPE_OFS_DELTA;\n+\t\tbreak;\n+\tdefault:\n+\t\toi->u.packed.type = PACKED_OBJECT_TYPE_FULL;\n+\t\tbreak;\n+\t}\n \n out:\n \tunuse_pack(&w_curs);\n@@ -2156,23 +2170,6 @@ int packfile_store_read_object_info(struct packfile_store *store,\n \t\treturn -1;\n \t}\n \n-\tif (oi->whence == OI_PACKED) {\n-\t\toi->u.packed.offset = e.offset;\n-\t\toi->u.packed.pack = e.p;\n-\n-\t\tswitch (rtype) {\n-\t\tcase OBJ_REF_DELTA:\n-\t\t\toi->u.packed.type = PACKED_OBJECT_TYPE_REF_DELTA;\n-\t\t\tbreak;\n-\t\tcase OBJ_OFS_DELTA:\n-\t\t\toi->u.packed.type = PACKED_OBJECT_TYPE_OFS_DELTA;\n-\t\t\tbreak;\n-\t\tdefault:\n-\t\t\toi->u.packed.type = PACKED_OBJECT_TYPE_FULL;\n-\t\t\tbreak;\n-\t\t}\n-\t}\n-\n \treturn 0;\n }\n \n\n-- \n2.52.0.351.gbe84eed79e.dirty\n\n"},{"id":"532458","messageId":"20251218-b4-pks-odb-read-object-info-improvements-v2-5-62e3e49072bc@pks.im","threadId":"64646","inReplyTo":"20251218-b4-pks-odb-read-object-info-improvements-v2-0-62e3e49072bc@pks.im","subject":"[PATCH v2 5/7] packfile: disentangle return value of `packed_object_info()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-18T10:54:17Z","receivedAt":"2025-12-18T10:54:38Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The `packed_object_info()` function returns the type of the packed\nobject. While we use an `enum object_type` to store the return value,\nthis type is not to be confused with the actual object type. It _may_\ncontain the object type, but it may just as well encode that the given\npacked object is stored as a delta.\n\nWe have removed the only caller that relied on this returned object type\nin the preceding commit, so let's simplify semantics and return either 0\non success or a negative error code otherwise.\n\nThis unblocks a small optimization where we can skip reading the object\ntype altogether.\n\nSigned-off-by: Patrick Steinhardt \n---\n packfile.c | 21 ++++++++++++---------\n packfile.h | 4 ++++\n 2 files changed, 16 insertions(+), 9 deletions(-)\n\ndiff --git a/packfile.c b/packfile.c\nindex f7c33a2f77..8c6ef45a67 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -1587,6 +1587,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \tunsigned long size;\n \toff_t curpos = obj_offset;\n \tenum object_type type;\n+\tint ret;\n \n \t/*\n \t * We always get the representation type, but only convert it to\n@@ -1607,12 +1608,12 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \t\t\toff_t base_offset = get_delta_base(p, &w_curs, &tmp_pos,\n \t\t\t\t\t\t\t type, obj_offset);\n \t\t\tif (!base_offset) {\n-\t\t\t\ttype = OBJ_BAD;\n+\t\t\t\tret = -1;\n \t\t\t\tgoto out;\n \t\t\t}\n \t\t\t*oi->sizep = get_size_from_delta(p, &w_curs, tmp_pos);\n \t\t\tif (*oi->sizep == 0) {\n-\t\t\t\ttype = OBJ_BAD;\n+\t\t\t\tret = -1;\n \t\t\t\tgoto out;\n \t\t\t}\n \t\t} else {\n@@ -1625,7 +1626,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \t\tif (offset_to_pack_pos(p, obj_offset, &pos) < 0) {\n \t\t\terror(\"could not find object at offset %\"PRIuMAX\" \"\n \t\t\t \"in pack %s\", (uintmax_t)obj_offset, p->pack_name);\n-\t\t\ttype = OBJ_BAD;\n+\t\t\tret = -1;\n \t\t\tgoto out;\n \t\t}\n \n@@ -1639,7 +1640,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \t\tif (oi->typep)\n \t\t\t*oi->typep = ptot;\n \t\tif (ptot < 0) {\n-\t\t\ttype = OBJ_BAD;\n+\t\t\tret = -1;\n \t\t\tgoto out;\n \t\t}\n \t}\n@@ -1649,7 +1650,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \t\t\tif (get_delta_base_oid(p, &w_curs, curpos,\n \t\t\t\t\t oi->delta_base_oid,\n \t\t\t\t\t type, obj_offset) < 0) {\n-\t\t\t\ttype = OBJ_BAD;\n+\t\t\t\tret = -1;\n \t\t\t\tgoto out;\n \t\t\t}\n \t\t} else\n@@ -1672,9 +1673,11 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \t\tbreak;\n \t}\n \n+\tret = 0;\n+\n out:\n \tunuse_pack(&w_curs);\n-\treturn type;\n+\treturn ret;\n }\n \n static void *unpack_compressed_entry(struct packed_git *p,\n@@ -2152,7 +2155,7 @@ int packfile_store_read_object_info(struct packfile_store *store,\n \t\t\t\t unsigned flags UNUSED)\n {\n \tstruct pack_entry e;\n-\tint rtype;\n+\tint ret;\n \n \tif (!find_pack_entry(store->odb->repo, oid, &e))\n \t\treturn 1;\n@@ -2164,8 +2167,8 @@ int packfile_store_read_object_info(struct packfile_store *store,\n \tif (!oi)\n \t\treturn 0;\n \n-\trtype = packed_object_info(store->odb->repo, e.p, e.offset, oi);\n-\tif (rtype < 0) {\n+\tret = packed_object_info(store->odb->repo, e.p, e.offset, oi);\n+\tif (ret < 0) {\n \t\tmark_bad_packed_object(e.p, oid);\n \t\treturn -1;\n \t}\ndiff --git a/packfile.h b/packfile.h\nindex 59d162a3f4..07f5bfbc4f 100644\n--- a/packfile.h\n+++ b/packfile.h\n@@ -378,6 +378,10 @@ void release_pack_memory(size_t);\n /* global flag to enable extra checks when accessing packed objects */\n extern int do_check_packed_object_crc;\n \n+/*\n+ * Look up the object info for a specific offset in the packfile.\n+ * success, a negative error code otherwise.\n+ */\n int packed_object_info(struct repository *r,\n \t\t struct packed_git *pack,\n \t\t off_t offset, struct object_info *);\n\n-- \n2.52.0.351.gbe84eed79e.dirty\n\n"},{"id":"532459","messageId":"20251218-b4-pks-odb-read-object-info-improvements-v2-6-62e3e49072bc@pks.im","threadId":"64646","inReplyTo":"20251218-b4-pks-odb-read-object-info-improvements-v2-0-62e3e49072bc@pks.im","subject":"[PATCH v2 6/7] packfile: skip unpacking object header for disk size requests","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-18T10:54:18Z","receivedAt":"2025-12-18T10:54:40Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"While most of the object info requests for a packed object require us to\nunpack its headers, reading its disk size doesn't. We still unpack the\nobject header in that case though, which is unnecessary work.\n\nSkip reading the header if only the disk size is requested. This leads\nto a small speedup when reading disk size, only. The following benchmark\nwas done in the Git repository:\n\n Benchmark 1: ./git rev-list --disk-usage HEAD (rev = HEAD~)\n Time (mean ± σ): 105.2 ms ± 0.6 ms [User: 91.4 ms, System: 13.3 ms]\n Range (min … max): 103.7 ms … 106.0 ms 27 runs\n\n Benchmark 2: ./git rev-list --disk-usage HEAD (rev = HEAD)\n Time (mean ± σ): 96.7 ms ± 0.4 ms [User: 86.2 ms, System: 10.0 ms]\n Range (min … max): 96.2 ms … 98.1 ms 30 runs\n\n Summary\n ./git rev-list --disk-usage HEAD (rev = HEAD) ran\n 1.09 ± 0.01 times faster than ./git rev-list --disk-usage HEAD (rev = HEAD~)\n\nSigned-off-by: Patrick Steinhardt \n---\n packfile.c | 7 +++++--\n 1 file changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/packfile.c b/packfile.c\nindex 8c6ef45a67..a2ba237ce7 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -1586,7 +1586,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \tstruct pack_window *w_curs = NULL;\n \tunsigned long size;\n \toff_t curpos = obj_offset;\n-\tenum object_type type;\n+\tenum object_type type = OBJ_NONE;\n \tint ret;\n \n \t/*\n@@ -1598,7 +1598,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \t\t\t\t\t\t &type);\n \t\tif (!*oi->contentp)\n \t\t\ttype = OBJ_BAD;\n-\t} else {\n+\t} else if (oi->sizep || oi->typep || oi->delta_base_oid) {\n \t\ttype = unpack_object_header(p, &w_curs, &curpos, &size);\n \t}\n \n@@ -1662,6 +1662,9 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \toi->u.packed.pack = p;\n \n \tswitch (type) {\n+\tcase OBJ_NONE:\n+\t\toi->u.packed.type = PACKED_OBJECT_TYPE_UNKNOWN;\n+\t\tbreak;\n \tcase OBJ_REF_DELTA:\n \t\toi->u.packed.type = PACKED_OBJECT_TYPE_REF_DELTA;\n \t\tbreak;\n\n-- \n2.52.0.351.gbe84eed79e.dirty\n\n"},{"id":"532460","messageId":"20251218-b4-pks-odb-read-object-info-improvements-v2-7-62e3e49072bc@pks.im","threadId":"64646","inReplyTo":"20251218-b4-pks-odb-read-object-info-improvements-v2-0-62e3e49072bc@pks.im","subject":"[PATCH v2 7/7] packfile: drop repository parameter from `packed_object_info()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-12-18T10:54:19Z","receivedAt":"2025-12-18T10:54:46Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The function `packed_object_info()` takes a packfile and offset and\nreturns the object info for the corresponding object. Despite these two\nparameters though it also takes a repository pointer. This is redundant\ninformation though, as `struct packed_git` already has a repository\npointer that is always populated.\n\nDrop the redundant parameter.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/cat-file.c | 3 +--\n builtin/pack-objects.c | 4 ++--\n commit-graph.c | 2 +-\n pack-bitmap.c | 3 +--\n packfile.c | 8 ++++----\n packfile.h | 3 +--\n 6 files changed, 10 insertions(+), 13 deletions(-)\n\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex 505ddaa12f..2ad712e9f8 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -487,8 +487,7 @@ static void batch_object_write(const char *obj_name,\n \t\t\tdata->info.sizep = &data->size;\n \n \t\tif (pack)\n-\t\t\tret = packed_object_info(the_repository, pack,\n-\t\t\t\t\t\t offset, &data->info);\n+\t\t\tret = packed_object_info(pack, offset, &data->info);\n \t\telse\n \t\t\tret = odb_read_object_info_extended(the_repository->objects,\n \t\t\t\t\t\t\t &data->oid, &data->info,\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 1ce8d6ee21..85762f8c4f 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -2411,7 +2411,7 @@ static void drop_reused_delta(struct object_entry *entry)\n \n \toi.sizep = &size;\n \toi.typep = &type;\n-\tif (packed_object_info(the_repository, IN_PACK(entry), entry->in_pack_offset, &oi) < 0) {\n+\tif (packed_object_info(IN_PACK(entry), entry->in_pack_offset, &oi) < 0) {\n \t\t/*\n \t\t * We failed to get the info from this pack for some reason;\n \t\t * fall back to odb_read_object_info, which may find another copy.\n@@ -3748,7 +3748,7 @@ static int add_object_entry_from_pack(const struct object_id *oid,\n \t\tstruct object_info oi = OBJECT_INFO_INIT;\n \n \t\toi.typep = &type;\n-\t\tif (packed_object_info(the_repository, p, ofs, &oi) < 0) {\n+\t\tif (packed_object_info(p, ofs, &oi) < 0) {\n \t\t\tdie(_(\"could not get type of object %s in pack %s\"),\n \t\t\t oid_to_hex(oid), p->pack_name);\n \t\t} else if (type == OBJ_COMMIT) {\ndiff --git a/commit-graph.c b/commit-graph.c\nindex 80be2ff2c3..f572670bd0 100644\n--- a/commit-graph.c\n+++ b/commit-graph.c\n@@ -1499,7 +1499,7 @@ static int add_packed_commits(const struct object_id *oid,\n \t\tdisplay_progress(ctx->progress, ++ctx->progress_done);\n \n \toi.typep = &type;\n-\tif (packed_object_info(ctx->r, pack, offset, &oi) < 0)\n+\tif (packed_object_info(pack, offset, &oi) < 0)\n \t\tdie(_(\"unable to get type of object %s\"), oid_to_hex(oid));\n \n \tif (type != OBJ_COMMIT)\ndiff --git a/pack-bitmap.c b/pack-bitmap.c\nindex 8ca79725b1..972203f12b 100644\n--- a/pack-bitmap.c\n+++ b/pack-bitmap.c\n@@ -1876,8 +1876,7 @@ static unsigned long get_size_by_pos(struct bitmap_index *bitmap_git,\n \t\t\tofs = pack_pos_to_offset(pack, pos);\n \t\t}\n \n-\t\tif (packed_object_info(bitmap_repo(bitmap_git), pack, ofs,\n-\t\t\t\t &oi) < 0) {\n+\t\tif (packed_object_info(pack, ofs, &oi) < 0) {\n \t\t\tstruct object_id oid;\n \t\t\tnth_bitmap_object_oid(bitmap_git, &oid,\n \t\t\t\t\t pack_pos_to_index(pack, pos));\ndiff --git a/packfile.c b/packfile.c\nindex a2ba237ce7..39899aec49 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -1580,7 +1580,7 @@ static void add_delta_base_cache(struct packed_git *p, off_t base_offset,\n \thashmap_add(&delta_base_cache, &ent->ent);\n }\n \n-int packed_object_info(struct repository *r, struct packed_git *p,\n+int packed_object_info(struct packed_git *p,\n \t\t off_t obj_offset, struct object_info *oi)\n {\n \tstruct pack_window *w_curs = NULL;\n@@ -1594,7 +1594,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \t * a \"real\" type later if the caller is interested.\n \t */\n \tif (oi->contentp) {\n-\t\t*oi->contentp = cache_or_unpack_entry(r, p, obj_offset, oi->sizep,\n+\t\t*oi->contentp = cache_or_unpack_entry(p->repo, p, obj_offset, oi->sizep,\n \t\t\t\t\t\t &type);\n \t\tif (!*oi->contentp)\n \t\t\ttype = OBJ_BAD;\n@@ -1635,7 +1635,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \n \tif (oi->typep) {\n \t\tenum object_type ptot;\n-\t\tptot = packed_to_object_type(r, p, obj_offset,\n+\t\tptot = packed_to_object_type(p->repo, p, obj_offset,\n \t\t\t\t\t type, &w_curs, curpos);\n \t\tif (oi->typep)\n \t\t\t*oi->typep = ptot;\n@@ -2170,7 +2170,7 @@ int packfile_store_read_object_info(struct packfile_store *store,\n \tif (!oi)\n \t\treturn 0;\n \n-\tret = packed_object_info(store->odb->repo, e.p, e.offset, oi);\n+\tret = packed_object_info(e.p, e.offset, oi);\n \tif (ret < 0) {\n \t\tmark_bad_packed_object(e.p, oid);\n \t\treturn -1;\ndiff --git a/packfile.h b/packfile.h\nindex 07f5bfbc4f..573d06f6ba 100644\n--- a/packfile.h\n+++ b/packfile.h\n@@ -382,8 +382,7 @@ extern int do_check_packed_object_crc;\n * Look up the object info for a specific offset in the packfile.\n * success, a negative error code otherwise.\n */\n-int packed_object_info(struct repository *r,\n-\t\t struct packed_git *pack,\n+int packed_object_info(struct packed_git *pack,\n \t\t off_t offset, struct object_info *);\n \n void mark_bad_packed_object(struct packed_git *, const struct object_id *);\n\n-- \n2.52.0.351.gbe84eed79e.dirty\n\n"},{"id":"532852","messageId":"62dfd1ff-cc19-43bb-a622-af480fd72d2b@app.fastmail.com","threadId":"64646","inReplyTo":"20251218-b4-pks-odb-read-object-info-improvements-v2-4-62e3e49072bc@pks.im","subject":"Re: [PATCH v2 4/7] packfile: always populate pack-specific info when reading object info","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2025-12-30T17:03:24Z","receivedAt":"2025-12-30T17:03:45Z","isPatch":true,"sender":{"key":"kristofferhaugsbakk@fastmail.com","avatar":null},"body":"On Thu, Dec 18, 2025, at 11:54, Patrick Steinhardt wrote:\n> When reading object information via `packed_object_info()` we may not\n> populate the object info's packfile-specific fields. This leads to\n> inconsistent object info depending on whether the info was populated via\n> `packfile_store_read_object_info()` or `packed_object_info()`.\n>\n> Fix this inconsistecny so that we can always assume the pack info to be\n\ns/inconsistecny/inconsistency/\n\n> populated when reading object info from a pack.\n>\n> Signed-off-by: Patrick Steinhardt \n>[snip]\n"},{"id":"533027","messageId":"aVuirWnh5Yjj24XM@pks.im","threadId":"64646","inReplyTo":"62dfd1ff-cc19-43bb-a622-af480fd72d2b@app.fastmail.com","subject":"Re: [PATCH v2 4/7] packfile: always populate pack-specific info when reading object info","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-05T11:38:21Z","receivedAt":"2026-01-05T11:38:32Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Dec 30, 2025 at 06:03:24PM +0100, Kristoffer Haugsbakk wrote:\n> On Thu, Dec 18, 2025, at 11:54, Patrick Steinhardt wrote:\n> > When reading object information via `packed_object_info()` we may not\n> > populate the object info's packfile-specific fields. This leads to\n> > inconsistent object info depending on whether the info was populated via\n> > `packfile_store_read_object_info()` or `packed_object_info()`.\n> >\n> > Fix this inconsistecny so that we can always assume the pack info to be\n> \n> s/inconsistecny/inconsistency/\n\nThanks, I've queued this change locally now. I'll hold off sending a new\niteration for now though.\n\nPatrick\n"},{"id":"533051","messageId":"87seckp1zl.fsf@iotcl.com","threadId":"64646","inReplyTo":"20251218-b4-pks-odb-read-object-info-improvements-v2-2-62e3e49072bc@pks.im","subject":"Re: [PATCH v2 2/7] packfile: always declare object info to be OI_PACKED","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2026-01-05T14:29:02Z","receivedAt":"2026-01-05T14:29:30Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Patrick Steinhardt writes:\n\n> When reading object info via a packfile we yield one of two types:\n>\n> - The object can either be OI_PACKED, which is what a caller would\n> typically expect.\n>\n> - Or it can be OI_DBCACHED if it is stored in the delta base cache.\n>\n> The latter really is an implementation detail though, and callers\n> typically don't care at all about the difference. Furthermore, the\n> information whether or not it is part of the delta base cache can\n> already be derived via the `is_delta` field, so the fact that we discern\n> between OI_PACKED and OI_DBCACHED only further complicates the\n> interface.\n>\n> There aren't all that many callers that care about the `whence` field in\n> the first place. In fact, there's only three:\n>\n> - `packfile_store_read_object_info()` checks for `whence == OI_PACKED`\n> and then populates the packfile information of the object info\n> structure. We now start to do this also for deltified objects, which\n> gives its callers strictly more information.\n>\n> - `repack_local_links()` wants to determine whether the object is part\n> of a promisor pack and checks for `whence == OI_PACKED`. If so, it\n> verifies that the packfile is a promisor pack. It's arguably wrong\n> to declare that an object is not part of a promisor pack only\n> because it is stored in the delta base cache.\n>\n> - `is_not_in_promisor_pack_obj()` does the same, but checks that a\n> specific object is _not_ part of a promisor pack. The same reasoning\n> as above applies.\n>\n> Drop the OI_DBCACHED enum completely. None of the callers seem to care\n> about the distinction.\n\nThanks for clarifying these. I agree it makes sense to drop it.\n\n-- \nCheers,\nToon\n"},{"id":"533062","messageId":"87o6n8oyw2.fsf@iotcl.com","threadId":"64646","inReplyTo":"20251218-b4-pks-odb-read-object-info-improvements-v2-3-62e3e49072bc@pks.im","subject":"Re: [PATCH v2 3/7] packfile: extend `is_delta` field to allow for \"unknown\" state","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2026-01-05T15:35:57Z","receivedAt":"2026-01-05T15:36:06Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Patrick Steinhardt writes:\n\n> The `struct object_info::u::packed::is_delta` field determines whether\n> or not a specific object is stored as a delta. It only stores whether or\n> not the object is stored as delta, so it is treated as a boolean value.\n>\n> This boolean is insufficient though: when reading a packed object via\n> `packfile_store_read_object_info()` we know to skip parsing the actual\n> object when the user didn't request any object-specific data. In that\n> case we won't read the object itself, but will only look up its position\n> in the packfile. Consequently, we do not know whether it is a delta or\n> not.\n\nThis explains why you're introducing \"unknown\", but I'm having trouble\nunderstanding why we need distinction between ofs-delta and ref-delta?\n\n(To any other reader: If you want to know what those two are, check\n\"Deltified representation\" in Documentation/gitformat-pack.adoc)\n\n> This isn't really an issue right now, as the check for an empty request\n> is broken. But a subsequent commit will fix it, and once we do we will\n> have the need to also represent an \"unknown\" delta state.\n>\n> Prepare for this change by introducing a new enum that encodes the\n> object type. We don't use the \"unknown\" state just yet, but will start\n> to do so in the next commit.\n\nA little bit confusing this \"next commit\" is [PATCH 6/7], but that's a\nnote to any other reader and not so much a nitpick worth addressing.\n\n-- \nCheers,\nToon\n"},{"id":"533095","messageId":"aVytEHdNHDHHNpLt@pks.im","threadId":"64646","inReplyTo":"87o6n8oyw2.fsf@iotcl.com","subject":"Re: [PATCH v2 3/7] packfile: extend `is_delta` field to allow for \"unknown\" state","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-06T06:34:56Z","receivedAt":"2026-01-06T06:35:03Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Jan 05, 2026 at 04:35:57PM +0100, Toon Claes wrote:\n> Patrick Steinhardt writes:\n> \n> > The `struct object_info::u::packed::is_delta` field determines whether\n> > or not a specific object is stored as a delta. It only stores whether or\n> > not the object is stored as delta, so it is treated as a boolean value.\n> >\n> > This boolean is insufficient though: when reading a packed object via\n> > `packfile_store_read_object_info()` we know to skip parsing the actual\n> > object when the user didn't request any object-specific data. In that\n> > case we won't read the object itself, but will only look up its position\n> > in the packfile. Consequently, we do not know whether it is a delta or\n> > not.\n> \n> This explains why you're introducing \"unknown\", but I'm having trouble\n> understanding why we need distinction between ofs-delta and ref-delta?\n> \n> (To any other reader: If you want to know what those two are, check\n> \"Deltified representation\" in Documentation/gitformat-pack.adoc)\n\nGood question, and indeed we don't need that information at any\ncallsite right now. We do discern the delta type in various locations,\nbut mostly do so internally at \"packfile.c\" so that we know how to read\nthe given object. And there we rely on the `OBJ_REF_DELTA` and\n`OBJ_OFS_DELTA` types.\n\nWe could of course adapt this to only use `PACKED_OBJECT_TYPE_DELTA`.\nBut we already have the information readily available at our figertips,\nand over time I'd ideally rather want to get rid of the `OBJ_*_DELTA`\nvalues as they leak internal implementation details of the packfile\nstore into the generic object interfaces. That's way down the road, but\nby keeping around the information now it makes such a later conversion\neasier.\n\n> > This isn't really an issue right now, as the check for an empty request\n> > is broken. But a subsequent commit will fix it, and once we do we will\n> > have the need to also represent an \"unknown\" delta state.\n> >\n> > Prepare for this change by introducing a new enum that encodes the\n> > object type. We don't use the \"unknown\" state just yet, but will start\n> > to do so in the next commit.\n> \n> A little bit confusing this \"next commit\" is [PATCH 6/7], but that's a\n> note to any other reader and not so much a nitpick worth addressing.\n\nFair. I've fixed this up locally to say \"subsequent commit\".\n\nThanks!\n\nPatrick\n"},{"id":"533097","messageId":"20260106-b4-pks-odb-read-object-info-improvements-v3-0-b5e02fae1fb0@pks.im","threadId":"64646","inReplyTo":"20251218-b4-pks-odb-read-object-info-improvements-v1-0-81c8368492be@pks.im","subject":"[PATCH v3 0/7] Improvements for reading object info","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-06T06:54:56Z","receivedAt":"2026-01-06T06:55:05Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nthis patch series contains various small improvements for reading object\ninfo for either loose or packed objects. These improvements were split\nout of a larger patch series where I'm about to introduce a new generic\n`odb_for_each_object()` function.\n\nChanges in v3:\n - Fix a commit message typo.\n - Fix a function comment missing some words.\n - Link to v2: https://lore.kernel.org/r/20251218-b4-pks-odb-read-object-info-improvements-v2-0-62e3e49072bc@pks.im\n\nChanges in v2:\n - Rebase the series on top of master with jc/object-read-stream-fix\n merged into it. I've also evicted the patch that fixes the same\n underlying issue.\n - Improve the commit message that drops OI_DBCACHED to explain why\n this is a safe refactoring.\n - Link to v1: https://lore.kernel.org/r/20251218-b4-pks-odb-read-object-info-improvements-v1-0-81c8368492be@pks.im\n\nThanks!\n\nPatrick\n\n---\nPatrick Steinhardt (7):\n object-file: always set OI_LOOSE when reading object info\n packfile: always declare object info to be OI_PACKED\n packfile: extend `is_delta` field to allow for \"unknown\" state\n packfile: always populate pack-specific info when reading object info\n packfile: disentangle return value of `packed_object_info()`\n packfile: skip unpacking object header for disk size requests\n packfile: drop repository parameter from `packed_object_info()`\n\n builtin/cat-file.c | 3 +--\n builtin/pack-objects.c | 4 ++--\n commit-graph.c | 2 +-\n object-file.c | 19 ++++++++++++----\n odb.h | 8 +++++--\n pack-bitmap.c | 3 +--\n packfile.c | 61 ++++++++++++++++++++++++++++++--------------------\n packfile.h | 7 ++++--\n 8 files changed, 68 insertions(+), 39 deletions(-)\n\nRange-diff versus v2:\n\n1: 8b6b891c2f = 1: 9efc7d00c1 object-file: always set OI_LOOSE when reading object info\n2: b83dd3d689 = 2: efd29f0e27 packfile: always declare object info to be OI_PACKED\n3: 6815b23dd7 ! 3: 1448cd37b3 packfile: extend `is_delta` field to allow for \"unknown\" state\n @@ Commit message\n \n Prepare for this change by introducing a new enum that encodes the\n object type. We don't use the \"unknown\" state just yet, but will start\n - to do so in the next commit.\n + to do so in a subsequent commit.\n \n Signed-off-by: Patrick Steinhardt \n \n4: ec18d71d07 ! 4: 8eb063df04 packfile: always populate pack-specific info when reading object info\n @@ Commit message\n inconsistent object info depending on whether the info was populated via\n `packfile_store_read_object_info()` or `packed_object_info()`.\n \n - Fix this inconsistecny so that we can always assume the pack info to be\n + Fix this inconsistency so that we can always assume the pack info to be\n populated when reading object info from a pack.\n \n Signed-off-by: Patrick Steinhardt \n5: cc0694bb0f ! 5: 33582ef5e0 packfile: disentangle return value of `packed_object_info()`\n @@ packfile.h: void release_pack_memory(size_t);\n \n +/*\n + * Look up the object info for a specific offset in the packfile.\n -+ * success, a negative error code otherwise.\n ++ * Returns zero on success, a negative error code otherwise.\n + */\n int packed_object_info(struct repository *r,\n \t\t struct packed_git *pack,\n6: 98eee570b8 = 6: 42ec9b8170 packfile: skip unpacking object header for disk size requests\n7: 9c9e71d7b2 ! 7: 03bc55e74f packfile: drop repository parameter from `packed_object_info()`\n @@ packfile.c: int packfile_store_read_object_info(struct packfile_store *store,\n ## packfile.h ##\n @@ packfile.h: extern int do_check_packed_object_crc;\n * Look up the object info for a specific offset in the packfile.\n - * success, a negative error code otherwise.\n + * Returns zero on success, a negative error code otherwise.\n */\n -int packed_object_info(struct repository *r,\n -\t\t struct packed_git *pack,\n\n---\nbase-commit: 7df68b50e49b6a1b576abb19b2e5d457749bc28b\nchange-id: 20251215-b4-pks-odb-read-object-info-improvements-0e031ef827d2\n\n"},{"id":"533098","messageId":"20260106-b4-pks-odb-read-object-info-improvements-v3-1-b5e02fae1fb0@pks.im","threadId":"64646","inReplyTo":"20260106-b4-pks-odb-read-object-info-improvements-v3-0-b5e02fae1fb0@pks.im","subject":"[PATCH v3 1/7] object-file: always set OI_LOOSE when reading object info","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-06T06:54:57Z","receivedAt":"2026-01-06T06:55:06Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"There are some early returns in ``odb_source_loose_read_object_info()`\nin cases where we don't have to open the loose object. These return\npaths do not set `struct object_info::whence` to `OI_LOOSE` though, so\nit becomes impossible for the caller to tell the format of such an\nobject.\n\nNobody seems to care about this right now, but it's a bug waiting to\nhappen. Fix this by always setting `whence` on success.\n\nSigned-off-by: Patrick Steinhardt \n---\n object-file.c | 19 +++++++++++++++----\n 1 file changed, 15 insertions(+), 4 deletions(-)\n\ndiff --git a/object-file.c b/object-file.c\nindex 6280e42f34..d566df427a 100644\n--- a/object-file.c\n+++ b/object-file.c\n@@ -439,12 +439,23 @@ int odb_source_loose_read_object_info(struct odb_source *source,\n \t */\n \tif (!oi || (!oi->typep && !oi->sizep && !oi->contentp)) {\n \t\tstruct stat st;\n-\t\tif ((!oi || !oi->disk_sizep) && (flags & OBJECT_INFO_QUICK))\n-\t\t\treturn quick_has_loose(source->loose, oid) ? 0 : -1;\n+\n+\t\tif ((!oi || !oi->disk_sizep) && (flags & OBJECT_INFO_QUICK)) {\n+\t\t\tstatus = quick_has_loose(source->loose, oid) ? 0 : -1;\n+\t\t\tif (!status && oi)\n+\t\t\t\toi->whence = OI_LOOSE;\n+\t\t\treturn status;\n+\t\t}\n+\n \t\tif (stat_loose_object(source->loose, oid, &st, &path) < 0)\n \t\t\treturn -1;\n-\t\tif (oi && oi->disk_sizep)\n-\t\t\t*oi->disk_sizep = st.st_size;\n+\n+\t\tif (oi) {\n+\t\t\tif (oi->disk_sizep)\n+\t\t\t\t*oi->disk_sizep = st.st_size;\n+\t\t\toi->whence = OI_LOOSE;\n+\t\t}\n+\n \t\treturn 0;\n \t}\n \n\n-- \n2.52.0.508.g883dcfc63e.dirty\n\n"},{"id":"533099","messageId":"20260106-b4-pks-odb-read-object-info-improvements-v3-2-b5e02fae1fb0@pks.im","threadId":"64646","inReplyTo":"20260106-b4-pks-odb-read-object-info-improvements-v3-0-b5e02fae1fb0@pks.im","subject":"[PATCH v3 2/7] packfile: always declare object info to be OI_PACKED","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-06T06:54:58Z","receivedAt":"2026-01-06T06:55:09Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When reading object info via a packfile we yield one of two types:\n\n - The object can either be OI_PACKED, which is what a caller would\n typically expect.\n\n - Or it can be OI_DBCACHED if it is stored in the delta base cache.\n\nThe latter really is an implementation detail though, and callers\ntypically don't care at all about the difference. Furthermore, the\ninformation whether or not it is part of the delta base cache can\nalready be derived via the `is_delta` field, so the fact that we discern\nbetween OI_PACKED and OI_DBCACHED only further complicates the\ninterface.\n\nThere aren't all that many callers that care about the `whence` field in\nthe first place. In fact, there's only three:\n\n - `packfile_store_read_object_info()` checks for `whence == OI_PACKED`\n and then populates the packfile information of the object info\n structure. We now start to do this also for deltified objects, which\n gives its callers strictly more information.\n\n - `repack_local_links()` wants to determine whether the object is part\n of a promisor pack and checks for `whence == OI_PACKED`. If so, it\n verifies that the packfile is a promisor pack. It's arguably wrong\n to declare that an object is not part of a promisor pack only\n because it is stored in the delta base cache.\n\n - `is_not_in_promisor_pack_obj()` does the same, but checks that a\n specific object is _not_ part of a promisor pack. The same reasoning\n as above applies.\n\nDrop the OI_DBCACHED enum completely. None of the callers seem to care\nabout the distinction.\n\nSigned-off-by: Patrick Steinhardt \n---\n odb.h | 1 -\n packfile.c | 3 +--\n 2 files changed, 1 insertion(+), 3 deletions(-)\n\ndiff --git a/odb.h b/odb.h\nindex 014cd9585a..73b0b87ad5 100644\n--- a/odb.h\n+++ b/odb.h\n@@ -330,7 +330,6 @@ struct object_info {\n \t\tOI_CACHED,\n \t\tOI_LOOSE,\n \t\tOI_PACKED,\n-\t\tOI_DBCACHED\n \t} whence;\n \tunion {\n \t\t/*\ndiff --git a/packfile.c b/packfile.c\nindex 08a0863fc3..b0c6665c87 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -1656,8 +1656,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \t\t\toidclr(oi->delta_base_oid, p->repo->hash_algo);\n \t}\n \n-\toi->whence = in_delta_base_cache(p, obj_offset) ? OI_DBCACHED :\n-\t\t\t\t\t\t\t OI_PACKED;\n+\toi->whence = OI_PACKED;\n \n out:\n \tunuse_pack(&w_curs);\n\n-- \n2.52.0.508.g883dcfc63e.dirty\n\n"},{"id":"533100","messageId":"20260106-b4-pks-odb-read-object-info-improvements-v3-3-b5e02fae1fb0@pks.im","threadId":"64646","inReplyTo":"20260106-b4-pks-odb-read-object-info-improvements-v3-0-b5e02fae1fb0@pks.im","subject":"[PATCH v3 3/7] packfile: extend `is_delta` field to allow for \"unknown\" state","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-06T06:54:59Z","receivedAt":"2026-01-06T06:55:11Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The `struct object_info::u::packed::is_delta` field determines whether\nor not a specific object is stored as a delta. It only stores whether or\nnot the object is stored as delta, so it is treated as a boolean value.\n\nThis boolean is insufficient though: when reading a packed object via\n`packfile_store_read_object_info()` we know to skip parsing the actual\nobject when the user didn't request any object-specific data. In that\ncase we won't read the object itself, but will only look up its position\nin the packfile. Consequently, we do not know whether it is a delta or\nnot.\n\nThis isn't really an issue right now, as the check for an empty request\nis broken. But a subsequent commit will fix it, and once we do we will\nhave the need to also represent an \"unknown\" delta state.\n\nPrepare for this change by introducing a new enum that encodes the\nobject type. We don't use the \"unknown\" state just yet, but will start\nto do so in a subsequent commit.\n\nSigned-off-by: Patrick Steinhardt \n---\n odb.h | 7 ++++++-\n packfile.c | 17 ++++++++++++++---\n 2 files changed, 20 insertions(+), 4 deletions(-)\n\ndiff --git a/odb.h b/odb.h\nindex 73b0b87ad5..afae5e5c01 100644\n--- a/odb.h\n+++ b/odb.h\n@@ -343,7 +343,12 @@ struct object_info {\n \t\tstruct {\n \t\t\tstruct packed_git *pack;\n \t\t\toff_t offset;\n-\t\t\tunsigned int is_delta;\n+\t\t\tenum packed_object_type {\n+\t\t\t\tPACKED_OBJECT_TYPE_UNKNOWN,\n+\t\t\t\tPACKED_OBJECT_TYPE_FULL,\n+\t\t\t\tPACKED_OBJECT_TYPE_OFS_DELTA,\n+\t\t\t\tPACKED_OBJECT_TYPE_REF_DELTA,\n+\t\t\t} type;\n \t\t} packed;\n \t} u;\n };\ndiff --git a/packfile.c b/packfile.c\nindex b0c6665c87..cc797b2b6a 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -2159,8 +2159,18 @@ int packfile_store_read_object_info(struct packfile_store *store,\n \tif (oi->whence == OI_PACKED) {\n \t\toi->u.packed.offset = e.offset;\n \t\toi->u.packed.pack = e.p;\n-\t\toi->u.packed.is_delta = (rtype == OBJ_REF_DELTA ||\n-\t\t\t\t\t rtype == OBJ_OFS_DELTA);\n+\n+\t\tswitch (rtype) {\n+\t\tcase OBJ_REF_DELTA:\n+\t\t\toi->u.packed.type = PACKED_OBJECT_TYPE_REF_DELTA;\n+\t\t\tbreak;\n+\t\tcase OBJ_OFS_DELTA:\n+\t\t\toi->u.packed.type = PACKED_OBJECT_TYPE_OFS_DELTA;\n+\t\t\tbreak;\n+\t\tdefault:\n+\t\t\toi->u.packed.type = PACKED_OBJECT_TYPE_FULL;\n+\t\t\tbreak;\n+\t\t}\n \t}\n \n \treturn 0;\n@@ -2531,7 +2541,8 @@ int packfile_store_read_object_stream(struct odb_read_stream **out,\n \toi.sizep = &size;\n \n \tif (packfile_store_read_object_info(store, oid, &oi, 0) ||\n-\t oi.u.packed.is_delta ||\n+\t oi.u.packed.type == PACKED_OBJECT_TYPE_REF_DELTA ||\n+\t oi.u.packed.type == PACKED_OBJECT_TYPE_OFS_DELTA ||\n \t repo_settings_get_big_file_threshold(store->odb->repo) >= size)\n \t\treturn -1;\n \n\n-- \n2.52.0.508.g883dcfc63e.dirty\n\n"},{"id":"533101","messageId":"20260106-b4-pks-odb-read-object-info-improvements-v3-4-b5e02fae1fb0@pks.im","threadId":"64646","inReplyTo":"20260106-b4-pks-odb-read-object-info-improvements-v3-0-b5e02fae1fb0@pks.im","subject":"[PATCH v3 4/7] packfile: always populate pack-specific info when reading object info","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-06T06:55:00Z","receivedAt":"2026-01-06T06:55:14Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When reading object information via `packed_object_info()` we may not\npopulate the object info's packfile-specific fields. This leads to\ninconsistent object info depending on whether the info was populated via\n`packfile_store_read_object_info()` or `packed_object_info()`.\n\nFix this inconsistency so that we can always assume the pack info to be\npopulated when reading object info from a pack.\n\nSigned-off-by: Patrick Steinhardt \n---\n packfile.c | 31 ++++++++++++++-----------------\n 1 file changed, 14 insertions(+), 17 deletions(-)\n\ndiff --git a/packfile.c b/packfile.c\nindex cc797b2b6a..f7c33a2f77 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -1657,6 +1657,20 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \t}\n \n \toi->whence = OI_PACKED;\n+\toi->u.packed.offset = obj_offset;\n+\toi->u.packed.pack = p;\n+\n+\tswitch (type) {\n+\tcase OBJ_REF_DELTA:\n+\t\toi->u.packed.type = PACKED_OBJECT_TYPE_REF_DELTA;\n+\t\tbreak;\n+\tcase OBJ_OFS_DELTA:\n+\t\toi->u.packed.type = PACKED_OBJECT_TYPE_OFS_DELTA;\n+\t\tbreak;\n+\tdefault:\n+\t\toi->u.packed.type = PACKED_OBJECT_TYPE_FULL;\n+\t\tbreak;\n+\t}\n \n out:\n \tunuse_pack(&w_curs);\n@@ -2156,23 +2170,6 @@ int packfile_store_read_object_info(struct packfile_store *store,\n \t\treturn -1;\n \t}\n \n-\tif (oi->whence == OI_PACKED) {\n-\t\toi->u.packed.offset = e.offset;\n-\t\toi->u.packed.pack = e.p;\n-\n-\t\tswitch (rtype) {\n-\t\tcase OBJ_REF_DELTA:\n-\t\t\toi->u.packed.type = PACKED_OBJECT_TYPE_REF_DELTA;\n-\t\t\tbreak;\n-\t\tcase OBJ_OFS_DELTA:\n-\t\t\toi->u.packed.type = PACKED_OBJECT_TYPE_OFS_DELTA;\n-\t\t\tbreak;\n-\t\tdefault:\n-\t\t\toi->u.packed.type = PACKED_OBJECT_TYPE_FULL;\n-\t\t\tbreak;\n-\t\t}\n-\t}\n-\n \treturn 0;\n }\n \n\n-- \n2.52.0.508.g883dcfc63e.dirty\n\n"},{"id":"533102","messageId":"20260106-b4-pks-odb-read-object-info-improvements-v3-5-b5e02fae1fb0@pks.im","threadId":"64646","inReplyTo":"20260106-b4-pks-odb-read-object-info-improvements-v3-0-b5e02fae1fb0@pks.im","subject":"[PATCH v3 5/7] packfile: disentangle return value of `packed_object_info()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-06T06:55:01Z","receivedAt":"2026-01-06T06:55:16Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The `packed_object_info()` function returns the type of the packed\nobject. While we use an `enum object_type` to store the return value,\nthis type is not to be confused with the actual object type. It _may_\ncontain the object type, but it may just as well encode that the given\npacked object is stored as a delta.\n\nWe have removed the only caller that relied on this returned object type\nin the preceding commit, so let's simplify semantics and return either 0\non success or a negative error code otherwise.\n\nThis unblocks a small optimization where we can skip reading the object\ntype altogether.\n\nSigned-off-by: Patrick Steinhardt \n---\n packfile.c | 21 ++++++++++++---------\n packfile.h | 4 ++++\n 2 files changed, 16 insertions(+), 9 deletions(-)\n\ndiff --git a/packfile.c b/packfile.c\nindex f7c33a2f77..8c6ef45a67 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -1587,6 +1587,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \tunsigned long size;\n \toff_t curpos = obj_offset;\n \tenum object_type type;\n+\tint ret;\n \n \t/*\n \t * We always get the representation type, but only convert it to\n@@ -1607,12 +1608,12 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \t\t\toff_t base_offset = get_delta_base(p, &w_curs, &tmp_pos,\n \t\t\t\t\t\t\t type, obj_offset);\n \t\t\tif (!base_offset) {\n-\t\t\t\ttype = OBJ_BAD;\n+\t\t\t\tret = -1;\n \t\t\t\tgoto out;\n \t\t\t}\n \t\t\t*oi->sizep = get_size_from_delta(p, &w_curs, tmp_pos);\n \t\t\tif (*oi->sizep == 0) {\n-\t\t\t\ttype = OBJ_BAD;\n+\t\t\t\tret = -1;\n \t\t\t\tgoto out;\n \t\t\t}\n \t\t} else {\n@@ -1625,7 +1626,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \t\tif (offset_to_pack_pos(p, obj_offset, &pos) < 0) {\n \t\t\terror(\"could not find object at offset %\"PRIuMAX\" \"\n \t\t\t \"in pack %s\", (uintmax_t)obj_offset, p->pack_name);\n-\t\t\ttype = OBJ_BAD;\n+\t\t\tret = -1;\n \t\t\tgoto out;\n \t\t}\n \n@@ -1639,7 +1640,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \t\tif (oi->typep)\n \t\t\t*oi->typep = ptot;\n \t\tif (ptot < 0) {\n-\t\t\ttype = OBJ_BAD;\n+\t\t\tret = -1;\n \t\t\tgoto out;\n \t\t}\n \t}\n@@ -1649,7 +1650,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \t\t\tif (get_delta_base_oid(p, &w_curs, curpos,\n \t\t\t\t\t oi->delta_base_oid,\n \t\t\t\t\t type, obj_offset) < 0) {\n-\t\t\t\ttype = OBJ_BAD;\n+\t\t\t\tret = -1;\n \t\t\t\tgoto out;\n \t\t\t}\n \t\t} else\n@@ -1672,9 +1673,11 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \t\tbreak;\n \t}\n \n+\tret = 0;\n+\n out:\n \tunuse_pack(&w_curs);\n-\treturn type;\n+\treturn ret;\n }\n \n static void *unpack_compressed_entry(struct packed_git *p,\n@@ -2152,7 +2155,7 @@ int packfile_store_read_object_info(struct packfile_store *store,\n \t\t\t\t unsigned flags UNUSED)\n {\n \tstruct pack_entry e;\n-\tint rtype;\n+\tint ret;\n \n \tif (!find_pack_entry(store->odb->repo, oid, &e))\n \t\treturn 1;\n@@ -2164,8 +2167,8 @@ int packfile_store_read_object_info(struct packfile_store *store,\n \tif (!oi)\n \t\treturn 0;\n \n-\trtype = packed_object_info(store->odb->repo, e.p, e.offset, oi);\n-\tif (rtype < 0) {\n+\tret = packed_object_info(store->odb->repo, e.p, e.offset, oi);\n+\tif (ret < 0) {\n \t\tmark_bad_packed_object(e.p, oid);\n \t\treturn -1;\n \t}\ndiff --git a/packfile.h b/packfile.h\nindex 59d162a3f4..d7cce582af 100644\n--- a/packfile.h\n+++ b/packfile.h\n@@ -378,6 +378,10 @@ void release_pack_memory(size_t);\n /* global flag to enable extra checks when accessing packed objects */\n extern int do_check_packed_object_crc;\n \n+/*\n+ * Look up the object info for a specific offset in the packfile.\n+ * Returns zero on success, a negative error code otherwise.\n+ */\n int packed_object_info(struct repository *r,\n \t\t struct packed_git *pack,\n \t\t off_t offset, struct object_info *);\n\n-- \n2.52.0.508.g883dcfc63e.dirty\n\n"},{"id":"533103","messageId":"20260106-b4-pks-odb-read-object-info-improvements-v3-6-b5e02fae1fb0@pks.im","threadId":"64646","inReplyTo":"20260106-b4-pks-odb-read-object-info-improvements-v3-0-b5e02fae1fb0@pks.im","subject":"[PATCH v3 6/7] packfile: skip unpacking object header for disk size requests","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-06T06:55:02Z","receivedAt":"2026-01-06T06:55:20Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"While most of the object info requests for a packed object require us to\nunpack its headers, reading its disk size doesn't. We still unpack the\nobject header in that case though, which is unnecessary work.\n\nSkip reading the header if only the disk size is requested. This leads\nto a small speedup when reading disk size, only. The following benchmark\nwas done in the Git repository:\n\n Benchmark 1: ./git rev-list --disk-usage HEAD (rev = HEAD~)\n Time (mean ± σ): 105.2 ms ± 0.6 ms [User: 91.4 ms, System: 13.3 ms]\n Range (min … max): 103.7 ms … 106.0 ms 27 runs\n\n Benchmark 2: ./git rev-list --disk-usage HEAD (rev = HEAD)\n Time (mean ± σ): 96.7 ms ± 0.4 ms [User: 86.2 ms, System: 10.0 ms]\n Range (min … max): 96.2 ms … 98.1 ms 30 runs\n\n Summary\n ./git rev-list --disk-usage HEAD (rev = HEAD) ran\n 1.09 ± 0.01 times faster than ./git rev-list --disk-usage HEAD (rev = HEAD~)\n\nSigned-off-by: Patrick Steinhardt \n---\n packfile.c | 7 +++++--\n 1 file changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/packfile.c b/packfile.c\nindex 8c6ef45a67..a2ba237ce7 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -1586,7 +1586,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \tstruct pack_window *w_curs = NULL;\n \tunsigned long size;\n \toff_t curpos = obj_offset;\n-\tenum object_type type;\n+\tenum object_type type = OBJ_NONE;\n \tint ret;\n \n \t/*\n@@ -1598,7 +1598,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \t\t\t\t\t\t &type);\n \t\tif (!*oi->contentp)\n \t\t\ttype = OBJ_BAD;\n-\t} else {\n+\t} else if (oi->sizep || oi->typep || oi->delta_base_oid) {\n \t\ttype = unpack_object_header(p, &w_curs, &curpos, &size);\n \t}\n \n@@ -1662,6 +1662,9 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \toi->u.packed.pack = p;\n \n \tswitch (type) {\n+\tcase OBJ_NONE:\n+\t\toi->u.packed.type = PACKED_OBJECT_TYPE_UNKNOWN;\n+\t\tbreak;\n \tcase OBJ_REF_DELTA:\n \t\toi->u.packed.type = PACKED_OBJECT_TYPE_REF_DELTA;\n \t\tbreak;\n\n-- \n2.52.0.508.g883dcfc63e.dirty\n\n"},{"id":"533104","messageId":"20260106-b4-pks-odb-read-object-info-improvements-v3-7-b5e02fae1fb0@pks.im","threadId":"64646","inReplyTo":"20260106-b4-pks-odb-read-object-info-improvements-v3-0-b5e02fae1fb0@pks.im","subject":"[PATCH v3 7/7] packfile: drop repository parameter from `packed_object_info()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-06T06:55:03Z","receivedAt":"2026-01-06T06:55:22Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The function `packed_object_info()` takes a packfile and offset and\nreturns the object info for the corresponding object. Despite these two\nparameters though it also takes a repository pointer. This is redundant\ninformation though, as `struct packed_git` already has a repository\npointer that is always populated.\n\nDrop the redundant parameter.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/cat-file.c | 3 +--\n builtin/pack-objects.c | 4 ++--\n commit-graph.c | 2 +-\n pack-bitmap.c | 3 +--\n packfile.c | 8 ++++----\n packfile.h | 3 +--\n 6 files changed, 10 insertions(+), 13 deletions(-)\n\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex 505ddaa12f..2ad712e9f8 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -487,8 +487,7 @@ static void batch_object_write(const char *obj_name,\n \t\t\tdata->info.sizep = &data->size;\n \n \t\tif (pack)\n-\t\t\tret = packed_object_info(the_repository, pack,\n-\t\t\t\t\t\t offset, &data->info);\n+\t\t\tret = packed_object_info(pack, offset, &data->info);\n \t\telse\n \t\t\tret = odb_read_object_info_extended(the_repository->objects,\n \t\t\t\t\t\t\t &data->oid, &data->info,\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 1ce8d6ee21..85762f8c4f 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -2411,7 +2411,7 @@ static void drop_reused_delta(struct object_entry *entry)\n \n \toi.sizep = &size;\n \toi.typep = &type;\n-\tif (packed_object_info(the_repository, IN_PACK(entry), entry->in_pack_offset, &oi) < 0) {\n+\tif (packed_object_info(IN_PACK(entry), entry->in_pack_offset, &oi) < 0) {\n \t\t/*\n \t\t * We failed to get the info from this pack for some reason;\n \t\t * fall back to odb_read_object_info, which may find another copy.\n@@ -3748,7 +3748,7 @@ static int add_object_entry_from_pack(const struct object_id *oid,\n \t\tstruct object_info oi = OBJECT_INFO_INIT;\n \n \t\toi.typep = &type;\n-\t\tif (packed_object_info(the_repository, p, ofs, &oi) < 0) {\n+\t\tif (packed_object_info(p, ofs, &oi) < 0) {\n \t\t\tdie(_(\"could not get type of object %s in pack %s\"),\n \t\t\t oid_to_hex(oid), p->pack_name);\n \t\t} else if (type == OBJ_COMMIT) {\ndiff --git a/commit-graph.c b/commit-graph.c\nindex 80be2ff2c3..f572670bd0 100644\n--- a/commit-graph.c\n+++ b/commit-graph.c\n@@ -1499,7 +1499,7 @@ static int add_packed_commits(const struct object_id *oid,\n \t\tdisplay_progress(ctx->progress, ++ctx->progress_done);\n \n \toi.typep = &type;\n-\tif (packed_object_info(ctx->r, pack, offset, &oi) < 0)\n+\tif (packed_object_info(pack, offset, &oi) < 0)\n \t\tdie(_(\"unable to get type of object %s\"), oid_to_hex(oid));\n \n \tif (type != OBJ_COMMIT)\ndiff --git a/pack-bitmap.c b/pack-bitmap.c\nindex 8ca79725b1..972203f12b 100644\n--- a/pack-bitmap.c\n+++ b/pack-bitmap.c\n@@ -1876,8 +1876,7 @@ static unsigned long get_size_by_pos(struct bitmap_index *bitmap_git,\n \t\t\tofs = pack_pos_to_offset(pack, pos);\n \t\t}\n \n-\t\tif (packed_object_info(bitmap_repo(bitmap_git), pack, ofs,\n-\t\t\t\t &oi) < 0) {\n+\t\tif (packed_object_info(pack, ofs, &oi) < 0) {\n \t\t\tstruct object_id oid;\n \t\t\tnth_bitmap_object_oid(bitmap_git, &oid,\n \t\t\t\t\t pack_pos_to_index(pack, pos));\ndiff --git a/packfile.c b/packfile.c\nindex a2ba237ce7..39899aec49 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -1580,7 +1580,7 @@ static void add_delta_base_cache(struct packed_git *p, off_t base_offset,\n \thashmap_add(&delta_base_cache, &ent->ent);\n }\n \n-int packed_object_info(struct repository *r, struct packed_git *p,\n+int packed_object_info(struct packed_git *p,\n \t\t off_t obj_offset, struct object_info *oi)\n {\n \tstruct pack_window *w_curs = NULL;\n@@ -1594,7 +1594,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \t * a \"real\" type later if the caller is interested.\n \t */\n \tif (oi->contentp) {\n-\t\t*oi->contentp = cache_or_unpack_entry(r, p, obj_offset, oi->sizep,\n+\t\t*oi->contentp = cache_or_unpack_entry(p->repo, p, obj_offset, oi->sizep,\n \t\t\t\t\t\t &type);\n \t\tif (!*oi->contentp)\n \t\t\ttype = OBJ_BAD;\n@@ -1635,7 +1635,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \n \tif (oi->typep) {\n \t\tenum object_type ptot;\n-\t\tptot = packed_to_object_type(r, p, obj_offset,\n+\t\tptot = packed_to_object_type(p->repo, p, obj_offset,\n \t\t\t\t\t type, &w_curs, curpos);\n \t\tif (oi->typep)\n \t\t\t*oi->typep = ptot;\n@@ -2170,7 +2170,7 @@ int packfile_store_read_object_info(struct packfile_store *store,\n \tif (!oi)\n \t\treturn 0;\n \n-\tret = packed_object_info(store->odb->repo, e.p, e.offset, oi);\n+\tret = packed_object_info(e.p, e.offset, oi);\n \tif (ret < 0) {\n \t\tmark_bad_packed_object(e.p, oid);\n \t\treturn -1;\ndiff --git a/packfile.h b/packfile.h\nindex d7cce582af..33fed26362 100644\n--- a/packfile.h\n+++ b/packfile.h\n@@ -382,8 +382,7 @@ extern int do_check_packed_object_crc;\n * Look up the object info for a specific offset in the packfile.\n * Returns zero on success, a negative error code otherwise.\n */\n-int packed_object_info(struct repository *r,\n-\t\t struct packed_git *pack,\n+int packed_object_info(struct packed_git *pack,\n \t\t off_t offset, struct object_info *);\n \n void mark_bad_packed_object(struct packed_git *, const struct object_id *);\n\n-- \n2.52.0.508.g883dcfc63e.dirty\n\n"},{"id":"533188","messageId":"CAOLa=ZSNmi_Lzb=3EdWks=mMOPvfijT2659y4YtxWnUKVUOXaA@mail.gmail.com","threadId":"64646","inReplyTo":"20260106-b4-pks-odb-read-object-info-improvements-v3-1-b5e02fae1fb0@pks.im","subject":"Re: [PATCH v3 1/7] object-file: always set OI_LOOSE when reading object info","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-01-07T08:50:45Z","receivedAt":"2026-01-07T08:50:47Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt writes:\n\n> There are some early returns in ``odb_source_loose_read_object_info()`\n\nNit: s/``/`\n\n> in cases where we don't have to open the loose object. These return\n> paths do not set `struct object_info::whence` to `OI_LOOSE` though, so\n> it becomes impossible for the caller to tell the format of such an\n> object.\n>\n> Nobody seems to care about this right now, but it's a bug waiting to\n> happen. Fix this by always setting `whence` on success.\n>\n\n> Signed-off-by: Patrick Steinhardt \n> ---\n> object-file.c | 19 +++++++++++++++----\n> 1 file changed, 15 insertions(+), 4 deletions(-)\n>\n> diff --git a/object-file.c b/object-file.c\n> index 6280e42f34..d566df427a 100644\n> --- a/object-file.c\n> +++ b/object-file.c\n> @@ -439,12 +439,23 @@ int odb_source_loose_read_object_info(struct odb_source *source,\n> \t */\n> \tif (!oi || (!oi->typep && !oi->sizep && !oi->contentp)) {\n> \t\tstruct stat st;\n> -\t\tif ((!oi || !oi->disk_sizep) && (flags & OBJECT_INFO_QUICK))\n> -\t\t\treturn quick_has_loose(source->loose, oid) ? 0 : -1;\n> +\n> +\t\tif ((!oi || !oi->disk_sizep) && (flags & OBJECT_INFO_QUICK)) {\n> +\t\t\tstatus = quick_has_loose(source->loose, oid) ? 0 : -1;\n> +\t\t\tif (!status && oi)\n> +\t\t\t\toi->whence = OI_LOOSE;\n> +\t\t\treturn status;\n> +\t\t}\n> +\n> \t\tif (stat_loose_object(source->loose, oid, &st, &path) < 0)\n> \t\t\treturn -1;\n> -\t\tif (oi && oi->disk_sizep)\n> -\t\t\t*oi->disk_sizep = st.st_size;\n> +\n> +\t\tif (oi) {\n> +\t\t\tif (oi->disk_sizep)\n> +\t\t\t\t*oi->disk_sizep = st.st_size;\n> +\t\t\toi->whence = OI_LOOSE;\n> +\t\t}\n> +\n> \t\treturn 0;\n> \t}\n>\n\nThe change looks good. I'm wary of early returns independently doing the\ncleanup, wonder if it'd be better to do `status = ...; goto cleanup`\ninstead.\n"},{"id":"533203","messageId":"CAOLa=ZQ0wYjDiYYgsiR=p4rM0SCgjwhcub_j0vz5kVWhzqzMWA@mail.gmail.com","threadId":"64646","inReplyTo":"20260106-b4-pks-odb-read-object-info-improvements-v3-3-b5e02fae1fb0@pks.im","subject":"Re: [PATCH v3 3/7] packfile: extend `is_delta` field to allow for \"unknown\" state","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-01-07T10:12:04Z","receivedAt":"2026-01-07T10:12:06Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt writes:\n\n> The `struct object_info::u::packed::is_delta` field determines whether\n> or not a specific object is stored as a delta. It only stores whether or\n> not the object is stored as delta, so it is treated as a boolean value.\n>\n> This boolean is insufficient though: when reading a packed object via\n> `packfile_store_read_object_info()` we know to skip parsing the actual\n> object when the user didn't request any object-specific data. In that\n> case we won't read the object itself, but will only look up its position\n> in the packfile. Consequently, we do not know whether it is a delta or\n> not.\n>\n> This isn't really an issue right now, as the check for an empty request\n> is broken. But a subsequent commit will fix it, and once we do we will\n> have the need to also represent an \"unknown\" delta state.\n>\n> Prepare for this change by introducing a new enum that encodes the\n> object type. We don't use the \"unknown\" state just yet, but will start\n> to do so in a subsequent commit.\n>\n> Signed-off-by: Patrick Steinhardt \n> ---\n> odb.h | 7 ++++++-\n> packfile.c | 17 ++++++++++++++---\n> 2 files changed, 20 insertions(+), 4 deletions(-)\n>\n> diff --git a/odb.h b/odb.h\n> index 73b0b87ad5..afae5e5c01 100644\n> --- a/odb.h\n> +++ b/odb.h\n> @@ -343,7 +343,12 @@ struct object_info {\n> \t\tstruct {\n> \t\t\tstruct packed_git *pack;\n> \t\t\toff_t offset;\n> -\t\t\tunsigned int is_delta;\n> +\t\t\tenum packed_object_type {\n> +\t\t\t\tPACKED_OBJECT_TYPE_UNKNOWN,\n> +\t\t\t\tPACKED_OBJECT_TYPE_FULL,\n> +\t\t\t\tPACKED_OBJECT_TYPE_OFS_DELTA,\n> +\t\t\t\tPACKED_OBJECT_TYPE_REF_DELTA,\n> +\t\t\t} type;\n> \t\t} packed;\n> \t} u;\n> };\n> diff --git a/packfile.c b/packfile.c\n> index b0c6665c87..cc797b2b6a 100644\n> --- a/packfile.c\n> +++ b/packfile.c\n> @@ -2159,8 +2159,18 @@ int packfile_store_read_object_info(struct packfile_store *store,\n> \tif (oi->whence == OI_PACKED) {\n> \t\toi->u.packed.offset = e.offset;\n> \t\toi->u.packed.pack = e.p;\n> -\t\toi->u.packed.is_delta = (rtype == OBJ_REF_DELTA ||\n> -\t\t\t\t\t rtype == OBJ_OFS_DELTA);\n> +\n> +\t\tswitch (rtype) {\n> +\t\tcase OBJ_REF_DELTA:\n> +\t\t\toi->u.packed.type = PACKED_OBJECT_TYPE_REF_DELTA;\n> +\t\t\tbreak;\n> +\t\tcase OBJ_OFS_DELTA:\n> +\t\t\toi->u.packed.type = PACKED_OBJECT_TYPE_OFS_DELTA;\n> +\t\t\tbreak;\n> +\t\tdefault:\n> +\t\t\toi->u.packed.type = PACKED_OBJECT_TYPE_FULL;\n> +\t\t\tbreak;\n> +\t\t}\n> \t}\n>\n\nSo we get `rtype` from `packed_object_info()` which can return OBJ_BAD,\nbut return early in such a scenario. So overall this makes sense. I like\nthat we are now storing more and clearer information.\n\n[snip]\n"},{"id":"533205","messageId":"CAOLa=ZQujhfSP9EmqgiD7z+NxdD99cc0Tqarm1ROdwrTP0ATNA@mail.gmail.com","threadId":"64646","inReplyTo":"20260106-b4-pks-odb-read-object-info-improvements-v3-0-b5e02fae1fb0@pks.im","subject":"Re: [PATCH v3 0/7] Improvements for reading object info","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-01-07T10:17:16Z","receivedAt":"2026-01-07T10:17:18Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt writes:\n\n> Hi,\n>\n> this patch series contains various small improvements for reading object\n> info for either loose or packed objects. These improvements were split\n> out of a larger patch series where I'm about to introduce a new generic\n> `odb_for_each_object()` function.\n>\n\nI'm dropping into v3 of the series for review, as such the series looks\ngood to me.\n\nThanks!\n\n[snip]\n"},{"id":"533208","messageId":"aV5DHI04KBs4GJn-@pks.im","threadId":"64646","inReplyTo":"CAOLa=ZSNmi_Lzb=3EdWks=mMOPvfijT2659y4YtxWnUKVUOXaA@mail.gmail.com","subject":"Re: [PATCH v3 1/7] object-file: always set OI_LOOSE when reading object info","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-07T11:27:24Z","receivedAt":"2026-01-07T11:27:30Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Jan 07, 2026 at 12:50:45AM -0800, Karthik Nayak wrote:\n> Patrick Steinhardt writes:\n> > diff --git a/object-file.c b/object-file.c\n> > index 6280e42f34..d566df427a 100644\n> > --- a/object-file.c\n> > +++ b/object-file.c\n> > @@ -439,12 +439,23 @@ int odb_source_loose_read_object_info(struct odb_source *source,\n> > \t */\n> > \tif (!oi || (!oi->typep && !oi->sizep && !oi->contentp)) {\n> > \t\tstruct stat st;\n> > -\t\tif ((!oi || !oi->disk_sizep) && (flags & OBJECT_INFO_QUICK))\n> > -\t\t\treturn quick_has_loose(source->loose, oid) ? 0 : -1;\n> > +\n> > +\t\tif ((!oi || !oi->disk_sizep) && (flags & OBJECT_INFO_QUICK)) {\n> > +\t\t\tstatus = quick_has_loose(source->loose, oid) ? 0 : -1;\n> > +\t\t\tif (!status && oi)\n> > +\t\t\t\toi->whence = OI_LOOSE;\n> > +\t\t\treturn status;\n> > +\t\t}\n> > +\n> > \t\tif (stat_loose_object(source->loose, oid, &st, &path) < 0)\n> > \t\t\treturn -1;\n> > -\t\tif (oi && oi->disk_sizep)\n> > -\t\t\t*oi->disk_sizep = st.st_size;\n> > +\n> > +\t\tif (oi) {\n> > +\t\t\tif (oi->disk_sizep)\n> > +\t\t\t\t*oi->disk_sizep = st.st_size;\n> > +\t\t\toi->whence = OI_LOOSE;\n> > +\t\t}\n> > +\n> > \t\treturn 0;\n> > \t}\n> >\n> \n> The change looks good. I'm wary of early returns independently doing the\n> cleanup, wonder if it'd be better to do `status = ...; goto cleanup`\n> instead.\n\nI share that sentiment, and I was in fact having a look at what it would\ntake to have a single exit path in this function. I eventually discarded\nthe work though because it required a bunch of changes to really make\nthis whole function more readable than it currently is.\n\nBut now that you're the second one thinking this I'll probably bite the\nbullet and just do it.\n\nThanks!\n\nPatrick\n"},{"id":"533211","messageId":"20260107-b4-pks-odb-read-object-info-improvements-v4-0-b5d55c47082a@pks.im","threadId":"64646","inReplyTo":"20251218-b4-pks-odb-read-object-info-improvements-v1-0-81c8368492be@pks.im","subject":"[PATCH v4 0/7] Improvements for reading object info","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-07T13:07:59Z","receivedAt":"2026-01-07T13:08:16Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nthis patch series contains various small improvements for reading object\ninfo for either loose or packed objects. These improvements were split\nout of a larger patch series where I'm about to introduce a new generic\n`odb_for_each_object()` function.\n\nChanges in v4:\n - Extend the fix for OI_LOOSE and refactor the whole function to have\n a single exit path as proposed by Karthik. This results in a lot\n more changes, but makes the function way easier to reason about\n going forward.\n - Link to v3: https://lore.kernel.org/r/20260106-b4-pks-odb-read-object-info-improvements-v3-0-b5e02fae1fb0@pks.im\n\nChanges in v3:\n - Fix a commit message typo.\n - Fix a function comment missing some words.\n - Link to v2: https://lore.kernel.org/r/20251218-b4-pks-odb-read-object-info-improvements-v2-0-62e3e49072bc@pks.im\n\nChanges in v2:\n - Rebase the series on top of master with jc/object-read-stream-fix\n merged into it. I've also evicted the patch that fixes the same\n underlying issue.\n - Improve the commit message that drops OI_DBCACHED to explain why\n this is a safe refactoring.\n - Link to v1: https://lore.kernel.org/r/20251218-b4-pks-odb-read-object-info-improvements-v1-0-81c8368492be@pks.im\n\nThanks!\n\nPatrick\n\n---\nPatrick Steinhardt (7):\n object-file: always set OI_LOOSE when reading object info\n packfile: always declare object info to be OI_PACKED\n packfile: extend `is_delta` field to allow for \"unknown\" state\n packfile: always populate pack-specific info when reading object info\n packfile: disentangle return value of `packed_object_info()`\n packfile: skip unpacking object header for disk size requests\n packfile: drop repository parameter from `packed_object_info()`\n\n builtin/cat-file.c | 3 +-\n builtin/pack-objects.c | 4 +-\n commit-graph.c | 2 +-\n object-file.c | 115 ++++++++++++++++++++++++++++++-------------------\n odb.h | 8 +++-\n pack-bitmap.c | 3 +-\n packfile.c | 61 +++++++++++++++-----------\n packfile.h | 7 ++-\n 8 files changed, 124 insertions(+), 79 deletions(-)\n\nRange-diff versus v3:\n\n1: 5c67d9abe8 < -: ---------- object-file: always set OI_LOOSE when reading object info\n-: ---------- > 1: 7708b50c2a object-file: always set OI_LOOSE when reading object info\n2: 8b106feb28 = 2: a96ac5b351 packfile: always declare object info to be OI_PACKED\n3: adbd3e5ae5 = 3: 8e3193a06e packfile: extend `is_delta` field to allow for \"unknown\" state\n4: 218c64c9a5 = 4: e718161286 packfile: always populate pack-specific info when reading object info\n5: dcae7be795 = 5: 217bec7e3b packfile: disentangle return value of `packed_object_info()`\n6: beac514592 = 6: 2aaacfd639 packfile: skip unpacking object header for disk size requests\n7: dacccf1cb4 = 7: a9e37b7e00 packfile: drop repository parameter from `packed_object_info()`\n\n---\nbase-commit: 7df68b50e49b6a1b576abb19b2e5d457749bc28b\nchange-id: 20251215-b4-pks-odb-read-object-info-improvements-0e031ef827d2\n\n"},{"id":"533212","messageId":"20260107-b4-pks-odb-read-object-info-improvements-v4-1-b5d55c47082a@pks.im","threadId":"64646","inReplyTo":"20260107-b4-pks-odb-read-object-info-improvements-v4-0-b5d55c47082a@pks.im","subject":"[PATCH v4 1/7] object-file: always set OI_LOOSE when reading object info","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-07T13:08:00Z","receivedAt":"2026-01-07T13:08:17Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"There are some early returns in `odb_source_loose_read_object_info()`\nin cases where we don't have to open the loose object. These return\npaths do not set `struct object_info::whence` to `OI_LOOSE` though, so\nit becomes impossible for the caller to tell the format of such an\nobject.\n\nThe root cause of this really is that we have so many different return\npaths in the function. As a consequence, it's harder than necessary to\nmake sure that all successful exit paths sot up the `whence` field as\nexpected.\n\nAddress this by refactoring the function to have a single exit path.\nLike this, we can trivially set up the `whence` field when we exit\nsuccessfully from the function.\n\nNote that we also:\n\n - Rename `status` to `ret` to match our usual coding style, but also\n to show that the old `status` variable is now always getting the\n expected value. Furthermore, the value is not initialized anymore,\n which has the consequence that most compilers will warn for exit\n paths where we forgot to set it.\n\n - Move the setup of scratch pointers closer to `parse_loose_header()`\n to show where it's needed.\n\n - Guard a couple of variables on cleanup so that they only get\n released in case they have been set up.\n\n - Reset `oi->delta_base_oid` towards the end of the function, together\n with all the other object info pointers.\n\nOverall, all these changes result in a diff that is somewhat hard to\nread. But the end result is significantly easier to read and reason\nabout, so I'd argue this one-time churn is worth it.\n\nSigned-off-by: Patrick Steinhardt \n---\n object-file.c | 115 ++++++++++++++++++++++++++++++++++++----------------------\n 1 file changed, 71 insertions(+), 44 deletions(-)\n\ndiff --git a/object-file.c b/object-file.c\nindex 6280e42f34..e7e4c3348f 100644\n--- a/object-file.c\n+++ b/object-file.c\n@@ -416,19 +416,16 @@ int odb_source_loose_read_object_info(struct odb_source *source,\n \t\t\t\t const struct object_id *oid,\n \t\t\t\t struct object_info *oi, int flags)\n {\n-\tint status = 0;\n+\tint ret;\n \tint fd;\n \tunsigned long mapsize;\n \tconst char *path;\n-\tvoid *map;\n-\tgit_zstream stream;\n+\tvoid *map = NULL;\n+\tgit_zstream stream, *stream_to_end = NULL;\n \tchar hdr[MAX_HEADER_LEN];\n \tunsigned long size_scratch;\n \tenum object_type type_scratch;\n \n-\tif (oi && oi->delta_base_oid)\n-\t\toidclr(oi->delta_base_oid, source->odb->repo->hash_algo);\n-\n \t/*\n \t * If we don't care about type or size, then we don't\n \t * need to look inside the object at all. Note that we\n@@ -439,71 +436,101 @@ int odb_source_loose_read_object_info(struct odb_source *source,\n \t */\n \tif (!oi || (!oi->typep && !oi->sizep && !oi->contentp)) {\n \t\tstruct stat st;\n-\t\tif ((!oi || !oi->disk_sizep) && (flags & OBJECT_INFO_QUICK))\n-\t\t\treturn quick_has_loose(source->loose, oid) ? 0 : -1;\n-\t\tif (stat_loose_object(source->loose, oid, &st, &path) < 0)\n-\t\t\treturn -1;\n+\n+\t\tif ((!oi || !oi->disk_sizep) && (flags & OBJECT_INFO_QUICK)) {\n+\t\t\tret = quick_has_loose(source->loose, oid) ? 0 : -1;\n+\t\t\tgoto out;\n+\t\t}\n+\n+\t\tif (stat_loose_object(source->loose, oid, &st, &path) < 0) {\n+\t\t\tret = -1;\n+\t\t\tgoto out;\n+\t\t}\n+\n \t\tif (oi && oi->disk_sizep)\n \t\t\t*oi->disk_sizep = st.st_size;\n-\t\treturn 0;\n+\n+\t\tret = 0;\n+\t\tgoto out;\n \t}\n \n \tfd = open_loose_object(source->loose, oid, &path);\n \tif (fd < 0) {\n \t\tif (errno != ENOENT)\n \t\t\terror_errno(_(\"unable to open loose object %s\"), oid_to_hex(oid));\n-\t\treturn -1;\n+\t\tret = -1;\n+\t\tgoto out;\n \t}\n-\tmap = map_fd(fd, path, &mapsize);\n-\tif (!map)\n-\t\treturn -1;\n \n-\tif (!oi->sizep)\n-\t\toi->sizep = &size_scratch;\n-\tif (!oi->typep)\n-\t\toi->typep = &type_scratch;\n+\tmap = map_fd(fd, path, &mapsize);\n+\tif (!map) {\n+\t\tret = -1;\n+\t\tgoto out;\n+\t}\n \n \tif (oi->disk_sizep)\n \t\t*oi->disk_sizep = mapsize;\n \n+\tstream_to_end = &stream;\n+\n \tswitch (unpack_loose_header(&stream, map, mapsize, hdr, sizeof(hdr))) {\n \tcase ULHR_OK:\n-\t\tif (parse_loose_header(hdr, oi) < 0)\n-\t\t\tstatus = error(_(\"unable to parse %s header\"), oid_to_hex(oid));\n-\t\telse if (*oi->typep < 0)\n+\t\tif (!oi->sizep)\n+\t\t\toi->sizep = &size_scratch;\n+\t\tif (!oi->typep)\n+\t\t\toi->typep = &type_scratch;\n+\n+\t\tif (parse_loose_header(hdr, oi) < 0) {\n+\t\t\tret = error(_(\"unable to parse %s header\"), oid_to_hex(oid));\n+\t\t\tgoto corrupt;\n+\t\t}\n+\n+\t\tif (*oi->typep < 0)\n \t\t\tdie(_(\"invalid object type\"));\n \n-\t\tif (!oi->contentp)\n-\t\t\tbreak;\n-\t\t*oi->contentp = unpack_loose_rest(&stream, hdr, *oi->sizep, oid);\n-\t\tif (*oi->contentp)\n-\t\t\tgoto cleanup;\n+\t\tif (oi->contentp) {\n+\t\t\t*oi->contentp = unpack_loose_rest(&stream, hdr, *oi->sizep, oid);\n+\t\t\tif (!*oi->contentp) {\n+\t\t\t\tret = -1;\n+\t\t\t\tgoto corrupt;\n+\t\t\t}\n+\t\t}\n \n-\t\tstatus = -1;\n \t\tbreak;\n \tcase ULHR_BAD:\n-\t\tstatus = error(_(\"unable to unpack %s header\"),\n-\t\t\t oid_to_hex(oid));\n-\t\tbreak;\n+\t\tret = error(_(\"unable to unpack %s header\"),\n+\t\t\t oid_to_hex(oid));\n+\t\tgoto corrupt;\n \tcase ULHR_TOO_LONG:\n-\t\tstatus = error(_(\"header for %s too long, exceeds %d bytes\"),\n-\t\t\t oid_to_hex(oid), MAX_HEADER_LEN);\n-\t\tbreak;\n+\t\tret = error(_(\"header for %s too long, exceeds %d bytes\"),\n+\t\t\t oid_to_hex(oid), MAX_HEADER_LEN);\n+\t\tgoto corrupt;\n \t}\n \n-\tif (status && (flags & OBJECT_INFO_DIE_IF_CORRUPT))\n+\tret = 0;\n+\n+corrupt:\n+\tif (ret && (flags & OBJECT_INFO_DIE_IF_CORRUPT))\n \t\tdie(_(\"loose object %s (stored in %s) is corrupt\"),\n \t\t oid_to_hex(oid), path);\n \n-cleanup:\n-\tgit_inflate_end(&stream);\n-\tmunmap(map, mapsize);\n-\tif (oi->sizep == &size_scratch)\n-\t\toi->sizep = NULL;\n-\tif (oi->typep == &type_scratch)\n-\t\toi->typep = NULL;\n-\toi->whence = OI_LOOSE;\n-\treturn status;\n+out:\n+\tif (stream_to_end)\n+\t\tgit_inflate_end(stream_to_end);\n+\tif (map)\n+\t\tmunmap(map, mapsize);\n+\tif (oi) {\n+\t\tif (oi->sizep == &size_scratch)\n+\t\t\toi->sizep = NULL;\n+\t\tif (oi->typep == &type_scratch)\n+\t\t\toi->typep = NULL;\n+\t\tif (oi->delta_base_oid)\n+\t\t\toidclr(oi->delta_base_oid, source->odb->repo->hash_algo);\n+\t\tif (!ret)\n+\t\t\toi->whence = OI_LOOSE;\n+\t}\n+\n+\treturn ret;\n }\n \n static void hash_object_body(const struct git_hash_algo *algo, struct git_hash_ctx *c,\n\n-- \n2.52.0.542.g9473a8513b.dirty\n\n"},{"id":"533213","messageId":"20260107-b4-pks-odb-read-object-info-improvements-v4-2-b5d55c47082a@pks.im","threadId":"64646","inReplyTo":"20260107-b4-pks-odb-read-object-info-improvements-v4-0-b5d55c47082a@pks.im","subject":"[PATCH v4 2/7] packfile: always declare object info to be OI_PACKED","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-07T13:08:01Z","receivedAt":"2026-01-07T13:08:20Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When reading object info via a packfile we yield one of two types:\n\n - The object can either be OI_PACKED, which is what a caller would\n typically expect.\n\n - Or it can be OI_DBCACHED if it is stored in the delta base cache.\n\nThe latter really is an implementation detail though, and callers\ntypically don't care at all about the difference. Furthermore, the\ninformation whether or not it is part of the delta base cache can\nalready be derived via the `is_delta` field, so the fact that we discern\nbetween OI_PACKED and OI_DBCACHED only further complicates the\ninterface.\n\nThere aren't all that many callers that care about the `whence` field in\nthe first place. In fact, there's only three:\n\n - `packfile_store_read_object_info()` checks for `whence == OI_PACKED`\n and then populates the packfile information of the object info\n structure. We now start to do this also for deltified objects, which\n gives its callers strictly more information.\n\n - `repack_local_links()` wants to determine whether the object is part\n of a promisor pack and checks for `whence == OI_PACKED`. If so, it\n verifies that the packfile is a promisor pack. It's arguably wrong\n to declare that an object is not part of a promisor pack only\n because it is stored in the delta base cache.\n\n - `is_not_in_promisor_pack_obj()` does the same, but checks that a\n specific object is _not_ part of a promisor pack. The same reasoning\n as above applies.\n\nDrop the OI_DBCACHED enum completely. None of the callers seem to care\nabout the distinction.\n\nSigned-off-by: Patrick Steinhardt \n---\n odb.h | 1 -\n packfile.c | 3 +--\n 2 files changed, 1 insertion(+), 3 deletions(-)\n\ndiff --git a/odb.h b/odb.h\nindex 014cd9585a..73b0b87ad5 100644\n--- a/odb.h\n+++ b/odb.h\n@@ -330,7 +330,6 @@ struct object_info {\n \t\tOI_CACHED,\n \t\tOI_LOOSE,\n \t\tOI_PACKED,\n-\t\tOI_DBCACHED\n \t} whence;\n \tunion {\n \t\t/*\ndiff --git a/packfile.c b/packfile.c\nindex 08a0863fc3..b0c6665c87 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -1656,8 +1656,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \t\t\toidclr(oi->delta_base_oid, p->repo->hash_algo);\n \t}\n \n-\toi->whence = in_delta_base_cache(p, obj_offset) ? OI_DBCACHED :\n-\t\t\t\t\t\t\t OI_PACKED;\n+\toi->whence = OI_PACKED;\n \n out:\n \tunuse_pack(&w_curs);\n\n-- \n2.52.0.542.g9473a8513b.dirty\n\n"},{"id":"533214","messageId":"20260107-b4-pks-odb-read-object-info-improvements-v4-3-b5d55c47082a@pks.im","threadId":"64646","inReplyTo":"20260107-b4-pks-odb-read-object-info-improvements-v4-0-b5d55c47082a@pks.im","subject":"[PATCH v4 3/7] packfile: extend `is_delta` field to allow for \"unknown\" state","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-07T13:08:02Z","receivedAt":"2026-01-07T13:08:22Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The `struct object_info::u::packed::is_delta` field determines whether\nor not a specific object is stored as a delta. It only stores whether or\nnot the object is stored as delta, so it is treated as a boolean value.\n\nThis boolean is insufficient though: when reading a packed object via\n`packfile_store_read_object_info()` we know to skip parsing the actual\nobject when the user didn't request any object-specific data. In that\ncase we won't read the object itself, but will only look up its position\nin the packfile. Consequently, we do not know whether it is a delta or\nnot.\n\nThis isn't really an issue right now, as the check for an empty request\nis broken. But a subsequent commit will fix it, and once we do we will\nhave the need to also represent an \"unknown\" delta state.\n\nPrepare for this change by introducing a new enum that encodes the\nobject type. We don't use the \"unknown\" state just yet, but will start\nto do so in a subsequent commit.\n\nSigned-off-by: Patrick Steinhardt \n---\n odb.h | 7 ++++++-\n packfile.c | 17 ++++++++++++++---\n 2 files changed, 20 insertions(+), 4 deletions(-)\n\ndiff --git a/odb.h b/odb.h\nindex 73b0b87ad5..afae5e5c01 100644\n--- a/odb.h\n+++ b/odb.h\n@@ -343,7 +343,12 @@ struct object_info {\n \t\tstruct {\n \t\t\tstruct packed_git *pack;\n \t\t\toff_t offset;\n-\t\t\tunsigned int is_delta;\n+\t\t\tenum packed_object_type {\n+\t\t\t\tPACKED_OBJECT_TYPE_UNKNOWN,\n+\t\t\t\tPACKED_OBJECT_TYPE_FULL,\n+\t\t\t\tPACKED_OBJECT_TYPE_OFS_DELTA,\n+\t\t\t\tPACKED_OBJECT_TYPE_REF_DELTA,\n+\t\t\t} type;\n \t\t} packed;\n \t} u;\n };\ndiff --git a/packfile.c b/packfile.c\nindex b0c6665c87..cc797b2b6a 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -2159,8 +2159,18 @@ int packfile_store_read_object_info(struct packfile_store *store,\n \tif (oi->whence == OI_PACKED) {\n \t\toi->u.packed.offset = e.offset;\n \t\toi->u.packed.pack = e.p;\n-\t\toi->u.packed.is_delta = (rtype == OBJ_REF_DELTA ||\n-\t\t\t\t\t rtype == OBJ_OFS_DELTA);\n+\n+\t\tswitch (rtype) {\n+\t\tcase OBJ_REF_DELTA:\n+\t\t\toi->u.packed.type = PACKED_OBJECT_TYPE_REF_DELTA;\n+\t\t\tbreak;\n+\t\tcase OBJ_OFS_DELTA:\n+\t\t\toi->u.packed.type = PACKED_OBJECT_TYPE_OFS_DELTA;\n+\t\t\tbreak;\n+\t\tdefault:\n+\t\t\toi->u.packed.type = PACKED_OBJECT_TYPE_FULL;\n+\t\t\tbreak;\n+\t\t}\n \t}\n \n \treturn 0;\n@@ -2531,7 +2541,8 @@ int packfile_store_read_object_stream(struct odb_read_stream **out,\n \toi.sizep = &size;\n \n \tif (packfile_store_read_object_info(store, oid, &oi, 0) ||\n-\t oi.u.packed.is_delta ||\n+\t oi.u.packed.type == PACKED_OBJECT_TYPE_REF_DELTA ||\n+\t oi.u.packed.type == PACKED_OBJECT_TYPE_OFS_DELTA ||\n \t repo_settings_get_big_file_threshold(store->odb->repo) >= size)\n \t\treturn -1;\n \n\n-- \n2.52.0.542.g9473a8513b.dirty\n\n"},{"id":"533215","messageId":"20260107-b4-pks-odb-read-object-info-improvements-v4-4-b5d55c47082a@pks.im","threadId":"64646","inReplyTo":"20260107-b4-pks-odb-read-object-info-improvements-v4-0-b5d55c47082a@pks.im","subject":"[PATCH v4 4/7] packfile: always populate pack-specific info when reading object info","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-07T13:08:03Z","receivedAt":"2026-01-07T13:08:25Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When reading object information via `packed_object_info()` we may not\npopulate the object info's packfile-specific fields. This leads to\ninconsistent object info depending on whether the info was populated via\n`packfile_store_read_object_info()` or `packed_object_info()`.\n\nFix this inconsistency so that we can always assume the pack info to be\npopulated when reading object info from a pack.\n\nSigned-off-by: Patrick Steinhardt \n---\n packfile.c | 31 ++++++++++++++-----------------\n 1 file changed, 14 insertions(+), 17 deletions(-)\n\ndiff --git a/packfile.c b/packfile.c\nindex cc797b2b6a..f7c33a2f77 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -1657,6 +1657,20 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \t}\n \n \toi->whence = OI_PACKED;\n+\toi->u.packed.offset = obj_offset;\n+\toi->u.packed.pack = p;\n+\n+\tswitch (type) {\n+\tcase OBJ_REF_DELTA:\n+\t\toi->u.packed.type = PACKED_OBJECT_TYPE_REF_DELTA;\n+\t\tbreak;\n+\tcase OBJ_OFS_DELTA:\n+\t\toi->u.packed.type = PACKED_OBJECT_TYPE_OFS_DELTA;\n+\t\tbreak;\n+\tdefault:\n+\t\toi->u.packed.type = PACKED_OBJECT_TYPE_FULL;\n+\t\tbreak;\n+\t}\n \n out:\n \tunuse_pack(&w_curs);\n@@ -2156,23 +2170,6 @@ int packfile_store_read_object_info(struct packfile_store *store,\n \t\treturn -1;\n \t}\n \n-\tif (oi->whence == OI_PACKED) {\n-\t\toi->u.packed.offset = e.offset;\n-\t\toi->u.packed.pack = e.p;\n-\n-\t\tswitch (rtype) {\n-\t\tcase OBJ_REF_DELTA:\n-\t\t\toi->u.packed.type = PACKED_OBJECT_TYPE_REF_DELTA;\n-\t\t\tbreak;\n-\t\tcase OBJ_OFS_DELTA:\n-\t\t\toi->u.packed.type = PACKED_OBJECT_TYPE_OFS_DELTA;\n-\t\t\tbreak;\n-\t\tdefault:\n-\t\t\toi->u.packed.type = PACKED_OBJECT_TYPE_FULL;\n-\t\t\tbreak;\n-\t\t}\n-\t}\n-\n \treturn 0;\n }\n \n\n-- \n2.52.0.542.g9473a8513b.dirty\n\n"},{"id":"533216","messageId":"20260107-b4-pks-odb-read-object-info-improvements-v4-5-b5d55c47082a@pks.im","threadId":"64646","inReplyTo":"20260107-b4-pks-odb-read-object-info-improvements-v4-0-b5d55c47082a@pks.im","subject":"[PATCH v4 5/7] packfile: disentangle return value of `packed_object_info()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-07T13:08:04Z","receivedAt":"2026-01-07T13:08:28Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The `packed_object_info()` function returns the type of the packed\nobject. While we use an `enum object_type` to store the return value,\nthis type is not to be confused with the actual object type. It _may_\ncontain the object type, but it may just as well encode that the given\npacked object is stored as a delta.\n\nWe have removed the only caller that relied on this returned object type\nin the preceding commit, so let's simplify semantics and return either 0\non success or a negative error code otherwise.\n\nThis unblocks a small optimization where we can skip reading the object\ntype altogether.\n\nSigned-off-by: Patrick Steinhardt \n---\n packfile.c | 21 ++++++++++++---------\n packfile.h | 4 ++++\n 2 files changed, 16 insertions(+), 9 deletions(-)\n\ndiff --git a/packfile.c b/packfile.c\nindex f7c33a2f77..8c6ef45a67 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -1587,6 +1587,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \tunsigned long size;\n \toff_t curpos = obj_offset;\n \tenum object_type type;\n+\tint ret;\n \n \t/*\n \t * We always get the representation type, but only convert it to\n@@ -1607,12 +1608,12 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \t\t\toff_t base_offset = get_delta_base(p, &w_curs, &tmp_pos,\n \t\t\t\t\t\t\t type, obj_offset);\n \t\t\tif (!base_offset) {\n-\t\t\t\ttype = OBJ_BAD;\n+\t\t\t\tret = -1;\n \t\t\t\tgoto out;\n \t\t\t}\n \t\t\t*oi->sizep = get_size_from_delta(p, &w_curs, tmp_pos);\n \t\t\tif (*oi->sizep == 0) {\n-\t\t\t\ttype = OBJ_BAD;\n+\t\t\t\tret = -1;\n \t\t\t\tgoto out;\n \t\t\t}\n \t\t} else {\n@@ -1625,7 +1626,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \t\tif (offset_to_pack_pos(p, obj_offset, &pos) < 0) {\n \t\t\terror(\"could not find object at offset %\"PRIuMAX\" \"\n \t\t\t \"in pack %s\", (uintmax_t)obj_offset, p->pack_name);\n-\t\t\ttype = OBJ_BAD;\n+\t\t\tret = -1;\n \t\t\tgoto out;\n \t\t}\n \n@@ -1639,7 +1640,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \t\tif (oi->typep)\n \t\t\t*oi->typep = ptot;\n \t\tif (ptot < 0) {\n-\t\t\ttype = OBJ_BAD;\n+\t\t\tret = -1;\n \t\t\tgoto out;\n \t\t}\n \t}\n@@ -1649,7 +1650,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \t\t\tif (get_delta_base_oid(p, &w_curs, curpos,\n \t\t\t\t\t oi->delta_base_oid,\n \t\t\t\t\t type, obj_offset) < 0) {\n-\t\t\t\ttype = OBJ_BAD;\n+\t\t\t\tret = -1;\n \t\t\t\tgoto out;\n \t\t\t}\n \t\t} else\n@@ -1672,9 +1673,11 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \t\tbreak;\n \t}\n \n+\tret = 0;\n+\n out:\n \tunuse_pack(&w_curs);\n-\treturn type;\n+\treturn ret;\n }\n \n static void *unpack_compressed_entry(struct packed_git *p,\n@@ -2152,7 +2155,7 @@ int packfile_store_read_object_info(struct packfile_store *store,\n \t\t\t\t unsigned flags UNUSED)\n {\n \tstruct pack_entry e;\n-\tint rtype;\n+\tint ret;\n \n \tif (!find_pack_entry(store->odb->repo, oid, &e))\n \t\treturn 1;\n@@ -2164,8 +2167,8 @@ int packfile_store_read_object_info(struct packfile_store *store,\n \tif (!oi)\n \t\treturn 0;\n \n-\trtype = packed_object_info(store->odb->repo, e.p, e.offset, oi);\n-\tif (rtype < 0) {\n+\tret = packed_object_info(store->odb->repo, e.p, e.offset, oi);\n+\tif (ret < 0) {\n \t\tmark_bad_packed_object(e.p, oid);\n \t\treturn -1;\n \t}\ndiff --git a/packfile.h b/packfile.h\nindex 59d162a3f4..d7cce582af 100644\n--- a/packfile.h\n+++ b/packfile.h\n@@ -378,6 +378,10 @@ void release_pack_memory(size_t);\n /* global flag to enable extra checks when accessing packed objects */\n extern int do_check_packed_object_crc;\n \n+/*\n+ * Look up the object info for a specific offset in the packfile.\n+ * Returns zero on success, a negative error code otherwise.\n+ */\n int packed_object_info(struct repository *r,\n \t\t struct packed_git *pack,\n \t\t off_t offset, struct object_info *);\n\n-- \n2.52.0.542.g9473a8513b.dirty\n\n"},{"id":"533217","messageId":"20260107-b4-pks-odb-read-object-info-improvements-v4-6-b5d55c47082a@pks.im","threadId":"64646","inReplyTo":"20260107-b4-pks-odb-read-object-info-improvements-v4-0-b5d55c47082a@pks.im","subject":"[PATCH v4 6/7] packfile: skip unpacking object header for disk size requests","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-07T13:08:05Z","receivedAt":"2026-01-07T13:08:30Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"While most of the object info requests for a packed object require us to\nunpack its headers, reading its disk size doesn't. We still unpack the\nobject header in that case though, which is unnecessary work.\n\nSkip reading the header if only the disk size is requested. This leads\nto a small speedup when reading disk size, only. The following benchmark\nwas done in the Git repository:\n\n Benchmark 1: ./git rev-list --disk-usage HEAD (rev = HEAD~)\n Time (mean ± σ): 105.2 ms ± 0.6 ms [User: 91.4 ms, System: 13.3 ms]\n Range (min … max): 103.7 ms … 106.0 ms 27 runs\n\n Benchmark 2: ./git rev-list --disk-usage HEAD (rev = HEAD)\n Time (mean ± σ): 96.7 ms ± 0.4 ms [User: 86.2 ms, System: 10.0 ms]\n Range (min … max): 96.2 ms … 98.1 ms 30 runs\n\n Summary\n ./git rev-list --disk-usage HEAD (rev = HEAD) ran\n 1.09 ± 0.01 times faster than ./git rev-list --disk-usage HEAD (rev = HEAD~)\n\nSigned-off-by: Patrick Steinhardt \n---\n packfile.c | 7 +++++--\n 1 file changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/packfile.c b/packfile.c\nindex 8c6ef45a67..a2ba237ce7 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -1586,7 +1586,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \tstruct pack_window *w_curs = NULL;\n \tunsigned long size;\n \toff_t curpos = obj_offset;\n-\tenum object_type type;\n+\tenum object_type type = OBJ_NONE;\n \tint ret;\n \n \t/*\n@@ -1598,7 +1598,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \t\t\t\t\t\t &type);\n \t\tif (!*oi->contentp)\n \t\t\ttype = OBJ_BAD;\n-\t} else {\n+\t} else if (oi->sizep || oi->typep || oi->delta_base_oid) {\n \t\ttype = unpack_object_header(p, &w_curs, &curpos, &size);\n \t}\n \n@@ -1662,6 +1662,9 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \toi->u.packed.pack = p;\n \n \tswitch (type) {\n+\tcase OBJ_NONE:\n+\t\toi->u.packed.type = PACKED_OBJECT_TYPE_UNKNOWN;\n+\t\tbreak;\n \tcase OBJ_REF_DELTA:\n \t\toi->u.packed.type = PACKED_OBJECT_TYPE_REF_DELTA;\n \t\tbreak;\n\n-- \n2.52.0.542.g9473a8513b.dirty\n\n"},{"id":"533218","messageId":"20260107-b4-pks-odb-read-object-info-improvements-v4-7-b5d55c47082a@pks.im","threadId":"64646","inReplyTo":"20260107-b4-pks-odb-read-object-info-improvements-v4-0-b5d55c47082a@pks.im","subject":"[PATCH v4 7/7] packfile: drop repository parameter from `packed_object_info()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-07T13:08:06Z","receivedAt":"2026-01-07T13:08:33Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The function `packed_object_info()` takes a packfile and offset and\nreturns the object info for the corresponding object. Despite these two\nparameters though it also takes a repository pointer. This is redundant\ninformation though, as `struct packed_git` already has a repository\npointer that is always populated.\n\nDrop the redundant parameter.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/cat-file.c | 3 +--\n builtin/pack-objects.c | 4 ++--\n commit-graph.c | 2 +-\n pack-bitmap.c | 3 +--\n packfile.c | 8 ++++----\n packfile.h | 3 +--\n 6 files changed, 10 insertions(+), 13 deletions(-)\n\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex 505ddaa12f..2ad712e9f8 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -487,8 +487,7 @@ static void batch_object_write(const char *obj_name,\n \t\t\tdata->info.sizep = &data->size;\n \n \t\tif (pack)\n-\t\t\tret = packed_object_info(the_repository, pack,\n-\t\t\t\t\t\t offset, &data->info);\n+\t\t\tret = packed_object_info(pack, offset, &data->info);\n \t\telse\n \t\t\tret = odb_read_object_info_extended(the_repository->objects,\n \t\t\t\t\t\t\t &data->oid, &data->info,\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 1ce8d6ee21..85762f8c4f 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -2411,7 +2411,7 @@ static void drop_reused_delta(struct object_entry *entry)\n \n \toi.sizep = &size;\n \toi.typep = &type;\n-\tif (packed_object_info(the_repository, IN_PACK(entry), entry->in_pack_offset, &oi) < 0) {\n+\tif (packed_object_info(IN_PACK(entry), entry->in_pack_offset, &oi) < 0) {\n \t\t/*\n \t\t * We failed to get the info from this pack for some reason;\n \t\t * fall back to odb_read_object_info, which may find another copy.\n@@ -3748,7 +3748,7 @@ static int add_object_entry_from_pack(const struct object_id *oid,\n \t\tstruct object_info oi = OBJECT_INFO_INIT;\n \n \t\toi.typep = &type;\n-\t\tif (packed_object_info(the_repository, p, ofs, &oi) < 0) {\n+\t\tif (packed_object_info(p, ofs, &oi) < 0) {\n \t\t\tdie(_(\"could not get type of object %s in pack %s\"),\n \t\t\t oid_to_hex(oid), p->pack_name);\n \t\t} else if (type == OBJ_COMMIT) {\ndiff --git a/commit-graph.c b/commit-graph.c\nindex 80be2ff2c3..f572670bd0 100644\n--- a/commit-graph.c\n+++ b/commit-graph.c\n@@ -1499,7 +1499,7 @@ static int add_packed_commits(const struct object_id *oid,\n \t\tdisplay_progress(ctx->progress, ++ctx->progress_done);\n \n \toi.typep = &type;\n-\tif (packed_object_info(ctx->r, pack, offset, &oi) < 0)\n+\tif (packed_object_info(pack, offset, &oi) < 0)\n \t\tdie(_(\"unable to get type of object %s\"), oid_to_hex(oid));\n \n \tif (type != OBJ_COMMIT)\ndiff --git a/pack-bitmap.c b/pack-bitmap.c\nindex 8ca79725b1..972203f12b 100644\n--- a/pack-bitmap.c\n+++ b/pack-bitmap.c\n@@ -1876,8 +1876,7 @@ static unsigned long get_size_by_pos(struct bitmap_index *bitmap_git,\n \t\t\tofs = pack_pos_to_offset(pack, pos);\n \t\t}\n \n-\t\tif (packed_object_info(bitmap_repo(bitmap_git), pack, ofs,\n-\t\t\t\t &oi) < 0) {\n+\t\tif (packed_object_info(pack, ofs, &oi) < 0) {\n \t\t\tstruct object_id oid;\n \t\t\tnth_bitmap_object_oid(bitmap_git, &oid,\n \t\t\t\t\t pack_pos_to_index(pack, pos));\ndiff --git a/packfile.c b/packfile.c\nindex a2ba237ce7..39899aec49 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -1580,7 +1580,7 @@ static void add_delta_base_cache(struct packed_git *p, off_t base_offset,\n \thashmap_add(&delta_base_cache, &ent->ent);\n }\n \n-int packed_object_info(struct repository *r, struct packed_git *p,\n+int packed_object_info(struct packed_git *p,\n \t\t off_t obj_offset, struct object_info *oi)\n {\n \tstruct pack_window *w_curs = NULL;\n@@ -1594,7 +1594,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \t * a \"real\" type later if the caller is interested.\n \t */\n \tif (oi->contentp) {\n-\t\t*oi->contentp = cache_or_unpack_entry(r, p, obj_offset, oi->sizep,\n+\t\t*oi->contentp = cache_or_unpack_entry(p->repo, p, obj_offset, oi->sizep,\n \t\t\t\t\t\t &type);\n \t\tif (!*oi->contentp)\n \t\t\ttype = OBJ_BAD;\n@@ -1635,7 +1635,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \n \tif (oi->typep) {\n \t\tenum object_type ptot;\n-\t\tptot = packed_to_object_type(r, p, obj_offset,\n+\t\tptot = packed_to_object_type(p->repo, p, obj_offset,\n \t\t\t\t\t type, &w_curs, curpos);\n \t\tif (oi->typep)\n \t\t\t*oi->typep = ptot;\n@@ -2170,7 +2170,7 @@ int packfile_store_read_object_info(struct packfile_store *store,\n \tif (!oi)\n \t\treturn 0;\n \n-\tret = packed_object_info(store->odb->repo, e.p, e.offset, oi);\n+\tret = packed_object_info(e.p, e.offset, oi);\n \tif (ret < 0) {\n \t\tmark_bad_packed_object(e.p, oid);\n \t\treturn -1;\ndiff --git a/packfile.h b/packfile.h\nindex d7cce582af..33fed26362 100644\n--- a/packfile.h\n+++ b/packfile.h\n@@ -382,8 +382,7 @@ extern int do_check_packed_object_crc;\n * Look up the object info for a specific offset in the packfile.\n * Returns zero on success, a negative error code otherwise.\n */\n-int packed_object_info(struct repository *r,\n-\t\t struct packed_git *pack,\n+int packed_object_info(struct packed_git *pack,\n \t\t off_t offset, struct object_info *);\n \n void mark_bad_packed_object(struct packed_git *, const struct object_id *);\n\n-- \n2.52.0.542.g9473a8513b.dirty\n\n"},{"id":"533276","messageId":"CAOLa=ZSWKzOzN103CyuVstnaiviFDm8KB6mQOQLyyExy4TiUzA@mail.gmail.com","threadId":"64646","inReplyTo":"20260107-b4-pks-odb-read-object-info-improvements-v4-1-b5d55c47082a@pks.im","subject":"Re: [PATCH v4 1/7] object-file: always set OI_LOOSE when reading object info","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-01-08T09:30:16Z","receivedAt":"2026-01-08T09:30:21Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt writes:\n\n> There are some early returns in `odb_source_loose_read_object_info()`\n> in cases where we don't have to open the loose object. These return\n> paths do not set `struct object_info::whence` to `OI_LOOSE` though, so\n> it becomes impossible for the caller to tell the format of such an\n> object.\n>\n> The root cause of this really is that we have so many different return\n> paths in the function. As a consequence, it's harder than necessary to\n> make sure that all successful exit paths sot up the `whence` field as\n> expected.\n>\n> Address this by refactoring the function to have a single exit path.\n> Like this, we can trivially set up the `whence` field when we exit\n> successfully from the function.\n>\n> Note that we also:\n>\n> - Rename `status` to `ret` to match our usual coding style, but also\n> to show that the old `status` variable is now always getting the\n> expected value. Furthermore, the value is not initialized anymore,\n> which has the consequence that most compilers will warn for exit\n> paths where we forgot to set it.\n>\n> - Move the setup of scratch pointers closer to `parse_loose_header()`\n> to show where it's needed.\n>\n> - Guard a couple of variables on cleanup so that they only get\n> released in case they have been set up.\n>\n> - Reset `oi->delta_base_oid` towards the end of the function, together\n> with all the other object info pointers.\n>\n> Overall, all these changes result in a diff that is somewhat hard to\n> read. But the end result is significantly easier to read and reason\n> about, so I'd argue this one-time churn is worth it.\n>\n> Signed-off-by: Patrick Steinhardt \n> ---\n> object-file.c | 115 ++++++++++++++++++++++++++++++++++++----------------------\n> 1 file changed, 71 insertions(+), 44 deletions(-)\n>\n> diff --git a/object-file.c b/object-file.c\n> index 6280e42f34..e7e4c3348f 100644\n> --- a/object-file.c\n> +++ b/object-file.c\n> @@ -416,19 +416,16 @@ int odb_source_loose_read_object_info(struct odb_source *source,\n> \t\t\t\t const struct object_id *oid,\n> \t\t\t\t struct object_info *oi, int flags)\n> {\n> -\tint status = 0;\n> +\tint ret;\n> \tint fd;\n> \tunsigned long mapsize;\n> \tconst char *path;\n> -\tvoid *map;\n> -\tgit_zstream stream;\n> +\tvoid *map = NULL;\n> +\tgit_zstream stream, *stream_to_end = NULL;\n> \tchar hdr[MAX_HEADER_LEN];\n> \tunsigned long size_scratch;\n> \tenum object_type type_scratch;\n>\n> -\tif (oi && oi->delta_base_oid)\n> -\t\toidclr(oi->delta_base_oid, source->odb->repo->hash_algo);\n> -\n> \t/*\n> \t * If we don't care about type or size, then we don't\n> \t * need to look inside the object at all. Note that we\n> @@ -439,71 +436,101 @@ int odb_source_loose_read_object_info(struct odb_source *source,\n> \t */\n> \tif (!oi || (!oi->typep && !oi->sizep && !oi->contentp)) {\n> \t\tstruct stat st;\n> -\t\tif ((!oi || !oi->disk_sizep) && (flags & OBJECT_INFO_QUICK))\n> -\t\t\treturn quick_has_loose(source->loose, oid) ? 0 : -1;\n> -\t\tif (stat_loose_object(source->loose, oid, &st, &path) < 0)\n> -\t\t\treturn -1;\n> +\n> +\t\tif ((!oi || !oi->disk_sizep) && (flags & OBJECT_INFO_QUICK)) {\n> +\t\t\tret = quick_has_loose(source->loose, oid) ? 0 : -1;\n> +\t\t\tgoto out;\n> +\t\t}\n> +\n> +\t\tif (stat_loose_object(source->loose, oid, &st, &path) < 0) {\n> +\t\t\tret = -1;\n> +\t\t\tgoto out;\n> +\t\t}\n> +\n> \t\tif (oi && oi->disk_sizep)\n> \t\t\t*oi->disk_sizep = st.st_size;\n> -\t\treturn 0;\n> +\n> +\t\tret = 0;\n> +\t\tgoto out;\n> \t}\n>\n> \tfd = open_loose_object(source->loose, oid, &path);\n> \tif (fd < 0) {\n> \t\tif (errno != ENOENT)\n> \t\t\terror_errno(_(\"unable to open loose object %s\"), oid_to_hex(oid));\n> -\t\treturn -1;\n> +\t\tret = -1;\n> +\t\tgoto out;\n> \t}\n> -\tmap = map_fd(fd, path, &mapsize);\n> -\tif (!map)\n> -\t\treturn -1;\n>\n> -\tif (!oi->sizep)\n> -\t\toi->sizep = &size_scratch;\n> -\tif (!oi->typep)\n> -\t\toi->typep = &type_scratch;\n> +\tmap = map_fd(fd, path, &mapsize);\n> +\tif (!map) {\n> +\t\tret = -1;\n> +\t\tgoto out;\n> +\t}\n>\n> \tif (oi->disk_sizep)\n> \t\t*oi->disk_sizep = mapsize;\n>\n> +\tstream_to_end = &stream;\n> +\n>\n\nOkay we use `stream_to_end` to simply identify if the stream needs to be\ncleared.\n\nThe changes look good and indeed the final outcome is better here.\nThanks.\n"},{"id":"533277","messageId":"CAOLa=ZTOG7UGzch9y8-15QUDmMMSR4HqdRMyO-izSriLrKBM5g@mail.gmail.com","threadId":"64646","inReplyTo":"20260107-b4-pks-odb-read-object-info-improvements-v4-0-b5d55c47082a@pks.im","subject":"Re: [PATCH v4 0/7] Improvements for reading object info","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-01-08T09:30:53Z","receivedAt":"2026-01-08T09:30:58Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt writes:\n\n> Hi,\n>\n> this patch series contains various small improvements for reading object\n> info for either loose or packed objects. These improvements were split\n> out of a larger patch series where I'm about to introduce a new generic\n> `odb_for_each_object()` function.\n>\n> Changes in v4:\n> - Extend the fix for OI_LOOSE and refactor the whole function to have\n> a single exit path as proposed by Karthik. This results in a lot\n> more changes, but makes the function way easier to reason about\n> going forward.\n> - Link to v3: https://lore.kernel.org/r/20260106-b4-pks-odb-read-object-info-improvements-v3-0-b5e02fae1fb0@pks.im\n>\n\nI had a look at the first commit which was changed in this version and\nit looks much nicer now. Thanks!\n"},{"id":"533569","messageId":"20260112-b4-pks-odb-read-object-info-improvements-v5-0-9a6124e95bf2@pks.im","threadId":"64646","inReplyTo":"20251218-b4-pks-odb-read-object-info-improvements-v1-0-81c8368492be@pks.im","subject":"[PATCH v5 0/7] Improvements for reading object info","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-12T09:00:40Z","receivedAt":"2026-01-12T09:00:58Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nthis patch series contains various small improvements for reading object\ninfo for either loose or packed objects. These improvements were split\nout of a larger patch series where I'm about to introduce a new generic\n`odb_for_each_object()` function.\n\nChanges in v5:\n - I discovered that this patch series incidentally fixes a segfault\n when using git-archive(1) to read deltified blobs that are larger\n than \"core.bigFileThreshold\". So the only change is an added test\n case that will detect this regression going forward.\n - Link to v4: https://lore.kernel.org/r/20260107-b4-pks-odb-read-object-info-improvements-v4-0-b5d55c47082a@pks.im\n\nChanges in v4:\n - Extend the fix for OI_LOOSE and refactor the whole function to have\n a single exit path as proposed by Karthik. This results in a lot\n more changes, but makes the function way easier to reason about\n going forward.\n - Link to v3: https://lore.kernel.org/r/20260106-b4-pks-odb-read-object-info-improvements-v3-0-b5e02fae1fb0@pks.im\n\nChanges in v3:\n - Fix a commit message typo.\n - Fix a function comment missing some words.\n - Link to v2: https://lore.kernel.org/r/20251218-b4-pks-odb-read-object-info-improvements-v2-0-62e3e49072bc@pks.im\n\nChanges in v2:\n - Rebase the series on top of master with jc/object-read-stream-fix\n merged into it. I've also evicted the patch that fixes the same\n underlying issue.\n - Improve the commit message that drops OI_DBCACHED to explain why\n this is a safe refactoring.\n - Link to v1: https://lore.kernel.org/r/20251218-b4-pks-odb-read-object-info-improvements-v1-0-81c8368492be@pks.im\n\nThanks!\n\nPatrick\n\n---\nPatrick Steinhardt (7):\n object-file: always set OI_LOOSE when reading object info\n packfile: always declare object info to be OI_PACKED\n packfile: extend `is_delta` field to allow for \"unknown\" state\n packfile: always populate pack-specific info when reading object info\n packfile: disentangle return value of `packed_object_info()`\n packfile: skip unpacking object header for disk size requests\n packfile: drop repository parameter from `packed_object_info()`\n\n builtin/cat-file.c | 3 +-\n builtin/pack-objects.c | 4 +-\n commit-graph.c | 2 +-\n object-file.c | 115 ++++++++++++++++++++++++++++++-------------------\n odb.h | 8 +++-\n pack-bitmap.c | 3 +-\n packfile.c | 61 +++++++++++++++-----------\n packfile.h | 7 ++-\n t/t5003-archive-zip.sh | 34 +++++++++++++++\n 9 files changed, 158 insertions(+), 79 deletions(-)\n\nRange-diff versus v4:\n\n1: 07f529a631 = 1: da9d514001 object-file: always set OI_LOOSE when reading object info\n2: b547df2885 ! 2: c7b29f3789 packfile: always declare object info to be OI_PACKED\n @@ Commit message\n Drop the OI_DBCACHED enum completely. None of the callers seem to care\n about the distinction.\n \n + Note that this also fixes a segfault introduced in 8c1b84bc97\n + (streaming: move logic to read packed objects streams into backend,\n + 2025-11-23), which refactors how we stream packed objects. The intent is\n + to only read packed objects in case they are stored non-deltified as\n + we'd otherwise have to deflate them first. But the check for whether or\n + not the object is stored as a delta was unconditionally done via\n + `oi.u.packed.is_delta`, which is only valid in case `oi.whence` is\n + `OI_PACKED`. But under some circumstances we got `OI_DBCACHED` here,\n + which means that none of the `oi.u.packed` fields were initialized at\n + all. Consequently, we assumed the object was not stored as a delta, and\n + then try to read the object from `oi.u.packed.pack`, which is a `NULL`\n + pointer and thus causes a segfault.\n +\n + Add a test case for this issue so that this cannot regress in the\n + future anymore.\n +\n + Reported-by: Matt Smiley \n Signed-off-by: Patrick Steinhardt \n \n ## odb.h ##\n @@ packfile.c: int packed_object_info(struct repository *r, struct packed_git *p,\n \n out:\n \tunuse_pack(&w_curs);\n +\n + ## t/t5003-archive-zip.sh ##\n +@@ t/t5003-archive-zip.sh: check_zip with_untracked2\n + check_added with_untracked2 untracked one/untracked\n + check_added with_untracked2 untracked two/untracked\n + \n ++test_expect_success 'git-archive --format=zip with bigFile delta chains' '\n ++\ttest_when_finished rm -rf repo &&\n ++\tgit init repo &&\n ++\t(\n ++\t\tcd repo &&\n ++\t\ttest-tool genrandom foo 100000 >base &&\n ++\t\t{\n ++\t\t\tcat base &&\n ++\t\t\techo \"trailing data\"\n ++\t\t} >delta-1 &&\n ++\t\t{\n ++\t\t\tcat delta-1 &&\n ++\t\t\techo \"trailing data\"\n ++\t\t} >delta-2 &&\n ++\t\tgit add . &&\n ++\t\tgit commit -m \"blobs\" &&\n ++\t\tgit repack -Ad &&\n ++\t\tgit verify-pack -v .git/objects/pack/pack-*.idx >stats &&\n ++\t\ttest_grep \"chain length = 1: 1 object\" stats &&\n ++\t\ttest_grep \"chain length = 2: 1 object\" stats &&\n ++\n ++\t\tgit -c core.bigFileThreshold=1k archive --format=zip HEAD >archive.zip &&\n ++\t\tif test_have_prereq UNZIP\n ++\t\tthen\n ++\t\t\tmkdir unpack &&\n ++\t\t\tcd unpack &&\n ++\t\t\t\"$GIT_UNZIP\" ../archive.zip &&\n ++\t\t\ttest_cmp base ../base &&\n ++\t\t\ttest_cmp delta-1 ../delta-1 &&\n ++\t\t\ttest_cmp delta-2 ../delta-2\n ++\t\tfi\n ++\t)\n ++'\n ++\n + # Test remote archive over HTTP protocol.\n + #\n + # Note: this should be the last part of this test suite, because\n3: 28940ce932 = 3: ef5ac585f0 packfile: extend `is_delta` field to allow for \"unknown\" state\n4: c13c74467d = 4: 2a844d61fe packfile: always populate pack-specific info when reading object info\n5: d3c17fcc71 = 5: a23f59d530 packfile: disentangle return value of `packed_object_info()`\n6: 1c598686c5 = 6: f246dc3745 packfile: skip unpacking object header for disk size requests\n7: afc5d85991 = 7: a0c4f59547 packfile: drop repository parameter from `packed_object_info()`\n\n---\nbase-commit: 7df68b50e49b6a1b576abb19b2e5d457749bc28b\nchange-id: 20251215-b4-pks-odb-read-object-info-improvements-0e031ef827d2\n\n"},{"id":"533570","messageId":"20260112-b4-pks-odb-read-object-info-improvements-v5-1-9a6124e95bf2@pks.im","threadId":"64646","inReplyTo":"20260112-b4-pks-odb-read-object-info-improvements-v5-0-9a6124e95bf2@pks.im","subject":"[PATCH v5 1/7] object-file: always set OI_LOOSE when reading object info","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-12T09:00:41Z","receivedAt":"2026-01-12T09:01:00Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"There are some early returns in `odb_source_loose_read_object_info()`\nin cases where we don't have to open the loose object. These return\npaths do not set `struct object_info::whence` to `OI_LOOSE` though, so\nit becomes impossible for the caller to tell the format of such an\nobject.\n\nThe root cause of this really is that we have so many different return\npaths in the function. As a consequence, it's harder than necessary to\nmake sure that all successful exit paths sot up the `whence` field as\nexpected.\n\nAddress this by refactoring the function to have a single exit path.\nLike this, we can trivially set up the `whence` field when we exit\nsuccessfully from the function.\n\nNote that we also:\n\n - Rename `status` to `ret` to match our usual coding style, but also\n to show that the old `status` variable is now always getting the\n expected value. Furthermore, the value is not initialized anymore,\n which has the consequence that most compilers will warn for exit\n paths where we forgot to set it.\n\n - Move the setup of scratch pointers closer to `parse_loose_header()`\n to show where it's needed.\n\n - Guard a couple of variables on cleanup so that they only get\n released in case they have been set up.\n\n - Reset `oi->delta_base_oid` towards the end of the function, together\n with all the other object info pointers.\n\nOverall, all these changes result in a diff that is somewhat hard to\nread. But the end result is significantly easier to read and reason\nabout, so I'd argue this one-time churn is worth it.\n\nSigned-off-by: Patrick Steinhardt \n---\n object-file.c | 115 ++++++++++++++++++++++++++++++++++++----------------------\n 1 file changed, 71 insertions(+), 44 deletions(-)\n\ndiff --git a/object-file.c b/object-file.c\nindex 6280e42f34..e7e4c3348f 100644\n--- a/object-file.c\n+++ b/object-file.c\n@@ -416,19 +416,16 @@ int odb_source_loose_read_object_info(struct odb_source *source,\n \t\t\t\t const struct object_id *oid,\n \t\t\t\t struct object_info *oi, int flags)\n {\n-\tint status = 0;\n+\tint ret;\n \tint fd;\n \tunsigned long mapsize;\n \tconst char *path;\n-\tvoid *map;\n-\tgit_zstream stream;\n+\tvoid *map = NULL;\n+\tgit_zstream stream, *stream_to_end = NULL;\n \tchar hdr[MAX_HEADER_LEN];\n \tunsigned long size_scratch;\n \tenum object_type type_scratch;\n \n-\tif (oi && oi->delta_base_oid)\n-\t\toidclr(oi->delta_base_oid, source->odb->repo->hash_algo);\n-\n \t/*\n \t * If we don't care about type or size, then we don't\n \t * need to look inside the object at all. Note that we\n@@ -439,71 +436,101 @@ int odb_source_loose_read_object_info(struct odb_source *source,\n \t */\n \tif (!oi || (!oi->typep && !oi->sizep && !oi->contentp)) {\n \t\tstruct stat st;\n-\t\tif ((!oi || !oi->disk_sizep) && (flags & OBJECT_INFO_QUICK))\n-\t\t\treturn quick_has_loose(source->loose, oid) ? 0 : -1;\n-\t\tif (stat_loose_object(source->loose, oid, &st, &path) < 0)\n-\t\t\treturn -1;\n+\n+\t\tif ((!oi || !oi->disk_sizep) && (flags & OBJECT_INFO_QUICK)) {\n+\t\t\tret = quick_has_loose(source->loose, oid) ? 0 : -1;\n+\t\t\tgoto out;\n+\t\t}\n+\n+\t\tif (stat_loose_object(source->loose, oid, &st, &path) < 0) {\n+\t\t\tret = -1;\n+\t\t\tgoto out;\n+\t\t}\n+\n \t\tif (oi && oi->disk_sizep)\n \t\t\t*oi->disk_sizep = st.st_size;\n-\t\treturn 0;\n+\n+\t\tret = 0;\n+\t\tgoto out;\n \t}\n \n \tfd = open_loose_object(source->loose, oid, &path);\n \tif (fd < 0) {\n \t\tif (errno != ENOENT)\n \t\t\terror_errno(_(\"unable to open loose object %s\"), oid_to_hex(oid));\n-\t\treturn -1;\n+\t\tret = -1;\n+\t\tgoto out;\n \t}\n-\tmap = map_fd(fd, path, &mapsize);\n-\tif (!map)\n-\t\treturn -1;\n \n-\tif (!oi->sizep)\n-\t\toi->sizep = &size_scratch;\n-\tif (!oi->typep)\n-\t\toi->typep = &type_scratch;\n+\tmap = map_fd(fd, path, &mapsize);\n+\tif (!map) {\n+\t\tret = -1;\n+\t\tgoto out;\n+\t}\n \n \tif (oi->disk_sizep)\n \t\t*oi->disk_sizep = mapsize;\n \n+\tstream_to_end = &stream;\n+\n \tswitch (unpack_loose_header(&stream, map, mapsize, hdr, sizeof(hdr))) {\n \tcase ULHR_OK:\n-\t\tif (parse_loose_header(hdr, oi) < 0)\n-\t\t\tstatus = error(_(\"unable to parse %s header\"), oid_to_hex(oid));\n-\t\telse if (*oi->typep < 0)\n+\t\tif (!oi->sizep)\n+\t\t\toi->sizep = &size_scratch;\n+\t\tif (!oi->typep)\n+\t\t\toi->typep = &type_scratch;\n+\n+\t\tif (parse_loose_header(hdr, oi) < 0) {\n+\t\t\tret = error(_(\"unable to parse %s header\"), oid_to_hex(oid));\n+\t\t\tgoto corrupt;\n+\t\t}\n+\n+\t\tif (*oi->typep < 0)\n \t\t\tdie(_(\"invalid object type\"));\n \n-\t\tif (!oi->contentp)\n-\t\t\tbreak;\n-\t\t*oi->contentp = unpack_loose_rest(&stream, hdr, *oi->sizep, oid);\n-\t\tif (*oi->contentp)\n-\t\t\tgoto cleanup;\n+\t\tif (oi->contentp) {\n+\t\t\t*oi->contentp = unpack_loose_rest(&stream, hdr, *oi->sizep, oid);\n+\t\t\tif (!*oi->contentp) {\n+\t\t\t\tret = -1;\n+\t\t\t\tgoto corrupt;\n+\t\t\t}\n+\t\t}\n \n-\t\tstatus = -1;\n \t\tbreak;\n \tcase ULHR_BAD:\n-\t\tstatus = error(_(\"unable to unpack %s header\"),\n-\t\t\t oid_to_hex(oid));\n-\t\tbreak;\n+\t\tret = error(_(\"unable to unpack %s header\"),\n+\t\t\t oid_to_hex(oid));\n+\t\tgoto corrupt;\n \tcase ULHR_TOO_LONG:\n-\t\tstatus = error(_(\"header for %s too long, exceeds %d bytes\"),\n-\t\t\t oid_to_hex(oid), MAX_HEADER_LEN);\n-\t\tbreak;\n+\t\tret = error(_(\"header for %s too long, exceeds %d bytes\"),\n+\t\t\t oid_to_hex(oid), MAX_HEADER_LEN);\n+\t\tgoto corrupt;\n \t}\n \n-\tif (status && (flags & OBJECT_INFO_DIE_IF_CORRUPT))\n+\tret = 0;\n+\n+corrupt:\n+\tif (ret && (flags & OBJECT_INFO_DIE_IF_CORRUPT))\n \t\tdie(_(\"loose object %s (stored in %s) is corrupt\"),\n \t\t oid_to_hex(oid), path);\n \n-cleanup:\n-\tgit_inflate_end(&stream);\n-\tmunmap(map, mapsize);\n-\tif (oi->sizep == &size_scratch)\n-\t\toi->sizep = NULL;\n-\tif (oi->typep == &type_scratch)\n-\t\toi->typep = NULL;\n-\toi->whence = OI_LOOSE;\n-\treturn status;\n+out:\n+\tif (stream_to_end)\n+\t\tgit_inflate_end(stream_to_end);\n+\tif (map)\n+\t\tmunmap(map, mapsize);\n+\tif (oi) {\n+\t\tif (oi->sizep == &size_scratch)\n+\t\t\toi->sizep = NULL;\n+\t\tif (oi->typep == &type_scratch)\n+\t\t\toi->typep = NULL;\n+\t\tif (oi->delta_base_oid)\n+\t\t\toidclr(oi->delta_base_oid, source->odb->repo->hash_algo);\n+\t\tif (!ret)\n+\t\t\toi->whence = OI_LOOSE;\n+\t}\n+\n+\treturn ret;\n }\n \n static void hash_object_body(const struct git_hash_algo *algo, struct git_hash_ctx *c,\n\n-- \n2.52.0.590.g1f87b77810.dirty\n\n"},{"id":"533571","messageId":"20260112-b4-pks-odb-read-object-info-improvements-v5-2-9a6124e95bf2@pks.im","threadId":"64646","inReplyTo":"20260112-b4-pks-odb-read-object-info-improvements-v5-0-9a6124e95bf2@pks.im","subject":"[PATCH v5 2/7] packfile: always declare object info to be OI_PACKED","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-12T09:00:42Z","receivedAt":"2026-01-12T09:01:02Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When reading object info via a packfile we yield one of two types:\n\n - The object can either be OI_PACKED, which is what a caller would\n typically expect.\n\n - Or it can be OI_DBCACHED if it is stored in the delta base cache.\n\nThe latter really is an implementation detail though, and callers\ntypically don't care at all about the difference. Furthermore, the\ninformation whether or not it is part of the delta base cache can\nalready be derived via the `is_delta` field, so the fact that we discern\nbetween OI_PACKED and OI_DBCACHED only further complicates the\ninterface.\n\nThere aren't all that many callers that care about the `whence` field in\nthe first place. In fact, there's only three:\n\n - `packfile_store_read_object_info()` checks for `whence == OI_PACKED`\n and then populates the packfile information of the object info\n structure. We now start to do this also for deltified objects, which\n gives its callers strictly more information.\n\n - `repack_local_links()` wants to determine whether the object is part\n of a promisor pack and checks for `whence == OI_PACKED`. If so, it\n verifies that the packfile is a promisor pack. It's arguably wrong\n to declare that an object is not part of a promisor pack only\n because it is stored in the delta base cache.\n\n - `is_not_in_promisor_pack_obj()` does the same, but checks that a\n specific object is _not_ part of a promisor pack. The same reasoning\n as above applies.\n\nDrop the OI_DBCACHED enum completely. None of the callers seem to care\nabout the distinction.\n\nNote that this also fixes a segfault introduced in 8c1b84bc97\n(streaming: move logic to read packed objects streams into backend,\n2025-11-23), which refactors how we stream packed objects. The intent is\nto only read packed objects in case they are stored non-deltified as\nwe'd otherwise have to deflate them first. But the check for whether or\nnot the object is stored as a delta was unconditionally done via\n`oi.u.packed.is_delta`, which is only valid in case `oi.whence` is\n`OI_PACKED`. But under some circumstances we got `OI_DBCACHED` here,\nwhich means that none of the `oi.u.packed` fields were initialized at\nall. Consequently, we assumed the object was not stored as a delta, and\nthen try to read the object from `oi.u.packed.pack`, which is a `NULL`\npointer and thus causes a segfault.\n\nAdd a test case for this issue so that this cannot regress in the\nfuture anymore.\n\nReported-by: Matt Smiley \nSigned-off-by: Patrick Steinhardt \n---\n odb.h | 1 -\n packfile.c | 3 +--\n t/t5003-archive-zip.sh | 34 ++++++++++++++++++++++++++++++++++\n 3 files changed, 35 insertions(+), 3 deletions(-)\n\ndiff --git a/odb.h b/odb.h\nindex 014cd9585a..73b0b87ad5 100644\n--- a/odb.h\n+++ b/odb.h\n@@ -330,7 +330,6 @@ struct object_info {\n \t\tOI_CACHED,\n \t\tOI_LOOSE,\n \t\tOI_PACKED,\n-\t\tOI_DBCACHED\n \t} whence;\n \tunion {\n \t\t/*\ndiff --git a/packfile.c b/packfile.c\nindex 08a0863fc3..b0c6665c87 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -1656,8 +1656,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \t\t\toidclr(oi->delta_base_oid, p->repo->hash_algo);\n \t}\n \n-\toi->whence = in_delta_base_cache(p, obj_offset) ? OI_DBCACHED :\n-\t\t\t\t\t\t\t OI_PACKED;\n+\toi->whence = OI_PACKED;\n \n out:\n \tunuse_pack(&w_curs);\ndiff --git a/t/t5003-archive-zip.sh b/t/t5003-archive-zip.sh\nindex 961c6aac25..c8c1c5c06b 100755\n--- a/t/t5003-archive-zip.sh\n+++ b/t/t5003-archive-zip.sh\n@@ -239,6 +239,40 @@ check_zip with_untracked2\n check_added with_untracked2 untracked one/untracked\n check_added with_untracked2 untracked two/untracked\n \n+test_expect_success 'git-archive --format=zip with bigFile delta chains' '\n+\ttest_when_finished rm -rf repo &&\n+\tgit init repo &&\n+\t(\n+\t\tcd repo &&\n+\t\ttest-tool genrandom foo 100000 >base &&\n+\t\t{\n+\t\t\tcat base &&\n+\t\t\techo \"trailing data\"\n+\t\t} >delta-1 &&\n+\t\t{\n+\t\t\tcat delta-1 &&\n+\t\t\techo \"trailing data\"\n+\t\t} >delta-2 &&\n+\t\tgit add . &&\n+\t\tgit commit -m \"blobs\" &&\n+\t\tgit repack -Ad &&\n+\t\tgit verify-pack -v .git/objects/pack/pack-*.idx >stats &&\n+\t\ttest_grep \"chain length = 1: 1 object\" stats &&\n+\t\ttest_grep \"chain length = 2: 1 object\" stats &&\n+\n+\t\tgit -c core.bigFileThreshold=1k archive --format=zip HEAD >archive.zip &&\n+\t\tif test_have_prereq UNZIP\n+\t\tthen\n+\t\t\tmkdir unpack &&\n+\t\t\tcd unpack &&\n+\t\t\t\"$GIT_UNZIP\" ../archive.zip &&\n+\t\t\ttest_cmp base ../base &&\n+\t\t\ttest_cmp delta-1 ../delta-1 &&\n+\t\t\ttest_cmp delta-2 ../delta-2\n+\t\tfi\n+\t)\n+'\n+\n # Test remote archive over HTTP protocol.\n #\n # Note: this should be the last part of this test suite, because\n\n-- \n2.52.0.590.g1f87b77810.dirty\n\n"},{"id":"533572","messageId":"20260112-b4-pks-odb-read-object-info-improvements-v5-3-9a6124e95bf2@pks.im","threadId":"64646","inReplyTo":"20260112-b4-pks-odb-read-object-info-improvements-v5-0-9a6124e95bf2@pks.im","subject":"[PATCH v5 3/7] packfile: extend `is_delta` field to allow for \"unknown\" state","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-12T09:00:43Z","receivedAt":"2026-01-12T09:01:05Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The `struct object_info::u::packed::is_delta` field determines whether\nor not a specific object is stored as a delta. It only stores whether or\nnot the object is stored as delta, so it is treated as a boolean value.\n\nThis boolean is insufficient though: when reading a packed object via\n`packfile_store_read_object_info()` we know to skip parsing the actual\nobject when the user didn't request any object-specific data. In that\ncase we won't read the object itself, but will only look up its position\nin the packfile. Consequently, we do not know whether it is a delta or\nnot.\n\nThis isn't really an issue right now, as the check for an empty request\nis broken. But a subsequent commit will fix it, and once we do we will\nhave the need to also represent an \"unknown\" delta state.\n\nPrepare for this change by introducing a new enum that encodes the\nobject type. We don't use the \"unknown\" state just yet, but will start\nto do so in a subsequent commit.\n\nSigned-off-by: Patrick Steinhardt \n---\n odb.h | 7 ++++++-\n packfile.c | 17 ++++++++++++++---\n 2 files changed, 20 insertions(+), 4 deletions(-)\n\ndiff --git a/odb.h b/odb.h\nindex 73b0b87ad5..afae5e5c01 100644\n--- a/odb.h\n+++ b/odb.h\n@@ -343,7 +343,12 @@ struct object_info {\n \t\tstruct {\n \t\t\tstruct packed_git *pack;\n \t\t\toff_t offset;\n-\t\t\tunsigned int is_delta;\n+\t\t\tenum packed_object_type {\n+\t\t\t\tPACKED_OBJECT_TYPE_UNKNOWN,\n+\t\t\t\tPACKED_OBJECT_TYPE_FULL,\n+\t\t\t\tPACKED_OBJECT_TYPE_OFS_DELTA,\n+\t\t\t\tPACKED_OBJECT_TYPE_REF_DELTA,\n+\t\t\t} type;\n \t\t} packed;\n \t} u;\n };\ndiff --git a/packfile.c b/packfile.c\nindex b0c6665c87..cc797b2b6a 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -2159,8 +2159,18 @@ int packfile_store_read_object_info(struct packfile_store *store,\n \tif (oi->whence == OI_PACKED) {\n \t\toi->u.packed.offset = e.offset;\n \t\toi->u.packed.pack = e.p;\n-\t\toi->u.packed.is_delta = (rtype == OBJ_REF_DELTA ||\n-\t\t\t\t\t rtype == OBJ_OFS_DELTA);\n+\n+\t\tswitch (rtype) {\n+\t\tcase OBJ_REF_DELTA:\n+\t\t\toi->u.packed.type = PACKED_OBJECT_TYPE_REF_DELTA;\n+\t\t\tbreak;\n+\t\tcase OBJ_OFS_DELTA:\n+\t\t\toi->u.packed.type = PACKED_OBJECT_TYPE_OFS_DELTA;\n+\t\t\tbreak;\n+\t\tdefault:\n+\t\t\toi->u.packed.type = PACKED_OBJECT_TYPE_FULL;\n+\t\t\tbreak;\n+\t\t}\n \t}\n \n \treturn 0;\n@@ -2531,7 +2541,8 @@ int packfile_store_read_object_stream(struct odb_read_stream **out,\n \toi.sizep = &size;\n \n \tif (packfile_store_read_object_info(store, oid, &oi, 0) ||\n-\t oi.u.packed.is_delta ||\n+\t oi.u.packed.type == PACKED_OBJECT_TYPE_REF_DELTA ||\n+\t oi.u.packed.type == PACKED_OBJECT_TYPE_OFS_DELTA ||\n \t repo_settings_get_big_file_threshold(store->odb->repo) >= size)\n \t\treturn -1;\n \n\n-- \n2.52.0.590.g1f87b77810.dirty\n\n"},{"id":"533573","messageId":"20260112-b4-pks-odb-read-object-info-improvements-v5-4-9a6124e95bf2@pks.im","threadId":"64646","inReplyTo":"20260112-b4-pks-odb-read-object-info-improvements-v5-0-9a6124e95bf2@pks.im","subject":"[PATCH v5 4/7] packfile: always populate pack-specific info when reading object info","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-12T09:00:44Z","receivedAt":"2026-01-12T09:01:07Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When reading object information via `packed_object_info()` we may not\npopulate the object info's packfile-specific fields. This leads to\ninconsistent object info depending on whether the info was populated via\n`packfile_store_read_object_info()` or `packed_object_info()`.\n\nFix this inconsistency so that we can always assume the pack info to be\npopulated when reading object info from a pack.\n\nSigned-off-by: Patrick Steinhardt \n---\n packfile.c | 31 ++++++++++++++-----------------\n 1 file changed, 14 insertions(+), 17 deletions(-)\n\ndiff --git a/packfile.c b/packfile.c\nindex cc797b2b6a..f7c33a2f77 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -1657,6 +1657,20 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \t}\n \n \toi->whence = OI_PACKED;\n+\toi->u.packed.offset = obj_offset;\n+\toi->u.packed.pack = p;\n+\n+\tswitch (type) {\n+\tcase OBJ_REF_DELTA:\n+\t\toi->u.packed.type = PACKED_OBJECT_TYPE_REF_DELTA;\n+\t\tbreak;\n+\tcase OBJ_OFS_DELTA:\n+\t\toi->u.packed.type = PACKED_OBJECT_TYPE_OFS_DELTA;\n+\t\tbreak;\n+\tdefault:\n+\t\toi->u.packed.type = PACKED_OBJECT_TYPE_FULL;\n+\t\tbreak;\n+\t}\n \n out:\n \tunuse_pack(&w_curs);\n@@ -2156,23 +2170,6 @@ int packfile_store_read_object_info(struct packfile_store *store,\n \t\treturn -1;\n \t}\n \n-\tif (oi->whence == OI_PACKED) {\n-\t\toi->u.packed.offset = e.offset;\n-\t\toi->u.packed.pack = e.p;\n-\n-\t\tswitch (rtype) {\n-\t\tcase OBJ_REF_DELTA:\n-\t\t\toi->u.packed.type = PACKED_OBJECT_TYPE_REF_DELTA;\n-\t\t\tbreak;\n-\t\tcase OBJ_OFS_DELTA:\n-\t\t\toi->u.packed.type = PACKED_OBJECT_TYPE_OFS_DELTA;\n-\t\t\tbreak;\n-\t\tdefault:\n-\t\t\toi->u.packed.type = PACKED_OBJECT_TYPE_FULL;\n-\t\t\tbreak;\n-\t\t}\n-\t}\n-\n \treturn 0;\n }\n \n\n-- \n2.52.0.590.g1f87b77810.dirty\n\n"},{"id":"533574","messageId":"20260112-b4-pks-odb-read-object-info-improvements-v5-5-9a6124e95bf2@pks.im","threadId":"64646","inReplyTo":"20260112-b4-pks-odb-read-object-info-improvements-v5-0-9a6124e95bf2@pks.im","subject":"[PATCH v5 5/7] packfile: disentangle return value of `packed_object_info()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-12T09:00:45Z","receivedAt":"2026-01-12T09:01:10Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The `packed_object_info()` function returns the type of the packed\nobject. While we use an `enum object_type` to store the return value,\nthis type is not to be confused with the actual object type. It _may_\ncontain the object type, but it may just as well encode that the given\npacked object is stored as a delta.\n\nWe have removed the only caller that relied on this returned object type\nin the preceding commit, so let's simplify semantics and return either 0\non success or a negative error code otherwise.\n\nThis unblocks a small optimization where we can skip reading the object\ntype altogether.\n\nSigned-off-by: Patrick Steinhardt \n---\n packfile.c | 21 ++++++++++++---------\n packfile.h | 4 ++++\n 2 files changed, 16 insertions(+), 9 deletions(-)\n\ndiff --git a/packfile.c b/packfile.c\nindex f7c33a2f77..8c6ef45a67 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -1587,6 +1587,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \tunsigned long size;\n \toff_t curpos = obj_offset;\n \tenum object_type type;\n+\tint ret;\n \n \t/*\n \t * We always get the representation type, but only convert it to\n@@ -1607,12 +1608,12 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \t\t\toff_t base_offset = get_delta_base(p, &w_curs, &tmp_pos,\n \t\t\t\t\t\t\t type, obj_offset);\n \t\t\tif (!base_offset) {\n-\t\t\t\ttype = OBJ_BAD;\n+\t\t\t\tret = -1;\n \t\t\t\tgoto out;\n \t\t\t}\n \t\t\t*oi->sizep = get_size_from_delta(p, &w_curs, tmp_pos);\n \t\t\tif (*oi->sizep == 0) {\n-\t\t\t\ttype = OBJ_BAD;\n+\t\t\t\tret = -1;\n \t\t\t\tgoto out;\n \t\t\t}\n \t\t} else {\n@@ -1625,7 +1626,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \t\tif (offset_to_pack_pos(p, obj_offset, &pos) < 0) {\n \t\t\terror(\"could not find object at offset %\"PRIuMAX\" \"\n \t\t\t \"in pack %s\", (uintmax_t)obj_offset, p->pack_name);\n-\t\t\ttype = OBJ_BAD;\n+\t\t\tret = -1;\n \t\t\tgoto out;\n \t\t}\n \n@@ -1639,7 +1640,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \t\tif (oi->typep)\n \t\t\t*oi->typep = ptot;\n \t\tif (ptot < 0) {\n-\t\t\ttype = OBJ_BAD;\n+\t\t\tret = -1;\n \t\t\tgoto out;\n \t\t}\n \t}\n@@ -1649,7 +1650,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \t\t\tif (get_delta_base_oid(p, &w_curs, curpos,\n \t\t\t\t\t oi->delta_base_oid,\n \t\t\t\t\t type, obj_offset) < 0) {\n-\t\t\t\ttype = OBJ_BAD;\n+\t\t\t\tret = -1;\n \t\t\t\tgoto out;\n \t\t\t}\n \t\t} else\n@@ -1672,9 +1673,11 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \t\tbreak;\n \t}\n \n+\tret = 0;\n+\n out:\n \tunuse_pack(&w_curs);\n-\treturn type;\n+\treturn ret;\n }\n \n static void *unpack_compressed_entry(struct packed_git *p,\n@@ -2152,7 +2155,7 @@ int packfile_store_read_object_info(struct packfile_store *store,\n \t\t\t\t unsigned flags UNUSED)\n {\n \tstruct pack_entry e;\n-\tint rtype;\n+\tint ret;\n \n \tif (!find_pack_entry(store->odb->repo, oid, &e))\n \t\treturn 1;\n@@ -2164,8 +2167,8 @@ int packfile_store_read_object_info(struct packfile_store *store,\n \tif (!oi)\n \t\treturn 0;\n \n-\trtype = packed_object_info(store->odb->repo, e.p, e.offset, oi);\n-\tif (rtype < 0) {\n+\tret = packed_object_info(store->odb->repo, e.p, e.offset, oi);\n+\tif (ret < 0) {\n \t\tmark_bad_packed_object(e.p, oid);\n \t\treturn -1;\n \t}\ndiff --git a/packfile.h b/packfile.h\nindex 59d162a3f4..d7cce582af 100644\n--- a/packfile.h\n+++ b/packfile.h\n@@ -378,6 +378,10 @@ void release_pack_memory(size_t);\n /* global flag to enable extra checks when accessing packed objects */\n extern int do_check_packed_object_crc;\n \n+/*\n+ * Look up the object info for a specific offset in the packfile.\n+ * Returns zero on success, a negative error code otherwise.\n+ */\n int packed_object_info(struct repository *r,\n \t\t struct packed_git *pack,\n \t\t off_t offset, struct object_info *);\n\n-- \n2.52.0.590.g1f87b77810.dirty\n\n"},{"id":"533575","messageId":"20260112-b4-pks-odb-read-object-info-improvements-v5-6-9a6124e95bf2@pks.im","threadId":"64646","inReplyTo":"20260112-b4-pks-odb-read-object-info-improvements-v5-0-9a6124e95bf2@pks.im","subject":"[PATCH v5 6/7] packfile: skip unpacking object header for disk size requests","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-12T09:00:46Z","receivedAt":"2026-01-12T09:01:13Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"While most of the object info requests for a packed object require us to\nunpack its headers, reading its disk size doesn't. We still unpack the\nobject header in that case though, which is unnecessary work.\n\nSkip reading the header if only the disk size is requested. This leads\nto a small speedup when reading disk size, only. The following benchmark\nwas done in the Git repository:\n\n Benchmark 1: ./git rev-list --disk-usage HEAD (rev = HEAD~)\n Time (mean ± σ): 105.2 ms ± 0.6 ms [User: 91.4 ms, System: 13.3 ms]\n Range (min … max): 103.7 ms … 106.0 ms 27 runs\n\n Benchmark 2: ./git rev-list --disk-usage HEAD (rev = HEAD)\n Time (mean ± σ): 96.7 ms ± 0.4 ms [User: 86.2 ms, System: 10.0 ms]\n Range (min … max): 96.2 ms … 98.1 ms 30 runs\n\n Summary\n ./git rev-list --disk-usage HEAD (rev = HEAD) ran\n 1.09 ± 0.01 times faster than ./git rev-list --disk-usage HEAD (rev = HEAD~)\n\nSigned-off-by: Patrick Steinhardt \n---\n packfile.c | 7 +++++--\n 1 file changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/packfile.c b/packfile.c\nindex 8c6ef45a67..a2ba237ce7 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -1586,7 +1586,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \tstruct pack_window *w_curs = NULL;\n \tunsigned long size;\n \toff_t curpos = obj_offset;\n-\tenum object_type type;\n+\tenum object_type type = OBJ_NONE;\n \tint ret;\n \n \t/*\n@@ -1598,7 +1598,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \t\t\t\t\t\t &type);\n \t\tif (!*oi->contentp)\n \t\t\ttype = OBJ_BAD;\n-\t} else {\n+\t} else if (oi->sizep || oi->typep || oi->delta_base_oid) {\n \t\ttype = unpack_object_header(p, &w_curs, &curpos, &size);\n \t}\n \n@@ -1662,6 +1662,9 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \toi->u.packed.pack = p;\n \n \tswitch (type) {\n+\tcase OBJ_NONE:\n+\t\toi->u.packed.type = PACKED_OBJECT_TYPE_UNKNOWN;\n+\t\tbreak;\n \tcase OBJ_REF_DELTA:\n \t\toi->u.packed.type = PACKED_OBJECT_TYPE_REF_DELTA;\n \t\tbreak;\n\n-- \n2.52.0.590.g1f87b77810.dirty\n\n"},{"id":"533576","messageId":"20260112-b4-pks-odb-read-object-info-improvements-v5-7-9a6124e95bf2@pks.im","threadId":"64646","inReplyTo":"20260112-b4-pks-odb-read-object-info-improvements-v5-0-9a6124e95bf2@pks.im","subject":"[PATCH v5 7/7] packfile: drop repository parameter from `packed_object_info()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-01-12T09:00:47Z","receivedAt":"2026-01-12T09:01:16Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The function `packed_object_info()` takes a packfile and offset and\nreturns the object info for the corresponding object. Despite these two\nparameters though it also takes a repository pointer. This is redundant\ninformation though, as `struct packed_git` already has a repository\npointer that is always populated.\n\nDrop the redundant parameter.\n\nSigned-off-by: Patrick Steinhardt \n---\n builtin/cat-file.c | 3 +--\n builtin/pack-objects.c | 4 ++--\n commit-graph.c | 2 +-\n pack-bitmap.c | 3 +--\n packfile.c | 8 ++++----\n packfile.h | 3 +--\n 6 files changed, 10 insertions(+), 13 deletions(-)\n\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex 505ddaa12f..2ad712e9f8 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -487,8 +487,7 @@ static void batch_object_write(const char *obj_name,\n \t\t\tdata->info.sizep = &data->size;\n \n \t\tif (pack)\n-\t\t\tret = packed_object_info(the_repository, pack,\n-\t\t\t\t\t\t offset, &data->info);\n+\t\t\tret = packed_object_info(pack, offset, &data->info);\n \t\telse\n \t\t\tret = odb_read_object_info_extended(the_repository->objects,\n \t\t\t\t\t\t\t &data->oid, &data->info,\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 1ce8d6ee21..85762f8c4f 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -2411,7 +2411,7 @@ static void drop_reused_delta(struct object_entry *entry)\n \n \toi.sizep = &size;\n \toi.typep = &type;\n-\tif (packed_object_info(the_repository, IN_PACK(entry), entry->in_pack_offset, &oi) < 0) {\n+\tif (packed_object_info(IN_PACK(entry), entry->in_pack_offset, &oi) < 0) {\n \t\t/*\n \t\t * We failed to get the info from this pack for some reason;\n \t\t * fall back to odb_read_object_info, which may find another copy.\n@@ -3748,7 +3748,7 @@ static int add_object_entry_from_pack(const struct object_id *oid,\n \t\tstruct object_info oi = OBJECT_INFO_INIT;\n \n \t\toi.typep = &type;\n-\t\tif (packed_object_info(the_repository, p, ofs, &oi) < 0) {\n+\t\tif (packed_object_info(p, ofs, &oi) < 0) {\n \t\t\tdie(_(\"could not get type of object %s in pack %s\"),\n \t\t\t oid_to_hex(oid), p->pack_name);\n \t\t} else if (type == OBJ_COMMIT) {\ndiff --git a/commit-graph.c b/commit-graph.c\nindex 80be2ff2c3..f572670bd0 100644\n--- a/commit-graph.c\n+++ b/commit-graph.c\n@@ -1499,7 +1499,7 @@ static int add_packed_commits(const struct object_id *oid,\n \t\tdisplay_progress(ctx->progress, ++ctx->progress_done);\n \n \toi.typep = &type;\n-\tif (packed_object_info(ctx->r, pack, offset, &oi) < 0)\n+\tif (packed_object_info(pack, offset, &oi) < 0)\n \t\tdie(_(\"unable to get type of object %s\"), oid_to_hex(oid));\n \n \tif (type != OBJ_COMMIT)\ndiff --git a/pack-bitmap.c b/pack-bitmap.c\nindex 8ca79725b1..972203f12b 100644\n--- a/pack-bitmap.c\n+++ b/pack-bitmap.c\n@@ -1876,8 +1876,7 @@ static unsigned long get_size_by_pos(struct bitmap_index *bitmap_git,\n \t\t\tofs = pack_pos_to_offset(pack, pos);\n \t\t}\n \n-\t\tif (packed_object_info(bitmap_repo(bitmap_git), pack, ofs,\n-\t\t\t\t &oi) < 0) {\n+\t\tif (packed_object_info(pack, ofs, &oi) < 0) {\n \t\t\tstruct object_id oid;\n \t\t\tnth_bitmap_object_oid(bitmap_git, &oid,\n \t\t\t\t\t pack_pos_to_index(pack, pos));\ndiff --git a/packfile.c b/packfile.c\nindex a2ba237ce7..39899aec49 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -1580,7 +1580,7 @@ static void add_delta_base_cache(struct packed_git *p, off_t base_offset,\n \thashmap_add(&delta_base_cache, &ent->ent);\n }\n \n-int packed_object_info(struct repository *r, struct packed_git *p,\n+int packed_object_info(struct packed_git *p,\n \t\t off_t obj_offset, struct object_info *oi)\n {\n \tstruct pack_window *w_curs = NULL;\n@@ -1594,7 +1594,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \t * a \"real\" type later if the caller is interested.\n \t */\n \tif (oi->contentp) {\n-\t\t*oi->contentp = cache_or_unpack_entry(r, p, obj_offset, oi->sizep,\n+\t\t*oi->contentp = cache_or_unpack_entry(p->repo, p, obj_offset, oi->sizep,\n \t\t\t\t\t\t &type);\n \t\tif (!*oi->contentp)\n \t\t\ttype = OBJ_BAD;\n@@ -1635,7 +1635,7 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \n \tif (oi->typep) {\n \t\tenum object_type ptot;\n-\t\tptot = packed_to_object_type(r, p, obj_offset,\n+\t\tptot = packed_to_object_type(p->repo, p, obj_offset,\n \t\t\t\t\t type, &w_curs, curpos);\n \t\tif (oi->typep)\n \t\t\t*oi->typep = ptot;\n@@ -2170,7 +2170,7 @@ int packfile_store_read_object_info(struct packfile_store *store,\n \tif (!oi)\n \t\treturn 0;\n \n-\tret = packed_object_info(store->odb->repo, e.p, e.offset, oi);\n+\tret = packed_object_info(e.p, e.offset, oi);\n \tif (ret < 0) {\n \t\tmark_bad_packed_object(e.p, oid);\n \t\treturn -1;\ndiff --git a/packfile.h b/packfile.h\nindex d7cce582af..33fed26362 100644\n--- a/packfile.h\n+++ b/packfile.h\n@@ -382,8 +382,7 @@ extern int do_check_packed_object_crc;\n * Look up the object info for a specific offset in the packfile.\n * Returns zero on success, a negative error code otherwise.\n */\n-int packed_object_info(struct repository *r,\n-\t\t struct packed_git *pack,\n+int packed_object_info(struct packed_git *pack,\n \t\t off_t offset, struct object_info *);\n \n void mark_bad_packed_object(struct packed_git *, const struct object_id *);\n\n-- \n2.52.0.590.g1f87b77810.dirty\n\n"},{"id":"533639","messageId":"xmqqzf6inapc.fsf@gitster.g","threadId":"64646","inReplyTo":"20260112-b4-pks-odb-read-object-info-improvements-v5-2-9a6124e95bf2@pks.im","subject":"Re: [PATCH v5 2/7] packfile: always declare object info to be OI_PACKED","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-01-12T14:54:07Z","receivedAt":"2026-01-12T14:54:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n> Note that this also fixes a segfault introduced in 8c1b84bc97\n> (streaming: move logic to read packed objects streams into backend,\n> 2025-11-23), which refactors how we stream packed objects. The intent is\n> to only read packed objects in case they are stored non-deltified as\n> we'd otherwise have to deflate them first. But the check for whether or\n> not the object is stored as a delta was unconditionally done via\n> `oi.u.packed.is_delta`, which is only valid in case `oi.whence` is\n> `OI_PACKED`. But under some circumstances we got `OI_DBCACHED` here,\n> which means that none of the `oi.u.packed` fields were initialized at\n> all. Consequently, we assumed the object was not stored as a delta, and\n> then try to read the object from `oi.u.packed.pack`, which is a `NULL`\n> pointer and thus causes a segfault.\n>\n> Add a test case for this issue so that this cannot regress in the\n> future anymore.\n\nGreat. Thanks. Will requeue.\n\n> Reported-by: Matt Smiley \n> Signed-off-by: Patrick Steinhardt \n> ---\n> odb.h | 1 -\n> packfile.c | 3 +--\n> t/t5003-archive-zip.sh | 34 ++++++++++++++++++++++++++++++++++\n> 3 files changed, 35 insertions(+), 3 deletions(-)\n>\n> diff --git a/odb.h b/odb.h\n> index 014cd9585a..73b0b87ad5 100644\n> --- a/odb.h\n> +++ b/odb.h\n> @@ -330,7 +330,6 @@ struct object_info {\n> \t\tOI_CACHED,\n> \t\tOI_LOOSE,\n> \t\tOI_PACKED,\n> -\t\tOI_DBCACHED\n> \t} whence;\n\n"}]}