Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor method to remove extra tap requires #18010

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions Library/Homebrew/brew.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,8 @@
internal_cmd = Commands.valid_internal_cmd?(cmd) || Commands.valid_internal_dev_cmd?(cmd) if cmd

unless internal_cmd
require "tap"

# Add contributed commands to PATH before checking.
homebrew_path.append(Tap.cmd_directories)
homebrew_path.append(Commands.tap_cmd_directories)

# External commands expect a normal PATH
ENV["PATH"] = homebrew_path.to_s
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/cmd/tap-info.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ def print_tap_info(taps)
info += ", #{private_count} private"
info += ", #{Utils.pluralize("formula", formula_count, plural: "e", 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
else
info = ""
Expand Down
22 changes: 10 additions & 12 deletions Library/Homebrew/commands.rb
Original file line number Diff line number Diff line change
Expand Up @@ -70,23 +70,17 @@ def self.internal_dev_cmd_path(cmd)

# Ruby commands which can be `require`d without being run.
def self.external_ruby_v2_cmd_path(cmd)
require "tap"

path = which("#{cmd}.rb", Tap.cmd_directories)
path = which("#{cmd}.rb", tap_cmd_directories)
path if require?(path)
end

# Ruby commands which are run by being `require`d.
def self.external_ruby_cmd_path(cmd)
require "tap"

which("brew-#{cmd}.rb", PATH.new(ENV.fetch("PATH")).append(Tap.cmd_directories))
which("brew-#{cmd}.rb", PATH.new(ENV.fetch("PATH")).append(tap_cmd_directories))
end

def self.external_cmd_path(cmd)
require "tap"

which("brew-#{cmd}", PATH.new(ENV.fetch("PATH")).append(Tap.cmd_directories))
which("brew-#{cmd}", PATH.new(ENV.fetch("PATH")).append(tap_cmd_directories))
end

def self.path(cmd)
Expand All @@ -107,6 +101,12 @@ def self.commands(external: true, aliases: false)
cmds.sort
end

# An array of all tap cmd directory {Pathname}s.
sig { returns(T::Array[Pathname]) }
def self.tap_cmd_directories
Pathname.glob HOMEBREW_TAP_DIRECTORY/"*/*/cmd"
end

def self.internal_commands_paths
find_commands HOMEBREW_CMD_PATH
end
Expand Down Expand Up @@ -144,9 +144,7 @@ def self.find_internal_commands(path)
end

def self.external_commands
require "tap"

Tap.cmd_directories.flat_map do |path|
tap_cmd_directories.flat_map do |path|
find_commands(path).select(&:executable?)
.map { basename_without_extension(_1) }
.map { |p| p.to_s.delete_prefix("brew-").strip }
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/diagnostic.rb
Original file line number Diff line number Diff line change
Expand Up @@ -764,7 +764,7 @@ def check_for_unlinked_but_not_keg_only
end

def check_for_external_cmd_name_conflict
cmds = Tap.cmd_directories.flat_map { |p| Dir["#{p}/brew-*"] }.uniq
cmds = Commands.tap_cmd_directories.flat_map { |p| Dir["#{p}/brew-*"] }.uniq
cmds = cmds.select { |cmd| File.file?(cmd) && File.executable?(cmd) }
cmd_map = {}
cmds.each do |cmd|
Expand Down
3 changes: 3 additions & 0 deletions Library/Homebrew/startup/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@
tmp.realpath
end.freeze

# Where installed taps live
HOMEBREW_TAP_DIRECTORY = (HOMEBREW_LIBRARY/"Taps").freeze

Check warning on line 55 in Library/Homebrew/startup/config.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/startup/config.rb#L55

Added line #L55 was not covered by tests

# The Ruby path and args to use for forked Ruby calls
HOMEBREW_RUBY_EXEC_ARGS = [
RUBY_PATH,
Expand Down
16 changes: 4 additions & 12 deletions Library/Homebrew/tap.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@
class Tap
extend Cachable

TAP_DIRECTORY = (HOMEBREW_LIBRARY/"Taps").freeze

HOMEBREW_TAP_CASK_RENAMES_FILE = "cask_renames.json"
private_constant :HOMEBREW_TAP_CASK_RENAMES_FILE
HOMEBREW_TAP_FORMULA_RENAMES_FILE = "formula_renames.json"
Expand Down Expand Up @@ -201,7 +199,7 @@ def initialize(user, repository)
@repository = repository
@name = "#{@user}/#{@repository}".downcase
@full_name = "#{@user}/homebrew-#{@repository}"
@path = TAP_DIRECTORY/@full_name.downcase
@path = HOMEBREW_TAP_DIRECTORY/@full_name.downcase
@git_repository = GitRepository.new(@path)
end

Expand Down Expand Up @@ -286,7 +284,7 @@ def default_remote
sig { returns(String) }
def repository_var_suffix
@repository_var_suffix ||= path.to_s
.delete_prefix(TAP_DIRECTORY.to_s)
.delete_prefix(HOMEBREW_TAP_DIRECTORY.to_s)
.tr("^A-Za-z0-9", "_")
.upcase
end
Expand Down Expand Up @@ -1021,8 +1019,8 @@ def hash
# @api public
sig { returns(T::Array[Tap]) }
def self.installed
cache[:installed] ||= if TAP_DIRECTORY.directory?
TAP_DIRECTORY.subdirs.flat_map(&:subdirs).map { from_path(_1) }
cache[:installed] ||= if HOMEBREW_TAP_DIRECTORY.directory?
HOMEBREW_TAP_DIRECTORY.subdirs.flat_map(&:subdirs).map { from_path(_1) }
else
[]
end
Expand Down Expand Up @@ -1060,12 +1058,6 @@ def self.names
map(&:name).sort
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
sig { returns(T::Array[String]) }
def self.untapped_official_taps
Expand Down
4 changes: 2 additions & 2 deletions Library/Homebrew/test/api/internal_tap_json/formula_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@

context "when generating JSON", :needs_macos 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.
(Tap::TAP_DIRECTORY/"homebrew/homebrew-core").tap do |core_tap|
(HOMEBREW_TAP_DIRECTORY/"homebrew/homebrew-core").tap do |core_tap|
mkdir(core_tap/"Aliases")
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")
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/test/commands_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@

FileUtils.touch "#{dir}/brew-t4"

allow(Tap).to receive(:cmd_directories).and_return([dir])
allow(described_class).to receive(:tap_cmd_directories).and_return([dir])

cmds = described_class.external_commands

Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/test/dev-cmd/extract_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

context "when extracting a formula" do
let!(:target) do
path = Tap::TAP_DIRECTORY/"homebrew/homebrew-foo"
path = HOMEBREW_TAP_DIRECTORY/"homebrew/homebrew-foo"
(path/"Formula").mkpath
target = Tap.from_path(path)
core_tap = CoreTap.instance
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/test/dev-cmd/pr-pull_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ class Foo < Formula
let(:tap) { Tap.fetch("Homebrew", "foo") }
let(:formula_file) { tap.path/"Formula/foo.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"

Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/test/diagnostic_checks_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@
FileUtils.chmod 0755, cmd
end

allow(Tap).to receive(:cmd_directories).and_return([path1, path2])
allow(Commands).to receive(:tap_cmd_directories).and_return([path1, path2])

expect(checks.check_for_external_cmd_name_conflict)
.to match("brew-foo")
Expand Down
4 changes: 2 additions & 2 deletions Library/Homebrew/test/formula_auditor_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@
@count += 1
end
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(:tap_path) { Tap::TAP_DIRECTORY/"homebrew/homebrew-bar" }
let(:tap_path) { HOMEBREW_TAP_DIRECTORY/"homebrew/homebrew-bar" }
let(:formula_path) { tap_path/formula_subpath }

def formula_auditor(name, text, options = {})
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/test/formulary_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -751,7 +751,7 @@ def formula_json_contents(extra_items = {})
end

after do
FileUtils.rm_rf Tap::TAP_DIRECTORY/"another"
FileUtils.rm_rf HOMEBREW_TAP_DIRECTORY/"another"
end

# FIXME
Expand Down
4 changes: 2 additions & 2 deletions Library/Homebrew/test/missing_formula_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
subject { described_class.tap_migration_reason(formula) }

before do
tap_path = Tap::TAP_DIRECTORY/"homebrew/homebrew-foo"
tap_path = HOMEBREW_TAP_DIRECTORY/"homebrew/homebrew-foo"
tap_path.mkpath
(tap_path/"tap_migrations.json").write <<~JSON
{ "migrated-formula": "homebrew/bar" }
Expand All @@ -58,7 +58,7 @@
subject { described_class.deleted_reason(formula, silent: true) }

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/deleted-formula.rb").write "placeholder"
ENV.delete "GIT_AUTHOR_DATE"
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/test/rubocops/text/make_check_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
RSpec.describe RuboCop::Cop::FormulaAuditStrict::MakeCheck do
subject(:cop) { described_class.new }

let(:path) { Tap::TAP_DIRECTORY/"homebrew/homebrew-core" }
let(:path) { HOMEBREW_TAP_DIRECTORY/"homebrew/homebrew-core" }

before do
path.mkpath
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ def install_test_formula(name, content = nil, build_bottle: false)
end

def setup_test_tap
path = Tap::TAP_DIRECTORY/"homebrew/homebrew-foo"
path = HOMEBREW_TAP_DIRECTORY/"homebrew/homebrew-foo"
path.mkpath
path.cd do
system "git", "init"
Expand Down
1 change: 1 addition & 0 deletions Library/Homebrew/test/support/lib/startup/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
HOMEBREW_CELLAR = (HOMEBREW_PREFIX.parent/"cellar").freeze
HOMEBREW_LOGS = (HOMEBREW_PREFIX.parent/"logs").freeze
HOMEBREW_TEMP = (HOMEBREW_PREFIX.parent/"temp").freeze
HOMEBREW_TAP_DIRECTORY = (HOMEBREW_LIBRARY/"Taps").freeze
HOMEBREW_RUBY_EXEC_ARGS = [
RUBY_PATH,
ENV.fetch("HOMEBREW_RUBY_WARNINGS"),
Expand Down
10 changes: 5 additions & 5 deletions Library/Homebrew/test/tap_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

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(:alias_file) { path/"Aliases/bar" }
let(:cmd_file) { path/"cmd/brew-tap-cmd.rb" }
Expand Down Expand Up @@ -172,7 +172,7 @@ def setup_completion(link:)

specify "#issues_url" do
t = described_class.fetch("someone", "foo")
path = Tap::TAP_DIRECTORY/"someone/homebrew-foo"
path = HOMEBREW_TAP_DIRECTORY/"someone/homebrew-foo"
path.mkpath
cd path do
system "git", "init"
Expand All @@ -182,7 +182,7 @@ def setup_completion(link:)
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")

(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
ensure
path.parent.rmtree
Expand Down Expand Up @@ -378,7 +378,7 @@ def setup_completion(link:)
end.to raise_error(ErrorDuringExecution)

expect(tap).not_to be_installed
expect(Tap::TAP_DIRECTORY/"user").not_to exist
expect(HOMEBREW_TAP_DIRECTORY/"user").not_to exist
end
end

Expand Down Expand Up @@ -652,7 +652,7 @@ def setup_completion(link:)
end

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"
core_tap.formula_dir.mkpath
formula_file.write <<~RUBY
Expand Down