From: Patrick Steinhardt <ps@pks.im>
Date: Mon, 15 Dec 2025 05:33:20 GMT
Subject: Re: [PATCH v2 4/7] builtin/repo: add inflated object info to keyvalue structure output
Message-ID: <aT-doNe94GYmodQl@pks.im>
In-Reply-To: <20251212223644.3090879-5-jltobler@gmail.com>

On Fri, Dec 12, 2025 at 04:36:41PM -0600, Justin Tobler wrote:
> diff --git a/builtin/repo.c b/builtin/repo.c
> index d3dfe416d0..3a2d15cec4 100644
> --- a/builtin/repo.c
> +++ b/builtin/repo.c
> @@ -500,20 +513,38 @@ static int count_objects(const char *path UNUSED, struct oid_array *oids,
>  {
>  	struct count_objects_data *data = cb_data;
>  	struct object_stats *stats = data->stats;
> +	size_t inflated_total = 0;
>  	size_t object_count;
>  
> +	for (size_t i = 0; i < oids->nr; i++) {
> +		struct object_info oi = OBJECT_INFO_INIT;
> +		unsigned long inflated;
> +
> +		oi.sizep = &inflated;
> +
> +		if (odb_read_object_info_extended(data->odb, &oids->oid[i], &oi,
> +						  OBJECT_INFO_FOR_PREFETCH) < 0)

Using `OBJECT_INFO_FOR_PREFETCH` feels a bit weird to me, as we're not
in a context where we want to do a prefetch. And if we ever were to
extend that flag to have more semantics that are relevant to prefetches,
only, then this code here might become broken.

Using `SKIP_FETCH_OBJECT | INFO_QUICK` does make sense though, so I'd
suggest to expand the flag here.

Patrick