connected: search promisor objects generically

When performing connectivity checks we have to figure out whether any of
the new objects are promisor objects, as we cannot assume full
connectivity if so.

This check is performed by iterating through all packfiles in the
repository and searching each of them for the given object. Of course,
this mechanism is quite specific to implementation details of the object
database, as we assume that it uses packfiles in the first place.

Refactor the logic so that we instead use `odb_for_each_object_ext()`
with an object prefix filter and the `ODB_FOR_EACH_OBJECT_PROMISOR_ONLY`
flag. This will yield all objects that have the exact object name and
that are part of a promisor pack in a generic way.

Add a test to verify that we indeed use the optimization.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/connected.c b/connected.c
index d2b334173f0e767d8f517419cdc41251ec785d4d..929b9bd28d6fabb6aa91ad940e94a002327a6caf 100644 (file)
--- a/connected.c
+++ b/connected.c
@@ -11,6 +11,15 @@
 #include "packfile.h"
 #include "promisor-remote.h"
 
+static int promised_object_cb(const struct object_id *oid UNUSED,
+                             struct object_info *oi UNUSED,
+                             void *payload)
+{
+       bool *found = payload;
+       *found = true;
+       return 1;
+}
+
 /*
  * 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
@@ -30,25 +39,29 @@ static int check_connected_promisor(oid_iterate_fn fn,
                                    void *cb_data,
                                    const struct object_id **oid)
 {
+       struct odb_for_each_object_options opts = {
+               .flags = ODB_FOR_EACH_OBJECT_PROMISOR_ONLY,
+               .prefix_hex_len = the_repository->hash_algo->hexsz,
+       };
+       int err;
+
        odb_reprepare(the_repository->objects);
        do {
-               struct packed_git *p;
+               bool found = false;
 
-               repo_for_each_pack(the_repository, p) {
-                       if (!p->pack_promisor)
-                               continue;
-                       if (find_pack_entry_one(*oid, p))
-                               goto promisor_pack_found;
-               }
+               opts.prefix = *oid;
+
+               err = odb_for_each_object_ext(the_repository->objects, NULL,
+                                             promised_object_cb, &found, &opts);
+               if (err < 0)
+                       return err;
 
                /*
                 * We have found an object that is not part of a promisor pack,
                 * and thus we cannot skip the full connectivity check.
                 */
-               return 0;
-
-promisor_pack_found:
-               ;
+               if (!found)
+                       return 0;
        } while ((*oid = fn(cb_data)) != NULL);
 
        return 1;

diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh
index 1c2805accac63691d0e18919e71cf29037b9cb0c..905052072db7c1a9e07301246402f57e36418bb2 100755 (executable)
--- a/t/t5616-partial-clone.sh
+++ b/t/t5616-partial-clone.sh
@@ -97,6 +97,30 @@ test_expect_success 'partial fetch inherits filter settings' '
        test_line_count = 5 observed
 '
 
+test_expect_success 'partial fetch does not spawn rev-list connectivity check' '
+       test_when_finished "rm -rf connectivity-remote connectivity-client" &&
+       git init connectivity-remote &&
+       test_commit -C connectivity-remote one &&
+       git -C connectivity-remote config uploadpack.allowfilter 1 &&
+       git -C connectivity-remote config uploadpack.allowanysha1inwant 1 &&
+
+       git clone --no-checkout --filter=blob:none \
+               "file://$(pwd)/connectivity-remote" connectivity-client &&
+
+       # When doing a partial fetch where all tips are part of a promisor pack
+       # we want to skip the connectivity check, as these objects are allowed
+       # to not be fully connected.
+       test_commit -C connectivity-remote two &&
+       GIT_TRACE2_EVENT="$(pwd)/partial.trace" git -C connectivity-client fetch origin &&
+       test_subcommand_flex ! git rev-list --objects --stdin <partial.trace &&
+
+       # Otherwise, when doing a fetch where any of the tips is not part of a
+       # promisor pack, then we must run the connectivity check.
+       test_commit -C connectivity-remote three &&
+       GIT_TRACE2_EVENT="$(pwd)/full.trace" git -C connectivity-client fetch --no-filter origin &&
+       test_subcommand_flex git rev-list --objects --stdin <full.trace
+'
+
 # force dynamic object fetch using diff.
 # we should only get 1 new blob (for the file in origin/main).
 test_expect_success 'verify diff causes dynamic object fetch' '