From 3834ef1b738b6c0de29dfa485def8e8a84514713 Mon Sep 17 00:00:00 2001 From: apainintheneck Date: Tue, 5 Mar 2024 23:53:52 -0800 Subject: [PATCH] tap: cache more things at the Tap level I added two new methods to cache both installed and all taps. All taps includes core taps no matter if they're installed locally since they're always provided by the API anyway. This makes it easier to cache `Tap.each` while making the code easier to reason about. It also will be useful because we'll be able to avoid the `Tap.select(&:installed?` pattern that has recently invaded the codebase. Note: I also stopped clearing all tap instance caches before tests. Running `Tap.each` would cache existing taps which would lead to unexpected behavior since the only existing tap before each test is the core tap. This is the only tap whose directory is not cleaned up between tests so we just clear it's cache directly. We also now clear all tap instances after tests as well regardless of whether the API was used that time. --- Library/Homebrew/tap.rb | 45 +++++++++++++++++++--------- Library/Homebrew/test/spec_helper.rb | 6 ++-- 2 files changed, 33 insertions(+), 18 deletions(-) diff --git a/Library/Homebrew/tap.rb b/Library/Homebrew/tap.rb index 8e779c6ecf..2cf5efc378 100644 --- a/Library/Homebrew/tap.rb +++ b/Library/Homebrew/tap.rb @@ -402,6 +402,7 @@ class Tap end clear_cache + Tap.clear_cache $stderr.ohai "Tapping #{name}" unless quiet args = %W[clone #{requested_remote} #{path}] @@ -546,6 +547,7 @@ class Tap Commands.rebuild_commands_completion_list clear_cache + Tap.clear_cache return if !manual || !official? @@ -931,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) - return to_enum unless block + sig { returns(Integer) } + def hash + [self.class, name].hash + end - installed_taps = if TAP_DIRECTORY.directory? - TAP_DIRECTORY.subdirs - .flat_map(&:subdirs) - .map(&method(:from_path)) + # All locally installed taps. + sig { returns(T::Array[Tap]) } + def self.installed + 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? - installed_taps + # All locally installed and core taps. Core taps might not be installed locally when using the API. + 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]) - default_taps << CoreCaskTap.instance if OS.mac? # rubocop:disable Homebrew/MoveToExtendOS - installed_taps + default_taps - end.sort_by(&:name).uniq - - available_taps.each(&block) + all.each(&block) + end end # An array of all installed {Tap} names. diff --git a/Library/Homebrew/test/spec_helper.rb b/Library/Homebrew/test/spec_helper.rb index 48068f8f37..b8af2d24e9 100644 --- a/Library/Homebrew/test/spec_helper.rb +++ b/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 [