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

`brew bundle` sorts installed formulae into dependency order for display
and iteration, feeding runtime dependencies from installed keg tabs into a
TSort graph. Those recorded tab closures can disagree with the current
formulae and form a cycle the live graph does not have (e.g. a keg built
when `webp` depended on `libtiff`, against today's `libtiff` -> `webp`).

`sort!` responded to `TSort::Cyclic` by `odie`ing the entire run, so a
single stale tab aborts `brew bundle` for everyone. This sort only orders
the list; `brew install` resolves its own real dependencies, so a degraded
order is harmless. Every other sorter in the codebase degrades instead
(`upgrade.rb`, `cask/installer.rb`).

Warn and continue with a best-effort order from
`each_strongly_connected_component` instead of aborting, and surface the
real cyclic component from `strongly_connected_components` rather than
parsing it out of the exception message. The reinstall remedy is kept as
advice but softened, since it does not reliably clear these phantom cycles.

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

Author: bendrucker

Library/Homebrew/bundle/brew.rb

@@ -426,24 +426,32 @@ def sort!(formulae)
           raise "formulae_by_full_name is nil" if @formulae_by_full_name.nil?
           raise "formulae_by_name is nil" if @formulae_by_name.nil?
 
-          @formulae = topo.tsort
-                          .map { |name| @formulae_by_full_name[name] || @formulae_by_name[name] }
-                          .uniq { |f| f[:full_name] }
-        rescue TSort::Cyclic => e
-          e.message =~ /\["([^"]*)".*"([^"]*)"\]/
-          cycle_first = Regexp.last_match(1)
-          cycle_last = Regexp.last_match(2)
-          odie e.message if !cycle_first || !cycle_last
-
-          odie <<~EOS
-            Formulae dependency graph sorting failed (likely due to a circular dependency):
-            #{cycle_first}: #{topo[cycle_first] if topo}
-            #{cycle_last}: #{topo[cycle_last] if topo}
-            Please run the following commands and try again:
-              brew update
-              brew uninstall --ignore-dependencies --force #{cycle_first} #{cycle_last}
-              brew install #{cycle_first} #{cycle_last}
-          EOS
+          sorted_names = begin
+            topo.tsort
+          rescue TSort::Cyclic
+            # The graph is built from runtime dependencies recorded in installed
+            # keg tabs, which can disagree with the current formulae and form a
+            # cycle the live graph does not have (see Homebrew/homebrew-bundle#1513).
+            # Warn and keep the bundle running with a best-effort order rather than
+            # aborting. The reinstall remedy below does not reliably clear such cycles.
+            cyclic = (topo.strongly_connected_components.find { |c| c.size > 1 } || [])
+                     .filter_map { |n| @formulae_by_full_name[n] || @formulae_by_name[n] }
+                     .uniq { |f| f[:full_name] }
+                     .map { |f| f[:full_name] }
+            opoo <<~EOS
+              Formulae dependency graph sorting failed (likely due to a circular dependency):
+                #{cyclic.join(", ")}
+              If this persists, run the following commands and try again:
+                brew update
+                brew uninstall --ignore-dependencies --force #{cyclic.join(" ")}
+                brew install #{cyclic.join(" ")}
+            EOS
+            topo.each_strongly_connected_component.to_a.flatten
+          end
+
+          @formulae = sorted_names
+                      .map { |name| @formulae_by_full_name[name] || @formulae_by_name[name] }
+                      .uniq { |f| f[:full_name] }
         end
       end
 

Library/Homebrew/test/bundle/brew_spec.rb

@@ -160,6 +160,26 @@
       }
     end
 
+    def formula_double_depending_on(name, dependency)
+      instance_double(Formula,
+                      name:,
+                      desc:                   name,
+                      oldnames:               [],
+                      full_name:              name,
+                      any_version_installed?: true,
+                      aliases:                [],
+                      runtime_dependencies:   [instance_double(Dependency, name: dependency)],
+                      deps:                   [],
+                      conflicts:              [],
+                      any_installed_prefix:   nil,
+                      linked?:                false,
+                      keg_only?:              true,
+                      pinned?:                false,
+                      outdated?:              false,
+                      stable:                 instance_double(SoftwareSpec, bottle_defined?: false, bottled?: false),
+                      tap:                    instance_double(Tap, official?: false))
+    end
+
     before do
       described_class.reset!
     end
@@ -180,13 +200,13 @@
         expect(dumper.formulae_by_full_name("bar")).to eql({})
       end
 
-      it "exits on cyclic exceptions" do
-        expect(Formula).to receive(:installed).and_return([foo, bar, baz])
-        expect_any_instance_of(Homebrew::Bundle::Brew::Topo).to receive(:tsort).and_raise(
-          TSort::Cyclic,
-          'topological sort failed: ["foo", "bar"]',
-        )
-        expect { dumper.formulae_by_full_name }.to raise_error(SystemExit)
+      it "warns and continues on cyclic dependencies instead of exiting" do
+        cyclic_foo = formula_double_depending_on("foo", "bar")
+        cyclic_bar = formula_double_depending_on("bar", "foo")
+        expect(Formula).to receive(:installed).and_return([cyclic_foo, cyclic_bar])
+
+        expect { dumper.formulae_by_full_name }.to output(/dependency graph sorting failed/).to_stderr
+        expect(dumper.formulae.map { |f| f[:full_name] }).to contain_exactly("foo", "bar")
       end
 
       it "returns a hash for a formula" do
@@ -916,5 +936,13 @@
 
       expect(topo.tsort).to eq(["libice", "b", "a"])
     end
+
+    it "flattens a cyclic graph via strongly connected components without raising" do
+      topo = described_class.new
+      topo["a"] = ["b"]
+      topo["b"] = ["a"]
+
+      expect(topo.each_strongly_connected_component.to_a.flatten).to contain_exactly("a", "b")
+    end
   end
 end