livecheck: move #preprocess_url into strategies

`Livecheck#preprocess_url` only contains logic for rewriting Git URLs, so it makes more sense for this code to be part of the `Git` strategy instead. Outside of better code organization, this saves us from having to maintain the list of strategies to skip processing (which is sometimes forgotten when a new strategy is added) and makes it easier to do something similar in other strategies as needed. One thing to note is that `Livecheck#preprocess_url` was previously called on the URL before each strategy's `#match?` method was called. To maintain the existing behavior, this calls `Git#preprocess_url` in `Git#match?`. However, we need the processed URL when we use the `Git` strategy, so we have to call `Git#preprocess_url` again. To avoid duplicating effort, I've added a `@processed_urls` hash to the `Git` strategy and have set up `Git#preprocess_url` to cache processed URLs, so we only do the work once. There may be a better way of handling it but this seems to work as expected.
Homebrew · Sep 28, 2024 · 9e47fc9 · 9e47fc9
1 parent da74ca9
commit 9e47fc9
Show file tree

Hide file tree

Showing 4 changed files with 163 additions and 162 deletions.
diff --git a/Library/Homebrew/livecheck/livecheck.rb b/Library/Homebrew/livecheck/livecheck.rb
@@ -13,31 +13,6 @@ module Homebrew
   # command. These methods print the requested livecheck information
   # for formulae.
   module Livecheck
-    GITEA_INSTANCES = T.let(%w[
-      codeberg.org
-      gitea.com
-      opendev.org
-      tildegit.org
-    ].freeze, T::Array[String])
-    private_constant :GITEA_INSTANCES
-
-    GOGS_INSTANCES = T.let(%w[
-      lolg.it
-    ].freeze, T::Array[String])
-    private_constant :GOGS_INSTANCES
-
-    STRATEGY_SYMBOLS_TO_SKIP_PREPROCESS_URL = T.let([
-      :extract_plist,
-      :github_latest,
-      :header_match,
-      :json,
-      :page_match,
-      :sparkle,
-      :xml,
-      :yaml,
-    ].freeze, T::Array[Symbol])
-    private_constant :STRATEGY_SYMBOLS_TO_SKIP_PREPROCESS_URL
-
     UNSTABLE_VERSION_KEYWORDS = T.let(%w[
       alpha
       beta
@@ -573,48 +548,6 @@ def self.checkable_urls(package_or_resource)
       urls.compact.uniq
     end
 
-    # Preprocesses and returns the URL used by livecheck.
-    sig { params(url: String).returns(String) }
-    def self.preprocess_url(url)
-      begin
-        uri = Addressable::URI.parse url
-      rescue Addressable::URI::InvalidURIError
-        return url
-      end
-
-      host = uri.host
-      path = uri.path
-      return url if host.nil? || path.nil?
-
-      host = "github.com" if host == "github.s3.amazonaws.com"
-      path = path.delete_prefix("/").delete_suffix(".git")
-      scheme = uri.scheme
-
-      if host == "github.com"
-        return url if path.match? %r{/releases/latest/?$}
-
-        owner, repo = path.delete_prefix("downloads/").split("/")
-        url = "#{scheme}://#{host}/#{owner}/#{repo}.git"
-      elsif GITEA_INSTANCES.include?(host)
-        return url if path.match? %r{/releases/latest/?$}
-
-        owner, repo = path.split("/")
-        url = "#{scheme}://#{host}/#{owner}/#{repo}.git"
-      elsif GOGS_INSTANCES.include?(host)
-        owner, repo = path.split("/")
-        url = "#{scheme}://#{host}/#{owner}/#{repo}.git"
-      # sourcehut
-      elsif host == "git.sr.ht"
-        owner, repo = path.split("/")
-        url = "#{scheme}://#{host}/#{owner}/#{repo}"
-      # GitLab (gitlab.com or self-hosted)
-      elsif path.include?("/-/archive/")
-        url = url.sub(%r{/-/archive/.*$}i, ".git")
-      end
-
-      url
-    end
-
     # livecheck should fetch a URL using brewed curl if the formula/cask
     # contains a `stable`/`url` or `head` URL `using: :homebrew_curl` that
     # shares the same root domain.
@@ -705,12 +638,7 @@ def self.latest_version(
 
       checked_urls = []
       urls.each_with_index do |original_url, i|
-        # Only preprocess the URL when it's appropriate
-        url = if STRATEGY_SYMBOLS_TO_SKIP_PREPROCESS_URL.include?(livecheck_strategy)
-          original_url
-        else
-          preprocess_url(original_url)
-        end
+        url = original_url
         next if checked_urls.include?(url)
 
         strategies = Strategy.from_url(
@@ -722,6 +650,11 @@ def self.latest_version(
         strategy = Strategy.from_symbol(livecheck_strategy) || strategies.first
         strategy_name = livecheck_strategy_names[strategy]
 
+        if strategy.respond_to?(:preprocess_url)
+          url = strategy.preprocess_url(url)
+          next if checked_urls.include?(url)
+        end
+
         if debug
           puts
           if livecheck_url.is_a?(Symbol)
@@ -921,10 +854,6 @@ def self.resource_version(
       checked_urls = []
       urls.each_with_index do |original_url, i|
         url = original_url.gsub(Constants::LATEST_VERSION, formula_latest)
-
-        # Only preprocess the URL when it's appropriate
-        url = preprocess_url(url) unless STRATEGY_SYMBOLS_TO_SKIP_PREPROCESS_URL.include?(livecheck_strategy)
-
         next if checked_urls.include?(url)
 
         strategies = Strategy.from_url(
@@ -936,6 +865,11 @@ def self.resource_version(
         strategy = Strategy.from_symbol(livecheck_strategy) || strategies.first
         strategy_name = livecheck_strategy_names[strategy]
 
+        if strategy.respond_to?(:preprocess_url)
+          url = strategy.preprocess_url(url)
+          next if checked_urls.include?(url)
+        end
+
         if debug
           puts
           if livecheck_url.is_a?(Symbol)

diff --git a/Library/Homebrew/livecheck/strategy/git.rb b/Library/Homebrew/livecheck/strategy/git.rb
@@ -1,6 +1,7 @@
 # typed: strict
 # frozen_string_literal: true
 
+require "addressable"
 require "system_command"
 
 module Homebrew
@@ -26,6 +27,9 @@ module Strategy
       class Git
         extend SystemCommand::Mixin
 
+        # Used to cache processed URLs, to avoid duplicating effort.
+        @processed_urls = T.let({}, T::Hash[String, String])
+
         # The priority of the strategy on an informal scale of 1 to 10 (from
         # lowest to highest).
         PRIORITY = 8
@@ -34,12 +38,75 @@ class Git
         # regex isn't provided.
         DEFAULT_REGEX = /\D*(.+)/
 
+        GITEA_INSTANCES = T.let(%w[
+          codeberg.org
+          gitea.com
+          opendev.org
+          tildegit.org
+        ].freeze, T::Array[String])
+        private_constant :GITEA_INSTANCES
+
+        GOGS_INSTANCES = T.let(%w[
+          lolg.it
+        ].freeze, T::Array[String])
+        private_constant :GOGS_INSTANCES
+
+        # Processes and returns the URL used by livecheck.
+        sig { params(url: String).returns(String) }
+        def self.preprocess_url(url)
+          processed_url = @processed_urls[url]
+          return processed_url if processed_url
+
+          begin
+            uri = Addressable::URI.parse url
+          rescue Addressable::URI::InvalidURIError
+            return url
+          end
+
+          host = uri.host
+          path = uri.path
+          return url if host.nil? || path.nil?
+
+          host = "github.com" if host == "github.s3.amazonaws.com"
+          path = path.delete_prefix("/").delete_suffix(".git")
+          scheme = uri.scheme
+
+          if host == "github.com"
+            return url if path.match? %r{/releases/latest/?$}
+
+            owner, repo = path.delete_prefix("downloads/").split("/")
+            processed_url = "#{scheme}://#{host}/#{owner}/#{repo}.git"
+          elsif GITEA_INSTANCES.include?(host)
+            return url if path.match? %r{/releases/latest/?$}
+
+            owner, repo = path.split("/")
+            processed_url = "#{scheme}://#{host}/#{owner}/#{repo}.git"
+          elsif GOGS_INSTANCES.include?(host)
+            owner, repo = path.split("/")
+            processed_url = "#{scheme}://#{host}/#{owner}/#{repo}.git"
+          # sourcehut
+          elsif host == "git.sr.ht"
+            owner, repo = path.split("/")
+            processed_url = "#{scheme}://#{host}/#{owner}/#{repo}"
+          # GitLab (gitlab.com or self-hosted)
+          elsif path.include?("/-/archive/")
+            processed_url = url.sub(%r{/-/archive/.*$}i, ".git")
+          end
+
+          if processed_url && (processed_url != url)
+            @processed_urls[url] = processed_url
+          else
+            url
+          end
+        end
+
         # Whether the strategy can be applied to the provided URL.
         #
         # @param url [String] the URL to match against
         # @return [Boolean]
         sig { params(url: String).returns(T::Boolean) }
         def self.match?(url)
+          url = preprocess_url(url)
           (DownloadStrategyDetector.detect(url) <= GitDownloadStrategy) == true
         end
 

diff --git a/Library/Homebrew/test/livecheck/livecheck_spec.rb b/Library/Homebrew/test/livecheck/livecheck_spec.rb
@@ -256,89 +256,4 @@
       expect(livecheck.use_homebrew_curl?(f_homebrew_curl, "test")).to be(false)
     end
   end
-
-  describe "::preprocess_url" do
-    let(:github_git_url_with_extension) { "https://github.com/Homebrew/brew.git" }
-
-    it "returns the unmodified URL for an unparsable URL" do
-      # Modeled after the `head` URL in the `ncp` formula
-      expect(livecheck.preprocess_url(":something:cvs:@cvs.brew.sh:/cvs"))
-        .to eq(":something:cvs:@cvs.brew.sh:/cvs")
-    end
-
-    it "returns the unmodified URL for a GitHub URL ending in .git" do
-      expect(livecheck.preprocess_url(github_git_url_with_extension))
-        .to eq(github_git_url_with_extension)
-    end
-
-    it "returns the Git repository URL for a GitHub URL not ending in .git" do
-      expect(livecheck.preprocess_url("https://github.com/Homebrew/brew"))
-        .to eq(github_git_url_with_extension)
-    end
-
-    it "returns the unmodified URL for a GitHub /releases/latest URL" do
-      expect(livecheck.preprocess_url("https://github.com/Homebrew/brew/releases/latest"))
-        .to eq("https://github.com/Homebrew/brew/releases/latest")
-    end
-
-    it "returns the Git repository URL for a GitHub AWS URL" do
-      expect(livecheck.preprocess_url("https://github.s3.amazonaws.com/downloads/Homebrew/brew/1.0.0.tar.gz"))
-        .to eq(github_git_url_with_extension)
-    end
-
-    it "returns the Git repository URL for a github.com/downloads/... URL" do
-      expect(livecheck.preprocess_url("https://github.com/downloads/Homebrew/brew/1.0.0.tar.gz"))
-        .to eq(github_git_url_with_extension)
-    end
-
-    it "returns the Git repository URL for a GitHub tag archive URL" do
-      expect(livecheck.preprocess_url("https://github.com/Homebrew/brew/archive/1.0.0.tar.gz"))
-        .to eq(github_git_url_with_extension)
-    end
-
-    it "returns the Git repository URL for a GitHub release archive URL" do
-      expect(livecheck.preprocess_url("https://github.com/Homebrew/brew/releases/download/1.0.0/brew-1.0.0.tar.gz"))
-        .to eq(github_git_url_with_extension)
-    end
-
-    it "returns the Git repository URL for a gitlab.com archive URL" do
-      expect(livecheck.preprocess_url("https://gitlab.com/Homebrew/brew/-/archive/1.0.0/brew-1.0.0.tar.gz"))
-        .to eq("https://gitlab.com/Homebrew/brew.git")
-    end
-
-    it "returns the Git repository URL for a self-hosted GitLab archive URL" do
-      expect(livecheck.preprocess_url("https://brew.sh/Homebrew/brew/-/archive/1.0.0/brew-1.0.0.tar.gz"))
-        .to eq("https://brew.sh/Homebrew/brew.git")
-    end
-
-    it "returns the Git repository URL for a Codeberg archive URL" do
-      expect(livecheck.preprocess_url("https://codeberg.org/Homebrew/brew/archive/brew-1.0.0.tar.gz"))
-        .to eq("https://codeberg.org/Homebrew/brew.git")
-    end
-
-    it "returns the Git repository URL for a Gitea archive URL" do
-      expect(livecheck.preprocess_url("https://gitea.com/Homebrew/brew/archive/brew-1.0.0.tar.gz"))
-        .to eq("https://gitea.com/Homebrew/brew.git")
-    end
-
-    it "returns the Git repository URL for an Opendev archive URL" do
-      expect(livecheck.preprocess_url("https://opendev.org/Homebrew/brew/archive/brew-1.0.0.tar.gz"))
-        .to eq("https://opendev.org/Homebrew/brew.git")
-    end
-
-    it "returns the Git repository URL for a tildegit archive URL" do
-      expect(livecheck.preprocess_url("https://tildegit.org/Homebrew/brew/archive/brew-1.0.0.tar.gz"))
-        .to eq("https://tildegit.org/Homebrew/brew.git")
-    end
-
-    it "returns the Git repository URL for a LOL Git archive URL" do
-      expect(livecheck.preprocess_url("https://lolg.it/Homebrew/brew/archive/brew-1.0.0.tar.gz"))
-        .to eq("https://lolg.it/Homebrew/brew.git")
-    end
-
-    it "returns the Git repository URL for a sourcehut archive URL" do
-      expect(livecheck.preprocess_url("https://git.sr.ht/~Homebrew/brew/archive/1.0.0.tar.gz"))
-        .to eq("https://git.sr.ht/~Homebrew/brew")
-    end
-  end
 end