Merge pull request #10815 from Bo98/fi-more-perf

formula_installer: further reduce dependency calculations

Library/Homebrew/formula_installer.rb

@@ -288,49 +288,51 @@ class FormulaInstaller
 
     return if ignore_deps?
 
-    recursive_deps = formula.recursive_dependencies
-    recursive_formulae = recursive_deps.map(&:to_formula)
+    if Homebrew::EnvConfig.developer?
+      # `recursive_dependencies` trims cyclic dependencies, so we do one level and take the recursive deps of that.
+      # Mapping direct dependencies to deeper dependencies in a hash is also useful for the cyclic output below.
+      recursive_dep_map = formula.deps.to_h { |dep| [dep, dep.to_formula.recursive_dependencies] }
 
-    recursive_dependencies = []
-    invalid_arch_dependencies = []
-    recursive_formulae.each do |dep|
-      dep_recursive_dependencies = dep.recursive_dependencies.map(&:to_s)
-      if dep_recursive_dependencies.include?(formula.name)
-        recursive_dependencies << "#{formula.full_name} depends on #{dep.full_name}"
-        recursive_dependencies << "#{dep.full_name} depends on #{formula.full_name}"
+      cyclic_dependencies = []
+      recursive_dep_map.each do |dep, recursive_deps|
+        if [formula.name, formula.full_name].include?(dep.name)
+          cyclic_dependencies << "#{formula.full_name} depends on itself directly"
+        elsif recursive_deps.any? { |rdep| [formula.name, formula.full_name].include?(rdep.name) }
+          cyclic_dependencies << "#{formula.full_name} depends on itself via #{dep.name}"
+        end
       end
 
-      if (tab = Tab.for_formula(dep)) && tab.arch.present? && tab.arch.to_s != Hardware::CPU.arch.to_s
+      if cyclic_dependencies.present?
+        raise CannotInstallFormulaError, <<~EOS
+          #{formula.full_name} contains a recursive dependency on itself:
+            #{cyclic_dependencies.join("\n  ")}
+        EOS
+      end
+
+      # Merge into one list
+      recursive_deps = recursive_dep_map.flat_map { |dep, rdeps| [dep] + rdeps }
+      Dependency.merge_repeats(recursive_deps)
+    else
+      recursive_deps = formula.recursive_dependencies
+    end
+
+    invalid_arch_dependencies = []
+    pinned_unsatisfied_deps = []
+    recursive_deps.each do |dep|
+      if (tab = Tab.for_formula(dep.to_formula)) && tab.arch.present? && tab.arch.to_s != Hardware::CPU.arch.to_s
         invalid_arch_dependencies << "#{dep} was built for #{tab.arch}"
       end
+
+      pinned_unsatisfied_deps << dep if dep.to_formula.pinned? && !dep.satisfied?(inherited_options_for(dep))
     end
 
-    unless recursive_dependencies.empty?
-      raise CannotInstallFormulaError, <<~EOS
-        #{formula.full_name} contains a recursive dependency on itself:
-          #{recursive_dependencies.join("\n  ")}
-      EOS
-    end
-
-    if recursive_formulae.flat_map(&:recursive_dependencies)
-                         .map(&:to_s)
-                         .include?(formula.name)
-      raise CannotInstallFormulaError, <<~EOS
-        #{formula.full_name} contains a recursive dependency on itself!
-      EOS
-    end
-
-    unless invalid_arch_dependencies.empty?
+    if invalid_arch_dependencies.present?
       raise CannotInstallFormulaError, <<~EOS
         #{formula.full_name} dependencies not built for the #{Hardware::CPU.arch} CPU architecture:
           #{invalid_arch_dependencies.join("\n  ")}
       EOS
     end
 
-    pinned_unsatisfied_deps = recursive_deps.select do |dep|
-      dep.to_formula.pinned? && !dep.satisfied?(inherited_options_for(dep))
-    end
-
     return if pinned_unsatisfied_deps.empty?
 
     raise CannotInstallFormulaError,
@@ -1153,10 +1155,12 @@ class FormulaInstaller
 
     tab = Tab.for_keg(keg)
 
-    CxxStdlib.check_compatibility(
-      formula, formula.recursive_dependencies,
-      Keg.new(formula.prefix), tab.compiler
-    )
+    unless ignore_deps?
+      CxxStdlib.check_compatibility(
+        formula, formula.recursive_dependencies,
+        Keg.new(formula.prefix), tab.compiler
+      )
+    end
 
     tab.tap = formula.tap
     tab.poured_from_bottle = true

Library/Homebrew/test/formula_installer_spec.rb

@@ -96,40 +96,97 @@ describe FormulaInstaller do
     end
   end
 
-  specify "check installation sanity pinned dependency" do
-    dep_name = "dependency"
-    dep_path = CoreTap.new.formula_dir/"#{dep_name}.rb"
-    dep_path.write <<~RUBY
-      class #{Formulary.class_s(dep_name)} < Formula
-        url "foo"
-        version "0.2"
-      end
-    RUBY
+  describe "#check_install_sanity" do
+    it "raises on direct cyclic dependency" do
+      ENV["HOMEBREW_DEVELOPER"] = "1"
 
-    Formulary.cache.delete(dep_path)
-    dependency = Formulary.factory(dep_name)
+      dep_name = "homebrew-test-cyclic"
+      dep_path = CoreTap.new.formula_dir/"#{dep_name}.rb"
+      dep_path.write <<~RUBY
+        class #{Formulary.class_s(dep_name)} < Formula
+          url "foo"
+          version "0.1"
+          depends_on "#{dep_name}"
+        end
+      RUBY
+      Formulary.cache.delete(dep_path)
+      f = Formulary.factory(dep_name)
 
-    dependent = formula do
-      url "foo"
-      version "0.5"
-      depends_on dependency.name.to_s
+      fi = described_class.new(f)
+
+      expect {
+        fi.check_install_sanity
+      }.to raise_error(CannotInstallFormulaError)
     end
 
-    (dependency.prefix("0.1")/"bin"/"a").mkpath
-    HOMEBREW_PINNED_KEGS.mkpath
-    FileUtils.ln_s dependency.prefix("0.1"), HOMEBREW_PINNED_KEGS/dep_name
+    it "raises on indirect cyclic dependency" do
+      ENV["HOMEBREW_DEVELOPER"] = "1"
 
-    dependency_keg = Keg.new(dependency.prefix("0.1"))
-    dependency_keg.link
+      formula1_name = "homebrew-test-formula1"
+      formula2_name = "homebrew-test-formula2"
+      formula1_path = CoreTap.new.formula_dir/"#{formula1_name}.rb"
+      formula1_path.write <<~RUBY
+        class #{Formulary.class_s(formula1_name)} < Formula
+          url "foo"
+          version "0.1"
+          depends_on "#{formula2_name}"
+        end
+      RUBY
+      Formulary.cache.delete(formula1_path)
+      formula1 = Formulary.factory(formula1_name)
 
-    expect(dependency_keg).to be_linked
-    expect(dependency).to be_pinned
+      formula2_path = CoreTap.new.formula_dir/"#{formula2_name}.rb"
+      formula2_path.write <<~RUBY
+        class #{Formulary.class_s(formula2_name)} < Formula
+          url "foo"
+          version "0.1"
+          depends_on "#{formula1_name}"
+        end
+      RUBY
+      Formulary.cache.delete(formula2_path)
 
-    fi = described_class.new(dependent)
+      fi = described_class.new(formula1)
 
-    expect {
-      fi.check_install_sanity
-    }.to raise_error(CannotInstallFormulaError)
+      expect {
+        fi.check_install_sanity
+      }.to raise_error(CannotInstallFormulaError)
+    end
+
+    it "raises on pinned dependency" do
+      dep_name = "homebrew-test-dependency"
+      dep_path = CoreTap.new.formula_dir/"#{dep_name}.rb"
+      dep_path.write <<~RUBY
+        class #{Formulary.class_s(dep_name)} < Formula
+          url "foo"
+          version "0.2"
+        end
+      RUBY
+
+      Formulary.cache.delete(dep_path)
+      dependency = Formulary.factory(dep_name)
+
+      dependent = formula do
+        url "foo"
+        version "0.5"
+        depends_on dependency.name.to_s
+      end
+
+      (dependency.prefix("0.1")/"bin"/"a").mkpath
+      HOMEBREW_PINNED_KEGS.mkpath
+      FileUtils.ln_s dependency.prefix("0.1"), HOMEBREW_PINNED_KEGS/dep_name
+
+      dependency_keg = Keg.new(dependency.prefix("0.1"))
+      dependency_keg.link
+
+      expect(dependency_keg).to be_linked
+      expect(dependency).to be_pinned
+
+      fi = described_class.new(dependent)
+
+      expect {
+        fi.check_install_sanity
+      }.to raise_error(CannotInstallFormulaError)
+    end
   end
 
   specify "install fails with BuildError when a system() call fails" do