Refactor method to remove extra tap requires

We were selectively requiring the tap.rb file in a few places for
performance reasons. The main method we were referencing was the
`Tap.cmd_directories` method which uses `Pathname` and the `TAP_DIRECTORY`
constant internally. `Tap.cmd_directories` is mostly used in the `Commands`
module and that is loaded very early on in the program so it made sense
to move that command to that module. To facilitate that I moved the
`TAP_DIRECTORY` constant to the top-level and renamed it to
`HOMEBREW_TAP_DIRECTORY`. It now lies in the tap_constants.rb file.

A nice bonus of this refactor is that it speeds up loading external
commands since the tap.rb file is no longer required by default in
those cases.
This commit is contained in:
apainintheneck 2024-08-10 13:35:20 -07:00
parent 8e08a698d1
commit a3e917afe1
17 changed files with 37 additions and 47 deletions

View File

@ -65,10 +65,8 @@ begin
internal_cmd = Commands.valid_internal_cmd?(cmd) || Commands.valid_internal_dev_cmd?(cmd) if cmd internal_cmd = Commands.valid_internal_cmd?(cmd) || Commands.valid_internal_dev_cmd?(cmd) if cmd
unless internal_cmd unless internal_cmd
require "tap"
# Add contributed commands to PATH before checking. # Add contributed commands to PATH before checking.
homebrew_path.append(Tap.cmd_directories) homebrew_path.append(Commands.cmd_directories)
# External commands expect a normal PATH # External commands expect a normal PATH
ENV["PATH"] = homebrew_path.to_s ENV["PATH"] = homebrew_path.to_s

View File

@ -58,7 +58,7 @@ module Homebrew
info += ", #{private_count} private" info += ", #{private_count} private"
info += ", #{Utils.pluralize("formula", formula_count, plural: "e", include_count: true)}" info += ", #{Utils.pluralize("formula", formula_count, plural: "e", include_count: true)}"
info += ", #{Utils.pluralize("command", command_count, include_count: true)}" info += ", #{Utils.pluralize("command", command_count, include_count: true)}"
info += ", #{Tap::TAP_DIRECTORY.dup.abv}" if Tap::TAP_DIRECTORY.directory? info += ", #{HOMEBREW_TAP_DIRECTORY.dup.abv}" if HOMEBREW_TAP_DIRECTORY.directory?
puts info puts info
else else
info = "" info = ""

View File

@ -70,23 +70,17 @@ module Commands
# Ruby commands which can be `require`d without being run. # Ruby commands which can be `require`d without being run.
def self.external_ruby_v2_cmd_path(cmd) def self.external_ruby_v2_cmd_path(cmd)
require "tap" path = which("#{cmd}.rb", cmd_directories)
path = which("#{cmd}.rb", Tap.cmd_directories)
path if require?(path) path if require?(path)
end end
# Ruby commands which are run by being `require`d. # Ruby commands which are run by being `require`d.
def self.external_ruby_cmd_path(cmd) def self.external_ruby_cmd_path(cmd)
require "tap" which("brew-#{cmd}.rb", PATH.new(ENV.fetch("PATH")).append(cmd_directories))
which("brew-#{cmd}.rb", PATH.new(ENV.fetch("PATH")).append(Tap.cmd_directories))
end end
def self.external_cmd_path(cmd) def self.external_cmd_path(cmd)
require "tap" which("brew-#{cmd}", PATH.new(ENV.fetch("PATH")).append(cmd_directories))
which("brew-#{cmd}", PATH.new(ENV.fetch("PATH")).append(Tap.cmd_directories))
end end
def self.path(cmd) def self.path(cmd)
@ -107,6 +101,12 @@ module Commands
cmds.sort cmds.sort
end end
# An array of all tap cmd directory {Pathname}s.
sig { returns(T::Array[Pathname]) }
def self.cmd_directories
Pathname.glob HOMEBREW_TAP_DIRECTORY/"*/*/cmd"
end
def self.internal_commands_paths def self.internal_commands_paths
find_commands HOMEBREW_CMD_PATH find_commands HOMEBREW_CMD_PATH
end end
@ -144,9 +144,7 @@ module Commands
end end
def self.external_commands def self.external_commands
require "tap" cmd_directories.flat_map do |path|
Tap.cmd_directories.flat_map do |path|
find_commands(path).select(&:executable?) find_commands(path).select(&:executable?)
.map { basename_without_extension(_1) } .map { basename_without_extension(_1) }
.map { |p| p.to_s.delete_prefix("brew-").strip } .map { |p| p.to_s.delete_prefix("brew-").strip }

View File

@ -764,7 +764,7 @@ module Homebrew
end end
def check_for_external_cmd_name_conflict def check_for_external_cmd_name_conflict
cmds = Tap.cmd_directories.flat_map { |p| Dir["#{p}/brew-*"] }.uniq cmds = Commands.cmd_directories.flat_map { |p| Dir["#{p}/brew-*"] }.uniq
cmds = cmds.select { |cmd| File.file?(cmd) && File.executable?(cmd) } cmds = cmds.select { |cmd| File.file?(cmd) && File.executable?(cmd) }
cmd_map = {} cmd_map = {}
cmds.each do |cmd| cmds.each do |cmd|

View File

@ -14,8 +14,6 @@ require "settings"
class Tap class Tap
extend Cachable extend Cachable
TAP_DIRECTORY = (HOMEBREW_LIBRARY/"Taps").freeze
HOMEBREW_TAP_CASK_RENAMES_FILE = "cask_renames.json" HOMEBREW_TAP_CASK_RENAMES_FILE = "cask_renames.json"
private_constant :HOMEBREW_TAP_CASK_RENAMES_FILE private_constant :HOMEBREW_TAP_CASK_RENAMES_FILE
HOMEBREW_TAP_FORMULA_RENAMES_FILE = "formula_renames.json" HOMEBREW_TAP_FORMULA_RENAMES_FILE = "formula_renames.json"
@ -201,7 +199,7 @@ class Tap
@repository = repository @repository = repository
@name = "#{@user}/#{@repository}".downcase @name = "#{@user}/#{@repository}".downcase
@full_name = "#{@user}/homebrew-#{@repository}" @full_name = "#{@user}/homebrew-#{@repository}"
@path = TAP_DIRECTORY/@full_name.downcase @path = HOMEBREW_TAP_DIRECTORY/@full_name.downcase
@git_repository = GitRepository.new(@path) @git_repository = GitRepository.new(@path)
end end
@ -286,7 +284,7 @@ class Tap
sig { returns(String) } sig { returns(String) }
def repository_var_suffix def repository_var_suffix
@repository_var_suffix ||= path.to_s @repository_var_suffix ||= path.to_s
.delete_prefix(TAP_DIRECTORY.to_s) .delete_prefix(HOMEBREW_TAP_DIRECTORY.to_s)
.tr("^A-Za-z0-9", "_") .tr("^A-Za-z0-9", "_")
.upcase .upcase
end end
@ -1021,8 +1019,8 @@ class Tap
# @api public # @api public
sig { returns(T::Array[Tap]) } sig { returns(T::Array[Tap]) }
def self.installed def self.installed
cache[:installed] ||= if TAP_DIRECTORY.directory? cache[:installed] ||= if HOMEBREW_TAP_DIRECTORY.directory?
TAP_DIRECTORY.subdirs.flat_map(&:subdirs).map { from_path(_1) } HOMEBREW_TAP_DIRECTORY.subdirs.flat_map(&:subdirs).map { from_path(_1) }
else else
[] []
end end
@ -1060,12 +1058,6 @@ class Tap
map(&:name).sort map(&:name).sort
end end
# An array of all tap cmd directory {Pathname}s.
sig { returns(T::Array[Pathname]) }
def self.cmd_directories
Pathname.glob TAP_DIRECTORY/"*/*/cmd"
end
# An array of official taps that have been manually untapped # An array of official taps that have been manually untapped
sig { returns(T::Array[String]) } sig { returns(T::Array[String]) }
def self.untapped_official_taps def self.untapped_official_taps

View File

@ -1,6 +1,8 @@
# typed: strict # typed: strict
# frozen_string_literal: true # frozen_string_literal: true
HOMEBREW_TAP_DIRECTORY = T.let((HOMEBREW_LIBRARY/"Taps").freeze, Pathname)
# Match a formula name. # Match a formula name.
HOMEBREW_TAP_FORMULA_NAME_REGEX = T.let(/(?<name>[\w+\-.@]+)/, Regexp) HOMEBREW_TAP_FORMULA_NAME_REGEX = T.let(/(?<name>[\w+\-.@]+)/, Regexp)
# Match taps' formulae, e.g. `someuser/sometap/someformula`. # Match taps' formulae, e.g. `someuser/sometap/someformula`.

View File

@ -8,10 +8,10 @@ RSpec.describe "Internal Tap JSON -- Formula", type: :system do
context "when generating JSON", :needs_macos do context "when generating JSON", :needs_macos do
before do before do
cp_r(TEST_FIXTURE_DIR/"internal_tap_json/homebrew-core", Tap::TAP_DIRECTORY/"homebrew") cp_r(TEST_FIXTURE_DIR/"internal_tap_json/homebrew-core", HOMEBREW_TAP_DIRECTORY/"homebrew")
# NOTE: Symlinks can't be copied recursively so we create them manually here. # NOTE: Symlinks can't be copied recursively so we create them manually here.
(Tap::TAP_DIRECTORY/"homebrew/homebrew-core").tap do |core_tap| (HOMEBREW_TAP_DIRECTORY/"homebrew/homebrew-core").tap do |core_tap|
mkdir(core_tap/"Aliases") mkdir(core_tap/"Aliases")
ln_s(core_tap/"Formula/f/fennel.rb", core_tap/"Aliases/fennel-lang") ln_s(core_tap/"Formula/f/fennel.rb", core_tap/"Aliases/fennel-lang")
ln_s(core_tap/"Formula/p/ponyc.rb", core_tap/"Aliases/ponyc-lang") ln_s(core_tap/"Formula/p/ponyc.rb", core_tap/"Aliases/ponyc-lang")

View File

@ -53,7 +53,7 @@ RSpec.describe Commands do
FileUtils.touch "#{dir}/brew-t4" FileUtils.touch "#{dir}/brew-t4"
allow(Tap).to receive(:cmd_directories).and_return([dir]) allow(described_class).to receive(:cmd_directories).and_return([dir])
cmds = described_class.external_commands cmds = described_class.external_commands

View File

@ -8,7 +8,7 @@ RSpec.describe Homebrew::DevCmd::Extract do
context "when extracting a formula" do context "when extracting a formula" do
let!(:target) do let!(:target) do
path = Tap::TAP_DIRECTORY/"homebrew/homebrew-foo" path = HOMEBREW_TAP_DIRECTORY/"homebrew/homebrew-foo"
(path/"Formula").mkpath (path/"Formula").mkpath
target = Tap.from_path(path) target = Tap.from_path(path)
core_tap = CoreTap.instance core_tap = CoreTap.instance

View File

@ -80,7 +80,7 @@ RSpec.describe Homebrew::DevCmd::PrPull do
let(:tap) { Tap.fetch("Homebrew", "foo") } let(:tap) { Tap.fetch("Homebrew", "foo") }
let(:formula_file) { tap.path/"Formula/foo.rb" } let(:formula_file) { tap.path/"Formula/foo.rb" }
let(:cask_file) { tap.cask_dir/"food.rb" } let(:cask_file) { tap.cask_dir/"food.rb" }
let(:path) { Pathname(Tap::TAP_DIRECTORY/"homebrew/homebrew-foo") } let(:path) { Pathname(HOMEBREW_TAP_DIRECTORY/"homebrew/homebrew-foo") }
it_behaves_like "parseable arguments" it_behaves_like "parseable arguments"

View File

@ -101,7 +101,7 @@ RSpec.describe Homebrew::Diagnostic::Checks do
FileUtils.chmod 0755, cmd FileUtils.chmod 0755, cmd
end end
allow(Tap).to receive(:cmd_directories).and_return([path1, path2]) allow(Commands).to receive(:cmd_directories).and_return([path1, path2])
expect(checks.check_for_external_cmd_name_conflict) expect(checks.check_for_external_cmd_name_conflict)
.to match("brew-foo") .to match("brew-foo")

View File

@ -11,9 +11,9 @@ RSpec.describe Homebrew::FormulaAuditor do
@count += 1 @count += 1
end end
let(:formula_subpath) { "Formula/foo#{foo_version}.rb" } let(:formula_subpath) { "Formula/foo#{foo_version}.rb" }
let(:origin_tap_path) { Tap::TAP_DIRECTORY/"homebrew/homebrew-foo" } let(:origin_tap_path) { HOMEBREW_TAP_DIRECTORY/"homebrew/homebrew-foo" }
let(:origin_formula_path) { origin_tap_path/formula_subpath } let(:origin_formula_path) { origin_tap_path/formula_subpath }
let(:tap_path) { Tap::TAP_DIRECTORY/"homebrew/homebrew-bar" } let(:tap_path) { HOMEBREW_TAP_DIRECTORY/"homebrew/homebrew-bar" }
let(:formula_path) { tap_path/formula_subpath } let(:formula_path) { tap_path/formula_subpath }
def formula_auditor(name, text, options = {}) def formula_auditor(name, text, options = {})

View File

@ -751,7 +751,7 @@ RSpec.describe Formulary do
end end
after do after do
FileUtils.rm_rf Tap::TAP_DIRECTORY/"another" FileUtils.rm_rf HOMEBREW_TAP_DIRECTORY/"another"
end end
# FIXME # FIXME

View File

@ -34,7 +34,7 @@ RSpec.describe Homebrew::MissingFormula do
subject { described_class.tap_migration_reason(formula) } subject { described_class.tap_migration_reason(formula) }
before do before do
tap_path = Tap::TAP_DIRECTORY/"homebrew/homebrew-foo" tap_path = HOMEBREW_TAP_DIRECTORY/"homebrew/homebrew-foo"
tap_path.mkpath tap_path.mkpath
(tap_path/"tap_migrations.json").write <<~JSON (tap_path/"tap_migrations.json").write <<~JSON
{ "migrated-formula": "homebrew/bar" } { "migrated-formula": "homebrew/bar" }
@ -58,7 +58,7 @@ RSpec.describe Homebrew::MissingFormula do
subject { described_class.deleted_reason(formula, silent: true) } subject { described_class.deleted_reason(formula, silent: true) }
before do before do
tap_path = Tap::TAP_DIRECTORY/"homebrew/homebrew-foo" tap_path = HOMEBREW_TAP_DIRECTORY/"homebrew/homebrew-foo"
(tap_path/"Formula").mkpath (tap_path/"Formula").mkpath
(tap_path/"Formula/deleted-formula.rb").write "placeholder" (tap_path/"Formula/deleted-formula.rb").write "placeholder"
ENV.delete "GIT_AUTHOR_DATE" ENV.delete "GIT_AUTHOR_DATE"

View File

@ -5,7 +5,7 @@ require "rubocops/lines"
RSpec.describe RuboCop::Cop::FormulaAuditStrict::MakeCheck do RSpec.describe RuboCop::Cop::FormulaAuditStrict::MakeCheck do
subject(:cop) { described_class.new } subject(:cop) { described_class.new }
let(:path) { Tap::TAP_DIRECTORY/"homebrew/homebrew-core" } let(:path) { HOMEBREW_TAP_DIRECTORY/"homebrew/homebrew-core" }
before do before do
path.mkpath path.mkpath

View File

@ -210,7 +210,7 @@ RSpec.shared_context "integration test" do # rubocop:disable RSpec/ContextWordin
end end
def setup_test_tap def setup_test_tap
path = Tap::TAP_DIRECTORY/"homebrew/homebrew-foo" path = HOMEBREW_TAP_DIRECTORY/"homebrew/homebrew-foo"
path.mkpath path.mkpath
path.cd do path.cd do
system "git", "init" system "git", "init"

View File

@ -8,7 +8,7 @@ RSpec.describe Tap do
subject(:homebrew_foo_tap) { described_class.fetch("Homebrew", "foo") } subject(:homebrew_foo_tap) { described_class.fetch("Homebrew", "foo") }
let(:path) { Tap::TAP_DIRECTORY/"homebrew/homebrew-foo" } let(:path) { HOMEBREW_TAP_DIRECTORY/"homebrew/homebrew-foo" }
let(:formula_file) { path/"Formula/foo.rb" } let(:formula_file) { path/"Formula/foo.rb" }
let(:alias_file) { path/"Aliases/bar" } let(:alias_file) { path/"Aliases/bar" }
let(:cmd_file) { path/"cmd/brew-tap-cmd.rb" } let(:cmd_file) { path/"cmd/brew-tap-cmd.rb" }
@ -172,7 +172,7 @@ RSpec.describe Tap do
specify "#issues_url" do specify "#issues_url" do
t = described_class.fetch("someone", "foo") t = described_class.fetch("someone", "foo")
path = Tap::TAP_DIRECTORY/"someone/homebrew-foo" path = HOMEBREW_TAP_DIRECTORY/"someone/homebrew-foo"
path.mkpath path.mkpath
cd path do cd path do
system "git", "init" system "git", "init"
@ -182,7 +182,7 @@ RSpec.describe Tap do
expect(t.issues_url).to eq("https://github.com/someone/homebrew-foo/issues") expect(t.issues_url).to eq("https://github.com/someone/homebrew-foo/issues")
expect(homebrew_foo_tap.issues_url).to eq("https://github.com/Homebrew/homebrew-foo/issues") expect(homebrew_foo_tap.issues_url).to eq("https://github.com/Homebrew/homebrew-foo/issues")
(Tap::TAP_DIRECTORY/"someone/homebrew-no-git").mkpath (HOMEBREW_TAP_DIRECTORY/"someone/homebrew-no-git").mkpath
expect(described_class.fetch("someone", "no-git").issues_url).to be_nil expect(described_class.fetch("someone", "no-git").issues_url).to be_nil
ensure ensure
path.parent.rmtree path.parent.rmtree
@ -378,7 +378,7 @@ RSpec.describe Tap do
end.to raise_error(ErrorDuringExecution) end.to raise_error(ErrorDuringExecution)
expect(tap).not_to be_installed expect(tap).not_to be_installed
expect(Tap::TAP_DIRECTORY/"user").not_to exist expect(HOMEBREW_TAP_DIRECTORY/"user").not_to exist
end end
end end
@ -652,7 +652,7 @@ RSpec.describe Tap do
end end
specify "files" do specify "files" do
path = Tap::TAP_DIRECTORY/"homebrew/homebrew-core" path = HOMEBREW_TAP_DIRECTORY/"homebrew/homebrew-core"
formula_file = core_tap.formula_dir/"foo.rb" formula_file = core_tap.formula_dir/"foo.rb"
core_tap.formula_dir.mkpath core_tap.formula_dir.mkpath
formula_file.write <<~RUBY formula_file.write <<~RUBY