Subject: Re: [PATCH v2 1/3] object-file: don't exit early if skipping loose

From: Junio C Hamano <[email-masked]>
Date: Wed, 07 Dec 2022 10:12:13 +0900

Jonathan Tan <[email-masked]> writes:

> Instead, also search the submodule object stores and promisor remotes.
>
> This also centralizes what happens when the object is not found (the
> "return -1"), which is useful for a subsequent patch.
>
> Signed-off-by: Jonathan Tan <[email-masked]>
> ---
>  object-file.c | 23 +++++++++++------------
>  1 file changed, 11 insertions(+), 12 deletions(-)
>
> diff --git a/object-file.c b/object-file.c
> index 26290554bb..596dd049fd 100644
> --- a/object-file.c
> +++ b/object-file.c
> @@ -1575,18 +1575,17 @@ static int do_oid_object_info_extended(struct repository *r,
>  		if (find_pack_entry(r, real, &e))
>  			break;
>  
> -		if (flags & OBJECT_INFO_IGNORE_LOOSE)
> -			return -1;
> -
> -		/* Most likely it's a loose object. */
> -		if (!loose_object_info(r, real, oi, flags))
> -			return 0;
> -
> -		/* Not a loose object; someone else may have just packed it. */
> -		if (!(flags & OBJECT_INFO_QUICK)) {
> -			reprepare_packed_git(r);
> -			if (find_pack_entry(r, real, &e))
> -				break;
> +		if (!(flags & OBJECT_INFO_IGNORE_LOOSE)) {
> +			/* Most likely it's a loose object. */
> +			if (!loose_object_info(r, real, oi, flags))
> +				return 0;
> +
> +			/* Not a loose object; someone else may have just packed it. */
> +			if (!(flags & OBJECT_INFO_QUICK)) {
> +				reprepare_packed_git(r);
> +				if (find_pack_entry(r, real, &e))
> +					break;
> +			}
>  		}

Hmph, who passes IGNORE_LOOSE and why?  Explaining the answer to
that question would give us confidence why this change is safe.

If the reason IGNORE_LOOSE is set by the callers is because they are
interested only in locally packed objects, then this change would
break them because they end up triggering the lazy fetch in the
updated code, no?  Or do all callers that set IGNORE_LOOSE drop the
fetch_if_missing global before calling us?

Thanks.