Merge pull request #1082 from alyssais/uninstall_dependancy_error

uninstall: refuse when dependents still installed

Library/Homebrew/cmd/missing.rb

@@ -1,6 +1,9 @@
-#:  * `missing` [<formulae>]:
+#:  * `missing` [`--hide=`<hidden>] [<formulae>]:
 #:    Check the given <formulae> for missing dependencies. If no <formulae> are
 #:    given, check all installed brews.
+#:
+#:    If `--hide=`<hidden> is passed, act as if none of <hidden> are installed.
+#:    <hidden> should be a comma-separated list of formulae.
 
 require "formula"
 require "tab"
@@ -18,8 +21,11 @@ module Homebrew
       ARGV.resolved_formulae
     end
 
-    Diagnostic.missing_deps(ff) do |name, missing|
+    ff.each do |f|
-      print "#{name}: " if ff.size > 1
+      missing = f.missing_dependencies(hide: ARGV.values("hide"))
+      next if missing.empty?
+
+      print "#{f}: " if ff.size > 1
       puts missing.join(" ")
     end
   end

Library/Homebrew/cmd/uninstall.rb

@@ -1,11 +1,15 @@
-#:  * `uninstall`, `rm`, `remove` [`--force`] <formula>:
+#:  * `uninstall`, `rm`, `remove` [`--force`] [`--ignore-dependencies`] <formula>:
 #:    Uninstall <formula>.
 #:
 #:    If `--force` is passed, and there are multiple versions of <formula>
 #:    installed, delete all installed versions.
+#:
+#:    If `--ignore-dependencies` is passed, uninstalling won't fail, even if
+#:    formulae depending on <formula> would still be installed.
 
 require "keg"
 require "formula"
+require "diagnostic"
 require "migrator"
 
 module Homebrew
@@ -14,38 +18,50 @@ module Homebrew
   def uninstall
     raise KegUnspecifiedError if ARGV.named.empty?
 
-    if !ARGV.force?
+    kegs_by_rack = if ARGV.force?
-      ARGV.kegs.each do |keg|
+      Hash[ARGV.named.map do |name|
-        keg.lock do
-          puts "Uninstalling #{keg}... (#{keg.abv})"
-          keg.unlink
-          keg.uninstall
-          rack = keg.rack
-          rm_pin rack
-
-          if rack.directory?
-            versions = rack.subdirs.map(&:basename)
-            verb = versions.length == 1 ? "is" : "are"
-            puts "#{keg.name} #{versions.join(", ")} #{verb} still installed."
-            puts "Remove all versions with `brew uninstall --force #{keg.name}`."
-          end
-        end
-      end
-    else
-      ARGV.named.each do |name|
         rack = Formulary.to_rack(name)
+        [rack, rack.subdirs.map { |d| Keg.new(d) }]
+      end]
+    else
+      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
+
+    kegs_by_rack.each do |rack, kegs|
+      if ARGV.force?
         name = rack.basename
 
         if rack.directory?
           puts "Uninstalling #{name}... (#{rack.abv})"
-          rack.subdirs.each do |d|
+          kegs.each do |keg|
-            keg = Keg.new(d)
             keg.unlink
             keg.uninstall
           end
         end
 
         rm_pin rack
+      else
+        kegs.each do |keg|
+          keg.lock do
+            puts "Uninstalling #{keg}... (#{keg.abv})"
+            keg.unlink
+            keg.uninstall
+            rack = keg.rack
+            rm_pin rack
+
+            if rack.directory?
+              versions = rack.subdirs.map(&:basename)
+              verb = versions.length == 1 ? "is" : "are"
+              puts "#{keg.name} #{versions.join(", ")} #{verb} still installed."
+              puts "Remove all versions with `brew uninstall --force #{keg.name}`."
+            end
+          end
+        end
       end
     end
   rescue MultipleVersionsInstalledError => e
@@ -61,6 +77,30 @@ 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
+  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(" ")}`."
+
+    true
+  end
+
   def rm_pin(rack)
     Formulary.from_rack(rack).unpin
   rescue

Library/Homebrew/diagnostic.rb

@@ -7,24 +7,14 @@ require "utils/shell"
 
 module Homebrew
   module Diagnostic
-    def self.missing_deps(ff)
+    def self.missing_deps(ff, hide = nil)
       missing = {}
       ff.each do |f|
-        missing_deps = f.recursive_dependencies do |dependent, dep|
+        missing_dependencies = f.missing_dependencies(hide: hide)
-          if dep.optional? || dep.recommended?
-            tab = Tab.for_formula(dependent)
-            Dependency.prune unless tab.with?(dep)
-          elsif dep.build?
-            Dependency.prune
-          end
-        end
 
-        missing_deps.map!(&:to_formula)
+        unless missing_dependencies.empty?
-        missing_deps.reject! { |d| d.installed_prefixes.any? }
+          yield f.full_name, missing_dependencies if block_given?
-
+          missing[f.full_name] = missing_dependencies
-        unless missing_deps.empty?
-          yield f.full_name, missing_deps if block_given?
-          missing[f.full_name] = missing_deps
         end
       end
       missing

Library/Homebrew/extend/ARGV.rb

@@ -121,6 +121,13 @@ module HomebrewArgvExtension
     flag_with_value.strip_prefix(arg_prefix) if flag_with_value
   end
 
+  # Returns an array of values that were given as a comma-seperated list.
+  # @see value
+  def values(name)
+    return unless val = value(name)
+    val.split(",")
+  end
+
   def force?
     flag? "--force"
   end

Library/Homebrew/formula.rb

@@ -1337,6 +1337,13 @@ class Formula
     end
   end
 
+  # Clear caches of .racks and .installed.
+  # @private
+  def self.clear_cache
+    @racks = nil
+    @installed = nil
+  end
+
   # An array of all racks currently installed.
   # @private
   def self.racks
@@ -1459,6 +1466,26 @@ class Formula
     recursive_dependencies.reject(&:build?)
   end
 
+  # Returns a list of formulae depended on by this formula that aren't
+  # installed
+  def missing_dependencies(hide: nil)
+    hide ||= []
+    missing_dependencies = recursive_dependencies do |dependent, dep|
+      if dep.optional? || dep.recommended?
+        tab = Tab.for_formula(dependent)
+        Dependency.prune unless tab.with?(dep)
+      elsif dep.build?
+        Dependency.prune
+      end
+    end
+
+    missing_dependencies.map!(&:to_formula)
+    missing_dependencies.select! do |d|
+      hide.include?(d.name) || d.installed_prefixes.empty?
+    end
+    missing_dependencies
+  end
+
   # @private
   def to_hash
     hsh = {

Library/Homebrew/keg.rb

@@ -87,6 +87,41 @@ class Keg
     mime-info pixmaps sounds postgresql
   ].freeze
 
+  # Will return some kegs, and some dependencies, if they're present.
+  # For efficiency, we don't bother trying to get complete data.
+  def self.find_some_installed_dependents(kegs)
+    # First, check in the tabs of installed Formulae.
+    kegs.each do |keg|
+      dependents = keg.installed_dependents - kegs
+      dependents.map! { |d| "#{d.name} #{d.version}" }
+      return [keg], dependents if dependents.any?
+    end
+
+    # Some kegs won't have modern Tabs with the dependencies listed.
+    # In this case, fall back to Formula#missing_dependencies.
+
+    # Find formulae that didn't have dependencies saved in all of their kegs,
+    # so need them to be calculated now.
+    #
+    # This happens after the initial dependency check because it's sloooow.
+    remaining_formulae = Formula.installed.select do |f|
+      f.installed_kegs.any? { |k| Tab.for_keg(k).runtime_dependencies.nil? }
+    end
+
+    keg_names = kegs.map(&:name)
+    kegs_by_name = kegs.group_by(&:to_formula)
+    remaining_formulae.each do |dependent|
+      required = dependent.missing_dependencies(hide: keg_names)
+      required.select! { |f| kegs_by_name.key?(f) }
+      next unless required.any?
+
+      required_kegs = required.map { |f| kegs_by_name[f].sort_by(&:version).last }
+      return required_kegs, [dependent.to_s]
+    end
+
+    nil
+  end
+
   # if path is a file in a keg then this will return the containing Keg object
   def self.for(path)
     path = path.realpath
@@ -292,6 +327,23 @@ class Keg
     PkgVersion.parse(path.basename.to_s)
   end
 
+  def to_formula
+    Formulary.from_keg(self)
+  end
+
+  def installed_dependents
+    Formula.installed.flat_map(&:installed_kegs).select do |keg|
+      tab = Tab.for_keg(keg)
+      next if tab.runtime_dependencies.nil? # no dependency information saved.
+      tab.runtime_dependencies.any? do |dep|
+        # Resolve formula rather than directly comparing names
+        # in case of conflicts between formulae from different taps.
+        dep_formula = Formulary.factory(dep["full_name"])
+        dep_formula == to_formula && dep["version"] == version.to_s
+      end
+    end
+  end
+
   def find(*args, &block)
     path.find(*args, &block)
   end

Library/Homebrew/test/helper/integration_command_test_case.rb

@@ -73,10 +73,12 @@ class IntegrationCommandTestCase < Homebrew::TestCase
     cmd_args << "-rintegration_mocks"
     cmd_args << (HOMEBREW_LIBRARY_PATH/"brew.rb").resolved_path.to_s
     cmd_args += args
+    developer = ENV["HOMEBREW_DEVELOPER"]
     Bundler.with_original_env do
       ENV["HOMEBREW_BREW_FILE"] = HOMEBREW_PREFIX/"bin/brew"
       ENV["HOMEBREW_INTEGRATION_TEST"] = cmd_id_from_args(args)
       ENV["HOMEBREW_TEST_TMPDIR"] = TEST_TMPDIR
+      ENV["HOMEBREW_DEVELOPER"] = developer
       env.each_pair do |k, v|
         ENV[k] = v
       end
@@ -127,7 +129,6 @@ class IntegrationCommandTestCase < Homebrew::TestCase
         sha256 "#{TESTBALL_SHA256}"
 
         option "with-foo", "Build with foo"
-        #{content}
 
         def install
           (prefix/"foo"/"test").write("test") if build.with? "foo"
@@ -138,6 +139,8 @@ class IntegrationCommandTestCase < Homebrew::TestCase
           system ENV.cc, "test.c", "-o", bin/"test"
         end
 
+        #{content}
+
         # something here
       EOS
     when "foo"

Library/Homebrew/test/test_ARGV.rb

@@ -62,4 +62,19 @@ class ArgvExtensionTests < Homebrew::TestCase
     assert !@argv.flag?("--frotz")
     assert !@argv.flag?("--debug")
   end
+
+  def test_value
+    @argv << "--foo=" << "--bar=ab"
+    assert_equal "", @argv.value("foo")
+    assert_equal "ab", @argv.value("bar")
+    assert_nil @argv.value("baz")
+  end
+
+  def test_values
+    @argv << "--foo=" << "--bar=a" << "--baz=b,c"
+    assert_equal [], @argv.values("foo")
+    assert_equal ["a"], @argv.values("bar")
+    assert_equal ["b", "c"], @argv.values("baz")
+    assert_nil @argv.values("qux")
+  end
 end

Library/Homebrew/test/test_keg.rb

@@ -5,15 +5,22 @@ require "stringio"
 class LinkTests < Homebrew::TestCase
   include FileUtils
 
-  def setup
+  def setup_test_keg(name, version)
-    keg = HOMEBREW_CELLAR.join("foo", "1.0")
+    path = HOMEBREW_CELLAR.join(name, version)
-    keg.join("bin").mkpath
+    path.join("bin").mkpath
 
     %w[hiworld helloworld goodbye_cruel_world].each do |file|
-      touch keg.join("bin", file)
+      touch path.join("bin", file)
     end
 
-    @keg = Keg.new(keg)
+    keg = Keg.new(path)
+    @kegs ||= []
+    @kegs << keg
+    keg
+  end
+
+  def setup
+    @keg = setup_test_keg("foo", "1.0")
     @dst = HOMEBREW_PREFIX.join("bin", "helloworld")
     @nonexistent = Pathname.new("/some/nonexistent/path")
 
@@ -27,8 +34,10 @@ class LinkTests < Homebrew::TestCase
   end
 
   def teardown
-    @keg.unlink
+    @kegs.each do |keg|
-    @keg.uninstall
+      keg.unlink
+      keg.uninstall
+    end
 
     $stdout = @old_stdout
 
@@ -305,3 +314,72 @@ class LinkTests < Homebrew::TestCase
     keg.uninstall
   end
 end
+
+class InstalledDependantsTests < LinkTests
+  def stub_formula_name(name)
+    stub_formula_loader formula(name) { url "foo-1.0" }
+  end
+
+  def setup_test_keg(name, version)
+    stub_formula_name(name)
+    keg = super
+    Formula.clear_cache
+    keg
+  end
+
+  def setup
+    super
+    @dependent = setup_test_keg("bar", "1.0")
+  end
+
+  def alter_tab(keg = @dependent)
+    tab = Tab.for_keg(keg)
+    yield tab
+    tab.write
+  end
+
+  def dependencies(deps)
+    alter_tab do |tab|
+      tab.tabfile = @dependent.join("INSTALL_RECEIPT.json")
+      tab.runtime_dependencies = deps
+    end
+  end
+
+  def test_no_dependencies_anywhere
+    dependencies nil
+    assert_empty @keg.installed_dependents
+    assert_nil Keg.find_some_installed_dependents([@keg])
+  end
+
+  def test_missing_formula_dependency
+    dependencies nil
+    Formula["bar"].class.depends_on "foo"
+    assert_empty @keg.installed_dependents
+    assert_equal [[@keg], ["bar"]], Keg.find_some_installed_dependents([@keg])
+  end
+
+  def test_empty_dependencies_in_tab
+    dependencies []
+    assert_empty @keg.installed_dependents
+    assert_nil Keg.find_some_installed_dependents([@keg])
+  end
+
+  def test_same_name_different_version_in_tab
+    dependencies [{ "full_name" => "foo", "version" => "1.1" }]
+    assert_empty @keg.installed_dependents
+    assert_nil Keg.find_some_installed_dependents([@keg])
+  end
+
+  def test_different_name_same_version_in_tab
+    stub_formula_name("baz")
+    dependencies [{ "full_name" => "baz", "version" => @keg.version.to_s }]
+    assert_empty @keg.installed_dependents
+    assert_nil Keg.find_some_installed_dependents([@keg])
+  end
+
+  def test_same_name_and_version_in_tab
+    dependencies [{ "full_name" => "foo", "version" => "1.0" }]
+    assert_equal [@dependent], @keg.installed_dependents
+    assert_equal [[@keg], ["bar 1.0"]], Keg.find_some_installed_dependents([@keg])
+  end
+end

Library/Homebrew/test/test_missing.rb

@@ -1,11 +1,34 @@
 require "helper/integration_command_test_case"
 
 class IntegrationCommandTestMissing < IntegrationCommandTestCase
-  def test_missing
+  def setup
+    super
+
     setup_test_formula "foo"
     setup_test_formula "bar"
+  end
+
+  def make_prefix(name)
+    (HOMEBREW_CELLAR/name/"1.0").mkpath
+  end
+
+  def test_missing_missing
+    make_prefix "bar"
 
-    (HOMEBREW_CELLAR/"bar/1.0").mkpath
     assert_match "foo", cmd("missing")
   end
+
+  def test_missing_not_missing
+    make_prefix "foo"
+    make_prefix "bar"
+
+    assert_empty cmd("missing")
+  end
+
+  def test_missing_hide
+    make_prefix "foo"
+    make_prefix "bar"
+
+    assert_match "foo", cmd("missing", "--hide=foo")
+  end
 end

Library/Homebrew/test/test_uninstall.rb

@@ -1,4 +1,26 @@
 require "helper/integration_command_test_case"
+require "cmd/uninstall"
+
+class UninstallTests < Homebrew::TestCase
+  def test_check_for_testball_f2s_when_developer
+    refute_predicate Homebrew, :should_check_for_dependents?
+  end
+
+  def test_check_for_dependents_when_not_developer
+    run_as_not_developer do
+      assert_predicate Homebrew, :should_check_for_dependents?
+    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?
+    end
+  ensure
+    ARGV.delete("--ignore-dependencies")
+  end
+end
 
 class IntegrationCommandTestUninstall < IntegrationCommandTestCase
   def test_uninstall

docs/brew.1.html

@@ -245,8 +245,11 @@ packages.</p>
 
 <p>If <code>--force</code> is passed, then treat installed <var>formulae</var> and passed <var>formulae</var>
 like if they are from same taps and migrate them anyway.</p></dd>
-<dt><code>missing</code> [<var>formulae</var>]</dt><dd><p>Check the given <var>formulae</var> for missing dependencies. If no <var>formulae</var> are
+<dt><code>missing</code> [<code>--hide=</code><var>hidden</var>] [<var>formulae</var>]</dt><dd><p>Check the given <var>formulae</var> for missing dependencies. If no <var>formulae</var> are
-given, check all installed brews.</p></dd>
+given, check all installed brews.</p>
+
+<p>If <code>--hide=</code><var>hidden</var> is passed, act as if none of <var>hidden</var> are installed.
+<var>hidden</var> should be a comma-separated list of formulae.</p></dd>
 <dt><code>options</code> [<code>--compact</code>] (<code>--all</code>|<code>--installed</code>|<var>formulae</var>)</dt><dd><p>Display install options specific to <var>formulae</var>.</p>
 
 <p>If <code>--compact</code> is passed, show all options on a single line separated by
@@ -348,10 +351,13 @@ for <var>version</var> is <code>v1</code>.</p>
 <dt><code>tap-pin</code> <var>tap</var></dt><dd><p>Pin <var>tap</var>, prioritizing its formulae over core when formula names are supplied
 by the user. See also <code>tap-unpin</code>.</p></dd>
 <dt><code>tap-unpin</code> <var>tap</var></dt><dd><p>Unpin <var>tap</var> so its formulae are no longer prioritized. See also <code>tap-pin</code>.</p></dd>
-<dt><code>uninstall</code>, <code>rm</code>, <code>remove</code> [<code>--force</code>] <var>formula</var></dt><dd><p>Uninstall <var>formula</var>.</p>
+<dt><code>uninstall</code>, <code>rm</code>, <code>remove</code> [<code>--force</code>] [<code>--ignore-dependencies</code>] <var>formula</var></dt><dd><p>Uninstall <var>formula</var>.</p>
 
 <p>If <code>--force</code> is passed, and there are multiple versions of <var>formula</var>
-installed, delete all installed versions.</p></dd>
+installed, delete all installed versions.</p>
+
+<p>If <code>--ignore-dependencies</code> is passed, uninstalling won't fail, even if
+formulae depending on <var>formula</var> would still be installed.</p></dd>
 <dt><code>unlink</code> [<code>--dry-run</code>] <var>formula</var></dt><dd><p>Remove symlinks for <var>formula</var> from the Homebrew prefix. This can be useful
 for temporarily disabling a formula:
 <code>brew unlink foo &amp;&amp; commands &amp;&amp; brew link foo</code>.</p>

manpages/brew.1

@@ -329,9 +329,12 @@ Migrate renamed packages to new name, where \fIformulae\fR are old names of pack
 If \fB\-\-force\fR is passed, then treat installed \fIformulae\fR and passed \fIformulae\fR like if they are from same taps and migrate them anyway\.
 .
 .TP
-\fBmissing\fR [\fIformulae\fR]
+\fBmissing\fR [\fB\-\-hide=\fR\fIhidden\fR] [\fIformulae\fR]
 Check the given \fIformulae\fR for missing dependencies\. If no \fIformulae\fR are given, check all installed brews\.
 .
+.IP
+If \fB\-\-hide=\fR\fIhidden\fR is passed, act as if none of \fIhidden\fR are installed\. \fIhidden\fR should be a comma\-separated list of formulae\.
+.
 .TP
 \fBoptions\fR [\fB\-\-compact\fR] (\fB\-\-all\fR|\fB\-\-installed\fR|\fIformulae\fR)
 Display install options specific to \fIformulae\fR\.
@@ -484,12 +487,15 @@ Pin \fItap\fR, prioritizing its formulae over core when formula names are suppli
 Unpin \fItap\fR so its formulae are no longer prioritized\. See also \fBtap\-pin\fR\.
 .
 .TP
-\fBuninstall\fR, \fBrm\fR, \fBremove\fR [\fB\-\-force\fR] \fIformula\fR
+\fBuninstall\fR, \fBrm\fR, \fBremove\fR [\fB\-\-force\fR] [\fB\-\-ignore\-dependencies\fR] \fIformula\fR
 Uninstall \fIformula\fR\.
 .
 .IP
 If \fB\-\-force\fR is passed, and there are multiple versions of \fIformula\fR installed, delete all installed versions\.
 .
+.IP
+If \fB\-\-ignore\-dependencies\fR is passed, uninstalling won\'t fail, even if formulae depending on \fIformula\fR would still be installed\.
+.
 .TP
 \fBunlink\fR [\fB\-\-dry\-run\fR] \fIformula\fR
 Remove symlinks for \fIformula\fR from the Homebrew prefix\. This can be useful for temporarily disabling a formula: \fBbrew unlink foo && commands && brew link foo\fR\.