Re: [PATCH v2 1/2] Documentation: describe connectivity checking

Thanks, it's clear I'll need to do some more wordsmithing to
improve the clarity here.  I'll reroll/rewrite parts of it in
some way (and aligned with the answers below).

On Mon, 5 Oct 2026 at 09:59, Patrick Steinhardt [off-list ref] wrote:

quoted

+A repository is connected when every object reachable from its
+references is available locally (with exceptions noted below).

Right. I think it would also be important to spell out the reverse of
this, which is that nothing can be assumed about objects that aren't
reachable by any reference. So even if an object already exists in the
object database, it is not safe to assume that it is fully connected
unless it is referenced.

Fair, I can make that more explicit in the text.

quoted

+The connectivity check maintains this invariant when references
+are updated.  It trusts the existing connected state and verifies

Nit: it's basically already implicit, but I'd clarify that "existing
connected state" is again just the connected state of objects reachable
from reference tips. So maybe "It trusts that all objects reachable from
references are already fully connected and verifies..."

Agreed, I will make that more clear.

quoted

+Full connectivity check
+-----------------------
+
+`check_connected()` (see `connected.c`) normally performs the

I'm always a bit hesitant to directly refer to code in our docs. We
should either make this documentation part of "connected.c" directly, or
we should not refer to code. Otherwise, chances that this documentation
grows stale is very high.

Good point, I will remove it and make the logic more self contained.

quoted

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

I feel like these phases here basically just explain how revision walks
work without adding any more details that are specifically relevant to
the connectivity check.

Yes, though I think this is important for understanding how the
connectivity-check is implemented to see the relation to the
rev-list walk.  I have some hope that this dependency can go
away in the future, since the general rev-list function is not
necessarily the most optimal way to reason about connectivity.

That said, I am happy to remove this part of it's not deemed
useful.

quoted

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

Huh, what subsequent object traversal? This part puzzles me a bit.

Good point, this is meant to reference back to the rev-list
implementation but that's not very clear.  And like
the previous feedback, perhaps this should also go away
(or be reworded in a more self contained way and less tied
to rev-list)

Thanks,
Kristofer