Thread (22 messages) 22 messages, 5 authors, 5d ago

Re: [PATCH v3 1/3] commit-reach: handle cycles in contains walk

From: Kristofer Karlsson <hidden>
Date: 2026-06-12 06:53:53

On Fri, 12 Jun 2026 at 05:00, Tamir Duberstein [off-list ref] wrote:
The memoized contains traversal used by git tag assumes that commit
ancestry is acyclic. Replacement refs can violate that assumption,
causing it to keep pushing an already active commit until memory is
exhausted.
The cycle detection itself makes sense, but would it be simpler to
just die() when a cycle is found rather than falling back to a
second reachability walk?

A cycle in the commit graph means replacement refs are
misconfigured.  The existing code already loops forever when it
hits one, so detecting and dying is strictly an improvement.  The
fallback adds a second codepath through the function, discards all
cached results (so later candidates redo work), and papers over
what is really a broken invariant.

do_lookup_replace_object() already dies when replacement refs
chain deeper than MAXREPLACEDEPTH (which covers cycles), so the
existing contract treats this as a fatal configuration error.
parse_commit_or_die() sets the same precedent within the walk
itself.

Kristofer
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help