Merge pull request #16863 from apainintheneck/memoize-installed-tap-loading-v2

Memoize installed tap loading v2

Library/Homebrew/cask/cask.rb

@@ -87,7 +87,7 @@ module Cask
     sig { returns(T::Array[String]) }
     def old_tokens
       @old_tokens ||= if (tap = self.tap)
-        Tap.reverse_tap_migrations_renames.fetch("#{tap}/#{token}", []) +
+        Tap.tap_migration_oldnames(tap, token) +
           tap.cask_reverse_renames.fetch(token, [])
       else
         []

Library/Homebrew/cmd/readall.rb

@@ -57,7 +57,7 @@ module Homebrew
           raise UsageError, "`brew readall` needs a tap or `--eval-all` passed or `HOMEBREW_EVAL_ALL` set!"
         end
 
-        Tap.select(&:installed?)
+        Tap.installed
       else
         args.named.to_installed_taps
       end

Library/Homebrew/cmd/tap.rb

@@ -54,12 +54,12 @@ module Homebrew
     args = tap_args.parse
 
     if args.repair?
-      Tap.select(&:installed?).each do |tap|
+      Tap.installed.each do |tap|
         tap.link_completions_and_manpages
         tap.fix_remote_configuration
       end
     elsif args.no_named?
-      puts Tap.select(&:installed?)
+      puts Tap.installed.sort_by(&:name)
     else
       tap = Tap.fetch(args.named.first)
       begin

Library/Homebrew/cmd/update-report.rb

@@ -146,7 +146,7 @@ module Homebrew
     hub = ReporterHub.new
 
     updated_taps = []
-    Tap.select(&:installed?).each do |tap|
+    Tap.installed.each do |tap|
       next if !tap.git? || tap.git_repo.origin_url.nil?
       next if (tap.core_tap? || tap.core_cask_tap?) && !Homebrew::EnvConfig.no_install_from_api?
 
@@ -254,7 +254,7 @@ module Homebrew
 
     Commands.rebuild_commands_completion_list
     link_completions_manpages_and_docs
-    Tap.select(&:installed?).each(&:link_completions_and_manpages)
+    Tap.installed.each(&:link_completions_and_manpages)
 
     failed_fetch_dirs = ENV["HOMEBREW_MISSING_REMOTE_REF_DIRS"]&.split("\n")
     if failed_fetch_dirs.present?

Library/Homebrew/completions.rb

@@ -72,7 +72,7 @@ module Homebrew
     sig { void }
     def self.link!
       Settings.write :linkcompletions, true
-      Tap.select(&:installed?).each do |tap|
+      Tap.installed.each do |tap|
         Utils::Link.link_completions tap.path, "brew completions link"
       end
     end
@@ -80,7 +80,7 @@ module Homebrew
     sig { void }
     def self.unlink!
       Settings.write :linkcompletions, false
-      Tap.select(&:installed?).each do |tap|
+      Tap.installed.each do |tap|
         next if tap.official?
 
         Utils::Link.unlink_completions tap.path
@@ -94,7 +94,7 @@ module Homebrew
 
     sig { returns(T::Boolean) }
     def self.completions_to_link?
-      Tap.select(&:installed?).each do |tap|
+      Tap.installed.each do |tap|
         next if tap.official?
 
         SHELLS.each do |shell|

Library/Homebrew/dev-cmd/audit.rb

@@ -189,7 +189,7 @@ module Homebrew
 
     # Run tap audits first
     named_arg_taps = [*audit_formulae, *audit_casks].map(&:tap).uniq if !args.tap && !no_named_args
-    tap_problems = Tap.select(&:installed?).each_with_object({}) do |tap, problems|
+    tap_problems = Tap.installed.each_with_object({}) do |tap, problems|
       next if args.tap && tap != args.tap
       next if named_arg_taps&.exclude?(tap)
 

Library/Homebrew/diagnostic.rb

@@ -545,7 +545,7 @@ module Homebrew
         return if ENV["CI"]
         return unless Utils::Git.available?
 
-        commands = Tap.select(&:installed?).filter_map do |tap|
+        commands = Tap.installed.filter_map do |tap|
           next if tap.git_repo.default_origin_branch?
 
           "git -C $(brew --repo #{tap.name}) checkout #{tap.git_repo.origin_branch_name}"
@@ -794,7 +794,7 @@ module Homebrew
 
       def check_for_tap_ruby_files_locations
         bad_tap_files = {}
-        Tap.select(&:installed?).each do |tap|
+        Tap.installed.each do |tap|
           unused_formula_dirs = tap.potential_formula_dirs - [tap.formula_dir]
           unused_formula_dirs.each do |dir|
             next unless dir.exist?

Library/Homebrew/formula.rb

@@ -552,7 +552,7 @@ class Formula
   sig { returns(T::Array[String]) }
   def oldnames
     @oldnames ||= if (tap = self.tap)
-      Tap.reverse_tap_migrations_renames.fetch("#{tap}/#{name}", []) +
+      Tap.tap_migration_oldnames(tap, name) +
         tap.formula_reverse_renames.fetch(name, [])
     else
       []

Library/Homebrew/tap.rb

@@ -189,6 +189,7 @@ class Tap
     @command_files = nil
 
     @tap_migrations = nil
+    @reverse_tap_migrations_renames = nil
 
     @audit_exceptions = nil
     @style_exceptions = nil
@@ -393,6 +394,7 @@ class Tap
     end
 
     clear_cache
+    Tap.clear_cache
 
     $stderr.ohai "Tapping #{name}" unless quiet
     args =  %W[clone #{requested_remote} #{path}]
@@ -535,6 +537,7 @@ class Tap
 
     Commands.rebuild_commands_completion_list
     clear_cache
+    Tap.clear_cache
 
     return if !manual || !official?
 
@@ -840,21 +843,6 @@ class Tap
     end
   end
 
-  sig { returns(T::Hash[String, T::Array[String]]) }
-  def self.reverse_tap_migrations_renames
-    Tap.each_with_object({}) do |tap, hash|
-      tap.tap_migrations.each do |old_name, new_name|
-        new_tap_user, new_tap_repo, new_name = new_name.split("/", 3)
-        next unless new_name
-
-        new_tap = Tap.fetch(T.must(new_tap_user), T.must(new_tap_repo))
-
-        hash["#{new_tap}/#{new_name}"] ||= []
-        hash["#{new_tap}/#{new_name}"] << old_name
-      end
-    end
-  end
-
   # Hash with tap migrations.
   sig { returns(T::Hash[String, String]) }
   def tap_migrations
@@ -865,6 +853,31 @@ class Tap
     end
   end
 
+  sig { returns(T::Hash[String, T::Array[String]]) }
+  def reverse_tap_migrations_renames
+    @reverse_tap_migrations_renames ||= tap_migrations.each_with_object({}) do |(old_name, new_name), hash|
+      # Only include renames:
+      # + `homebrew/cask/water-buffalo`
+      # - `homebrew/cask`
+      next if new_name.count("/") != 2
+
+      hash[new_name] ||= []
+      hash[new_name] << old_name
+    end
+  end
+
+  # The old names a formula or cask had before getting migrated to the current tap.
+  sig { params(current_tap: Tap, name_or_token: String).returns(T::Array[String]) }
+  def self.tap_migration_oldnames(current_tap, name_or_token)
+    key = "#{current_tap}/#{name_or_token}"
+
+    Tap.each_with_object([]) do |tap, array|
+      next unless (renames = tap.reverse_tap_migrations_renames[key])
+
+      array.concat(renames)
+    end
+  end
+
   # Array with autobump names
   sig { returns(T::Array[String]) }
   def autobump
@@ -920,27 +933,42 @@ class Tap
     other = Tap.fetch(other) if other.is_a?(String)
     other.is_a?(self.class) && name == other.name
   end
+  alias eql? ==
 
-  def self.each(&block)
+  sig { returns(Integer) }
-    return to_enum unless block
+  def hash
+    [self.class, name].hash
+  end
 
-    installed_taps = if TAP_DIRECTORY.directory?
+  # All locally installed taps.
-      TAP_DIRECTORY.subdirs
+  sig { returns(T::Array[Tap]) }
-                   .flat_map(&:subdirs)
+  def self.installed
-                   .map(&method(:from_path))
+    cache[:installed] ||= if TAP_DIRECTORY.directory?
+      TAP_DIRECTORY.subdirs.flat_map(&:subdirs).map(&method(:from_path))
     else
       []
     end
+  end
 
-    available_taps = if Homebrew::EnvConfig.no_install_from_api?
+  # All locally installed and core taps. Core taps might not be installed locally when using the API.
-      installed_taps
+  sig { returns(T::Array[Tap]) }
+  def self.all
+    cache[:all] ||= begin
+      core_taps = [
+        CoreTap.instance,
+        (CoreCaskTap.instance if OS.mac?), # rubocop:disable Homebrew/MoveToExtendOS
+      ].compact
+
+      installed | core_taps
+    end
+  end
+
+  def self.each(&block)
+    if Homebrew::EnvConfig.no_install_from_api?
+      installed.each(&block)
     else
-      default_taps = T.let([CoreTap.instance], T::Array[Tap])
+      all.each(&block)
-      default_taps << CoreCaskTap.instance if OS.mac? # rubocop:disable Homebrew/MoveToExtendOS
+    end
-      installed_taps + default_taps
-    end.sort_by(&:name).uniq
-
-    available_taps.each(&block)
   end
 
   # An array of all installed {Tap} names.

Library/Homebrew/test/api/internal_tap_json/formula_spec.rb

@@ -33,7 +33,7 @@ RSpec.describe "Internal Tap JSON -- Formula" do
         .with("internal/v3/homebrew-core.jws.json")
         .and_return([JSON.parse(internal_tap_json), false])
 
-      # `Tap.reverse_tap_migrations_renames` looks for renames in every
+      # `Tap.tap_migration_oldnames` looks for renames in every
       # tap so `CoreCaskTap.tap_migrations` gets called and tries to
       # fetch stuff from the API. This just avoids errors.
       allow(Homebrew::API).to receive(:fetch_json_api_file)

Library/Homebrew/test/spec_helper.rb

@@ -204,7 +204,7 @@ RSpec.configure do |config|
   config.around do |example|
     Homebrew.raise_deprecation_exceptions = true
 
-    Tap.each(&:clear_cache)
+    Tap.installed.each(&:clear_cache)
     Cachable::Registry.clear_all_caches
     FormulaInstaller.clear_attempted
     FormulaInstaller.clear_installed
@@ -244,9 +244,6 @@ RSpec.configure do |config|
     rescue SystemExit => e
       example.example.set_exception(e)
     ensure
-      # This depends on `HOMEBREW_NO_INSTALL_FROM_API`.
-      Tap.each(&:clear_cache)
-
       ENV.replace(@__env)
       Context.current = Context::ContextStruct.new
 
@@ -257,6 +254,7 @@ RSpec.configure do |config|
       @__stderr.close
       @__stdin.close
 
+      Tap.all.each(&:clear_cache)
       Cachable::Registry.clear_all_caches
 
       FileUtils.rm_rf [

Library/Homebrew/test/tap_spec.rb

@@ -470,10 +470,52 @@ RSpec.describe Tap do
     expect(homebrew_foo_tap.config[:foo]).to be_nil
   end
 
-  describe "#each" do
+  describe ".each" do
     it "returns an enumerator if no block is passed" do
       expect(described_class.each).to be_an_instance_of(Enumerator)
     end
+
+    context "when the core tap is not installed" do
+      around do |example|
+        FileUtils.rm_rf CoreTap.instance.path
+        example.run
+      ensure
+        (CoreTap.instance.path/"Formula").mkpath
+      end
+
+      it "includes the core tap with the api" do
+        ENV.delete("HOMEBREW_NO_INSTALL_FROM_API")
+        expect(described_class.to_a).to include(CoreTap.instance)
+      end
+
+      it "omits the core tap without the api" do
+        ENV["HOMEBREW_NO_INSTALL_FROM_API"] = "1"
+        expect(described_class.to_a).not_to include(CoreTap.instance)
+      end
+    end
+  end
+
+  describe ".installed" do
+    it "includes only installed taps" do
+      expect(described_class.installed)
+        .to contain_exactly(CoreTap.instance, described_class.fetch("homebrew/foo"))
+    end
+  end
+
+  describe ".all" do
+    it "includes the core and cask taps by default", :needs_macos do
+      expect(described_class.all).to contain_exactly(
+        CoreTap.instance,
+        CoreCaskTap.instance,
+        described_class.fetch("homebrew/foo"),
+        described_class.fetch("third-party/tap"),
+      )
+    end
+
+    it "includes the core tap and excludes the cask tap by default", :needs_linux do
+      expect(described_class.all)
+        .to contain_exactly(CoreTap.instance, described_class.fetch("homebrew/foo"))
+    end
   end
 
   describe "Formula Lists" do
@@ -495,6 +537,47 @@ RSpec.describe Tap do
       end
     end
 
+    describe "tap migration renames" do
+      before do
+        (path/"tap_migrations.json").write <<~JSON
+          {
+            "adobe-air-sdk": "homebrew/cask",
+            "app-engine-go-32": "homebrew/cask/google-cloud-sdk",
+            "app-engine-go-64": "homebrew/cask/google-cloud-sdk",
+            "gimp": "homebrew/cask",
+            "horndis": "homebrew/cask",
+            "inkscape": "homebrew/cask",
+            "schismtracker": "homebrew/cask/schism-tracker"
+          }
+        JSON
+      end
+
+      describe "#reverse_tap_migration_renames" do
+        it "returns the expected hash" do
+          expect(homebrew_foo_tap.reverse_tap_migrations_renames).to eq({
+            "homebrew/cask/google-cloud-sdk" => %w[app-engine-go-32 app-engine-go-64],
+            "homebrew/cask/schism-tracker"   => %w[schismtracker],
+          })
+        end
+      end
+
+      describe ".tap_migration_oldnames" do
+        let(:cask_tap) { CoreCaskTap.instance }
+        let(:core_tap) { CoreTap.instance }
+
+        it "returns expected renames" do
+          [
+            [cask_tap, "gimp", []],
+            [core_tap, "schism-tracker", []],
+            [cask_tap, "schism-tracker", %w[schismtracker]],
+            [cask_tap, "google-cloud-sdk", %w[app-engine-go-32 app-engine-go-64]],
+          ].each do |tap, name, result|
+            expect(described_class.tap_migration_oldnames(tap, name)).to eq(result)
+          end
+        end
+      end
+    end
+
     describe "#audit_exceptions" do
       it "returns the audit_exceptions hash" do
         setup_tap_files