From eb74717a9e24eb94de9a60508a208b224dff8172 Mon Sep 17 00:00:00 2001
From: Mike McQuaid <mike@mikemcquaid.com>
Date: Fri, 11 Dec 2020 16:57:47 +0000
Subject: [PATCH] Fix unbottled dependency handling

- Rename `BuildToolsError` to `UnbottledError` to better reflect what it
  does (particularly now)
- Improve the `UnbottledError` messaging
- Raise the `UnbottledError` for dependencies also when building bottles
  (so that we don't try and build bottles against things that have been
  built from source).
---
 Library/Homebrew/exceptions.rb                | 17 ++++++------
 Library/Homebrew/formula_installer.rb         | 26 ++++++++++---------
 .../test/formula_installer_bottle_spec.rb     |  2 +-
 3 files changed, 24 insertions(+), 21 deletions(-)

diff --git a/Library/Homebrew/exceptions.rb b/Library/Homebrew/exceptions.rb
index 428140ac4d..b1ffb46b75 100644
--- a/Library/Homebrew/exceptions.rb
+++ b/Library/Homebrew/exceptions.rb
@@ -488,17 +488,18 @@ class BuildError < RuntimeError
   end
 end
 
-# Raised by {FormulaInstaller#check_dependencies_bottled} and
-# {FormulaInstaller#install} if the formula or its dependencies are not bottled
-# and are being installed on a system without necessary build tools.
-class BuildToolsError < RuntimeError
+# Raised if the formula or its dependencies are not bottled and are being
+# installed in a situation where a bottle is required.
+class UnbottledError < RuntimeError
   def initialize(formulae)
-    super <<~EOS
-      The following #{"formula".pluralize(formulae.count)}
+    msg = +<<~EOS
+      The following #{"formula".pluralize(formulae.count)} cannot be installed from #{"bottle".pluralize(formulae.count)} and must be
+      built from source.
         #{formulae.to_sentence}
-      cannot be installed as #{"binary package".pluralize(formulae.count)} and must be built from source.
-      #{DevelopmentTools.installation_instructions}
     EOS
+    msg += "#{DevelopmentTools.installation_instructions}\n" unless DevelopmentTools.installed?
+    msg.freeze
+    super(msg)
   end
 end
 
diff --git a/Library/Homebrew/formula_installer.rb b/Library/Homebrew/formula_installer.rb
index 679f7500ca..72fcfc6c70 100644
--- a/Library/Homebrew/formula_installer.rb
+++ b/Library/Homebrew/formula_installer.rb
@@ -356,11 +356,17 @@ class FormulaInstaller
 
     check_conflicts
 
-    raise BuildToolsError, [formula] if !pour_bottle? && !formula.bottle_unneeded? && !DevelopmentTools.installed?
+    raise UnbottledError, [formula] if !pour_bottle? && !formula.bottle_unneeded? && !DevelopmentTools.installed?
 
     unless ignore_deps?
       deps = compute_dependencies
-      check_dependencies_bottled(deps) if pour_bottle? && !DevelopmentTools.installed?
+      if ((pour_bottle? && !DevelopmentTools.installed?) || build_bottle?) &&
+         (unbottled = unbottled_dependencies(deps)).presence
+        # Check that each dependency in deps has a bottle available, terminating
+        # abnormally with a UnbottledError if one or more don't.
+        raise UnbottledError, unbottled
+      end
+
       install_dependencies(deps)
     end
 
@@ -412,7 +418,7 @@ class FormulaInstaller
         @pour_failed = true
         onoe e.message
         opoo "Bottle installation failed: building from source."
-        raise BuildToolsError, [formula] unless DevelopmentTools.installed?
+        raise UnbottledError, [formula] unless DevelopmentTools.installed?
 
         compute_and_install_dependencies unless ignore_deps?
       else
@@ -490,16 +496,12 @@ class FormulaInstaller
     expand_dependencies(req_deps + formula.deps)
   end
 
-  # Check that each dependency in deps has a bottle available, terminating
-  # abnormally with a BuildToolsError if one or more don't.
-  # Only invoked when the user has no developer tools.
-  def check_dependencies_bottled(deps)
-    unbottled = deps.reject do |dep, _|
-      dep_f = dep.to_formula
-      dep_f.pour_bottle? || dep_f.bottle_unneeded?
-    end
+  def unbottled_dependencies(deps)
+    deps.map(&:first).map(&:to_formula).reject do |dep_f|
+      next false unless dep_f.pour_bottle?
 
-    raise BuildToolsError, unbottled unless unbottled.empty?
+      dep_f.bottle_unneeded? || dep_f.bottled?
+    end
   end
 
   def compute_and_install_dependencies
diff --git a/Library/Homebrew/test/formula_installer_bottle_spec.rb b/Library/Homebrew/test/formula_installer_bottle_spec.rb
index b42513f75d..d307e7a607 100644
--- a/Library/Homebrew/test/formula_installer_bottle_spec.rb
+++ b/Library/Homebrew/test/formula_installer_bottle_spec.rb
@@ -83,7 +83,7 @@ describe FormulaInstaller do
 
     expect {
       described_class.new(formula).install
-    }.to raise_error(BuildToolsError)
+    }.to raise_error(UnbottledError)
 
     expect(formula).not_to be_latest_version_installed
   end