Refactor cask dependency resolution.

Library/Homebrew/cask/all.rb

@@ -11,12 +11,10 @@
 require "cask/caskroom"
 require "cask/checkable"
 require "cask/cmd"
-require "cask/cask_dependencies"
 require "cask/exceptions"
 require "cask/installer"
 require "cask/macos"
 require "cask/pkg"
 require "cask/staged"
-require "cask/topological_hash"
 require "cask/utils"
 require "cask/verify"

Library/Homebrew/cask/installer.rb

@@ -5,7 +5,7 @@
 require "formula_installer"
 require "unpack_strategy"
 
-require "cask/cask_dependencies"
+require "cask/topological_hash"
 require "cask/config"
 require "cask/download"
 require "cask/staged"
@@ -60,20 +60,18 @@ def self.caveats(cask)
     def fetch
       odebug "Cask::Installer#fetch"
 
-      satisfy_dependencies
-
       verify_has_sha if require_sha? && !force?
       download
       verify
+
+      satisfy_dependencies
     end
 
     def stage
       odebug "Cask::Installer#stage"
 
       Caskroom.ensure_caskroom_exists
 
-      unpack_dependencies
-
       extract_primary_container
       save_caskfile
     rescue => e
@@ -248,12 +246,10 @@ def install_artifacts
     def satisfy_dependencies
       return unless @cask.depends_on
 
-      ohai "Satisfying dependencies"
       macos_dependencies
       arch_dependencies
       x11_dependencies
-      formula_dependencies
-      cask_dependencies unless skip_cask_deps? || installed_as_dependency?
+      formula_and_cask_dependencies
     end
 
     def macos_dependencies
@@ -283,80 +279,90 @@ def x11_dependencies
       raise CaskX11DependencyError, @cask.token unless MacOS::X11.installed?
     end
 
-    def formula_dependencies
-      formulae = @cask.depends_on.formula.map { |f| Formula[f] }
-      return if formulae.empty?
-
-      if formulae.all?(&:any_version_installed?)
-        puts "All Formula dependencies satisfied."
-        return
-      end
+    def graph_dependencies(cask_or_formula, acc = TopologicalHash.new)
+      return acc if acc.key?(cask_or_formula)
 
-      not_installed = formulae.reject(&:any_version_installed?)
-
-      ohai "Installing Formula dependencies: #{not_installed.map(&:to_s).join(", ")}"
-      not_installed.each do |formula|
-        FormulaInstaller.new(formula).tap do |fi|
-          fi.installed_as_dependency = true
-          fi.installed_on_request = false
-          fi.show_header = true
-          fi.verbose = verbose?
-          fi.prelude
-          fi.install
-          fi.finish
-        end
+      if cask_or_formula.is_a?(Cask)
+        formula_deps = cask_or_formula.depends_on.formula.map { |f| Formula[f] }
+        cask_deps = cask_or_formula.depends_on.cask.map(&CaskLoader.public_method(:load))
+      else
+        formula_deps = cask_or_formula.deps.reject(&:build?).map(&:to_formula)
+        cask_deps = cask_or_formula.requirements.map(&:cask).compact.map(&CaskLoader.public_method(:load))
       end
-    end
 
-    def cask_dependencies
-      casks = CaskDependencies.new(@cask)
-      return if casks.empty?
+      acc[cask_or_formula] ||= []
+      acc[cask_or_formula] += formula_deps
+      acc[cask_or_formula] += cask_deps
 
-      if casks.all?(&:installed?)
-        puts "All Cask dependencies satisfied."
-        return
+      formula_deps.each do |f|
+        graph_dependencies(f, acc)
       end
 
-      not_installed = casks.reject(&:installed?)
-
-      ohai "Installing Cask dependencies: #{not_installed.map(&:to_s).join(", ")}"
-      not_installed.each do |cask|
-        Installer.new(
-          cask,
-          binaries:                binaries?,
-          verbose:                 verbose?,
-          installed_as_dependency: true,
-          force:                   false,
-        ).install
+      cask_deps.each do |c|
+        graph_dependencies(c, acc)
       end
+
+      acc
     end
 
-    def unpack_dependencies
-      formulae = primary_container.dependencies.select { |dep| dep.is_a?(Formula) }
-      casks = primary_container.dependencies.select { |dep| dep.is_a?(Cask) }
-                               .flat_map { |cask| [*CaskDependencies.new(cask), cask] }
+    def formula_and_cask_dependencies
+      return if installed_as_dependency?
+
+      graph = graph_dependencies(@cask)
 
-      not_installed_formulae = formulae.reject(&:any_version_installed?)
-      not_installed_casks = casks.reject(&:installed?)
+      raise CaskSelfReferencingDependencyError, cask.token if graph[@cask].include?(@cask)
 
-      return if (not_installed_formulae + not_installed_casks).empty?
+      primary_container.dependencies.each do |dep|
+        graph_dependencies(dep, graph)
+      end
 
-      ohai "Satisfying unpack dependencies"
+      formulae_and_casks = begin
+        graph.tsort - [@cask]
+      rescue TSort::Cyclic
+        strongly_connected_components = graph.strongly_connected_components.sort_by(&:count)
+        cyclic_dependencies = strongly_connected_components.last - [@cask]
+        raise CaskCyclicDependencyError.new(@cask.token, cyclic_dependencies.to_sentence)
+      end
 
-      not_installed_formulae.each do |formula|
-        FormulaInstaller.new(formula).tap do |fi|
-          fi.installed_as_dependency = true
-          fi.installed_on_request = false
-          fi.show_header = true
-          fi.verbose = verbose?
-          fi.prelude
-          fi.install
-          fi.finish
-        end
+      return if formulae_and_casks.empty?
+
+      not_installed_formulae_and_casks = formulae_and_casks
+                                         .reject do |cask_or_formula|
+        (cask_or_formula.respond_to?(:installed?) && cask_or_formula.installed?) ||
+          (cask_or_formula.respond_to?(:any_version_installed?) && cask_or_formula.any_version_installed?)
+      end
+
+      if not_installed_formulae_and_casks.empty?
+        puts "All Formula dependencies satisfied."
+        return
       end
 
-      not_installed_casks.each do |cask|
-        Installer.new(cask, verbose: verbose?, installed_as_dependency: true).install
+      ohai "Installing dependencies: #{not_installed_formulae_and_casks.map(&:to_s).join(", ")}"
+      not_installed_formulae_and_casks.each do |cask_or_formula|
+        if cask_or_formula.is_a?(Cask)
+          if skip_cask_deps?
+            opoo "`--skip-cask-deps` is set; skipping installation of #{@cask}."
+            next
+          end
+
+          Installer.new(
+            cask_or_formula,
+            binaries:                binaries?,
+            verbose:                 verbose?,
+            installed_as_dependency: true,
+            force:                   false,
+          ).install
+        else
+          FormulaInstaller.new(cask_or_formula).yield_self do |fi|
+            fi.installed_as_dependency = true
+            fi.installed_on_request = false
+            fi.show_header = true
+            fi.verbose = verbose?
+            fi.prelude
+            fi.install
+            fi.finish
+          end
+        end
       end
     end
 

Library/Homebrew/test/cask/installer_spec.rb

@@ -114,7 +114,6 @@
         described_class.new(with_installer_manual).install
       }.to output(
         <<~EOS,
-          ==> Satisfying dependencies
           ==> Downloading file://#{HOMEBREW_LIBRARY_PATH}/test/support/fixtures/cask/caffeine.zip
           ==> Verifying SHA-256 checksum for Cask 'with-installer-manual'.
           ==> Installing Cask with-installer-manual