From 05365b5542550baec39b7a6d807ea94f47a8c212 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Tue, 28 Jul 2020 14:08:40 +0200 Subject: [PATCH] Pass `args` more explicitly in `FormulaInstaller`. --- Library/Homebrew/build.rb | 12 +++++++-- Library/Homebrew/cli/args.rb | 22 ++++++++-------- Library/Homebrew/cmd/info.rb | 2 +- Library/Homebrew/cmd/install.rb | 12 ++++++--- Library/Homebrew/fetch.rb | 2 +- Library/Homebrew/formula_installer.rb | 25 ++++++++++++------- Library/Homebrew/reinstall.rb | 6 +++-- Library/Homebrew/requirement.rb | 15 ++++++----- .../Homebrew/test/java_requirement_spec.rb | 18 ++++++------- .../test/os/mac/java_requirement_spec.rb | 10 +++----- Library/Homebrew/test/requirement_spec.rb | 20 +++++++-------- .../requirements/linux_requirement_spec.rb | 4 +-- .../requirements/macos_requirement_spec.rb | 8 +++--- .../requirements/osxfuse_requirement_spec.rb | 8 +++--- Library/Homebrew/test/x11_requirement_spec.rb | 8 +++--- 15 files changed, 92 insertions(+), 80 deletions(-) diff --git a/Library/Homebrew/build.rb b/Library/Homebrew/build.rb index ae725d66ed..2313210c8a 100644 --- a/Library/Homebrew/build.rb +++ b/Library/Homebrew/build.rb @@ -97,7 +97,11 @@ class Build bottle_arch: args.bottle_arch, ) post_superenv_hacks - reqs.each { |req| req.modify_build_environment(args: args) } + reqs.each do |req| + req.modify_build_environment( + env: args.env, cc: args.cc, build_bottle: args.build_bottle?, bottle_arch: args.bottle_arch, + ) + end deps.each(&:modify_build_environment) else ENV.setup_build_environment( @@ -106,7 +110,11 @@ class Build build_bottle: args.build_bottle?, bottle_arch: args.bottle_arch, ) - reqs.each { |req| req.modify_build_environment(args: args) } + reqs.each do |req| + req.modify_build_environment( + env: args.env, cc: args.cc, build_bottle: args.build_bottle?, bottle_arch: args.bottle_arch, + ) + end deps.each(&:modify_build_environment) keg_only_deps.each do |dep| diff --git a/Library/Homebrew/cli/args.rb b/Library/Homebrew/cli/args.rb index b7dc3e787c..db5df5c245 100644 --- a/Library/Homebrew/cli/args.rb +++ b/Library/Homebrew/cli/args.rb @@ -150,18 +150,20 @@ module Homebrew !(HEAD? || devel?) end - # Whether a given formula should be built from source during the current - # installation run. - def build_formula_from_source?(f) - return false if !build_from_source? && !build_bottle? - - formulae.any? { |args_f| args_f.full_name == f.full_name } + def build_from_source_formulae + if build_from_source? || build_bottle? + formulae.map(&:full_name) + else + [] + end end - def include_formula_test_deps?(f) - return false unless include_test? - - formulae.any? { |args_f| args_f.full_name == f.full_name } + def include_test_formulae + if include_test? + formulae.map(&:full_name) + else + [] + end end def value(name) diff --git a/Library/Homebrew/cmd/info.rb b/Library/Homebrew/cmd/info.rb index 8a3cb6ae52..c43101e7f7 100644 --- a/Library/Homebrew/cmd/info.rb +++ b/Library/Homebrew/cmd/info.rb @@ -256,7 +256,7 @@ module Homebrew def decorate_requirements(requirements) req_status = requirements.map do |req| req_s = req.display_s - req.satisfied?(args: args) ? pretty_installed(req_s) : pretty_uninstalled(req_s) + req.satisfied? ? pretty_installed(req_s) : pretty_uninstalled(req_s) end req_status.join(", ") end diff --git a/Library/Homebrew/cmd/install.rb b/Library/Homebrew/cmd/install.rb index 74bb0bc285..6150c0dd9f 100644 --- a/Library/Homebrew/cmd/install.rb +++ b/Library/Homebrew/cmd/install.rb @@ -259,7 +259,7 @@ module Homebrew formulae.each do |f| Migrator.migrate_if_needed(f, force: args.force?) - install_formula(f) + install_formula(f, args: args) Cleanup.install_formula_clean!(f) end @@ -319,12 +319,16 @@ module Homebrew end end - def install_formula(f) + def install_formula(f, args:) f.print_tap_action build_options = f.build - fi = FormulaInstaller.new(f, force_bottle: args.force_bottle?, include_test: args.include_test?, - build_from_source: args.build_from_source?, args: args) + fi = FormulaInstaller.new(f, force_bottle: args.force_bottle?, + include_test: args.include_test?, + include_test_formulae: args.include_test_formulae, + build_from_source: args.build_from_source?, + build_from_source_formulae: args.build_from_source_formulae, + args: args) fi.options = build_options.used_options fi.env = args.env fi.force = args.force? diff --git a/Library/Homebrew/fetch.rb b/Library/Homebrew/fetch.rb index b6941223cc..7b0e76cd85 100644 --- a/Library/Homebrew/fetch.rb +++ b/Library/Homebrew/fetch.rb @@ -5,7 +5,7 @@ module Homebrew def fetch_bottle?(f, args:) return true if args.force_bottle? && f.bottle return false unless f.bottle && f.pour_bottle? - return false if args.build_formula_from_source?(f) + return false if args.build_from_source_formulae.include?(f.full_name) return false unless f.bottle.compatible_cellar? true diff --git a/Library/Homebrew/formula_installer.rb b/Library/Homebrew/formula_installer.rb index d939251705..669df60645 100644 --- a/Library/Homebrew/formula_installer.rb +++ b/Library/Homebrew/formula_installer.rb @@ -40,6 +40,7 @@ class FormulaInstaller attr_reader :formula attr_accessor :cc, :env, :args, :options, :build_bottle, :bottle_arch, + :build_from_source_formulae, :include_test_formulae, :installed_as_dependency, :installed_on_request, :link_keg, :other_installers mode_attr_accessor :show_summary_heading, :show_header @@ -47,7 +48,11 @@ class FormulaInstaller mode_attr_accessor :ignore_deps, :only_deps, :interactive, :git, :force, :keep_tmp mode_attr_accessor :verbose, :debug, :quiet - def initialize(formula, force_bottle: false, include_test: false, build_from_source: false, cc: nil, args: nil) + def initialize(formula, + force_bottle: false, + include_test: false, include_test_formulae: [], + build_from_source: false, build_from_source_formulae: [], + cc: nil, args: nil) @args = args @formula = formula @env = nil @@ -58,10 +63,12 @@ class FormulaInstaller @ignore_deps = false @only_deps = false @build_from_source = build_from_source + @build_from_source_formulae = build_from_source_formulae @build_bottle = false @bottle_arch = nil @force_bottle = force_bottle @include_test = include_test + @include_test_formulae = include_test_formulae @interactive = false @git = false @cc = cc @@ -154,7 +161,7 @@ class FormulaInstaller def install_bottle_for?(dep, build) return pour_bottle? if dep == formula - return false if args&.build_formula_from_source?(dep) + 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? @@ -483,7 +490,7 @@ class FormulaInstaller if req.prune_from_option?(build) Requirement.prune - elsif req.satisfied?(args: args) + elsif req.satisfied?(env: env, cc: cc, build_bottle: @build_bottle, bottle_arch: bottle_arch) Requirement.prune elsif (req.build? || req.test?) && !keep_build_test Requirement.prune @@ -513,7 +520,7 @@ class FormulaInstaller ) keep_build_test = false - keep_build_test ||= dep.test? && include_test? && args&.include_formula_test_deps?(dependent) + 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) @@ -591,8 +598,8 @@ class FormulaInstaller def fetch_dependency(dep) df = dep.to_formula fi = FormulaInstaller.new(df, force_bottle: false, - include_test: args&.include_formula_test_deps?(df), - build_from_source: args&.build_formula_from_source?(df), + include_test: include_test_formulae.include?(df.full_name), + build_from_source: build_from_source_formulae.include?(df.full_name), args: args) fi.force = force? @@ -633,8 +640,8 @@ class FormulaInstaller end fi = FormulaInstaller.new(df, force_bottle: false, - include_test: args&.include_formula_test_deps?(df), - build_from_source: args&.build_formula_from_source?(df), + include_test: include_test_formulae.include?(df.full_name), + build_from_source: build_from_source_formulae.include?(df.full_name), args: args) fi.options |= tab.used_options @@ -775,7 +782,7 @@ class FormulaInstaller formula.options.each do |opt| name = opt.name[/^([^=]+)=$/, 1] - value = args&.value(name) if name + value = self.args&.value(name) if name args << "--#{name}=#{value}" if value end diff --git a/Library/Homebrew/reinstall.rb b/Library/Homebrew/reinstall.rb index 30c4bb17a1..bba654734a 100644 --- a/Library/Homebrew/reinstall.rb +++ b/Library/Homebrew/reinstall.rb @@ -23,8 +23,10 @@ module Homebrew options |= f.build.used_options options &= f.options - fi = FormulaInstaller.new(f, force_bottle: args.force_bottle?, include_test: args.include_test?, - build_from_source: args.build_from_source?, args: args) + fi = FormulaInstaller.new(f, force_bottle: args.force_bottle?, + build_from_source: args.build_from_source?, + build_from_source_formulae: args.build_from_source_formulae, + args: args) fi.options = options fi.force = args.force? fi.keep_tmp = args.keep_tmp? diff --git a/Library/Homebrew/requirement.rb b/Library/Homebrew/requirement.rb index 1458370103..b9e4a09d10 100644 --- a/Library/Homebrew/requirement.rb +++ b/Library/Homebrew/requirement.rb @@ -53,11 +53,14 @@ class Requirement # Overriding {#satisfied?} is unsupported. # Pass a block or boolean to the satisfy DSL method instead. - def satisfied?(args: nil) + def satisfied?(env: nil, cc: nil, build_bottle: false, bottle_arch: nil) satisfy = self.class.satisfy return true unless satisfy - @satisfied_result = satisfy.yielder(args: args) { |p| instance_eval(&p) } + @satisfied_result = + satisfy.yielder(env: env, cc: cc, build_bottle: build_bottle, bottle_arch: bottle_arch) do |p| + instance_eval(&p) + end return false unless @satisfied_result true @@ -81,8 +84,8 @@ class Requirement # Overriding {#modify_build_environment} is unsupported. # Pass a block to the env DSL method instead. - def modify_build_environment(args:) - satisfied?(args: args) + def modify_build_environment(env: nil, cc: nil, build_bottle: false, bottle_arch: nil) + satisfied?(env: env, cc: cc, build_bottle: build_bottle, bottle_arch: bottle_arch) instance_eval(&env_proc) if env_proc # XXX If the satisfy block returns a Pathname, then make sure that it @@ -181,13 +184,13 @@ class Requirement @proc = block end - def yielder(args:) + def yielder(env: nil, cc: nil, build_bottle: false, bottle_arch: nil) if instance_variable_defined?(:@satisfied) @satisfied elsif @options[:build_env] require "extend/ENV" ENV.with_build_environment( - env: args.env, cc: args.cc, build_bottle: args.build_bottle?, bottle_arch: args.bottle_arch, + env: env, cc: cc, build_bottle: build_bottle, bottle_arch: bottle_arch, ) do yield @proc end diff --git a/Library/Homebrew/test/java_requirement_spec.rb b/Library/Homebrew/test/java_requirement_spec.rb index 694fe2f91d..590f27a1d0 100644 --- a/Library/Homebrew/test/java_requirement_spec.rb +++ b/Library/Homebrew/test/java_requirement_spec.rb @@ -41,16 +41,14 @@ describe JavaRequirement do describe "#satisfied?" do subject { described_class.new(%w[1.8]) } - let(:args) { Homebrew::CLI::Args.new } - it "returns false if no `java` executable can be found" do allow(File).to receive(:executable?).and_return(false) - expect(subject).not_to be_satisfied(args: args) + expect(subject).not_to be_satisfied end it "returns true if #preferred_java returns a path" do allow(subject).to receive(:preferred_java).and_return(Pathname.new("/usr/bin/java")) - expect(subject).to be_satisfied(args: args) + expect(subject).to be_satisfied end context "when #possible_javas contains paths" do @@ -74,17 +72,17 @@ describe JavaRequirement do it "returns false if all are lower" do setup_java_with_version "1.6.0_5" - expect(subject).not_to be_satisfied(args: args) + expect(subject).not_to be_satisfied end it "returns true if one is equal" do setup_java_with_version "1.7.0_5" - expect(subject).to be_satisfied(args: args) + expect(subject).to be_satisfied end it "returns false if all are higher" do setup_java_with_version "1.8.0_5" - expect(subject).not_to be_satisfied(args: args) + expect(subject).not_to be_satisfied end end @@ -93,17 +91,17 @@ describe JavaRequirement do it "returns false if all are lower" do setup_java_with_version "1.6.0_5" - expect(subject).not_to be_satisfied(args: args) + expect(subject).not_to be_satisfied end it "returns true if one is equal" do setup_java_with_version "1.7.0_5" - expect(subject).to be_satisfied(args: args) + expect(subject).to be_satisfied end it "returns true if one is higher" do setup_java_with_version "1.8.0_5" - expect(subject).to be_satisfied(args: args) + expect(subject).to be_satisfied end end end diff --git a/Library/Homebrew/test/os/mac/java_requirement_spec.rb b/Library/Homebrew/test/os/mac/java_requirement_spec.rb index 190b7c1aba..65729b23bc 100644 --- a/Library/Homebrew/test/os/mac/java_requirement_spec.rb +++ b/Library/Homebrew/test/os/mac/java_requirement_spec.rb @@ -9,8 +9,6 @@ describe JavaRequirement do let(:java_home) { mktmpdir } - let(:args) { Homebrew::CLI::Args.new } - before do FileUtils.mkdir java_home/"bin" FileUtils.touch java_home/"bin/java" @@ -18,23 +16,23 @@ describe JavaRequirement do end specify "Apple Java environment" do - expect(subject).to be_satisfied(args: args) + expect(subject).to be_satisfied expect(ENV).to receive(:prepend_path) expect(ENV).to receive(:append_to_cflags) - subject.modify_build_environment(args: args) + subject.modify_build_environment expect(ENV["JAVA_HOME"]).to eq(java_home.to_s) end specify "Oracle Java environment" do - expect(subject).to be_satisfied(args: args) + expect(subject).to be_satisfied FileUtils.mkdir java_home/"include" expect(ENV).to receive(:prepend_path) expect(ENV).to receive(:append_to_cflags).twice - subject.modify_build_environment(args: args) + subject.modify_build_environment expect(ENV["JAVA_HOME"]).to eq(java_home.to_s) end end diff --git a/Library/Homebrew/test/requirement_spec.rb b/Library/Homebrew/test/requirement_spec.rb index 12a0e302b3..ffcb3a8de1 100644 --- a/Library/Homebrew/test/requirement_spec.rb +++ b/Library/Homebrew/test/requirement_spec.rb @@ -11,8 +11,6 @@ describe Requirement do let(:klass) { Class.new(described_class) } - let(:args) { Homebrew::CLI::Args.new } - describe "#tags" do subject { described_class.new(tags) } @@ -67,7 +65,7 @@ describe Requirement do end end - it { is_expected.to be_satisfied(args: args) } + it { is_expected.to be_satisfied } end describe "#satisfy with block and build_env returns false" do @@ -79,7 +77,7 @@ describe Requirement do end end - it { is_expected.not_to be_satisfied(args: args) } + it { is_expected.not_to be_satisfied } end describe "#satisfy returns true" do @@ -89,7 +87,7 @@ describe Requirement do end end - it { is_expected.to be_satisfied(args: args) } + it { is_expected.to be_satisfied } end describe "#satisfy returns false" do @@ -99,7 +97,7 @@ describe Requirement do end end - it { is_expected.not_to be_satisfied(args: args) } + it { is_expected.not_to be_satisfied } end describe "#satisfy with block returning true and without :build_env" do @@ -113,7 +111,7 @@ describe Requirement do it "sets up build environment" do expect(ENV).to receive(:with_build_environment).and_call_original - subject.satisfied?(args: args) + subject.satisfied? end end @@ -128,7 +126,7 @@ describe Requirement do it "skips setting up build environment" do expect(ENV).not_to receive(:with_build_environment) - subject.satisfied?(args: args) + subject.satisfied? end end @@ -143,8 +141,8 @@ describe Requirement do it "infers path from #satisfy result" do expect(ENV).to receive(:prepend_path).with("PATH", Pathname.new("/foo/bar")) - subject.satisfied?(args: args) - subject.modify_build_environment(args: args) + subject.satisfied? + subject.modify_build_environment end end end @@ -182,7 +180,7 @@ describe Requirement do let(:klass) { Class.new(described_class) } it "returns nil" do - expect(subject.modify_build_environment(args: args)).to be nil + expect(subject.modify_build_environment).to be nil end end end diff --git a/Library/Homebrew/test/requirements/linux_requirement_spec.rb b/Library/Homebrew/test/requirements/linux_requirement_spec.rb index a6857365df..f328579fa2 100644 --- a/Library/Homebrew/test/requirements/linux_requirement_spec.rb +++ b/Library/Homebrew/test/requirements/linux_requirement_spec.rb @@ -7,10 +7,8 @@ describe LinuxRequirement do subject(:requirement) { described_class.new } describe "#satisfied?" do - let(:args) { Homebrew::CLI::Args.new } - it "returns true on Linux" do - expect(requirement.satisfied?(args: args)).to eq(OS.linux?) + expect(requirement.satisfied?).to eq(OS.linux?) end end end diff --git a/Library/Homebrew/test/requirements/macos_requirement_spec.rb b/Library/Homebrew/test/requirements/macos_requirement_spec.rb index ff160e560f..b966111262 100644 --- a/Library/Homebrew/test/requirements/macos_requirement_spec.rb +++ b/Library/Homebrew/test/requirements/macos_requirement_spec.rb @@ -7,20 +7,18 @@ describe MacOSRequirement do subject(:requirement) { described_class.new } describe "#satisfied?" do - let(:args) { Homebrew::CLI::Args.new } - it "returns true on macOS" do - expect(requirement.satisfied?(args: args)).to eq OS.mac? + expect(requirement.satisfied?).to eq OS.mac? end it "supports version symbols", :needs_macos do requirement = described_class.new([MacOS.version.to_sym]) - expect(requirement).to be_satisfied(args: args) + expect(requirement).to be_satisfied end it "supports maximum versions", :needs_macos do requirement = described_class.new([:catalina], comparator: "<=") - expect(requirement.satisfied?(args: args)).to eq MacOS.version <= :catalina + expect(requirement.satisfied?).to eq MacOS.version <= :catalina end end end diff --git a/Library/Homebrew/test/requirements/osxfuse_requirement_spec.rb b/Library/Homebrew/test/requirements/osxfuse_requirement_spec.rb index a5ceb17909..8c38a45884 100644 --- a/Library/Homebrew/test/requirements/osxfuse_requirement_spec.rb +++ b/Library/Homebrew/test/requirements/osxfuse_requirement_spec.rb @@ -22,23 +22,21 @@ describe OsxfuseRequirement do end describe "#modify_build_environment", :needs_macos do - let(:args) { Homebrew::CLI::Args.new } - it "adds the fuse directories to PKG_CONFIG_PATH" do allow(ENV).to receive(:append_path) - requirement.modify_build_environment(args: args) + requirement.modify_build_environment expect(ENV).to have_received(:append_path).with("PKG_CONFIG_PATH", any_args) end it "adds the fuse directories to HOMEBREW_LIBRARY_PATHS" do allow(ENV).to receive(:append_path) - requirement.modify_build_environment(args: args) + requirement.modify_build_environment expect(ENV).to have_received(:append_path).with("HOMEBREW_LIBRARY_PATHS", any_args) end it "adds the fuse directories to HOMEBREW_INCLUDE_PATHS" do allow(ENV).to receive(:append_path) - requirement.modify_build_environment(args: args) + requirement.modify_build_environment expect(ENV).to have_received(:append_path).with("HOMEBREW_INCLUDE_PATHS", any_args) end end diff --git a/Library/Homebrew/test/x11_requirement_spec.rb b/Library/Homebrew/test/x11_requirement_spec.rb index 133135eb0b..436e0c15ef 100644 --- a/Library/Homebrew/test/x11_requirement_spec.rb +++ b/Library/Homebrew/test/x11_requirement_spec.rb @@ -6,8 +6,6 @@ require "requirements/x11_requirement" describe X11Requirement do let(:default_name) { "x11" } - let(:args) { Homebrew::CLI::Args.new } - describe "#name" do it "defaults to x11" do expect(subject.name).to eq(default_name) @@ -25,7 +23,7 @@ describe X11Requirement do it "calls ENV#x11" do allow(subject).to receive(:satisfied?).and_return(true) expect(ENV).to receive(:x11) - subject.modify_build_environment(args: args) + subject.modify_build_environment end end @@ -33,12 +31,12 @@ describe X11Requirement do it "returns true if X11 is installed" do expect(MacOS::XQuartz).to receive(:version).and_return("2.7.5") expect(MacOS::XQuartz).to receive(:installed?).and_return(true) - expect(subject).to be_satisfied(args: args) + expect(subject).to be_satisfied end it "returns false if X11 is not installed" do expect(MacOS::XQuartz).to receive(:installed?).and_return(false) - expect(subject).not_to be_satisfied(args: args) + expect(subject).not_to be_satisfied end end end