Re: [External] Re: Missing Promisor Objects in Partial Repo Design Doc

Jonathan Tan <jonathantanmy@google.com> writes:

I had time to take a closer look at this solution. One problem that I've noticed is that the "best effort" promisor object check cannot naively replace is_promisor_object(), because a lot of the time (e.g. in revision.c's get_reference()) is_promisor_object() is used when an object is missing to check whether we need to error out or not. Our "best effort" promisor object check cannot replace this because it needs us to have looked up the object in the first place to check whether it's loose or packed (and if packed, which packfile it's in), so it can't work with an object that's missing.

So I think we'll need to use do_not_die_on_missing_objects. It does have the weakness that if the object is not supposed to be missing, we don't inform the user, but perhaps this is OK here because we know that all objects we encounter on this object walk are promisor objects, so if it's missing, it's OK.

In addition to do_not_die_on_missing_objects, we'll also need the actual code that stops iteration through objects that pass our "best effort" promisor object check. Probably the best place is in get_revision_1() after the NULL check, but I haven't fully thought through what happens if this option is used when some commits are UNINTERESTING. (For the repack-on-fetch, no commits are UNINTERESTING, but it's probably best to make sure our feature is as useful in as many cases as possible, especially since we're going to further complicate revision walking code, which is complicated enough.