From: Patrick Steinhardt Date: Thu, 18 Dec 2025 10:54:12 GMT Subject: [PATCH v2 0/7] Improvements for reading object info Message-ID: <20251218-b4-pks-odb-read-object-info-improvements-v2-0-62e3e49072bc@pks.im> In-Reply-To: <20251218-b4-pks-odb-read-object-info-improvements-v1-0-81c8368492be@pks.im> Hi, this patch series contains various small improvements for reading object info for either loose or packed objects. These improvements were split out of a larger patch series where I'm about to introduce a new generic `odb_for_each_object()` function. Changes in v2: - Rebase the series on top of master with jc/object-read-stream-fix merged into it. I've also evicted the patch that fixes the same underlying issue. - Improve the commit message that drops OI_DBCACHED to explain why this is a safe refactoring. - Link to v1: https://lore.kernel.org/r/20251218-b4-pks-odb-read-object-info-improvements-v1-0-81c8368492be@pks.im Thanks! Patrick --- Patrick Steinhardt (7): object-file: always set OI_LOOSE when reading object info packfile: always declare object info to be OI_PACKED packfile: extend `is_delta` field to allow for "unknown" state packfile: always populate pack-specific info when reading object info packfile: disentangle return value of `packed_object_info()` packfile: skip unpacking object header for disk size requests packfile: drop repository parameter from `packed_object_info()` builtin/cat-file.c | 3 +-- builtin/pack-objects.c | 4 ++-- commit-graph.c | 2 +- object-file.c | 19 ++++++++++++---- odb.h | 8 +++++-- pack-bitmap.c | 3 +-- packfile.c | 61 ++++++++++++++++++++++++++++++-------------------- packfile.h | 7 ++++-- 8 files changed, 68 insertions(+), 39 deletions(-) Range-diff versus v1: 1: 0c1a4a4745 < -: ---------- object-file: always set OI_LOOSE when reading object info -: ---------- > 1: 2287c0cbd9 object-file: always set OI_LOOSE when reading object info 2: 98962428cf ! 2: a1cd99af9c packfile: always declare object info to be OI_PACKED @@ Commit message between OI_PACKED and OI_DBCACHED only further complicates the interface. - Drop the OI_DBCACHED enum completely. There don't seem to be any callers - that care about the distinction. + There aren't all that many callers that care about the `whence` field in + the first place. In fact, there's only three: + + - `packfile_store_read_object_info()` checks for `whence == OI_PACKED` + and then populates the packfile information of the object info + structure. We now start to do this also for deltified objects, which + gives its callers strictly more information. + + - `repack_local_links()` wants to determine whether the object is part + of a promisor pack and checks for `whence == OI_PACKED`. If so, it + verifies that the packfile is a promisor pack. It's arguably wrong + to declare that an object is not part of a promisor pack only + because it is stored in the delta base cache. + + - `is_not_in_promisor_pack_obj()` does the same, but checks that a + specific object is _not_ part of a promisor pack. The same reasoning + as above applies. + + Drop the OI_DBCACHED enum completely. None of the callers seem to care + about the distinction. Signed-off-by: Patrick Steinhardt 3: 0a5b806934 = 3: 7a043c09ee packfile: extend `is_delta` field to allow for "unknown" state 4: 6a05c85683 ! 4: 448511cb19 packfile: always populate pack-specific info when reading object info @@ Metadata ## Commit message ## packfile: always populate pack-specific info when reading object info - When reading object information from a packfile we are not always - populating the pack-specific information. This happens in two cases: + When reading object information via `packed_object_info()` we may not + populate the object info's packfile-specific fields. This leads to + inconsistent object info depending on whether the info was populated via + `packfile_store_read_object_info()` or `packed_object_info()`. - - When calling `packed_object_info()` directly instead of - `packfile_store_read_object_info()`. - - - When we've got the empty request. - - Fix both of these issues so that we can always assume the pack info to - be populated when reading object info from a pack. - - Note that we don't really care about the second case right now, as the - condition will always evaluate to false anyway. This will be fixed in - the next commit. + Fix this inconsistecny so that we can always assume the pack info to be + populated when reading object info from a pack. Signed-off-by: Patrick Steinhardt @@ packfile.c: int packed_object_info(struct repository *r, struct packed_git *p, out: unuse_pack(&w_curs); -@@ packfile.c: int packfile_store_read_object_info(struct packfile_store *store, - * We know that the caller doesn't actually need the - * information below, so return early. - */ -- if (oi == &blank_oi) -+ if (oi == &blank_oi) { -+ oi->whence = OI_PACKED; -+ oi->u.packed.offset = e.offset; -+ oi->u.packed.pack = e.p; -+ oi->u.packed.type = PACKED_OBJECT_TYPE_UNKNOWN; - return 0; -+ } - - rtype = packed_object_info(store->odb->repo, e.p, e.offset, oi); - if (rtype < 0) { @@ packfile.c: int packfile_store_read_object_info(struct packfile_store *store, return -1; } 5: b09f37400c ! 5: e1ee6c7841 packfile: disentangle return value of `packed_object_info()` @@ packfile.c: int packed_object_info(struct repository *r, struct packed_git *p, static void *unpack_compressed_entry(struct packed_git *p, @@ packfile.c: int packfile_store_read_object_info(struct packfile_store *store, + unsigned flags UNUSED) { - static struct object_info blank_oi = OBJECT_INFO_INIT; struct pack_entry e; - int rtype; + int ret; @@ packfile.c: int packfile_store_read_object_info(struct packfile_store *store, if (!find_pack_entry(store->odb->repo, oid, &e)) return 1; @@ packfile.c: int packfile_store_read_object_info(struct packfile_store *store, + if (!oi) return 0; - } - rtype = packed_object_info(store->odb->repo, e.p, e.offset, oi); - if (rtype < 0) { 6: 253c0d47ab = 6: 77589e84b5 packfile: skip unpacking object header for disk size requests 7: 4dac51d4be < -: ---------- packfile: fix short-circuiting of empty requests 8: 2cf441de0d ! 7: 08f4b865e5 packfile: drop repository parameter from `packed_object_info()` @@ packfile.c: int packed_object_info(struct repository *r, struct packed_git *p, if (oi->typep) *oi->typep = ptot; @@ packfile.c: int packfile_store_read_object_info(struct packfile_store *store, + if (!oi) return 0; - } - ret = packed_object_info(store->odb->repo, e.p, e.offset, oi); + ret = packed_object_info(e.p, e.offset, oi); --- base-commit: 7df68b50e49b6a1b576abb19b2e5d457749bc28b change-id: 20251215-b4-pks-odb-read-object-info-improvements-0e031ef827d2