Merge pull request #1498 from alyssais/uninstall_developer_warning

Warn developers when uninstalling a dependency

4 files changed, 115 insertions, 22 deletions

diff --git a/Library/Homebrew/cmd/uninstall.rb b/Library/Homebrew/cmd/uninstall.rb
index d9e6a7377..34695bd1c 100644
--- a/Library/Homebrew/cmd/uninstall.rb
+++ b/Library/Homebrew/cmd/uninstall.rb
@@ -28,10 +28,8 @@ module Homebrew
       ARGV.kegs.group_by(&:rack)
     end
 
-    if should_check_for_dependents?
-      all_kegs = kegs_by_rack.values.flatten(1)
-      return if check_for_dependents all_kegs
-    end
+    handle_unsatisfied_dependents(kegs_by_rack)
+    return if Homebrew.failed?
 
     kegs_by_rack.each do |rack, kegs|
       if ARGV.force?
@@ -78,30 +76,77 @@ module Homebrew
     end
   end
 
-  def should_check_for_dependents?
-    # --ignore-dependencies, to be consistent with install
-    return false if ARGV.include?("--ignore-dependencies")
-    return false if ARGV.homebrew_developer?
-    true
+  def handle_unsatisfied_dependents(kegs_by_rack)
+    return if ARGV.include?("--ignore-dependencies")
+
+    all_kegs = kegs_by_rack.values.flatten(1)
+    check_for_dependents all_kegs
   end
 
   def check_for_dependents(kegs)
     return false unless result = Keg.find_some_installed_dependents(kegs)
 
-    requireds, dependents = result
-
-    msg = "Refusing to uninstall #{requireds.join(", ")} because "
-    msg << (requireds.count == 1 ? "it is" : "they are")
-    msg << " required by #{dependents.join(", ")}, which "
-    msg << (dependents.count == 1 ? "is" : "are")
-    msg << " currently installed."
-    ofail msg
-    print "You can override this and force removal with "
-    puts "`brew uninstall --ignore-dependencies #{requireds.map(&:name).join(" ")}`."
+    if ARGV.homebrew_developer?
+      DeveloperDependentsMessage.new(*result).output
+    else
+      NondeveloperDependentsMessage.new(*result).output
+    end
 
     true
   end
 
+  class DependentsMessage
+    attr_reader :reqs, :deps
+
+    def initialize(requireds, dependents)
+      @reqs = requireds
+      @deps = dependents
+    end
+
+    protected
+
+    def are(items)
+      items.count == 1 ? "is" : "are"
+    end
+
+    def they(items)
+      items.count == 1 ? "it" : "they"
+    end
+
+    def list(items)
+      items.join(", ")
+    end
+
+    def sample_command
+      "brew uninstall --ignore-dependencies #{list reqs.map(&:name)}"
+    end
+
+    def are_required_by_deps
+      "#{are reqs} required by #{list deps}, which #{are deps} currently installed"
+    end
+  end
+
+  class DeveloperDependentsMessage < DependentsMessage
+    def output
+      opoo <<-EOS.undent
+        #{list reqs} #{are_required_by_deps}.
+        You can silence this warning with:
+          #{sample_command}
+      EOS
+    end
+  end
+
+  class NondeveloperDependentsMessage < DependentsMessage
+    def output
+      ofail <<-EOS.undent
+        Refusing to uninstall #{list reqs}
+        because #{they reqs} #{are_required_by_deps}.
+        You can override this and force removal with:
+          #{sample_command}
+      EOS
+    end
+  end
+
   def rm_pin(rack)
     Formulary.from_rack(rack).unpin
   rescue
diff --git a/Library/Homebrew/test/test_uninstall.rb b/Library/Homebrew/test/test_uninstall.rb
index a7859b7ad..c9b3e0be3 100644
--- a/Library/Homebrew/test/test_uninstall.rb
+++ b/Library/Homebrew/test/test_uninstall.rb
@@ -2,20 +2,55 @@ require "helper/integration_command_test_case"
 require "cmd/uninstall"
 
 class UninstallTests < Homebrew::TestCase
+  def setup
+    @dependency = formula("dependency") { url "f-1" }
+    @dependent = formula("dependent") do
+      url "f-1"
+      depends_on "dependency"
+    end
+
+    [@dependency, @dependent].each { |f| f.installed_prefix.mkpath }
+
+    tab = Tab.empty
+    tab.tabfile = @dependent.installed_prefix/Tab::FILENAME
+    tab.runtime_dependencies = [
+      { "full_name" => "dependency", "version" => "1" },
+    ]
+    tab.write
+
+    stub_formula_loader @dependency
+    stub_formula_loader @dependent
+  end
+
+  def teardown
+    Homebrew.failed = false
+    [@dependency, @dependent].each { |f| f.rack.rmtree }
+  end
+
+  def handle_unsatisfied_dependents
+    capture_stderr do
+      opts = { @dependency.rack => [Keg.new(@dependency.installed_prefix)] }
+      Homebrew.handle_unsatisfied_dependents(opts)
+    end
+  end
+
   def test_check_for_testball_f2s_when_developer
-    refute_predicate Homebrew, :should_check_for_dependents?
+    assert_match "Warning", handle_unsatisfied_dependents
+    refute_predicate Homebrew, :failed?
   end
 
   def test_check_for_dependents_when_not_developer
     run_as_not_developer do
-      assert_predicate Homebrew, :should_check_for_dependents?
+      assert_match "Error", handle_unsatisfied_dependents
+      assert_predicate Homebrew, :failed?
     end
   end
 
   def test_check_for_dependents_when_ignore_dependencies
     ARGV << "--ignore-dependencies"
     run_as_not_developer do
-      refute_predicate Homebrew, :should_check_for_dependents?
+      assert_empty handle_unsatisfied_dependents
+      refute_predicate Homebrew, :failed?
     end
   ensure
     ARGV.delete("--ignore-dependencies")
diff --git a/Library/Homebrew/test/test_utils.rb b/Library/Homebrew/test/test_utils.rb
index 11332e450..7c0b6f78a 100644
--- a/Library/Homebrew/test/test_utils.rb
+++ b/Library/Homebrew/test/test_utils.rb
@@ -174,6 +174,10 @@ class UtilTests < Homebrew::TestCase
     end
   end
 
+  def test_capture_stderr
+    assert_equal "test\n", capture_stderr { $stderr.puts "test" }
+  end
+
   def test_shell_profile
     ENV["SHELL"] = "/bin/sh"
     assert_equal "~/.bash_profile", Utils::Shell.shell_profile
diff --git a/Library/Homebrew/utils.rb b/Library/Homebrew/utils.rb
index fce03f888..7a7673d01 100644
--- a/Library/Homebrew/utils.rb
+++ b/Library/Homebrew/utils.rb
@@ -376,6 +376,15 @@ ensure
   trap("INT", std_trap)
 end
 
+def capture_stderr
+  old = $stderr
+  $stderr = StringIO.new
+  yield
+  $stderr.string
+ensure
+  $stderr = old
+end
+
 def nostdout
   if ARGV.verbose?
     yield