Merge pull request #19709 from Homebrew/bump-cask-pr-respect-os-arch

bump-cask-pr: respect `depends_on arch`

Author: MikeMcQuaid

Library/Homebrew/dev-cmd/bump-cask-pr.rb

@@ -181,6 +181,40 @@ def shortened_version(version, cask:)
         end
       end
 
+      sig { params(cask: Cask::Cask).returns(T::Array[[Symbol, Symbol]]) }
+      def generate_system_options(cask)
+        current_os = Homebrew::SimulateSystem.current_os
+        current_os_is_macos = MacOSVersion::SYMBOLS.include?(current_os)
+        newest_macos = MacOSVersion::SYMBOLS.keys.first
+
+        depends_on_archs = cask.depends_on.arch&.filter_map { |arch| arch[:type] }&.uniq
+
+        # NOTE: We substitute the newest macOS (e.g. `:sequoia`) in place of
+        # `:macos` values (when used), as a generic `:macos` value won't apply
+        # to on_system blocks referencing macOS versions.
+        os_values = []
+        arch_values = depends_on_archs.presence || []
+        if cask.on_system_blocks_exist?
+          OnSystem::BASE_OS_OPTIONS.each do |os|
+            os_values << if os == :macos
+              (current_os_is_macos ? current_os : newest_macos)
+            else
+              os
+            end
+          end
+
+          arch_values = OnSystem::ARCH_OPTIONS if arch_values.empty?
+        else
+          # Architecture is only relevant if on_system blocks are present or
+          # the cask uses `depends_on arch`, otherwise we default to ARM for
+          # consistency.
+          os_values << (current_os_is_macos ? current_os : newest_macos)
+          arch_values << :arm if arch_values.empty?
+        end
+
+        os_values.product(arch_values)
+      end
+
       sig {
         params(
           cask:              Cask::Cask,
@@ -190,33 +224,8 @@ def shortened_version(version, cask:)
         ).returns(T::Array[[T.any(Regexp, String), T.any(Pathname, String)]])
       }
       def replace_version_and_checksum(cask, new_hash, new_version, replacement_pairs)
-        host_os = Homebrew::SimulateSystem.current_os
-        host_is_macos = MacOSVersion::SYMBOLS.include?(host_os)
-        newest_macos = MacOSVersion::SYMBOLS.keys.first
-
-        # NOTE: We substitute the newest macOS (e.g. `:sequoia`) in place of
-        # `:macos` values (when used), as a generic `:macos` value won't apply
-        # to on_system blocks referencing macOS versions. We also omit the OS
-        # when the value aligns with the host.
-        system_options = if cask.on_system_blocks_exist?
-          OnSystem::BASE_OS_OPTIONS.each_with_object([]) do |os, array|
-            OnSystem::ARCH_OPTIONS.each do |arch|
-              system_hash = { arch: }
-              system_hash[:os] = os if host_is_macos && os != :macos
-              system_hash[:os] = newest_macos if !host_is_macos && os == :macos
-              array << system_hash
-            end
-          end.uniq
-        else
-          # Architecture is only relevant if on_system blocks are present. When
-          # not present, we default to ARM for consistency.
-          system_hash = { arch: :arm }
-          system_hash[:os] = newest_macos unless host_is_macos
-          [system_hash]
-        end
-
-        system_options.each do |system_args|
-          SimulateSystem.with(**system_args) do
+        generate_system_options(cask).each do |os, arch|
+          SimulateSystem.with(os:, arch:) do
             # Handle the cask being invalid for specific os/arch combinations
             old_cask = begin
               Cask::CaskLoader.load(cask.sourcefile_path)
@@ -228,7 +237,7 @@ def replace_version_and_checksum(cask, new_hash, new_version, replacement_pairs)
             old_version = old_cask.version
             next unless old_version
 
-            bump_version = new_version.send(system_args[:arch]) || new_version.general
+            bump_version = new_version.send(arch) || new_version.general
 
             old_version_regex = old_version.latest? ? ":latest" : %Q(["']#{Regexp.escape(old_version.to_s)}["'])
             replacement_pairs << [/version\s+#{old_version_regex}/m,

Library/Homebrew/test/dev-cmd/bump-cask-pr_spec.rb

@@ -4,5 +4,139 @@
 require "dev-cmd/bump-cask-pr"
 
 RSpec.describe Homebrew::DevCmd::BumpCaskPr do
+  subject(:bump_cask_pr) { described_class.new(["test"]) }
+
+  let(:newest_macos) { MacOSVersion::SYMBOLS.keys.first }
+
+  let(:c) do
+    Cask::Cask.new("test") do
+      version "0.0.1,2"
+
+      url "https://brew.sh/test-0.0.1.dmg"
+      name "Test"
+      desc "Test cask"
+      homepage "https://brew.sh"
+    end
+  end
+
+  let(:c_depends_on_intel) do
+    Cask::Cask.new("test-depends-on-intel") do
+      version "0.0.1,2"
+
+      url "https://brew.sh/test-0.0.1.dmg"
+      name "Test"
+      desc "Test cask"
+      homepage "https://brew.sh"
+
+      depends_on arch: :x86_64
+    end
+  end
+
+  let(:c_on_system) do
+    Cask::Cask.new("test-on-system") do
+      os macos: "darwin", linux: "linux"
+
+      version "0.0.1,2"
+
+      url "https://brew.sh/test-0.0.1.dmg"
+      name "Test"
+      desc "Test cask"
+      homepage "https://brew.sh"
+    end
+  end
+
+  let(:c_on_system_depends_on_intel) do
+    Cask::Cask.new("test-on-system-depends-on-intel") do
+      os macos: "darwin", linux: "linux"
+
+      version "0.0.1,2"
+
+      url "https://brew.sh/test-0.0.1.dmg"
+      name "Test"
+      desc "Test cask"
+      homepage "https://brew.sh"
+
+      depends_on arch: :x86_64
+    end
+  end
+
   it_behaves_like "parseable arguments"
+
+  describe "::generate_system_options" do
+    # We simulate a macOS version older than the newest, as the method will use
+    # the host macOS version instead of the default (the newest macOS version).
+    let(:older_macos) { :big_sur }
+
+    context "when cask does not have on_system blocks/calls or `depends_on arch`" do
+      it "returns an array only including macOS/ARM" do
+        Homebrew::SimulateSystem.with(os: :linux) do
+          expect(bump_cask_pr.send(:generate_system_options, c))
+            .to eq([[newest_macos, :arm]])
+        end
+
+        Homebrew::SimulateSystem.with(os: older_macos) do
+          expect(bump_cask_pr.send(:generate_system_options, c))
+            .to eq([[older_macos, :arm]])
+        end
+      end
+    end
+
+    context "when cask does not have on_system blocks/calls but has `depends_on arch`" do
+      it "returns an array only including macOS/`depends_on arch` value" do
+        Homebrew::SimulateSystem.with(os: :linux, arch: :arm) do
+          expect(bump_cask_pr.send(:generate_system_options, c_depends_on_intel))
+            .to eq([[newest_macos, :intel]])
+        end
+
+        Homebrew::SimulateSystem.with(os: older_macos, arch: :arm) do
+          expect(bump_cask_pr.send(:generate_system_options, c_depends_on_intel))
+            .to eq([[older_macos, :intel]])
+        end
+      end
+    end
+
+    context "when cask has on_system blocks/calls but does not have `depends_on arch`" do
+      it "returns an array with combinations of `OnSystem::BASE_OS_OPTIONS` and `OnSystem::ARCH_OPTIONS`" do
+        Homebrew::SimulateSystem.with(os: :linux) do
+          expect(bump_cask_pr.send(:generate_system_options, c_on_system))
+            .to eq([
+              [newest_macos, :intel],
+              [newest_macos, :arm],
+              [:linux, :intel],
+              [:linux, :arm],
+            ])
+        end
+
+        Homebrew::SimulateSystem.with(os: older_macos) do
+          expect(bump_cask_pr.send(:generate_system_options, c_on_system))
+            .to eq([
+              [older_macos, :intel],
+              [older_macos, :arm],
+              [:linux, :intel],
+              [:linux, :arm],
+            ])
+        end
+      end
+    end
+
+    context "when cask has on_system blocks/calls and `depends_on arch`" do
+      it "returns an array with combinations of `OnSystem::BASE_OS_OPTIONS` and `depends_on arch` value" do
+        Homebrew::SimulateSystem.with(os: :linux, arch: :arm) do
+          expect(bump_cask_pr.send(:generate_system_options, c_on_system_depends_on_intel))
+            .to eq([
+              [newest_macos, :intel],
+              [:linux, :intel],
+            ])
+        end
+
+        Homebrew::SimulateSystem.with(os: older_macos, arch: :arm) do
+          expect(bump_cask_pr.send(:generate_system_options, c_on_system_depends_on_intel))
+            .to eq([
+              [older_macos, :intel],
+              [:linux, :intel],
+            ])
+        end
+      end
+    end
+  end
 end