Merge pull request #15725 from razvanazamfirei/bump-cask-pr-arch

`bump-cask-pr`: add arch-specific version support

Library/Homebrew/bump_version_parser.rb

@@ -0,0 +1,54 @@
+# typed: strict
+# frozen_string_literal: true
+
+module Homebrew
+  # Class handling architecture-specific version information.
+  #
+  # @api private
+  class BumpVersionParser
+    sig { returns(T.nilable(T.any(Version, Cask::DSL::Version))) }
+    attr_reader :arm, :general, :intel
+
+    sig {
+      params(general: T.nilable(T.any(Version, String)),
+             arm:     T.nilable(T.any(Version, String)),
+             intel:   T.nilable(T.any(Version, String))).void
+    }
+    def initialize(general: nil, arm: nil, intel: nil)
+      @general = T.let(parse_version(general), T.nilable(T.any(Version, Cask::DSL::Version))) if general.present?
+      @arm = T.let(parse_version(arm), T.nilable(T.any(Version, Cask::DSL::Version))) if arm.present?
+      @intel = T.let(parse_version(intel), T.nilable(T.any(Version, Cask::DSL::Version))) if intel.present?
+
+      return if @general.present?
+      raise UsageError, "`--version` must not be empty." if arm.blank? && intel.blank?
+      raise UsageError, "`--version-arm` must not be empty." if arm.blank?
+      raise UsageError, "`--version-intel` must not be empty." if intel.blank?
+    end
+
+    sig {
+      params(version: T.any(Version, String))
+        .returns(T.nilable(T.any(Version, Cask::DSL::Version)))
+    }
+    def parse_version(version)
+      if version.is_a?(Version)
+        version
+      elsif version.is_a?(String)
+        parse_cask_version(version)
+      end
+    end
+
+    sig { params(version: String).returns(T.nilable(Cask::DSL::Version)) }
+    def parse_cask_version(version)
+      if version == "latest"
+        Cask::DSL::Version.new(:latest)
+      else
+        Cask::DSL::Version.new(version)
+      end
+    end
+
+    sig { returns(T::Boolean) }
+    def blank?
+      @general.blank? && @arm.blank? && @intel.blank?
+    end
+  end
+end

Library/Homebrew/cask/cask.rbi

@@ -4,6 +4,8 @@ module Cask
   class Cask
     def appcast; end
 
+    def appdir; end
+
     def artifacts; end
 
     def auto_updates; end
@@ -14,9 +16,11 @@ module Cask
 
     def container; end
 
+    def depends_on; end
+
     def desc; end
 
-    def depends_on; end
+    def discontinued?; end
 
     def homepage; end
 
@@ -24,8 +28,14 @@ module Cask
 
     def languages; end
 
+    def livecheck; end
+
+    def livecheckable?; end
+
     def name; end
 
+    def on_system_blocks_exist?; end
+
     def sha256; end
 
     def staged_path; end
@@ -33,13 +43,5 @@ module Cask
     def url; end
 
     def version; end
-
-    def appdir; end
-
-    def discontinued?; end
-
-    def livecheck; end
-
-    def livecheckable?; end
   end
 end

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

@@ -1,6 +1,7 @@
-# typed: true
+# typed: strict
 # frozen_string_literal: true
 
+require "bump_version_parser"
 require "cask"
 require "cask/download"
 require "cli/parser"
@@ -38,6 +39,10 @@ module Homebrew
              description: "Don't try to fork the repository."
       flag   "--version=",
              description: "Specify the new <version> for the cask."
+      flag   "--version-arm=",
+             description: "Specify the new cask <version> for the ARM architecture."
+      flag   "--version-intel=",
+             description: "Specify the new cask <version> for the Intel architecture."
       flag   "--message=",
              description: "Prepend <message> to the default pull request message."
       flag   "--url=",
@@ -51,11 +56,14 @@ module Homebrew
 
       conflicts "--dry-run", "--write"
       conflicts "--no-audit", "--online"
+      conflicts "--version=", "--version-arm="
+      conflicts "--version=", "--version-intel="
 
       named_args :cask, number: 1, without_api: true
     end
   end
 
+  sig { void }
   def bump_cask_pr
     args = bump_cask_pr_args.parse
 
@@ -68,19 +76,18 @@ module Homebrew
     ENV["PATH"] = PATH.new(ORIGINAL_PATHS).to_s
 
     # Use the user's browser, too.
-    ENV["BROWSER"] = Homebrew::EnvConfig.browser
+    ENV["BROWSER"] = EnvConfig.browser
 
     cask = args.named.to_casks.first
 
     odie "This cask is not in a tap!" if cask.tap.blank?
     odie "This cask's tap is not a Git repository!" unless cask.tap.git?
 
-    new_version = unless (new_version = args.version).nil?
-      raise UsageError, "`--version` must not be empty." if new_version.blank?
-
-      new_version = :latest if ["latest", ":latest"].include?(new_version)
-      Cask::DSL::Version.new(new_version)
-    end
+    new_version = BumpVersionParser.new(
+      general: args.version,
+      intel:   args.version_intel,
+      arm:     args.version_arm,
+    )
 
     new_hash = unless (new_hash = args.sha256).nil?
       raise UsageError, "`--sha256` must not be empty." if new_hash.blank?
@@ -98,85 +105,33 @@ module Homebrew
       end
     end
 
-    if new_version.nil? && new_base_url.nil? && new_hash.nil?
+    if new_version.blank? && new_base_url.nil? && new_hash.nil?
       raise UsageError, "No `--version`, `--url` or `--sha256` argument specified!"
     end
 
-    old_version = cask.version
-    old_hash = cask.sha256
+    check_pull_requests(cask, args: args, new_version: new_version)
 
-    check_pull_requests(cask, state: "open", args: args)
-    # if we haven't already found open requests, try for an exact match across closed requests
-    check_pull_requests(cask, state: "closed", args: args, version: new_version) if new_version.present?
-
-    old_contents = File.read(cask.sourcefile_path)
-
-    replacement_pairs = []
+    replacement_pairs ||= []
     branch_name = "bump-#{cask.token}"
     commit_message = nil
 
-    if new_version
-      branch_name += "-#{new_version.tr(",:", "-")}"
-      commit_message_version = if new_version.before_comma == old_version.before_comma
-        new_version
+    if new_version.present?
+      # For simplicity, our naming defers to the arm version if we multiple architectures are specified
+      branch_version = new_version.arm || new_version.general
+      if branch_version.is_a?(Cask::DSL::Version)
+        commit_version = if branch_version.before_comma == cask.version.before_comma
+          branch_version
         else
-        new_version.before_comma
+          branch_version.before_comma
         end
-      commit_message ||= "#{cask.token} #{commit_message_version}"
+        branch_name = "bump-#{cask.token}-#{branch_version.tr(",:", "-")}"
+        commit_message ||= "#{cask.token} #{commit_version}"
+      end
+      replacement_pairs = replace_version_and_checksum(cask, new_hash, new_version, replacement_pairs)
+    end
+    # Now that we have all replacement pairs, we will replace them further down
 
-      old_version_regex = old_version.latest? ? ":latest" : "[\"']#{Regexp.escape(old_version.to_s)}[\"']"
-
-      replacement_pairs << [
-        /version\s+#{old_version_regex}/m,
-        "version #{new_version.latest? ? ":latest" : "\"#{new_version}\""}",
-      ]
-      if new_version.latest? || new_hash == :no_check
-        opoo "Ignoring specified `--sha256=` argument." if new_hash.is_a?(String)
-        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 old_hash != :no_check
-        if new_hash.nil? || cask.languages.present?
-          if new_hash && cask.languages.present?
-            opoo "Multiple checksum replacements required; ignoring specified `--sha256` argument."
-          end
-          tmp_contents = Utils::Inreplace.inreplace_pairs(cask.sourcefile_path,
-                                                          replacement_pairs.uniq.compact,
-                                                          read_only_run: true,
-                                                          silent:        true)
-
-          tmp_cask = Cask::CaskLoader.load(tmp_contents)
-          tmp_config = tmp_cask.config
-
-          OnSystem::ARCH_OPTIONS.each do |arch|
-            SimulateSystem.with arch: arch do
-              languages = cask.languages
-              languages = [nil] if languages.empty?
-              languages.each do |language|
-                new_hash_config = if language.blank?
-                  tmp_config
-                else
-                  tmp_config.merge(Cask::Config.new(explicit: { languages: [language] }))
-                end
-
-                new_hash_cask = Cask::CaskLoader.load(tmp_contents)
-                new_hash_cask.config = new_hash_config
-                old_hash = new_hash_cask.sha256.to_s
-
-                cask_download = Cask::Download.new(new_hash_cask, quarantine: true)
-                download = cask_download.fetch(verify_download_integrity: false)
-                Utils::Tar.validate_file(download)
-
-                replacement_pairs << [new_hash_cask.sha256.to_s, download.sha256]
-              end
-            end
-          end
-        elsif new_hash
-          opoo "Cask contains multiple hashes; only updating hash for current arch." if cask.on_system_blocks_exist?
-          replacement_pairs << [old_hash.to_s, new_hash]
-        end
-      end
-    end
+    old_contents = File.read(cask.sourcefile_path)
 
     if new_base_url
       commit_message ||= "#{cask.token}: update URL"
@@ -194,8 +149,10 @@ module Homebrew
 
     commit_message ||= "#{cask.token}: update checksum" if new_hash
 
+    # Remove nested arrays where elements are identical
+    replacement_pairs = replacement_pairs.reject { |pair| pair[0] == pair[1] }.uniq.compact
     Utils::Inreplace.inreplace_pairs(cask.sourcefile_path,
-                                     replacement_pairs.uniq.compact,
+                                     replacement_pairs,
                                      read_only_run: args.dry_run?,
                                      silent:        args.quiet?)
 
@@ -203,25 +160,110 @@ module Homebrew
     run_cask_style(cask, old_contents, args: args)
 
     pr_info = {
-      sourcefile_path: cask.sourcefile_path,
-      old_contents:    old_contents,
       branch_name:     branch_name,
       commit_message:  commit_message,
-      tap:             cask.tap,
+      old_contents:    old_contents,
       pr_message:      "Created with `brew bump-cask-pr`.",
+      sourcefile_path: cask.sourcefile_path,
+      tap:             cask.tap,
     }
     GitHub.create_bump_pr(pr_info, args: args)
   end
 
-  def check_pull_requests(cask, state:, args:, version: nil)
-    tap_remote_repo = cask.tap.full_name || cask.tap.remote_repo
-    GitHub.check_for_duplicate_pull_requests(cask.token, tap_remote_repo,
-                                             state:   state,
-                                             version: version,
-                                             file:    cask.sourcefile_path.relative_path_from(cask.tap.path).to_s,
-                                             args:    args)
+  sig {
+    params(
+      cask:              Cask::Cask,
+      new_hash:          T.nilable(String),
+      new_version:       BumpVersionParser,
+      replacement_pairs: T::Array[[T.any(Regexp, String), T.any(Regexp, String)]],
+    ).returns(T::Array[[T.any(Regexp, String), T.any(Regexp, String)]])
+  }
+  def replace_version_and_checksum(cask, new_hash, new_version, replacement_pairs)
+    # When blocks are absent, arch is not relevant. For consistency, we simulate the arm architecture.
+    arch_options = cask.on_system_blocks_exist? ? OnSystem::ARCH_OPTIONS : [:arm]
+    arch_options.each do |arch|
+      SimulateSystem.with arch: arch do
+        old_cask     = Cask::CaskLoader.load(cask.sourcefile_path)
+        old_version  = old_cask.version
+        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,
+                              "version #{bump_version.latest? ? ":latest" : %Q("#{bump_version}")}"]
+
+        # We are replacing our version here so we can get the new hash
+        tmp_contents = Utils::Inreplace.inreplace_pairs(cask.sourcefile_path,
+                                                        replacement_pairs.uniq.compact,
+                                                        read_only_run: true,
+                                                        silent:        true)
+
+        tmp_cask = Cask::CaskLoader.load(tmp_contents)
+        old_hash = tmp_cask.sha256
+        if tmp_cask.version.latest? || new_hash == :no_check
+          opoo "Ignoring specified `--sha256=` argument." if new_hash.is_a?(String)
+          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 old_hash != :no_check
+          if new_hash && cask.languages.present?
+            opoo "Multiple checksum replacements required; ignoring specified `--sha256` argument."
+          end
+          languages = if cask.languages.empty?
+            [nil]
+          else
+            cask.languages
+          end
+          languages.each do |language|
+            new_cask        = Cask::CaskLoader.load(tmp_contents)
+            new_cask.config = if language.blank?
+              tmp_cask.config
+            else
+              tmp_cask.config.merge(Cask::Config.new(explicit: { languages: [language] }))
+            end
+            download = Cask::Download.new(new_cask, quarantine: true).fetch(verify_download_integrity: false)
+            Utils::Tar.validate_file(download)
+
+            if new_cask.sha256.to_s != download.sha256
+              replacement_pairs << [new_cask.sha256.to_s,
+                                    download.sha256]
+            end
+          end
+        elsif new_hash
+          opoo "Cask contains multiple hashes; only updating hash for current arch." if cask.on_system_blocks_exist?
+          replacement_pairs << [old_hash.to_s, new_hash]
+        end
+      end
+    end
+    replacement_pairs
   end
 
+  sig { params(cask: Cask::Cask, args: CLI::Args, new_version: BumpVersionParser).void }
+  def check_pull_requests(cask, args:, new_version:)
+    tap_remote_repo = cask.tap.full_name || cask.tap.remote_repo
+
+    GitHub.check_for_duplicate_pull_requests(cask.token, tap_remote_repo,
+                                             state:   "open",
+                                             version: nil,
+                                             file:    cask.sourcefile_path.relative_path_from(cask.tap.path).to_s,
+                                             args:    args)
+
+    # if we haven't already found open requests, try for an exact match across closed requests
+    new_version.instance_variables.each do |version_type|
+      version = new_version.instance_variable_get(version_type)
+      next if version.blank?
+
+      GitHub.check_for_duplicate_pull_requests(
+        cask.token,
+        tap_remote_repo,
+        state:   "closed",
+        version: version,
+        file:    cask.sourcefile_path.relative_path_from(cask.tap.path).to_s,
+        args:    args,
+      )
+    end
+  end
+
+  sig { params(cask: Cask::Cask, old_contents: String, args: T.untyped).void }
   def run_cask_audit(cask, old_contents, args:)
     if args.dry_run?
       if args.no_audit?
@@ -249,6 +291,7 @@ module Homebrew
     odie "`brew audit` failed!"
   end
 
+  sig { params(cask: Cask::Cask, old_contents: String, args: T.untyped).void }
   def run_cask_style(cask, old_contents, args:)
     if args.dry_run?
       if args.no_style?

Library/Homebrew/test/bump_version_parser_spec.rb

@@ -0,0 +1,72 @@
+# frozen_string_literal: true
+
+require "bump_version_parser"
+
+describe Homebrew::BumpVersionParser do
+  let(:general_version) { "1.2.3" }
+  let(:intel_version) { "2.3.4" }
+  let(:arm_version) { "3.4.5" }
+
+  context "when initializing with no versions" do
+    it "raises a usage error" do
+      expect do
+        described_class.new
+      end.to raise_error(UsageError, "Invalid usage: `--version` must not be empty.")
+    end
+  end
+
+  context "when initializing with valid versions" do
+    let(:new_version) { described_class.new(general: general_version, arm: arm_version, intel: intel_version) }
+
+    it "correctly parses general version" do
+      expect(new_version.general).to eq(Cask::DSL::Version.new(general_version.to_s))
+    end
+
+    it "correctly parses arm version" do
+      expect(new_version.arm).to eq(Cask::DSL::Version.new(arm_version.to_s))
+    end
+
+    it "correctly parses intel version" do
+      expect(new_version.intel).to eq(Cask::DSL::Version.new(intel_version.to_s))
+    end
+
+    context "when only the intel version is provided" do
+      it "raises a UsageError" do
+        expect do
+          described_class.new(intel: intel_version)
+        end.to raise_error(UsageError,
+                           "Invalid usage: `--version-arm` must not be empty.")
+      end
+    end
+
+    context "when only the arm version is provided" do
+      it "raises a UsageError" do
+        expect do
+          described_class.new(arm: arm_version)
+        end.to raise_error(UsageError,
+                           "Invalid usage: `--version-intel` must not be empty.")
+      end
+    end
+
+    context "when the version is latest" do
+      it "returns a version object for latest" do
+        new_version = described_class.new(general: "latest")
+        expect(new_version.general.to_s).to eq("latest")
+      end
+
+      context "when the version is not latest" do
+        it "returns a version object for the given version" do
+          new_version = described_class.new(general: general_version)
+          expect(new_version.general.to_s).to eq(general_version)
+        end
+      end
+    end
+
+    context "when checking if VersionParser is blank" do
+      it "returns false if any version is present" do
+        new_version = described_class.new(general: general_version.to_s, arm: "", intel: "")
+        expect(new_version.blank?).to be(false)
+      end
+    end
+  end
+end

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

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 require "cmd/shared_examples/args_parse"
+require "dev-cmd/bump-cask-pr"
 
 describe "brew bump-cask-pr" do
   it_behaves_like "parseable arguments"

completions/bash/brew

@@ -478,6 +478,8 @@ _brew_bump_cask_pr() {
       --url
       --verbose
       --version
+      --version-arm
+      --version-intel
       --write-only
       "
       return

completions/fish/brew.fish

@@ -412,6 +412,8 @@ __fish_brew_complete_arg 'bump-cask-pr' -l sha256 -d 'Specify the SHA-256 checks
 __fish_brew_complete_arg 'bump-cask-pr' -l url -d 'Specify the URL for the new download'
 __fish_brew_complete_arg 'bump-cask-pr' -l verbose -d 'Make some output more verbose'
 __fish_brew_complete_arg 'bump-cask-pr' -l version -d 'Specify the new version for the cask'
+__fish_brew_complete_arg 'bump-cask-pr' -l version-arm -d 'Specify the new cask version for the ARM architecture'
+__fish_brew_complete_arg 'bump-cask-pr' -l version-intel -d 'Specify the new cask version for the Intel architecture'
 __fish_brew_complete_arg 'bump-cask-pr' -l write-only -d 'Make the expected file modifications without taking any Git actions'
 __fish_brew_complete_arg 'bump-cask-pr' -a '(__fish_brew_suggest_casks_all)'
 

completions/zsh/_brew

@@ -527,7 +527,9 @@ _brew_bump_cask_pr() {
     '--sha256[Specify the SHA-256 checksum of the new download]' \
     '--url[Specify the URL for the new download]' \
     '--verbose[Make some output more verbose]' \
-    '--version[Specify the new version for the cask]' \
+    '(--version-arm --version-intel)--version[Specify the new version for the cask]' \
+    '(--version)--version-arm[Specify the new cask version for the ARM architecture]' \
+    '(--version)--version-intel[Specify the new cask version for the Intel architecture]' \
     '--write-only[Make the expected file modifications without taking any Git actions]' \
     - cask \
     '*::cask:__brew_casks'

docs/Manpage.md

@@ -1048,6 +1048,10 @@ supplied by the user.
   Don't try to fork the repository.
 * `--version`:
   Specify the new *`version`* for the cask.
+* `--version-arm`:
+  Specify the new cask *`version`* for the ARM architecture.
+* `--version-intel`:
+  Specify the new cask *`version`* for the Intel architecture.
 * `--message`:
   Prepend *`message`* to the default pull request message.
 * `--url`:

manpages/brew.1

@@ -1500,6 +1500,14 @@ Don\'t try to fork the repository\.
 Specify the new \fIversion\fR for the cask\.
 .
 .TP
+\fB\-\-version\-arm\fR
+Specify the new cask \fIversion\fR for the ARM architecture\.
+.
+.TP
+\fB\-\-version\-intel\fR
+Specify the new cask \fIversion\fR for the Intel architecture\.
+.
+.TP
 \fB\-\-message\fR
 Prepend \fImessage\fR to the default pull request message\.
 .