[PATCH v2 0/7] Improvements for reading object info

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 <ps@pks.im>
     
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 <ps@pks.im>
     
    @@ 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