Refactor Formula/Cask dependencies.

Library/Homebrew/cask/lib/hbc/cask.rb

@@ -71,6 +71,15 @@ module Hbc
       @token
     end
 
+    def hash
+      token.hash
+    end
+
+    def eql?(other)
+      token == other.token
+    end
+    alias == eql?
+
     def dumpcask
       odebug "Cask instance dumps in YAML:"
       odebug "Cask instance toplevel:", to_yaml

Library/Homebrew/cask/lib/hbc/cask_dependencies.rb

@@ -1,35 +1,36 @@
+require "delegate"
+
 require "hbc/topological_hash"
 
 module Hbc
-  class CaskDependencies
-    attr_reader :cask, :graph, :sorted
+  class CaskDependencies < DelegateClass(Array)
+    attr_reader :cask, :graph
 
     def initialize(cask)
       @cask = cask
       @graph = graph_dependencies
-      @sorted = sort
+      super(sort)
     end
 
-    def graph_dependencies
-      deps_in = ->(csk) { csk.depends_on ? csk.depends_on.cask || [] : [] }
-      walk = lambda do |acc, deps|
+    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|
-          next if acc.key?(dep)
-          succs = deps_in.call CaskLoader.load(dep)
-          acc[dep] = succs
-          walk.call(acc, succs)
+        graph_dependencies(dep, acc)
       end
       acc
     end
 
-      graphed = walk.call({}, @cask.depends_on.cask)
-      TopologicalHash[graphed]
-    end
-
     def sort
-      @graph.tsort
+      raise CaskSelfReferencingDependencyError, cask.token if graph[cask].include?(cask)
+      graph.tsort - [cask]
     rescue TSort::Cyclic
-      raise CaskCyclicCaskDependencyError, @cask.token
+      strongly_connected_components = graph.strongly_connected_components.sort_by(&:count)
+      cyclic_dependencies = strongly_connected_components.last - [cask]
+      raise CaskCyclicDependencyError.new(cask.token, cyclic_dependencies.join(", "))
     end
   end
 end

Library/Homebrew/cask/lib/hbc/dsl.rb

@@ -211,10 +211,10 @@ module Hbc
 
     # depends_on uses a load method so that multiple stanzas can be merged
     def depends_on(*args)
-      return @depends_on if args.empty?
       @depends_on ||= DSL::DependsOn.new
+      return @depends_on if args.empty?
       begin
-        @depends_on.load(*args) unless args.empty?
+        @depends_on.load(*args)
       rescue RuntimeError => e
         raise CaskInvalidError.new(token, e)
       end

Library/Homebrew/cask/lib/hbc/dsl/depends_on.rb

@@ -24,6 +24,8 @@ module Hbc
 
       def initialize
         @pairs ||= {}
+        @cask ||= []
+        @formula ||= []
       end
 
       def load(pairs = {})
@@ -53,12 +55,10 @@ module Hbc
       end
 
       def formula=(*args)
-        @formula ||= []
         @formula.concat(args)
       end
 
       def cask=(*args)
-        @cask ||= []
         @cask.concat(args)
       end
 

Library/Homebrew/cask/lib/hbc/exceptions.rb

@@ -77,9 +77,15 @@ module Hbc
     end
   end
 
-  class CaskCyclicCaskDependencyError < AbstractCaskErrorWithToken
+  class CaskCyclicDependencyError < AbstractCaskErrorWithToken
     def to_s
-      "Cask '#{token}' includes cyclic dependencies on other Casks and could not be installed."
+      "Cask '#{token}' includes cyclic dependencies on other Casks" << (reason.empty? ? "." : ": #{reason}")
+    end
+  end
+
+  class CaskSelfReferencingDependencyError < CaskCyclicDependencyError
+    def to_s
+      "Cask '#{token}' depends on itself."
     end
   end
 
@@ -91,7 +97,7 @@ module Hbc
 
   class CaskInvalidError < AbstractCaskErrorWithToken
     def to_s
-      "Cask '#{token}' definition is invalid" << (reason.empty? ? ".": ": #{reason}")
+      "Cask '#{token}' definition is invalid" << (reason.empty? ? "." : ": #{reason}")
     end
   end
 

Library/Homebrew/cask/lib/hbc/installer.rb

@@ -1,5 +1,7 @@
 require "rubygems"
 
+require "formula_installer"
+
 require "hbc/cask_dependencies"
 require "hbc/staged"
 require "hbc/verify"
@@ -197,7 +199,6 @@ module Hbc
       x11_dependencies
       formula_dependencies
       cask_dependencies unless skip_cask_deps?
-      puts "complete"
     end
 
     def macos_dependencies
@@ -234,36 +235,50 @@ module Hbc
     end
 
     def formula_dependencies
-      return unless @cask.depends_on.formula && !@cask.depends_on.formula.empty?
-      ohai "Installing Formula dependencies from Homebrew"
-      @cask.depends_on.formula.each do |dep_name|
-        print "#{dep_name} ... "
-        installed = @command.run(HOMEBREW_BREW_FILE,
-                                 args:         ["list", "--versions", dep_name],
-                                 print_stderr: false).stdout.include?(dep_name)
-        if installed
-          puts "already installed"
-        else
-          @command.run!(HOMEBREW_BREW_FILE,
-                        args: ["install", dep_name])
-          puts "done"
+      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
+
+      not_installed = formulae.reject(&:any_version_installed?)
+
+      ohai "Installing Formula dependencies: #{not_installed.map(&:to_s).join(", ")}"
+      not_installed.each do |formula|
+        begin
+          old_argv = ARGV.dup
+          ARGV.replace([])
+          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
+        ensure
+          ARGV.replace(old_argv)
         end
       end
     end
 
     def cask_dependencies
-      return unless @cask.depends_on.cask && !@cask.depends_on.cask.empty?
-      ohai "Installing Cask dependencies: #{@cask.depends_on.cask.join(", ")}"
-      deps = CaskDependencies.new(@cask)
-      deps.sorted.each do |dep_token|
-        puts "#{dep_token} ..."
-        dep = CaskLoader.load(dep_token)
-        if dep.installed?
-          puts "already installed"
-        else
-          Installer.new(dep, binaries: binaries?, verbose: verbose?, skip_cask_deps: true, force: false).install
-          puts "done"
+      return if @cask.depends_on.cask.empty?
+      casks = CaskDependencies.new(@cask)
+
+      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?, skip_cask_deps: true, force: false).install
       end
     end
 

Library/Homebrew/test/cask/depends_on_spec.rb

@@ -12,7 +12,7 @@ describe "Satisfy Dependencies and Requirements", :cask do
   describe "depends_on cask" do
     context "when depends_on cask is cyclic" do
       let(:cask) { Hbc::CaskLoader.load_from_file(TEST_FIXTURE_DIR/"cask/Casks/with-depends-on-cask-cyclic.rb") }
-      it { is_expected.to raise_error(Hbc::CaskCyclicCaskDependencyError) }
+      it { is_expected.to raise_error(Hbc::CaskCyclicDependencyError) }
     end
 
     context do