Jonathan Tan wrote: > Commit dfa33a298d ("clone: do faster object check for partial clones", > 2019-04-21) optimized the connectivity check done when cloning with > --filter to check only the existence of objects directly pointed to by > refs. But this is not sufficient: they also need to be promisor objects. > Make this check more robust by instead checking that these objects are > promisor objects, that is, they appear in a promisor pack. > > Signed-off-by: Jonathan Tan <jonathantanmy@xxxxxxxxxx> > --- > builtin/clone.c | 5 +++-- > connected.c | 19 ++++++++++++++----- > connected.h | 11 ++++++----- > 3 files changed, 23 insertions(+), 12 deletions(-) Good call. Sorry for the slow review. [...] > --- a/connected.c > +++ b/connected.c > @@ -52,19 +52,28 @@ int check_connected(oid_iterate_fn fn, void *cb_data, > strbuf_release(&idx_file); > } > > - if (opt->check_refs_only) { > + if (opt->check_refs_are_promisor_objects_only) { > /* > * For partial clones, we don't want to have to do a regular > * connectivity check because we have to enumerate and exclude > * all promisor objects (slow), and then the connectivity check > * itself becomes a no-op because in a partial clone every > * object is a promisor object. Instead, just make sure we > - * received the objects pointed to by each wanted ref. > + * received, in a promisor packfile, the objects pointed to by > + * each wanted ref. > */ > do { > - if (!repo_has_object_file_with_flags(the_repository, &oid, > - OBJECT_INFO_SKIP_FETCH_OBJECT)) > - return 1; > + struct packed_git *p; > + > + for (p = get_all_packs(the_repository); p; p = p->next) { > + if (!p->pack_promisor) > + continue; > + if (find_pack_entry_one(oid.hash, p)) > + goto promisor_pack_found; > + } > + return 1; > +promisor_pack_found: > + ; > } while (!fn(cb_data, &oid)); > return 0; Yep, does what it says on the tin. Reviewed-by: Jonathan Nieder <jrnieder@xxxxxxxxx>