From 2c133a3d45366a503c99a02e7e255e4387e8bff7 Mon Sep 17 00:00:00 2001
From: Mike McQuaid <mike@mikemcquaid.com>
Date: Thu, 2 Jul 2020 12:53:52 +0100
Subject: [PATCH] Check installed dependents on install and reinstall

It's not sufficient to do this merely on `brew upgrade` because
`brew install` and `brew reinstall` can also result in formulae being
upgraded.

This requires moving logic from `cmd/upgrade.rb` to `upgrade.rb`. To
save you searching the diff the changes that resulted from doing that:

- Query the installed formulae from class state in `FormulaInstaller`
  rather than the (incomplete) list that we passed into it.
- Don't output the "Checking dependents" message. It was there for
  systems and configurations where this is slow but for most users
  and most installations this will be a (annoying, noisy) no-op.

Fixes https://github.com/Homebrew/brew/issues/7860
---
 Library/Homebrew/cmd/install.rb       |   4 +
 Library/Homebrew/cmd/reinstall.rb     |   4 +
 Library/Homebrew/cmd/upgrade.rb       | 244 +-------------------------
 Library/Homebrew/formula_installer.rb |  10 ++
 Library/Homebrew/test/spec_helper.rb  |   1 +
 Library/Homebrew/upgrade.rb           | 242 +++++++++++++++++++++++++
 6 files changed, 265 insertions(+), 240 deletions(-)
 create mode 100644 Library/Homebrew/upgrade.rb

diff --git a/Library/Homebrew/cmd/install.rb b/Library/Homebrew/cmd/install.rb
index d6ad01feef..a608b72ea9 100644
--- a/Library/Homebrew/cmd/install.rb
+++ b/Library/Homebrew/cmd/install.rb
@@ -7,6 +7,7 @@ require "install"
 require "search"
 require "cleanup"
 require "cli/parser"
+require "upgrade"
 
 module Homebrew
   module_function
@@ -261,6 +262,9 @@ module Homebrew
       install_formula(f)
       Cleanup.install_formula_clean!(f)
     end
+
+    check_installed_dependents
+
     Homebrew.messages.display_messages
   rescue FormulaUnreadableError, FormulaClassUnavailableError,
          TapFormulaUnreadableError, TapFormulaClassUnavailableError => e
diff --git a/Library/Homebrew/cmd/reinstall.rb b/Library/Homebrew/cmd/reinstall.rb
index c6abc8fa36..bc85d52f13 100644
--- a/Library/Homebrew/cmd/reinstall.rb
+++ b/Library/Homebrew/cmd/reinstall.rb
@@ -6,6 +6,7 @@ require "messages"
 require "reinstall"
 require "cli/parser"
 require "cleanup"
+require "upgrade"
 
 module Homebrew
   module_function
@@ -65,6 +66,9 @@ module Homebrew
       reinstall_formula(f)
       Cleanup.install_formula_clean!(f)
     end
+
+    check_installed_dependents
+
     Homebrew.messages.display_messages
   end
 end
diff --git a/Library/Homebrew/cmd/upgrade.rb b/Library/Homebrew/cmd/upgrade.rb
index 11efd5be00..66a8049165 100644
--- a/Library/Homebrew/cmd/upgrade.rb
+++ b/Library/Homebrew/cmd/upgrade.rb
@@ -1,12 +1,9 @@
 # frozen_string_literal: true
 
-require "install"
-require "reinstall"
-require "formula_installer"
-require "development_tools"
-require "messages"
-require "cleanup"
 require "cli/parser"
+require "formula_installer"
+require "install"
+require "upgrade"
 
 module Homebrew
   module_function
@@ -114,241 +111,8 @@ module Homebrew
 
     upgrade_formulae(formulae_to_install)
 
-    check_dependents(formulae_to_install)
+    check_installed_dependents
 
     Homebrew.messages.display_messages
   end
-
-  def upgrade_formulae(formulae_to_install)
-    return if formulae_to_install.empty?
-    return if args.dry_run?
-
-    # Sort keg-only before non-keg-only formulae to avoid any needless conflicts
-    # with outdated, non-keg-only versions of formulae being upgraded.
-    formulae_to_install.sort! do |a, b|
-      if !a.keg_only? && b.keg_only?
-        1
-      elsif a.keg_only? && !b.keg_only?
-        -1
-      else
-        0
-      end
-    end
-
-    formulae_to_install.each do |f|
-      Migrator.migrate_if_needed(f)
-      begin
-        upgrade_formula(f)
-        Cleanup.install_formula_clean!(f)
-      rescue UnsatisfiedRequirements => e
-        Homebrew.failed = true
-        onoe "#{f}: #{e}"
-      end
-    end
-  end
-
-  def upgrade_formula(f)
-    return if args.dry_run?
-
-    if f.opt_prefix.directory?
-      keg = Keg.new(f.opt_prefix.resolved_path)
-      keg_had_linked_opt = true
-      keg_was_linked = keg.linked?
-    end
-
-    formulae_maybe_with_kegs = [f] + f.old_installed_formulae
-    outdated_kegs = formulae_maybe_with_kegs
-                    .map(&:linked_keg)
-                    .select(&:directory?)
-                    .map { |k| Keg.new(k.resolved_path) }
-    linked_kegs = outdated_kegs.select(&:linked?)
-
-    if f.opt_prefix.directory?
-      keg = Keg.new(f.opt_prefix.resolved_path)
-      tab = Tab.for_keg(keg)
-    end
-
-    build_options = BuildOptions.new(Options.create(args.flags_only), f.options)
-    options = build_options.used_options
-    options |= f.build.used_options
-    options &= f.options
-
-    fi = FormulaInstaller.new(f)
-    fi.options = options
-    fi.build_bottle = args.build_bottle?
-    fi.installed_on_request = args.named.present?
-    fi.link_keg           ||= keg_was_linked if keg_had_linked_opt
-    if tab
-      fi.build_bottle          ||= tab.built_bottle?
-      fi.installed_as_dependency = tab.installed_as_dependency
-      fi.installed_on_request  ||= tab.installed_on_request
-    end
-
-    upgrade_version = if f.optlinked?
-      "#{Keg.new(f.opt_prefix).version} -> #{f.pkg_version}"
-    else
-      "-> #{f.pkg_version}"
-    end
-    oh1 "Upgrading #{Formatter.identifier(f.full_specified_name)} #{upgrade_version} #{fi.options.to_a.join(" ")}"
-
-    fi.prelude
-    fi.fetch
-
-    # first we unlink the currently active keg for this formula otherwise it is
-    # possible for the existing build to interfere with the build we are about to
-    # do! Seriously, it happens!
-    outdated_kegs.each(&:unlink)
-
-    fi.install
-    fi.finish
-  rescue FormulaInstallationAlreadyAttemptedError
-    # We already attempted to upgrade f as part of the dependency tree of
-    # another formula. In that case, don't generate an error, just move on.
-    nil
-  rescue CannotInstallFormulaError => e
-    ofail e
-  rescue BuildError => e
-    e.dump
-    puts
-    Homebrew.failed = true
-  rescue DownloadError => e
-    ofail e
-  ensure
-    # restore previous installation state if build failed
-    begin
-      linked_kegs.each(&:link) unless f.latest_version_installed?
-    rescue
-      nil
-    end
-  end
-
-  # @private
-  def depends_on(a, b)
-    if a.opt_or_installed_prefix_keg
-       &.runtime_dependencies
-       &.any? { |d| d["full_name"] == b.full_name }
-      1
-    else
-      a <=> b
-    end
-  end
-
-  def check_dependents(formulae_to_install)
-    return if formulae_to_install.empty?
-
-    oh1 "Checking for dependents of upgraded formulae..." unless args.dry_run?
-    outdated_dependents =
-      formulae_to_install.flat_map(&:runtime_installed_formula_dependents)
-                         .select(&:outdated?)
-    if outdated_dependents.blank?
-      ohai "No dependents found!" unless args.dry_run?
-      return
-    end
-    outdated_dependents -= formulae_to_install if args.dry_run?
-
-    upgradeable_dependents =
-      outdated_dependents.reject(&:pinned?)
-                         .sort { |a, b| depends_on(a, b) }
-    pinned_dependents =
-      outdated_dependents.select(&:pinned?)
-                         .sort { |a, b| depends_on(a, b) }
-
-    if pinned_dependents.present?
-      plural = "dependent".pluralize(pinned_dependents.count)
-      ohai "Not upgrading #{pinned_dependents.count} pinned #{plural}:"
-      puts(pinned_dependents.map do |f|
-        "#{f.full_specified_name} #{f.pkg_version}"
-      end.join(", "))
-    end
-
-    # Print the upgradable dependents.
-    if upgradeable_dependents.blank?
-      ohai "No outdated dependents to upgrade!" unless args.dry_run?
-    else
-      plural = "dependent".pluralize(upgradeable_dependents.count)
-      verb = args.dry_run? ? "Would upgrade" : "Upgrading"
-      ohai "#{verb} #{upgradeable_dependents.count} #{plural}:"
-      formulae_upgrades = upgradeable_dependents.map do |f|
-        name = f.full_specified_name
-        if f.optlinked?
-          "#{name} #{Keg.new(f.opt_prefix).version} -> #{f.pkg_version}"
-        else
-          "#{name} #{f.pkg_version}"
-        end
-      end
-      puts formulae_upgrades.join(", ")
-    end
-
-    upgrade_formulae(upgradeable_dependents)
-
-    # Assess the dependents tree again now we've upgraded.
-    oh1 "Checking for dependents of upgraded formulae..." unless args.dry_run?
-    broken_dependents = CacheStoreDatabase.use(:linkage) do |db|
-      formulae_to_install.flat_map(&:runtime_installed_formula_dependents)
-                         .select do |f|
-        keg = f.opt_or_installed_prefix_keg
-        next unless keg
-
-        LinkageChecker.new(keg, cache_db: db)
-                      .broken_library_linkage?
-      end.compact
-    end
-    if broken_dependents.blank?
-      if args.dry_run?
-        ohai "No currently broken dependents found!"
-        opoo "If they are broken by the upgrade they will also be upgraded or reinstalled."
-      else
-        ohai "No broken dependents found!"
-      end
-      return
-    end
-
-    reinstallable_broken_dependents =
-      broken_dependents.reject(&:outdated?)
-                       .reject(&:pinned?)
-                       .sort { |a, b| depends_on(a, b) }
-    outdated_pinned_broken_dependents =
-      broken_dependents.select(&:outdated?)
-                       .select(&:pinned?)
-                       .sort { |a, b| depends_on(a, b) }
-
-    # Print the pinned dependents.
-    if outdated_pinned_broken_dependents.present?
-      count = outdated_pinned_broken_dependents.count
-      plural = "dependent".pluralize(outdated_pinned_broken_dependents.count)
-      onoe "Not reinstalling #{count} broken and outdated, but pinned #{plural}:"
-      $stderr.puts(outdated_pinned_broken_dependents.map do |f|
-        "#{f.full_specified_name} #{f.pkg_version}"
-      end.join(", "))
-    end
-
-    # Print the broken dependents.
-    if reinstallable_broken_dependents.blank?
-      ohai "No broken dependents to reinstall!"
-    else
-      count = reinstallable_broken_dependents.count
-      plural = "dependent".pluralize(reinstallable_broken_dependents.count)
-      ohai "Reinstalling #{count} broken #{plural} from source:"
-      puts reinstallable_broken_dependents.map(&:full_specified_name)
-                                          .join(", ")
-    end
-
-    return if args.dry_run?
-
-    reinstallable_broken_dependents.each do |f|
-      reinstall_formula(f, build_from_source: true)
-    rescue FormulaInstallationAlreadyAttemptedError
-      # We already attempted to reinstall f as part of the dependency tree of
-      # another formula. In that case, don't generate an error, just move on.
-      nil
-    rescue CannotInstallFormulaError => e
-      ofail e
-    rescue BuildError => e
-      e.dump
-      puts
-      Homebrew.failed = true
-    rescue DownloadError => e
-      ofail e
-    end
-  end
 end
diff --git a/Library/Homebrew/formula_installer.rb b/Library/Homebrew/formula_installer.rb
index 41298db9e3..30876d2fa9 100644
--- a/Library/Homebrew/formula_installer.rb
+++ b/Library/Homebrew/formula_installer.rb
@@ -77,6 +77,14 @@ class FormulaInstaller
     @attempted = Set.new
   end
 
+  def self.installed
+    @installed ||= Set.new
+  end
+
+  def self.clear_installed
+    @installed = Set.new
+  end
+
   # When no build tools are available and build flags are passed through ARGV,
   # it's necessary to interrupt the user before any sort of installation
   # can proceed. Only invoked when the user has no developer tools.
@@ -700,6 +708,8 @@ class FormulaInstaller
 
     ohai "Summary" if verbose? || show_summary_heading?
     puts summary
+
+    self.class.installed << formula
   ensure
     unlock
   end
diff --git a/Library/Homebrew/test/spec_helper.rb b/Library/Homebrew/test/spec_helper.rb
index 355c223257..337569b431 100644
--- a/Library/Homebrew/test/spec_helper.rb
+++ b/Library/Homebrew/test/spec_helper.rb
@@ -163,6 +163,7 @@ RSpec.configure do |config|
       Keg.clear_cache
       Tab.clear_cache
       FormulaInstaller.clear_attempted
+      FormulaInstaller.clear_installed
 
       TEST_DIRECTORIES.each(&:mkpath)
 
diff --git a/Library/Homebrew/upgrade.rb b/Library/Homebrew/upgrade.rb
new file mode 100644
index 0000000000..a875bfef83
--- /dev/null
+++ b/Library/Homebrew/upgrade.rb
@@ -0,0 +1,242 @@
+# frozen_string_literal: true
+
+require "reinstall"
+require "formula_installer"
+require "development_tools"
+require "messages"
+require "cleanup"
+
+module Homebrew
+  module_function
+
+  def upgrade_formulae(formulae_to_install)
+    return if formulae_to_install.empty?
+    return if args.dry_run?
+
+    # Sort keg-only before non-keg-only formulae to avoid any needless conflicts
+    # with outdated, non-keg-only versions of formulae being upgraded.
+    formulae_to_install.sort! do |a, b|
+      if !a.keg_only? && b.keg_only?
+        1
+      elsif a.keg_only? && !b.keg_only?
+        -1
+      else
+        0
+      end
+    end
+
+    formulae_to_install.each do |f|
+      Migrator.migrate_if_needed(f)
+      begin
+        upgrade_formula(f)
+        Cleanup.install_formula_clean!(f)
+      rescue UnsatisfiedRequirements => e
+        Homebrew.failed = true
+        onoe "#{f}: #{e}"
+      end
+    end
+  end
+
+  def upgrade_formula(f)
+    return if args.dry_run?
+
+    if f.opt_prefix.directory?
+      keg = Keg.new(f.opt_prefix.resolved_path)
+      keg_had_linked_opt = true
+      keg_was_linked = keg.linked?
+    end
+
+    formulae_maybe_with_kegs = [f] + f.old_installed_formulae
+    outdated_kegs = formulae_maybe_with_kegs
+                    .map(&:linked_keg)
+                    .select(&:directory?)
+                    .map { |k| Keg.new(k.resolved_path) }
+    linked_kegs = outdated_kegs.select(&:linked?)
+
+    if f.opt_prefix.directory?
+      keg = Keg.new(f.opt_prefix.resolved_path)
+      tab = Tab.for_keg(keg)
+    end
+
+    build_options = BuildOptions.new(Options.create(args.flags_only), f.options)
+    options = build_options.used_options
+    options |= f.build.used_options
+    options &= f.options
+
+    fi = FormulaInstaller.new(f)
+    fi.options = options
+    fi.build_bottle = args.build_bottle?
+    fi.installed_on_request = args.named.present?
+    fi.link_keg           ||= keg_was_linked if keg_had_linked_opt
+    if tab
+      fi.build_bottle          ||= tab.built_bottle?
+      fi.installed_as_dependency = tab.installed_as_dependency
+      fi.installed_on_request  ||= tab.installed_on_request
+    end
+
+    upgrade_version = if f.optlinked?
+      "#{Keg.new(f.opt_prefix).version} -> #{f.pkg_version}"
+    else
+      "-> #{f.pkg_version}"
+    end
+    oh1 "Upgrading #{Formatter.identifier(f.full_specified_name)} #{upgrade_version} #{fi.options.to_a.join(" ")}"
+
+    fi.prelude
+    fi.fetch
+
+    # first we unlink the currently active keg for this formula otherwise it is
+    # possible for the existing build to interfere with the build we are about to
+    # do! Seriously, it happens!
+    outdated_kegs.each(&:unlink)
+
+    fi.install
+    fi.finish
+  rescue FormulaInstallationAlreadyAttemptedError
+    # We already attempted to upgrade f as part of the dependency tree of
+    # another formula. In that case, don't generate an error, just move on.
+    nil
+  rescue CannotInstallFormulaError => e
+    ofail e
+  rescue BuildError => e
+    e.dump
+    puts
+    Homebrew.failed = true
+  rescue DownloadError => e
+    ofail e
+  ensure
+    # restore previous installation state if build failed
+    begin
+      linked_kegs.each(&:link) unless f.latest_version_installed?
+    rescue
+      nil
+    end
+  end
+
+  def check_installed_dependents
+    installed_formulae = FormulaInstaller.installed.to_a
+    return if installed_formulae.empty?
+
+    outdated_dependents =
+      installed_formulae.flat_map(&:runtime_installed_formula_dependents)
+                        .select(&:outdated?)
+    return if outdated_dependents.blank?
+
+    outdated_dependents -= installed_formulae if args.dry_run?
+
+    upgradeable_dependents =
+      outdated_dependents.reject(&:pinned?)
+                         .sort { |a, b| depends_on(a, b) }
+    pinned_dependents =
+      outdated_dependents.select(&:pinned?)
+                         .sort { |a, b| depends_on(a, b) }
+
+    if pinned_dependents.present?
+      plural = "dependent".pluralize(pinned_dependents.count)
+      ohai "Not upgrading #{pinned_dependents.count} pinned #{plural}:"
+      puts(pinned_dependents.map do |f|
+        "#{f.full_specified_name} #{f.pkg_version}"
+      end.join(", "))
+    end
+
+    # Print the upgradable dependents.
+    if upgradeable_dependents.blank?
+      ohai "No outdated dependents to upgrade!" unless args.dry_run?
+    else
+      plural = "dependent".pluralize(upgradeable_dependents.count)
+      verb = args.dry_run? ? "Would upgrade" : "Upgrading"
+      ohai "#{verb} #{upgradeable_dependents.count} #{plural}:"
+      formulae_upgrades = upgradeable_dependents.map do |f|
+        name = f.full_specified_name
+        if f.optlinked?
+          "#{name} #{Keg.new(f.opt_prefix).version} -> #{f.pkg_version}"
+        else
+          "#{name} #{f.pkg_version}"
+        end
+      end
+      puts formulae_upgrades.join(", ")
+    end
+
+    upgrade_formulae(upgradeable_dependents)
+
+    # Assess the dependents tree again now we've upgraded.
+    oh1 "Checking for dependents of upgraded formulae..." unless args.dry_run?
+    broken_dependents = CacheStoreDatabase.use(:linkage) do |db|
+      formulae_to_install.flat_map(&:runtime_installed_formula_dependents)
+                         .select do |f|
+        keg = f.opt_or_installed_prefix_keg
+        next unless keg
+
+        LinkageChecker.new(keg, cache_db: db)
+                      .broken_library_linkage?
+      end.compact
+    end
+    if broken_dependents.blank?
+      if args.dry_run?
+        ohai "No currently broken dependents found!"
+        opoo "If they are broken by the upgrade they will also be upgraded or reinstalled."
+      else
+        ohai "No broken dependents found!"
+      end
+      return
+    end
+
+    reinstallable_broken_dependents =
+      broken_dependents.reject(&:outdated?)
+                       .reject(&:pinned?)
+                       .sort { |a, b| depends_on(a, b) }
+    outdated_pinned_broken_dependents =
+      broken_dependents.select(&:outdated?)
+                       .select(&:pinned?)
+                       .sort { |a, b| depends_on(a, b) }
+
+    # Print the pinned dependents.
+    if outdated_pinned_broken_dependents.present?
+      count = outdated_pinned_broken_dependents.count
+      plural = "dependent".pluralize(outdated_pinned_broken_dependents.count)
+      onoe "Not reinstalling #{count} broken and outdated, but pinned #{plural}:"
+      $stderr.puts(outdated_pinned_broken_dependents.map do |f|
+        "#{f.full_specified_name} #{f.pkg_version}"
+      end.join(", "))
+    end
+
+    # Print the broken dependents.
+    if reinstallable_broken_dependents.blank?
+      ohai "No broken dependents to reinstall!"
+    else
+      count = reinstallable_broken_dependents.count
+      plural = "dependent".pluralize(reinstallable_broken_dependents.count)
+      ohai "Reinstalling #{count} broken #{plural} from source:"
+      puts reinstallable_broken_dependents.map(&:full_specified_name)
+                                          .join(", ")
+    end
+
+    return if args.dry_run?
+
+    reinstallable_broken_dependents.each do |f|
+      reinstall_formula(f, build_from_source: true)
+    rescue FormulaInstallationAlreadyAttemptedError
+      # We already attempted to reinstall f as part of the dependency tree of
+      # another formula. In that case, don't generate an error, just move on.
+      nil
+    rescue CannotInstallFormulaError => e
+      ofail e
+    rescue BuildError => e
+      e.dump
+      puts
+      Homebrew.failed = true
+    rescue DownloadError => e
+      ofail e
+    end
+  end
+
+  # @private
+  def depends_on(a, b)
+    if a.opt_or_installed_prefix_keg
+       &.runtime_dependencies
+       &.any? { |d| d["full_name"] == b.full_name }
+      1
+    else
+      a <=> b
+    end
+  end
+end