Skip to content

Commit

Permalink
Merge pull request #16863 from apainintheneck/memoize-installed-tap-l…
Browse files Browse the repository at this point in the history
…oading-v2

Memoize installed tap loading v2
  • Loading branch information
MikeMcQuaid committed Mar 12, 2024
2 parents 2d77465 + 0844273 commit b1990ed
Show file tree
Hide file tree
Showing 12 changed files with 157 additions and 48 deletions.
2 changes: 1 addition & 1 deletion Library/Homebrew/cask/cask.rb
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ def initialize(token, sourcefile_path: nil, source: nil, tap: nil, loaded_from_a
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
[]
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/cmd/readall.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ def readall
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
Expand Down
4 changes: 2 additions & 2 deletions Library/Homebrew/cmd/tap.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,12 @@ def tap
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
Expand Down
4 changes: 2 additions & 2 deletions Library/Homebrew/cmd/update-report.rb
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ def output_update_report
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?

Expand Down Expand Up @@ -254,7 +254,7 @@ def output_update_report

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?
Expand Down
6 changes: 3 additions & 3 deletions Library/Homebrew/completions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,15 +72,15 @@ module Completions
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

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
Expand All @@ -94,7 +94,7 @@ def self.link_completions?

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|
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/dev-cmd/audit.rb
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ def self.audit

# 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)

Expand Down
4 changes: 2 additions & 2 deletions Library/Homebrew/diagnostic.rb
Original file line number Diff line number Diff line change
Expand Up @@ -545,7 +545,7 @@ def check_tap_git_branch
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}"
Expand Down Expand Up @@ -794,7 +794,7 @@ def check_for_external_cmd_name_conflict

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?
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/formula.rb
Original file line number Diff line number Diff line change
Expand Up @@ -552,7 +552,7 @@ def oldname
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
[]
Expand Down
86 changes: 57 additions & 29 deletions Library/Homebrew/tap.rb
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ def clear_cache
@command_files = nil

@tap_migrations = nil
@reverse_tap_migrations_renames = nil

@audit_exceptions = nil
@style_exceptions = nil
Expand Down Expand Up @@ -393,6 +394,7 @@ def install(quiet: false, clone_target: nil,
end

clear_cache
Tap.clear_cache

$stderr.ohai "Tapping #{name}" unless quiet
args = %W[clone #{requested_remote} #{path}]
Expand Down Expand Up @@ -535,6 +537,7 @@ def uninstall(manual: false)

Commands.rebuild_commands_completion_list
clear_cache
Tap.clear_cache

return if !manual || !official?

Expand Down Expand Up @@ -840,21 +843,6 @@ def formula_reverse_renames
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
Expand All @@ -865,6 +853,31 @@ def tap_migrations
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
Expand Down Expand Up @@ -920,27 +933,42 @@ def ==(other)
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

# 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

available_taps = if Homebrew::EnvConfig.no_install_from_api?
installed_taps
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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
.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)
Expand Down
6 changes: 2 additions & 4 deletions Library/Homebrew/test/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@
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
Expand Down Expand Up @@ -244,9 +244,6 @@
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

Expand All @@ -257,6 +254,7 @@
@__stderr.close
@__stdin.close

Tap.all.each(&:clear_cache)
Cachable::Registry.clear_all_caches

FileUtils.rm_rf [
Expand Down
85 changes: 84 additions & 1 deletion Library/Homebrew/test/tap_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -470,10 +470,52 @@ def setup_completion(link:)
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
Expand All @@ -495,6 +537,47 @@ def setup_completion(link:)
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
Expand Down

0 comments on commit b1990ed

Please sign in to comment.