From 5e9fcafbd8af57a5166c105f8aab3ca38edcfa63 Mon Sep 17 00:00:00 2001 From: Rylan Polster Date: Sat, 15 May 2021 14:05:50 -0400 Subject: [PATCH 1/5] formula: add preset `pour_bottle?` symbols --- Library/Homebrew/formula.rb | 28 ++++++++++++++++++++++++++- Library/Homebrew/test/formula_spec.rb | 25 ++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/Library/Homebrew/formula.rb b/Library/Homebrew/formula.rb index 5953b36097..56f587ff47 100644 --- a/Library/Homebrew/formula.rb +++ b/Library/Homebrew/formula.rb @@ -2918,8 +2918,34 @@ class Formula # # If `satisfy` returns `false` then a bottle will not be used and instead # the {Formula} will be built from source and `reason` will be printed. - def pour_bottle?(&block) + # + # Alternatively, a preset reason can be passed as a symbol: + # 
pour_bottle? :default_prefix_required
+ # 
pour_bottle? :clt_required
+ def pour_bottle?(requirement = nil, &block) @pour_bottle_check = PourBottleCheck.new(self) + + block ||= case requirement + when :default_prefix_required + lambda do |_| + T.cast(self, PourBottleCheck).reason(+<<~EOS) + The bottle needs to be installed into #{Homebrew::DEFAULT_PREFIX}. + EOS + T.cast(self, PourBottleCheck).satisfy { HOMEBREW_PREFIX.to_s == Homebrew::DEFAULT_PREFIX } + end + when :clt_required + lambda do |_| + on_macos do + T.cast(self, PourBottleCheck).reason(+<<~EOS) + The bottle needs the Apple Command Line Tools to be installed. + You can install them, if desired, with: + xcode-select --install + EOS + T.cast(self, PourBottleCheck).satisfy { MacOS::CLT.installed? } + end + end + end + @pour_bottle_check.instance_eval(&block) end diff --git a/Library/Homebrew/test/formula_spec.rb b/Library/Homebrew/test/formula_spec.rb index 2cdb36c091..182fbc8781 100644 --- a/Library/Homebrew/test/formula_spec.rb +++ b/Library/Homebrew/test/formula_spec.rb @@ -995,6 +995,31 @@ describe Formula do expect(f).to pour_bottle end + + it "returns false when set with a symbol" do + f = formula "foo" do + url "foo-1.0" + + pour_bottle? :default_prefix_required + end + + # Homebrew::DEFAULT_PREFIX is still /usr/local or /opt/homebrew + # and HOMEBREW_PREFIX is a temporary test directory + expect(f).not_to pour_bottle + end + + it "returns true when set with a symbol" do + # Ensure that prefix matches the default + stub_const "Homebrew::DEFAULT_PREFIX", HOMEBREW_PREFIX.to_s + + f = formula "foo" do + url "foo-1.0" + + pour_bottle? :default_prefix_required + end + + expect(f).to pour_bottle + end end describe "alias changes" do From 9e35b4de21037ea1fd68ab38f7366066231520ac Mon Sep 17 00:00:00 2001 From: Rylan Polster Date: Sat, 15 May 2021 14:51:11 -0400 Subject: [PATCH 2/5] fix test --- Library/Homebrew/test/formula_spec.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Library/Homebrew/test/formula_spec.rb b/Library/Homebrew/test/formula_spec.rb index 182fbc8781..f632ae7481 100644 --- a/Library/Homebrew/test/formula_spec.rb +++ b/Library/Homebrew/test/formula_spec.rb @@ -997,14 +997,15 @@ describe Formula do end it "returns false when set with a symbol" do + # Ensure that prefix does not match the default + stub_const "Homebrew::DEFAULT_PREFIX", "foo" + f = formula "foo" do url "foo-1.0" pour_bottle? :default_prefix_required end - # Homebrew::DEFAULT_PREFIX is still /usr/local or /opt/homebrew - # and HOMEBREW_PREFIX is a temporary test directory expect(f).not_to pour_bottle end From fb3bfbb65c75227abce523aa2446c3e8f390e3da Mon Sep 17 00:00:00 2001 From: Rylan Polster Date: Mon, 17 May 2021 10:55:46 -0400 Subject: [PATCH 3/5] Remove prefix option and add reason argument name --- Library/Homebrew/formula.rb | 17 ++++------------- Library/Homebrew/test/formula_spec.rb | 12 ++++++------ 2 files changed, 10 insertions(+), 19 deletions(-) diff --git a/Library/Homebrew/formula.rb b/Library/Homebrew/formula.rb index 56f587ff47..5e3c5eff1d 100644 --- a/Library/Homebrew/formula.rb +++ b/Library/Homebrew/formula.rb @@ -2920,21 +2920,12 @@ class Formula # the {Formula} will be built from source and `reason` will be printed. # # Alternatively, a preset reason can be passed as a symbol: - # 
pour_bottle? :default_prefix_required
- # 
pour_bottle? :clt_required
- def pour_bottle?(requirement = nil, &block) + # 
pour_bottle? reason: :clt_required
+ def pour_bottle?(reason: nil, &block) @pour_bottle_check = PourBottleCheck.new(self) - block ||= case requirement - when :default_prefix_required - lambda do |_| - T.cast(self, PourBottleCheck).reason(+<<~EOS) - The bottle needs to be installed into #{Homebrew::DEFAULT_PREFIX}. - EOS - T.cast(self, PourBottleCheck).satisfy { HOMEBREW_PREFIX.to_s == Homebrew::DEFAULT_PREFIX } - end - when :clt_required - lambda do |_| + if reason == :clt_required + block = lambda do |_| on_macos do T.cast(self, PourBottleCheck).reason(+<<~EOS) The bottle needs the Apple Command Line Tools to be installed. diff --git a/Library/Homebrew/test/formula_spec.rb b/Library/Homebrew/test/formula_spec.rb index f632ae7481..eb10500162 100644 --- a/Library/Homebrew/test/formula_spec.rb +++ b/Library/Homebrew/test/formula_spec.rb @@ -997,26 +997,26 @@ describe Formula do end it "returns false when set with a symbol" do - # Ensure that prefix does not match the default - stub_const "Homebrew::DEFAULT_PREFIX", "foo" + # Pretend CLT is not installed + allow(MacOS::CLT).to receive(:installed?).and_return(false) f = formula "foo" do url "foo-1.0" - pour_bottle? :default_prefix_required + pour_bottle? reason: :clt_required end expect(f).not_to pour_bottle end it "returns true when set with a symbol" do - # Ensure that prefix matches the default - stub_const "Homebrew::DEFAULT_PREFIX", HOMEBREW_PREFIX.to_s + # Pretend CLT is installed + allow(MacOS::CLT).to receive(:installed?).and_return(true) f = formula "foo" do url "foo-1.0" - pour_bottle? :default_prefix_required + pour_bottle? reason: :clt_required end expect(f).to pour_bottle From 69e29a358bb1bdea66db3d02e852a4858a11becd Mon Sep 17 00:00:00 2001 From: Rylan Polster Date: Mon, 17 May 2021 15:33:09 -0400 Subject: [PATCH 4/5] Raise errors on invalid symbol/block combinations --- Library/Homebrew/formula.rb | 11 +++++++++-- Library/Homebrew/test/formula_spec.rb | 23 +++++++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/Library/Homebrew/formula.rb b/Library/Homebrew/formula.rb index 5e3c5eff1d..d5c9730fb0 100644 --- a/Library/Homebrew/formula.rb +++ b/Library/Homebrew/formula.rb @@ -2924,8 +2924,13 @@ class Formula def pour_bottle?(reason: nil, &block) @pour_bottle_check = PourBottleCheck.new(self) - if reason == :clt_required - block = lambda do |_| + if reason.present? && block.present? + raise ArgumentError, "Do not pass both a reason and a block to `pour_bottle?`" + end + + block ||= case reason + when :clt_required + lambda do |_| on_macos do T.cast(self, PourBottleCheck).reason(+<<~EOS) The bottle needs the Apple Command Line Tools to be installed. @@ -2935,6 +2940,8 @@ class Formula T.cast(self, PourBottleCheck).satisfy { MacOS::CLT.installed? } end end + else + raise ArgumentError, "Invalid `pour_bottle?` reason" if reason.present? end @pour_bottle_check.instance_eval(&block) diff --git a/Library/Homebrew/test/formula_spec.rb b/Library/Homebrew/test/formula_spec.rb index eb10500162..3cf2db1aa8 100644 --- a/Library/Homebrew/test/formula_spec.rb +++ b/Library/Homebrew/test/formula_spec.rb @@ -1021,6 +1021,29 @@ describe Formula do expect(f).to pour_bottle end + + it "throws an error if passed both a symbol and a block" do + expect do + formula "foo" do + url "foo-1.0" + + pour_bottle? reason: :clt_required do + reason "true reason" + satisfy { true } + end + end + end.to raise_error(ArgumentError, "Do not pass both a reason and a block to `pour_bottle?`") + end + + it "throws an error if passed an invalid symbol" do + expect do + formula "foo" do + url "foo-1.0" + + pour_bottle? reason: :foo + end + end.to raise_error(ArgumentError, "Invalid `pour_bottle?` reason") + end end describe "alias changes" do From 722a8eda3c5f36d844083088608fefc6d2486f6c Mon Sep 17 00:00:00 2001 From: Rylan Polster Date: Tue, 18 May 2021 01:55:06 -0400 Subject: [PATCH 5/5] Fix tests --- Library/Homebrew/test/formula_spec.rb | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/Library/Homebrew/test/formula_spec.rb b/Library/Homebrew/test/formula_spec.rb index 3cf2db1aa8..79f484096e 100644 --- a/Library/Homebrew/test/formula_spec.rb +++ b/Library/Homebrew/test/formula_spec.rb @@ -996,7 +996,7 @@ describe Formula do expect(f).to pour_bottle end - it "returns false when set with a symbol" do + it "returns false with `reason: :clt_required` on macOS", :needs_macos do # Pretend CLT is not installed allow(MacOS::CLT).to receive(:installed?).and_return(false) @@ -1009,7 +1009,7 @@ describe Formula do expect(f).not_to pour_bottle end - it "returns true when set with a symbol" do + it "returns true with `reason: :clt_required` on macOS", :needs_macos do # Pretend CLT is installed allow(MacOS::CLT).to receive(:installed?).and_return(true) @@ -1022,6 +1022,16 @@ describe Formula do expect(f).to pour_bottle end + it "returns true with `reason: :clt_required` on Linux", :needs_linux do + f = formula "foo" do + url "foo-1.0" + + pour_bottle? reason: :clt_required + end + + expect(f).to pour_bottle + end + it "throws an error if passed both a symbol and a block" do expect do formula "foo" do