From 7c5d8a52884588774532a64d89fc3495e1b15920 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Wed, 18 Nov 2020 05:37:12 +0100 Subject: [PATCH] Refactor `FormulaInstaller`. --- Library/Homebrew/cask/installer.rb | 22 +- Library/Homebrew/cmd/install.rb | 37 ++-- Library/Homebrew/cmd/postinstall.rb | 2 +- Library/Homebrew/formula_installer.rb | 191 ++++++++++++------ Library/Homebrew/reinstall.rb | 34 ++-- .../spec/shared_context/integration_test.rb | 3 +- Library/Homebrew/upgrade.rb | 31 +-- 7 files changed, 197 insertions(+), 123 deletions(-) diff --git a/Library/Homebrew/cask/installer.rb b/Library/Homebrew/cask/installer.rb index 481307d800..5c91866086 100644 --- a/Library/Homebrew/cask/installer.rb +++ b/Library/Homebrew/cask/installer.rb @@ -371,15 +371,19 @@ module Cask force: false, ).install else - FormulaInstaller.new(cask_or_formula, verbose: verbose?).yield_self do |fi| - fi.installed_as_dependency = true - fi.installed_on_request = false - fi.show_header = true - fi.prelude - fi.fetch - fi.install - fi.finish - end + fi = FormulaInstaller.new( + cask_or_formula, + **{ + show_header: true, + installed_as_dependency: true, + installed_on_request: false, + verbose: verbose?, + }.compact, + ) + fi.prelude + fi.fetch + fi.install + fi.finish end end end diff --git a/Library/Homebrew/cmd/install.rb b/Library/Homebrew/cmd/install.rb index 9728aba9b9..9badbcdeaa 100644 --- a/Library/Homebrew/cmd/install.rb +++ b/Library/Homebrew/cmd/install.rb @@ -348,21 +348,28 @@ module Homebrew f.print_tap_action build_options = f.build - fi = FormulaInstaller.new(f, force_bottle: args.force_bottle?, - include_test_formulae: args.include_test_formulae, - build_from_source_formulae: args.build_from_source_formulae, - debug: args.debug?, quiet: args.quiet?, verbose: args.verbose?) - fi.options = build_options.used_options - fi.env = args.env - fi.force = args.force? - fi.keep_tmp = args.keep_tmp? - fi.ignore_deps = args.ignore_dependencies? - fi.only_deps = args.only_dependencies? - fi.build_bottle = args.build_bottle? - fi.bottle_arch = args.bottle_arch - fi.interactive = args.interactive? - fi.git = args.git? - fi.cc = args.cc + fi = FormulaInstaller.new( + f, + **{ + options: build_options.used_options, + build_bottle: args.build_bottle?, + force_bottle: args.force_bottle?, + bottle_arch: args.bottle_arch, + ignore_deps: args.ignore_dependencies?, + only_deps: args.only_dependencies?, + include_test_formulae: args.include_test_formulae, + build_from_source_formulae: args.build_from_source_formulae, + env: args.env, + cc: args.cc, + git: args.git?, + interactive: args.interactive?, + keep_tmp: args.keep_tmp?, + force: args.force?, + debug: args.debug?, + quiet: args.quiet?, + verbose: args.verbose?, + }.compact, + ) fi.prelude fi.fetch fi.install diff --git a/Library/Homebrew/cmd/postinstall.rb b/Library/Homebrew/cmd/postinstall.rb index 4bbee46b66..ce7113833d 100644 --- a/Library/Homebrew/cmd/postinstall.rb +++ b/Library/Homebrew/cmd/postinstall.rb @@ -28,7 +28,7 @@ module Homebrew args.named.to_resolved_formulae.each do |f| ohai "Postinstalling #{f}" - fi = FormulaInstaller.new(f, debug: args.debug?, quiet: args.quiet?, verbose: args.verbose?) + fi = FormulaInstaller.new(f, **{ debug: args.debug?, quiet: args.quiet?, verbose: args.verbose? }.compact) fi.post_install end end diff --git a/Library/Homebrew/formula_installer.rb b/Library/Homebrew/formula_installer.rb index 75daf6cc4a..ee2d2b0218 100644 --- a/Library/Homebrew/formula_installer.rb +++ b/Library/Homebrew/formula_installer.rb @@ -34,47 +34,64 @@ class FormulaInstaller extend Predicable attr_reader :formula - attr_accessor :cc, :env, :options, :build_bottle, :bottle_arch, - :build_from_source_formulae, :include_test_formulae, - :installed_as_dependency, :installed_on_request, :link_keg, :other_installers + attr_accessor :options, :link_keg + + attr_predicate :installed_as_dependency?, :installed_on_request? attr_predicate :show_summary_heading?, :show_header? - attr_writer :show_header - attr_predicate :force_bottle?, :ignore_deps?, :only_deps?, :interactive?, :git?, :force?, :keep_tmp? - attr_writer :force_bottle, :ignore_deps, :only_deps, :interactive, :git, :force, :keep_tmp - attr_predicate :verbose?, :debug?, :quiet? - def initialize(formula, - force_bottle: false, - include_test_formulae: [], - build_from_source_formulae: [], - cc: nil, - debug: false, quiet: false, verbose: false) + # TODO: Remove when removed from `test-bot`. + attr_writer :build_bottle + + def initialize( + formula, + link_keg: false, + installed_as_dependency: false, + installed_on_request: true, + show_header: false, + build_bottle: false, + force_bottle: false, + bottle_arch: nil, + ignore_deps: false, + only_deps: false, + include_test_formulae: [], + build_from_source_formulae: [], + env: nil, + git: false, + interactive: false, + keep_tmp: false, + cc: nil, + options: Options.new, + force: false, + debug: false, + quiet: false, + verbose: false + ) @formula = formula - @env = nil - @force = false - @keep_tmp = false - @link_keg = !formula.keg_only? - @show_header = false - @ignore_deps = false - @only_deps = false + @env = env + @force = force + @keep_tmp = keep_tmp + @link_keg = !formula.keg_only? || link_keg + @show_header = show_header + @ignore_deps = ignore_deps + @only_deps = only_deps @build_from_source_formulae = build_from_source_formulae - @build_bottle = false - @bottle_arch = nil + @build_bottle = build_bottle + @bottle_arch = bottle_arch @formula.force_bottle ||= force_bottle @force_bottle = @formula.force_bottle @include_test_formulae = include_test_formulae - @interactive = false - @git = false + @interactive = interactive + @git = git @cc = cc @verbose = verbose @quiet = quiet @debug = debug - @installed_as_dependency = false - @installed_on_request = true - @options = Options.new + @installed_as_dependency = installed_as_dependency + @installed_on_request = installed_on_request + @options = options @requirement_messages = [] @poured_bottle = false @pour_failed = false @@ -85,6 +102,7 @@ class FormulaInstaller @attempted ||= Set.new end + sig { void } def self.clear_attempted @attempted = Set.new end @@ -93,6 +111,7 @@ class FormulaInstaller @installed ||= Set.new end + sig { void } def self.clear_installed @installed = Set.new end @@ -116,14 +135,17 @@ class FormulaInstaller raise BuildFlagsError.new(build_flags, bottled: all_bottled) end + sig { returns(T::Boolean) } def build_from_source? - build_from_source_formulae.include?(formula.full_name) + @build_from_source_formulae.include?(formula.full_name) end + sig { returns(T::Boolean) } def include_test? - include_test_formulae.include?(formula.full_name) + @include_test_formulae.include?(formula.full_name) end + sig { returns(T::Boolean) } def build_bottle? return false unless @build_bottle @@ -137,7 +159,7 @@ class FormulaInstaller return false if !formula.bottled? && !formula.local_bottle_path return true if force_bottle? return false if build_from_source? || build_bottle? || interactive? - return false if cc + return false if @cc return false unless options.empty? return false if formula.bottle_disabled? @@ -168,7 +190,7 @@ class FormulaInstaller sig { params(dep: Formula, build: BuildOptions).returns(T::Boolean) } def install_bottle_for?(dep, build) return pour_bottle? if dep == formula - return false if build_from_source_formulae.include?(dep.full_name) + return false if @build_from_source_formulae.include?(dep.full_name) return false unless dep.bottle && dep.pour_bottle? return false unless build.used_options.empty? return false unless dep.bottle&.compatible_cellar? @@ -290,6 +312,7 @@ class FormulaInstaller end end + sig { void } def install lock @@ -341,7 +364,7 @@ class FormulaInstaller return if only_deps? - if build_bottle? && (arch = bottle_arch) && !Hardware::CPU.optimization_flags.include?(arch.to_sym) + if build_bottle? && (arch = @bottle_arch) && !Hardware::CPU.optimization_flags.include?(arch.to_sym) raise CannotInstallFormulaError, "Unrecognized architecture for --bottle-arch: #{arch}" end @@ -358,7 +381,7 @@ class FormulaInstaller action = "#{formula.full_name} #{options}".strip Utils::Analytics.report_event("install", action) - Utils::Analytics.report_event("install_on_request", action) if installed_on_request + Utils::Analytics.report_event("install_on_request", action) if installed_on_request? end self.class.attempted << formula @@ -416,8 +439,8 @@ class FormulaInstaller keg = Keg.new(formula.prefix) tab = Tab.for_keg(keg) - tab.installed_as_dependency = installed_as_dependency - tab.installed_on_request = installed_on_request + tab.installed_as_dependency = installed_as_dependency? + tab.installed_on_request = installed_on_request? tab.write end @@ -530,7 +553,7 @@ class FormulaInstaller keep_build_test ||= req.build? && !install_bottle_for_dependent && !dependent.latest_version_installed? if req.prune_from_option?(build) || - req.satisfied?(env: env, cc: cc, build_bottle: @build_bottle, bottle_arch: bottle_arch) || + req.satisfied?(env: @env, cc: @cc, build_bottle: @build_bottle, bottle_arch: @bottle_arch) || ((req.build? || req.test?) && !keep_build_test) || formula_deps_map[dependent.name]&.build? Requirement.prune @@ -558,7 +581,7 @@ class FormulaInstaller ) keep_build_test = false - keep_build_test ||= dep.test? && include_test? && include_test_formulae.include?(dependent.full_name) + keep_build_test ||= dep.test? && include_test? && @include_test_formulae.include?(dependent.full_name) keep_build_test ||= dep.build? && !install_bottle_for?(dependent, build) && !dependent.latest_version_installed? if dep.prune_from_option?(build) || ((dep.build? || dep.test?) && !keep_build_test) @@ -607,6 +630,7 @@ class FormulaInstaller options end + sig { params(dep: Dependency).returns(Options) } def inherited_options_for(dep) inherited_options = Options.new u = Option.new("universal") @@ -616,6 +640,7 @@ class FormulaInstaller inherited_options end + sig { params(deps: T::Array[[Formula, Options]]).void } def install_dependencies(deps) if deps.empty? && only_deps? puts "All dependencies for #{formula.full_name} are satisfied." @@ -629,22 +654,28 @@ class FormulaInstaller @show_header = true unless deps.empty? end + sig { params(dep: Formula).void } def fetch_dependency(dep) df = dep.to_formula - fi = FormulaInstaller.new(df, force_bottle: false, - include_test_formulae: include_test_formulae, - build_from_source_formulae: build_from_source_formulae, - debug: debug?, quiet: quiet?, verbose: verbose?) - - fi.force = force? - fi.keep_tmp = keep_tmp? - # When fetching we don't need to recurse the dependency tree as it's already - # been done for us in `compute_dependencies` and there's no requirement to - # fetch in a particular order. - fi.ignore_deps = true + fi = FormulaInstaller.new( + df, + force_bottle: false, + # When fetching we don't need to recurse the dependency tree as it's already + # been done for us in `compute_dependencies` and there's no requirement to + # fetch in a particular order. + ignore_deps: true, + include_test_formulae: @include_test_formulae, + build_from_source_formulae: @build_from_source_formulae, + keep_tmp: keep_tmp?, + force: force?, + debug: debug?, + quiet: quiet?, + verbose: verbose?, + ) fi.fetch end + sig { params(dep: Formula, inherited_options: Options).void } def install_dependency(dep, inherited_options) df = dep.to_formula tab = Tab.for_formula(df) @@ -670,20 +701,29 @@ class FormulaInstaller EOS end - fi = FormulaInstaller.new(df, force_bottle: false, - include_test_formulae: include_test_formulae, - build_from_source_formulae: build_from_source_formulae, - debug: debug?, quiet: quiet?, verbose: verbose?) + options = Options.new + options |= tab.used_options + options |= Tab.remap_deprecated_options(df.deprecated_options, dep.options) + options |= inherited_options + options &= df.options - fi.options |= tab.used_options - fi.options |= Tab.remap_deprecated_options(df.deprecated_options, dep.options) - fi.options |= inherited_options - fi.options &= df.options - fi.force = force? - fi.keep_tmp = keep_tmp? - fi.link_keg ||= keg_was_linked if keg_had_linked_keg - fi.installed_as_dependency = true - fi.installed_on_request = df.any_version_installed? && tab.installed_on_request + fi = FormulaInstaller.new( + df, + **{ + options: options, + link_keg: keg_had_linked_keg ? keg_was_linked : nil, + installed_as_dependency: true, + installed_on_request: df.any_version_installed? && tab.installed_on_request, + force_bottle: false, + include_test_formulae: @include_test_formulae, + build_from_source_formulae: @build_from_source_formulae, + keep_tmp: keep_tmp?, + force: force?, + debug: debug?, + quiet: quiet?, + verbose: verbose?, + }, + ) fi.prelude oh1 "Installing #{formula.full_name} dependency: #{Formatter.identifier(dep.name)}" fi.install @@ -702,6 +742,7 @@ class FormulaInstaller ignore_interrupts { tmp_keg.rmtree if tmp_keg&.directory? } end + sig { void } def caveats return if only_deps? @@ -716,6 +757,7 @@ class FormulaInstaller Homebrew.messages.record_caveats(formula, caveats) end + sig { void } def finish return if only_deps? @@ -781,24 +823,25 @@ class FormulaInstaller @build_time ||= Time.now - @start_time if @start_time && !interactive? end + sig { returns(T::Array[String]) } def sanitized_argv_options args = [] args << "--ignore-dependencies" if ignore_deps? if build_bottle? args << "--build-bottle" - args << "--bottle-arch=#{bottle_arch}" if bottle_arch + args << "--bottle-arch=#{@bottle_arch}" if @bottle_arch end args << "--git" if git? args << "--interactive" if interactive? args << "--verbose" if verbose? args << "--debug" if debug? - args << "--cc=#{cc}" if cc + args << "--cc=#{@cc}" if @cc args << "--keep-tmp" if keep_tmp? - if env.present? - args << "--env=#{env}" + if @env.present? + args << "--env=#{@env}" elsif formula.env.std? || formula.deps.select(&:build?).any? { |d| d.name == "scons" } args << "--env=std" end @@ -808,10 +851,12 @@ class FormulaInstaller args end + sig { returns(T::Array[String]) } def build_argv sanitized_argv_options + options.as_flags end + sig { void } def build FileUtils.rm_rf(formula.logs) @@ -865,6 +910,7 @@ class FormulaInstaller raise e end + sig { params(keg: Keg).void } def link(keg) Formula.clear_cache @@ -955,6 +1001,7 @@ class FormulaInstaller @show_summary_heading = true end + sig { void } def install_plist return unless formula.plist @@ -968,6 +1015,7 @@ class FormulaInstaller Homebrew.failed = true end + sig { params(keg: Keg).void } def fix_dynamic_linkage(keg) keg.fix_dynamic_linkage rescue Exception => e # rubocop:disable Lint/RescueException @@ -979,6 +1027,7 @@ class FormulaInstaller @show_summary_heading = true end + sig { void } def clean ohai "Cleaning" if verbose? Cleaner.new(formula).clean @@ -990,6 +1039,7 @@ class FormulaInstaller @show_summary_heading = true end + sig { void } def post_install args = %W[ nice #{RUBY_PATH} @@ -1026,6 +1076,7 @@ class FormulaInstaller @show_summary_heading = true end + sig { void } def fetch_dependencies return if ignore_deps? @@ -1035,6 +1086,7 @@ class FormulaInstaller deps.each { |dep, _options| fetch_dependency(dep) } end + sig { void } def fetch fetch_dependencies @@ -1072,6 +1124,7 @@ class FormulaInstaller end end + sig { void } def pour HOMEBREW_CELLAR.cd do downloader.stage @@ -1096,12 +1149,13 @@ class FormulaInstaller tab.time = Time.now.to_i tab.head = HOMEBREW_REPOSITORY.git_head tab.source["path"] = formula.specified_path.to_s - tab.installed_as_dependency = installed_as_dependency - tab.installed_on_request = installed_on_request + tab.installed_as_dependency = installed_as_dependency? + tab.installed_on_request = installed_on_request? tab.aliases = formula.aliases tab.write end + sig { params(output: T.nilable(String)).void } def problem_if_output(output) return unless output @@ -1125,6 +1179,7 @@ class FormulaInstaller attr_predicate :hold_locks? + sig { void } def lock return unless self.class.locked.empty? @@ -1139,6 +1194,7 @@ class FormulaInstaller @hold_locks = true end + sig { void } def unlock return unless hold_locks? @@ -1154,6 +1210,7 @@ class FormulaInstaller $stderr.puts @requirement_messages end + sig { void } def forbidden_license_check forbidden_licenses = Homebrew::EnvConfig.forbidden_licenses.to_s.dup SPDX::ALLOWED_LICENSE_SYMBOLS.each do |s| diff --git a/Library/Homebrew/reinstall.rb b/Library/Homebrew/reinstall.rb index b763cd03b5..f75867ed7e 100644 --- a/Library/Homebrew/reinstall.rb +++ b/Library/Homebrew/reinstall.rb @@ -27,21 +27,25 @@ module Homebrew build_from_source_formulae = args.build_from_source_formulae build_from_source_formulae << f.full_name if build_from_source - fi = FormulaInstaller.new(f, force_bottle: args.force_bottle?, - build_from_source_formulae: build_from_source_formulae, - debug: args.debug?, quiet: args.quiet?, verbose: args.verbose?) - fi.options = options - fi.force = args.force? - fi.keep_tmp = args.keep_tmp? - fi.build_bottle = args.build_bottle? - fi.interactive = args.interactive? - fi.git = args.git? - 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 + fi = FormulaInstaller.new( + f, + **{ + options: options, + link_keg: keg_had_linked_opt ? keg_was_linked : nil, + installed_as_dependency: tab&.installed_as_dependency, + installed_on_request: tab&.installed_on_request, + build_bottle: args.build_bottle? || tab&.built_bottle?, + force_bottle: args.force_bottle?, + build_from_source_formulae: build_from_source_formulae, + git: args.git?, + interactive: args.interactive?, + keep_tmp: args.keep_tmp?, + force: args.force?, + debug: args.debug?, + quiet: args.quiet?, + verbose: args.verbose?, + }.compact, + ) fi.prelude fi.fetch diff --git a/Library/Homebrew/test/support/helper/spec/shared_context/integration_test.rb b/Library/Homebrew/test/support/helper/spec/shared_context/integration_test.rb index 306b33fa9d..9f9e6e576d 100644 --- a/Library/Homebrew/test/support/helper/spec/shared_context/integration_test.rb +++ b/Library/Homebrew/test/support/helper/spec/shared_context/integration_test.rb @@ -178,8 +178,7 @@ RSpec.shared_context "integration test" do def install_test_formula(name, content = nil, build_bottle: false) setup_test_formula(name, content) - fi = FormulaInstaller.new(Formula[name]) - fi.build_bottle = build_bottle + fi = FormulaInstaller.new(Formula[name], build_bottle: build_bottle) fi.prelude fi.fetch fi.install diff --git a/Library/Homebrew/upgrade.rb b/Library/Homebrew/upgrade.rb index 15602d126a..dc6ab0052c 100644 --- a/Library/Homebrew/upgrade.rb +++ b/Library/Homebrew/upgrade.rb @@ -67,20 +67,23 @@ module Homebrew options |= f.build.used_options options &= f.options - fi = FormulaInstaller.new(f, force_bottle: args.force_bottle?, - build_from_source_formulae: args.build_from_source_formulae, - debug: args.debug?, quiet: args.quiet?, verbose: args.verbose?) - fi.options = options - fi.force = args.force? - fi.keep_tmp = args.keep_tmp? - 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 + fi = FormulaInstaller.new( + f, + **{ + options: options, + link_keg: keg_had_linked_opt ? keg_was_linked : nil, + installed_as_dependency: tab&.installed_as_dependency, + installed_on_request: args.named.present? || tab&.installed_on_request, + build_bottle: args.build_bottle? || tab&.built_bottle?, + force_bottle: args.force_bottle?, + build_from_source_formulae: args.build_from_source_formulae, + keep_tmp: args.keep_tmp?, + force: args.force?, + debug: args.debug?, + quiet: args.quiet?, + verbose: args.verbose?, + }.compact, + ) upgrade_version = if f.optlinked? "#{Keg.new(f.opt_prefix).version} -> #{f.pkg_version}"