Re: [PATCH 2/2] connected: add incremental connectivity check via rev-list
flat view
From: Junio C Hamano <hidden>
Date: 2026-09-14 15:26:52
"Kristofer Karlsson via GitGitGadget" [off-list ref] writes:
quoted hunk ↗ jump to hunk
+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?
quoted hunk ↗ jump to hunk
+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.