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

>
> On Fri, 12 Jun 2026 at 05:00, Tamir Duberstein <tamird@gmail.com> 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.