From 9e47fc9f31a6da568a64c30b9643a2cc00f53233 Mon Sep 17 00:00:00 2001 From: Sam Ford <1584702+samford@users.noreply.github.com> Date: Sat, 28 Sep 2024 10:22:12 -0400 Subject: [PATCH] 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. --- Library/Homebrew/livecheck/livecheck.rb | 88 +++---------------- Library/Homebrew/livecheck/strategy/git.rb | 67 ++++++++++++++ .../Homebrew/test/livecheck/livecheck_spec.rb | 85 ------------------ .../test/livecheck/strategy/git_spec.rb | 85 ++++++++++++++++++ 4 files changed, 163 insertions(+), 162 deletions(-) diff --git a/Library/Homebrew/livecheck/livecheck.rb b/Library/Homebrew/livecheck/livecheck.rb index 61bca36115188..dc89b367e57f2 100644 --- 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 index 2372802adcd50..b263404888cfc 100644 --- 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 index 1e18dfe666044..aa489b711b3b6 100644 --- 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 diff --git a/Library/Homebrew/test/livecheck/strategy/git_spec.rb b/Library/Homebrew/test/livecheck/strategy/git_spec.rb index 1afcd9c786c7c..506ebb072be40 100644 --- a/Library/Homebrew/test/livecheck/strategy/git_spec.rb +++ b/Library/Homebrew/test/livecheck/strategy/git_spec.rb @@ -37,6 +37,91 @@ 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(git.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(git.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(git.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(git.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(git.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(git.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(git.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(git.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(git.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(git.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(git.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(git.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(git.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(git.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(git.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(git.preprocess_url("https://git.sr.ht/~Homebrew/brew/archive/1.0.0.tar.gz")) + .to eq("https://git.sr.ht/~Homebrew/brew") + end + end + describe "::match?" do it "returns true for a Git repository URL" do expect(git.match?(git_url)).to be true