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.
This commit is contained in:
Sam Ford 2022-05-17 00:34:32 -04:00
parent 8a5f6645b8
commit fb8a4e5658
No known key found for this signature in database
GPG Key ID: 95209E46C7FFDEFE
4 changed files with 88 additions and 22 deletions

View File

@ -6,6 +6,7 @@ source "https://rubygems.org"
# * nokogiri - use rexml instead for XML parsing # * nokogiri - use rexml instead for XML parsing
# installed gems (should all be require: false) # installed gems (should all be require: false)
gem "addressable", require: false
gem "bootsnap", require: false gem "bootsnap", require: false
gem "byebug", require: false gem "byebug", require: false
gem "json_schemer", require: false gem "json_schemer", require: false

View File

@ -201,6 +201,7 @@ PLATFORMS
DEPENDENCIES DEPENDENCIES
activesupport (< 7) activesupport (< 7)
addressable
bootsnap bootsnap
byebug byebug
concurrent-ruby concurrent-ruby

View File

@ -5,6 +5,7 @@ require "livecheck/error"
require "livecheck/livecheck_version" require "livecheck/livecheck_version"
require "livecheck/skip_conditions" require "livecheck/skip_conditions"
require "livecheck/strategy" require "livecheck/strategy"
require "addressable"
require "ruby-progressbar" require "ruby-progressbar"
require "uri" require "uri"
@ -529,26 +530,33 @@ module Homebrew
url url
end end
# Fetch with brewed curl if using the download or homepage URL # livecheck should fetch a URL using brewed curl if the formula/cask
# and the download URL specifies `using: :homebrew_curl`. # 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) } sig { params(formula_or_cask: T.any(Formula, Cask::Cask), url: String).returns(T::Boolean) }
def use_homebrew_curl?(formula_or_cask, url) def use_homebrew_curl?(formula_or_cask, url)
if checkable_urls(formula_or_cask).include?(url) url_root_domain = Addressable::URI.parse(url)&.domain
case formula_or_cask return false if url_root_domain.blank?
when Formula
[:stable, :head].any? do |spec_name|
next false unless (spec = formula_or_cask.send(spec_name))
spec.using == :homebrew_curl # Collect root domains of URLs with `using: :homebrew_curl`
end homebrew_curl_root_domains = []
when Cask::Cask case formula_or_cask
formula_or_cask.url.using == :homebrew_curl when Formula
else [:stable, :head].each do |spec_name|
T.absurd(formula_or_cask) 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 end
else when Cask::Cask
false 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 end
homebrew_curl_root_domains.include?(url_root_domain)
end end
# Identifies the latest version of the formula and returns a Hash containing # Identifies the latest version of the formula and returns a Hash containing
@ -662,6 +670,7 @@ module Homebrew
when "PageMatch", "HeaderMatch" when "PageMatch", "HeaderMatch"
use_homebrew_curl?((referenced_formula_or_cask || formula_or_cask), url) use_homebrew_curl?((referenced_formula_or_cask || formula_or_cask), url)
end end
puts "Homebrew curl?: Yes" if debug && homebrew_curl.present?
strategy_data = strategy.find_versions( strategy_data = strategy.find_versions(
url: url, url: url,
@ -743,6 +752,7 @@ module Homebrew
version_info[:meta][:url][:strategy] = strategy_data[:url] version_info[:meta][:url][:strategy] = strategy_data[:url]
end end
version_info[:meta][:url][:final] = strategy_data[:final_url] if strategy_data[:final_url] 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][:strategy] = strategy.present? ? strategy_name : nil
version_info[:meta][:strategies] = strategies.map { |s| livecheck_strategy_names[s] } if strategies.present? version_info[:meta][:strategies] = strategies.map { |s| livecheck_strategy_names[s] } if strategies.present?

View File

@ -16,7 +16,7 @@ describe Homebrew::Livecheck do
formula("test") do formula("test") do
desc "Test formula" desc "Test formula"
homepage "https://brew.sh" 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" head "https://github.com/Homebrew/brew.git"
livecheck do livecheck do
@ -31,7 +31,7 @@ describe Homebrew::Livecheck do
cask "test" do cask "test" do
version "0.0.1,2" 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" name "Test"
desc "Test cask" desc "Test cask"
homepage "https://brew.sh" homepage "https://brew.sh"
@ -158,13 +158,67 @@ describe Homebrew::Livecheck do
end end
describe "::use_homebrew_curl?" do 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, livecheck_url)).to be(false)
expect(livecheck.use_homebrew_curl?(f, homepage_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(true) 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, livecheck_url)).to be(false)
expect(livecheck.use_homebrew_curl?(c, homepage_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(true) 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
end end