Merge pull request #16007 from Bo98/better-readall

Improve performance of `brew readall`

Library/Homebrew/extend/os/mac/readall.rb

@@ -3,7 +3,7 @@
 
 module Readall
   class << self
-    def valid_casks?(casks, os_name: nil, arch: Hardware::CPU.type)
+    def valid_casks?(tap, os_name: nil, arch: Hardware::CPU.type)
       return true if os_name == :linux
 
       current_macos_version = if os_name.is_a?(Symbol)
@@ -13,7 +13,7 @@ def valid_casks?(casks, os_name: nil, arch: Hardware::CPU.type)
       end
 
       success = T.let(true, T::Boolean)
-      casks.each do |file|
+      tap.cask_files.each do |file|
         cask = Cask::CaskLoader.load(file)
 
         # Fine to have missing URLs for unsupported macOS

Library/Homebrew/formula.rb

@@ -2397,7 +2397,7 @@ def to_hash_with_variations
 
     variations = {}
 
-    if path.exist? && (self.class.on_system_blocks_exist? || @on_system_blocks_exist)
+    if path.exist? && on_system_blocks_exist?
       formula_contents = path.read
       OnSystem::ALL_OS_ARCH_COMBINATIONS.each do |os, arch|
         bottle_tag = Utils::Bottles::Tag.new(system: os, arch: arch)
@@ -2451,6 +2451,11 @@ def bottle_hash
     hash
   end
 
+  # @private
+  def on_system_blocks_exist?
+    self.class.on_system_blocks_exist? || @on_system_blocks_exist
+  end
+
   # @private
   def fetch(verify_download_integrity: true)
     active_spec.fetch(verify_download_integrity: verify_download_integrity)

Library/Homebrew/readall.rb

@@ -9,6 +9,10 @@
 # @api private
 module Readall
   class << self
+    include Cachable
+
+    private :cache
+
     def valid_ruby_syntax?(ruby_files)
       failed = T.let(false, T::Boolean)
       ruby_files.each do |ruby_file|
@@ -39,19 +43,26 @@ def valid_aliases?(alias_dir, formula_dir)
       !failed
     end
 
-    def valid_formulae?(formulae, bottle_tag: nil)
-      success = T.let(true, T::Boolean)
-      formulae.each do |file|
-        base = Formulary.factory(file)
-        next if bottle_tag.blank? || !base.path.exist? || !base.class.on_system_blocks_exist?
-
-        formula_contents = base.path.read
+    def valid_formulae?(tap, bottle_tag: nil)
+      cache[:valid_formulae] ||= {}
 
-        readall_namespace = Formulary.class_s("Readall#{bottle_tag.to_sym.capitalize}")
-        readall_formula_class = Formulary.load_formula(base.name, base.path, formula_contents, readall_namespace,
-                                                       flags: base.class.build_flags, ignore_errors: true)
-        readall_formula_class.new(base.name, base.path, :stable,
-                                  alias_path: base.alias_path, force_bottle: base.force_bottle)
+      success = T.let(true, T::Boolean)
+      tap.formula_files.each do |file|
+        valid = cache[:valid_formulae][file]
+        next if valid == true || valid&.include?(bottle_tag)
+
+        formula_name = file.basename(".rb").to_s
+        formula_contents = file.read(encoding: "UTF-8")
+
+        readall_namespace = "ReadallNamespace"
+        readall_formula_class = Formulary.load_formula(formula_name, file, formula_contents, readall_namespace,
+                                                       flags: [], ignore_errors: false)
+        readall_formula = readall_formula_class.new(formula_name, file, :stable, tap: tap)
+        cache[:valid_formulae][file] = if readall_formula.on_system_blocks_exist?
+          [bottle_tag, *cache[:valid_formulae][file]]
+        else
+          true
+        end
       rescue Interrupt
         raise
       rescue Exception => e # rubocop:disable Lint/RescueException
@@ -62,7 +73,7 @@ def valid_formulae?(formulae, bottle_tag: nil)
       success
     end
 
-    def valid_casks?(_casks, os_name: nil, arch: nil)
+    def valid_casks?(_tap, os_name: nil, arch: nil)
       true
     end
 
@@ -75,16 +86,16 @@ def valid_tap?(tap, aliases: false, no_simulate: false, os_arch_combinations: On
       end
 
       if no_simulate
-        success = false unless valid_formulae?(tap.formula_files)
-        success = false unless valid_casks?(tap.cask_files)
+        success = false unless valid_formulae?(tap)
+        success = false unless valid_casks?(tap)
       else
         os_arch_combinations.each do |os, arch|
           bottle_tag = Utils::Bottles::Tag.new(system: os, arch: arch)
           next unless bottle_tag.valid_combination?
 
           Homebrew::SimulateSystem.with os: os, arch: arch do
-            success = false unless valid_formulae?(tap.formula_files, bottle_tag: bottle_tag)
-            success = false unless valid_casks?(tap.cask_files, os_name: os, arch: arch)
+            success = false unless valid_formulae?(tap, bottle_tag: bottle_tag)
+            success = false unless valid_casks?(tap, os_name: os, arch: arch)
           end
         end
       end

Library/Homebrew/test/spec_helper.rb

@@ -196,6 +196,7 @@ def find_files
     Tab.clear_cache
     Dependency.clear_cache
     Requirement.clear_cache
+    Readall.clear_cache if defined?(Readall)
     FormulaInstaller.clear_attempted
     FormulaInstaller.clear_installed
     FormulaInstaller.clear_fetched
@@ -251,6 +252,7 @@ def find_files
       Tab.clear_cache
       Dependency.clear_cache
       Requirement.clear_cache
+      Readall.clear_cache if defined?(Readall)
 
       FileUtils.rm_rf [
         *TEST_DIRECTORIES,