Skip to content

Commit

Permalink
Merge pull request #16011 from apainintheneck/clean-up-readall-command
Browse files Browse the repository at this point in the history
cmd/readall: clean up todos
  • Loading branch information
MikeMcQuaid authored Sep 20, 2023
2 parents aa2a77b + 5760ae4 commit 1f80e82
Show file tree
Hide file tree
Showing 7 changed files with 12 additions and 23 deletions.
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 @@ def os_arch_combinations
when :all
skip_invalid_combinations = true

[
*MacOSVersion::SYMBOLS.keys,
:linux,
]
OnSystem::ALL_OS_OPTIONS
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

0 comments on commit 1f80e82

Please sign in to comment.