Merge pull request #18812 from Homebrew/bump-skip-pr-checking-when-up-to-date

bump: skip PR checking when up to date or livecheck fails

Library/Homebrew/bump_version_parser.rb

@@ -48,5 +48,12 @@ module Homebrew
     def blank?
       @general.blank? && @arm.blank? && @intel.blank?
     end
+
+    sig { params(other: T.untyped).returns(T::Boolean) }
+    def ==(other)
+      return false unless other.is_a?(BumpVersionParser)
+
+      (general == other.general) && (arm == other.arm) && (intel == other.intel)
+    end
   end
 end

Library/Homebrew/dev-cmd/bump.rb

@@ -277,8 +277,9 @@ module Homebrew
           odebug "Error fetching pull requests for #{formula_or_cask} #{name}: #{e}"
           nil
         end
+        return if pull_requests.blank?
 
-        pull_requests&.map { |pr| "#{pr["title"]} (#{Formatter.url(pr["html_url"])})" }&.join(", ")
+        pull_requests.map { |pr| "#{pr["title"]} (#{Formatter.url(pr["html_url"])})" }.join(", ")
       end
 
       sig {
@@ -365,22 +366,28 @@ module Homebrew
           new_version = BumpVersionParser.new(general: "unable to get versions")
         end
 
-        # We use the arm version for the pull request version. This is consistent
+        if !args.no_pull_requests? &&
-        # with the behavior of bump-cask-pr.
+           (new_version.general != "unable to get versions") &&
-        pull_request_version = if multiple_versions && new_version.general != "unable to get versions"
+           (new_version != current_version)
-          new_version.arm.to_s
+          # We use the ARM version for the pull request version. This is
-        else
+          # consistent with the behavior of bump-cask-pr.
-          new_version.general.to_s
+          pull_request_version = if multiple_versions
+            new_version.arm.to_s
+          else
+            new_version.general.to_s
+          end
+
+          duplicate_pull_requests = retrieve_pull_requests(
+            formula_or_cask,
+            name,
+            version: pull_request_version,
+          )
+
+          maybe_duplicate_pull_requests = if duplicate_pull_requests.nil?
+            retrieve_pull_requests(formula_or_cask, name)
+          end
         end
 
-        duplicate_pull_requests = unless args.no_pull_requests?
-          retrieve_pull_requests(formula_or_cask, name, version: pull_request_version)
-        end.presence
-
-        maybe_duplicate_pull_requests = if !args.no_pull_requests? && duplicate_pull_requests.blank?
-          retrieve_pull_requests(formula_or_cask, name)
-        end.presence
-
         VersionBumpInfo.new(
           type:,
           multiple_versions:,
@@ -411,9 +418,7 @@ module Homebrew
         repology_latest = version_info.repology_latest
 
         # Check if all versions are equal
-        versions_equal = [:arm, :intel, :general].all? do |key|
+        versions_equal = (new_version == current_version)
-          current_version.send(key) == new_version.send(key)
-        end
 
         title_name = ambiguous_cask ? "#{name} (cask)" : name
         title = if (repology_latest == current_version.general || !repology_latest.is_a?(Version)) && versions_equal
@@ -439,8 +444,8 @@ module Homebrew
         end
 
         version_label = version_info.version_name
-        duplicate_pull_requests = version_info.duplicate_pull_requests.presence
+        duplicate_pull_requests = version_info.duplicate_pull_requests
-        maybe_duplicate_pull_requests = version_info.maybe_duplicate_pull_requests.presence
+        maybe_duplicate_pull_requests = version_info.maybe_duplicate_pull_requests
 
         ohai title
         puts <<~EOS
@@ -457,10 +462,24 @@ module Homebrew
                                       #{outdated_synced_formulae.join(", ")}.
           EOS
         end
-        puts <<~EOS unless args.no_pull_requests?
+        if !args.no_pull_requests? &&
-          Duplicate pull requests:       #{duplicate_pull_requests       || "none"}
+           (new_version.general != "unable to get versions") &&
-          Maybe duplicate pull requests: #{maybe_duplicate_pull_requests || "none"}
+           !versions_equal
-        EOS
+          if duplicate_pull_requests
+            duplicate_pull_requests_text = duplicate_pull_requests
+          elsif maybe_duplicate_pull_requests
+            duplicate_pull_requests_text = "none"
+            maybe_duplicate_pull_requests_text = maybe_duplicate_pull_requests
+          else
+            duplicate_pull_requests_text = "none"
+            maybe_duplicate_pull_requests_text = "none"
+          end
+
+          puts "Duplicate pull requests:  #{duplicate_pull_requests_text}"
+          if maybe_duplicate_pull_requests_text
+            puts "Maybe duplicate pull requests: #{maybe_duplicate_pull_requests_text}"
+          end
+        end
 
         return unless args.open_pr?
 

Library/Homebrew/test/bump_version_parser_spec.rb

@@ -11,41 +11,61 @@ RSpec.describe Homebrew::BumpVersionParser 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.to raise_error(UsageError,
+                         "Invalid usage: `--version` must not be empty.")
     end
   end
 
-  context "when initializing with valid versions" do
+  context "when initializing with only an intel version" 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 initializing with only an arm version" 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 initializing with arm and intel versions" do
+    let(:new_version_arm_intel) { described_class.new(arm: arm_version, intel: intel_version) }
+
+    it "correctly parses arm and intel versions" do
+      expect(new_version_arm_intel.arm).to eq(Cask::DSL::Version.new(arm_version.to_s))
+      expect(new_version_arm_intel.intel).to eq(Cask::DSL::Version.new(intel_version.to_s))
+    end
+  end
+
+  context "when initializing with all versions" do
     let(:new_version) { described_class.new(general: general_version, arm: arm_version, intel: intel_version) }
+    let(:new_version_version) do
+      described_class.new(
+        general: Version.new(general_version),
+        arm:     Version.new(arm_version),
+        intel:   Version.new(intel_version),
+      )
+    end
 
     it "correctly parses general version" do
       expect(new_version.general).to eq(Cask::DSL::Version.new(general_version.to_s))
+      expect(new_version_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))
+      expect(new_version_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
+      expect(new_version_version.intel).to eq(Cask::DSL::Version.new(intel_version.to_s))
-
-    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
@@ -69,4 +89,39 @@ RSpec.describe Homebrew::BumpVersionParser do
       end
     end
   end
+
+  describe "#==" do
+    let(:new_version) { described_class.new(general: general_version, arm: arm_version, intel: intel_version) }
+
+    context "when other object is not a `BumpVersionParser`" do
+      it "returns false" do
+        expect(new_version == Version.new("1.2.3")).to be(false)
+      end
+    end
+
+    context "when comparing objects with equal versions" do
+      it "returns true" do
+        same_version = described_class.new(general: general_version, arm: arm_version, intel: intel_version)
+        expect(new_version == same_version).to be(true)
+      end
+    end
+
+    context "when comparing objects with different versions" do
+      it "returns false" do
+        different_general_version = described_class.new(general: "3.2.1", arm: arm_version, intel: intel_version)
+        different_arm_version = described_class.new(general: general_version, arm: "4.3.2", intel: intel_version)
+        different_intel_version = described_class.new(general: general_version, arm: arm_version, intel: "5.4.3")
+        different_general_arm_versions = described_class.new(general: "3.2.1", arm: "4.3.2", intel: intel_version)
+        different_arm_intel_versions = described_class.new(general: general_version, arm: "4.3.2", intel: "5.4.3")
+        different_general_intel_versions = described_class.new(general: "3.2.1", arm: arm_version, intel: "5.4.3")
+
+        expect(new_version == different_general_version).to be(false)
+        expect(new_version == different_arm_version).to be(false)
+        expect(new_version == different_intel_version).to be(false)
+        expect(new_version == different_general_arm_versions).to be(false)
+        expect(new_version == different_arm_intel_versions).to be(false)
+        expect(new_version == different_general_intel_versions).to be(false)
+      end
+    end
+  end
 end