Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions Documentation/config/transfer.adoc
Original file line number Diff line number Diff line change
@@ -1,3 +1,23 @@
transfer.connectivityCheck::

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Junio C Hamano wrote on the Git mailing list (how to reply to this email):

"Kristofer Karlsson via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> +static void verify_blob(struct repository *repo,
> +			const struct object_id *oid,
> +			struct verify_state *vs)
> +{
> +	int type;
> +
> +	if (oidset_contains(&vs->trusted_blobs, oid))
> +		return;
> +
> +	vs->blobs_checked++;
> +	type = odb_read_object_info(repo->objects, oid, NULL);
> +	if (type == OBJ_BLOB) {
> +		oidset_insert(&vs->trusted_blobs, oid);
> +		return;
> +	}
> +	if (type >= 0)
> +		die(_("object %s is a %s, not a blob"),
> +		    oid_to_hex(oid), type_name(type));
> +	if (vs->exclude_promisor_objects &&
> +	    is_promisor_object(repo, oid))
> +		return;
> +	die(_("missing blob object '%s'"), oid_to_hex(oid));
> +}

I wonder if this is_promisor_object() call comes a bit too late, as
we earlier already have called odb_read_object_info() which may have
fetched it lazily from the promisor remote?  Or do we globally
disable promisor_remote_get_direct() call somehow without having to
pass OBJECT_INFO_SKIP_FETCH_OBJECT flag?

> +static void verify_commit_tree(struct repository *repo,
> +			       struct commit *commit,
> +			       struct verify_state *vs)
> +{
> +	struct oid_array base_trees = OID_ARRAY_INIT;
> +	struct commit_list *p;
> +
> +	/*
> +	 * Parent trees are trusted: boundary parents are already
> +	 * connected, and earlier incoming parents were verified
> +	 * first due to the topological processing order.
> +	 */
> +	for (p = commit->parents; p; p = p->next) {
> +		const struct object_id *tree_oid;
> +		parse_commit_or_die(p->item);
> +		tree_oid = get_commit_tree_oid(p->item);
> +		tree_map_add(vs->trees, tree_oid, TREE_TRUSTED);
> +		oid_array_append(&base_trees, tree_oid);
> +	}
> +
> +	verify_tree(repo, get_commit_tree_oid(commit),
> +		    &base_trees, vs, 0);
> +	oid_array_clear(&base_trees);
> +}

Do we assume that we do not have to deal with repository corruption
in any graceful way?  I am just wondering what happens when
get_commit_tree_oid() yields NULL after parse_commit_or_die() finds
p->item is a valid-looking commit object but the tree within it is
not, and we end up passing NULL to tree_map_add(), perhaps?

The same potential issue may exist in the get_commit_tree_oid() call
outside the look at the end on the incoming commit's tree.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kristofer Karlsson wrote on the Git mailing list (how to reply to this email):

On Mon, 14 Sept 2026 at 17:26, Junio C Hamano <gitster@pobox.com> wrote:
>
> "Kristofer Karlsson via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> I wonder if this is_promisor_object() call comes a bit too late, as
> we earlier already have called odb_read_object_info() which may have
> fetched it lazily from the promisor remote?  Or do we globally
> disable promisor_remote_get_direct() call somehow without having to
> pass OBJECT_INFO_SKIP_FETCH_OBJECT flag?

Yes, I think it's safe due to the following mechanism:

1. If promisors exist, the connectivity-check will invoke
   rev-list with --exclude-promisor-objects.
2. rev-list in turn sets repo->fetch_if_missing = 0 on startup.
3. Then the odb read goes down into do_oid_object_info_extended()
   which respects that flag.

However, my paranoia kicked in so I re-ran my test for this,
after adding some temporary code inside
verify_commits_incremental():

    repo->fetch_if_missing = 1;

And fortunately, one of the tests failed as expected.

    Exactly 1 failure out of 62 tests: test 53
      "incremental: verifies new subtree when parent subtree is
       promised".

And the relevant assertion is this one:

    test_must_fail env GIT_NO_LAZY_FETCH=1 \
        git cat-file -e "$parent_subtree"

which ensures that the object was never fetched.

However, the test only catches this scenario for trees,
not blobs -- that's an oversight, I will add a matching
test for blobs too.

I think the code technically works as-is, but I could also try
to rewrite the code to stop depending on odb_read_object_info()
and instead use odb_read_object_info_extended() which allows
me to pass the flags.  That gives us belts and suspenders, which
may be nicer here.

> Do we assume that we do not have to deal with repository corruption
> in any graceful way?  I am just wondering what happens when
> get_commit_tree_oid() yields NULL after parse_commit_or_die() finds
> p->item is a valid-looking commit object but the tree within it is
> not, and we end up passing NULL to tree_map_add(), perhaps?
>
> The same potential issue may exist in the get_commit_tree_oid() call
> outside the look at the end on the incoming commit's tree.

You're right, this is an oversight.
I think I incorrectly assumed that parse_commit_or_die()
would catch any malformed commit.

I will add a NULL check and a die()-exit at the two call sites
in verify_commit_tree()

    die(_("unable to load root tree for commit %s"),
        oid_to_hex(&commit->object.oid));

Thanks for spotting these errors,
Kristofer

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.
Falls back to `full` for deepening fetches.
--

transfer.credentialsInUrl::
A configured URL can contain plaintext credentials in the form
`<protocol>://<user>:<password>@<domain>/<path>`. You may want
Expand Down
6 changes: 6 additions & 0 deletions Documentation/rev-list-options.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -1089,6 +1089,12 @@ we cannot get their Object ID though, an error will be raised.
stronger than `--missing=allow-promisor` because it limits the
traversal, rather than just silencing errors about missing
objects.

`--verify-trees-incremental`::
(For internal use only.) Verify tree connectivity
incrementally by comparing each commit's tree against its
parent trees. Used by `check_connected()` when
`transfer.connectivityCheck` is set to `incremental`.
endif::git-rev-list[]

`--no-walk[=(sorted|unsorted)]`::
Expand Down
243 changes: 243 additions & 0 deletions Documentation/technical/connectivity-check.adoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,243 @@
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 connectivity check maintains this invariant when references
are updated. It trusts the existing connected state 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 trusted 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
-----------------------

`check_connected()` (see `connected.c`) normally performs the
connectivity check using 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]

When promisor remotes are configured, `check_connected()` first
attempts a fast path based on promisor packfiles. If it falls
back to the `rev-list` check, `--exclude-promisor-objects` is
added so that the traversal does not trigger on-demand fetches.

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 check proceeds in three phases:

1. Walk from the incoming tips (T1, T2) against the trusted
refs (L1, L2) to find the incoming set ({N1, N2, N3, T1, T2}).

2. Walk the trees of the boundary commits (B1, B2) and mark
those objects uninteresting. These trees are already trusted
because their commits are on the already-connected side.

3. Walk the trees of each incoming commit and verify that every
referenced object is connected, stopping at objects already
marked uninteresting in phase 2.

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 handled by the subsequent object
traversal.

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 parents.

Trust model
~~~~~~~~~~~

A tree is trusted when its transitive object closure is known to
be connected. Trees reachable from commits on the
already-connected side of the boundary are therefore trusted.

Incoming commits are processed with ancestors before descendants.
Once an incoming commit's tree has been verified, it is trusted
and can be used as a comparison base for later descendants.

This gives an inductive correctness argument: every parent of the
commit currently being verified is either already connected or is
an earlier incoming commit whose tree has already been verified.

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.

Worked 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.
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -1357,6 +1357,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
Expand Down
18 changes: 18 additions & 0 deletions builtin/rev-list.c
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include "commit-reach.h"
#include "quote.h"
#include "strbuf.h"
#include "tree-verify.h"

struct rev_list_info {
struct rev_info *revs;
Expand Down Expand Up @@ -706,6 +707,7 @@ int cmd_rev_list(int argc,
int bisect_find_all = 0;
int use_bitmap_index = 0;
int filter_provided_objects = 0;
int verify_trees_incremental = 0;
const char *show_progress = NULL;
int ret = 0;

Expand Down Expand Up @@ -748,6 +750,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 (!strcmp(arg, "--verify-trees-incremental")) {
verify_trees_incremental = 1;
} else if (skip_prefix(arg, "--missing=", &arg)) {
parse_missing_action_value(repo, arg);
} else if (!strcmp(arg, "-z")) {
Expand Down Expand Up @@ -822,6 +826,8 @@ int cmd_rev_list(int argc,

if (!strcmp(arg, "--exclude-promisor-objects"))
continue; /* already handled above */
if (!strcmp(arg, "--verify-trees-incremental"))
continue; /* already handled above */
if (skip_prefix(arg, "--missing=", &arg))
continue; /* already handled above */

Expand Down Expand Up @@ -935,6 +941,18 @@ int cmd_rev_list(int argc,

prepare_maximal_independent(&revs);

if (verify_trees_incremental) {
struct commit *commit;
struct commit_list *new_commits = NULL;

while ((commit = get_revision(&revs)) != NULL)
commit_list_insert(commit, &new_commits);

verify_commits_incremental(repo, &new_commits,
revs.exclude_promisor_objects);
commit_list_free(new_commits);
}

if (revs.tree_objects)
mark_edges_uninteresting(&revs, show_edge, 0);

Expand Down
24 changes: 24 additions & 0 deletions connected.c
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#define USE_THE_REPOSITORY_VARIABLE

#include "git-compat-util.h"
#include "config.h"
#include "gettext.h"
#include "hex.h"
#include "odb.h"
Expand Down Expand Up @@ -67,6 +68,26 @@ static int check_connected_promisor(oid_iterate_fn fn,
return 1;
}

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, "full"))
return 0;
if (strcasecmp(algorithm, "incremental"))
die(_("unknown transfer.connectivityCheck algorithm '%s'"),
algorithm);

if (opt->is_deepening_fetch)
return 0;

return 1;
}

/*
* If we feed all the commits we want to verify to this command
*
Expand Down Expand Up @@ -133,6 +154,9 @@ int check_connected(oid_iterate_fn fn, void *cb_data,
if (opt->progress)
strvec_pushf(&rev_list.args, "--progress=%s",
_("Checking connectivity"));
if (incremental_check_applicable(opt))
strvec_push(&rev_list.args,
"--verify-trees-incremental");

rev_list.git_cmd = 1;
if (opt->env)
Expand Down
1 change: 1 addition & 0 deletions meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
Loading
Loading