Merge branch 'ps/connected-generic-promisor-checks'

The connectivity check has been refactored to search for promisor
objects in a generic way using the object database interface,
rather than iterating packfiles directly. This allows connectivity
checks to work properly in repositories that do not use packfiles.

* ps/connected-generic-promisor-checks:
  connected: search promisor objects generically
  connected: split out promisor-based connectivity check
  odb/source-packed: support flags when iterating an object prefix
  odb/source-packed: extract logic to skip certain packs
main
Junio C Hamano 2026-07-06 15:50:24 -07:00
commit a4d2e8a074
3 changed files with 125 additions and 47 deletions

View File

@ -11,6 +11,62 @@
#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
* 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, in a promisor packfile, the objects pointed to by each wanted ref.
*
* Before checking for promisor packs, be sure we have the latest pack-files
* loaded into memory.
*
* Returns 1 when all object IDs have been found in promisor packs, in which
* case we're fully connected and thus done. Returns 0 when we have found
* objects in non-promisor packs, in which case we'll have to fall back to the
* rev-list-based connectivity checks. Returns a negative error code on error.
*/
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 {
bool found = false;

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.
*/
if (!found)
return 0;
} while ((*oid = fn(cb_data)) != NULL);

return 1;
}

/*
* If we feed all the commits we want to verify to this command
*
@ -46,42 +102,16 @@ int check_connected(oid_iterate_fn fn, void *cb_data,
}

if (repo_has_promisor_remote(the_repository)) {
/*
* 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, in a promisor packfile, the objects pointed to by
* each wanted ref.
*
* Before checking for promisor packs, be sure we have the
* latest pack-files loaded into memory.
*/
odb_reprepare(the_repository->objects);
do {
struct packed_git *p;

repo_for_each_pack(the_repository, p) {
if (!p->pack_promisor)
continue;
if (find_pack_entry_one(oid, p))
goto promisor_pack_found;
}
/*
* Fallback to rev-list with oid and the rest of the
* object IDs provided by fn.
*/
goto no_promisor_pack_found;
promisor_pack_found:
;
} while ((oid = fn(cb_data)) != NULL);
if (opt->err_fd)
close(opt->err_fd);
return 0;
err = check_connected_promisor(fn, cb_data, &oid);
if (err) {
if (opt->err_fd)
close(opt->err_fd);
if (err > 0)
err = 0;
return err;
}
}

no_promisor_pack_found:
if (opt->shallow_file) {
strvec_push(&rev_list.args, "--shallow-file");
strvec_push(&rev_list.args, opt->shallow_file);

View File

@ -126,12 +126,29 @@ static int match_hash(unsigned len, const unsigned char *a, const unsigned char
return 1;
}

static bool should_exclude_pack(struct packed_git *p, enum odb_for_each_object_flags flags)
{
if ((flags & ODB_FOR_EACH_OBJECT_LOCAL_ONLY) && !p->pack_local)
return true;
if ((flags & ODB_FOR_EACH_OBJECT_PROMISOR_ONLY) &&
!p->pack_promisor)
return true;
if ((flags & ODB_FOR_EACH_OBJECT_SKIP_IN_CORE_KEPT_PACKS) &&
p->pack_keep_in_core)
return true;
if ((flags & ODB_FOR_EACH_OBJECT_SKIP_ON_DISK_KEPT_PACKS) &&
p->pack_keep)
return true;
return false;
}

static int for_each_prefixed_object_in_midx(
struct odb_source_packed *store,
struct multi_pack_index *m,
const struct odb_for_each_object_options *opts,
struct odb_source_packed_for_each_object_wrapper_data *data)
{
bool pack_errors = false;
int ret;

for (; m; m = m->base_midx) {
@ -160,6 +177,20 @@ static int for_each_prefixed_object_in_midx(
if (!match_hash(len, opts->prefix->hash, current->hash))
break;

if (opts->flags) {
uint32_t pack_id = nth_midxed_pack_int_id(m, i);
struct packed_git *pack;

if (prepare_midx_pack(m, pack_id)) {
pack_errors = true;
continue;
}

pack = nth_midxed_pack(m, pack_id);
if (should_exclude_pack(pack, opts->flags))
continue;
}

if (data->request) {
struct object_info oi = *data->request;

@ -182,6 +213,8 @@ static int for_each_prefixed_object_in_midx(
ret = 0;

out:
if (!ret && pack_errors)
ret = -1;
return ret;
}

@ -244,9 +277,6 @@ static int odb_source_packed_for_each_prefixed_object(
bool pack_errors = false;
int ret;

if (opts->flags)
BUG("flags unsupported");

store->skip_mru_updates = true;

m = get_multi_pack_index(store);
@ -259,6 +289,8 @@ static int odb_source_packed_for_each_prefixed_object(
for (e = packfile_store_get_packs(store); e; e = e->next) {
if (e->pack->multi_pack_index)
continue;
if (should_exclude_pack(e->pack, opts->flags))
continue;

if (open_pack_index(e->pack)) {
pack_errors = true;
@ -306,17 +338,9 @@ static int odb_source_packed_for_each_object(struct odb_source *source,
for (e = packfile_store_get_packs(packed); e; e = e->next) {
struct packed_git *p = e->pack;

if ((opts->flags & ODB_FOR_EACH_OBJECT_LOCAL_ONLY) && !p->pack_local)
continue;
if ((opts->flags & ODB_FOR_EACH_OBJECT_PROMISOR_ONLY) &&
!p->pack_promisor)
continue;
if ((opts->flags & ODB_FOR_EACH_OBJECT_SKIP_IN_CORE_KEPT_PACKS) &&
p->pack_keep_in_core)
continue;
if ((opts->flags & ODB_FOR_EACH_OBJECT_SKIP_ON_DISK_KEPT_PACKS) &&
p->pack_keep)
if (should_exclude_pack(p, opts->flags))
continue;

if (open_pack_index(p)) {
pack_errors = 1;
continue;

View File

@ -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' '