Skip to content

pr-2279/spkrka/replace-connectivity-v1

tagged this 02 Nov 15:47
From: Kristofer Karlsson <krka@spotify.com>

The connectivity check follows replace refs, but other operations
that depend on the repository being complete, such as fsck, prune,
pack-objects and upload-pack, do not. They were taught to ignore
replacements in dae556bdb1 (environment: add global variable to
disable replacement, 2009-01-23), as they must work with the
original objects, and check_connected() came later without doing
the same.

Teach the connectivity check to ignore replacements as well, so
that it verifies the same objects that the commands relying on its
result will read. This is not a problem seen in practice, but the
two can disagree. If a replace ref names an object that is not yet
present locally, the check follows the replacement instead of that
object when it arrives, and accepts an incoming pack that lacks
objects below it. fsck and repack then fail. Such a replace ref can
be the result of fetching refs/replace/* without the branch that
contains the replaced commit, which fsck does not complain about,
but it still takes a sender that sends an incomplete pack.

check_connected() passes --connectivity-check to its rev-list
subprocess, so make rev-list disable replacements when that option
is given, for both the full and the incremental check. Also drop
OBJECT_INFO_LOOKUP_REPLACE from the object reads in tree-verify.c,
used by the incremental check, to match. Disabling replacements
already overrides the flag, but it suggests the opposite intent.

The t5413 test that expected the check to accept a commit whose
original tree is incomplete because a replacement is complete now
expects a rejection, and the test that disabled replacements
explicitly to get one is redundant.

Signed-off-by: Kristofer Karlsson <krka@spotify.com>

Submitted-As: https://lore.kernel.org/git/pull.2279.git.1793634429929.gitgitgadget@gmail.com
Assets 2
Loading