From fb8a4e565836e33f2195ff49eaf10535d8bb78a0 Mon Sep 17 00:00:00 2001 From: Sam Ford <1584702+samford@users.noreply.github.com> Date: Tue, 17 May 2022 00:34:32 -0400 Subject: [PATCH] Livecheck: Use Homebrew curl based on root domain At the moment, `#use_homebrew_curl?` can only be true for a `homepage` or `stable`/cask `url` with `using: :homebrew_curl`. If the checked URL differs from these URLs, livecheck won't use brewed curl. This limitation prevents livecheck from using brewed curl for a `livecheck` block URL that's a string literal (not a symbol for a `#checkable_url` like `:stable`, `:head`, `:url`). `libzip` was the original formula referenced in the related brew issue and it meets this criterion, so it doesn't appear to be handled by the existing `#use_homebrew_curl?` implementation. Additionally, the existing behavior can cause livecheck to unnecessarily use brewed curl for a completely different website (e.g., `cubelib`, `otf2`). For example, if the `stable` URL has `using: :homebrew_curl` and the `livecheck` block has `url :homepage`, livecheck will use brewed curl when checking the `homepage`. If these are completely different domains/servers, it's unlikely that we would need to use brewed curl when checking the `homepage`, so this particular behavior may not be beneficial. This commit reimplements `use_homebrew_curl?` to apply brewed curl when the checked URL's root domain is the same as the root domain of an aforementioned formula/cask URL with `using: :homebrew_curl`. For example, this looser approach would allow a `livecheck` block checking `https://www.example.com/downloads/` to use brewed curl if the `stable` URL was `https://downloads.example.com/example.zip` with `using: :homebrew_curl`. These could be different servers but, based on related formulae, this looseness is necessary for the moment. This approach aims to resolve both issues, allowing brewed curl to be applied to a slightly broader range of URLs (i.e., not limited to just the `#checkable_urls`) while also helping to avoid unnecessarily applying brewed curl when it's less likely to be useful (completely different domains). Neither approach is perfect but this one may be more useful in the interim time. Depending on how this looser approach works in practice, we may want to consider returning to a stricter approach once we have something like `using: :homebrew_curl` in `livecheck` blocks (this is forthcoming). Being explicit in a `livecheck` block is the most reliable approach (i.e., only use brewed curl when needed), so we could favor that and pare down the automated approach to only what's needed to support implicit checks (i.e., with no `livecheck` block). Of course, it's also possible to drop the automated approach entirely and simply require a `livecheck` block in this scenario but we can decide on how to handle this when the time comes. --- Library/Homebrew/Gemfile | 1 + Library/Homebrew/Gemfile.lock | 1 + Library/Homebrew/livecheck/livecheck.rb | 40 +++++++---- .../Homebrew/test/livecheck/livecheck_spec.rb | 68 +++++++++++++++++-- 4 files changed, 88 insertions(+), 22 deletions(-) diff --git a/Library/Homebrew/Gemfile b/Library/Homebrew/Gemfile index e644c41c96..d869363722 100644 --- a/Library/Homebrew/Gemfile +++ b/Library/Homebrew/Gemfile @@ -6,6 +6,7 @@ source "https://rubygems.org" # * nokogiri - use rexml instead for XML parsing # installed gems (should all be require: false) +gem "addressable", require: false gem "bootsnap", require: false gem "byebug", require: false gem "json_schemer", require: false diff --git a/Library/Homebrew/Gemfile.lock b/Library/Homebrew/Gemfile.lock index df52b3f7bb..85f9f66110 100644 --- a/Library/Homebrew/Gemfile.lock +++ b/Library/Homebrew/Gemfile.lock @@ -201,6 +201,7 @@ PLATFORMS DEPENDENCIES activesupport (< 7) + addressable bootsnap byebug concurrent-ruby diff --git a/Library/Homebrew/livecheck/livecheck.rb b/Library/Homebrew/livecheck/livecheck.rb index 1ac6b31b74..1f03af6e96 100644 --- a/Library/Homebrew/livecheck/livecheck.rb +++ b/Library/Homebrew/livecheck/livecheck.rb @@ -5,6 +5,7 @@ require "livecheck/error" require "livecheck/livecheck_version" require "livecheck/skip_conditions" require "livecheck/strategy" +require "addressable" require "ruby-progressbar" require "uri" @@ -529,26 +530,33 @@ module Homebrew url end - # Fetch with brewed curl if using the download or homepage URL - # and the download URL specifies `using: :homebrew_curl`. + # 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. sig { params(formula_or_cask: T.any(Formula, Cask::Cask), url: String).returns(T::Boolean) } def use_homebrew_curl?(formula_or_cask, url) - if checkable_urls(formula_or_cask).include?(url) - case formula_or_cask - when Formula - [:stable, :head].any? do |spec_name| - next false unless (spec = formula_or_cask.send(spec_name)) + url_root_domain = Addressable::URI.parse(url)&.domain + return false if url_root_domain.blank? - spec.using == :homebrew_curl - end - when Cask::Cask - formula_or_cask.url.using == :homebrew_curl - else - T.absurd(formula_or_cask) + # Collect root domains of URLs with `using: :homebrew_curl` + homebrew_curl_root_domains = [] + case formula_or_cask + when Formula + [:stable, :head].each do |spec_name| + next unless (spec = formula_or_cask.send(spec_name)) + next unless spec.using == :homebrew_curl + + domain = Addressable::URI.parse(spec.url)&.domain + homebrew_curl_root_domains << domain if domain.present? end - else - false + when Cask::Cask + return false unless formula_or_cask.url.using == :homebrew_curl + + domain = Addressable::URI.parse(formula_or_cask.url.to_s)&.domain + homebrew_curl_root_domains << domain if domain.present? end + + homebrew_curl_root_domains.include?(url_root_domain) end # Identifies the latest version of the formula and returns a Hash containing @@ -662,6 +670,7 @@ module Homebrew when "PageMatch", "HeaderMatch" use_homebrew_curl?((referenced_formula_or_cask || formula_or_cask), url) end + puts "Homebrew curl?: Yes" if debug && homebrew_curl.present? strategy_data = strategy.find_versions( url: url, @@ -743,6 +752,7 @@ module Homebrew version_info[:meta][:url][:strategy] = strategy_data[:url] end version_info[:meta][:url][:final] = strategy_data[:final_url] if strategy_data[:final_url] + version_info[:meta][:url][:homebrew_curl] = homebrew_curl if homebrew_curl.present? version_info[:meta][:strategy] = strategy.present? ? strategy_name : nil version_info[:meta][:strategies] = strategies.map { |s| livecheck_strategy_names[s] } if strategies.present? diff --git a/Library/Homebrew/test/livecheck/livecheck_spec.rb b/Library/Homebrew/test/livecheck/livecheck_spec.rb index f2fbb3f97f..9b68e3f94b 100644 --- a/Library/Homebrew/test/livecheck/livecheck_spec.rb +++ b/Library/Homebrew/test/livecheck/livecheck_spec.rb @@ -16,7 +16,7 @@ describe Homebrew::Livecheck do formula("test") do desc "Test formula" homepage "https://brew.sh" - url "https://brew.sh/test-0.0.1.tgz", using: :homebrew_curl + url "https://brew.sh/test-0.0.1.tgz" head "https://github.com/Homebrew/brew.git" livecheck do @@ -31,7 +31,7 @@ describe Homebrew::Livecheck do cask "test" do version "0.0.1,2" - url "https://brew.sh/test-0.0.1.dmg", using: :homebrew_curl + url "https://brew.sh/test-0.0.1.dmg" name "Test" desc "Test cask" homepage "https://brew.sh" @@ -158,13 +158,67 @@ describe Homebrew::Livecheck do end describe "::use_homebrew_curl?" do - it "uses brewed curl if called for by the download URL" do + let(:example_url) { "https://www.example.com/test-0.0.1.tgz" } + + let(:f_homebrew_curl) do + formula("test") do + desc "Test formula" + homepage "https://brew.sh" + url "https://brew.sh/test-0.0.1.tgz", using: :homebrew_curl + # head is deliberably omitted to exercise more of the method + + livecheck do + url "https://formulae.brew.sh/api/formula/ruby.json" + regex(/"stable":"(\d+(?:\.\d+)+)"/i) + end + end + end + + let(:c_homebrew_curl) do + Cask::CaskLoader.load(+<<-RUBY) + cask "test" do + version "0.0.1,2" + + url "https://brew.sh/test-0.0.1.dmg", using: :homebrew_curl + name "Test" + desc "Test cask" + homepage "https://brew.sh" + + livecheck do + url "https://formulae.brew.sh/api/formula/ruby.json" + regex(/"stable":"(\d+(?:\.\d+)+)"/i) + end + end + RUBY + end + + it "returns `true` when URL matches a `using: :homebrew_curl` URL" do + expect(livecheck.use_homebrew_curl?(f_homebrew_curl, livecheck_url)).to be(true) + expect(livecheck.use_homebrew_curl?(f_homebrew_curl, homepage_url)).to be(true) + expect(livecheck.use_homebrew_curl?(f_homebrew_curl, stable_url)).to be(true) + expect(livecheck.use_homebrew_curl?(c_homebrew_curl, livecheck_url)).to be(true) + expect(livecheck.use_homebrew_curl?(c_homebrew_curl, homepage_url)).to be(true) + expect(livecheck.use_homebrew_curl?(c_homebrew_curl, cask_url)).to be(true) + end + + it "returns `false` if URL root domain differs from `using: :homebrew_curl` URLs" do + expect(livecheck.use_homebrew_curl?(f_homebrew_curl, example_url)).to be(false) + expect(livecheck.use_homebrew_curl?(c_homebrew_curl, example_url)).to be(false) + end + + it "returns `false` if a `using: homebrew_curl` URL is not present" do expect(livecheck.use_homebrew_curl?(f, livecheck_url)).to be(false) - expect(livecheck.use_homebrew_curl?(f, homepage_url)).to be(true) - expect(livecheck.use_homebrew_curl?(f, stable_url)).to be(true) + expect(livecheck.use_homebrew_curl?(f, homepage_url)).to be(false) + expect(livecheck.use_homebrew_curl?(f, stable_url)).to be(false) + expect(livecheck.use_homebrew_curl?(f, example_url)).to be(false) expect(livecheck.use_homebrew_curl?(c, livecheck_url)).to be(false) - expect(livecheck.use_homebrew_curl?(c, homepage_url)).to be(true) - expect(livecheck.use_homebrew_curl?(c, cask_url)).to be(true) + expect(livecheck.use_homebrew_curl?(c, homepage_url)).to be(false) + expect(livecheck.use_homebrew_curl?(c, cask_url)).to be(false) + expect(livecheck.use_homebrew_curl?(c, example_url)).to be(false) + end + + it "returns `false` if URL string does not contain a domain" do + expect(livecheck.use_homebrew_curl?(f_homebrew_curl, "test")).to be(false) end end