Refactor cask dependency resolution.

Library/Homebrew/cask/all.rb

@@ -11,12 +11,10 @@ require "cask/cask_loader"
 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/cask_dependencies.rb

@@ -1,40 +0,0 @@
-# frozen_string_literal: true
-
-require "delegate"
-
-require "cask/topological_hash"
-
-module Cask
-  class CaskDependencies < DelegateClass(Array)
-    attr_reader :cask, :graph
-
-    def initialize(cask)
-      @cask = cask
-      @graph = graph_dependencies
-      super(sort)
-    end
-
-    private
-
-    def graph_dependencies(cask = self.cask, acc = TopologicalHash.new)
-      return acc if acc.key?(cask)
-
-      deps = cask.depends_on.cask.map(&CaskLoader.public_method(:load))
-      acc[cask] = deps
-      deps.each do |dep|
-        graph_dependencies(dep, acc)
-      end
-      acc
-    end
-
-    def sort
-      raise CaskSelfReferencingDependencyError, cask.token if graph[cask].include?(cask)
-
-      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
-  end
-end

Library/Homebrew/cask/installer.rb

@@ -5,7 +5,7 @@ require "rubygems"
 require "formula_installer"
 require "unpack_strategy"
 
-require "cask/cask_dependencies"
+require "cask/topological_hash"
 require "cask/config"
 require "cask/download"
 require "cask/staged"
@@ -60,11 +60,11 @@ module Cask
     def fetch
       odebug "Cask::Installer#fetch"
 
-      satisfy_dependencies
-
       verify_has_sha if require_sha? && !force?
       download
       verify
+
+      satisfy_dependencies
     end
 
     def stage
@@ -72,8 +72,6 @@ module Cask
 
       Caskroom.ensure_caskroom_exists
 
-      unpack_dependencies
-
       extract_primary_container
       save_caskfile
     rescue => e
@@ -248,12 +246,10 @@ module Cask
     def satisfy_dependencies
       return unless @cask.depends_on
 
-      ohai "Satisfying dependencies"
       macos_dependencies
       arch_dependencies
       x11_dependencies
-      formula_dependencies
+      formula_and_cask_dependencies
-      cask_dependencies unless skip_cask_deps? || installed_as_dependency?
     end
 
     def macos_dependencies
@@ -283,83 +279,93 @@ module Cask
       raise CaskX11DependencyError, @cask.token unless MacOS::X11.installed?
     end
 
-    def formula_dependencies
+    def graph_dependencies(cask_or_formula, acc = TopologicalHash.new)
-      formulae = @cask.depends_on.formula.map { |f| Formula[f] }
+      return acc if acc.key?(cask_or_formula)
-      return if formulae.empty?
 
-      if formulae.all?(&:any_version_installed?)
+      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
+
+      acc[cask_or_formula] ||= []
+      acc[cask_or_formula] += formula_deps
+      acc[cask_or_formula] += cask_deps
+
+      formula_deps.each do |f|
+        graph_dependencies(f, acc)
+      end
+
+      cask_deps.each do |c|
+        graph_dependencies(c, acc)
+      end
+
+      acc
+    end
+
+    def formula_and_cask_dependencies
+      return if installed_as_dependency?
+
+      graph = graph_dependencies(@cask)
+
+      raise CaskSelfReferencingDependencyError, cask.token if graph[@cask].include?(@cask)
+
+      primary_container.dependencies.each do |dep|
+        graph_dependencies(dep, graph)
+      end
+
+      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
+
+      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 = formulae.reject(&:any_version_installed?)
+      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
 
-      ohai "Installing Formula dependencies: #{not_installed.map(&:to_s).join(", ")}"
+          Installer.new(
-      not_installed.each do |formula|
+            cask_or_formula,
-        FormulaInstaller.new(formula).tap do |fi|
+            binaries:                binaries?,
-          fi.installed_as_dependency = true
+            verbose:                 verbose?,
-          fi.installed_on_request = false
+            installed_as_dependency: true,
-          fi.show_header = true
+            force:                   false,
-          fi.verbose = verbose?
+          ).install
-          fi.prelude
+        else
-          fi.install
+          FormulaInstaller.new(cask_or_formula).yield_self do |fi|
-          fi.finish
+            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
 
-    def cask_dependencies
-      casks = CaskDependencies.new(@cask)
-      return if casks.empty?
-
-      if casks.all?(&:installed?)
-        puts "All Cask dependencies satisfied."
-        return
-      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
-      end
-    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] }
-
-      not_installed_formulae = formulae.reject(&:any_version_installed?)
-      not_installed_casks = casks.reject(&:installed?)
-
-      return if (not_installed_formulae + not_installed_casks).empty?
-
-      ohai "Satisfying unpack dependencies"
-
-      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
-      end
-
-      not_installed_casks.each do |cask|
-        Installer.new(cask, verbose: verbose?, installed_as_dependency: true).install
-      end
-    end
-
     def caveats
       self.class.caveats(@cask)
     end

Library/Homebrew/test/cask/installer_spec.rb

@@ -114,7 +114,6 @@ describe Cask::Installer, :cask do
         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