Re: [PATCH v2 2/7] packfile: always declare object info to be OI_PACKED
Patrick Steinhardt <ps@pks.im> writes:
Show 34 quoted lines
> 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