Re: [RFC PATCH 0/2] Re: An endless loop fetching issue with partial clone, alternates and commit graph

[Jonathan Tan <jonathantanmy@google.com>]

Han Xin <hanxin.hx@bytedance.com> writes:
> On Wed, Jun 15, 2022 at 10:18 AM Taylor Blau <me@ttaylorr.com> wrote:
> >
> > [+cc Stolee]
> >
> > On Tue, Jun 14, 2022 at 03:25:13PM +0800, Haiyng Tan wrote:
> > > I think it's caused by using lazy-fetch in
> > > deref_without_lazy_fetch_extended().  In lookup_commit_in_graph(),
> > > lazy-fetch is initiated by repo_has_object_file() used.  has_object()
> > > should be used, it's no-lazy-fetch.
> >
> > Hmm. Are there cases where lookup_commit_in_graph() is expected to
> > lazily fetch missing objects from promisor remotes? If so, then this
> > wouldn't quite work. If not, then this seems like an appropriate fix to
> > me.
> >
> > Thanks,
> > Taylor

I think that if a commit is in the commit graph, we would expect the
commit to also be present. So changing to has_object() makes sense.

> We can see the use of has_object() in RelNotes/2.29.0.txt[1]：
>    * A new helper function has_object() has been introduced to make it
>      easier to mark object existence checks that do and don't want to
>      trigger lazy fetches, and a few such checks are converted using it.

Also relevant is the comment on repo_has_object_file() in
object-store.h.

> So, an appropriate fix can be that let lookup_commit_in_graph() pickup
> oi_flags and pass it to oid_object_info_extended(), then the fetching
> loop will be prevent by the given flag OBJECT_INFO_SKIP_FETCH_OBJECT.

Hmm...why not change it to has_object_file() instead, as Haiyng Tan
mentioned?