Warn instead of aborting brew bundle on stale-tab dependency cycles

[bendrucker]

brew bundle sorts installed formulae into dependency order for display and iteration. The edges come from each installed keg's recorded runtime dependencies (tab.json), not the live formula graph. Those recorded closures are frozen at install time, so kegs of different vintages can disagree and form a cycle the current formulae don't have (e.g. a webp keg built when webp depended on libtiff, against today's libtiff -> webp).

I hit this here:

https://github.com/bendrucker/dotfiles/actions/runs/28981041126/job/85999630681#step:10:45

2004l25h1002l1003l1006l::error::Formulae dependency graph sorting failed (likely due to a circular dependency):%0Alibtiff: ["linux-headers@6.8", "glibc", "gmp", "isl", "mpfr", "libmpc", "lz4", "xz", "zlib-ng-compat", "zstd", "binutils", "gcc", "jpeg-turbo", "giflib", "libpng", "webp"]%0Awebp: ["linux-headers@6.8", "glibc", "gmp", "isl", "mpfr", "libmpc", "lz4", "xz", "zlib-ng-compat", "zstd", "binutils", "gcc", "giflib", "jpeg-turbo", "libpng", "libtiff"]%0APlease run the following commands and try again:%0A  brew update%0A  brew uninstall --ignore-dependencies --force libtiff webp%0A  brew install libtiff webp%0A

sort! responded to the cycle by odieing the entire run, so one stale tab aborts brew bundle regardless of the Brewfile. But this sort only orders a display/iteration list. brew install resolves its own dependencies, so a degraded order is harmless, and brew bundle already tolerates this same tab-derived dependency data being cyclic elsewhere (parallel_installer falls back to sequential scheduling rather than failing).

This orders with each_strongly_connected_component instead. It yields nodes dependency-first like tsort but does not raise on a cycle, and for an acyclic graph the result is identical to tsort, so normal runs are unchanged. When a cycle exists it collapses into one component: brew bundle warns, names that component, and continues.

The warning now names the full cyclic component rather than the two endpoints the old code parsed out of the exception message. The stale edge can live on any member's tab, so reinstalling only the two named formulae often left the cycle in place (homebrew-bundle#1513). Naming every member makes the suggested uninstall/install rewrite each tab in the cycle.

The flipped spec drives a real cycle through fixtures so it exercises the fallback end to end and fails if the hard exit returns.

---

• Have you followed our Contributing guidelines?
• Have you checked for other open Pull Requests for the same change?
• Have you explained what your changes do? Performance claims (e.g. "this is faster") must include Hyperfine benchmarks.
• Have you explained why you'd like these changes included, not just what they do?
• For bug fixes, have you given step-by-step brew commands to reproduce the bug?
• Have you written new tests (excluding integration tests)? Here's an example.
• Have you successfully run brew lgtm (style, typechecking and tests) locally?

---

• AI was used to generate or assist with generating this PR.

Claude Code / Opus 4.8

Verification Steps:

• Regression test, red/green: against main the odie aborts and the example raises SystemExit; it passes with the fix.
• Acyclic path unchanged: for a non-cyclic graph each_strongly_connected_component yields the same dependencies-first order as tsort, and the .map/.uniq mapping is untouched, so normal runs produce identical output.
• Confirmed the live webp/libtiff formulae are one-directional, so the reported cycle comes from a stale keg tab rather than the current formulae.

An end-to-end repro needs an installed keg carrying a stale tab, which isn't practical in CI, so the flipped spec is the regression guard rather than an integration test.

[Copilot]

Pull request overview

Updates brew bundle’s formula dependency ordering so stale keg-tab dependency cycles no longer abort the entire run, instead warning and continuing with a best-effort order derived from strongly connected components.

Changes:

• Replace TSort::Cyclic hard-exit during formula sorting with an opoo warning + fallback ordering.
• Improve cycle reporting by using strongly connected components rather than parsing the exception message.
• Update/extend specs to assert the new “warn and continue” behavior on cyclic graphs.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
Library/Homebrew/bundle/brew.rb	Changes cycle handling in formula dependency sorting to warn and continue with SCC-based fallback ordering.
Library/Homebrew/test/bundle/brew_spec.rb	Updates expectations to ensure cyclic dependency sorting no longer exits, and adds coverage around SCC behavior.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[bendrucker]

Addressed the Copilot review (both threads resolved).

Its concern was that a self-referential dependency would still raise TSort::Cyclic while producing an empty cyclic list and a bare brew uninstall ... --force with no arguments. That path isn't reachable: Ruby's TSort#tsort only raises for a strongly-connected component with more than one node, and a self-loop is a size-1 component that sorts normally.

t = Topo.new
t["a"] = ["a"]
t.tsort   # => ["a"], no raise

A real two-node cycle (a -> b -> a) does raise. So whenever this rescue runs there is by definition a size > 1 component for find { |c| c.size > 1 } to return, and cyclic is never empty. No code change was warranted.

[MikeMcQuaid]

Thanks for PR! I agree this is a problem that should be fixed but I'm not (yet) convinced this is the best/right solution yet.

[bendrucker]

Fair enough let me scrutinize this harder and try to think up some alternatives. Will reply to the comments as well.

[bendrucker]

Reworked, definitely a better way to go about this.

Couple notes:

• brew install resolves its own deps, so a degraded order is harmless. This is only affecting dump/iteration output.
• Cycles here are artifacts of stale data. Edges come from keg-tab recorded runtime closures frozen at install time. A live declared cycle would break Dependency.expand everywhere, so it can't occur.
• The old odie's own remedy was broken. It parsed only the first/last SCC members out of the exception message, but the stale edge can sit on any member's tab, so the reinstall often didn't clear it (may explain hbc/cmd/style: bump rubocop-cask to 0.10.6 #1513). The revised code names the full component, so the suggested uninstall/install rewrites every tab in the cycle.
• Similarly, parallel_installer already falls back to sequential scheduling when the same tab-derived dep map is unsatisfiable, rather than failing.
• Alternatively, I could implement something more like upgrade.rb: quiet/silent for users, raise for developer.

[MikeMcQuaid]

Thanks. Messaging looking better! Getting closer.

[MikeMcQuaid]

Thanks, this is a really nice cleanup as well as a user improvement, glad we had a few rounds of back and forth on this. Great work @bendrucker!