Merge pull request #10887 from Bo98/dep-expand-caching

Implement caching for dependency expansion

Library/Homebrew/dependencies.rbi

@@ -0,0 +1,9 @@
+# typed: strict
+
+class Dependencies < SimpleDelegator
+  include Kernel
+end
+
+class Requirements < SimpleDelegator
+  include Kernel
+end

Library/Homebrew/dependencies_helpers.rb

@@ -35,15 +35,11 @@ def args_includes_ignores(args)
   end
 
   def recursive_includes(klass, root_dependent, includes, ignores)
-    type = if klass == Dependency
-      :dependencies
-    elsif klass == Requirement
-      :requirements
-    else
-      raise ArgumentError, "Invalid class argument: #{klass}"
-    end
+    raise ArgumentError, "Invalid class argument: #{klass}" if klass != Dependency && klass != Requirement
+
+    cache_key = "recursive_includes_#{includes}_#{ignores}"
 
-    root_dependent.send("recursive_#{type}") do |dependent, dep|
+    klass.expand(root_dependent, cache_key: cache_key) do |dependent, dep|
       if dep.recommended?
         klass.prune if ignores.include?("recommended?") || dependent.build.without?(dep)
       elsif dep.optional?
@@ -57,7 +53,7 @@ def recursive_includes(klass, root_dependent, includes, ignores)
 
       # If a tap isn't installed, we can't find the dependencies of one of
       # its formulae, and an exception will be thrown if we try.
-      if type == :dependencies &&
+      if klass == Dependency &&
          dep.is_a?(TapDependency) &&
          !dep.tap.installed?
         Dependency.keep_but_prune_recursive_deps

Library/Homebrew/dependency.rb

@@ -11,6 +11,7 @@ class Dependency
 
   extend Forwardable
   include Dependable
+  extend Cachable
 
   attr_reader :name, :tags, :env_proc, :option_names
 
@@ -87,12 +88,17 @@ class << self
     # `[dependent, dep]` pairs to allow callers to apply arbitrary filters to
     # the list.
     # The default filter, which is applied when a block is not given, omits
-    # optionals and recommendeds based on what the dependent has asked for.
-    def expand(dependent, deps = dependent.deps, &block)
+    # optionals and recommendeds based on what the dependent has asked for
+    def expand(dependent, deps = dependent.deps, cache_key: nil, &block)
       # Keep track dependencies to avoid infinite cyclic dependency recursion.
       @expand_stack ||= []
       @expand_stack.push dependent.name
 
+      if cache_key.present?
+        cache[cache_key] ||= {}
+        return cache[cache_key][dependent.full_name].dup if cache[cache_key][dependent.full_name]
+      end
+
       expanded_deps = []
 
       deps.each do |dep|
@@ -104,18 +110,20 @@ def expand(dependent, deps = dependent.deps, &block)
         when :skip
           next if @expand_stack.include? dep.name
 
-          expanded_deps.concat(expand(dep.to_formula, &block))
+          expanded_deps.concat(expand(dep.to_formula, cache_key: cache_key, &block))
         when :keep_but_prune_recursive_deps
           expanded_deps << dep
         else
           next if @expand_stack.include? dep.name
 
-          expanded_deps.concat(expand(dep.to_formula, &block))
+          expanded_deps.concat(expand(dep.to_formula, cache_key: cache_key, &block))
           expanded_deps << dep
         end
       end
 
-      merge_repeats(expanded_deps)
+      expanded_deps = merge_repeats(expanded_deps)
+      cache[cache_key][dependent.full_name] = expanded_deps.dup if cache_key.present?
+      expanded_deps
     ensure
       @expand_stack.pop
     end

Library/Homebrew/dev-cmd/bottle.rb

@@ -310,6 +310,8 @@ def bottle_formula(f, args:)
         Formula.clear_cache
         Keg.clear_cache
         Tab.clear_cache
+        Dependency.clear_cache
+        Requirement.clear_cache
         tab = Tab.for_keg(keg)
         original_tab = tab.dup
         tab.poured_from_bottle = false

Library/Homebrew/formula.rb

@@ -165,7 +165,7 @@ class Formula
   # during the installation of a {Formula}. This is annoying but the result of
   # state that we're trying to eliminate.
   # @return [BuildOptions]
-  attr_accessor :build
+  attr_reader :build
 
   # Whether this formula should be considered outdated
   # if the target of the alias it was installed with has since changed.
@@ -222,10 +222,28 @@ def active_spec=(spec_sym)
     spec = send(spec_sym)
     raise FormulaSpecificationError, "#{spec_sym} spec is not available for #{full_name}" unless spec
 
+    old_spec_sym = @active_spec_sym
     @active_spec = spec
     @active_spec_sym = spec_sym
     validate_attributes!
     @build = active_spec.build
+
+    return if spec_sym == old_spec_sym
+
+    Dependency.clear_cache
+    Requirement.clear_cache
+  end
+
+  # @private
+  def build=(build_options)
+    old_options = @build
+    @build = build_options
+
+    return if old_options.used_options == build_options.used_options &&
+              old_options.unused_options == build_options.unused_options
+
+    Dependency.clear_cache
+    Requirement.clear_cache
   end
 
   private
@@ -1657,13 +1675,15 @@ def print_tap_action(options = {})
   # means if a depends on b then b will be ordered before a in this list
   # @private
   def recursive_dependencies(&block)
-    Dependency.expand(self, &block)
+    cache_key = "Formula#recursive_dependencies" unless block
+    Dependency.expand(self, cache_key: cache_key, &block)
   end
 
   # The full set of Requirements for this formula's dependency tree.
   # @private
   def recursive_requirements(&block)
-    Requirement.expand(self, &block)
+    cache_key = "Formula#recursive_requirements" unless block
+    Requirement.expand(self, cache_key: cache_key, &block)
   end
 
   # Returns a Keg for the opt_prefix or installed_prefix if they exist.

Library/Homebrew/formula_installer.rb

@@ -572,8 +572,8 @@ def expand_requirements
     unsatisfied_reqs = Hash.new { |h, k| h[k] = [] }
     req_deps = []
     formulae = [formula]
-    formula_deps_map = Dependency.expand(formula)
-                                 .index_by(&:name)
+    formula_deps_map = formula.recursive_dependencies
+                              .index_by(&:name)
 
     while (f = formulae.pop)
       runtime_requirements = runtime_requirements(f)

Library/Homebrew/options.rb

@@ -114,6 +114,12 @@ def *(other)
     @options.to_a * other
   end
 
+  def ==(other)
+    instance_of?(other.class) &&
+      to_a == other.to_a
+  end
+  alias eql? ==
+
   def empty?
     @options.empty?
   end

Library/Homebrew/requirement.rb

@@ -15,6 +15,7 @@ class Requirement
   extend T::Sig
 
   include Dependable
+  extend Cachable
 
   attr_reader :tags, :name, :cask, :download
 
@@ -219,7 +220,12 @@ class << self
     # the list.
     # The default filter, which is applied when a block is not given, omits
     # optionals and recommendeds based on what the dependent has asked for.
-    def expand(dependent, &block)
+    def expand(dependent, cache_key: nil, &block)
+      if cache_key.present?
+        cache[cache_key] ||= {}
+        return cache[cache_key][dependent.full_name].dup if cache[cache_key][dependent.full_name]
+      end
+
       reqs = Requirements.new
 
       formulae = dependent.recursive_dependencies.map(&:to_formula)
@@ -233,6 +239,7 @@ def expand(dependent, &block)
         end
       end
 
+      cache[cache_key][dependent.full_name] = reqs.dup if cache_key.present?
       reqs
     end
 

Library/Homebrew/test/formula_spec.rb

@@ -166,8 +166,7 @@
     end
 
     build_values_with_no_installed_alias = [
-      nil,
-      BuildOptions.new({}, {}),
+      BuildOptions.new(Options.new, f.options),
       Tab.new(source: { "path" => f.path.to_s }),
     ]
     build_values_with_no_installed_alias.each do |build|
@@ -201,7 +200,10 @@
       url "foo-1.0"
     end
 
-    build_values_with_no_installed_alias = [nil, BuildOptions.new({}, {}), Tab.new(source: { "path" => f.path })]
+    build_values_with_no_installed_alias = [
+      BuildOptions.new(Options.new, f.options),
+      Tab.new(source: { "path" => f.path }),
+    ]
     build_values_with_no_installed_alias.each do |build|
       f.build = build
       expect(f.installed_alias_path).to be nil
@@ -405,7 +407,7 @@
       f = formula alias_path: alias_path do
         url "foo-1.0"
       end
-      f.build = BuildOptions.new({}, {})
+      f.build = BuildOptions.new(Options.new, f.options)
 
       expect(f.alias_path).to eq(alias_path)
       expect(f.installed_alias_path).to be nil
@@ -802,7 +804,7 @@
 
     expect(Set.new(f1.recursive_requirements)).to eq(Set[])
 
-    f1.build = BuildOptions.new(["--with-xcode"], f1.options)
+    f1.build = BuildOptions.new(Options.create(["--with-xcode"]), f1.options)
 
     expect(Set.new(f1.recursive_requirements)).to eq(Set[xcode])
 

Library/Homebrew/test/spec_helper.rb

@@ -185,6 +185,8 @@ def find_files
     Formula.clear_cache
     Keg.clear_cache
     Tab.clear_cache
+    Dependency.clear_cache
+    Requirement.clear_cache
     FormulaInstaller.clear_attempted
     FormulaInstaller.clear_installed
 
@@ -229,6 +231,8 @@ def find_files
       Formula.clear_cache
       Keg.clear_cache
       Tab.clear_cache
+      Dependency.clear_cache
+      Requirement.clear_cache
 
       FileUtils.rm_rf [
         *TEST_DIRECTORIES,