Re: [PATCH v3 4/4] commit: don't lazy-fetch commits

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Thu, Dec 08 2022, Jonathan Tan wrote:


> diff --git a/commit.c b/commit.c
> index 572301b80a..a02723f06b 100644
> --- a/commit.c
> +++ b/commit.c
> @@ -508,6 +508,17 @@ int repo_parse_commit_internal(struct repository *r,
>  	enum object_type type;
>  	void *buffer;
>  	unsigned long size;
> +	struct object_info oi = {
> +		.typep = &type,
> +		.sizep = &size,
> +		.contentp = &buffer,
> +	};
> +	/*
> +	 * Git does not support partial clones that exclude commits, so set
> +	 * OBJECT_INFO_SKIP_FETCH_OBJECT to fail fast when an object is missing.
> +	 */
> +	int flags = OBJECT_INFO_LOOKUP_REPLACE | OBJECT_INFO_SKIP_FETCH_OBJECT |
> +		OBJECT_INFO_DIE_IF_CORRUPT;
>  	int ret;
>  
>  	if (!item)
> @@ -516,8 +527,8 @@ int repo_parse_commit_internal(struct repository *r,
>  		return 0;
>  	if (use_commit_graph && parse_commit_in_graph(r, item))
>  		return 0;
> -	buffer = repo_read_object_file(r, &item->object.oid, &type, &size);
> -	if (!buffer)
> +
> +	if (oid_object_info_extended(r, &item->object.oid, &oi, flags) < 0)

Style: you're adding another \n here, usually we'd prefer it, but here
the function already has all these checks bundled together without a \n,
and this "if" is followed by one without it.

But then again those two "if"'s have to do with populating the "oi" and
then reading out the "type", so it's probably fine & OK.