From 9e7a2fb9cef08fdb4056eb76951a52e20ca350c5 Mon Sep 17 00:00:00 2001 From: Kristofer Karlsson Date: Thu, 27 Aug 2026 21:14:38 +0200 Subject: [PATCH 1/5] 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 bb0bc0ae11ef00bea82c453a2c191b2ad089620a Mon Sep 17 00:00:00 2001 From: Kristofer Karlsson Date: Fri, 28 Aug 2026 08:13:23 +0200 Subject: [PATCH 2/5] connected: add incremental connectivity check The connectivity check uses rev-list to find commits that are not reachable from local refs and then walks their object closure. Commit traversal stops at the connectivity boundary, but the trees and blobs reachable from that boundary still need to be walked so they can be marked uninteresting. On repositories where the boundary commits have large trees, this makes small incoming changes expensive. Add an alternative connectivity check that verifies incoming commits incrementally against their parents. The verifier walks a new commit's tree alongside its parent trees. Previously verified entries are skipped; changed subtrees are descended into recursively, and newly referenced blobs are checked for existence in the object database. For example, consider a commit changing one file under lib/: Parent tree New tree +-- src/ (aaa) +-- dev/ (aaa) +-- lib/ (bbb) +-- lib/ (ccc) +-- foo.c (ddd) +-- foo.c (ddd) +-- bar.c (eee) +-- bar.c (fff) 1. Read the parent root and add its direct entries to the verified set: aaa and bbb. 2. Walk the new root: - aaa is already verified, so the subtree is skipped even though it appears at a different path. - ccc is new, so recurse into it using bbb as the parent subtree. 3. Read bbb and add its direct entries ddd and eee to the verified set. 4. Walk ccc: - ddd is already verified. - fff is new, so check that the blob exists. Thus the changed lib/ subtree is the only subtree recursively explored, and only the new bar.c blob needs an existence check. The root trees still need to be read and scanned as comparison bases. New commits are processed with ancestors before descendants. Parents outside the incoming commit set are reachable from existing refs and form the initial trusted boundary. Once an incoming commit has been verified, its tree can in turn be used as a trusted base for its children. Because object contents are immutable, the verified sets persist across commits -- trees and blobs verified for one commit can be reused for later ones (changes, reverts, subtree moves, merges). Only objects derived from that trusted commit boundary or verified while processing incoming commits enter the verified sets. Merely having an unrelated object in the object database does not make it trusted. Missed reuse only costs extra work, not correctness. The parent-side scan is intentionally greedy and does not find every reuse opportunity. Pruning occurs when an object is seen by the parent-side scan before it is encountered on the child side. When that does not happen, the subtree is verified again. For example, if a commit moves the subtree aaa from x/y/ to y/, the parent-side scan never descends into x/ and never sees aaa, so the child traversal verifies that subtree recursively. 1. Collect and peel tips. Non-commit tips are verified immediately; tips already covered by a self-contained pack verified by index-pack are skipped. 2. Find the incoming commit boundary with rev-list --stdin --not --all. 3. Process those commits in topological order and verify their trees incrementally against their parents. Gate the new algorithm behind transfer.connectivityCheck=incremental. Fall back to the existing rev-list path for shallow fetches, partial clones, replacement objects, and deepening fetches. Benchmarks on a large repository (~3M commits, ~200K trees and ~500K blobs reachable from the tip), 1 new commit changing 1 file, measured with hyperfine --warmup 1: With ~10K local refs: rev-list: 1849 ms +/- 60 ms incremental: 143 ms +/- 19 ms (12.9x faster) With 1 local ref: rev-list: 1791 ms +/- 72 ms incremental: 17 ms +/- 1 ms (107x faster) The benefit grows with tree closure size in these measurements: linux.git (~100K reachable objects) shows ~2x; git.git (~5K objects) shows no measurable difference. Signed-off-by: Kristofer Karlsson --- Documentation/config/transfer.adoc | 22 ++ Makefile | 1 + connected.c | 610 +++++++++++++++++++++++++++++ t/helper/meson.build | 1 + t/helper/test-check-connected.c | 67 ++++ t/helper/test-tool.c | 1 + t/helper/test-tool.h | 1 + t/meson.build | 1 + t/t5412-connectivity-check.sh | 587 +++++++++++++++++++++++++++ 9 files changed, 1291 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..2a6b08e8445a1a 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,605 @@ 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, since the entire subtree is + * already verified. + * - Only new or changed entries cause recursive descent and blob + * verification. + * - The verified set persists across commits, letting us skip + * subtrees seen earlier (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. + */ +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. + */ +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, "rev-list")) + return 0; + if (strcasecmp(algorithm, "incremental")) + die(_("unknown transfer.connectivityCheck algorithm '%s'"), + algorithm); + + 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 +748,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..7e888c00a65d04 --- /dev/null +++ b/t/helper/test-check-connected.c @@ -0,0 +1,67 @@ +#define USE_THE_REPOSITORY_VARIABLE + +#include "test-tool.h" +#include "git-compat-util.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 (!strcmp(argv[i], "--err-file")) { + if (++i >= argc) + die("--err-file requires a path argument"); + opt.err_fd = open(argv[i], + O_WRONLY | O_CREAT | O_TRUNC, 0644); + if (opt.err_fd < 0) + die_errno("could not open '%s'", argv[i]); + continue; + } + if (!strcmp(argv[i], "--quiet")) { + opt.quiet = 1; + 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 ] [--err-file ] [--quiet] ..."); + + 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..dcfd260f9078a8 --- /dev/null +++ b/t/t5412-connectivity-check.sh @@ -0,0 +1,587 @@ +#!/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 +' + +# Error routing: verify --err-file and --quiet behavior. + +test_expect_success 'incremental: errors written to err-file' ' + ( + cd main-repo && + set_connectivity_check incremental && + fake_oid=$(cat .git/fake-oid) && + bad_tree=$(printf "100644 blob %s\tfile.txt\n" "$fake_oid" | + git mktree --missing) && + bad_commit=$(git commit-tree "$bad_tree" -p HEAD -m "bad") && + + test_must_fail test-tool check-connected \ + --err-file err.out "$bad_commit" 2>stderr.out && + test_grep "missing blob object" err.out && + test_must_be_empty stderr.out + ) +' + +test_expect_success 'incremental: --quiet suppresses errors without err-file' ' + ( + cd main-repo && + set_connectivity_check incremental && + fake_oid=$(cat .git/fake-oid) && + bad_tree=$(printf "100644 blob %s\tfile.txt\n" "$fake_oid" | + git mktree --missing) && + bad_commit=$(git commit-tree "$bad_tree" -p HEAD -m "bad") && + + test_must_fail test-tool check-connected \ + --quiet "$bad_commit" 2>stderr.out && + test_must_be_empty stderr.out + ) +' + +test_expect_success 'incremental: --quiet with err-file still writes to fd' ' + ( + cd main-repo && + set_connectivity_check incremental && + fake_oid=$(cat .git/fake-oid) && + bad_tree=$(printf "100644 blob %s\tfile.txt\n" "$fake_oid" | + git mktree --missing) && + bad_commit=$(git commit-tree "$bad_tree" -p HEAD -m "bad") && + + test_must_fail test-tool check-connected \ + --quiet --err-file err-quiet.out "$bad_commit" 2>stderr.out && + test_grep "missing blob object" err-quiet.out && + test_must_be_empty stderr.out + ) +' + +test_expect_success 'incremental: boundary search errors routed to err-file' ' + test_when_finished "rm -rf broken-parent" && + + git init broken-parent && + ( + cd broken-parent && + set_connectivity_check incremental && + + test_commit --no-tag first file.txt && + parent=$(git rev-parse HEAD) && + test_commit --no-tag second file.txt update && + child=$(git rev-parse HEAD) && + + rm -rf .git/objects/info/commit-graph* && + rm .git/objects/$(test_oid_to_path "$parent") && + + git update-ref -d refs/heads/main && + git update-ref -d HEAD && + + test_must_fail test-tool check-connected \ + --err-file boundary-err.out "$child" 2>stderr.out && + test_file_not_empty boundary-err.out && + test_must_be_empty stderr.out + ) +' + +test_done From ef6a4710605b84780d69afdb8ac9ebc992cb6545 Mon Sep 17 00:00:00 2001 From: Kristofer Karlsson Date: Fri, 28 Aug 2026 08:15:14 +0200 Subject: [PATCH 3/5] 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 | 60 +++++++++++++++++++++++++++--- t/t5412-connectivity-check.sh | 45 ++++++++++++++++++++++ 3 files changed, 102 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 2a6b08e8445a1a..4f6f48539c7771 100644 --- a/connected.c +++ b/connected.c @@ -462,13 +462,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; @@ -486,8 +489,11 @@ 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 have no parents. */ static int verify_new_commits(struct commit_list **new_commits, + const struct oidset *shallow_commits, struct verify_state *vs) { struct commit_list *iter; @@ -509,11 +515,43 @@ 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) { + 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; +} + /* * Find the connectivity boundary: the set of new commits not yet * reachable from local refs. Feeds commit_tips to rev-list via @@ -531,6 +569,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) { @@ -636,6 +678,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; @@ -644,6 +687,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); @@ -657,12 +707,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); @@ -691,8 +743,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 dcfd260f9078a8..3de8985195de8e 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 502b32887fcf8a1b60dca94eab22d7c418f7d8b6 Mon Sep 17 00:00:00 2001 From: Kristofer Karlsson Date: Fri, 28 Aug 2026 08:15:30 +0200 Subject: [PATCH 4/5] connected: handle partial clones in incremental check Teach the incremental connectivity check to handle partial clones. When a tree or blob cannot be read, check whether it is a promisor object before reporting an error. Promisor tips with missing targets are skipped during tag peeling. 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 4f6f48539c7771..d4be3075845c75 100644 --- a/connected.c +++ b/connected.c @@ -128,6 +128,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) @@ -264,6 +265,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), @@ -315,6 +320,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; @@ -339,6 +348,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) { @@ -373,8 +383,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, }; @@ -390,6 +402,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; @@ -423,13 +437,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; } @@ -575,6 +592,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) @@ -743,8 +762,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 3de8985195de8e..f936bb3204115a 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" ' From 452d6b921a97baca199972729b86671f382a17fd Mon Sep 17 00:00:00 2001 From: Kristofer Karlsson Date: Mon, 31 Aug 2026 20:38:24 +0200 Subject: [PATCH 5/5] connected: use two-paint walk for in-process boundary search NOTE: This commit is an experimental proof of concept to illustrate the optimization potential. It is not meant to be merged. The incremental connectivity check shells out to rev-list to find the boundary between incoming and locally-known commits. With many local refs, this subprocess dominates the total cost even though only a handful of new commits need to be identified. Add a bespoke boundary walk in commit-reach.c that avoids spawning rev-list entirely. This is based on the existing paint_down_to_common() with some minor differences. The two sides naturally represent the incoming tips and a subset of the old existing tips. If the incoming tip side gets exhausted a valid boundary has been found. The walk aborts and falls back to using rev-list if it walks too far. This is a heuristic based on the commit-graph; commits that exist in the commit-graph are typically reachable from local tips. This is not required for correctness, since it is always valid to fall back if the boundary cannot be proven. Two things make this heuristic avoid work that would otherwise slow it down: 1. For the happy path, most pushes have a nearby common ancestor with one of the main local tips or the previously seen tip. This is also the set of tips we use for walking, instead of loading all tips -- which may be too many to be efficient. 2. For the negative case where a boundary can not be found this way, the walk quickly aborts as soon as it reaches the commit-graph. Thus, the efficiency of this approach depends on the commit-graph being somewhat up to date, but it does not need to be perfectly up to date. Gate this on the commit-graph having corrected commit dates (the generation data v2 chunk), since v1 generation numbers can diverge too far from topological depth. When no suitable commit-graph is available, fall back to the rev-list path. Benchmarks on a large repository (~3M commits, ~10K local refs), 1 new commit changing 1 file, measured with hyperfine --warmup 1: rev-list: 1877 ms +/- 32 ms incremental (rev-list boundary): 365 ms +/- 46 ms incremental (paint boundary): 14 ms +/- 1 ms (134x faster) Signed-off-by: Kristofer Karlsson --- builtin/fetch.c | 20 ++++++- builtin/receive-pack.c | 9 +++ commit-reach.c | 104 ++++++++++++++++++++++++++++++++++ commit-reach.h | 13 +++++ connected.c | 69 +++++++++++++++++++++- connected.h | 9 +++ t/t5412-connectivity-check.sh | 35 ++++++++++++ 7 files changed, 257 insertions(+), 2 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index ab7db2be06d14c..9a3176f10e26b1 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -1247,14 +1247,23 @@ static int store_updated_refs(struct display_state *display_state, if (!connectivity_checked) { struct check_connected_options opt = CHECK_CONNECTED_INIT; + struct ref *r; + + for (r = ref_map; r; r = r->next) { + if (r->peer_ref && !is_null_oid(&r->peer_ref->old_oid)) + oid_array_append(&opt.old_tips, + &r->peer_ref->old_oid); + } opt.exclude_hidden_refs_section = "fetch"; rm = ref_map; if (check_connected(iterate_ref_map, &rm, &opt)) { rc = error(_("%s did not send all necessary objects"), display_state->url); + oid_array_clear(&opt.old_tips); goto abort; } + oid_array_clear(&opt.old_tips); } /* @@ -1391,6 +1400,7 @@ static int check_exist_and_connected(struct ref *ref_map) struct ref *rm = ref_map; struct check_connected_options opt = CHECK_CONNECTED_INIT; struct ref *r; + int ret; /* * If we are deepening a shallow clone we already have these @@ -1420,9 +1430,17 @@ static int check_exist_and_connected(struct ref *ref_map) return -1; } + for (r = rm; r; r = r->next) { + if (r->peer_ref && !is_null_oid(&r->peer_ref->old_oid)) + oid_array_append(&opt.old_tips, + &r->peer_ref->old_oid); + } + opt.quiet = 1; opt.exclude_hidden_refs_section = "fetch"; - return check_connected(iterate_ref_map, &rm, &opt); + ret = check_connected(iterate_ref_map, &rm, &opt); + oid_array_clear(&opt.old_tips); + return ret; } static int fetch_and_consume_refs(struct display_state *display_state, diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index 86933d8d7e4b33..a5b4cd0900c67a 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -2059,6 +2059,14 @@ static void execute_commands(struct command *commands, /* ...else, continue without relaying sideband */ } + for (cmd = commands; cmd; cmd = cmd->next) { + if (!is_null_oid(&cmd->old_oid) && + !is_null_oid(&cmd->new_oid) && + !cmd->skip_update) + oid_array_append(&opt.old_tips, + &cmd->old_oid); + } + data.cmds = commands; data.si = si; opt.err_fd = err_fd; @@ -2074,6 +2082,7 @@ static void execute_commands(struct command *commands, finish_async(&muxer); strvec_clear(&env); + oid_array_clear(&opt.old_tips); } reject_updates_to_hidden(commands); diff --git a/commit-reach.c b/commit-reach.c index b16eea2355356f..d5281fede3f098 100644 --- a/commit-reach.c +++ b/commit-reach.c @@ -276,6 +276,110 @@ static int paint_down_to_common(struct repository *r, return 0; } +static void commit_list_drop_flags(struct commit_list **listp, unsigned flags) +{ + while (*listp) { + if ((*listp)->item->object.flags & flags) { + struct commit_list *entry = *listp; + *listp = entry->next; + free(entry); + } else { + listp = &(*listp)->next; + } + } +} + +/* + * Paint commits reachable from 'tips' with PARENT1 and commits + * reachable from 'bases' with PARENT2, then collect the commits + * painted only with PARENT1 into new_commits. + * + * Uses corrected commit dates for generation ordering. Bails out + * once the walk drops below the generation where incoming commits + * enter the commit-graph, since the narrow seed set may not + * converge efficiently below that point. + * + * Returns -1 if the walk gets aborted. + */ +int repo_find_boundary_commits(struct repository *r, + struct commit **bases, size_t nr_bases, + size_t nr_tips, struct commit **tips, + struct commit_list **new_commits) +{ + struct paint_state state = { + .queue = { compare_commits_by_gen_then_commit_date } + }; + struct commit *commit; + timestamp_t gen_floor = GENERATION_NUMBER_INFINITY; + int ret = -1; + size_t i; + + if (!corrected_commit_dates_enabled(r)) + return -1; + + state.last_gen = GENERATION_NUMBER_INFINITY; + state.topo_ceiling = GENERATION_NUMBER_INFINITY; + + for (i = 0; i < nr_tips; i++) + paint_queue_put(&state, tips[i], PARENT1); + for (i = 0; i < nr_bases; i++) + paint_queue_put(&state, bases[i], PARENT2); + + while ((commit = paint_queue_get(&state))) { + struct commit_list *parents; + unsigned flags; + timestamp_t gen = commit_graph_generation(commit); + + flags = commit->object.flags & (PARENT1 | PARENT2); + + if (flags == PARENT1) + commit_list_insert(commit, new_commits); + + for (parents = commit->parents; parents; parents = parents->next) { + struct commit *p = parents->item; + if ((p->object.flags & flags) == flags) + continue; + if (repo_parse_commit(r, p)) + goto done; + if (flags == PARENT1 && + gen == GENERATION_NUMBER_INFINITY) { + timestamp_t pgen = commit_graph_generation(p); + if (pgen < gen_floor) + gen_floor = pgen; + } + paint_queue_put(&state, p, flags); + } + + /* + * gen_floor is the lowest generation where an incoming + * commit (PARENT1, gen=INFINITY) first enters the + * commit-graph. Below this point the narrow seed set + * may not converge efficiently. + */ + if (!state.parent1_count || gen < gen_floor) + break; + } + + if (state.parent1_count) + goto done; + + commit_list_drop_flags(new_commits, PARENT2); + + ret = 0; + +done: + clear_prio_queue(&state.queue); + for (i = 0; i < nr_tips; i++) + clear_commit_marks(tips[i], all_flags); + for (i = 0; i < nr_bases; i++) + clear_commit_marks(bases[i], all_flags); + if (ret) { + commit_list_free(*new_commits); + *new_commits = NULL; + } + return ret; +} + static int merge_bases_many(struct repository *r, struct commit *one, int n, struct commit **twos, diff --git a/commit-reach.h b/commit-reach.h index f908d305b16902..44c5a208aaf3a9 100644 --- a/commit-reach.h +++ b/commit-reach.h @@ -167,4 +167,17 @@ int get_branch_base_for_tip(struct repository *r, struct commit **bases, size_t bases_nr); +/* + * Find commits reachable from 'tips' but not from 'bases' using a + * two-paint walk. Requires a commit-graph with corrected commit + * dates (generation data v2). Returns 0 on success, -1 if the + * walk gets aborted. + * + * All tips and bases must already be parsed. + */ +int repo_find_boundary_commits(struct repository *r, + struct commit **bases, size_t nr_bases, + size_t nr_tips, struct commit **tips, + struct commit_list **new_commits); + #endif diff --git a/connected.c b/connected.c index d4be3075845c75..9640d8d8c05b63 100644 --- a/connected.c +++ b/connected.c @@ -2,6 +2,7 @@ #include "git-compat-util.h" #include "commit.h" +#include "commit-reach.h" #include "config.h" #include "gettext.h" #include "hex.h" @@ -19,6 +20,7 @@ #include "promisor-remote.h" #include "tree-walk.h" #include "tree.h" +#include "refs.h" static int promised_object_cb(const struct object_id *oid UNUSED, struct object_info *oi UNUSED, @@ -569,6 +571,70 @@ static int parse_shallow_file_gently(const char *path, return err; } +/* TODO: make seed refs configurable (e.g. transfer.connectivitySeedRefs) */ +static const char *boundary_seed_refs[] = { + "HEAD", + "refs/remotes/origin/HEAD", + "refs/remotes/origin/master", +}; + +static int find_boundary_from_commit_graph(struct oid_array *commit_tips, + struct oid_array *old_tips, + struct commit_list **new_commits) +{ + struct commit **bases = NULL; + size_t nr_bases = 0; + size_t alloc_bases; + struct commit **tips; + int ret; + size_t i; + + alloc_bases = ARRAY_SIZE(boundary_seed_refs) + old_tips->nr; + ALLOC_ARRAY(bases, alloc_bases); + + for (i = 0; i < old_tips->nr; i++) { + struct commit *c; + c = lookup_commit(the_repository, &old_tips->oid[i]); + if (!c || repo_parse_commit(the_repository, c)) + continue; + bases[nr_bases++] = c; + } + + for (i = 0; i < ARRAY_SIZE(boundary_seed_refs); i++) { + struct object_id oid; + struct commit *c; + if (!refs_resolve_ref_unsafe(get_main_ref_store(the_repository), + boundary_seed_refs[i], + RESOLVE_REF_READING, &oid, NULL)) + continue; + c = lookup_commit(the_repository, &oid); + if (!c || repo_parse_commit(the_repository, c)) + continue; + bases[nr_bases++] = c; + } + if (!nr_bases) { + free(bases); + return -1; + } + + ALLOC_ARRAY(tips, commit_tips->nr); + for (i = 0; i < commit_tips->nr; i++) { + tips[i] = lookup_commit(the_repository, &commit_tips->oid[i]); + if (!tips[i] || repo_parse_commit(the_repository, tips[i])) { + free(tips); + free(bases); + return -1; + } + } + + ret = repo_find_boundary_commits(the_repository, + bases, nr_bases, + commit_tips->nr, tips, new_commits); + free(tips); + free(bases); + return ret; +} + /* * Find the connectivity boundary: the set of new commits not yet * reachable from local refs. Feeds commit_tips to rev-list via @@ -719,7 +785,8 @@ static int check_connected_incremental(oid_iterate_fn fn, void *cb_data, trace2_region_leave("connectivity", "collect-tips", the_repository); trace2_region_enter("connectivity", "find-boundary", the_repository); - if (!err) + if (!err && find_boundary_from_commit_graph(&commit_tips, &opt->old_tips, + &new_commits)) err = find_connectivity_boundary(opt, &commit_tips, &new_commits, &vs); trace2_region_leave("connectivity", "find-boundary", the_repository); diff --git a/connected.h b/connected.h index 16b2c84f2e35fc..636b18fb1e4138 100644 --- a/connected.h +++ b/connected.h @@ -4,6 +4,8 @@ struct object_id; struct transport; +#include "oid-array.h" + /* * Take callback data, and return next object name in the buffer. * When called after returning the name for the last object, return -1 @@ -53,6 +55,13 @@ struct check_connected_options { * already-reachable refs. */ const char *exclude_hidden_refs_section; + + /* + * Old values of refs being updated. Used as PARENT2 seeds + * for the in-process boundary walk, since they are almost + * always direct ancestors of the incoming tips. + */ + struct oid_array old_tips; }; #define CHECK_CONNECTED_INIT { 0 } diff --git a/t/t5412-connectivity-check.sh b/t/t5412-connectivity-check.sh index f936bb3204115a..2566f0f5970d1d 100755 --- a/t/t5412-connectivity-check.sh +++ b/t/t5412-connectivity-check.sh @@ -724,4 +724,39 @@ test_expect_success 'incremental: boundary search errors routed to err-file' ' ) ' +# Commit-graph boundary: verify the in-process boundary search +# using the commit-graph. + +test_expect_success 'incremental uses commit-graph boundary search' ' + test_when_finished "rm -rf graph-src graph-dst" && + + git init graph-src && + test_commit -C graph-src --no-tag base file.txt && + git clone graph-src graph-dst && + git -C graph-dst commit-graph write --reachable && + test_commit -C graph-src --no-tag update file.txt updated && + + set_connectivity_check graph-dst incremental && + GIT_TRACE2_EVENT="$(pwd)/trace-graph.txt" \ + git -C graph-dst fetch origin main && + test_region connectivity find-boundary trace-graph.txt && + test_grep ! rev-list trace-graph.txt +' + +test_expect_success 'incremental falls back to rev-list without commit-graph' ' + test_when_finished "rm -rf no-graph-src no-graph-dst" && + + git init no-graph-src && + test_commit -C no-graph-src --no-tag base file.txt && + git clone no-graph-src no-graph-dst && + test_commit -C no-graph-src --no-tag update file.txt updated && + + rm -rf no-graph-dst/.git/objects/info/commit-graph* && + set_connectivity_check no-graph-dst incremental && + GIT_TRACE2_EVENT="$(pwd)/trace-no-graph.txt" \ + git -C no-graph-dst fetch origin main && + test_region connectivity find-boundary trace-no-graph.txt && + test_grep rev-list trace-no-graph.txt +' + test_done