cmd/readall: clean up todos

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.

Library/Homebrew/cli/args.rb

@@ -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

Library/Homebrew/cmd/readall.rb

@@ -38,6 +38,7 @@ def readall_args
   def readall
     args = readall_args.parse
 
+    # Consider removing the no_simulate parameter from `Readall.valid_tap?` when this gets fully disabled.
     odeprecated "--no-simulate", "nothing (i.e. not passing `--os` or `--arch`)" if args.no_simulate?
 
     if args.syntax? && args.no_named?
@@ -48,11 +49,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!"

Library/Homebrew/extend/on_system.rb

@@ -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)

Library/Homebrew/formula.rb

@@ -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?
 

Library/Homebrew/readall.rb

@@ -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?

Library/Homebrew/tap.rb

@@ -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

Library/Homebrew/test/cmd/readall_spec.rb

@@ -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

Library/Homebrew/test/formula_spec.rb

@@ -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"))