From 2d603b0c263474eb985e07b206a97e1d7601dcfa Mon Sep 17 00:00:00 2001 From: Sam Ford <1584702+samford@users.noreply.github.com> Date: Sat, 5 Apr 2025 16:03:18 -0400 Subject: [PATCH 1/7] OnSystem: enable strict typing --- Library/Homebrew/extend/on_system.rb | 59 ++++++++++++++++------------ Library/Homebrew/readall.rb | 2 +- 2 files changed, 35 insertions(+), 26 deletions(-) diff --git a/Library/Homebrew/extend/on_system.rb b/Library/Homebrew/extend/on_system.rb index 4e1854ad19fc6..a4426cd5e00d6 100644 --- a/Library/Homebrew/extend/on_system.rb +++ b/Library/Homebrew/extend/on_system.rb @@ -1,20 +1,26 @@ -# typed: true # rubocop:todo Sorbet/StrictSigil +# typed: strict # frozen_string_literal: true require "simulate_system" 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 - - VALID_OS_ARCH_TAGS = ALL_OS_ARCH_COMBINATIONS.filter_map do |os, arch| - tag = Utils::Bottles::Tag.new(system: os, arch:) - next unless tag.valid_combination? - - tag - end.freeze + ARCH_OPTIONS = T.let([:intel, :arm].freeze, T::Array[Symbol]) + BASE_OS_OPTIONS = T.let([:macos, :linux].freeze, T::Array[Symbol]) + ALL_OS_OPTIONS = T.let([*MacOSVersion::SYMBOLS.keys, :linux].freeze, T::Array[Symbol]) + ALL_OS_ARCH_COMBINATIONS = T.let( + ALL_OS_OPTIONS.product(ARCH_OPTIONS).freeze, + T::Array[[Symbol, Symbol]], + ) + + VALID_OS_ARCH_TAGS = T.let( + ALL_OS_ARCH_COMBINATIONS.filter_map do |os, arch| + tag = Utils::Bottles::Tag.new(system: os, arch:) + next unless tag.valid_combination? + + tag + end.freeze, + T::Array[Utils::Bottles::Tag], + ) sig { params(arch: Symbol).returns(T::Boolean) } def self.arch_condition_met?(arch) @@ -55,15 +61,15 @@ def self.condition_from_method_name(method_name) method_name.to_s.sub(/^on_/, "").to_sym end - sig { params(base: Class).void } + sig { params(base: T::Class[T.anything]).void } def self.setup_arch_methods(base) ARCH_OPTIONS.each do |arch| base.define_method(:"on_#{arch}") do |&block| - @on_system_blocks_exist = true + @on_system_blocks_exist = T.let(true, T.nilable(T::Boolean)) return unless OnSystem.arch_condition_met? OnSystem.condition_from_method_name(T.must(__method__)) - @called_in_on_system_block = true + @called_in_on_system_block = T.let(true, T.nilable(T::Boolean)) result = block.call @called_in_on_system_block = false @@ -82,7 +88,7 @@ def self.setup_arch_methods(base) end end - sig { params(base: Class).void } + sig { params(base: T::Class[T.anything]).void } def self.setup_base_os_methods(base) BASE_OS_OPTIONS.each do |base_os| base.define_method(:"on_#{base_os}") do |&block| @@ -128,7 +134,7 @@ def self.setup_base_os_methods(base) end end - sig { params(base: Class).void } + sig { params(base: T::Class[T.anything]).void } def self.setup_macos_methods(base) MacOSVersion::SYMBOLS.each_key do |os_name| base.define_method(:"on_#{os_name}") do |or_condition = nil, &block| @@ -137,11 +143,14 @@ def self.setup_macos_methods(base) os_condition = OnSystem.condition_from_method_name T.must(__method__) return unless OnSystem.os_condition_met? os_condition, or_condition - @on_system_block_min_os = if or_condition == :or_older - @called_in_on_system_block ? @on_system_block_min_os : MacOSVersion.new(HOMEBREW_MACOS_OLDEST_ALLOWED) - else - MacOSVersion.from_symbol(os_condition) - end + @on_system_block_min_os = T.let( + if or_condition == :or_older + @called_in_on_system_block ? @on_system_block_min_os : MacOSVersion.new(HOMEBREW_MACOS_OLDEST_ALLOWED) + else + MacOSVersion.from_symbol(os_condition) + end, + T.nilable(MacOSVersion), + ) @called_in_on_system_block = true result = block.call @called_in_on_system_block = false @@ -151,13 +160,13 @@ def self.setup_macos_methods(base) end end - sig { params(_base: Class).void } + sig { params(_base: T::Class[T.anything]).void } def self.included(_base) raise "Do not include `OnSystem` directly. Instead, include `OnSystem::MacOSAndLinux` or `OnSystem::MacOSOnly`" end module MacOSAndLinux - sig { params(base: Class).void } + sig { params(base: T::Class[T.anything]).void } def self.included(base) OnSystem.setup_arch_methods(base) OnSystem.setup_base_os_methods(base) @@ -166,7 +175,7 @@ def self.included(base) end module MacOSOnly - sig { params(base: Class).void } + sig { params(base: T::Class[T.anything]).void } def self.included(base) OnSystem.setup_arch_methods(base) OnSystem.setup_macos_methods(base) diff --git a/Library/Homebrew/readall.rb b/Library/Homebrew/readall.rb index 363fad0d29380..36ae737fcf60d 100644 --- a/Library/Homebrew/readall.rb +++ b/Library/Homebrew/readall.rb @@ -90,7 +90,7 @@ def self.valid_casks?(_tap, os_name: nil, arch: nil) sig { params( - tap: Tap, aliases: T::Boolean, no_simulate: T::Boolean, os_arch_combinations: T::Array[T::Array[String]], + tap: Tap, aliases: T::Boolean, no_simulate: T::Boolean, os_arch_combinations: T::Array[[Symbol, Symbol]], ).returns(T::Boolean) } def self.valid_tap?(tap, aliases: false, no_simulate: false, From bafa2ec1262730987f76bc09ee308c731ab36be1 Mon Sep 17 00:00:00 2001 From: Sam Ford <1584702+samford@users.noreply.github.com> Date: Sun, 6 Apr 2025 16:19:38 -0400 Subject: [PATCH 2/7] OnSystem: add tests `OnSystem` is exercised by other tests that include its modules but this adds some baseline tests to ensure some of these methods work as expected when tested in isolation. Between these added tests and existing tests, we should have 100% coverage when run on Homebrew/brew CI. --- .../Homebrew/test/extend/on_system_spec.rb | 176 ++++++++++++++++++ 1 file changed, 176 insertions(+) create mode 100644 Library/Homebrew/test/extend/on_system_spec.rb diff --git a/Library/Homebrew/test/extend/on_system_spec.rb b/Library/Homebrew/test/extend/on_system_spec.rb new file mode 100644 index 0000000000000..605c37f0d88e4 --- /dev/null +++ b/Library/Homebrew/test/extend/on_system_spec.rb @@ -0,0 +1,176 @@ +# frozen_string_literal: true + +require "extend/on_system" + +RSpec.describe OnSystem do + shared_examples "a class with on_arch methods" do + it "adds on_arch methods to class for `ARCH_OPTIONS`" do + OnSystem::ARCH_OPTIONS.each do |arch| + expect(subject.method_defined?(:"on_#{arch}")).to be true + end + end + end + + shared_examples "a class with on_base_os methods" do + it "adds on_os methods to class for `BASE_OS_OPTIONS`" do + OnSystem::BASE_OS_OPTIONS.each do |os| + expect(subject.method_defined?(:"on_#{os}")).to be true + end + end + end + + shared_examples "a class with on_macos methods" do + it "adds on_os methods to class for `MacOSVersion::SYMBOLS` keys" do + MacOSVersion::SYMBOLS.each_key do |os| + expect(subject.method_defined?(:"on_#{os}")).to be true + end + end + end + + describe "::arch_condition_met?" do + it "returns true if current arch equals provided arch" do + Homebrew::SimulateSystem.with(arch: :arm) do + expect(described_class.arch_condition_met?(:arm)).to be true + end + end + + it "returns false if current arch does not equal provided arch" do + Homebrew::SimulateSystem.with(arch: :arm) do + expect(described_class.arch_condition_met?(:intel)).to be false + end + end + + it "raises error if provided arch is not valid" do + expect { described_class.arch_condition_met?(:nonexistent_arch) } + .to raise_error(ArgumentError) + end + end + + describe "::os_condition_met?" do + let(:newest_macos) { MacOSVersion::SYMBOLS.keys.first } + + it "returns result of `SimulateSystem.simulating_or_running_on_` for supported OS" do + Homebrew::SimulateSystem.with(os: newest_macos) do + # This needs to use a value from `BASE_OS_OPTIONS` + expect(described_class.os_condition_met?(:macos)).to be true + end + end + + it "returns false if `os_name` is a macOS version but OS is Linux" do + Homebrew::SimulateSystem.with(os: :linux) do + expect(described_class.os_condition_met?(newest_macos)).to be false + end + end + + it "returns false if current OS is `:macos` and `os_name` is a macOS version" do + # A generic macOS version is treated as less than any other version. + Homebrew::SimulateSystem.with(os: :macos) do + expect(described_class.os_condition_met?(newest_macos)).to be false + end + end + + it "returns true if current OS equals the `os_name` macOS version" do + Homebrew::SimulateSystem.with(os: newest_macos) do + expect(described_class.os_condition_met?(newest_macos)).to be true + end + end + + it "returns true if current OS meets the `or_condition` for `os_name` macOS version" do + current_os = :ventura + Homebrew::SimulateSystem.with(os: current_os) do + expect(described_class.os_condition_met?(current_os, :or_newer)).to be true + expect(described_class.os_condition_met?(:big_sur, :or_newer)).to be true + expect(described_class.os_condition_met?(current_os, :or_older)).to be true + expect(described_class.os_condition_met?(newest_macos, :or_older)).to be true + end + end + + it "returns false if current OS does not meet the `or_condition` for `os_name` macOS version" do + Homebrew::SimulateSystem.with(os: :ventura) do + expect(described_class.os_condition_met?(newest_macos, :or_newer)).to be false + expect(described_class.os_condition_met?(:big_sur, :or_older)).to be false + end + end + + it "raises an error if `os_name` is not valid" do + expect { described_class.os_condition_met?(:nonexistent_os) } + .to raise_error(ArgumentError, /Invalid OS condition:/) + end + + it "raises an error if `os_name` is a macOS version but `or_condition` is not valid" do + expect do + described_class.os_condition_met?(newest_macos, :nonexistent_condition) + end.to raise_error(ArgumentError, /Invalid OS `or_\*` condition:/) + end + end + + describe "::condition_from_method_name" do + it "returns a symbol with any `on_` prefix removed" do + expect(described_class.condition_from_method_name(:on_arm)).to eq(:arm) + end + + it "returns provided symbol if there is no `on_` prefix" do + expect(described_class.condition_from_method_name(:arm)).to eq(:arm) + end + end + + describe "::setup_arch_methods" do + subject do + klass = Class.new + described_class.setup_arch_methods(klass) + klass + end + + it_behaves_like "a class with on_arch methods" + end + + describe "::setup_base_os_methods" do + subject do + klass = Class.new + described_class.setup_base_os_methods(klass) + klass + end + + it_behaves_like "a class with on_base_os methods" + end + + describe "::setup_macos_methods" do + subject do + klass = Class.new + described_class.setup_macos_methods(klass) + klass + end + + it_behaves_like "a class with on_macos methods" + end + + describe "::included" do + it "raises an error" do + expect { Class.new { include OnSystem } } + .to raise_error(/Do not include `OnSystem` directly/) + end + end + + describe "::MacOSAndLinux" do + subject { Class.new { include OnSystem::MacOSAndLinux } } + + it "can be included" do + expect { Class.new { include OnSystem::MacOSAndLinux } }.not_to raise_error + end + + it_behaves_like "a class with on_arch methods" + it_behaves_like "a class with on_base_os methods" + it_behaves_like "a class with on_macos methods" + end + + describe "::MacOSOnly" do + subject { Class.new { include OnSystem::MacOSAndLinux } } + + it "can be included" do + expect { Class.new { include OnSystem::MacOSOnly } }.not_to raise_error + end + + it_behaves_like "a class with on_arch methods" + it_behaves_like "a class with on_macos methods" + end +end From 3df8f70511c70b02852d32e18d0dc8ea49d60cb8 Mon Sep 17 00:00:00 2001 From: Sam Ford <1584702+samford@users.noreply.github.com> Date: Mon, 7 Apr 2025 12:38:50 -0400 Subject: [PATCH 3/7] OnSystem: Add UsesOnSystem class This adds a `UsesOnSystem` class to `OnSystem`, containing boolean instance variables to indicate which types of on_system methods are used in a formula or cask. This is intended as a replacement for `@on_system_blocks_exist`, which doesn't allow us to determine what kinds of on_system calls were used. This provides more granularity but we can still use `@uses_on_system.present?` to determine whether any on_system calls were used (and this doubles as a `nil` check in `Formula`, as the `self.class` instance variable has to use a nilable type). The `UsesOnSystem` instance variables cover the current `ARCH_OPTIONS` and `BASE_OS_OPTIONS`. At the moment, we mostly need to tell whether there are macOS/Linux or Intel/ARM on_system calls, so I've omitted instance variables for specific macOS version until we have a need for them. As a practical example, if you wanted to determine whether a cask uses Linux on_system calls, you can call `cask.uses_on_system.linux?`. The `linux` boolean will be `true` if the cask has an `on_linux` block, an `on_system` block (which requires Linux), or uses `os linux: ...`. This is something that would be challenging to determine from outside of `OnSystem` but it's relatively easy to collect the information in `OnSystem` methods and make it available like this. --- Library/Homebrew/cask/audit.rb | 2 +- Library/Homebrew/cask/cask.rb | 2 +- Library/Homebrew/cask/dsl.rb | 31 +- Library/Homebrew/dev-cmd/bump-cask-pr.rb | 6 +- Library/Homebrew/dev-cmd/bump.rb | 2 +- Library/Homebrew/extend/on_system.rb | 43 ++- Library/Homebrew/formula.rb | 23 +- Library/Homebrew/readall.rb | 2 +- Library/Homebrew/sorbet/rbi/dsl/cask/cask.rbi | 6 +- Library/Homebrew/test/cask/dsl_spec.rb | 294 ++++++++++++++++++ .../Homebrew/test/extend/on_system_spec.rb | 25 ++ Library/Homebrew/test/formula_spec.rb | 183 +++++++++++ .../fixtures/cask/Casks/arch-intel-only.rb | 11 + .../cask/Casks/invalid/invalid-two-os.rb | 12 + .../fixtures/cask/Casks/os-linux-only.rb | 11 + .../fixtures/cask/Casks/os-macos-only.rb | 11 + .../cask/Casks/with-on-arch-blocks.rb | 15 + .../Casks/with-on-arch-conditional-no-args.rb | 8 + .../cask/Casks/with-on-arch-conditional.rb | 8 + .../Casks/with-on-macos-version-blocks.rb | 19 ++ .../fixtures/cask/Casks/with-on-os-blocks.rb | 15 + .../cask/Casks/with-on-system-blocks.rb | 11 + .../with-on-system-conditional-no-args.rb | 8 + .../cask/Casks/with-on-system-conditional.rb | 8 + 24 files changed, 720 insertions(+), 36 deletions(-) create mode 100644 Library/Homebrew/test/support/fixtures/cask/Casks/arch-intel-only.rb create mode 100644 Library/Homebrew/test/support/fixtures/cask/Casks/invalid/invalid-two-os.rb create mode 100644 Library/Homebrew/test/support/fixtures/cask/Casks/os-linux-only.rb create mode 100644 Library/Homebrew/test/support/fixtures/cask/Casks/os-macos-only.rb create mode 100644 Library/Homebrew/test/support/fixtures/cask/Casks/with-on-arch-blocks.rb create mode 100644 Library/Homebrew/test/support/fixtures/cask/Casks/with-on-arch-conditional-no-args.rb create mode 100644 Library/Homebrew/test/support/fixtures/cask/Casks/with-on-arch-conditional.rb create mode 100644 Library/Homebrew/test/support/fixtures/cask/Casks/with-on-macos-version-blocks.rb create mode 100644 Library/Homebrew/test/support/fixtures/cask/Casks/with-on-os-blocks.rb create mode 100644 Library/Homebrew/test/support/fixtures/cask/Casks/with-on-system-blocks.rb create mode 100644 Library/Homebrew/test/support/fixtures/cask/Casks/with-on-system-conditional-no-args.rb create mode 100644 Library/Homebrew/test/support/fixtures/cask/Casks/with-on-system-conditional.rb diff --git a/Library/Homebrew/cask/audit.rb b/Library/Homebrew/cask/audit.rb index 4f8bae5c14dee..caf308cd65d7d 100644 --- a/Library/Homebrew/cask/audit.rb +++ b/Library/Homebrew/cask/audit.rb @@ -718,7 +718,7 @@ def audit_min_os cask_min_os = [on_system_block_min_os, cask.depends_on.macos&.minimum_version].compact.max odebug "Declared minimum OS version: #{cask_min_os&.to_sym}" return if cask_min_os&.to_sym == min_os.to_sym - return if cask.on_system_blocks_exist? && + return if cask.uses_on_system.present? && OnSystem.arch_condition_met?(:arm) && cask_min_os.present? && cask_min_os < MacOSVersion.new("11") diff --git a/Library/Homebrew/cask/cask.rb b/Library/Homebrew/cask/cask.rb index ec26010e90b84..c1deddc9ce9c2 100644 --- a/Library/Homebrew/cask/cask.rb +++ b/Library/Homebrew/cask/cask.rb @@ -414,7 +414,7 @@ def to_hash_with_variations hash = to_h variations = {} - if @dsl.on_system_blocks_exist? + if @dsl.uses_on_system.present? begin OnSystem::VALID_OS_ARCH_TAGS.each do |bottle_tag| next if bottle_tag.linux? && @dsl.os.nil? diff --git a/Library/Homebrew/cask/dsl.rb b/Library/Homebrew/cask/dsl.rb index ca35eebe3989e..b77003c608b0e 100644 --- a/Library/Homebrew/cask/dsl.rb +++ b/Library/Homebrew/cask/dsl.rb @@ -106,9 +106,9 @@ class DSL :no_autobump!, :autobump?, :no_autobump_message, - :on_system_blocks_exist?, - :on_system_block_min_os, :depends_on_set_in_block?, + :on_system_block_min_os, + :uses_on_system, *ORDINARY_ARTIFACT_CLASSES.map(&:dsl_key), *ACTIVATABLE_ARTIFACT_CLASSES.map(&:dsl_key), *ARTIFACT_BLOCK_CLASSES.flat_map { |klass| [klass.dsl_key, klass.uninstall_dsl_key] }, @@ -121,6 +121,11 @@ class DSL :disable_date, :disable_reason, :disable_replacement_cask, :disable_replacement_formula, :on_system_block_min_os + # A `UsesOnSystem` object that contains boolean instance variables + # indicating whether the cask uses specific on_system methods. + sig { returns(OnSystem::UsesOnSystem) } + attr_reader :uses_on_system + sig { params(cask: Cask).void } def initialize(cask) # NOTE: Variables set by `set_unique_stanza` must be initialized to `nil`. @@ -153,13 +158,13 @@ def initialize(cask) @name = T.let([], T::Array[String]) @autobump = T.let(true, T::Boolean) @no_autobump_defined = T.let(false, T::Boolean) - @on_system_blocks_exist = T.let(false, T::Boolean) @os = T.let(nil, T.nilable(String)) @on_system_block_min_os = T.let(nil, T.nilable(MacOSVersion)) @sha256 = T.let(nil, T.nilable(T.any(Checksum, Symbol))) @staged_path = T.let(nil, T.nilable(Pathname)) @token = T.let(cask.token, String) @url = T.let(nil, T.nilable(URL)) + @uses_on_system = T.let(OnSystem::UsesOnSystem.new, OnSystem::UsesOnSystem) @version = T.let(nil, T.nilable(DSL::Version)) end @@ -175,9 +180,6 @@ def disabled? = @disabled sig { returns(T::Boolean) } def livecheck_defined? = @livecheck_defined - sig { returns(T::Boolean) } - def on_system_blocks_exist? = @on_system_blocks_exist - # Specifies the cask's name. # # NOTE: Multiple names can be specified. @@ -392,8 +394,15 @@ def sha256(arg = nil, arm: nil, intel: nil, x86_64: nil, x86_64_linux: nil, arm6 x86_64 ||= intel if intel.present? && x86_64.nil? set_unique_stanza(:sha256, should_return) do - if arm.present? || x86_64.present? || x86_64_linux.present? || arm64_linux.present? - @on_system_blocks_exist = true + @uses_on_system.arm = true if arm.present? + @uses_on_system.intel = true if x86_64.present? + if x86_64_linux.present? + @uses_on_system.intel = true + @uses_on_system.linux = true + end + if arm64_linux.present? + @uses_on_system.arm = true + @uses_on_system.linux = true end val = arg || on_system_conditional( @@ -424,7 +433,8 @@ def arch(arm: nil, intel: nil) should_return = arm.nil? && intel.nil? set_unique_stanza(:arch, should_return) do - @on_system_blocks_exist = true + @uses_on_system.arm = true if arm + @uses_on_system.intel = true if intel on_arch_conditional(arm:, intel:) end @@ -449,7 +459,8 @@ def os(macos: nil, linux: nil) should_return = macos.nil? && linux.nil? set_unique_stanza(:os, should_return) do - @on_system_blocks_exist = true + @uses_on_system.macos = true if macos + @uses_on_system.linux = true if linux on_system_conditional(macos:, linux:) end diff --git a/Library/Homebrew/dev-cmd/bump-cask-pr.rb b/Library/Homebrew/dev-cmd/bump-cask-pr.rb index 72e20c132e8a8..83661f4aa7810 100644 --- a/Library/Homebrew/dev-cmd/bump-cask-pr.rb +++ b/Library/Homebrew/dev-cmd/bump-cask-pr.rb @@ -199,7 +199,7 @@ def generate_system_options(cask) # to on_system blocks referencing macOS versions. os_values = [] arch_values = depends_on_archs.presence || [] - if cask.on_system_blocks_exist? + if cask.uses_on_system.present? OnSystem::BASE_OS_OPTIONS.each do |os| os_values << if os == :macos (current_os_is_macos ? current_os : newest_macos) @@ -235,7 +235,7 @@ def replace_version_and_checksum(cask, new_hash, new_version, replacement_pairs) old_cask = begin Cask::CaskLoader.load(cask.sourcefile_path) rescue Cask::CaskInvalidError, Cask::CaskUnreadableError - raise unless cask.on_system_blocks_exist? + raise unless cask.uses_on_system.present? end next if old_cask.nil? @@ -262,7 +262,7 @@ def replace_version_and_checksum(cask, new_hash, new_version, replacement_pairs) replacement_pairs << [/"#{old_hash}"/, ":no_check"] if old_hash != :no_check elsif old_hash == :no_check && new_hash != :no_check replacement_pairs << [":no_check", "\"#{new_hash}\""] if new_hash.is_a?(String) - elsif new_hash && !cask.on_system_blocks_exist? && cask.languages.empty? + elsif new_hash && cask.uses_on_system.blank? && cask.languages.empty? replacement_pairs << [old_hash.to_s, new_hash.to_s] elsif old_hash != :no_check opoo "Multiple checksum replacements required; ignoring specified `--sha256` argument." if new_hash diff --git a/Library/Homebrew/dev-cmd/bump.rb b/Library/Homebrew/dev-cmd/bump.rb index 1d510088fa124..88f6802119362 100644 --- a/Library/Homebrew/dev-cmd/bump.rb +++ b/Library/Homebrew/dev-cmd/bump.rb @@ -305,7 +305,7 @@ def retrieve_pull_requests(formula_or_cask, name, version: nil) ).returns(VersionBumpInfo) } def retrieve_versions_by_arch(formula_or_cask:, repositories:, name:) - is_cask_with_blocks = formula_or_cask.is_a?(Cask::Cask) && formula_or_cask.on_system_blocks_exist? + is_cask_with_blocks = formula_or_cask.is_a?(Cask::Cask) && formula_or_cask.uses_on_system.present? type, version_name = if formula_or_cask.is_a?(Formula) [:formula, "formula version:"] else diff --git a/Library/Homebrew/extend/on_system.rb b/Library/Homebrew/extend/on_system.rb index a4426cd5e00d6..f881dbd3b1956 100644 --- a/Library/Homebrew/extend/on_system.rb +++ b/Library/Homebrew/extend/on_system.rb @@ -22,6 +22,28 @@ module OnSystem T::Array[Utils::Bottles::Tag], ) + class UsesOnSystem < T::Struct + prop :arm, T::Boolean, default: false + prop :intel, T::Boolean, default: false + prop :linux, T::Boolean, default: false + prop :macos, T::Boolean, default: false + + alias arm? arm + alias intel? intel + alias linux? linux + alias macos? macos + + # Whether the object has only default values. + sig { returns(T::Boolean) } + def empty? + !@arm && !@intel && !@linux && !@macos + end + + # Whether the object has any non-default values. + sig { returns(T::Boolean) } + def present? = !empty? + end + sig { params(arch: Symbol).returns(T::Boolean) } def self.arch_condition_met?(arch) raise ArgumentError, "Invalid arch condition: #{arch.inspect}" if ARCH_OPTIONS.exclude?(arch) @@ -65,7 +87,8 @@ def self.condition_from_method_name(method_name) def self.setup_arch_methods(base) ARCH_OPTIONS.each do |arch| base.define_method(:"on_#{arch}") do |&block| - @on_system_blocks_exist = T.let(true, T.nilable(T::Boolean)) + @uses_on_system ||= T.let(OnSystem::UsesOnSystem.new, T.nilable(OnSystem::UsesOnSystem)) + @uses_on_system.send(:"#{arch}=", true) return unless OnSystem.arch_condition_met? OnSystem.condition_from_method_name(T.must(__method__)) @@ -78,7 +101,9 @@ def self.setup_arch_methods(base) end base.define_method(:on_arch_conditional) do |arm: nil, intel: nil| - @on_system_blocks_exist = true + @uses_on_system ||= T.let(OnSystem::UsesOnSystem.new, T.nilable(OnSystem::UsesOnSystem)) + @uses_on_system.arm = true if arm + @uses_on_system.intel = true if intel if OnSystem.arch_condition_met? :arm arm @@ -92,7 +117,8 @@ def self.setup_arch_methods(base) def self.setup_base_os_methods(base) BASE_OS_OPTIONS.each do |base_os| base.define_method(:"on_#{base_os}") do |&block| - @on_system_blocks_exist = true + @uses_on_system ||= T.let(OnSystem::UsesOnSystem.new, T.nilable(OnSystem::UsesOnSystem)) + @uses_on_system.send(:"#{base_os}=", true) return unless OnSystem.os_condition_met? OnSystem.condition_from_method_name(T.must(__method__)) @@ -105,7 +131,9 @@ def self.setup_base_os_methods(base) end base.define_method(:on_system) do |linux, macos:, &block| - @on_system_blocks_exist = true + @uses_on_system ||= T.let(OnSystem::UsesOnSystem.new, T.nilable(OnSystem::UsesOnSystem)) + @uses_on_system.linux = true + @uses_on_system.macos = true raise ArgumentError, "The first argument to `on_system` must be `:linux`" if linux != :linux @@ -124,7 +152,9 @@ def self.setup_base_os_methods(base) end base.define_method(:on_system_conditional) do |macos: nil, linux: nil| - @on_system_blocks_exist = true + @uses_on_system ||= T.let(OnSystem::UsesOnSystem.new, T.nilable(OnSystem::UsesOnSystem)) + @uses_on_system.macos = true if macos + @uses_on_system.linux = true if linux if OnSystem.os_condition_met?(:macos) && macos.present? macos @@ -138,7 +168,8 @@ def self.setup_base_os_methods(base) def self.setup_macos_methods(base) MacOSVersion::SYMBOLS.each_key do |os_name| base.define_method(:"on_#{os_name}") do |or_condition = nil, &block| - @on_system_blocks_exist = true + @uses_on_system ||= T.let(OnSystem::UsesOnSystem.new, T.nilable(OnSystem::UsesOnSystem)) + @uses_on_system.macos = true os_condition = OnSystem.condition_from_method_name T.must(__method__) return unless OnSystem.os_condition_met? os_condition, or_condition diff --git a/Library/Homebrew/formula.rb b/Library/Homebrew/formula.rb index 740e258a32de0..3a2c31a34ef6a 100644 --- a/Library/Homebrew/formula.rb +++ b/Library/Homebrew/formula.rb @@ -270,7 +270,7 @@ def initialize(name, path, spec, alias_path: nil, tap: nil, force_bottle: false) @follow_installed_alias = T.let(true, T::Boolean) @prefix_returns_versioned_prefix = T.let(false, T.nilable(T::Boolean)) @oldname_locks = T.let([], T::Array[FormulaLock]) - @on_system_blocks_exist = T.let(false, T::Boolean) + @uses_on_system = T.let(OnSystem::UsesOnSystem.new, OnSystem::UsesOnSystem) end sig { params(spec_sym: Symbol).void } @@ -2591,7 +2591,7 @@ def to_hash_with_variations variations = {} - if path.exist? && on_system_blocks_exist? + if path.exist? && uses_on_system.present? formula_contents = path.read OnSystem::VALID_OS_ARCH_TAGS.each do |bottle_tag| Homebrew::SimulateSystem.with_tag(bottle_tag) do @@ -2789,9 +2789,11 @@ def internal_dependencies_hash(spec_symbol) end end - sig { returns(T.nilable(T::Boolean)) } - def on_system_blocks_exist? - self.class.on_system_blocks_exist? || @on_system_blocks_exist + # A `UsesOnSystem` object that contains boolean instance variables indicating + # whether the formula uses specific on_system methods. + sig { returns(OnSystem::UsesOnSystem) } + def uses_on_system + self.class.uses_on_system || @uses_on_system end sig { @@ -3331,7 +3333,7 @@ def inherited(child) @skip_clean_paths = T.let(Set.new, T.nilable(T::Set[T.any(String, Symbol)])) @link_overwrite_paths = T.let(Set.new, T.nilable(T::Set[String])) @loaded_from_api = T.let(false, T.nilable(T::Boolean)) - @on_system_blocks_exist = T.let(false, T.nilable(T::Boolean)) + @uses_on_system = T.let(OnSystem::UsesOnSystem.new, T.nilable(OnSystem::UsesOnSystem)) @network_access_allowed = T.let(SUPPORTED_NETWORK_ACCESS_PHASES.to_h do |phase| [phase, DEFAULT_NETWORK_ACCESS_ALLOWED] end, T.nilable(T::Hash[Symbol, T::Boolean])) @@ -3345,6 +3347,7 @@ def freeze @conflicts.freeze @skip_clean_paths.freeze @link_overwrite_paths.freeze + @uses_on_system.freeze super end @@ -3355,10 +3358,10 @@ def network_access_allowed = T.must(@network_access_allowed) sig { returns(T::Boolean) } def loaded_from_api? = !!@loaded_from_api - # Whether this formula contains OS/arch-specific blocks - # (e.g. `on_macos`, `on_arm`, `on_monterey :or_older`, `on_system :linux, macos: :big_sur_or_newer`). - sig { returns(T::Boolean) } - def on_system_blocks_exist? = !!@on_system_blocks_exist + # A `UsesOnSystem` object that contains boolean instance variables + # indicating whether the formula uses specific on_system methods. + sig { returns(T.nilable(OnSystem::UsesOnSystem)) } + attr_reader :uses_on_system # The reason for why this software is not linked (by default) to {::HOMEBREW_PREFIX}. sig { returns(T.nilable(KegOnlyReason)) } diff --git a/Library/Homebrew/readall.rb b/Library/Homebrew/readall.rb index 36ae737fcf60d..5d2c2ae5bee51 100644 --- a/Library/Homebrew/readall.rb +++ b/Library/Homebrew/readall.rb @@ -66,7 +66,7 @@ def self.valid_formulae?(tap, bottle_tag: nil) readall_formula = readall_formula_class.new(formula_name, file, :stable, tap:) readall_formula.to_hash # TODO: Remove check for MACOS_MODULE_REGEX once the `MacOS` module is undefined on Linux - cache[:valid_formulae][file] = if readall_formula.on_system_blocks_exist? || + cache[:valid_formulae][file] = if readall_formula.uses_on_system.present? || formula_contents.match?(MACOS_MODULE_REGEX) [bottle_tag, *cache[:valid_formulae][file]] else diff --git a/Library/Homebrew/sorbet/rbi/dsl/cask/cask.rbi b/Library/Homebrew/sorbet/rbi/dsl/cask/cask.rbi index 1ea511aa2e303..6f73b6ca5e16e 100644 --- a/Library/Homebrew/sorbet/rbi/dsl/cask/cask.rbi +++ b/Library/Homebrew/sorbet/rbi/dsl/cask/cask.rbi @@ -153,9 +153,6 @@ class Cask::Cask sig { params(args: T.untyped, block: T.untyped).returns(T.nilable(MacOSVersion)) } def on_system_block_min_os(*args, &block); end - sig { params(args: T.untyped, block: T.untyped).returns(T::Boolean) } - def on_system_blocks_exist?(*args, &block); end - sig { params(args: T.untyped, block: T.untyped).returns(T.untyped) } def os(*args, &block); end @@ -204,6 +201,9 @@ class Cask::Cask sig { params(args: T.untyped, block: T.untyped).returns(T.nilable(::Cask::URL)) } def url(*args, &block); end + sig { params(args: T.untyped, block: T.untyped).returns(T.untyped) } + def uses_on_system(*args, &block); end + sig { params(args: T.untyped, block: T.untyped).returns(T.untyped) } def version(*args, &block); end diff --git a/Library/Homebrew/test/cask/dsl_spec.rb b/Library/Homebrew/test/cask/dsl_spec.rb index c9b75cf48267c..0895cbdfb2c00 100644 --- a/Library/Homebrew/test/cask/dsl_spec.rb +++ b/Library/Homebrew/test/cask/dsl_spec.rb @@ -141,6 +141,10 @@ it "stores only the arm checksum" do expect(cask.sha256).to eq("imasha2arm") end + + it "sets @uses_on_system.arm to true" do + expect(cask.uses_on_system.arm?).to be true + end end context "when running on intel" do @@ -151,6 +155,86 @@ it "stores only the intel checksum" do expect(cask.sha256).to eq("imasha2intel") end + + it "sets @uses_on_system.intel to true" do + expect(cask.uses_on_system.intel?).to be true + end + end + end + + context "with an arm64_linux checksum" do + let(:cask_arm_linux) do + Homebrew::SimulateSystem.with os: :linux, arch: :arm do + Cask::Cask.new("arm64_linux-checksum-cask") do + sha256 arm64_linux: "imasha2armlinux" + end + end + end + + before do + allow(Hardware::CPU).to receive(:type).and_return(:arm) + end + + it "stores only the intel checksum" do + expect(cask_arm_linux.sha256).to eq("imasha2armlinux") + end + + it "sets @uses_on_system.arm to true" do + expect(cask_arm_linux.uses_on_system.arm?).to be true + end + + it "sets @uses_on_system.linux to true" do + expect(cask_arm_linux.uses_on_system.linux?).to be true + end + end + + context "with an x86_64_linux checksum" do + let(:cask_intel_linux) do + Homebrew::SimulateSystem.with os: :linux, arch: :intel do + Cask::Cask.new("x86_64_linux-checksum-cask") do + sha256 x86_64_linux: "imasha2intellinux" + end + end + end + + before do + allow(Hardware::CPU).to receive(:type).and_return(:intel) + end + + it "stores only the intel checksum" do + expect(cask_intel_linux.sha256).to eq("imasha2intellinux") + end + + it "sets @uses_on_system.intel to true" do + expect(cask_intel_linux.uses_on_system.intel?).to be true + end + + it "sets @uses_on_system.linux to true" do + expect(cask_intel_linux.uses_on_system.linux?).to be true + end + end + + context "with a :no_check checksum" do + let(:cask_no_check) do + Cask::Cask.new("no_check-checksum-cask") do + sha256 :no_check + end + end + + it "stores the :no_check symbol" do + expect(cask_no_check.sha256).to eq(:no_check) + end + end + + context "with an invalid checksum value" do + let(:cask_invalid_checksum) do + Cask::Cask.new("invalid-checksum-cask") do + sha256 :invalid_symbol + end + end + + it "refuses to load" do + expect { cask_invalid_checksum }.to raise_error(Cask::CaskInvalidError) end end end @@ -369,13 +453,45 @@ def caveats it "returns the value" do expect(cask.url.to_s).to eq "file://#{TEST_FIXTURE_DIR}/cask/caffeine-arm.zip" end + + it "sets uses_on_system.arm to true" do + expect(cask.uses_on_system.arm?).to be true + end + end + + context "when running on intel" do + before do + allow(Hardware::CPU).to receive(:type).and_return(:intel) + end + + it "defaults to `nil` for the other when no arrays are passed" do + expect(cask.url.to_s).to eq "file://#{TEST_FIXTURE_DIR}/cask/caffeine.zip" + end end + end + + context "when no arm value is specified" do + let(:token) { "arch-intel-only" } context "when running on intel" do before do allow(Hardware::CPU).to receive(:type).and_return(:intel) end + it "returns the value" do + expect(cask.url.to_s).to eq "file://#{TEST_FIXTURE_DIR}/cask/caffeine-intel.zip" + end + + it "sets uses_on_system.intel to true" do + expect(cask.uses_on_system.intel?).to be true + end + end + + context "when running on arm" do + before do + allow(Hardware::CPU).to receive(:type).and_return(:arm) + end + it "defaults to `nil` for the other when no arrays are passed" do expect(cask.url.to_s).to eq "file://#{TEST_FIXTURE_DIR}/cask/caffeine.zip" end @@ -383,6 +499,76 @@ def caveats end end + describe "os stanza" do + context "when os is called more than once" do + let(:cask_multiple_os_calls) { Cask::CaskLoader.load(cask_path("invalid/invalid-two-os")) } + + it "prevents defining multiple oss" do + expect { cask_multiple_os_calls }.to raise_error(Cask::CaskInvalidError, /'os' stanza may only appear once/) + end + end + + context "when only a macos value is specified" do + context "when running on macos" do + let(:cask_os_macos_only) do + Homebrew::SimulateSystem.with(os: :sequoia) do + Cask::CaskLoader.load(cask_path("os-macos-only")) + end + end + + it "returns the value" do + expect(cask_os_macos_only.url.to_s).to eq "file://#{TEST_FIXTURE_DIR}/cask/caffeine-darwin.zip" + end + + it "sets uses_on_system.macos to true" do + expect(cask_os_macos_only.uses_on_system.macos?).to be true + end + end + + context "when running on linux" do + let(:cask_os_macos_only) do + Homebrew::SimulateSystem.with(os: :linux) do + Cask::CaskLoader.load(cask_path("os-macos-only")) + end + end + + it "defaults to `nil`" do + expect(cask_os_macos_only.url.to_s).to eq "file://#{TEST_FIXTURE_DIR}/cask/caffeine.zip" + end + end + end + + context "when only a linux value is specified" do + context "when running on linux" do + let(:cask_os_linux_only) do + Homebrew::SimulateSystem.with(os: :linux) do + Cask::CaskLoader.load(cask_path("os-linux-only")) + end + end + + it "returns the value" do + expect(cask_os_linux_only.url.to_s).to eq "file://#{TEST_FIXTURE_DIR}/cask/caffeine-linux.zip" + end + + it "sets uses_on_system.linux to true" do + expect(cask_os_linux_only.uses_on_system.linux?).to be true + end + end + + context "when running on macos" do + let(:cask_os_linux_only) do + Homebrew::SimulateSystem.with(os: :sequoia) do + Cask::CaskLoader.load(cask_path("os-linux-only")) + end + end + + it "defaults to `nil`" do + expect(cask_os_linux_only.url.to_s).to eq "file://#{TEST_FIXTURE_DIR}/cask/caffeine.zip" + end + end + end + end + describe "depends_on stanza" do let(:token) { "invalid-depends-on-key" } @@ -598,4 +784,112 @@ def caveats ] end end + + describe "#uses_on_system" do + context "when cask uses on_arm" do + let(:token) { "with-on-arch-blocks" } + + it "sets @uses_on_system.arm to true" do + expect(cask.uses_on_system.arm?).to be true + end + end + + context "when cask uses on_intel" do + let(:token) { "with-on-arch-blocks" } + + it "sets @uses_on_system.intel to true" do + expect(cask.uses_on_system.intel?).to be true + end + end + + context "when cask uses on_arch_conditional" do + context "when arm argument is provided" do + let(:token) { "with-on-arch-conditional" } + + it "sets @uses_on_system.arm to true" do + expect(cask.uses_on_system.arm?).to be true + end + end + + context "when intel argument is provided" do + let(:token) { "with-on-arch-conditional" } + + it "sets @uses_on_system.intel to true" do + expect(cask.uses_on_system.intel?).to be true + end + end + + context "when no arguments are provided" do + let(:token) { "with-on-arch-conditional-no-args" } + + it "doesn't set @uses_on_system.arm or .intel to true" do + expect(cask.uses_on_system.arm?).to be false + expect(cask.uses_on_system.intel?).to be false + end + end + end + + context "when cask uses on_linux" do + let(:token) { "with-on-os-blocks" } + + it "sets @uses_on_system.linux to true" do + expect(cask.uses_on_system.linux?).to be true + end + end + + context "when cask uses on_macos" do + let(:token) { "with-on-os-blocks" } + + it "sets @uses_on_system.macos to true" do + expect(cask.uses_on_system.macos?).to be true + end + end + + context "when cask uses on_system" do + let(:token) { "with-on-system-blocks" } + + it "sets @uses_on_system.linux to true" do + expect(cask.uses_on_system.linux?).to be true + end + + it "sets @uses_on_system.macos to true" do + expect(cask.uses_on_system.macos?).to be true + end + end + + context "when cask uses on_system_conditional" do + context "when linux argument is provided" do + let(:token) { "with-on-system-conditional" } + + it "sets @uses_on_system.linux to true" do + expect(cask.uses_on_system.linux?).to be true + end + end + + context "when macos argument is provided" do + let(:token) { "with-on-system-conditional" } + + it "sets @uses_on_system.macos to true" do + expect(cask.uses_on_system.macos?).to be true + end + end + + context "when no arguments are provided" do + let(:token) { "with-on-system-conditional-no-args" } + + it "doesn't set @uses_on_system.linux or .macos to true" do + expect(cask.uses_on_system.linux?).to be false + expect(cask.uses_on_system.macos?).to be false + end + end + end + + context "when cask uses on_* macos version methods" do + let(:token) { "with-on-macos-version-blocks" } + + it "sets @uses_on_system.macos to true" do + expect(cask.uses_on_system.macos?).to be true + end + end + end end diff --git a/Library/Homebrew/test/extend/on_system_spec.rb b/Library/Homebrew/test/extend/on_system_spec.rb index 605c37f0d88e4..84fc72c00683b 100644 --- a/Library/Homebrew/test/extend/on_system_spec.rb +++ b/Library/Homebrew/test/extend/on_system_spec.rb @@ -27,6 +27,31 @@ end end + describe "UsesOnSystem" do + uses_on_system_empty = described_class::UsesOnSystem.new + uses_on_system_present = described_class::UsesOnSystem.new(arm: true) + + describe "#empty?" do + it "returns true if all properties are default values" do + expect(uses_on_system_empty.empty?).to be true + end + + it "returns false if any properties have a non-default value" do + expect(uses_on_system_present.empty?).to be false + end + end + + describe "#present?" do + it "returns true if object is not empty" do + expect(uses_on_system_present.present?).to be true + end + + it "returns false if object is empty" do + expect(uses_on_system_empty.present?).to be false + end + end + end + describe "::arch_condition_met?" do it "returns true if current arch equals provided arch" do Homebrew::SimulateSystem.with(arch: :arm) do diff --git a/Library/Homebrew/test/formula_spec.rb b/Library/Homebrew/test/formula_spec.rb index ada1618966628..f9d2aa28ab78e 100644 --- a/Library/Homebrew/test/formula_spec.rb +++ b/Library/Homebrew/test/formula_spec.rb @@ -1951,6 +1951,189 @@ def install end end + describe "#uses_on_system" do + # These formula definitions use a different way of creating the formula, + # as the `Class.new` approach doesn't seem to work as expected for + # `uses_on_system`. + context "when formula uses on_arm" do + let(:f_on_arm) do + formula("on-arm") do + on_arm do + url "https://brew.sh/foo-1.0-arm.tar.gz" + end + + url "https://brew.sh/foo-1.0.tar.gz" + end + end + + it "sets @uses_on_system.arm to true" do + expect(f_on_arm.uses_on_system.arm?).to be true + end + end + + context "when formula uses on_intel" do + let(:f_on_intel) do + formula("on-intel") do + on_intel do + url "https://brew.sh/foo-1.0-arm.tar.gz" + end + + url "https://brew.sh/foo-1.0.tar.gz" + end + end + + it "sets @uses_on_system.intel to true" do + expect(f_on_intel.uses_on_system.intel?).to be true + end + end + + context "when formula uses on_arch_conditional" do + let(:f_on_arch_conditional) do + formula("on-arch-conditional") do + folder = on_arch_conditional arm: "arm/", intel: "intel/" + + url "https://brew.sh/#{folder}foo-1.0.tar.gz" + end + end + let(:f_on_arch_conditional_no_args) do + formula("on-arch-conditional-no-args") do + folder = on_arch_conditional + + url "https://brew.sh/#{folder}foo-1.0.tar.gz" + end + end + + context "when arm argument is provided" do + it "sets @uses_on_system.arm to true" do + expect(f_on_arch_conditional.uses_on_system.arm?).to be true + end + end + + context "when intel argument is provided" do + it "sets @uses_on_system.intel to true" do + expect(f_on_arch_conditional.uses_on_system.intel?).to be true + end + end + + context "when no arguments are provided" do + it "doesn't set @uses_on_system.arm or .intel to true" do + expect(f_on_arch_conditional_no_args.uses_on_system.arm?).to be false + expect(f_on_arch_conditional_no_args.uses_on_system.intel?).to be false + end + end + end + + context "when formula uses on_linux" do + let(:f_on_linux) do + formula("on-linux") do + on_linux do + url "https://brew.sh/foo-1.0-linux.tar.gz" + end + + url "https://brew.sh/foo-1.0.tar.gz" + end + end + + it "sets @uses_on_system.linux to true" do + expect(f_on_linux.uses_on_system.linux?).to be true + end + end + + context "when formula uses on_macos" do + let(:f_on_macos) do + formula("on-macos") do + on_macos do + url "https://brew.sh/foo-1.0-macos.tar.gz" + end + + url "https://brew.sh/foo-1.0.tar.gz" + end + end + + it "sets @uses_on_system.macos to true" do + expect(f_on_macos.uses_on_system.macos?).to be true + end + end + + context "when formula uses on_system" do + let(:f_on_system) do + formula("on-system") do + on_system :linux, macos: :sequoia_or_newer do + url "https://brew.sh/foo-1.0-new.tar.gz" + end + + url "https://brew.sh/foo-1.0.tar.gz" + end + end + + it "sets @uses_on_system.linux to true" do + expect(f_on_system.uses_on_system.linux?).to be true + end + + it "sets @uses_on_system.macos to true" do + expect(f_on_system.uses_on_system.macos?).to be true + end + end + + context "when formula uses on_system_conditional" do + let(:f_on_system_conditional) do + formula("on-system-conditional") do + folder = on_system_conditional linux: "linux/", macos: "macos/" + + url "https://brew.sh/#{folder}foo-1.0.tar.gz" + end + end + let(:f_on_system_conditional_no_args) do + formula("on-system-conditional-no-args") do + folder = on_system_conditional + + url "https://brew.sh/#{folder}foo-1.0.tar.gz" + end + end + + context "when linux argument is provided" do + it "sets @uses_on_system.linux to true" do + expect(f_on_system_conditional.uses_on_system.linux?).to be true + end + end + + context "when macos argument is provided" do + it "sets @uses_on_system.macos to true" do + expect(f_on_system_conditional.uses_on_system.macos?).to be true + end + end + + context "when no arguments are provided" do + it "doesn't set @uses_on_system.linux or .macos to true" do + expect(f_on_system_conditional_no_args.uses_on_system.linux?).to be false + expect(f_on_system_conditional_no_args.uses_on_system.macos?).to be false + end + end + end + + context "when formula uses on_* macos version methods" do + let(:f_on_macos_versions) do + formula("on-macos-versions") do + url "https://brew.sh/foo-1.0.tar.gz" + + on_big_sur :or_older do + depends_on "abc" + end + on_monterey do + depends_on "def" + end + on_ventura :or_newer do + depends_on "ghi" + end + end + end + + it "sets @uses_on_system.macos to true" do + expect(f_on_macos_versions.uses_on_system.macos?).to be true + end + end + end + describe "#network_access_allowed?" do it "throws an error when passed an invalid symbol" do f = Testball.new diff --git a/Library/Homebrew/test/support/fixtures/cask/Casks/arch-intel-only.rb b/Library/Homebrew/test/support/fixtures/cask/Casks/arch-intel-only.rb new file mode 100644 index 0000000000000..477c6fea25881 --- /dev/null +++ b/Library/Homebrew/test/support/fixtures/cask/Casks/arch-intel-only.rb @@ -0,0 +1,11 @@ +cask "arch-intel-only" do + arch intel: "-intel" + + version "1.2.3" + sha256 "67cdb8a02803ef37fdbf7e0be205863172e41a561ca446cd84f0d7ab35a99d94" + + url "file://#{TEST_FIXTURE_DIR}/cask/caffeine#{arch}.zip" + homepage "https://brew.sh/" + + app "Caffeine.app" +end diff --git a/Library/Homebrew/test/support/fixtures/cask/Casks/invalid/invalid-two-os.rb b/Library/Homebrew/test/support/fixtures/cask/Casks/invalid/invalid-two-os.rb new file mode 100644 index 0000000000000..bd83bab1dcee6 --- /dev/null +++ b/Library/Homebrew/test/support/fixtures/cask/Casks/invalid/invalid-two-os.rb @@ -0,0 +1,12 @@ +cask "invalid-two-os" do + os linux: "linux", macos: "darwin" + os linux: "linux2", macos: "darwin2" + + version "1.2.3" + sha256 "67cdb8a02803ef37fdbf7e0be205863172e41a561ca446cd84f0d7ab35a99d94" + + url "file://#{TEST_FIXTURE_DIR}/cask/caffeine.zip" + homepage "https://brew.sh/" + + app "Caffeine.app" +end diff --git a/Library/Homebrew/test/support/fixtures/cask/Casks/os-linux-only.rb b/Library/Homebrew/test/support/fixtures/cask/Casks/os-linux-only.rb new file mode 100644 index 0000000000000..ad03208574a24 --- /dev/null +++ b/Library/Homebrew/test/support/fixtures/cask/Casks/os-linux-only.rb @@ -0,0 +1,11 @@ +cask "os-linux-only" do + os linux: "-linux" + + version "1.2.3" + sha256 "67cdb8a02803ef37fdbf7e0be205863172e41a561ca446cd84f0d7ab35a99d94" + + url "file://#{TEST_FIXTURE_DIR}/cask/caffeine#{os}.zip" + homepage "https://brew.sh/" + + app "Caffeine.app" +end diff --git a/Library/Homebrew/test/support/fixtures/cask/Casks/os-macos-only.rb b/Library/Homebrew/test/support/fixtures/cask/Casks/os-macos-only.rb new file mode 100644 index 0000000000000..1dccf7ae6f6eb --- /dev/null +++ b/Library/Homebrew/test/support/fixtures/cask/Casks/os-macos-only.rb @@ -0,0 +1,11 @@ +cask "os-macos-only" do + os macos: "-darwin" + + version "1.2.3" + sha256 "67cdb8a02803ef37fdbf7e0be205863172e41a561ca446cd84f0d7ab35a99d94" + + url "file://#{TEST_FIXTURE_DIR}/cask/caffeine#{os}.zip" + homepage "https://brew.sh/" + + app "Caffeine.app" +end diff --git a/Library/Homebrew/test/support/fixtures/cask/Casks/with-on-arch-blocks.rb b/Library/Homebrew/test/support/fixtures/cask/Casks/with-on-arch-blocks.rb new file mode 100644 index 0000000000000..6017046dfd4c9 --- /dev/null +++ b/Library/Homebrew/test/support/fixtures/cask/Casks/with-on-arch-blocks.rb @@ -0,0 +1,15 @@ +cask "with-on-arch-blocks" do + on_arm do + version "1.2.4" + sha256 "67cdb8a02803ef37fdbf7e0be205863172e41a561ca446cd84f0d7ab35a99d94" + end + on_intel do + version "1.2.3" + sha256 "8c62a2b791cf5f0da6066a0a4b6e85f62949cd60975da062df44adf887f4370b" + end + + url "https://brew.sh/TestCask-#{version}.dmg" + homepage "https://brew.sh/" + + app "TestCask.app" +end diff --git a/Library/Homebrew/test/support/fixtures/cask/Casks/with-on-arch-conditional-no-args.rb b/Library/Homebrew/test/support/fixtures/cask/Casks/with-on-arch-conditional-no-args.rb new file mode 100644 index 0000000000000..8451837397c03 --- /dev/null +++ b/Library/Homebrew/test/support/fixtures/cask/Casks/with-on-arch-conditional-no-args.rb @@ -0,0 +1,8 @@ +cask "with-on-arch-conditional-no-args" do + folder = on_arch_conditional + + url "https://brew.sh/#{folder}TestCask-#{version}.dmg" + homepage "https://brew.sh/" + + app "TestCask.app" +end diff --git a/Library/Homebrew/test/support/fixtures/cask/Casks/with-on-arch-conditional.rb b/Library/Homebrew/test/support/fixtures/cask/Casks/with-on-arch-conditional.rb new file mode 100644 index 0000000000000..935c92e4b86a7 --- /dev/null +++ b/Library/Homebrew/test/support/fixtures/cask/Casks/with-on-arch-conditional.rb @@ -0,0 +1,8 @@ +cask "with-on-arch-conditional" do + folder = on_arch_conditional arm: "arm-dir/", intel: "intel-dir/" + + url "https://brew.sh/#{folder}TestCask-#{version}.dmg" + homepage "https://brew.sh/" + + app "TestCask.app" +end diff --git a/Library/Homebrew/test/support/fixtures/cask/Casks/with-on-macos-version-blocks.rb b/Library/Homebrew/test/support/fixtures/cask/Casks/with-on-macos-version-blocks.rb new file mode 100644 index 0000000000000..5a560468971d1 --- /dev/null +++ b/Library/Homebrew/test/support/fixtures/cask/Casks/with-on-macos-version-blocks.rb @@ -0,0 +1,19 @@ +cask "with-on-macos-version-blocks" do + on_big_sur :or_older do + version "1.2.3" + sha256 "8c62a2b791cf5f0da6066a0a4b6e85f62949cd60975da062df44adf887f4370b" + end + on_monterey do + version "1.2.4" + sha256 "67cdb8a02803ef37fdbf7e0be205863172e41a561ca446cd84f0d7ab35a99d94" + end + on_ventura :or_newer do + version "1.2.5" + sha256 "306c6ca7407560340797866e077e053627ad409277d1b9da58106fce4cf717cb" + end + + url "https://brew.sh/TestCask-#{version}.dmg" + homepage "https://brew.sh/" + + app "TestCask.app" +end diff --git a/Library/Homebrew/test/support/fixtures/cask/Casks/with-on-os-blocks.rb b/Library/Homebrew/test/support/fixtures/cask/Casks/with-on-os-blocks.rb new file mode 100644 index 0000000000000..68915e1a8630d --- /dev/null +++ b/Library/Homebrew/test/support/fixtures/cask/Casks/with-on-os-blocks.rb @@ -0,0 +1,15 @@ +cask "with-on-os-blocks" do + on_linux do + version "1.2.3" + sha256 "8c62a2b791cf5f0da6066a0a4b6e85f62949cd60975da062df44adf887f4370b" + end + on_macos do + version "1.2.4" + sha256 "67cdb8a02803ef37fdbf7e0be205863172e41a561ca446cd84f0d7ab35a99d94" + end + + url "https://brew.sh/TestCask-#{version}.zip" + homepage "https://brew.sh/" + + binary "testcask" +end diff --git a/Library/Homebrew/test/support/fixtures/cask/Casks/with-on-system-blocks.rb b/Library/Homebrew/test/support/fixtures/cask/Casks/with-on-system-blocks.rb new file mode 100644 index 0000000000000..e47885dbc94c4 --- /dev/null +++ b/Library/Homebrew/test/support/fixtures/cask/Casks/with-on-system-blocks.rb @@ -0,0 +1,11 @@ +cask "with-on-system-blocks" do + on_system :linux, macos: :sequoia_or_newer do + version "1.2.4" + sha256 "67cdb8a02803ef37fdbf7e0be205863172e41a561ca446cd84f0d7ab35a99d94" + end + + url "https://brew.sh/TestCask-#{version}.dmg" + homepage "https://brew.sh/" + + app "TestCask.app" +end diff --git a/Library/Homebrew/test/support/fixtures/cask/Casks/with-on-system-conditional-no-args.rb b/Library/Homebrew/test/support/fixtures/cask/Casks/with-on-system-conditional-no-args.rb new file mode 100644 index 0000000000000..1e4bf32f4ce69 --- /dev/null +++ b/Library/Homebrew/test/support/fixtures/cask/Casks/with-on-system-conditional-no-args.rb @@ -0,0 +1,8 @@ +cask "with-on-system-conditional-no-args" do + folder = on_system_conditional + + url "https://brew.sh/#{folder}TestCask-#{version}.dmg" + homepage "https://brew.sh/" + + app "TestCask.app" +end diff --git a/Library/Homebrew/test/support/fixtures/cask/Casks/with-on-system-conditional.rb b/Library/Homebrew/test/support/fixtures/cask/Casks/with-on-system-conditional.rb new file mode 100644 index 0000000000000..3dae56528d44b --- /dev/null +++ b/Library/Homebrew/test/support/fixtures/cask/Casks/with-on-system-conditional.rb @@ -0,0 +1,8 @@ +cask "with-on-system-conditional" do + folder = on_system_conditional linux: "linux-dir/", macos: "macos-dir/" + + url "https://brew.sh/#{folder}TestCask-#{version}.dmg" + homepage "https://brew.sh/" + + app "TestCask.app" +end From 01825acabd9b9607f32c915a20b2ac82778fae17 Mon Sep 17 00:00:00 2001 From: Sam Ford <1584702+samford@users.noreply.github.com> Date: Fri, 11 Apr 2025 16:42:31 -0400 Subject: [PATCH 4/7] UsesOnSystem: Collect macOS requirements When determining macOS requirements for a cask, we may need to reference requirements from related on_system blocks (e.g., `on_monterey :or_older`, `on_ventura`, `on_sonoma :or_newer`) when `depends_on macos` isn't adequate. Sometimes casks specify different `depends_on macos` values in macOS on_system blocks but that value is only set when the cask is loaded in an environment that satisfies the on_system block's requirements. There are other casks that contain macOS on_system blocks and use `depends_on macos` outside of the on_system blocks but it may only use the macOS version of the lowest on_system block (e.g., `>= :monterey`), which isn't sufficient if the cask's values vary based on macOS version. To be able to simulate macOS versions that meet the requirements of all the on_system blocks in a cask, we need to collect the macOS requirements in a way that doesn't require OS simulation. This is also something that's easy to do in on_system methods, so this adds a `macos_requirements` array to `UsesOnSystem`, containing `MacOSRequirement` objects created from the cask's macOS on_system block conditions. --- Library/Homebrew/extend/on_system.rb | 24 ++++++++++++++++++- Library/Homebrew/test/cask/dsl_spec.rb | 14 +++++++++++ .../Homebrew/test/extend/on_system_spec.rb | 18 ++++++++++++++ Library/Homebrew/test/formula_spec.rb | 14 +++++++++++ 4 files changed, 69 insertions(+), 1 deletion(-) diff --git a/Library/Homebrew/extend/on_system.rb b/Library/Homebrew/extend/on_system.rb index f881dbd3b1956..35fedde1d129a 100644 --- a/Library/Homebrew/extend/on_system.rb +++ b/Library/Homebrew/extend/on_system.rb @@ -1,6 +1,7 @@ # typed: strict # frozen_string_literal: true +require "requirements/macos_requirement" require "simulate_system" module OnSystem @@ -27,6 +28,7 @@ class UsesOnSystem < T::Struct prop :intel, T::Boolean, default: false prop :linux, T::Boolean, default: false prop :macos, T::Boolean, default: false + prop :macos_requirements, T::Set[MacOSRequirement], default: Set[] alias arm? arm alias intel? intel @@ -36,7 +38,7 @@ class UsesOnSystem < T::Struct # Whether the object has only default values. sig { returns(T::Boolean) } def empty? - !@arm && !@intel && !@linux && !@macos + !@arm && !@intel && !@linux && !@macos && @macos_requirements.empty? end # Whether the object has any non-default values. @@ -44,6 +46,20 @@ def empty? def present? = !empty? end + # Converts an `or_condition` value to a suitable `MacOSRequirements` + # `comparator` string, defaulting to `==` if the provided argument is `nil`. + sig { params(symbol: T.nilable(Symbol)).returns(String) } + def self.comparator_from_or_condition(symbol) + case symbol + when :or_newer + ">=" + when :or_older + "<=" + else + "==" + end + end + sig { params(arch: Symbol).returns(T::Boolean) } def self.arch_condition_met?(arch) raise ArgumentError, "Invalid arch condition: #{arch.inspect}" if ARCH_OPTIONS.exclude?(arch) @@ -142,6 +158,9 @@ def self.setup_base_os_methods(base) else [macos.to_sym, nil] end + + comparator = OnSystem.comparator_from_or_condition(or_condition) + @uses_on_system.macos_requirements << MacOSRequirement.new([os_version], comparator:) return if !OnSystem.os_condition_met?(os_version, or_condition) && !OnSystem.os_condition_met?(:linux) @called_in_on_system_block = true @@ -172,6 +191,9 @@ def self.setup_macos_methods(base) @uses_on_system.macos = true os_condition = OnSystem.condition_from_method_name T.must(__method__) + comparator = OnSystem.comparator_from_or_condition(or_condition) + @uses_on_system.macos_requirements << MacOSRequirement.new([os_condition], comparator:) + return unless OnSystem.os_condition_met? os_condition, or_condition @on_system_block_min_os = T.let( diff --git a/Library/Homebrew/test/cask/dsl_spec.rb b/Library/Homebrew/test/cask/dsl_spec.rb index 0895cbdfb2c00..82c384c4d4efa 100644 --- a/Library/Homebrew/test/cask/dsl_spec.rb +++ b/Library/Homebrew/test/cask/dsl_spec.rb @@ -855,6 +855,12 @@ def caveats it "sets @uses_on_system.macos to true" do expect(cask.uses_on_system.macos?).to be true end + + it "sets @uses_on_system.macos_requirements using macos argument" do + expect(cask.uses_on_system.macos_requirements).to eq(Set[ + MacOSRequirement.new([:sequoia], comparator: ">="), + ]) + end end context "when cask uses on_system_conditional" do @@ -890,6 +896,14 @@ def caveats it "sets @uses_on_system.macos to true" do expect(cask.uses_on_system.macos?).to be true end + + it "sets @uses_on_system.macos_requirements using on_* macos version conditions" do + expect(cask.uses_on_system.macos_requirements).to eq(Set[ + MacOSRequirement.new([:big_sur], comparator: "<="), + MacOSRequirement.new([:monterey], comparator: "=="), + MacOSRequirement.new([:ventura], comparator: ">="), + ]) + end end end end diff --git a/Library/Homebrew/test/extend/on_system_spec.rb b/Library/Homebrew/test/extend/on_system_spec.rb index 84fc72c00683b..1789999a52d42 100644 --- a/Library/Homebrew/test/extend/on_system_spec.rb +++ b/Library/Homebrew/test/extend/on_system_spec.rb @@ -52,6 +52,24 @@ end end + describe "::comparator_from_or_condition" do + it 'returns ">=" for `:or_newer` symbol' do + expect(described_class.comparator_from_or_condition(:or_newer)).to eq(">=") + end + + it 'returns "<=" for `:or_older` symbol' do + expect(described_class.comparator_from_or_condition(:or_older)).to eq("<=") + end + + it 'returns "==" for an unsupported symbol' do + expect(described_class.comparator_from_or_condition(:nonexistent)).to eq("==") + end + + it 'returns "==" for `nil`' do + expect(described_class.comparator_from_or_condition(nil)).to eq("==") + end + end + describe "::arch_condition_met?" do it "returns true if current arch equals provided arch" do Homebrew::SimulateSystem.with(arch: :arm) do diff --git a/Library/Homebrew/test/formula_spec.rb b/Library/Homebrew/test/formula_spec.rb index f9d2aa28ab78e..05cae65fc7554 100644 --- a/Library/Homebrew/test/formula_spec.rb +++ b/Library/Homebrew/test/formula_spec.rb @@ -2073,6 +2073,12 @@ def install it "sets @uses_on_system.macos to true" do expect(f_on_system.uses_on_system.macos?).to be true end + + it "sets @uses_on_system.macos_requirements using macos argument" do + expect(f_on_system.uses_on_system.macos_requirements).to eq(Set[ + MacOSRequirement.new([:sequoia], comparator: ">="), + ]) + end end context "when formula uses on_system_conditional" do @@ -2131,6 +2137,14 @@ def install it "sets @uses_on_system.macos to true" do expect(f_on_macos_versions.uses_on_system.macos?).to be true end + + it "sets @uses_on_system.macos_requirements using on_* macos version conditions" do + expect(f_on_macos_versions.uses_on_system.macos_requirements).to eq(Set[ + MacOSRequirement.new([:big_sur], comparator: "<="), + MacOSRequirement.new([:monterey], comparator: "=="), + MacOSRequirement.new([:ventura], comparator: ">="), + ]) + end end end From 8350b8a43ad68e99e5fce5ec5290df592a591a20 Mon Sep 17 00:00:00 2001 From: Sam Ford <1584702+samford@users.noreply.github.com> Date: Fri, 11 Apr 2025 17:16:06 -0400 Subject: [PATCH 5/7] MacOSRequirement: add #highest_allowed method This adds a `#highest_allowed` method to `MacOSRequirement`, so we can easily identify the highest supported macOS version that a requirement allows, if any. This can be used when producing a minimal set of supported macOS versions that meet requirements in `UsesOnSystem.macos_requirements`. One of the intended use cases for this is to identify which macOS versions we could simulate to work with all variations of a cask that uses macOS on_system blocks. --- .../Homebrew/requirements/macos_requirement.rb | 10 ++++++++++ .../test/requirements/macos_requirement_spec.rb | 16 ++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/Library/Homebrew/requirements/macos_requirement.rb b/Library/Homebrew/requirements/macos_requirement.rb index 366348b893c1d..9b0be0d8d3e8f 100644 --- a/Library/Homebrew/requirements/macos_requirement.rb +++ b/Library/Homebrew/requirements/macos_requirement.rb @@ -88,6 +88,16 @@ def allows?(other) end end + # Finds the highest supported {MacOSVersion} that meets the requirement, if + # any. + sig { returns(T.nilable(MacOSVersion)) } + def highest_allowed + MacOSVersion::SYMBOLS.each_key do |sym| + candidate_version = MacOSVersion.from_symbol(sym) + return candidate_version if allows?(candidate_version) + end + end + def message(type: :formula) return "macOS is required for this software." unless version_specified? diff --git a/Library/Homebrew/test/requirements/macos_requirement_spec.rb b/Library/Homebrew/test/requirements/macos_requirement_spec.rb index f983abd2790fe..b9608c5690c1a 100644 --- a/Library/Homebrew/test/requirements/macos_requirement_spec.rb +++ b/Library/Homebrew/test/requirements/macos_requirement_spec.rb @@ -7,6 +7,7 @@ let(:macos_oldest_allowed) { MacOSVersion.new(HOMEBREW_MACOS_OLDEST_ALLOWED) } let(:macos_newest_allowed) { MacOSVersion.new(HOMEBREW_MACOS_NEWEST_UNSUPPORTED) } + let(:macos_newest_supported) { MacOSVersion.new(MacOSVersion::SYMBOLS.values.first) } let(:big_sur_major) { MacOSVersion.new("11.0") } describe "#satisfied?" do @@ -63,4 +64,19 @@ expect(exact_requirement.allows?(big_sur_major)).to be true expect(range_requirement.allows?(big_sur_major)).to be true end + + specify "#highest_allowed" do + macos_version_sonoma = MacOSVersion.new("14") + + no_requirement = described_class.new + max_requirement = described_class.new([:sonoma], comparator: "<=") + min_requirement = described_class.new([:sonoma], comparator: ">=") + exact_requirement = described_class.new([:sonoma], comparator: "==") + range_requirement = described_class.new([[:sonoma, :monterey]], comparator: "==") + expect(no_requirement.highest_allowed).to eq macos_newest_supported + expect(max_requirement.highest_allowed).to eq macos_version_sonoma + expect(min_requirement.highest_allowed).to eq macos_newest_supported + expect(exact_requirement.highest_allowed).to eq macos_version_sonoma + expect(range_requirement.highest_allowed).to eq macos_version_sonoma + end end From ccfc31bc18f9be86671db1c6c82ea4d3f19ed7c9 Mon Sep 17 00:00:00 2001 From: Sam Ford <1584702+samford@users.noreply.github.com> Date: Fri, 11 Apr 2025 17:18:41 -0400 Subject: [PATCH 6/7] MacOSRequirement: add more type signatures This adds more type signatures to `MacOSRequirement` to move it closer to `typed: strict`. There are a few areas that aren't quite clear to me based on the code and existing tests, so I've done what I can at the moment. --- Library/Homebrew/cask/dsl/depends_on.rb | 4 ++-- .../requirements/macos_requirement.rb | 22 ++++++++++++++----- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/Library/Homebrew/cask/dsl/depends_on.rb b/Library/Homebrew/cask/dsl/depends_on.rb index 7a5756f9c0584..4e08bb893239f 100644 --- a/Library/Homebrew/cask/dsl/depends_on.rb +++ b/Library/Homebrew/cask/dsl/depends_on.rb @@ -60,9 +60,9 @@ def macos=(*args) elsif MacOSVersion::SYMBOLS.key?(args.first) MacOSRequirement.new([args.first], comparator: "==") elsif (md = /^\s*(?<|>|[=<>]=)\s*:(?\S+)\s*$/.match(first_arg)) - MacOSRequirement.new([T.must(md[:version]).to_sym], comparator: md[:comparator]) + MacOSRequirement.new([T.must(md[:version]).to_sym], comparator: T.must(md[:comparator])) elsif (md = /^\s*(?<|>|[=<>]=)\s*(?\S+)\s*$/.match(first_arg)) - MacOSRequirement.new([md[:version]], comparator: md[:comparator]) + MacOSRequirement.new([md[:version]], comparator: T.must(md[:comparator])) # This is not duplicate of the first case: see `args.first` and a different comparator. else # rubocop:disable Lint/DuplicateBranch MacOSRequirement.new([args.first], comparator: "==") diff --git a/Library/Homebrew/requirements/macos_requirement.rb b/Library/Homebrew/requirements/macos_requirement.rb index 9b0be0d8d3e8f..4624349739ccf 100644 --- a/Library/Homebrew/requirements/macos_requirement.rb +++ b/Library/Homebrew/requirements/macos_requirement.rb @@ -7,14 +7,17 @@ class MacOSRequirement < Requirement fatal true - attr_reader :comparator, :version + sig { returns(String) } + attr_reader :comparator + + attr_reader :version # TODO: when Yosemite is removed here, keep these around as empty arrays so we # can keep the deprecation/disabling code the same. - DISABLED_MACOS_VERSIONS = [ + DISABLED_MACOS_VERSIONS = T.let([ :yosemite, - ].freeze - DEPRECATED_MACOS_VERSIONS = [].freeze + ].freeze, T::Array[Symbol]) + DEPRECATED_MACOS_VERSIONS = T.let([].freeze, T::Array[Symbol]) def initialize(tags = [], comparator: ">=") @version = begin @@ -44,10 +47,11 @@ def initialize(tags = [], comparator: ">=") MacOSVersion.new(HOMEBREW_MACOS_OLDEST_ALLOWED) if comparator == ">=" end - @comparator = comparator + @comparator = T.let(comparator, String) super(tags.drop(1)) end + sig { returns(T::Boolean) } def version_specified? @version.present? end @@ -61,6 +65,7 @@ def version_specified? false end + sig { returns(MacOSVersion) } def minimum_version return MacOSVersion.new(HOMEBREW_MACOS_OLDEST_ALLOWED) if @comparator == "<=" || !version_specified? return @version.min if @version.respond_to?(:to_ary) @@ -68,6 +73,7 @@ def minimum_version @version end + sig { returns(MacOSVersion) } def maximum_version return MacOSVersion.new(HOMEBREW_MACOS_NEWEST_UNSUPPORTED) if @comparator == ">=" || !version_specified? return @version.max if @version.respond_to?(:to_ary) @@ -75,6 +81,7 @@ def maximum_version @version end + sig { params(other: MacOSVersion).returns(T::Boolean) } def allows?(other) return true unless version_specified? @@ -98,6 +105,7 @@ def highest_allowed end end + sig { params(type: Symbol).returns(String) } def message(type: :formula) return "macOS is required for this software." unless version_specified? @@ -113,6 +121,8 @@ def message(type: :formula) EOS when :cask "This cask does not run on macOS versions newer than #{@version.pretty_name}." + else + "This does not run on macOS versions newer than #{@version.pretty_name}." end else if @version.respond_to?(:to_ary) @@ -129,6 +139,7 @@ def ==(other) end alias eql? == + sig { returns(Integer) } def hash [super, comparator, version].hash end @@ -151,6 +162,7 @@ def display_s end end + sig { params(options: T.untyped).returns(String) } def to_json(options) comp = @comparator.to_s return { comp => @version.map(&:to_s) }.to_json(options) if @version.is_a?(Array) From 473f448b224afc4626dfab75e27ee685355673f7 Mon Sep 17 00:00:00 2001 From: Sam Ford <1584702+samford@users.noreply.github.com> Date: Sun, 8 Jun 2025 00:37:02 -0400 Subject: [PATCH 7/7] OnSystem: handle non-macOS current_os values The `OnSystem.os_condition_met?` method only handles Linux and macOS values for the current OS, so this fails when testing with a generic OS. This shortcoming is only being surfaced now because there weren't any tests for `OnSystem` before. This addresses the issue by accounting for macOS values (`:macos` or a symbol from `MacOSVersion::SYMBOLS`) and returning `false` for any other values (`:linux`, `:generic`, etc.). --- Library/Homebrew/extend/on_system.rb | 13 +++++++------ Library/Homebrew/test/extend/on_system_spec.rb | 5 +++++ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/Library/Homebrew/extend/on_system.rb b/Library/Homebrew/extend/on_system.rb index 35fedde1d129a..a187cdeab7e4e 100644 --- a/Library/Homebrew/extend/on_system.rb +++ b/Library/Homebrew/extend/on_system.rb @@ -77,16 +77,17 @@ def self.os_condition_met?(os_name, or_condition = nil) raise ArgumentError, "Invalid OS `or_*` condition: #{or_condition.inspect}" end - return false if Homebrew::SimulateSystem.simulating_or_running_on_linux? - - base_os = MacOSVersion.from_symbol(os_name) - current_os = if Homebrew::SimulateSystem.current_os == :macos + current_os_symbol = Homebrew::SimulateSystem.current_os + current_os = if current_os_symbol == :macos # Assume the oldest macOS version when simulating a generic macOS version # Version::NULL is always treated as less than any other version. Version::NULL + elsif MacOSVersion::SYMBOLS.key?(current_os_symbol) + MacOSVersion.from_symbol(current_os_symbol) else - MacOSVersion.from_symbol(Homebrew::SimulateSystem.current_os) + return false end + base_os = MacOSVersion.from_symbol(os_name) return current_os >= base_os if or_condition == :or_newer return current_os <= base_os if or_condition == :or_older @@ -194,7 +195,7 @@ def self.setup_macos_methods(base) comparator = OnSystem.comparator_from_or_condition(or_condition) @uses_on_system.macos_requirements << MacOSRequirement.new([os_condition], comparator:) - return unless OnSystem.os_condition_met? os_condition, or_condition + return unless OnSystem.os_condition_met?(os_condition, or_condition) @on_system_block_min_os = T.let( if or_condition == :or_older diff --git a/Library/Homebrew/test/extend/on_system_spec.rb b/Library/Homebrew/test/extend/on_system_spec.rb index 1789999a52d42..428b307f718fe 100644 --- a/Library/Homebrew/test/extend/on_system_spec.rb +++ b/Library/Homebrew/test/extend/on_system_spec.rb @@ -105,6 +105,11 @@ end end + it "returns false if `os_name` is a macOS version but current OS is `:generic`" do + allow(Homebrew::SimulateSystem).to receive(:current_os).and_return(:generic) + expect(described_class.os_condition_met?(newest_macos)).to be false + end + it "returns false if current OS is `:macos` and `os_name` is a macOS version" do # A generic macOS version is treated as less than any other version. Homebrew::SimulateSystem.with(os: :macos) do