From: Karthik Nayak <karthik.188@gmail.com>
Date: Wed, 07 Jan 2026 10:12:04 GMT
Subject: Re: [PATCH v3 3/7] packfile: extend `is_delta` field to allow for "unknown" state
Message-ID: <CAOLa=ZQ0wYjDiYYgsiR=p4rM0SCgjwhcub_j0vz5kVWhzqzMWA@mail.gmail.com>
In-Reply-To: <20260106-b4-pks-odb-read-object-info-improvements-v3-3-b5e02fae1fb0@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

> The `struct object_info::u::packed::is_delta` field determines whether
> or not a specific object is stored as a delta. It only stores whether or
> not the object is stored as delta, so it is treated as a boolean value.
>
> This boolean is insufficient though: when reading a packed object via
> `packfile_store_read_object_info()` we know to skip parsing the actual
> object when the user didn't request any object-specific data. In that
> case we won't read the object itself, but will only look up its position
> in the packfile. Consequently, we do not know whether it is a delta or
> not.
>
> This isn't really an issue right now, as the check for an empty request
> is broken. But a subsequent commit will fix it, and once we do we will
> have the need to also represent an "unknown" delta state.
>
> Prepare for this change by introducing a new enum that encodes the
> object type. We don't use the "unknown" state just yet, but will start
> to do so in a subsequent commit.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  odb.h      |  7 ++++++-
>  packfile.c | 17 ++++++++++++++---
>  2 files changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/odb.h b/odb.h
> index 73b0b87ad5..afae5e5c01 100644
> --- a/odb.h
> +++ b/odb.h
> @@ -343,7 +343,12 @@ struct object_info {
>  		struct {
>  			struct packed_git *pack;
>  			off_t offset;
> -			unsigned int is_delta;
> +			enum packed_object_type {
> +				PACKED_OBJECT_TYPE_UNKNOWN,
> +				PACKED_OBJECT_TYPE_FULL,
> +				PACKED_OBJECT_TYPE_OFS_DELTA,
> +				PACKED_OBJECT_TYPE_REF_DELTA,
> +			} type;
>  		} packed;
>  	} u;
>  };
> diff --git a/packfile.c b/packfile.c
> index b0c6665c87..cc797b2b6a 100644
> --- a/packfile.c
> +++ b/packfile.c
> @@ -2159,8 +2159,18 @@ int packfile_store_read_object_info(struct packfile_store *store,
>  	if (oi->whence == OI_PACKED) {
>  		oi->u.packed.offset = e.offset;
>  		oi->u.packed.pack = e.p;
> -		oi->u.packed.is_delta = (rtype == OBJ_REF_DELTA ||
> -					 rtype == OBJ_OFS_DELTA);
> +
> +		switch (rtype) {
> +		case OBJ_REF_DELTA:
> +			oi->u.packed.type = PACKED_OBJECT_TYPE_REF_DELTA;
> +			break;
> +		case OBJ_OFS_DELTA:
> +			oi->u.packed.type = PACKED_OBJECT_TYPE_OFS_DELTA;
> +			break;
> +		default:
> +			oi->u.packed.type = PACKED_OBJECT_TYPE_FULL;
> +			break;
> +		}
>  	}
>

So we get `rtype` from `packed_object_info()` which can return OBJ_BAD,
but return early in such a scenario. So overall this makes sense. I like
that we are now storing more and clearer information.

[snip]