From 9e7a2fb9cef08fdb4056eb76951a52e20ca350c5 Mon Sep 17 00:00:00 2001 From: Kristofer Karlsson Date: Thu, 27 Aug 2026 21:14:38 +0200 Subject: [PATCH 1/4] connected: extract get_self_contained_pack() helper Move the inline self-contained pack detection into a helper function. This makes check_connected() easier to follow and makes the detection logic available as a standalone helper. No functional change. Signed-off-by: Kristofer Karlsson --- connected.c | 45 +++++++++++++++++++++++++++++++-------------- 1 file changed, 31 insertions(+), 14 deletions(-) diff --git a/connected.c b/connected.c index 929b9bd28d6fab..e62a25db323a49 100644 --- a/connected.c +++ b/connected.c @@ -7,6 +7,7 @@ #include "run-command.h" #include "sigchain.h" #include "connected.h" +#include "strbuf.h" #include "transport.h" #include "packfile.h" #include "promisor-remote.h" @@ -67,6 +68,35 @@ static int check_connected_promisor(oid_iterate_fn fn, return 1; } +/* + * If index-pack already verified that the new pack is self-contained + * (no dangling pointers), return the pack so tips found in it can + * skip connectivity checking. + */ +static struct packed_git *get_self_contained_pack(struct transport *transport) +{ + size_t base_len; + + if (transport && transport->smart_options && + transport->smart_options->self_contained_and_connected && + transport->pack_lockfiles.nr == 1 && + strip_suffix(transport->pack_lockfiles.items[0].string, + ".keep", &base_len)) { + struct strbuf idx_file = STRBUF_INIT; + struct packed_git *pack; + + strbuf_add(&idx_file, + transport->pack_lockfiles.items[0].string, + base_len); + strbuf_addstr(&idx_file, ".idx"); + pack = add_packed_git(the_repository, idx_file.buf, + idx_file.len, 1); + strbuf_release(&idx_file); + return pack; + } + return NULL; +} + /* * If we feed all the commits we want to verify to this command * @@ -88,7 +118,6 @@ int check_connected(oid_iterate_fn fn, void *cb_data, int err = 0; struct packed_git *new_pack = NULL; struct transport *transport; - size_t base_len; if (!opt) opt = &defaults; @@ -151,19 +180,7 @@ int check_connected(oid_iterate_fn fn, void *cb_data, rev_list_in = xfdopen(rev_list.in, "w"); - if (transport && transport->smart_options && - transport->smart_options->self_contained_and_connected && - transport->pack_lockfiles.nr == 1 && - strip_suffix(transport->pack_lockfiles.items[0].string, - ".keep", &base_len)) { - struct strbuf idx_file = STRBUF_INIT; - strbuf_add(&idx_file, transport->pack_lockfiles.items[0].string, - base_len); - strbuf_addstr(&idx_file, ".idx"); - new_pack = add_packed_git(the_repository, idx_file.buf, - idx_file.len, 1); - strbuf_release(&idx_file); - } + new_pack = get_self_contained_pack(transport); do { /* From 6f3a9d66f9603dfd8175768460b621ebcd15e56a Mon Sep 17 00:00:00 2001 From: Kristofer Karlsson Date: Fri, 28 Aug 2026 08:13:23 +0200 Subject: [PATCH 2/4] connected: add incremental connectivity check The connectivity check walks objects reachable from incoming tips using rev-list. mark_edges_uninteresting() prunes at commit boundaries, but the traversal still visits the full tree closure of each boundary commit to classify every entry. For repositories with large trees, this is expensive. Add an alternative that verifies incoming commits by diffing their trees against parent trees, recursively descending only into entries whose OIDs have changed. Use the Merkle tree property -- an unchanged tree entry proves the entire subtree is intact -- to skip subtrees that already exist in the repository. Cross-directory moves may be reverified rather than recognized by the parent-tree comparison, but correctness is preserved. The algorithm works in three phases: 1. Collect and peel tips: consume the tip iterator, peel tags, and verify non-commit objects immediately. Tips found in a self-contained pack (verified by index-pack) are skipped. 2. Find boundary: feed commit tips to rev-list --stdin --not --all to identify new commits not yet reachable from local refs. 3. Verify trees: walk new commits in topological order. For each commit, diff its tree against parent trees. Unchanged entries are skipped (Merkle property). The verified set persists across commits so subtrees seen in earlier commits (change-then-revert, subtree moves, merges) are not re-walked. Gate the new algorithm behind transfer.connectivityCheck=incremental. Fall back to rev-list for shallow fetches, partial clones, replacement objects, and deepening fetches. Signed-off-by: Kristofer Karlsson --- Documentation/config/transfer.adoc | 22 + Makefile | 1 + connected.c | 617 +++++++++++++++++++++++++++++ t/helper/meson.build | 1 + t/helper/test-check-connected.c | 53 +++ t/helper/test-tool.c | 1 + t/helper/test-tool.h | 1 + t/meson.build | 1 + t/t5412-connectivity-check.sh | 512 ++++++++++++++++++++++++ 9 files changed, 1209 insertions(+) create mode 100644 t/helper/test-check-connected.c create mode 100755 t/t5412-connectivity-check.sh diff --git a/Documentation/config/transfer.adoc b/Documentation/config/transfer.adoc index f1ce50f4a6e6ba..91891d02b751fc 100644 --- a/Documentation/config/transfer.adoc +++ b/Documentation/config/transfer.adoc @@ -1,3 +1,25 @@ +transfer.connectivityCheck:: + Choose which algorithm to use for the connectivity check + performed during object transfer operations such as + linkgit:git-fetch[1] and linkgit:git-receive-pack[1]. + The connectivity check verifies that all objects reachable + from the incoming tips are available locally or, in a partial + clone, promised by a promisor remote. + The variants are as follows: ++ +-- +`rev-list` (default);; + Delegate to `rev-list --objects --not --all`. This walks + the full object closure of the boundary commits. +`incremental`;; + Verify incoming commits by diffing their trees against parent + trees, recursively descending only into entries that differ. + The largest benefits occur when incoming commits change a + small fraction of a large tree closure. + Falls back to `rev-list` for shallow fetches, partial + clones, replacement objects, or deepening fetches. +-- + transfer.credentialsInUrl:: A configured URL can contain plaintext credentials in the form `://:@/`. You may want diff --git a/Makefile b/Makefile index d4b775953d3842..1782bd2b123b65 100644 --- a/Makefile +++ b/Makefile @@ -812,6 +812,7 @@ TEST_BUILTINS_OBJS += test-bitmap.o TEST_BUILTINS_OBJS += test-bloom.o TEST_BUILTINS_OBJS += test-bundle-uri.o TEST_BUILTINS_OBJS += test-cache-tree.o +TEST_BUILTINS_OBJS += test-check-connected.o TEST_BUILTINS_OBJS += test-chmtime.o TEST_BUILTINS_OBJS += test-config.o TEST_BUILTINS_OBJS += test-crontab.o diff --git a/connected.c b/connected.c index e62a25db323a49..c725aa90572fd2 100644 --- a/connected.c +++ b/connected.c @@ -1,16 +1,24 @@ #define USE_THE_REPOSITORY_VARIABLE #include "git-compat-util.h" +#include "commit.h" +#include "config.h" #include "gettext.h" #include "hex.h" #include "odb.h" +#include "oid-array.h" +#include "replace-object.h" #include "run-command.h" #include "sigchain.h" #include "connected.h" #include "strbuf.h" +#include "tag.h" +#include "trace2.h" #include "transport.h" #include "packfile.h" #include "promisor-remote.h" +#include "tree-walk.h" +#include "tree.h" static int promised_object_cb(const struct object_id *oid UNUSED, struct object_info *oi UNUSED, @@ -97,6 +105,612 @@ static struct packed_git *get_self_contained_pack(struct transport *transport) return NULL; } +/* + * Incremental connectivity verification. + * + * Instead of a full rev-list --objects traversal, verify each new + * commit's tree by walking its entries and skipping any that were + * previously verified: + * + * - Before walking a commit's tree, the top-level entries from each + * direct parent tree are added to the verified set. Matching + * entries in the child are skipped (Merkle property: an unchanged + * OID proves the entire subtree is intact). + * - Only new or changed entries cause recursive descent and blob + * verification. + * - The verified set persists across commits. This is safe because + * object contents are immutable: a tree OID verified reachable and + * intact in one commit's context remains so in every other. This + * lets us skip subtrees seen in earlier commits (change-then-revert, + * subtree moves, merges). + */ + +struct loaded_tree { + struct tree_desc desc; + void *buf; +}; + +static int read_tree_nofetch(struct loaded_tree *pt, + const struct object_id *oid, + enum object_type *actual_type) +{ + enum object_type type; + size_t size; + struct object_info oi = OBJECT_INFO_INIT; + + oi.typep = &type; + oi.sizep = &size; + oi.contentp = &pt->buf; + if (odb_read_object_info_extended(the_repository->objects, oid, &oi, + OBJECT_INFO_SKIP_FETCH_OBJECT | + OBJECT_INFO_DIE_IF_CORRUPT | + OBJECT_INFO_LOOKUP_REPLACE) < 0) { + if (actual_type) + *actual_type = OBJ_NONE; + return -1; + } + if (type != OBJ_TREE) { + FREE_AND_NULL(pt->buf); + if (actual_type) + *actual_type = type; + return -1; + } + init_tree_desc(&pt->desc, oid, pt->buf, size); + return 0; +} + +static int tree_seek(struct tree_desc *desc, + const struct name_entry *want) +{ + while (desc->size) { + const struct name_entry *have = &desc->entry; + int cmp = base_name_compare( + have->path, have->pathlen, have->mode, + want->path, want->pathlen, want->mode); + if (cmp > 0) + return 0; + if (cmp == 0) + return 1; + update_tree_entry(desc); + } + return 0; +} + +struct verify_state { + struct oidset verified_trees; + struct oidset verified_blobs; + int trees_walked; + int blobs_checked; + int err_fd; + int quiet; +}; + +__attribute__((format (printf, 2, 3))) +static void verify_error(struct verify_state *vs, const char *fmt, ...) +{ + va_list ap; + struct strbuf buf = STRBUF_INIT; + + if (vs->quiet && !vs->err_fd) + return; + + if (vs->err_fd) { + strbuf_addstr(&buf, "error: "); + va_start(ap, fmt); + strbuf_vaddf(&buf, fmt, ap); + va_end(ap); + strbuf_addch(&buf, '\n'); + sigchain_push(SIGPIPE, SIG_IGN); + write_in_full(vs->err_fd, buf.buf, buf.len); + sigchain_pop(SIGPIPE); + } else { + va_start(ap, fmt); + strbuf_vaddf(&buf, fmt, ap); + va_end(ap); + error("%s", buf.buf); + } + strbuf_release(&buf); +} + +static int verify_tree(const struct object_id *new_tree_oid, + const struct oid_array *base_trees, + struct verify_state *vs, int depth); + +static int verify_subtree(const struct name_entry *entry, + struct loaded_tree *parents, + size_t nr_parents, + struct verify_state *vs, int depth) +{ + struct oid_array sub_bases = OID_ARRAY_INIT; + size_t i; + int ret; + + /* + * tree_seek() advances each parent desc destructively. + * This is safe because both sides are in canonical sort + * order and verify_tree() calls us in that same order. + */ + for (i = 0; i < nr_parents; i++) { + if (!tree_seek(&parents[i].desc, entry)) + continue; + if (S_ISDIR(parents[i].desc.entry.mode)) + oid_array_append(&sub_bases, + &parents[i].desc.entry.oid); + } + + ret = verify_tree(&entry->oid, &sub_bases, vs, depth + 1); + oid_array_clear(&sub_bases); + return ret; +} + +static int verify_tree(const struct object_id *new_tree_oid, + const struct oid_array *base_trees, + struct verify_state *vs, int depth) +{ + struct loaded_tree new_tree = { 0 }; + struct loaded_tree *parents = NULL; + struct tree_desc scan; + struct name_entry entry, scan_entry; + enum object_type type; + struct object_info oi = OBJECT_INFO_INIT; + size_t nr_parents = 0; + size_t i; + int ret = 0; + + if (depth > the_repository->settings.max_allowed_tree_depth) { + verify_error(vs, _("exceeded maximum allowed tree depth")); + return -1; + } + + if (oidset_contains(&vs->verified_trees, new_tree_oid)) + return 0; + + if (read_tree_nofetch(&new_tree, new_tree_oid, &type)) { + if (type != OBJ_NONE) + verify_error(vs, _("object %s is a %s, not a tree"), + oid_to_hex(new_tree_oid), + type_name(type)); + else + verify_error(vs, _("bad tree object %s"), + oid_to_hex(new_tree_oid)); + return -1; + } + vs->trees_walked++; + + if (base_trees->nr) + CALLOC_ARRAY(parents, base_trees->nr); + for (i = 0; i < base_trees->nr; i++) { + if (read_tree_nofetch(&parents[nr_parents], &base_trees->oid[i], NULL)) + continue; + scan = parents[nr_parents].desc; + while (tree_entry(&scan, &scan_entry)) { + if (S_ISGITLINK(scan_entry.mode)) + continue; + if (S_ISDIR(scan_entry.mode)) + oidset_insert(&vs->verified_trees, &scan_entry.oid); + else + oidset_insert(&vs->verified_blobs, &scan_entry.oid); + } + nr_parents++; + } + + while (tree_entry(&new_tree.desc, &entry)) { + if (S_ISGITLINK(entry.mode)) + continue; + + if (S_ISDIR(entry.mode)) { + if (oidset_contains(&vs->verified_trees, &entry.oid)) + continue; + ret = verify_subtree(&entry, parents, nr_parents, + vs, depth); + if (ret) + break; + oidset_insert(&vs->verified_trees, &entry.oid); + continue; + } + + if (oidset_contains(&vs->verified_blobs, &entry.oid)) + continue; + vs->blobs_checked++; + + oi.typep = &type; + if (odb_read_object_info_extended( + the_repository->objects, &entry.oid, &oi, + OBJECT_INFO_SKIP_FETCH_OBJECT | OBJECT_INFO_LOOKUP_REPLACE) < 0) { + verify_error(vs, _("missing blob object '%s'"), + oid_to_hex(&entry.oid)); + ret = -1; + break; + } + if (type != OBJ_BLOB) { + verify_error(vs, _("object %s is a %s, not a blob"), + oid_to_hex(&entry.oid), + type_name(type)); + ret = -1; + break; + } + oidset_insert(&vs->verified_blobs, &entry.oid); + } + + if (!ret) + oidset_insert(&vs->verified_trees, new_tree_oid); + free(new_tree.buf); + for (i = 0; i < nr_parents; i++) + free(parents[i].buf); + free(parents); + return ret; +} + +static int read_tag_target_nofetch(const struct object_id *tag_oid, + struct object_id *target) +{ + enum object_type type; + size_t size; + void *buf; + struct object_info oi = OBJECT_INFO_INIT; + struct object *obj; + int eaten; + + oi.typep = &type; + oi.sizep = &size; + oi.contentp = &buf; + if (odb_read_object_info_extended( + the_repository->objects, tag_oid, &oi, + OBJECT_INFO_SKIP_FETCH_OBJECT | OBJECT_INFO_DIE_IF_CORRUPT | + OBJECT_INFO_LOOKUP_REPLACE) < 0) + return -1; + if (type != OBJ_TAG) { + free(buf); + return -1; + } + + obj = parse_object_buffer(the_repository, tag_oid, type, + (unsigned long)size, buf, &eaten); + if (!eaten) + free(buf); + if (!obj || obj->type != OBJ_TAG || !((struct tag *)obj)->tagged) + return -1; + + oidcpy(target, get_tagged_oid((struct tag *)obj)); + return 0; +} + +enum peel_nofetch_result { + PEEL_NOFETCH_OK = 0, + PEEL_NOFETCH_ERROR = -1, +}; + +static enum peel_nofetch_result peel_to_non_tag_nofetch(struct object_id *oid, + enum object_type *type, + struct verify_state *vs) +{ + struct object_info oi = OBJECT_INFO_INIT; + struct object_id target; + + oi.typep = type; + for (;;) { + if (odb_read_object_info_extended( + the_repository->objects, oid, &oi, + OBJECT_INFO_SKIP_FETCH_OBJECT | OBJECT_INFO_LOOKUP_REPLACE) < 0) { + verify_error(vs, _("unable to read object %s"), + oid_to_hex(oid)); + return PEEL_NOFETCH_ERROR; + } + if (*type != OBJ_TAG) + return PEEL_NOFETCH_OK; + if (read_tag_target_nofetch(oid, &target)) { + verify_error(vs, _("unable to peel tag %s"), + oid_to_hex(oid)); + return PEEL_NOFETCH_ERROR; + } + oidcpy(oid, &target); + } +} + +/* + * Consume the tip iterator, peel all tags, and verify non-commit + * objects immediately. Returns commit OIDs in commit_tips for + * boundary finding. Tips already in the self-contained pack + * (verified by index-pack) are skipped. + */ +static int collect_and_peel_tips(const struct object_id *oid, + oid_iterate_fn fn, void *cb_data, + struct transport *transport, + struct oid_array *commit_tips, + struct verify_state *vs) +{ + struct packed_git *new_pack = get_self_contained_pack(transport); + int err = 0; + + do { + struct object_id peeled; + enum object_type type; + + if (new_pack && find_pack_entry_one(oid, new_pack)) + continue; + + oidcpy(&peeled, oid); + if (peel_to_non_tag_nofetch(&peeled, &type, vs) == + PEEL_NOFETCH_ERROR) { + err = -1; + break; + } + + switch (type) { + case OBJ_COMMIT: + oid_array_append(commit_tips, &peeled); + break; + case OBJ_TREE: { + const struct oid_array empty = OID_ARRAY_INIT; + err = verify_tree(&peeled, &empty, vs, 0); + break; + } + case OBJ_BLOB: + /* existence confirmed by peel step */ + break; + default: + verify_error(vs, _("unknown object type %d for %s"), + type, oid_to_hex(&peeled)); + err = -1; + break; + } + } while (!err && (oid = fn(cb_data)) != NULL); + + if (new_pack) { + close_pack(new_pack); + free(new_pack); + } + return err; +} + +static int verify_commit_tree(struct commit *commit, + struct verify_state *vs) +{ + struct oid_array base_trees = OID_ARRAY_INIT; + struct commit_list *p; + int ret; + + for (p = commit->parents; p; p = p->next) { + const struct object_id *tree_oid; + if (repo_parse_commit_gently(the_repository, p->item, 1)) + continue; + tree_oid = get_commit_tree_oid(p->item); + oidset_insert(&vs->verified_trees, tree_oid); + oid_array_append(&base_trees, tree_oid); + } + + ret = verify_tree(get_commit_tree_oid(commit), + &base_trees, vs, 0); + oid_array_clear(&base_trees); + return ret; +} + +/* + * Walk new commits in topological order (parents before children) + * and verify each commit's tree, skipping previously verified entries. + */ +static int verify_new_commits(struct commit_list **new_commits, + struct verify_state *vs) +{ + struct commit_list *iter; + unsigned nr_before; + int err = 0; + + nr_before = commit_list_count(*new_commits); + sort_in_topological_order(new_commits, REV_SORT_IN_GRAPH_ORDER); + /* + * sort_in_topological_order() uses an in-degree-based algorithm + * that drops commits involved in cycles; a count decrease means + * a cycle was present. + */ + if (commit_list_count(*new_commits) < nr_before) { + verify_error(vs, _("cycle detected in incoming commit graph")); + return -1; + } + + *new_commits = commit_list_reverse(*new_commits); + + for (iter = *new_commits; !err && iter; iter = iter->next) + err = verify_commit_tree(iter->item, vs); + + return err; +} + +/* + * Find the connectivity boundary: the set of new commits not yet + * reachable from local refs. Feeds commit_tips to rev-list via + * stdin, collects output commit OIDs into new_commits. + * + * The rev-list setup partially duplicates check_connected() but + * differs enough (stdin piping, output parsing) to stay separate. + */ +static int find_connectivity_boundary(struct check_connected_options *opt, + struct oid_array *commit_tips, + struct commit_list **new_commits, + struct verify_state *vs) +{ + struct child_process rev_list = CHILD_PROCESS_INIT; + FILE *rev_list_in; + FILE *rev_list_out; + struct strbuf line = STRBUF_INIT; + int err = 0; + size_t i; + + strvec_push(&rev_list.args, "rev-list"); + strvec_push(&rev_list.args, "--stdin"); + if (!opt->is_deepening_fetch) { + strvec_push(&rev_list.args, "--not"); + if (opt->exclude_hidden_refs_section) + strvec_pushf(&rev_list.args, "--exclude-hidden=%s", + opt->exclude_hidden_refs_section); + strvec_push(&rev_list.args, "--all"); + } + strvec_push(&rev_list.args, "--alternate-refs"); + if (opt->progress) + strvec_pushf(&rev_list.args, "--progress=%s", + _("Finding connectivity boundary")); + + rev_list.git_cmd = 1; + if (opt->env) + strvec_pushv(&rev_list.env, opt->env); + rev_list.in = -1; + rev_list.out = -1; + if (vs->err_fd) { + int fd = dup(vs->err_fd); + if (fd < 0) + return error_errno(_("could not duplicate error fd")); + rev_list.err = fd; + } else { + rev_list.no_stderr = opt->quiet; + } + + if (start_command(&rev_list)) + return error(_("could not run 'git rev-list'")); + + sigchain_push(SIGPIPE, SIG_IGN); + + /* + * rev-list --stdin consumes all input revisions before starting + * the revision walk, so it cannot fill stdout while we feed + * stdin. Write-then-read is safe here despite both pipes. + */ + rev_list_in = xfdopen(rev_list.in, "w"); + + for (i = 0; i < commit_tips->nr; i++) { + if (fprintf(rev_list_in, "%s\n", + oid_to_hex(&commit_tips->oid[i])) < 0) + break; + } + + if (ferror(rev_list_in) || fflush(rev_list_in)) { + if (errno != EPIPE && errno != EINVAL) + error_errno(_("failed write to rev-list")); + err = -1; + } + if (fclose(rev_list_in)) + err = error_errno(_("failed to close rev-list's stdin")); + + rev_list_out = xfdopen(rev_list.out, "r"); + + while (!err && strbuf_getline(&line, rev_list_out) != EOF) { + struct object_id commit_oid; + struct commit *commit; + const char *end; + + if (parse_oid_hex(line.buf, &commit_oid, &end) || *end) { + verify_error(vs, + _("bad rev-list output: %s"), line.buf); + err = -1; + break; + } + + commit = lookup_commit(the_repository, &commit_oid); + if (!commit || repo_parse_commit_gently(the_repository, + commit, 1)) { + verify_error(vs, _("unable to parse commit %s"), + oid_to_hex(&commit_oid)); + err = -1; + break; + } + + commit_list_insert(commit, new_commits); + } + + strbuf_release(&line); + fclose(rev_list_out); + sigchain_pop(SIGPIPE); + + if (finish_command(&rev_list)) + err = -1; + + if (err) { + commit_list_free(*new_commits); + *new_commits = NULL; + } + + return err; +} + +/* + * Collect tips, find the connectivity boundary via rev-list, + * then verify new commits' trees. See the overview comment + * above struct loaded_tree for the algorithm. + */ +static int check_connected_incremental(oid_iterate_fn fn, void *cb_data, + struct check_connected_options *opt, + const struct object_id *oid) +{ + struct verify_state vs = { 0 }; + struct commit_list *new_commits = NULL; + struct oid_array commit_tips = OID_ARRAY_INIT; + int err = 0; + + vs.quiet = opt->quiet; + vs.err_fd = opt->err_fd; + + trace2_region_enter("connectivity", "incremental", the_repository); + + trace2_region_enter("connectivity", "collect-tips", the_repository); + err = collect_and_peel_tips(oid, fn, cb_data, opt->transport, + &commit_tips, &vs); + trace2_region_leave("connectivity", "collect-tips", the_repository); + + trace2_region_enter("connectivity", "find-boundary", the_repository); + if (!err) + err = find_connectivity_boundary(opt, &commit_tips, + &new_commits, &vs); + trace2_region_leave("connectivity", "find-boundary", the_repository); + + trace2_region_enter("connectivity", "verify-new-commits", the_repository); + if (!err) + err = verify_new_commits(&new_commits, &vs); + trace2_region_leave("connectivity", "verify-new-commits", the_repository); + + if (vs.err_fd) + close(vs.err_fd); + commit_list_free(new_commits); + oid_array_clear(&commit_tips); + oidset_clear(&vs.verified_trees); + oidset_clear(&vs.verified_blobs); + trace2_data_intmax("connectivity", the_repository, + "trees_walked", vs.trees_walked); + trace2_data_intmax("connectivity", the_repository, + "blobs_checked", vs.blobs_checked); + trace2_region_leave("connectivity", "incremental", the_repository); + + return err; +} + +static int incremental_check_applicable(struct check_connected_options *opt) +{ + const char *algorithm = NULL; + + if (repo_config_get_string_tmp(the_repository, + "transfer.connectivitycheck", + &algorithm)) + return 0; + if (strcasecmp(algorithm, "incremental")) { + if (strcasecmp(algorithm, "rev-list")) + die(_("unknown transfer.connectivityCheck algorithm '%s'"), + algorithm); + return 0; + } + + if (opt->is_deepening_fetch) + return 0; + if (opt->shallow_file) + return 0; + if (repo_has_promisor_remote(the_repository)) + return 0; + if (replace_refs_enabled(the_repository)) { + prepare_replace_object(the_repository); + if (oidmap_get_size(&the_repository->objects->replace_map)) + return 0; + } + + return 1; +} + /* * If we feed all the commits we want to verify to this command * @@ -141,6 +755,9 @@ int check_connected(oid_iterate_fn fn, void *cb_data, } } + if (incremental_check_applicable(opt)) + return check_connected_incremental(fn, cb_data, opt, oid); + if (opt->shallow_file) { strvec_push(&rev_list.args, "--shallow-file"); strvec_push(&rev_list.args, opt->shallow_file); diff --git a/t/helper/meson.build b/t/helper/meson.build index 3235f10ab8aae1..ec0f65deaa780c 100644 --- a/t/helper/meson.build +++ b/t/helper/meson.build @@ -5,6 +5,7 @@ test_tool_sources = [ 'test-bloom.c', 'test-bundle-uri.c', 'test-cache-tree.c', + 'test-check-connected.c', 'test-chmtime.c', 'test-config.c', 'test-crontab.c', diff --git a/t/helper/test-check-connected.c b/t/helper/test-check-connected.c new file mode 100644 index 00000000000000..a7412142da3dd3 --- /dev/null +++ b/t/helper/test-check-connected.c @@ -0,0 +1,53 @@ +#define USE_THE_REPOSITORY_VARIABLE + +#include "test-tool.h" +#include "hex.h" +#include "connected.h" +#include "oid-array.h" +#include "setup.h" + +struct cb_data { + struct oid_array *oids; + size_t idx; +}; + +static const struct object_id *iterate_oids(void *data) +{ + struct cb_data *cb = data; + if (cb->idx >= cb->oids->nr) + return NULL; + return &cb->oids->oid[cb->idx++]; +} + +int cmd__check_connected(int argc, const char **argv) +{ + struct oid_array oids = OID_ARRAY_INIT; + struct check_connected_options opt = CHECK_CONNECTED_INIT; + struct cb_data cb; + int i, ret; + + setup_git_directory(the_repository); + + for (i = 1; i < argc; i++) { + struct object_id oid; + if (!strcmp(argv[i], "--shallow-file")) { + if (++i >= argc) + die("--shallow-file requires an argument"); + opt.shallow_file = argv[i]; + continue; + } + if (get_oid_hex(argv[i], &oid)) + die("not a valid object: %s", argv[i]); + oid_array_append(&oids, &oid); + } + + if (!oids.nr) + die("usage: test-tool check-connected [--shallow-file ] ..."); + + cb.oids = &oids; + cb.idx = 0; + + ret = check_connected(iterate_oids, &cb, &opt); + oid_array_clear(&oids); + return !!ret; +} diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c index b71a22b43bbc9e..cacd1cc96e9690 100644 --- a/t/helper/test-tool.c +++ b/t/helper/test-tool.c @@ -15,6 +15,7 @@ static struct test_cmd cmds[] = { { "bloom", cmd__bloom }, { "bundle-uri", cmd__bundle_uri }, { "cache-tree", cmd__cache_tree }, + { "check-connected", cmd__check_connected }, { "chmtime", cmd__chmtime }, { "config", cmd__config }, { "crontab", cmd__crontab }, diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h index f2885b33d58aa8..0777d120b3d56a 100644 --- a/t/helper/test-tool.h +++ b/t/helper/test-tool.h @@ -8,6 +8,7 @@ int cmd__bitmap(int argc, const char **argv); int cmd__bloom(int argc, const char **argv); int cmd__bundle_uri(int argc, const char **argv); int cmd__cache_tree(int argc, const char **argv); +int cmd__check_connected(int argc, const char **argv); int cmd__chmtime(int argc, const char **argv); int cmd__config(int argc, const char **argv); int cmd__crontab(int argc, const char **argv); diff --git a/t/meson.build b/t/meson.build index 181d61a8a0bd18..6079a6bff148c9 100644 --- a/t/meson.build +++ b/t/meson.build @@ -652,6 +652,7 @@ integration_tests = [ 't5409-colorize-remote-messages.sh', 't5410-receive-pack.sh', 't5411-proc-receive-hook.sh', + 't5412-connectivity-check.sh', 't5500-fetch-pack.sh', 't5501-fetch-push-alternates.sh', 't5502-quickfetch.sh', diff --git a/t/t5412-connectivity-check.sh b/t/t5412-connectivity-check.sh new file mode 100755 index 00000000000000..f78a76a579abbc --- /dev/null +++ b/t/t5412-connectivity-check.sh @@ -0,0 +1,512 @@ +#!/bin/sh + +test_description='connectivity check (transfer.connectivityCheck)' +GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main +export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME + +. ./test-lib.sh + +test_oid_cache <<-\EOF +missing sha1:0000000000000000000000000000000000000001 +missing sha256:0000000000000000000000000000000000000000000000000000000000000001 +EOF + +set_connectivity_check () { + if test $# -eq 2 + then + git -C "$1" config transfer.connectivityCheck "$2" + else + git config transfer.connectivityCheck "$1" + fi +} + +test_trace2_count_for_incremental () { + if test "$mode" = incremental + then + test_trace2_data_singular connectivity "$@" + fi +} + +# Create a commit with one file changed, without modifying HEAD, +# index, or worktree. Prints the new commit OID on stdout. +# Usage: commit_with_change +commit_with_change () { + new_blob=$(echo "$3" | git hash-object -w --stdin) && + TMP_IDX=.git/tmp-idx && + GIT_INDEX_FILE=$TMP_IDX git read-tree "$1" && + GIT_INDEX_FILE=$TMP_IDX git update-index --replace \ + --cacheinfo "100644,$new_blob,$2" && + new_tree=$(GIT_INDEX_FILE=$TMP_IDX git write-tree) && + rm -f "$TMP_IDX" && + git commit-tree "$new_tree" -p "$1" -m "modify $2" +} + +# Run a test inside a directory with connectivity check mode set. +# Usage: test_expect_success_in "title" 'body' +test_expect_success_in () { + dir=$1 && shift && + case $# in + 2) + test_expect_success "$1" \ + "( cd $dir && set_connectivity_check \$mode && $2 )" + ;; + 3) + test_expect_success "$1" "$2" \ + "( cd $dir && set_connectivity_check \$mode && $3 )" + ;; + *) + BUG "test_expect_success_in requires 3 or 4 arguments" + ;; + esac +} + +# Check one or more OIDs with test-tool, optionally verifying trace2 counts. +# Usage: check_connected_trace ... +# An empty string for or skips that assertion. +check_connected_trace () { + trace_file=$1 trees=$2 blobs=$3 && + shift 3 && + GIT_TRACE2_EVENT="$(pwd)/$trace_file" \ + test-tool check-connected "$@" && + if test -n "$trees" + then + test_trace2_count_for_incremental trees_walked "$trees" \ + <"$trace_file" + fi && + if test -n "$blobs" + then + test_trace2_count_for_incremental blobs_checked "$blobs" \ + <"$trace_file" + fi +} + +# Shared setup: a repo with several root-level files and nested dirs. +# The unchanged/ subtree (10 dirs x 10 files = 100 blobs, 11 trees) +# acts as a canary: any test asserting small tree/blob counts would +# fail dramatically if incremental accidentally walked into it. +# +# Graph: +# initial -- root-level files (file-{1..5}.txt) +# nested -- adds a/b/c/deep.txt and a/other.txt +# canary -- adds unchanged/{dir-1..10}/{file-1..10}.txt + +test_expect_success 'setup main repo' ' + git init main-repo && + ( + cd main-repo && + for i in $(test_seq 1 5) + do + echo "file $i" >"file-$i.txt" || return 1 + done && + git add file-*.txt && + git commit -m "initial" && + + mkdir -p a/b/c && + echo deep >a/b/c/deep.txt && + echo other >a/other.txt && + git add a/b/c/deep.txt a/other.txt && + git commit -m "add nested dirs" && + + for i in $(test_seq 1 10) + do + mkdir -p "unchanged/dir-$i" && + for j in $(test_seq 1 10) + do + echo "unchanged $i $j" \ + >"unchanged/dir-$i/file-$j.txt" || + return 1 + done + done && + git add unchanged/ && + git commit -m "add unchanged canary subtree" && + + test_oid missing >.git/fake-oid + ) +' + +test_expect_success 'setup replacement object repo' ' + git init replace-test && + ( + cd replace-test && + + test_commit --no-tag original file.txt && + original=$(git rev-parse HEAD) && + orig_blob=$(git rev-parse HEAD:file.txt) && + + # Orphan replacement commit with a different tree + replacement_tree=$(echo replaced | git hash-object -w --stdin | + xargs -I{} git mktree <<-EOF + 100644 blob {} file.txt + EOF + ) && + replacement=$(git commit-tree -m "replacement" \ + "$replacement_tree") && + + git replace "$original" "$replacement" && + + # Remove the original blob so only the replacement + # tree is complete. + rm .git/objects/$(test_oid_to_path "$orig_blob") && + + # Drop branch and HEAD so --not --all does not + # exclude the original commit. + git update-ref -d refs/heads/main && + git update-ref -d HEAD && + + echo "$original" >.git/test-oid + ) +' + +for mode in rev-list incremental +do + +# Corruption detection: craft broken object graphs and verify detection. +# All tests use main-repo without modifying its refs or worktree. + +test_expect_success_in main-repo "$mode: rejects commit with missing blob" ' + fake_oid=$(cat .git/fake-oid) && + bad_tree=$(printf "100644 blob ${fake_oid}\tfile.txt\n" | + git mktree --missing) && + bad_commit=$(git commit-tree "$bad_tree" -p HEAD -m "bad") && + + test_must_fail test-tool check-connected "$bad_commit" 2>err && + test_grep "missing blob object" err +' + +test_expect_success_in main-repo "$mode: rejects commit with missing subtree" ' + fake_oid=$(cat .git/fake-oid) && + bad_tree=$(printf "40000 tree ${fake_oid}\tdir\n" | + git mktree --missing) && + bad_commit=$(git commit-tree "$bad_tree" -p HEAD -m "bad") && + + test_must_fail test-tool check-connected "$bad_commit" 2>err && + test_grep "bad tree object" err +' + +test_expect_success_in main-repo "$mode: rejects missing blob under annotated tag" ' + fake_oid=$(cat .git/fake-oid) && + bad_tree=$(printf "100644 blob ${fake_oid}\tfile.txt\n" | + git mktree --missing) && + bad_commit=$(git commit-tree "$bad_tree" -p HEAD -m "bad") && + git tag -a -m "annotated" bad-tag "$bad_commit" && + tag_oid=$(git rev-parse bad-tag) && + git tag -d bad-tag && + + test_must_fail test-tool check-connected "$tag_oid" 2>err && + test_grep "missing blob object" err +' + +test_expect_success_in main-repo "$mode: verifies direct tree tip" ' + fake_oid=$(cat .git/fake-oid) && + bad_tree=$(printf "100644 blob ${fake_oid}\tfile.txt\n" | + git mktree --missing) && + + test_must_fail test-tool check-connected "$bad_tree" 2>err && + test_grep "missing blob object" err +' + +test_expect_success_in main-repo "$mode: verifies direct blob tip" ' + blob_oid=$(echo "hello" | git hash-object -w --stdin) && + test-tool check-connected "$blob_oid" +' + +test_expect_success_in main-repo "$mode: rejects missing direct blob tip" ' + test_must_fail test-tool check-connected \ + $(cat .git/fake-oid) 2>err +' + +test_expect_success_in main-repo "$mode: accepts tag pointing to existing blob" ' + blob_oid=$(echo "content" | git hash-object -w --stdin) && + git tag -a -m "tag a blob" blob-tag "$blob_oid" && + tag_oid=$(git rev-parse blob-tag) && + git tag -d blob-tag && + + test-tool check-connected "$tag_oid" +' + +test_expect_success_in main-repo PERL_TEST_HELPERS \ + "$mode: rejects blob OID reused as tree entry" ' + blob_oid=$(git rev-parse HEAD:file-1.txt) && + bin_oid=$(echo "$blob_oid" | hex2oct) && + + bad_tree=$(printf "40000 subdir\0$bin_oid" | + git hash-object -t tree -w --stdin) && + bad_commit=$(git commit-tree -p HEAD -m "child" "$bad_tree") && + + test_must_fail test-tool check-connected "$bad_commit" 2>err && + test_grep "not a tree" err +' + +test_expect_success_in main-repo PERL_TEST_HELPERS \ + "$mode: rejects tree OID reused as blob entry" ' + tree_oid=$(git rev-parse HEAD:a) && + bin_oid=$(echo "$tree_oid" | hex2oct) && + + bad_tree=$(printf "100644 fakefile\0$bin_oid" | + git hash-object -t tree -w --stdin) && + bad_commit=$(git commit-tree -p HEAD -m "child" "$bad_tree") && + + test_must_fail test-tool check-connected "$bad_commit" 2>err && + test_grep "not a blob" err +' + +test_expect_success_in main-repo "$mode: peels nested tag chain" ' + # Create a chain: outer -> inner -> commit + git tag -a -m "inner tag" inner HEAD && + inner_oid=$(git rev-parse inner) && + git tag -a -m "outer tag" outer inner && + outer_oid=$(git rev-parse outer) && + git tag -d outer && + git tag -d inner && + + test-tool check-connected "$outer_oid" +' + +test_expect_success_in main-repo "$mode: rejects missing intermediate tag in chain" ' + git tag -a -m "inner tag" inner HEAD && + inner_oid=$(git rev-parse inner) && + git tag -a -m "outer tag" outer inner && + outer_oid=$(git rev-parse outer) && + git tag -d outer && + git tag -d inner && + + # Remove the inner tag object + rm .git/objects/$(test_oid_to_path "$inner_oid") && + + test_must_fail test-tool check-connected "$outer_oid" +' + +test_expect_success_in main-repo "$mode: checks multiple tips" ' + c1=$(commit_with_change HEAD file-1.txt "tip-a") && + c2=$(commit_with_change HEAD file-2.txt "tip-b") && + git tag -a -m "tagged" multi-tag "$c1" && + tag_oid=$(git rev-parse multi-tag) && + git tag -d multi-tag && + + test-tool check-connected "$c2" "$tag_oid" +' + +# Tree-diff optimization: verify trace2 counts in incremental mode. +# These tests create commits without modifying refs and check them +# directly with test-tool check-connected. + +test_expect_success_in main-repo "$mode: skips unchanged subtrees (single file change)" ' + oid=$(commit_with_change HEAD file-1.txt "changed") && + + # Only the root tree is walked; a/ subtree is unchanged. + # 1 changed blob verified, rest pre-trusted from parent. + check_connected_trace trace-flat.txt 1 1 "$oid" +' + +test_expect_success_in main-repo "$mode: visits depth-proportional trees (nested change)" ' + oid=$(commit_with_change HEAD a/b/c/deep.txt "deep-changed") && + + # root + a + b + c = 4 trees walked, 1 changed blob. + check_connected_trace trace-nested.txt 4 1 "$oid" +' + +test_expect_success_in main-repo "$mode: verifies annotated tag target" ' + oid=$(commit_with_change HEAD file-1.txt "tag-verify") && + git tag -a -m "annotated" verify-tag "$oid" && + tag_oid=$(git rev-parse verify-tag) && + git tag -d verify-tag && + + check_connected_trace trace-tag.txt 1 1 "$tag_oid" +' + +test_expect_success_in main-repo "$mode: reuses tree OID after change-then-revert" ' + c1=$(commit_with_change HEAD file-1.txt "revert-tmp") && + c2=$(commit_with_change "$c1" file-1.txt "file 1") && + c3=$(commit_with_change "$c2" file-1.txt "revert-final") && + + # c2 reverts file-1.txt to original content, so its root + # tree matches HEAD. Visited-set dedup means it is not + # re-walked when reached from c3. + check_connected_trace trace-revert.txt 2 2 "$c3" +' + +test_expect_success_in main-repo "$mode: handles repeated blob content across commits" ' + c1=$(commit_with_change HEAD file-1.txt "shared") && + c2=$(commit_with_change "$c1" file-1.txt "temp") && + c3=$(commit_with_change "$c2" file-1.txt "shared") && + + # c1 and c3 share the same blob OID for file-1.txt. + # The visited set deduplicates so the shared blob is + # only counted once. + check_connected_trace trace-repeat.txt "" 2 "$c3" +' + +# Merge with shared subtree at different paths. +# Needs its own setup because the merge topology cannot be built +# with commit_with_change. + +test_expect_success_in main-repo "$mode: skips subtree reused at different path (merge)" ' + # Build two branch commits that add the same subtree + # content at different paths, without updating any refs. + blob_a=$(echo a | git hash-object -w --stdin) && + blob_b=$(echo b | git hash-object -w --stdin) && + blob_c=$(echo c | git hash-object -w --stdin) && + shared_tree=$(printf "100644 blob %s\tfile1.txt\n100644 blob %s\tfile2.txt\n100644 blob %s\tfile3.txt\n" \ + "$blob_a" "$blob_b" "$blob_c" | git mktree) && + + TMP_IDX=.git/tmp-idx && + + GIT_INDEX_FILE=$TMP_IDX git read-tree HEAD && + GIT_INDEX_FILE=$TMP_IDX git read-tree --prefix=shared-a/ "$shared_tree" && + tree_a=$(GIT_INDEX_FILE=$TMP_IDX git write-tree) && + commit_a=$(git commit-tree "$tree_a" -p HEAD -m "branch-a") && + + GIT_INDEX_FILE=$TMP_IDX git read-tree HEAD && + GIT_INDEX_FILE=$TMP_IDX git read-tree --prefix=shared-b/ "$shared_tree" && + tree_b=$(GIT_INDEX_FILE=$TMP_IDX git write-tree) && + commit_b=$(git commit-tree "$tree_b" -p HEAD -m "branch-b") && + + rm -f "$TMP_IDX" && + + # Merge the two branches (using branch-a tree as the + # merge result -- the exact content does not matter, + # only that both parents are walked). + merge=$(git commit-tree "$tree_a" \ + -p "$commit_a" -p "$commit_b" -m "merge") && + + oid=$(commit_with_change "$merge" file-1.txt "post-merge") && + + check_connected_trace trace-reuse.txt 4 4 "$oid" +' + +test_expect_success_in main-repo "$mode: traverses octopus merge (3 parents)" ' + c1=$(commit_with_change HEAD file-1.txt "oct-a") && + c2=$(commit_with_change HEAD file-2.txt "oct-b") && + c3=$(commit_with_change HEAD a/other.txt "oct-c") && + + # Octopus: merge tree uses c1 as base, all three are parents. + merge_tree=$(git rev-parse "$c1^{tree}") && + octopus=$(git commit-tree "$merge_tree" \ + -p "$c1" -p "$c2" -p "$c3" -m "octopus") && + + # c1: root tree walked, 1 blob (file-1.txt). + # c2: root tree walked, 1 blob (file-2.txt). + # c3: root + a/ walked, 1 blob (a/other.txt). + # octopus: tree matches c1, already verified -- skipped. + # Total: 4 trees, 3 blobs. + check_connected_trace trace-octopus.txt 4 3 "$octopus" +' + +test_expect_success_in main-repo "$mode: handles gitlink entries (submodules)" ' + fake_oid=$(cat .git/fake-oid) && + TMP_IDX=.git/tmp-idx && + GIT_INDEX_FILE=$TMP_IDX git read-tree HEAD && + GIT_INDEX_FILE=$TMP_IDX git update-index --add \ + --cacheinfo "160000,$fake_oid,my-submodule" && + gitlink_tree=$(GIT_INDEX_FILE=$TMP_IDX git write-tree) && + rm -f "$TMP_IDX" && + gitlink_commit=$(git commit-tree "$gitlink_tree" -p HEAD \ + -m "add gitlink") && + + # Gitlink entries are skipped -- the missing submodule + # commit OID does not cause a failure. + check_connected_trace trace-gitlink.txt 1 0 "$gitlink_commit" +' + +# Replacement objects. + +test_expect_success_in replace-test "$mode: accepts with replacement objects" ' + original=$(cat .git/test-oid) && + test-tool check-connected "$original" +' + +test_expect_success_in replace-test "$mode: rejects without replacement objects" ' + original=$(cat .git/test-oid) && + test_must_fail env GIT_NO_REPLACE_OBJECTS=1 \ + test-tool check-connected "$original" 2>err && + test_grep "missing blob object" err +' + +# Deepening fetch: verify the operation succeeds with both modes. + +test_expect_success "$mode: deepening fetch succeeds" ' + test_when_finished "rm -rf deepen-src deepen-server.git deepen-client" && + + git init deepen-src && + test_commit -C deepen-src --no-tag c1 file.txt && + test_commit -C deepen-src --no-tag c2 file.txt && + test_commit -C deepen-src --no-tag c3 file.txt && + git clone --bare deepen-src deepen-server.git && + git clone --depth=1 "file://$(pwd)/deepen-server.git" deepen-client && + set_connectivity_check deepen-client $mode && + test -f deepen-client/.git/shallow && + git -C deepen-client fetch --deepen=2 origin main +' + +done + +# Algorithm selection: verify fallback and rejection behavior. + +test_expect_success 'incremental falls back with replacement objects' ' + ( + cd replace-test && + set_connectivity_check incremental && + original=$(cat .git/test-oid) && + GIT_TRACE2_EVENT="$(pwd)/trace-fallback.txt" \ + test-tool check-connected "$original" && + test_region ! connectivity incremental trace-fallback.txt + ) +' + +test_expect_success 'invalid transfer.connectivityCheck is rejected' ' + test_when_finished "rm -rf invalid-cfg" && + + git init invalid-cfg && + ( + cd invalid-cfg && + test_commit --no-tag base file.txt && + git config transfer.connectivityCheck bogus && + oid=$(git rev-parse HEAD) && + test_must_fail test-tool check-connected "$oid" 2>err && + test_grep "unknown transfer.connectivityCheck" err + ) +' + +# Integration: verify incremental runs during a real push. + +test_expect_success 'push uses incremental when configured' ' + test_when_finished "rm -rf int-src int-dst.git" && + + git init int-src && + test_commit -C int-src --no-tag base file.txt && + git clone --bare int-src int-dst.git && + test_commit -C int-src --no-tag update file.txt updated && + + set_connectivity_check int-dst.git incremental && + GIT_TRACE2_EVENT="$(pwd)/trace-push.txt" \ + git -C int-src push ../int-dst.git main && + test_region connectivity incremental trace-push.txt +' + +test_expect_success 'fetch uses incremental when configured' ' + test_when_finished "rm -rf fetch-src fetch-dst" && + + git init fetch-src && + test_commit -C fetch-src --no-tag base file.txt && + git clone fetch-src fetch-dst && + test_commit -C fetch-src --no-tag update file.txt updated && + + set_connectivity_check fetch-dst incremental && + GIT_TRACE2_EVENT="$(pwd)/trace-fetch.txt" \ + git -C fetch-dst fetch origin main && + test_region connectivity incremental trace-fetch.txt +' + +test_expect_success 'clone respects transfer.connectivityCheck' ' + test_when_finished "rm -rf clone-src clone-dst" && + + git init clone-src && + test_commit -C clone-src --no-tag base file.txt && + + GIT_TRACE2_EVENT="$(pwd)/trace-clone.txt" \ + git -c transfer.connectivityCheck=incremental \ + clone --no-local clone-src clone-dst && + test_region connectivity incremental trace-clone.txt +' + +test_done From 52e883f4ba96244d3c477d23b03cac02fae2fac3 Mon Sep 17 00:00:00 2001 From: Kristofer Karlsson Date: Fri, 28 Aug 2026 08:15:14 +0200 Subject: [PATCH 3/4] connected: handle shallow fetches in incremental check Teach the incremental connectivity check to handle shallow fetches. Shallow commits are treated as traversal roots with no parents, matching the boundary semantics of rev-list. Add parse_shallow_file_gently() to parse the temporary shallow file without dying on errors, and thread the resulting oidset through verify_new_commits and verify_commit_tree. Pass --shallow-file to the boundary-finding rev-list so it respects the shallow grafts. Remove the shallow_file guard from incremental_check_applicable() so incremental mode is now used for shallow fetches when configured. Signed-off-by: Kristofer Karlsson --- Documentation/config/transfer.adoc | 4 +- connected.c | 63 +++++++++++++++++++++++++++--- t/t5412-connectivity-check.sh | 45 +++++++++++++++++++++ 3 files changed, 105 insertions(+), 7 deletions(-) diff --git a/Documentation/config/transfer.adoc b/Documentation/config/transfer.adoc index 91891d02b751fc..bee31c207690d7 100644 --- a/Documentation/config/transfer.adoc +++ b/Documentation/config/transfer.adoc @@ -16,8 +16,8 @@ transfer.connectivityCheck:: trees, recursively descending only into entries that differ. The largest benefits occur when incoming commits change a small fraction of a large tree closure. - Falls back to `rev-list` for shallow fetches, partial - clones, replacement objects, or deepening fetches. + Falls back to `rev-list` for partial clones, replacement + objects, or deepening fetches. -- transfer.credentialsInUrl:: diff --git a/connected.c b/connected.c index c725aa90572fd2..ebf4fb13c56cb6 100644 --- a/connected.c +++ b/connected.c @@ -464,13 +464,16 @@ static int collect_and_peel_tips(const struct object_id *oid, } static int verify_commit_tree(struct commit *commit, + const struct oidset *shallow_commits, struct verify_state *vs) { struct oid_array base_trees = OID_ARRAY_INIT; struct commit_list *p; int ret; - for (p = commit->parents; p; p = p->next) { + p = oidset_contains(shallow_commits, &commit->object.oid) + ? NULL : commit->parents; + for (; p; p = p->next) { const struct object_id *tree_oid; if (repo_parse_commit_gently(the_repository, p->item, 1)) continue; @@ -488,8 +491,12 @@ static int verify_commit_tree(struct commit *commit, /* * Walk new commits in topological order (parents before children) * and verify each commit's tree, skipping previously verified entries. + * + * Shallow commits are treated as roots with no parents, matching + * rev-list's traversal boundary. */ static int verify_new_commits(struct commit_list **new_commits, + const struct oidset *shallow_commits, struct verify_state *vs) { struct commit_list *iter; @@ -511,8 +518,42 @@ static int verify_new_commits(struct commit_list **new_commits, *new_commits = commit_list_reverse(*new_commits); for (iter = *new_commits; !err && iter; iter = iter->next) - err = verify_commit_tree(iter->item, vs); + err = verify_commit_tree(iter->item, shallow_commits, vs); + + return err; +} +/* + * oidset_parse_file() cannot be reused because it calls die() on errors. + */ +static int parse_shallow_file_gently(const char *path, + struct oidset *shallow_commits, + struct verify_state *vs) +{ + FILE *fp; + struct strbuf line = STRBUF_INIT; + struct object_id oid; + int err = 0; + + fp = fopen(path, "r"); + if (!fp) { + if (errno == ENOENT) + return 0; + verify_error(vs, _("unable to open shallow file '%s': %s"), + path, strerror(errno)); + return -1; + } + while (strbuf_getline(&line, fp) != EOF) { + const char *end; + if (parse_oid_hex(line.buf, &oid, &end) || *end) { + verify_error(vs, _("bad shallow line: %s"), line.buf); + err = -1; + break; + } + oidset_insert(shallow_commits, &oid); + } + fclose(fp); + strbuf_release(&line); return err; } @@ -536,6 +577,10 @@ static int find_connectivity_boundary(struct check_connected_options *opt, int err = 0; size_t i; + if (opt->shallow_file) { + strvec_push(&rev_list.args, "--shallow-file"); + strvec_push(&rev_list.args, opt->shallow_file); + } strvec_push(&rev_list.args, "rev-list"); strvec_push(&rev_list.args, "--stdin"); if (!opt->is_deepening_fetch) { @@ -642,6 +687,7 @@ static int check_connected_incremental(oid_iterate_fn fn, void *cb_data, { struct verify_state vs = { 0 }; struct commit_list *new_commits = NULL; + struct oidset shallow_commits = OIDSET_INIT; struct oid_array commit_tips = OID_ARRAY_INIT; int err = 0; @@ -650,6 +696,13 @@ static int check_connected_incremental(oid_iterate_fn fn, void *cb_data, trace2_region_enter("connectivity", "incremental", the_repository); + if (opt->shallow_file && *opt->shallow_file) { + err = parse_shallow_file_gently(opt->shallow_file, + &shallow_commits, &vs); + if (err) + goto done; + } + trace2_region_enter("connectivity", "collect-tips", the_repository); err = collect_and_peel_tips(oid, fn, cb_data, opt->transport, &commit_tips, &vs); @@ -663,12 +716,14 @@ static int check_connected_incremental(oid_iterate_fn fn, void *cb_data, trace2_region_enter("connectivity", "verify-new-commits", the_repository); if (!err) - err = verify_new_commits(&new_commits, &vs); + err = verify_new_commits(&new_commits, &shallow_commits, &vs); trace2_region_leave("connectivity", "verify-new-commits", the_repository); +done: if (vs.err_fd) close(vs.err_fd); commit_list_free(new_commits); + oidset_clear(&shallow_commits); oid_array_clear(&commit_tips); oidset_clear(&vs.verified_trees); oidset_clear(&vs.verified_blobs); @@ -698,8 +753,6 @@ static int incremental_check_applicable(struct check_connected_options *opt) if (opt->is_deepening_fetch) return 0; - if (opt->shallow_file) - return 0; if (repo_has_promisor_remote(the_repository)) return 0; if (replace_refs_enabled(the_repository)) { diff --git a/t/t5412-connectivity-check.sh b/t/t5412-connectivity-check.sh index f78a76a579abbc..71c493869d7438 100755 --- a/t/t5412-connectivity-check.sh +++ b/t/t5412-connectivity-check.sh @@ -422,6 +422,51 @@ test_expect_success_in replace-test "$mode: rejects without replacement objects" test_grep "missing blob object" err ' +# Shallow edge cases. + +test_expect_success "$mode: rejects missing blob behind shallow boundary" ' + test_when_finished "rm -rf shallow-boundary" && + + git init shallow-boundary && + ( + cd shallow-boundary && + set_connectivity_check $mode && + + test_commit --no-tag "parent P" file.txt content && + parent=$(git rev-parse HEAD) && + blob_oid=$(git rev-parse HEAD:file.txt) && + + tree_oid=$(git rev-parse HEAD^{tree}) && + child=$(git commit-tree -p "$parent" -m "child S" "$tree_oid") && + + rm .git/objects/$(test_oid_to_path "$blob_oid") && + + echo "$child" >shallow_file && + + test_must_fail test-tool check-connected \ + --shallow-file shallow_file "$child" 2>err && + test_grep "missing blob object" err + ) +' + +test_expect_success "$mode: rejects malformed shallow file" ' + test_when_finished "rm -rf malformed-shallow" && + + git init malformed-shallow && + ( + cd malformed-shallow && + set_connectivity_check $mode && + test_commit --no-tag base file.txt content && + oid=$(git rev-parse HEAD) && + + echo "not-a-valid-oid" >bad_shallow && + + test_expect_code 1 test-tool check-connected \ + --shallow-file bad_shallow "$oid" 2>err && + test_grep "bad shallow line" err + ) +' + # Deepening fetch: verify the operation succeeds with both modes. test_expect_success "$mode: deepening fetch succeeds" ' From 68dbb9c354ad04e647aa125d7a0243b9a20f9ce4 Mon Sep 17 00:00:00 2001 From: Kristofer Karlsson Date: Fri, 28 Aug 2026 08:15:30 +0200 Subject: [PATCH 4/4] connected: handle partial clones in incremental check Teach the incremental connectivity check to handle partial clones where some objects are promised by a promisor remote but not present locally. When a tree or blob cannot be read, check whether it is a promisor object before reporting an error. Promisor objects are trusted and added to the verified set without fetching them. During tag peeling, a missing object that is a promisor object causes the tip to be silently skipped rather than treated as an error. Pass --exclude-promisor-objects to the boundary-finding rev-list so promisor commits do not pollute the set of commits to verify. Remove the repo_has_promisor_remote() guard from incremental_check_applicable() so incremental mode is now used for partial clones when configured. Signed-off-by: Kristofer Karlsson --- Documentation/config/transfer.adoc | 4 +- connected.c | 25 ++++++-- t/t5412-connectivity-check.sh | 95 ++++++++++++++++++++++++++++++ 3 files changed, 118 insertions(+), 6 deletions(-) diff --git a/Documentation/config/transfer.adoc b/Documentation/config/transfer.adoc index bee31c207690d7..f2548145b99030 100644 --- a/Documentation/config/transfer.adoc +++ b/Documentation/config/transfer.adoc @@ -16,8 +16,8 @@ transfer.connectivityCheck:: trees, recursively descending only into entries that differ. The largest benefits occur when incoming commits change a small fraction of a large tree closure. - Falls back to `rev-list` for partial clones, replacement - objects, or deepening fetches. + Falls back to `rev-list` when replacement objects are + active or a deepening fetch is in progress. -- transfer.credentialsInUrl:: diff --git a/connected.c b/connected.c index ebf4fb13c56cb6..6d346bc3a96042 100644 --- a/connected.c +++ b/connected.c @@ -130,6 +130,7 @@ struct loaded_tree { void *buf; }; +/* Read a tree without triggering lazy promisor fetches. */ static int read_tree_nofetch(struct loaded_tree *pt, const struct object_id *oid, enum object_type *actual_type) @@ -266,6 +267,10 @@ static int verify_tree(const struct object_id *new_tree_oid, return 0; if (read_tree_nofetch(&new_tree, new_tree_oid, &type)) { + if (is_promisor_object(the_repository, new_tree_oid)) { + oidset_insert(&vs->verified_trees, new_tree_oid); + return 0; + } if (type != OBJ_NONE) verify_error(vs, _("object %s is a %s, not a tree"), oid_to_hex(new_tree_oid), @@ -317,6 +322,10 @@ static int verify_tree(const struct object_id *new_tree_oid, if (odb_read_object_info_extended( the_repository->objects, &entry.oid, &oi, OBJECT_INFO_SKIP_FETCH_OBJECT | OBJECT_INFO_LOOKUP_REPLACE) < 0) { + if (is_promisor_object(the_repository, &entry.oid)) { + oidset_insert(&vs->verified_blobs, &entry.oid); + continue; + } verify_error(vs, _("missing blob object '%s'"), oid_to_hex(&entry.oid)); ret = -1; @@ -341,6 +350,7 @@ static int verify_tree(const struct object_id *new_tree_oid, return ret; } +/* Read a tag's target OID without triggering lazy promisor fetches. */ static int read_tag_target_nofetch(const struct object_id *tag_oid, struct object_id *target) { @@ -375,8 +385,10 @@ static int read_tag_target_nofetch(const struct object_id *tag_oid, return 0; } +/* Peel tags without triggering lazy promisor fetches. */ enum peel_nofetch_result { PEEL_NOFETCH_OK = 0, + PEEL_NOFETCH_PROMISOR = 1, PEEL_NOFETCH_ERROR = -1, }; @@ -392,6 +404,8 @@ static enum peel_nofetch_result peel_to_non_tag_nofetch(struct object_id *oid, if (odb_read_object_info_extended( the_repository->objects, oid, &oi, OBJECT_INFO_SKIP_FETCH_OBJECT | OBJECT_INFO_LOOKUP_REPLACE) < 0) { + if (is_promisor_object(the_repository, oid)) + return PEEL_NOFETCH_PROMISOR; verify_error(vs, _("unable to read object %s"), oid_to_hex(oid)); return PEEL_NOFETCH_ERROR; @@ -425,13 +439,16 @@ static int collect_and_peel_tips(const struct object_id *oid, do { struct object_id peeled; enum object_type type; + enum peel_nofetch_result peel_ret; if (new_pack && find_pack_entry_one(oid, new_pack)) continue; oidcpy(&peeled, oid); - if (peel_to_non_tag_nofetch(&peeled, &type, vs) == - PEEL_NOFETCH_ERROR) { + peel_ret = peel_to_non_tag_nofetch(&peeled, &type, vs); + if (peel_ret == PEEL_NOFETCH_PROMISOR) + continue; + if (peel_ret == PEEL_NOFETCH_ERROR) { err = -1; break; } @@ -583,6 +600,8 @@ static int find_connectivity_boundary(struct check_connected_options *opt, } strvec_push(&rev_list.args, "rev-list"); strvec_push(&rev_list.args, "--stdin"); + if (repo_has_promisor_remote(the_repository)) + strvec_push(&rev_list.args, "--exclude-promisor-objects"); if (!opt->is_deepening_fetch) { strvec_push(&rev_list.args, "--not"); if (opt->exclude_hidden_refs_section) @@ -753,8 +772,6 @@ static int incremental_check_applicable(struct check_connected_options *opt) if (opt->is_deepening_fetch) return 0; - if (repo_has_promisor_remote(the_repository)) - return 0; if (replace_refs_enabled(the_repository)) { prepare_replace_object(the_repository); if (oidmap_get_size(&the_repository->objects->replace_map)) diff --git a/t/t5412-connectivity-check.sh b/t/t5412-connectivity-check.sh index 71c493869d7438..768d97f83e6a14 100755 --- a/t/t5412-connectivity-check.sh +++ b/t/t5412-connectivity-check.sh @@ -467,6 +467,101 @@ test_expect_success "$mode: rejects malformed shallow file" ' ) ' +# Partial clone: promisor objects should be accepted. + +test_expect_success "$mode: accepts missing promised blob" ' + test_when_finished "rm -rf prom-src prom-server.git prom-client" && + + git init prom-src && + test_commit -C prom-src --no-tag base file.txt original && + test_commit -C prom-src --no-tag "add file2" file2.txt extra && + git clone --bare prom-src prom-server.git && + git -C prom-server.git config uploadpack.allowfilter true && + git -C prom-server.git config uploadpack.allowanysha1inwant true && + + git clone --no-checkout --filter=blob:none \ + "file://$(pwd)/prom-server.git" prom-client && + set_connectivity_check prom-client $mode && + + ( + cd prom-client && + promised_blob=$(git rev-parse HEAD:file2.txt) && + + test_must_fail env GIT_NO_LAZY_FETCH=1 \ + git cat-file -e "$promised_blob" && + + new_tree=$(printf "100644 blob %s\tnewname.txt\n" \ + "$promised_blob" | + git mktree --missing) && + new_commit=$(git commit-tree "$new_tree" \ + -p HEAD -m "reuse promised blob") && + + test-tool check-connected "$new_commit" && + + # Verify connectivity checking did not lazy-fetch it. + test_must_fail env GIT_NO_LAZY_FETCH=1 \ + git cat-file -e "$promised_blob" + ) +' + +test_expect_success "$mode: accepts missing promised tree" ' + test_when_finished "rm -rf prom-tree-src prom-tree-server.git prom-tree-client" && + + git init prom-tree-src && + mkdir -p prom-tree-src/a/b && + test_commit -C prom-tree-src --no-tag "nested dirs" a/b/file.txt deep && + git clone --bare prom-tree-src prom-tree-server.git && + git -C prom-tree-server.git config uploadpack.allowfilter true && + git -C prom-tree-server.git config uploadpack.allowanysha1inwant true && + + git clone --no-checkout --filter=tree:1 \ + "file://$(pwd)/prom-tree-server.git" prom-tree-client && + set_connectivity_check prom-tree-client $mode && + + ( + cd prom-tree-client && + # Subtree "a/" is promised but not present locally. + promised_tree=$(git ls-tree HEAD | grep " a$" | cut -f1 | awk "{print \$3}") && + test_must_fail env GIT_NO_LAZY_FETCH=1 \ + git cat-file -e "$promised_tree" && + + # Build a new tree that reuses the promised subtree + # at a different path. + new_tree=$(printf "40000 tree %s\trenamed\n" \ + "$promised_tree" | + git mktree --missing) && + new_commit=$(git commit-tree "$new_tree" \ + -p HEAD -m "reuse promised tree") && + + test-tool check-connected "$new_commit" && + + # Verify connectivity checking did not lazy-fetch it. + test_must_fail env GIT_NO_LAZY_FETCH=1 \ + git cat-file -e "$promised_tree" + ) +' + +test_expect_success "$mode: verifies local commit in partial clone" ' + test_when_finished "rm -rf pc-src pc-server.git pc-client" && + + git init pc-src && + test_commit -C pc-src --no-tag base file.txt && + git clone --bare pc-src pc-server.git && + git -C pc-server.git config uploadpack.allowfilter true && + git -C pc-server.git config uploadpack.allowanysha1inwant true && + git clone --filter=blob:none \ + "file://$(pwd)/pc-server.git" pc-client && + set_connectivity_check pc-client $mode && + + ( + cd pc-client && + test_commit --no-tag "local change" file.txt local-content && + local_commit=$(git rev-parse HEAD) && + + test-tool check-connected "$local_commit" + ) +' + # Deepening fetch: verify the operation succeeds with both modes. test_expect_success "$mode: deepening fetch succeeds" '