Skip to content

Commit

Permalink
cmd/readall: clean up todos
Browse files Browse the repository at this point in the history
Removes temporary default to readall for every os/arch combination
after updating brew-test-bot and brew/cask to pass the appropriate
arguments in on CI.

- #15470
- #15937 (comment)

Also, add more constants for os/arch combinations. This allows us
to make validating all os/arch combinations the default in
`Readall.valid_tap?` which is needed to keep the same behavior
in `brew tap` that we had before. I also updated a few other
spots around the codebase to use those new constants.

One more thing was updating the integration test. In local testing,
this didn't change the runtime so it seemed like a no-brainer.
  • Loading branch information
apainintheneck committed Sep 17, 2023
1 parent aa2a77b commit 5da7a72
Show file tree
Hide file tree
Showing 7 changed files with 13 additions and 18 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
8 changes: 4 additions & 4 deletions Library/Homebrew/cmd/readall.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,11 @@ def readall
end

options = {
aliases: args.aliases?,
no_simulate: args.no_simulate?,
aliases: args.aliases?,
no_simulate: args.no_simulate?,
os_arch_combinations: (args.os_arch_combinations if args.os || args.arch),
}
# 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 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
8 changes: 3 additions & 5 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

if no_simulate || os_arch_combinations.blank?
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
2 changes: 1 addition & 1 deletion Library/Homebrew/test/cmd/readall_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

FileUtils.ln_s formula_file, alias_file

expect { brew "readall", "--aliases", "--syntax", CoreTap.instance.name }
expect { brew "readall", "--os=all", "--arch=all", "--aliases", "--syntax", CoreTap.instance.name }
.to be_a_success
.and not_to_output.to_stdout
.and not_to_output.to_stderr
Expand Down

0 comments on commit 5da7a72

Please sign in to comment.