From: Toon Claes <toon@iotcl.com>
Date: Mon, 05 Jan 2026 14:29:02 GMT
Subject: Re: [PATCH v2 2/7] packfile: always declare object info to be OI_PACKED
Message-ID: <87seckp1zl.fsf@iotcl.com>
In-Reply-To: <20251218-b4-pks-odb-read-object-info-improvements-v2-2-62e3e49072bc@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

> When reading object info via a packfile we yield one of two types:
>
>   - The object can either be OI_PACKED, which is what a caller would
>     typically expect.
>
>   - Or it can be OI_DBCACHED if it is stored in the delta base cache.
>
> The latter really is an implementation detail though, and callers
> typically don't care at all about the difference. Furthermore, the
> information whether or not it is part of the delta base cache can
> already be derived via the `is_delta` field, so the fact that we discern
> between OI_PACKED and OI_DBCACHED only further complicates the
> interface.
>
> 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.

Thanks for clarifying these. I agree it makes sense to drop it.

-- 
Cheers,
Toon