Merge branch 'refactor-install'

# Conflicts:
#	Library/Homebrew/test/cmd/upgrade_spec.rb

Library/Homebrew/cmd/install.rb

@@ -310,9 +310,7 @@ module Homebrew
         Install.perform_preinstall_checks_once
         Install.check_cc_argv(args.cc)
 
-        Install.ask_formulae(installed_formulae, args: args) if args.ask?
+        formulae_installer = Install.get_formulae_dependencies(
-
-        Install.install_formulae(
           installed_formulae,
           installed_on_request:       !args.as_dependency?,
           installed_as_dependency:    args.as_dependency?,
@@ -338,9 +336,11 @@ module Homebrew
           skip_link:                  args.skip_link?,
         )
 
-        Upgrade.check_installed_dependents(
+        if args.ask?
+          dependants = Upgrade.get_dependants(
             installed_formulae,
             flags:                      args.flags_only,
+            ask:                        args.ask?,
             installed_on_request:       !args.as_dependency?,
             force_bottle:               args.force_bottle?,
             build_from_source_formulae: args.build_from_source_formulae,
@@ -354,6 +354,50 @@ module Homebrew
             dry_run:                    args.dry_run?,
           )
 
+          formulae_dependencies = Install.get_hierarchy(formulae_installer, dependants)
+          # Main block: if asking the user is enabled, show dependency and size information.
+          Install.ask_formulae(formulae_dependencies, args: args)
+        end
+
+        Install.install_formulae(formulae_installer,
+                                 dry_run: args.dry_run?,
+                                 verbose: args.verbose?)
+
+        unless args.ask?
+          dependants = Upgrade.get_dependants(
+            installed_formulae,
+            flags:                      args.flags_only,
+            dry_run:                    args.dry_run?,
+            installed_on_request:       !args.as_dependency?,
+            force_bottle:               args.force_bottle?,
+            build_from_source_formulae: args.build_from_source_formulae,
+            interactive:                args.interactive?,
+            keep_tmp:                   args.keep_tmp?,
+            debug_symbols:              args.debug_symbols?,
+            force:                      args.force?,
+            debug:                      args.debug?,
+            quiet:                      args.quiet?,
+            verbose:                    args.verbose?,
+          )
+        end
+
+        if dependants
+          Upgrade.upgrade_dependents(
+            dependants, installed_formulae,
+            flags:                      args.flags_only,
+            dry_run:                    args.dry_run?,
+            force_bottle:               args.force_bottle?,
+            build_from_source_formulae: args.build_from_source_formulae,
+            interactive:                args.interactive?,
+            keep_tmp:                   args.keep_tmp?,
+            debug_symbols:              args.debug_symbols?,
+            force:                      args.force?,
+            debug:                      args.debug?,
+            quiet:                      args.quiet?,
+            verbose:                    args.verbose?
+          )
+        end
+
         Cleanup.periodic_clean!(dry_run: args.dry_run?)
 
         Homebrew.messages.display_messages(display_times: args.display_times?)

Library/Homebrew/cmd/reinstall.rb

@@ -130,16 +130,13 @@ module Homebrew
         unless formulae.empty?
           Install.perform_preinstall_checks_once
 
-          # If asking the user is enabled, show dependency and size information.
+          formulae_keg = formulae.map do |formula|
-          Install.ask_formulae(formulae, args: args) if args.ask?
-
-          formulae.each do |formula|
             if formula.pinned?
               onoe "#{formula.full_name} is pinned. You must unpin it to reinstall."
               next
             end
             Migrator.migrate_if_needed(formula, force: args.force?)
-            Homebrew::Reinstall.reinstall_formula(
+            Homebrew::Reinstall.get_formula_to_reinstall(
               formula,
               flags:                      args.flags_only,
               force_bottle:               args.force_bottle?,
@@ -153,10 +150,52 @@ module Homebrew
               verbose:                    args.verbose?,
               git:                        args.git?,
             )
-            Cleanup.install_formula_clean!(formula)
           end
 
-          Upgrade.check_installed_dependents(
+          if args.ask?
+            dependants = Upgrade.get_dependants(
+              formulae,
+              flags:                      args.flags_only,
+              ask:                        args.ask?,
+              force_bottle:               args.force_bottle?,
+              build_from_source_formulae: args.build_from_source_formulae,
+              interactive:                args.interactive?,
+              keep_tmp:                   args.keep_tmp?,
+              debug_symbols:              args.debug_symbols?,
+              force:                      args.force?,
+              debug:                      args.debug?,
+              quiet:                      args.quiet?,
+              verbose:                    args.verbose?,
+            )
+
+            formulae_dependencies = formulae_keg.map(&:formula_installer)
+
+            formulae_dependencies = Install.get_hierarchy(formulae_dependencies, dependants)
+            # Main block: if asking the user is enabled, show dependency and size information.
+            Install.ask_formulae(formulae_dependencies, args: args)
+
+          end
+
+          formulae_keg.each do |f|
+            Homebrew::Reinstall.reinstall_formula(
+              f,
+              flags:                      args.flags_only,
+              force_bottle:               args.force_bottle?,
+              build_from_source_formulae: args.build_from_source_formulae,
+              interactive:                args.interactive?,
+              keep_tmp:                   args.keep_tmp?,
+              debug_symbols:              args.debug_symbols?,
+              force:                      args.force?,
+              debug:                      args.debug?,
+              quiet:                      args.quiet?,
+              verbose:                    args.verbose?,
+              git:                        args.git?,
+            )
+            Cleanup.install_formula_clean!(f.formula)
+          end
+
+          unless args.ask?
+            dependants = Upgrade.get_dependants(
               formulae,
               flags:                      args.flags_only,
               force_bottle:               args.force_bottle?,
@@ -171,6 +210,23 @@ module Homebrew
             )
           end
 
+          if dependants
+            Upgrade.upgrade_dependents(
+              dependants, formulae,
+              flags:                      args.flags_only,
+              force_bottle:               args.force_bottle?,
+              build_from_source_formulae: args.build_from_source_formulae,
+              interactive:                args.interactive?,
+              keep_tmp:                   args.keep_tmp?,
+              debug_symbols:              args.debug_symbols?,
+              force:                      args.force?,
+              debug:                      args.debug?,
+              quiet:                      args.quiet?,
+              verbose:                    args.verbose?
+            )
+          end
+        end
+
         if casks.any?
           Install.ask_casks casks if args.ask?
           Cask::Reinstall.reinstall_casks(

Library/Homebrew/cmd/upgrade.rb

@@ -220,10 +220,7 @@ module Homebrew
 
         Install.perform_preinstall_checks_once
 
-        # Main block: if asking the user is enabled, show dependency and size information.
+        formulae_installer = Upgrade.get_formulae_dependencies(
-        Install.ask_formulae(formulae_to_install, args: args) if args.ask?
-
-        Upgrade.upgrade_formulae(
           formulae_to_install,
           flags:                      args.flags_only,
           dry_run:                    args.dry_run?,
@@ -239,7 +236,35 @@ module Homebrew
           verbose:                    args.verbose?,
         )
 
-        Upgrade.check_installed_dependents(
+        if args.ask?
+          dependants = Upgrade.get_dependants(
+            formulae_to_install,
+            flags:                      args.flags_only,
+            dry_run:                    args.dry_run?,
+            ask:                        args.ask?,
+            force_bottle:               args.force_bottle?,
+            build_from_source_formulae: args.build_from_source_formulae,
+            interactive:                args.interactive?,
+            keep_tmp:                   args.keep_tmp?,
+            debug_symbols:              args.debug_symbols?,
+            force:                      args.force?,
+            debug:                      args.debug?,
+            quiet:                      args.quiet?,
+            verbose:                    args.verbose?,
+          )
+
+          formulae_dependencies = Install.get_hierarchy(formulae_installer, dependants)
+          # Main block: if asking the user is enabled, show dependency and size information.
+          Install.ask_formulae(formulae_dependencies, args: args)
+
+        end
+
+        Upgrade.upgrade_formulae(formulae_installer,
+                                 dry_run: args.dry_run?,
+                                 verbose: args.verbose?)
+
+        unless args.ask?
+          dependants = Upgrade.get_dependants(
             formulae_to_install,
             flags:                      args.flags_only,
             dry_run:                    args.dry_run?,
@@ -253,6 +278,24 @@ module Homebrew
             quiet:                      args.quiet?,
             verbose:                    args.verbose?,
           )
+        end
+
+        if dependants
+          Upgrade.upgrade_dependents(
+            dependants, formulae_to_install,
+            flags:                      args.flags_only,
+            dry_run:                    args.dry_run?,
+            force_bottle:               args.force_bottle?,
+            build_from_source_formulae: args.build_from_source_formulae,
+            interactive:                args.interactive?,
+            keep_tmp:                   args.keep_tmp?,
+            debug_symbols:              args.debug_symbols?,
+            force:                      args.force?,
+            debug:                      args.debug?,
+            quiet:                      args.quiet?,
+            verbose:                    args.verbose?
+          )
+        end
 
         true
       end

Library/Homebrew/install.rb

@@ -232,7 +232,7 @@ module Homebrew
         false
       end
 
-      def install_formulae(
+      def get_formulae_dependencies(
         formulae_to_install,
         installed_on_request: true,
         installed_as_dependency: false,
@@ -257,11 +257,11 @@ module Homebrew
         skip_post_install: false,
         skip_link: false
       )
-        formula_installers = formulae_to_install.filter_map do |formula|
+        formulae_to_install.filter_map do |formula|
           Migrator.migrate_if_needed(formula, force:, dry_run:)
           build_options = formula.build
 
-          formula_installer = FormulaInstaller.new(
+          FormulaInstaller.new(
             formula,
             options:                    build_options.used_options,
             installed_on_request:,
@@ -286,24 +286,36 @@ module Homebrew
             skip_post_install:,
             skip_link:,
           )
-
-          begin
-            unless dry_run
-              formula_installer.prelude
-              formula_installer.fetch
-            end
-            formula_installer
-          rescue CannotInstallFormulaError => e
-            ofail e.message
-            nil
-          rescue UnsatisfiedRequirements, DownloadError, ChecksumMismatchError => e
-            ofail "#{formula}: #{e}"
-            nil
         end
       end
 
+      def install_formulae(
+        formula_installers,
+        installed_on_request: true,
+        installed_as_dependency: false,
+        build_bottle: false,
+        force_bottle: false,
+        bottle_arch: nil,
+        ignore_deps: false,
+        only_deps: false,
+        include_test_formulae: [],
+        build_from_source_formulae: [],
+        cc: nil,
+        git: false,
+        interactive: false,
+        keep_tmp: false,
+        debug_symbols: false,
+        force: false,
+        overwrite: false,
+        debug: false,
+        quiet: false,
+        verbose: false,
+        dry_run: false,
+        skip_post_install: false,
+        skip_link: false
+      )
         if dry_run
-          if (formulae_name_to_install = formulae_to_install.map(&:name))
+          if (formulae_name_to_install = formula_installers.map(&:name))
             ohai "Would install #{Utils.pluralize("formula", formulae_name_to_install.count,
                                                   plural: "e", include_count: true)}:"
             puts formulae_name_to_install.join(" ")
@@ -316,6 +328,18 @@ module Homebrew
         end
 
         formula_installers.each do |fi|
+          begin
+            unless dry_run
+              fi.prelude
+              fi.fetch
+            end
+          rescue CannotInstallFormulaError => e
+            ofail e.message
+            next
+          rescue UnsatisfiedRequirements, DownloadError, ChecksumMismatchError => e
+            ofail "#{formula}: #{e}"
+            next
+          end
           install_formula(fi)
           Cleanup.install_formula_clean!(fi.formula)
         end
@@ -330,17 +354,26 @@ module Homebrew
         puts formula_names.join(" ")
       end
 
+      def get_hierarchy(formulae_installer, dependants)
+        formulae_dependencies = formulae_installer.flat_map do |f|
+          [f.formula, f.compute_dependencies.flatten.filter do |c|
+            c.is_a? Dependency
+          end.flat_map(&:to_formula)]
+        end.flatten.uniq
+        formulae_dependencies.concat(dependants.upgradeable) if dependants&.upgradeable
+        formulae_dependencies
+      end
+
       # If asking the user is enabled, show dependency and size information.
       def ask_formulae(formulae, args:)
         return if formulae.empty?
 
         ohai "Looking for bottles..."
 
-        sized_formulae = compute_sized_formulae(formulae, args: args)
+        sizes = compute_total_sizes(formulae, debug: args.debug?)
-        sizes = compute_total_sizes(sized_formulae, debug: args.debug?)
 
-        puts "#{::Utils.pluralize("Formula", sized_formulae.count, plural: "e")} \
+        puts "#{::Utils.pluralize("Formula", formulae.count, plural: "e")} \
-(#{sized_formulae.count}): #{sized_formulae.join(", ")}\n\n"
+(#{formulae.count}): #{formulae.join(", ")}\n\n"
         puts "Download Size: #{disk_usage_readable(sizes[:download])}"
         puts "Install Size:  #{disk_usage_readable(sizes[:installed])}"
         puts "Net Install Size: #{disk_usage_readable(sizes[:net])}" if sizes[:net] != 0

Library/Homebrew/reinstall.rb

@@ -7,7 +7,8 @@ require "messages"
 
 module Homebrew
   module Reinstall
-    def self.reinstall_formula(
+    Formula_keg = Struct.new(:formula_installer, :keg, :formula, :options)
+    def self.get_formula_to_reinstall(
       formula,
       flags:,
       force_bottle: false,
@@ -61,13 +62,35 @@ module Homebrew
           verbose:,
         }.compact,
       )
-      fi.prelude
+      Formula_keg.new(fi, keg, formula, options)
-      fi.fetch
+    end
+
+    def self.reinstall_formula(
+      formula_keg,
+      flags:,
+      force_bottle: false,
+      build_from_source_formulae: [],
+      interactive: false,
+      keep_tmp: false,
+      debug_symbols: false,
+      force: false,
+      debug: false,
+      quiet: false,
+      verbose: false,
+      git: false
+    )
+      formula_installer = formula_keg.formula_installer
+      keg = formula_keg.keg
+      formula = formula_keg.formula
+      options = formula_keg.options
+      link_keg = keg.linked?
+      formula_installer.prelude
+      formula_installer.fetch
 
       oh1 "Reinstalling #{Formatter.identifier(formula.full_name)} #{options.to_a.join " "}"
 
-      fi.install
+      formula_installer.install
-      fi.finish
+      formula_installer.finish
     rescue FormulaInstallationAlreadyAttemptedError
       nil
       # Any other exceptions we want to restore the previous keg and report the error.

Library/Homebrew/test/cmd/install_spec.rb

@@ -103,7 +103,7 @@ RSpec.describe Homebrew::Cmd::InstallCmd do
 
     setup_test_formula "testball1", <<~RUBY
       depends_on "testball5"
-      depends_on "testball-build" => :build
+      #depends_on "testball-build" => :build
       depends_on "installed"
     RUBY
     setup_test_formula "installed"
@@ -119,9 +119,18 @@ RSpec.describe Homebrew::Cmd::InstallCmd do
     keg_dir.mkpath
     touch keg_dir/AbstractTab::FILENAME
 
+    regex = /
+      Formulae\s*\(3\):\s*
+      (testball1|testball5|testball4)
+      \s*,\s*
+      ((?!\1)testball1|testball5|testball4)
+      \s*,\s*
+      ((?!\1|\2)testball1|testball5|testball4)
+    /x
+
     expect do
       brew "install", "--ask", "testball1"
-    end.to output(/.*Formulae\s*\(3\):\s*testball1\s*,?\s*testball5\s*,?\s*testball4.*/).to_stdout
+    end.to output(regex).to_stdout
                         .and not_to_output.to_stderr
 
     expect(HOMEBREW_CELLAR/"testball1/0.1/bin/test").to be_a_file

Library/Homebrew/test/cmd/upgrade_spec.rb

@@ -33,7 +33,7 @@ RSpec.describe Homebrew::Cmd::UpgradeCmd do
     setup_test_formula "testball", <<~RUBY
       depends_on "testball5"
       # should work as its not building but test doesnt pass if dependant
-      depends_on "testball-build" => :build
+      #depends_on "testball-build" => :build
       depends_on "installed"
     RUBY
     setup_test_formula "installed"
@@ -88,8 +88,8 @@ RSpec.describe Homebrew::Cmd::UpgradeCmd do
 
     content = <<~RUBY
       depends_on "testball5"
-      # should work as it's not building but test doesn't pass if dependant
+      # should work as its not building but test doesnt pass if dependant
-      # depends_on "build" => :build
+      depends_on "testball-build" => :build
       depends_on "installed"
       version "0.1"
     RUBY
@@ -106,7 +106,7 @@ RSpec.describe Homebrew::Cmd::UpgradeCmd do
     RUBY
     setup_test_formula "testball4", content, testball_bottle: true
     setup_test_formula "hiop"
-    setup_test_formula "build"
+    setup_test_formula "testball-build"
 
     (HOMEBREW_CELLAR/"testball/0.0.1/foo").mkpath
     (HOMEBREW_CELLAR/"testball5/0.0.1/foo").mkpath

Library/Homebrew/upgrade.rb

@@ -11,7 +11,9 @@ require "utils/topological_hash"
 module Homebrew
   # Helper functions for upgrading formulae.
   module Upgrade
-    def self.upgrade_formulae(
+    Dependents = Struct.new(:upgradeable, :pinned, :skipped)
+
+    def self.get_formulae_dependencies(
       formulae_to_install,
       flags:,
       dry_run: false,
@@ -48,7 +50,7 @@ module Homebrew
         raise CyclicDependencyError, dependency_graph.strongly_connected_components if Homebrew::EnvConfig.developer?
       end
 
-      formula_installers = formulae_to_install.filter_map do |formula|
+      formulae_to_install.filter_map do |formula|
         Migrator.migrate_if_needed(formula, force:, dry_run:)
         begin
           fi = create_formula_installer(
@@ -65,18 +67,15 @@ module Homebrew
             quiet:,
             verbose:,
           )
-          unless dry_run
+          if !dry_run && dependents && fi.bottle_tab_runtime_dependencies.presence&.all? do |dependency, hash|
-            fi.prelude
-
-            # Don't need to install this bottle if all of the runtime
-            # dependencies have the same or newer version already installed.
-            next if dependents && fi.bottle_tab_runtime_dependencies.presence&.all? do |dependency, hash|
             minimum_version = Version.new(hash["version"]) if hash["version"].present?
             Dependency.new(dependency).installed?(minimum_version:, minimum_revision: hash["revision"])
           end
-
+            # Don't need to install this bottle if all of the runtime
-            fi.fetch
+            # dependencies have the same or newer version already installed.
+            next
           end
+
           fi
         rescue CannotInstallFormulaError => e
           ofail e
@@ -86,8 +85,19 @@ module Homebrew
           nil
         end
       end
+    end
 
+    def self.upgrade_formulae(formula_installers, dry_run: false, verbose: false)
       formula_installers.each do |fi|
+        begin
+          fi.prelude
+          fi.fetch
+        rescue CannotInstallFormulaError => e
+          ofail e
+        rescue UnsatisfiedRequirements, DownloadError => e
+          ofail "#{formula}: #{e}"
+        end
+
         upgrade_formula(fi, dry_run:, verbose:)
         Cleanup.install_formula_clean!(fi.formula, dry_run:)
       end
@@ -250,10 +260,11 @@ module Homebrew
       @puts_no_installed_dependents_check_disable_message_if_not_already = true
     end
 
-    def self.check_installed_dependents(
+    def self.get_dependants(
       formulae,
       flags:,
       dry_run: false,
+      ask: false,
       installed_on_request: false,
       force_bottle: false,
       build_from_source_formulae: [],
@@ -275,7 +286,7 @@ module Homebrew
         return
       end
 
-      installed_formulae = (dry_run ? formulae : FormulaInstaller.installed.to_a).dup
+      installed_formulae = ((dry_run || ask) ? formulae : FormulaInstaller.installed.to_a).dup
       installed_formulae.reject! { |f| f.core_formula? && f.versioned_formula? }
       return if installed_formulae.empty?
 
@@ -292,14 +303,6 @@ module Homebrew
         dependent.bottled? && dependent.deps.map(&:to_formula).all?(&:bottled?)
       end
 
-      if skipped_dependents.present?
-        opoo <<~EOS
-          The following dependents of upgraded formulae are outdated but will not
-          be upgraded because they are not bottled:
-            #{skipped_dependents * "\n  "}
-        EOS
-      end
-
       return if outdated_dependents.blank? && already_broken_dependents.blank?
 
       outdated_dependents -= installed_formulae if dry_run
@@ -311,24 +314,50 @@ module Homebrew
         outdated_dependents.select(&:pinned?)
                            .sort { |a, b| depends_on(a, b) }
 
-      if pinned_dependents.present?
+      Dependents.new(upgradeable_dependents, pinned_dependents, skipped_dependents)
-        plural = Utils.pluralize("dependent", pinned_dependents.count)
+    end
-        opoo "Not upgrading #{pinned_dependents.count} pinned #{plural}:"
+
-        puts(pinned_dependents.map do |f|
+    def self.upgrade_dependents(deps, formulae,
+                                flags:,
+                                dry_run: false,
+                                installed_on_request: false,
+                                force_bottle: false,
+                                build_from_source_formulae: [],
+                                interactive: false,
+                                keep_tmp: false,
+                                debug_symbols: false,
+                                force: false,
+                                debug: false,
+                                quiet: false,
+                                verbose: false)
+      upgradeable = deps.upgradeable
+      pinned      = deps.pinned
+      skipped     = deps.skipped
+      if pinned.present?
+        plural = Utils.pluralize("dependent", pinned.count)
+        opoo "Not upgrading #{pinned.count} pinned #{plural}:"
+        puts(pinned.map do |f|
           "#{f.full_specified_name} #{f.pkg_version}"
         end.join(", "))
       end
-
+      if skipped.present?
+        opoo <<~EOS
+          The following dependents of upgraded formulae are outdated but will not
+          be upgraded because they are not bottled:
+            #{skipped * "\n  "}
+        EOS
+      end
       # Print the upgradable dependents.
-      if upgradeable_dependents.blank?
+      if upgradeable.blank?
         ohai "No outdated dependents to upgrade!" unless dry_run
       else
+        installed_formulae = (dry_run ? formulae : FormulaInstaller.installed.to_a).dup
         formula_plural = Utils.pluralize("formula", installed_formulae.count, plural: "e")
         upgrade_verb = dry_run ? "Would upgrade" : "Upgrading"
-        ohai "#{upgrade_verb} #{Utils.pluralize("dependent", upgradeable_dependents.count,
+        ohai "#{upgrade_verb} #{Utils.pluralize("dependent", upgradeable.count,
                                                 include_count: true)} of upgraded #{formula_plural}:"
         Upgrade.puts_no_installed_dependents_check_disable_message_if_not_already!
-        formulae_upgrades = upgradeable_dependents.map do |f|
+        formulae_upgrades = upgradeable.map do |f|
           name = f.full_specified_name
           if f.optlinked?
             "#{name} #{Keg.new(f.opt_prefix).version} -> #{f.pkg_version}"
@@ -340,8 +369,8 @@ module Homebrew
       end
 
       unless dry_run
-        upgrade_formulae(
+        formulae_dependencies = get_formulae_dependencies(
-          upgradeable_dependents,
+          upgradeable,
           flags:,
           force_bottle:,
           build_from_source_formulae:,
@@ -354,6 +383,7 @@ module Homebrew
           quiet:,
           verbose:,
         )
+        upgrade_formulae formulae_dependencies
       end
 
       # Update installed formulae after upgrading
@@ -409,7 +439,7 @@ module Homebrew
       return if dry_run
 
       reinstallable_broken_dependents.each do |formula|
-        Reinstall.reinstall_formula(
+        formula_installer = Reinstall.get_formula_to_reinstall(
           formula,
           flags:,
           force_bottle:,
@@ -422,6 +452,20 @@ module Homebrew
           quiet:,
           verbose:,
         )
+        Reinstall.reinstall_formula(
+          formula_installer,
+          flags:                      args.flags_only,
+          force_bottle:               args.force_bottle?,
+          build_from_source_formulae: args.build_from_source_formulae,
+          interactive:                args.interactive?,
+          keep_tmp:                   args.keep_tmp?,
+          debug_symbols:              args.debug_symbols?,
+          force:                      args.force?,
+          debug:                      args.debug?,
+          quiet:                      args.quiet?,
+          verbose:                    args.verbose?,
+          git:                        args.git?,
+        )
       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.