diff --git a/Documentation/Makefile b/Documentation/Makefile index f8dea4b3953250..1747f4a00be120 100644 --- a/Documentation/Makefile +++ b/Documentation/Makefile @@ -122,6 +122,7 @@ TECH_DOCS += technical/bitmap-format TECH_DOCS += technical/build-systems TECH_DOCS += technical/bundle-uri TECH_DOCS += technical/commit-graph +TECH_DOCS += technical/connectivity-check TECH_DOCS += technical/directory-rename-detection TECH_DOCS += technical/hash-function-transition TECH_DOCS += technical/large-object-promisors diff --git a/Documentation/config/transfer.adoc b/Documentation/config/transfer.adoc index f1ce50f4a6e6ba..f77b25ff8b9732 100644 --- a/Documentation/config/transfer.adoc +++ b/Documentation/config/transfer.adoc @@ -1,3 +1,33 @@ +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: ++ +-- +`full` (default);; + Walk 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, and it can be slower + than `full` when there are many incoming commits. + Falls back to `full` for clones, pushes into an empty + repository and deepening fetches. +`auto`;; +`auto=`;; + Choose between `full` and `incremental` with a heuristic. + Currently `incremental` is used when there are at most + __ incoming commits (1000 by default), and `full` + otherwise. The heuristic may change in future versions. +-- ++ +Unknown values are treated as `full`, with a warning. + transfer.credentialsInUrl:: A configured URL can contain plaintext credentials in the form `://:@/`. You may want diff --git a/Documentation/rev-list-options.adoc b/Documentation/rev-list-options.adoc index bd9f3456902c8f..285fb3809ff8f4 100644 --- a/Documentation/rev-list-options.adoc +++ b/Documentation/rev-list-options.adoc @@ -1102,6 +1102,11 @@ It is an error to use `--missing-only` without `--missing=print` or stronger than `--missing=allow-promisor` because it limits the traversal, rather than just silencing errors about missing objects. + +`--connectivity-check=(full|incremental|auto[=])`:: + (For internal use only.) Used by `check_connected()` to + select the connectivity check algorithm; see + `transfer.connectivityCheck` in linkgit:git-config[1]. endif::git-rev-list[] `--no-walk[=(sorted|unsorted)]`:: diff --git a/Documentation/technical/connectivity-check.adoc b/Documentation/technical/connectivity-check.adoc new file mode 100644 index 00000000000000..8b7d619d4c8bc0 --- /dev/null +++ b/Documentation/technical/connectivity-check.adoc @@ -0,0 +1,259 @@ +Connectivity checking +===================== + +After receiving new objects via fetch, push (receive-pack), clone, +or bundle, Git verifies that the new reference tips do not leave +the repository in a state where reachable objects are missing. +This verification is called the connectivity check. + +Connectivity invariant +---------------------- + +A repository is connected when every object reachable from its +references is available locally (with exceptions noted below). +The reverse does not hold: objects that exist in the object +database but are not reachable from any reference may be +incomplete or lack a connected closure. + +The connectivity check maintains this invariant when references +are updated. It trusts that all objects reachable from existing +reference tips are already fully connected and verifies that the +new reference tips do not introduce references to unavailable +objects. Verification is permitted to stop when it reaches +objects already reachable from existing references, since their +closure is already connected. These trusted references include +local references and references from alternate object stores. + +Without this check, a truncated or corrupted transfer could leave +a repository in a state where later history walks encounter +missing objects. + +Exceptions +~~~~~~~~~~ + +Gitlink entries (submodule references) are excluded from +connectivity checking. Their target objects belong to a separate +repository. + +In partial clones, objects promised by a promisor remote are +accepted as connected without requiring local existence. The +check excludes promisor objects from traversal so that it does +not trigger on-demand fetches for them. + +Full connectivity check +----------------------- + +The full connectivity check runs a `rev-list` subprocess, feeding +the new reference tips via stdin. A normal invocation is roughly: + + git rev-list --objects --stdin --not --all --quiet + --alternate-refs [--exclude-promisor-objects] + +In a partial clone, objects received from a promisor remote do +not need to be checked: anything they reference is promised by +that remote and can be fetched on demand. So if every new tip is +found in a promisor packfile, `rev-list` is not run at all. +Otherwise it runs with `--exclude-promisor-objects`, so that the +traversal stops at promisor objects instead of fetching them. + +Consider the following graph after a fetch, where all reference +tips point directly to commits. For simplicity, only local +references appear on the already-connected side; alternate refs +play the same role. N3 is a merge commit: + + /-------------L2 + / + C1---B1---C2---B2-----L1 + \ \ + N1 N3---T2 + \ / + N2-----------T1 + + L1, L2: local refs + T1, T2: incoming tips (new refs) + N1, N2, N3: incoming commits (N3 is a merge) + B1, B2: boundary commits (already connected) + C1, C2: already connected (but not boundary) + +The incoming set is the commits reachable from the incoming +tips but not from the already-connected side. Boundary commits +are the already-connected commits at the edge of that set. Here +B1 is an ancestor of B2, which happens when incoming branches +fork at different depths in the existing history. + +The `--not --all` arguments establish the trust boundary: the +incoming set is the commits reachable from the new tips but not +from any existing reference. + +The traversal then walks the trees of the boundary commits and +marks those objects uninteresting. Trees of incoming commits +are then walked, stopping at objects already marked as +uninteresting. The cost of marking boundary trees uninteresting +dominates when the boundary commits have large trees, even when +the incoming changes are small. + +Deepening fetches +~~~~~~~~~~~~~~~~~ + +For deepening fetches (where the shallow boundary moves), the +full check omits `--not --all`. There is no existing-reference +boundary at which the walk can stop. Instead, traversal follows +the effective shallow boundary supplied for the deepened +repository. The new content may be below the old shallow +boundary even when the tips themselves have not changed. + +Non-commit tips +~~~~~~~~~~~~~~~ + +When a new reference points to a non-commit object, such as a +tag, tree, or blob, that object is not part of the commit walk. +These non-commit tips are verified during the tree and blob +traversal that follows the commit walk within the same rev-list +invocation. + +Incremental connectivity check +------------------------------ + +The incremental mode, selected by +`transfer.connectivityCheck=incremental`, avoids traversing the +full tree walk of the boundary commits. Instead, it verifies +each incoming commit's tree against the already-trusted trees of +its parent commits. + +The same `--not --all` boundary as the full check identifies the +incoming commits. The incremental mode reuses this boundary but +replaces what happens afterwards: rather than walking boundary +trees to mark objects uninteresting, it compares each incoming +commit's tree against its parents' trees, processing commits in +topological order (ancestors first). This all happens inside +the same `rev-list` invocation, and the promisor fast path above +still runs first. + +Trust model +~~~~~~~~~~~ + +A tree is trusted when its transitive object closure is known to +be connected. + +Every parent of an incoming commit is either on the +already-connected side of the `--not --all` boundary (a boundary +parent whose tree is trusted by definition) or is an earlier +incoming commit that was already verified due to the topological +processing order. In both cases the parent's tree is trusted, so +all parent trees can be unconditionally used as comparison bases. + +Once an incoming commit's tree has been verified, it is trusted +and can serve as a comparison base for later descendants. This +gives an inductive correctness argument: the base case is the +trust boundary established by `--not --all`, and the inductive +step is that verifying a commit extends the trusted set. + +Tree states +~~~~~~~~~~~ + +The verifier tracks tree OIDs in three states: + +untrusted:: + The tree has not yet been established as connected. This + is the implicit state of an OID not present in the state + map. + +trusted:: + The tree is known to have a connected transitive closure, + but its direct entries have not yet been published into the + verifier's trusted object sets. + +expanded:: + The tree is trusted and its direct non-gitlink entries + have also been published into the trusted object sets. + +State transitions are monotonic: a tree may move from untrusted +to trusted to expanded, but never backwards. An untrusted tree +that is successfully verified goes directly to expanded. + +Blobs require only trusted/untrusted state: a blob becomes +trusted when it is found in a trusted tree or when its existence +and type have been verified directly. + +The trusted/expanded distinction is an optimization. An expanded +parent does not need to be reread merely to publish entries that +are already trusted, though it may still be read when same-path +subtree bases are needed for recursive comparison. + +Algorithm +~~~~~~~~~ + +At a high level: + + verify(commits): + sort topologically (ancestors first) + for each commit: + mark parent root trees as trusted + verify_tree(commit.tree, parent root trees) + + verify_tree(tree, base_trees): + if tree already trusted: return + read tree, collect entries not already trusted + for each available base tree: + publish its entries as trusted + record same-path subtrees as bases + for each collected entry: + if now trusted: skip + if blob: verify blob connectivity and type + if tree: verify_tree(entry, its recorded bases) + mark tree as expanded + +The important ordering within `verify_tree` is that entries from +the new tree are collected before the base trees are scanned, but +are processed only afterwards. Trust learned from any base can +therefore eliminate work before recursive verification begins. + +Same-path parent subtrees are passed down as comparison bases +during recursive descent. If no comparison base is available, +the new subtree is verified from scratch. + +Example +~~~~~~~ + +Consider a commit that changes one file under `lib/` and moves an +unchanged subtree from `src/` to `dev/`: + + Parent tree New tree + +-- src/ (aaa) +-- dev/ (aaa) + +-- lib/ (bbb) +-- lib/ (ccc) + +-- foo.c (ddd) +-- foo.c (ddd) + +-- bar.c (eee) +-- bar.c (fff) + +Scanning the parent makes `aaa` trusted even though it moved from +`src/` to `dev/`, so that subtree is skipped. The changed `ccc` +subtree is compared against its same-path parent `bbb`; scanning +`bbb` makes `ddd` and `eee` trusted, leaving only the new `fff` +blob to be checked. + +This illustrates two properties: + +* Trust is OID-based rather than path-based. An unchanged subtree + is recognized after a move. + +* Same-path parent subtrees provide recursive comparison bases. + These bases improve pruning efficiency but are not required for + correctness; a new subtree can always be verified from scratch. + +Multiple parents +~~~~~~~~~~~~~~~~ + +For a merge commit, root trees from all parents are comparison +bases. When several parents contain a same-path subtree, each +matching subtree is collected as a recursive comparison base. + +Entries published from any trusted parent become globally trusted, +so a matching tree or blob entry present in any parent can be +skipped while verifying the merge tree. + +Missing comparison bases +~~~~~~~~~~~~~~~~~~~~~~~~ + +A promised base tree may not be locally available for comparison. +In that case the verifier skips that base and verifies the new +subtree without it. The missing comparison can reduce pruning but +does not affect correctness. diff --git a/Documentation/technical/meson.build b/Documentation/technical/meson.build index 9ce11d5e484d9c..c439a7a072d395 100644 --- a/Documentation/technical/meson.build +++ b/Documentation/technical/meson.build @@ -11,6 +11,7 @@ articles = [ 'build-systems.adoc', 'bundle-uri.adoc', 'commit-graph.adoc', + 'connectivity-check.adoc', 'directory-rename-detection.adoc', 'hash-function-transition.adoc', 'large-object-promisors.adoc', diff --git a/Makefile b/Makefile index a96be506b59f2d..fa42e295e22b13 100644 --- a/Makefile +++ b/Makefile @@ -1339,6 +1339,7 @@ LIB_OBJS += trailer.o LIB_OBJS += transport-helper.o LIB_OBJS += transport.o LIB_OBJS += tree-diff.o +LIB_OBJS += tree-verify.o LIB_OBJS += tree-walk.o LIB_OBJS += tree.o LIB_OBJS += unpack-trees.o diff --git a/builtin/clone.c b/builtin/clone.c index 44037dd428e402..1532361ea0ee7c 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -572,7 +572,7 @@ static void update_remote_refs(const struct ref *refs, opt.transport = transport; opt.progress = transport->progress; - if (check_connected(iterate_ref_map, &rm, &opt)) + if (check_connected(the_repository, iterate_ref_map, &rm, &opt)) die(_("remote did not send all necessary objects")); } diff --git a/builtin/fetch.c b/builtin/fetch.c index b2decc6cfdb8ae..0ef7fe92f11c36 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -1250,7 +1250,8 @@ static int store_updated_refs(struct display_state *display_state, opt.exclude_hidden_refs_section = "fetch"; rm = ref_map; - if (check_connected(iterate_ref_map, &rm, &opt)) { + if (check_connected(the_repository, iterate_ref_map, + &rm, &opt)) { rc = error(_("%s did not send all necessary objects"), display_state->url); goto abort; @@ -1422,7 +1423,7 @@ static int check_exist_and_connected(struct ref *ref_map) opt.quiet = 1; opt.exclude_hidden_refs_section = "fetch"; - return check_connected(iterate_ref_map, &rm, &opt); + return check_connected(the_repository, iterate_ref_map, &rm, &opt); } static int fetch_and_consume_refs(struct display_state *display_state, diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index 1d5b050beb4e72..4f8f58950a9be1 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -1385,7 +1385,8 @@ static int update_shallow_ref(struct command *cmd, struct shallow_info *si) oid_array_append(&extra, &si->shallow->oid[i]); setup_alternate_shallow(&shallow_lock, &opt.shallow_file, &extra); - if (check_connected(command_singleton_iterator, cmd, &opt)) { + if (check_connected(the_repository, command_singleton_iterator, + cmd, &opt)) { rollback_shallow_file(the_repository, &shallow_lock); oid_array_clear(&extra); return -1; @@ -1827,8 +1828,9 @@ static void set_connectivity_errors(struct command *commands, odb_transaction_env(transaction, &env); opt.env = env.v; - if (!check_connected(command_singleton_iterator, &singleton, - &opt)) + if (!check_connected(the_repository, + command_singleton_iterator, + &singleton, &opt)) continue; cmd->error_string = "missing necessary objects"; @@ -2090,7 +2092,9 @@ static void execute_commands(struct command *commands, opt.env = env.v; opt.exclude_hidden_refs_section = "receive"; - if (check_connected(iterate_receive_command_list, &data, &opt)) + if (check_connected(the_repository, + iterate_receive_command_list, + &data, &opt)) set_connectivity_errors(commands, si, transaction); if (use_sideband) diff --git a/builtin/rev-list.c b/builtin/rev-list.c index 0faa833facfd4b..85efc0fac8eda6 100644 --- a/builtin/rev-list.c +++ b/builtin/rev-list.c @@ -28,6 +28,43 @@ #include "commit-reach.h" #include "quote.h" #include "strbuf.h" +#include "tree-verify.h" +#include "parse.h" + +#define INCREMENTAL_MAX_COMMITS 1000 + +enum connectivity_check_mode { + CONNECTIVITY_CHECK_FULL, + CONNECTIVITY_CHECK_INCREMENTAL, + CONNECTIVITY_CHECK_AUTO, +}; + +struct connectivity_check { + enum connectivity_check_mode mode; + unsigned long max_commits; +}; + +static void parse_connectivity_check(struct repository *repo, + const char *arg, + struct connectivity_check *check) +{ + const char *cutoff; + + if (!strcmp(arg, "full")) + check->mode = CONNECTIVITY_CHECK_FULL; + else if (!strcmp(arg, "incremental")) + check->mode = CONNECTIVITY_CHECK_INCREMENTAL; + else if (!strcmp(arg, "auto")) + check->mode = CONNECTIVITY_CHECK_AUTO; + else if (skip_prefix(arg, "auto=", &cutoff) && + git_parse_ulong(cutoff, &check->max_commits)) + check->mode = CONNECTIVITY_CHECK_AUTO; + else + die(_("unknown --connectivity-check mode '%s'"), arg); + + if (check->mode != CONNECTIVITY_CHECK_FULL) + repo->fetch_if_missing = 0; +} struct rev_list_info { struct rev_info *revs; @@ -727,6 +764,9 @@ int cmd_rev_list(int argc, int bisect_find_all = 0; int use_bitmap_index = 0; int filter_provided_objects = 0; + struct connectivity_check connectivity_check = { + .max_commits = INCREMENTAL_MAX_COMMITS, + }; const char *show_progress = NULL; int ret = 0; @@ -769,6 +809,8 @@ int cmd_rev_list(int argc, if (!strcmp(arg, "--exclude-promisor-objects")) { repo->fetch_if_missing = 0; revs.exclude_promisor_objects = 1; + } else if (skip_prefix(arg, "--connectivity-check=", &arg)) { + parse_connectivity_check(repo, arg, &connectivity_check); } else if (skip_prefix(arg, "--missing=", &arg)) { parse_missing_action_value(repo, arg); } else if (!strcmp(arg, "--missing-only")) { @@ -848,6 +890,8 @@ int cmd_rev_list(int argc, if (!strcmp(arg, "--exclude-promisor-objects")) continue; /* already handled above */ + if (starts_with(arg, "--connectivity-check=")) + continue; /* already handled above */ if (skip_prefix(arg, "--missing=", &arg)) continue; /* already handled above */ @@ -969,6 +1013,39 @@ int cmd_rev_list(int argc, prepare_maximal_independent(&revs); + /* + * Without a boundary of negative revisions, as in a clone or a + * push into an empty repository, every incoming commit is new. + * The incremental check would still work, but it would verify + * the whole object graph like the full check, with extra + * per-commit overhead on top, so use the full check. + * + * Otherwise the walk is limited, and revs.commits already holds + * all incoming commits, so counting them for "auto" is cheap. + */ + if (!revs.limited) { + connectivity_check.mode = CONNECTIVITY_CHECK_FULL; + } else if (connectivity_check.mode == CONNECTIVITY_CHECK_AUTO) { + if (commit_list_count(revs.commits) <= + connectivity_check.max_commits) + connectivity_check.mode = CONNECTIVITY_CHECK_INCREMENTAL; + else + connectivity_check.mode = CONNECTIVITY_CHECK_FULL; + } + + if (connectivity_check.mode == CONNECTIVITY_CHECK_INCREMENTAL) { + struct commit *commit; + struct commit_list *new_commits = NULL; + + while ((commit = get_revision(&revs)) != NULL) + commit_list_insert(commit, &new_commits); + + if (verify_commits_incremental(repo, &new_commits, + revs.exclude_promisor_objects)) + die(_("connectivity check failed")); + commit_list_free(new_commits); + } + if (revs.tree_objects) mark_edges_uninteresting(&revs, show_edge, 0); diff --git a/bundle.c b/bundle.c index f55a521b2a1f12..0a68e856f57500 100644 --- a/bundle.c +++ b/bundle.c @@ -259,7 +259,7 @@ int verify_bundle(struct repository *r, if (ret) goto cleanup; - if ((ret = check_connected(iterate_ref_map, &iter, &opts))) + if ((ret = check_connected(r, iterate_ref_map, &iter, &opts))) error(_("some prerequisite commits exist in the object store, " "but are not connected to the repository's history")); diff --git a/connected.c b/connected.c index 929b9bd28d6fab..8ceaa45644b13c 100644 --- a/connected.c +++ b/connected.c @@ -1,9 +1,9 @@ -#define USE_THE_REPOSITORY_VARIABLE - #include "git-compat-util.h" +#include "config.h" #include "gettext.h" #include "hex.h" #include "odb.h" +#include "parse.h" #include "run-command.h" #include "sigchain.h" #include "connected.h" @@ -35,23 +35,24 @@ static int promised_object_cb(const struct object_id *oid UNUSED, * objects in non-promisor packs, in which case we'll have to fall back to the * rev-list-based connectivity checks. Returns a negative error code on error. */ -static int check_connected_promisor(oid_iterate_fn fn, +static int check_connected_promisor(struct repository *repo, + oid_iterate_fn fn, void *cb_data, const struct object_id **oid) { struct odb_for_each_object_options opts = { .flags = ODB_FOR_EACH_OBJECT_PROMISOR_ONLY, - .prefix_hex_len = the_repository->hash_algo->hexsz, + .prefix_hex_len = repo->hash_algo->hexsz, }; int err; - odb_reprepare(the_repository->objects); + odb_reprepare(repo->objects); do { bool found = false; opts.prefix = *oid; - err = odb_for_each_object_ext(the_repository->objects, NULL, + err = odb_for_each_object_ext(repo->objects, NULL, promised_object_cb, &found, &opts); if (err < 0) return err; @@ -67,6 +68,45 @@ static int check_connected_promisor(oid_iterate_fn fn, return 1; } +/* + * Fills in the --connectivity-check mode to pass to rev-list. + */ +static void connectivity_check_mode(struct repository *repo, + struct check_connected_options *opt, + struct strbuf *mode) +{ + const char *algorithm = NULL, *cutoff; + unsigned long max_commits; + + /* + * Deepening fetches do not have the usual boundary of + * existing references, so use the full check for them. + */ + if (opt->is_deepening_fetch) { + strbuf_addstr(mode, "full"); + return; + } + + if (repo_config_get_string_tmp(repo, + "transfer.connectivitycheck", + &algorithm)) + algorithm = "full"; + if (!strcasecmp(algorithm, "full")) + strbuf_addstr(mode, "full"); + else if (!strcasecmp(algorithm, "incremental")) + strbuf_addstr(mode, "incremental"); + else if (!strcasecmp(algorithm, "auto")) + strbuf_addstr(mode, "auto"); + else if (skip_iprefix(algorithm, "auto=", &cutoff) && + git_parse_ulong(cutoff, &max_commits)) + strbuf_addf(mode, "auto=%lu", max_commits); + else { + warning(_("unknown transfer.connectivityCheck algorithm '%s', " + "using 'full'"), algorithm); + strbuf_addstr(mode, "full"); + } +} + /* * If we feed all the commits we want to verify to this command * @@ -78,12 +118,14 @@ static int check_connected_promisor(oid_iterate_fn fn, * * Returns 0 if everything is connected, non-zero otherwise. */ -int check_connected(oid_iterate_fn fn, void *cb_data, +int check_connected(struct repository *repo, + oid_iterate_fn fn, void *cb_data, struct check_connected_options *opt) { struct child_process rev_list = CHILD_PROCESS_INIT; FILE *rev_list_in; struct check_connected_options defaults = CHECK_CONNECTED_INIT; + struct strbuf mode = STRBUF_INIT; const struct object_id *oid; int err = 0; struct packed_git *new_pack = NULL; @@ -101,8 +143,8 @@ int check_connected(oid_iterate_fn fn, void *cb_data, return err; } - if (repo_has_promisor_remote(the_repository)) { - err = check_connected_promisor(fn, cb_data, &oid); + if (repo_has_promisor_remote(repo)) { + err = check_connected_promisor(repo, fn, cb_data, &oid); if (err) { if (opt->err_fd) close(opt->err_fd); @@ -119,7 +161,7 @@ int check_connected(oid_iterate_fn fn, void *cb_data, strvec_push(&rev_list.args,"rev-list"); strvec_push(&rev_list.args, "--objects"); strvec_push(&rev_list.args, "--stdin"); - if (repo_has_promisor_remote(the_repository)) + if (repo_has_promisor_remote(repo)) strvec_push(&rev_list.args, "--exclude-promisor-objects"); if (!opt->is_deepening_fetch) { strvec_push(&rev_list.args, "--not"); @@ -133,6 +175,9 @@ int check_connected(oid_iterate_fn fn, void *cb_data, if (opt->progress) strvec_pushf(&rev_list.args, "--progress=%s", _("Checking connectivity")); + connectivity_check_mode(repo, opt, &mode); + strvec_pushf(&rev_list.args, "--connectivity-check=%s", mode.buf); + strbuf_release(&mode); rev_list.git_cmd = 1; if (opt->env) @@ -160,7 +205,7 @@ int check_connected(oid_iterate_fn fn, void *cb_data, 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, + new_pack = add_packed_git(repo, idx_file.buf, idx_file.len, 1); strbuf_release(&idx_file); } diff --git a/connected.h b/connected.h index 16b2c84f2e35fc..0624965baa7fb1 100644 --- a/connected.h +++ b/connected.h @@ -2,6 +2,7 @@ #define CONNECTED_H struct object_id; +struct repository; struct transport; /* @@ -66,7 +67,8 @@ struct check_connected_options { * * If "opt" is NULL, behaves as if CHECK_CONNECTED_INIT was passed. */ -int check_connected(oid_iterate_fn fn, void *cb_data, +int check_connected(struct repository *repo, + oid_iterate_fn fn, void *cb_data, struct check_connected_options *opt); #endif /* CONNECTED_H */ diff --git a/fetch-pack.c b/fetch-pack.c index 30a4dd6119049d..2cf8f8d653fc53 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -2186,7 +2186,8 @@ struct ref *fetch_pack(struct fetch_pack_args *args, opt.shallow_file = alternate_shallow_file; if (args->deepen) opt.is_deepening_fetch = 1; - if (check_connected(iterate_ref_map, &iterator, &opt)) { + if (check_connected(the_repository, iterate_ref_map, + &iterator, &opt)) { error(_("remote did not send all necessary objects")); free_refs(ref_cpy); ref_cpy = NULL; diff --git a/meson.build b/meson.build index 0a95d90d211fd6..d94dedc26cc0c6 100644 --- a/meson.build +++ b/meson.build @@ -562,6 +562,7 @@ libgit_sources = [ 'transport-helper.c', 'transport.c', 'tree-diff.c', + 'tree-verify.c', 'tree-walk.c', 'tree.c', 'unpack-trees.c', diff --git a/t/meson.build b/t/meson.build index f65eb04684b669..b23b4c90b0427b 100644 --- a/t/meson.build +++ b/t/meson.build @@ -654,6 +654,7 @@ integration_tests = [ 't5410-receive-pack.sh', 't5411-proc-receive-hook.sh', 't5412-receive-report-hook.sh', + 't5413-connectivity-check.sh', 't5500-fetch-pack.sh', 't5501-fetch-push-alternates.sh', 't5502-quickfetch.sh', diff --git a/t/perf/generate-repo-p5413-connectivity-check.perl b/t/perf/generate-repo-p5413-connectivity-check.perl new file mode 100644 index 00000000000000..bfa25f60fceac5 --- /dev/null +++ b/t/perf/generate-repo-p5413-connectivity-check.perl @@ -0,0 +1,44 @@ +#!/usr/bin/perl +# +# Generate a fast-import stream for p5413 connectivity check benchmarks. +# +# Usage: generate-repo-p5413-connectivity-check.perl +# [] [] +# +# Creates one initial commit with dirs*files_per_dir files, then +# additional commits each modifying +# files in directories chosen round-robin from 1... + +use strict; +use warnings; + +my ($nd, $nf, $nc, $hot, $fpc) = @ARGV; +$hot = $nd if !$hot || $hot > $nd; +$fpc = 1 if !$fpc; + +sub data { + printf "data %d\n%s\n", length($_[0]), $_[0]; +} + +# Initial tree: one commit with nd*nf files. +printf "commit refs/heads/main\n"; +printf "committer perf now\n"; +data("initial"); +for my $d (1..$nd) { + for my $f (1..$nf) { + printf "M 100644 inline d-%04d/f-%03d\n", $d, $f; + data(sprintf "%03d%03d", $d, $f); + } +} + +# Subsequent commits (auto-chained by fast-import). +for my $i (1..$nc) { + printf "commit refs/heads/main\n"; + printf "committer perf now\n"; + data(sprintf "change-%03d", $i); + for my $j (0..$fpc-1) { + my $d = (($i + $j) % $hot) + 1; + printf "M 100644 inline d-%04d/f-001\n", $d; + data(sprintf "c%d-%d", $i, $j); + } +} diff --git a/t/perf/p5413-connectivity-check.sh b/t/perf/p5413-connectivity-check.sh new file mode 100755 index 00000000000000..250140a79abf63 --- /dev/null +++ b/t/perf/p5413-connectivity-check.sh @@ -0,0 +1,92 @@ +#!/bin/sh + +test_description='performance of connectivity check modes + +Compare the default and incremental rev-list connectivity modes +directly, avoiding pack transfer noise. + +Each repository has a flat tree of many directories with 100 files +in each. Three axes are scaled independently: tree size, commit +count, and files changed per commit.' + +. ./perf-lib.sh + +test_perf_fresh_repo + +generate="$TEST_DIRECTORY/perf/generate-repo-p5413-connectivity-check.perl" + +# $1=dirs $2=files_per_dir $3=commits $4=hot_dirs (optional, default=all) +# $5=files_per_commit (optional, default=1) +test_perf_conn () { + local nd="$1" nf="$2" nc="$3" hot="${4:-$1}" fpc="${5:-1}" + local total=$(($nd * $nf)) + local name="repo-${nd}d-${nf}f-${nc}c-${hot}h-${fpc}fpc" + local label="${total} files, ${nc} commits" + if test "$hot" -lt "$nd" + then + label="$label (${hot} hot dirs)" + fi + if test "$fpc" -gt 1 + then + label="$label (${fpc} files/commit)" + fi + + test_expect_success PERL "setup $label" ' + if test -d '"$name"' + then + true + else + git init '"$name"' && + "$PERL_PATH" '"$generate"' '"$nd"' '"$nf"' '"$nc"' '"$hot"' '"$fpc"' | + git -C '"$name"' fast-import --date-format=now --quiet && + ( + cd '"$name"' && + git rev-parse main~'"$nc"' >../'"${name}"'_old && + git rev-parse main >../'"${name}"'_new && + git update-ref refs/heads/main \ + $(cat ../'"${name}"'_old) && + git repack -ad && + git config gc.auto 0 + ) + fi + ' + + test_perf "$label (full)" --prereq PERL ' + cat '"${name}"'_new | + git -C '"$name"' rev-list --connectivity-check=full \ + --objects --stdin --not --all --quiet \ + --exclude-promisor-objects + ' + + test_perf "$label (incremental)" --prereq PERL ' + cat '"${name}"'_new | + git -C '"$name"' rev-list --connectivity-check=incremental \ + --objects --stdin --not --all --quiet \ + --exclude-promisor-objects + ' +} + +# Scaling tree size (10 commits, 10 files/commit). +test_perf_conn 50 100 10 50 10 +test_perf_conn 500 100 10 500 10 +test_perf_conn 2000 100 10 2000 10 +test_perf_conn 8000 100 10 8000 10 + +# Scaling commit count (200K files, 10 files/commit). +test_perf_conn 2000 100 1 2000 10 +test_perf_conn 2000 100 10 2000 10 +test_perf_conn 2000 100 100 2000 10 +test_perf_conn 2000 100 500 2000 10 +test_perf_conn 2000 100 3000 2000 10 +test_perf_conn 2000 100 5000 2000 10 +test_perf_conn 2000 100 10000 2000 10 + +# Scaling files per commit (200K files, 10 commits). +test_perf_conn 2000 100 10 2000 1 +test_perf_conn 2000 100 10 2000 10 +test_perf_conn 2000 100 10 2000 100 +test_perf_conn 2000 100 10 2000 500 +test_perf_conn 2000 100 10 2000 1000 +test_perf_conn 2000 100 10 2000 2000 + +test_done diff --git a/t/t5413-connectivity-check.sh b/t/t5413-connectivity-check.sh new file mode 100755 index 00000000000000..60edf99b63b363 --- /dev/null +++ b/t/t5413-connectivity-check.sh @@ -0,0 +1,795 @@ +#!/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 () { + git -C "$1" config transfer.connectivityCheck "$2" +} + +# Run a connectivity check via rev-list directly. Uses $mode +# (set by the enclosing for-loop) to choose full or incremental. +check_connected () { + printf '%s\n' "$@" | + git rev-list --connectivity-check="$mode" \ + --objects --stdin --not --all --quiet \ + --exclude-promisor-objects +} + +# Run git with a temporary index, leaving the real index untouched. +tmpgit () { + GIT_INDEX_FILE=.git/tmp-idx git "$@" +} + +# 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) && + tmpgit read-tree "$1" && + tmpgit update-index --replace \ + --cacheinfo "100644,$new_blob,$2" && + new_tree=$(tmpgit write-tree) && + rm -f .git/tmp-idx && + git commit-tree "$new_tree" -p "$1" -m "modify $2" +} + +# Check OIDs and optionally verify 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 && + test_env GIT_TRACE2_EVENT="$(pwd)/$trace_file" \ + check_connected "$@" && + if test "$mode" != incremental + then + return + fi && + if test -n "$trees" + then + test_trace2_data_singular connectivity trees_loaded "$trees" \ + <"$trace_file" + fi && + if test -n "$blobs" + then + test_trace2_data_singular connectivity 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. + +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 + d="unchanged/dir-$i" && + mkdir -p "$d" && + for j in $(test_seq 1 10) + do + echo "$i $j" >"$d/file-$j.txt" || return 1 + done + done && + git add unchanged/ && + git commit -m "add unchanged canary subtree" + ) +' + +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 && + echo "$orig_blob" >.git/test-blob + ) +' + +missing_oid=$(test_oid missing) +original_oid=$(cat replace-test/.git/test-oid) +orig_blob_oid=$(cat replace-test/.git/test-blob) + +for mode in full incremental +do + +# The full traversal reports a missing tree as "bad tree object". +case "$mode" in +full) + missing_tree="bad tree object" + ;; +incremental) + missing_tree="missing tree object" + ;; +esac + +# Corruption detection: craft broken object graphs and verify detection. +# All tests use main-repo without modifying its refs or worktree. + +test_expect_success "$mode: rejects commit with missing blob" ' + ( + cd main-repo && + bad_tree=$(printf "100644 blob ${missing_oid}\tfile.txt\n" | + git mktree --missing) && + bad_commit=$(git commit-tree "$bad_tree" -p HEAD -m "bad") && + test_expect_code 128 check_connected "$bad_commit" 2>err && + test_grep "missing blob object.*$missing_oid" err + ) +' + +test_expect_success "$mode: rejects commit with missing subtree" ' + ( + cd main-repo && + bad_tree=$(printf "40000 tree ${missing_oid}\tdir\n" | + git mktree --missing) && + bad_commit=$(git commit-tree "$bad_tree" -p HEAD -m "bad") && + test_expect_code 128 check_connected "$bad_commit" 2>err && + test_grep "$missing_tree $missing_oid" err + ) +' + +test_expect_success "$mode: verifies direct tree tip" ' + ( + cd main-repo && + bad_tree=$(printf "100644 blob ${missing_oid}\tfile.txt\n" | + git mktree --missing) && + test_expect_code 128 check_connected "$bad_tree" 2>err && + test_grep "missing blob object.*$missing_oid" err + ) +' + +test_expect_success "$mode: verifies direct blob tip" ' + ( + cd main-repo && + blob_oid=$(echo "hello" | git hash-object -w --stdin) && + check_connected "$blob_oid" + ) +' + +test_expect_success "$mode: rejects missing direct blob tip" ' + ( + cd main-repo && + test_expect_code 128 check_connected \ + "$missing_oid" 2>err + ) +' + +test_expect_success PERL_TEST_HELPERS \ + "$mode: rejects blob OID reused as tree entry" ' + ( + cd main-repo && + + 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_expect_code 128 check_connected "$bad_commit" 2>err && + test_grep "not a tree" err + ) +' + +test_expect_success PERL_TEST_HELPERS \ + "$mode: rejects tree OID reused as blob entry" ' + ( + cd main-repo && + + 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_expect_code 128 check_connected "$bad_commit" 2>err && + test_grep "not a blob" err + ) +' + +test_expect_success "$mode: checks multiple tips" ' + ( + cd main-repo && + 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 && + check_connected "$c2" "$tag_oid" + ) +' + +# Tree-diff optimization: verify trace2 counts. + +test_expect_success "$mode: handles root commit (no parents)" ' + ( + cd main-repo && + blob_oid=$(echo "root-content" | git hash-object -w --stdin) && + tree_oid=$(printf "100644 blob %s\tfile.txt\n" \ + "$blob_oid" | git mktree) && + root_oid=$(git commit-tree "$tree_oid" -m "root") && + + # No parent trees, so the full tree is verified. + # 1 tree loaded (root), 1 blob checked. + check_connected_trace trace-root.txt 1 1 "$root_oid" + ) +' + +test_expect_success "$mode: handles single file change" ' + ( + cd main-repo && + oid=$(commit_with_change HEAD file-1.txt "changed") && + + # 2 trees loaded (new root + parent root), 1 blob checked. + check_connected_trace trace-flat.txt 2 1 "$oid" + ) +' + +test_expect_success "$mode: handles nested change" ' + ( + cd main-repo && + oid=$(commit_with_change HEAD a/b/c/deep.txt "deep-changed") && + # 4 new trees + 4 parent trees = 8 loaded, 1 blob checked. + check_connected_trace trace-nested.txt 8 1 "$oid" + ) +' + +test_expect_success "$mode: handles change-then-revert" ' + ( + cd main-repo && + 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") && + + # c1: new root + parent root = 2 loads. c2: root matches + # HEAD (already trusted), skipped. c3: new root + parent + # already expanded = 1 load. Total: 3 trees, 2 blobs. + check_connected_trace trace-revert.txt 3 2 "$c3" + ) +' + +test_expect_success "$mode: handles subtree moved to another path" ' + ( + cd main-repo && + moved_tree=$(git ls-tree HEAD | + sed "s/ a$/ moved/" | + git mktree) && + moved=$(git commit-tree "$moved_tree" -p HEAD -m move) && + # New root + parent root scanned, but the moved subtree + # (same OID) is trusted and not descended into. + check_connected_trace trace-move.txt 2 0 "$moved" + ) +' + +test_expect_success "$mode: handles multi-parent merge" ' + ( + cd main-repo && + left=$(commit_with_change HEAD file-1.txt left) && + right=$(commit_with_change HEAD file-2.txt right) && + git update-ref refs/heads/left "$left" && + git update-ref refs/heads/right "$right" && + left_blob=$(git rev-parse "$left:file-1.txt") && + right_blob=$(git rev-parse "$right:file-2.txt") && + tmpgit read-tree HEAD && + tmpgit update-index --replace \ + --cacheinfo "100644,$left_blob,file-1.txt" && + tmpgit update-index --replace \ + --cacheinfo "100644,$right_blob,file-2.txt" && + merge_tree=$(tmpgit write-tree) && + rm -f .git/tmp-idx && + merge=$(git commit-tree "$merge_tree" \ + -p "$left" -p "$right" -m merge) && + # Both parent root trees are scanned as bases, so + # blobs from each parent are trusted without ODB checks. + check_connected_trace trace-merge.txt 3 0 "$merge" && + git update-ref -d refs/heads/left && + git update-ref -d refs/heads/right + ) +' + +test_expect_success "$mode: handles merge with incoming and boundary parents" ' + ( + cd main-repo && + # parent1 is incoming (not a ref), HEAD is boundary. + parent1=$(commit_with_change HEAD file-1.txt merge-inc) && + merge=$(git commit-tree \ + "$(git rev-parse "$parent1^{tree}")" \ + -p "$parent1" -p HEAD -m merge-mixed) && + # parent1 verified first (topo order): 2 trees, 1 blob. + # merge tree = parent1 tree (already trusted): 0 extra. + check_connected_trace trace-merge-inc.txt 2 1 "$merge" + ) +' + +test_expect_success "$mode: handles gitlink entries (submodules)" ' + ( + cd main-repo && + tmpgit read-tree HEAD && + tmpgit update-index --add \ + --cacheinfo "160000,$missing_oid,my-submodule" && + gitlink_tree=$(tmpgit write-tree) && + rm -f .git/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 2 0 "$gitlink_commit" + ) +' + +test_expect_success "$mode: handles file-to-directory transition" ' + ( + cd main-repo && + + # Parent: "foo" is a blob at root. + blob_a=$(echo "file-content" | git hash-object -w --stdin) && + tmpgit read-tree HEAD && + tmpgit update-index --add \ + --cacheinfo "100644,$blob_a,foo" && + parent_tree=$(tmpgit write-tree) && + rm -f .git/tmp-idx && + parent=$(git commit-tree "$parent_tree" -p HEAD \ + -m "add foo as file") && + + # Child: "foo" becomes a directory (foo/bar.txt). + blob_b=$(echo "dir-content" | git hash-object -w --stdin) && + tmpgit read-tree "$parent" && + tmpgit update-index --remove foo && + tmpgit update-index --add \ + --cacheinfo "100644,$blob_b,foo/bar.txt" && + child_tree=$(tmpgit write-tree) && + rm -f .git/tmp-idx && + child=$(git commit-tree "$child_tree" -p "$parent" \ + -m "foo: file to directory") && + check_connected_trace trace-f2d.txt "" "" "$child" + ) +' + +test_expect_success "$mode: handles directory-to-file transition" ' + ( + cd main-repo && + + # Parent: "bar/baz.txt" exists (bar is a directory). + blob_a=$(echo "nested" | git hash-object -w --stdin) && + tmpgit read-tree HEAD && + tmpgit update-index --add \ + --cacheinfo "100644,$blob_a,bar/baz.txt" && + parent_tree=$(tmpgit write-tree) && + rm -f .git/tmp-idx && + parent=$(git commit-tree "$parent_tree" -p HEAD \ + -m "add bar as directory") && + + # Child: "bar" becomes a plain file. + blob_b=$(echo "flat" | git hash-object -w --stdin) && + tmpgit read-tree "$parent" && + tmpgit update-index --remove bar/baz.txt && + tmpgit update-index --add \ + --cacheinfo "100644,$blob_b,bar" && + child_tree=$(tmpgit write-tree) && + rm -f .git/tmp-idx && + child=$(git commit-tree "$child_tree" -p "$parent" \ + -m "bar: directory to file") && + check_connected_trace trace-d2f.txt "" "" "$child" + ) +' + +# Replacement objects. + +test_expect_success "$mode: accepts with replacement objects" ' + ( + cd replace-test && + check_connected "$original_oid" + ) +' + +test_expect_success "$mode: rejects without replacement objects" ' + ( + cd replace-test && + GIT_NO_REPLACE_OBJECTS=1 && + export GIT_NO_REPLACE_OBJECTS && + test_expect_code 128 check_connected \ + "$original_oid" 2>err && + test_grep "missing blob object.*$orig_blob_oid" err + ) +' + +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 && + ( + 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") && + check_connected "$new_commit" && + test_must_fail env GIT_NO_LAZY_FETCH=1 \ + git cat-file -e "$promised_blob" + ) +' + +test_expect_success "$mode: does not lazy-fetch promised blob" ' + test_when_finished "rm -rf nofetch-src nofetch-server.git nofetch-client" && + git init nofetch-src && + test_commit -C nofetch-src --no-tag base file.txt content && + git clone --bare nofetch-src nofetch-server.git && + git -C nofetch-server.git config uploadpack.allowfilter true && + git -C nofetch-server.git config uploadpack.allowanysha1inwant true && + git clone --no-checkout --filter=blob:none \ + "file://$(pwd)/nofetch-server.git" nofetch-client && + ( + cd nofetch-client && + promised_blob=$(git rev-parse HEAD:file.txt) && + test_must_fail env GIT_NO_LAZY_FETCH=1 \ + git cat-file -e "$promised_blob" && + new_tree=$(printf "100644 blob %s\tfile.txt\n" \ + "$promised_blob" | + git mktree --missing) && + new_commit=$(git commit-tree "$new_tree" \ + -m "root commit with promised blob") && + check_connected "$new_commit" && + 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 && + ( + cd prom-tree-client && + promised_tree=$(git ls-tree HEAD -- a | + awk "{print \$3}") && + test_must_fail env GIT_NO_LAZY_FETCH=1 \ + git cat-file -e "$promised_tree" && + 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") && + check_connected "$new_commit" && + test_must_fail env GIT_NO_LAZY_FETCH=1 \ + git cat-file -e "$promised_tree" + ) +' + +test_expect_success "$mode: verifies new subtree when parent subtree is promised" ' + test_when_finished "rm -rf prom-base-src prom-base-server.git prom-base-client" && + git init prom-base-src && + mkdir -p prom-base-src/a && + test_commit -C prom-base-src --no-tag "base" a/file.txt deep && + git clone --bare prom-base-src prom-base-server.git && + git -C prom-base-server.git config uploadpack.allowfilter true && + git -C prom-base-server.git config uploadpack.allowanysha1inwant true && + git clone --no-checkout --filter=tree:1 \ + "file://$(pwd)/prom-base-server.git" prom-base-client && + ( + cd prom-base-client && + parent_subtree=$(git ls-tree HEAD -- a | + awk "{print \$3}") && + test_must_fail env GIT_NO_LAZY_FETCH=1 \ + git cat-file -e "$parent_subtree" && + new_blob=$(echo "local-content" | git hash-object -w --stdin) && + new_subtree=$(printf "100644 blob %s\tfile.txt\n" \ + "$new_blob" | git mktree) && + new_root=$(printf "40000 tree %s\ta\n" \ + "$new_subtree" | git mktree) && + new_commit=$(git commit-tree "$new_root" \ + -p HEAD -m "replace promised subtree") && + check_connected "$new_commit" && + test_must_fail env GIT_NO_LAZY_FETCH=1 \ + git cat-file -e "$parent_subtree" + ) +' + +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 && + ( + cd pc-client && + local_commit=$(commit_with_change HEAD file.txt local-content) && + check_connected "$local_commit" + ) +' + +test_expect_success "$mode: tolerates missing promisor parent commit" ' + test_when_finished "rm -rf mp-src mp-server.git mp-client" && + git init mp-src && + test_commit -C mp-src --no-tag base file.txt && + git clone --bare mp-src mp-server.git && + git -C mp-server.git config uploadpack.allowfilter true && + git -C mp-server.git config uploadpack.allowanysha1inwant true && + git clone --filter=blob:none \ + "file://$(pwd)/mp-server.git" mp-client && + test_commit -C mp-src --no-tag c1 file.txt && + test_commit -C mp-src --no-tag c2 file.txt && + c1=$(git -C mp-src rev-parse HEAD~1) && + c2=$(git -C mp-src rev-parse HEAD) && + ( + cd mp-client && + # Add a promisor pack holding only c2, so its parent c1 + # is missing but promised (as after a lazy fetch that + # omits dependents). + echo "$c2" | + git -C ../mp-src pack-objects --stdout >c2.pack && + git index-pack --promisor --stdin 1112912053 -0700 + committer C O Mitter 1112912053 -0700 + + on top of a missing promisor commit + EOF + ) && + check_connected "$local_commit" + ) +' + +test_expect_success "$mode: respects shallow boundary" ' + test_when_finished "rm -rf shallow-src shallow" && + git init shallow-src && + test_commit -C shallow-src --no-tag base file content-1 && + mkdir shallow-src/sub && + test_commit -C shallow-src --no-tag change sub/other content-2 && + git clone --depth=1 "file://$(pwd)/shallow-src" shallow && + ( + cd shallow && + tip=$(git rev-parse HEAD) && + git for-each-ref --format="delete %(refname)" | + git update-ref --no-deref --stdin && + check_connected "$tip" + ) +' + +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_TRACE2_EVENT="$(pwd)/deepen-trace.txt" \ + git -C deepen-client fetch --deepen=2 origin main && + # Incremental falls back to full for deepening fetches, + # so the trees_loaded event should not appear. + test_grep ! trees_loaded deepen-trace.txt +' + +test_expect_success "$mode: malformed tree detected" ' + ( + cd main-repo && + echo abc >malformed-tree && + malformed_tree=$(git hash-object --literally -t tree -w \ + malformed-tree) && + malformed_commit=$(git commit-tree "$malformed_tree" \ + -p HEAD -m malformed) && + test_expect_code 128 check_connected \ + "$malformed_commit" 2>err + ) +' + +test_expect_success PERL_TEST_HELPERS \ + "$mode: mid-tree corruption detected" ' + ( + cd main-repo && + # Build a tree with one valid entry followed by garbage. + blob_oid=$(echo "valid" | git hash-object -w --stdin) && + bin_oid=$(echo "$blob_oid" | hex2oct) && + printf "100644 good\0${bin_oid}GARBAGE" >corrupt-mid-tree && + corrupt_tree=$(git hash-object --literally -t tree -w \ + corrupt-mid-tree) && + corrupt_commit=$(git commit-tree "$corrupt_tree" \ + -p HEAD -m "mid-tree corruption") && + test_expect_code 128 check_connected \ + "$corrupt_commit" 2>err && + test_grep "too-short tree object" err + ) +' + +done + +# The full check only checks that blobs exist, but the incremental +# check reads their headers and reports a corrupt blob. +test_expect_success 'incremental: rejects corrupt blob' ' + test_when_finished "rm -rf corrupt-repo" && + git init corrupt-repo && + ( + cd corrupt-repo && + test_commit base && + blob=$(echo corrupt | git hash-object -w --stdin) && + tree=$(printf "100644 blob $blob\tfile\n" | git mktree) && + commit=$(git commit-tree "$tree" -p HEAD -m corrupt) && + path=.git/objects/$(test_oid_to_path "$blob") && + chmod +w "$path" && + echo garbage >"$path" && + mode=incremental && + test_expect_code 128 check_connected "$commit" 2>err && + test_grep "unable to read blob object $blob" err + ) +' + +# Algorithm selection. + +# Run a command with its trace2 events in "trace" and its stderr +# in "err". +run_traced () { + rm -f trace && + GIT_TRACE2_EVENT="$(pwd)/trace" "$@" 2>err +} + +# Check which connectivity check the last run_traced used. +expect_mode () { + case "$1" in + full) + test_grep ! trees_loaded trace + ;; + incremental) + test_grep trees_loaded trace + ;; + *) + BUG "unknown mode: $1" + ;; + esac +} + +test_expect_success 'setup source repository' ' + git init src && + test_commit -C src base file.txt && + test_commit -C src --no-tag update file.txt updated +' + +test_expect_success 'unknown transfer.connectivityCheck falls back to full' ' + test_when_finished "rm -rf dst" && + git init --bare dst && + git -C dst fetch ../src base:refs/heads/main && + for value in bogus auto= auto=x + do + set_connectivity_check dst "$value" && + run_traced git -C dst fetch ../src main && + expect_mode full && + test_grep "unknown transfer.connectivityCheck algorithm .$value., using .full." err || + return 1 + done +' + +test_expect_success 'push uses incremental when configured' ' + test_when_finished "rm -rf dst" && + git init --bare dst && + git -C src push ../dst base:refs/heads/main && + set_connectivity_check dst incremental && + run_traced git -C src push ../dst main && + expect_mode incremental +' + +test_expect_success 'fetch uses incremental when configured' ' + test_when_finished "rm -rf dst" && + git init --bare dst && + git -C dst fetch ../src base:refs/heads/main && + set_connectivity_check dst incremental && + run_traced git -C dst fetch ../src main && + expect_mode incremental +' + +for mode in incremental auto +do + test_expect_success "push with $mode into an empty repository uses full" ' + test_when_finished "rm -rf dst" && + git init --bare dst && + set_connectivity_check dst $mode && + run_traced git -C src push ../dst main && + expect_mode full + ' + + test_expect_success "clone with $mode uses full" ' + test_when_finished "rm -rf dst" && + run_traced git -c transfer.connectivityCheck=$mode \ + clone --no-local src dst && + expect_mode full + ' +done + +for value in auto auto=1 +do + test_expect_success "push with $value uses incremental up to the cutoff" ' + test_when_finished "rm -rf dst" && + git init --bare dst && + git -C src push ../dst base:refs/heads/main && + set_connectivity_check dst $value && + run_traced git -C src push ../dst main && + expect_mode incremental + ' +done + +test_expect_success 'push with auto uses full above the cutoff' ' + test_when_finished "rm -rf dst" && + git init --bare dst && + git -C src push ../dst base:refs/heads/main && + set_connectivity_check dst auto=0 && + run_traced git -C src push ../dst main && + expect_mode full +' + +test_done diff --git a/t/t6700-tree-depth.sh b/t/t6700-tree-depth.sh index 0f6a2ad9b593f4..edb31e3914251f 100755 --- a/t/t6700-tree-depth.sh +++ b/t/t6700-tree-depth.sh @@ -82,6 +82,23 @@ test_expect_success 'default limit for rev-list fails gracefully' ' test_must_fail git rev-list --objects big >/dev/null ' +# The incremental check needs a boundary; without negative revisions +# rev-list falls back to the full check. +test_expect_success 'limit recursion of incremental check' ' + boundary=$(git commit-tree $EMPTY_TREE -m boundary) && + git $small_ok rev-list --connectivity-check=incremental \ + --objects small --not $boundary >/dev/null && + test_must_fail git $small_no rev-list \ + --connectivity-check=incremental --objects small \ + --not $boundary >/dev/null +' + +test_expect_success 'default limit for incremental check fails gracefully' ' + boundary=$(git commit-tree $EMPTY_TREE -m boundary) && + test_must_fail git rev-list --connectivity-check=incremental \ + --objects big --not $boundary >/dev/null +' + test_expect_success 'limit recursion of diff-tree -r' ' git $small_ok diff-tree -r $EMPTY_TREE small && test_must_fail git $small_no diff-tree -r $EMPTY_TREE small diff --git a/tree-verify.c b/tree-verify.c new file mode 100644 index 00000000000000..6126e99853f4a6 --- /dev/null +++ b/tree-verify.c @@ -0,0 +1,427 @@ +#include "git-compat-util.h" +#include "commit.h" +#include "gettext.h" +#include "hex.h" +#include "khash.h" +#include "object.h" +#include "odb.h" +#include "oid-array.h" +#include "oidset.h" +#include "tree.h" +#include "tree-walk.h" +#include "tree-verify.h" +#include "packfile.h" +#include "trace2.h" + +enum tree_state { + TREE_UNTRUSTED = 0, + TREE_TRUSTED = 1, + TREE_EXPANDED = 2, +}; + +/* Values are enum tree_state, stored as a byte to keep the map small. */ +KHASH_INIT(oid_tree, struct object_id, unsigned char, 1, + oidhash_by_value, oideq_by_value) + +static enum tree_state tree_map_get(kh_oid_tree_t *m, + const struct object_id *oid) +{ + khint_t pos = kh_get_oid_tree(m, *oid); + if (pos == kh_end(m)) + return TREE_UNTRUSTED; + return kh_val(m, pos); +} + +static void tree_map_add(kh_oid_tree_t *m, const struct object_id *oid, + enum tree_state state) +{ + int added; + khint_t pos = kh_put_oid_tree(m, *oid, &added); + if (added) + kh_val(m, pos) = state; + else if (state > kh_val(m, pos)) + kh_val(m, pos) = state; +} + +struct work_item { + struct name_entry entry; + struct oid_array parent_trees; +}; + +/* The entries of a tree that still need to be verified. */ +struct work_list { + struct work_item *items; + size_t nr, alloc; + bool has_subtrees; +}; + +static void work_list_clear(struct work_list *list) +{ + for (size_t i = 0; i < list->nr; i++) + oid_array_clear(&list->items[i].parent_trees); + free(list->items); +} + +struct verify_state { + kh_oid_tree_t *trees; + struct oidset trusted_blobs; + size_t trees_loaded; + size_t blobs_checked; + int exclude_promisor_objects; + + /* + * Scratch space for reading one tree at a time. It lives here + * rather than on the stack to keep the frame of the recursive + * verify_tree() small. + */ + struct tree_desc desc; + struct name_entry entry; +}; + +/* + * Merge-walk the work list against one base tree entry, recording + * same-path parent subtrees as recursive comparison bases. + * Returns the updated work-list cursor. + */ +static size_t collect_subtree_bases(struct work_list *list, size_t wi, + const struct name_entry *entry) +{ + while (wi < list->nr) { + struct work_item *item = &list->items[wi]; + int cmp = base_name_compare(item->entry.path, + item->entry.pathlen, + item->entry.mode, + entry->path, entry->pathlen, + entry->mode); + if (cmp > 0) + break; + if (cmp < 0) { + wi++; + continue; + } + if (S_ISDIR(item->entry.mode) && S_ISDIR(entry->mode)) + oid_array_append(&item->parent_trees, &entry->oid); + return wi + 1; + } + return wi; +} + +/* + * Report an object that could not be read, telling a missing object + * apart from one that exists but cannot be read, e.g. because it is + * corrupt. + */ +static int unreadable_object_error(struct repository *repo, + const struct object_id *oid, + const char *type) +{ + if (!odb_has_object(repo->objects, oid, 0)) + return error(_("missing %s object %s"), type, oid_to_hex(oid)); + return error(_("unable to read %s object %s"), type, oid_to_hex(oid)); +} + +static int verify_blob(struct repository *repo, + const struct object_id *oid, + struct verify_state *vs) +{ + int type; + + if (oidset_contains(&vs->trusted_blobs, oid)) + return 0; + + vs->blobs_checked++; + type = odb_read_object_info(repo->objects, oid, NULL); + if (type == OBJ_BLOB) { + oidset_insert(&vs->trusted_blobs, oid); + return 0; + } + if (type >= 0) + return error(_("object %s is a %s, not a %s"), + oid_to_hex(oid), type_name(type), "blob"); + if (vs->exclude_promisor_objects && + is_promisor_object(repo, oid)) + return 0; + return unreadable_object_error(repo, oid, "blob"); +} + +/* + * Read a tree object into *bufp. Returns -1 if the object exists + * but is not a tree. Otherwise returns 0, leaving *bufp NULL if the + * object could not be read. + */ +static int read_tree_object(struct object_database *odb, + const struct object_id *oid, + void **bufp, size_t *sizep) +{ + enum object_type type; + + *bufp = odb_read_object(odb, oid, &type, sizep); + if (*bufp && type != OBJ_TREE) { + FREE_AND_NULL(*bufp); + return error(_("object %s is a %s, not a %s"), + oid_to_hex(oid), type_name(type), "tree"); + } + return 0; +} + +/* List the entries of a tree that are not yet known to be trusted. */ +static void list_untrusted_entries(struct verify_state *vs, + const struct object_id *tree_oid, + void *tree_buf, size_t tree_size, + struct work_list *list) +{ + struct name_entry *entry = &vs->entry; + + init_tree_desc(&vs->desc, tree_oid, tree_buf, tree_size); + while (tree_entry(&vs->desc, entry)) { + if (S_ISGITLINK(entry->mode)) + continue; + if (S_ISDIR(entry->mode)) { + if (tree_map_get(vs->trees, &entry->oid) >= + TREE_TRUSTED) + continue; + list->has_subtrees = true; + } else { + if (oidset_contains(&vs->trusted_blobs, &entry->oid)) + continue; + } + ALLOC_GROW(list->items, list->nr + 1, list->alloc); + memset(&list->items[list->nr], 0, sizeof(list->items[0])); + list->items[list->nr++].entry = *entry; + } +} + +/* + * Scan one trusted base tree: expand it if that has not happened yet, + * and collect its same-path subtrees as bases for untrusted subtrees. + */ +static int scan_base_tree(struct repository *repo, struct verify_state *vs, + const struct object_id *base_oid, + struct work_list *list) +{ + int expanded = tree_map_get(vs->trees, base_oid) >= TREE_EXPANDED; + struct name_entry *entry = &vs->entry; + size_t wi = 0, base_size; + void *base_buf; + + if (expanded && !list->has_subtrees) + return 0; + + if (read_tree_object(repo->objects, base_oid, &base_buf, &base_size)) + return -1; + if (!base_buf) { + if (vs->exclude_promisor_objects && + is_promisor_object(repo, base_oid)) + return 0; + return unreadable_object_error(repo, base_oid, "tree"); + } + + vs->trees_loaded++; + init_tree_desc(&vs->desc, base_oid, base_buf, base_size); + + while (tree_entry(&vs->desc, entry)) { + if (S_ISGITLINK(entry->mode)) + continue; + + /* Same-path subtree as a base for an untrusted one. */ + if (list->has_subtrees) + wi = collect_subtree_bases(list, wi, entry); + + /* Expand: the base is trusted, so are its entries. */ + if (!expanded) { + if (S_ISDIR(entry->mode)) + tree_map_add(vs->trees, &entry->oid, + TREE_TRUSTED); + else + oidset_insert(&vs->trusted_blobs, + &entry->oid); + } + } + + if (!expanded) + tree_map_add(vs->trees, base_oid, TREE_EXPANDED); + + free(base_buf); + return 0; +} + +/* + * Verify a tree against the trusted base trees, loading as little as + * possible: + * + * First list the entries that are not yet known to be trusted. When + * the parent trees have been expanded before, only the changed entries + * remain. + * + * Then scan the trusted base trees (the same-path trees of the parent + * commits). A base that has not been expanded yet is read so that its + * entries can be marked trusted; this often shows that listed entries + * are trusted after all. An already expanded base is still read when + * there are untrusted subtrees, because its subtree at the same path is + * what lets the untrusted subtree skip its own unchanged entries in + * turn. + * + * Finally check that the remaining blobs exist, and recurse into the + * remaining subtrees with the same-path subtrees as their bases. + */ +static int verify_tree(struct repository *repo, + const struct object_id *tree_oid, + const struct oid_array *trusted_bases, + struct verify_state *vs, int depth) +{ + struct work_list list = { 0 }; + size_t i, tree_size; + void *tree_buf; + int ret = 0; + + /* + * Enforce core.maxTreeDepth like process_tree() in list-objects.c, + * so that the full and incremental checks reject the same trees. + */ + if (depth > repo->settings.max_allowed_tree_depth) + return error(_("exceeded maximum allowed tree depth")); + + if (tree_map_get(vs->trees, tree_oid) >= TREE_TRUSTED) + return 0; + + if (read_tree_object(repo->objects, tree_oid, &tree_buf, &tree_size)) + return -1; + if (!tree_buf) { + /* + * A missing tree that is promised can be fetched on demand, + * so it is not an error, as in the full check. + */ + if (vs->exclude_promisor_objects && + is_promisor_object(repo, tree_oid)) + return 0; + return unreadable_object_error(repo, tree_oid, "tree"); + } + + vs->trees_loaded++; + list_untrusted_entries(vs, tree_oid, tree_buf, tree_size, &list); + + /* Scan the base trees, unless nothing is left to verify. */ + for (i = 0; list.nr && i < trusted_bases->nr; i++) { + ret = scan_base_tree(repo, vs, &trusted_bases->oid[i], &list); + if (ret) + goto done; + } + + /* + * Check the remaining entries in tree order. Verifying a subtree + * can show that later entries are trusted, which then makes them + * cheap to skip. + */ + for (i = 0; i < list.nr && !ret; i++) { + struct work_item *item = &list.items[i]; + + if (S_ISDIR(item->entry.mode)) + ret = verify_tree(repo, &item->entry.oid, + &item->parent_trees, vs, depth + 1); + else + ret = verify_blob(repo, &item->entry.oid, vs); + } + if (!ret) + tree_map_add(vs->trees, tree_oid, TREE_EXPANDED); + +done: + free(tree_buf); + work_list_clear(&list); + return ret; +} + +static int verify_commit_tree(struct repository *repo, + struct commit *commit, + struct verify_state *vs) +{ + struct oid_array parent_trees = OID_ARRAY_INIT; + const struct object_id *tree_oid; + struct commit_list *p; + int ret; + + /* + * Parent trees are trusted: boundary parents are already + * connected, and earlier incoming parents were verified + * first due to the topological processing order. + * + * The revision walk skips missing parents that are promised + * (see process_parents()), so do the same here. Such a parent + * contributes no base, which only means less of the new tree + * can be skipped. + */ + for (p = commit->parents; p; p = p->next) { + if (repo_parse_commit_gently(repo, p->item, 1) < 0) { + if (vs->exclude_promisor_objects && + is_promisor_object(repo, &p->item->object.oid)) + continue; + ret = error(_("unable to parse commit %s"), + oid_to_hex(&p->item->object.oid)); + goto out; + } + tree_oid = get_commit_tree_oid(p->item); + if (!tree_oid) { + ret = error(_("unable to load root tree for commit %s"), + oid_to_hex(&p->item->object.oid)); + goto out; + } + tree_map_add(vs->trees, tree_oid, TREE_TRUSTED); + oid_array_append(&parent_trees, tree_oid); + } + + tree_oid = get_commit_tree_oid(commit); + if (!tree_oid) { + ret = error(_("unable to load root tree for commit %s"), + oid_to_hex(&commit->object.oid)); + goto out; + } + ret = verify_tree(repo, tree_oid, &parent_trees, vs, 0); +out: + oid_array_clear(&parent_trees); + return ret; +} + +int verify_commits_incremental(struct repository *repo, + struct commit_list **commits, + int exclude_promisor_objects) +{ + struct verify_state vs = { 0 }; + struct commit_list *iter; + unsigned nr_before; + int ret = 0; + + if (repo->fetch_if_missing) + BUG("verify_commits_incremental must not be called " + "with fetch_if_missing set"); + + vs.trees = kh_init_oid_tree(); + vs.exclude_promisor_objects = exclude_promisor_objects; + + /* + * Ancestors must be verified before descendants so that parent + * trees can be trusted without re-verification. Sort explicitly + * rather than relying on the caller's ordering. + * + * sort_in_topological_order() silently drops cycle members, + * so explicitly check if the size has changed. + */ + nr_before = commit_list_count(*commits); + sort_in_topological_order(commits, REV_SORT_IN_GRAPH_ORDER); + if (commit_list_count(*commits) < nr_before) { + ret = error(_("cycle detected in incoming commit graph")); + goto out; + } + + *commits = commit_list_reverse(*commits); + + for (iter = *commits; iter && !ret; iter = iter->next) + ret = verify_commit_tree(repo, iter->item, &vs); + +out: + kh_destroy_oid_tree(vs.trees); + oidset_clear(&vs.trusted_blobs); + trace2_data_intmax("connectivity", repo, + "trees_loaded", (intmax_t)vs.trees_loaded); + trace2_data_intmax("connectivity", repo, + "blobs_checked", (intmax_t)vs.blobs_checked); + return ret; +} diff --git a/tree-verify.h b/tree-verify.h new file mode 100644 index 00000000000000..ac7092da1cb0c3 --- /dev/null +++ b/tree-verify.h @@ -0,0 +1,15 @@ +#ifndef TREE_VERIFY_H +#define TREE_VERIFY_H + +struct commit_list; +struct repository; + +/* + * Verify trees of commits incrementally against their parents. + * Returns 0 on success, or reports an error and returns -1. + */ +int verify_commits_incremental(struct repository *repo, + struct commit_list **commits, + int exclude_promisor_objects); + +#endif /* TREE_VERIFY_H */