Re: [PATCH v2 2/2] fetch-pack: warn if in commit graph but not obj db

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 1, 2024, 02:22 UTC
Message-ID: <xmqqikt74rs5.fsf@gitster.g>
In-Reply-To: <631b9a86778f15b7086e5f17fe850ffa151dd341.1730409376.git.jonathantanmy@google.com>

Jonathan Tan <jonathantanmy@google.com> writes:

Show 17 quoted lines

>  static struct commit *deref_without_lazy_fetch(const struct object_id *oid,
> -					       int mark_tags_complete)
> +					       int mark_tags_complete_and_check_obj_db)
>  {
>  	enum object_type type;
>  	struct object_info info = { .typep = &type };
>  	struct commit *commit;
>  
>  	commit = lookup_commit_in_graph(the_repository, oid);
> -	if (commit)
> +	if (commit) {
> +		if (mark_tags_complete_and_check_obj_db) {
> +			if (!has_object(the_repository, oid, 0))
> +				die_in_commit_graph_only(oid);
> +		}
>  		return commit;
> +	}

Hmph, even when we are not doing the mark-tags-complete thing, wouldn't it be a fatal error if the commit graph claims a commit exists but we are missing it?

It also makes me wonder if it would be sufficient to prevent us from saying "have X" if we just pretend as if lookup_commit_in_graph() returned NULL in this case.

In any case, infinitely recursing to lazily fetch a single commit is definitely worth fixing. Thanks for digging to the bottom of the problem and fixing it.