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

cmd/readall: clean up todos #16011

Merged
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 1 addition & 4 deletions Library/Homebrew/cli/args.rb
Original file line number Diff line number Diff line change
Expand Up @@ -111,10 +111,7 @@
when :all
skip_invalid_combinations = true

[
*MacOSVersion::SYMBOLS.keys,
:linux,
]
OnSystem::ALL_OS_OPTIONS

Check warning on line 114 in Library/Homebrew/cli/args.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cli/args.rb#L114

Added line #L114 was not covered by tests
else
[os_sym]
end
Expand Down
5 changes: 2 additions & 3 deletions Library/Homebrew/cmd/readall.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@ def readall_args
def readall
args = readall_args.parse

odeprecated "--no-simulate", "nothing (i.e. not passing `--os` or `--arch`)" if args.no_simulate?

if args.syntax? && args.no_named?
scan_files = "#{HOMEBREW_LIBRARY_PATH}/**/*.rb"
ruby_files = Dir.glob(scan_files).grep_v(%r{/(vendor)/})
Expand All @@ -51,8 +49,8 @@ def readall
aliases: args.aliases?,
no_simulate: args.no_simulate?,
}
# TODO: Always pass this once `--os` and `--arch` are passed explicitly to `brew readall` in CI.
options[:os_arch_combinations] = args.os_arch_combinations if args.os || args.arch

taps = if args.no_named?
if !args.eval_all? && !Homebrew::EnvConfig.eval_all?
raise UsageError, "`brew readall` needs a tap or `--eval-all` passed or `HOMEBREW_EVAL_ALL` set!"
Expand All @@ -62,6 +60,7 @@ def readall
else
args.named.to_installed_taps
end

taps.each do |tap|
Homebrew.failed = true unless Readall.valid_tap?(tap, options)
end
Expand Down
2 changes: 2 additions & 0 deletions Library/Homebrew/extend/on_system.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
module OnSystem
ARCH_OPTIONS = [:intel, :arm].freeze
BASE_OS_OPTIONS = [:macos, :linux].freeze
ALL_OS_OPTIONS = [*MacOSVersion::SYMBOLS.keys, :linux].freeze
ALL_OS_ARCH_COMBINATIONS = ALL_OS_OPTIONS.product(ARCH_OPTIONS).freeze

sig { params(arch: Symbol).returns(T::Boolean) }
def self.arch_condition_met?(arch)
Expand Down
4 changes: 1 addition & 3 deletions Library/Homebrew/formula.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2398,11 +2398,9 @@ def to_hash_with_variations

variations = {}

os_versions = [*MacOSVersion::SYMBOLS.keys, :linux]

if path.exist? && (self.class.on_system_blocks_exist? || @on_system_blocks_exist)
formula_contents = path.read
os_versions.product(OnSystem::ARCH_OPTIONS).each do |os, arch|
OnSystem::ALL_OS_ARCH_COMBINATIONS.each do |os, arch|
bottle_tag = Utils::Bottles::Tag.new(system: os, arch: arch)
next unless bottle_tag.valid_combination?

Expand Down
6 changes: 2 additions & 4 deletions Library/Homebrew/readall.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,20 +66,18 @@ def valid_casks?(_casks, os_name: nil, arch: nil)
true
end

def valid_tap?(tap, aliases: false, no_simulate: false, os_arch_combinations: nil)
def valid_tap?(tap, aliases: false, no_simulate: false, os_arch_combinations: OnSystem::ALL_OS_ARCH_COMBINATIONS)
success = true

if aliases
valid_aliases = valid_aliases?(tap.alias_dir, tap.formula_dir)
success = false unless valid_aliases
end

if no_simulate
success = false unless valid_formulae?(tap.formula_files)
success = false unless valid_casks?(tap.cask_files)
else
# TODO: Remove this default case once `--os` and `--arch` are passed explicitly to `brew readall` in CI.
os_arch_combinations ||= [*MacOSVersion::SYMBOLS.keys, :linux].product(OnSystem::ARCH_OPTIONS)

os_arch_combinations.each do |os, arch|
bottle_tag = Utils::Bottles::Tag.new(system: os, arch: arch)
next unless bottle_tag.valid_combination?
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/tap.rb
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ def install(quiet: false, clone_target: nil, force_auto_update: nil,
begin
safe_system "git", *args

if verify && !Readall.valid_tap?(self, aliases: true) && !Homebrew::EnvConfig.developer?
if verify && !Homebrew::EnvConfig.developer? && !Readall.valid_tap?(self, aliases: true)
raise "Cannot tap #{name}: invalid syntax in tap!"
end
rescue Interrupt, RuntimeError
Expand Down
11 changes: 3 additions & 8 deletions Library/Homebrew/test/formula_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1027,14 +1027,9 @@ class FooVariations < Formula
end

before do
# Use a more limited symbols list to shorten the variations hash
symbols = {
monterey: "12",
big_sur: "11",
catalina: "10.15",
mojave: "10.14",
}
stub_const("MacOSVersion::SYMBOLS", symbols)
# Use a more limited os list to shorten the variations hash
os_list = [:monterey, :big_sur, :catalina, :mojave, :linux]
stub_const("OnSystem::ALL_OS_ARCH_COMBINATIONS", os_list.product(OnSystem::ARCH_OPTIONS))

# For consistency, always run on Monterey and ARM
allow(MacOS).to receive(:version).and_return(MacOSVersion.new("12"))
Expand Down