Skip to content

Commit

Permalink
livecheck: add Options class
Browse files Browse the repository at this point in the history
This adds a `Livecheck::Options` class, which is intended to house
various configuration options that are set in `livecheck` blocks,
conditionally set by livecheck at runtime, etc. The general idea is
that when we add features involving configurations options (e.g., for
livecheck, strategies, curl, etc.), we can make changes to `Options`
without needing to modify parameters for strategy `find_versions`
methods, `Strategy` methods like `page_headers` and `page_content`,
etc. This is something that I've been trying to improve over the years
and `Options` should help to reduce maintenance overhead in this area
while also strengthening type signatures.

`Options` replaces the existing `homebrew_curl` option (which related
strategies pass to `Strategy` methods and on to `curl_args`) and the
new `url_options` (which contains `post_form` or `post_json` values
that are used to make `POST` requests). I recently added `url_options`
as a temporary way of enabling `POST` support without `Options` but
this restores the original `Options`-based implementation.

Along the way, I added a `homebrew_curl` parameter to the `url` DSL
method, allowing us to set an explicit value in `livecheck` blocks.
This is something that we've needed in some cases but I also intend
to replace implicit/inferred `homebrew_curl` usage with explicit
values in `livecheck` blocks once this is available for use. My
intention is to eventually remove the implicit behavior and only rely
on explicit values. That will align with how `homebrew_curl` options
work for other URLs and makes the behavior clear just from looking at
the formula/cask file.

Lastly, this removes the `unused` rest parameter from `find_versions`
methods. I originally added `unused` as a way of handling parameters
that some `find_versions` methods have but others don't (e.g., `cask`
in `ExtractPlist`), as this allowed us to pass various arguments to
`find_versions` methods without worrying about whether a particular
parameter is available. This isn't an ideal solution and I originally
wanted to handle this situation by only passing expected arguments to
`find_versions` methods but there was a technical issue standing in
the way. I recently found an answer to the issue, so this also
replaces the existing `ExtractPlist` special case with generic logic
that checks the parameters for a strategy's `find_versions` method
and only passes expected arguments.

Replacing aforementioned `find_versions` parameters with `Options`
ensures that the remaining parameters are consistent across strategies
and any differences are handled by the aforementioned logic. Outside
of `ExtractPlist`, the only other difference is that some
`find_versions` methods have a `provided_content` parameter but
that's currently only used by tests (though it's intended for caching
support in the future). I will be renaming that parameter to `content`
in an upcoming PR and expanding it to the other strategies, which
should make them all consistent outside of `ExtractPlist`.
  • Loading branch information
samford committed Feb 13, 2025
1 parent 04a9150 commit f898a0a
Show file tree
Hide file tree
Showing 30 changed files with 513 additions and 251 deletions.
48 changes: 28 additions & 20 deletions Library/Homebrew/livecheck.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
# frozen_string_literal: true

require "livecheck/constants"
require "livecheck/options"
require "cask/cask"

# The {Livecheck} class implements the DSL methods used in a formula's, cask's
Expand All @@ -15,6 +16,10 @@
class Livecheck
extend Forwardable

# Options to modify livecheck's behavior.
sig { returns(Homebrew::Livecheck::Options) }
attr_reader :options

# A very brief description of why the formula/cask/resource is skipped (e.g.
# `No longer developed or maintained`).
sig { returns(T.nilable(String)) }
Expand All @@ -24,13 +29,10 @@ class Livecheck
sig { returns(T.nilable(Proc)) }
attr_reader :strategy_block

# Options used by `Strategy` methods to modify `curl` behavior.
sig { returns(T.nilable(T::Hash[Symbol, T.untyped])) }
attr_reader :url_options

sig { params(package_or_resource: T.any(Cask::Cask, T.class_of(Formula), Resource)).void }
def initialize(package_or_resource)
@package_or_resource = package_or_resource
@options = T.let(Homebrew::Livecheck::Options.new, Homebrew::Livecheck::Options)
@referenced_cask_name = T.let(nil, T.nilable(String))
@referenced_formula_name = T.let(nil, T.nilable(String))
@regex = T.let(nil, T.nilable(Regexp))
Expand All @@ -40,7 +42,6 @@ def initialize(package_or_resource)
@strategy_block = T.let(nil, T.nilable(Proc))
@throttle = T.let(nil, T.nilable(Integer))
@url = T.let(nil, T.any(NilClass, String, Symbol))
@url_options = T.let(nil, T.nilable(T::Hash[Symbol, T.untyped]))
end

# Sets the `@referenced_cask_name` instance variable to the provided `String`
Expand Down Expand Up @@ -169,16 +170,22 @@ def throttle(rate = T.unsafe(nil))
sig {
params(
# URL to check for version information.
url: T.any(String, Symbol),
post_form: T.nilable(T::Hash[T.any(String, Symbol), String]),
post_json: T.nilable(T::Hash[T.any(String, Symbol), String]),
url: T.any(String, Symbol),
homebrew_curl: T.nilable(T::Boolean),
post_form: T.nilable(T::Hash[T.any(String, Symbol), String]),
post_json: T.nilable(T::Hash[T.any(String, Symbol), String]),
).returns(T.nilable(T.any(String, Symbol)))
}
def url(url = T.unsafe(nil), post_form: nil, post_json: nil)
def url(url = T.unsafe(nil), homebrew_curl: nil, post_form: nil, post_json: nil)
raise ArgumentError, "Only use `post_form` or `post_json`, not both" if post_form && post_json

options = { post_form:, post_json: }.compact
@url_options = options if options.present?
if homebrew_curl || post_form || post_json
@options = @options.merge({
homebrew_curl:,
post_form:,
post_json:,
}.compact)
end

case url
when nil
Expand All @@ -190,6 +197,7 @@ def url(url = T.unsafe(nil), post_form: nil, post_json: nil)
end
end

delegate url_options: :@options
delegate version: :@package_or_resource
delegate arch: :@package_or_resource
private :version, :arch
Expand All @@ -198,15 +206,15 @@ def url(url = T.unsafe(nil), post_form: nil, post_json: nil)
sig { returns(T::Hash[String, T.untyped]) }
def to_hash
{
"cask" => @referenced_cask_name,
"formula" => @referenced_formula_name,
"regex" => @regex,
"skip" => @skip,
"skip_msg" => @skip_msg,
"strategy" => @strategy,
"throttle" => @throttle,
"url" => @url,
"url_options" => @url_options,
"options" => @options.to_hash,
"cask" => @referenced_cask_name,
"formula" => @referenced_formula_name,
"regex" => @regex,
"skip" => @skip,
"skip_msg" => @skip_msg,
"strategy" => @strategy,
"throttle" => @throttle,
"url" => @url,
}
end
end
81 changes: 54 additions & 27 deletions Library/Homebrew/livecheck/livecheck.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,24 @@ module Livecheck
T.must(@livecheck_strategy_names).freeze
end

sig { returns(T::Hash[T::Class[T.anything], T::Array[Symbol]]) }
private_class_method def self.livecheck_find_versions_parameters
return T.must(@livecheck_find_versions_parameters) if defined?(@livecheck_find_versions_parameters)

# Cache strategy `find_versions` method parameters, to avoid repeating
# this work
@livecheck_find_versions_parameters = T.let({}, T.nilable(T::Hash[T::Class[T.anything], T::Array[Symbol]]))
Strategy.constants.sort.each do |const_symbol|
constant = Strategy.const_get(const_symbol)
next unless constant.is_a?(Class)

T.must(@livecheck_find_versions_parameters)[constant] =
T::Utils.signature_for_method(constant.method(:find_versions))
&.parameters&.map(&:second)
end
T.must(@livecheck_find_versions_parameters).freeze
end

# Uses `formulae_and_casks_to_check` to identify taps in use other than
# homebrew/core and homebrew/cask and loads strategies from them.
sig { params(formulae_and_casks_to_check: T::Array[T.any(Formula, Cask::Cask)]).void }
Expand Down Expand Up @@ -613,8 +631,9 @@ def self.latest_version(
livecheck = formula_or_cask.livecheck
referenced_livecheck = referenced_formula_or_cask&.livecheck

livecheck_options = livecheck.options || referenced_livecheck&.options
livecheck_url_options = livecheck_options.url_options.compact
livecheck_url = livecheck.url || referenced_livecheck&.url
livecheck_url_options = livecheck.url_options || referenced_livecheck&.url_options
livecheck_regex = livecheck.regex || referenced_livecheck&.regex
livecheck_strategy = livecheck.strategy || referenced_livecheck&.strategy
livecheck_strategy_block = livecheck.strategy_block || referenced_livecheck&.strategy_block
Expand Down Expand Up @@ -674,8 +693,8 @@ def self.latest_version(
elsif original_url.present? && original_url != "None"
puts "URL: #{original_url}"
end
puts "URL Options: #{livecheck_url_options}" if livecheck_url_options.present?
puts "URL (processed): #{url}" if url != original_url
puts "URL Options: #{livecheck_url_options}" if livecheck_url_options.present?
if strategies.present? && verbose
puts "Strategies: #{strategies.map { |s| livecheck_strategy_names[s] }.join(", ")}"
end
Expand All @@ -695,23 +714,25 @@ def self.latest_version(

next if strategy.blank?

homebrew_curl = case strategy_name
when "PageMatch", "HeaderMatch"
use_homebrew_curl?(referenced_package, url)
if (livecheck_homebrew_curl = livecheck_options.homebrew_curl).nil?
case strategy_name
when "PageMatch", "HeaderMatch"
if (homebrew_curl = use_homebrew_curl?(referenced_package, url))
livecheck_options = livecheck_options.merge({ homebrew_curl: })
livecheck_homebrew_curl = homebrew_curl

Check warning on line 722 in Library/Homebrew/livecheck/livecheck.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/livecheck/livecheck.rb#L722

Added line #L722 was not covered by tests
end
end
end
puts "Homebrew curl?: Yes" if debug && homebrew_curl.present?

strategy_args = {
regex: livecheck_regex,
url_options: livecheck_url_options,
homebrew_curl:,
}
# TODO: Set `cask`/`url` args based on the presence of the keyword arg
# in the strategy's `#find_versions` method once we figure out why
# `strategy.method(:find_versions).parameters` isn't working as
# expected.
strategy_args[:cask] = cask if strategy_name == "ExtractPlist" && cask.present?
strategy_args[:url] = url
puts "Homebrew curl?: #{livecheck_homebrew_curl ? "Yes" : "No"}" if debug && !livecheck_homebrew_curl.nil?

# Only use arguments that the strategy's `#find_versions` method
# supports
find_versions_parameters = T.must(livecheck_find_versions_parameters[strategy])
strategy_args = {}
strategy_args[:cask] = cask if find_versions_parameters.include?(:cask)
strategy_args[:url] = url if find_versions_parameters.include?(:url)
strategy_args[:regex] = livecheck_regex if find_versions_parameters.include?(:regex)
strategy_args[:options] = livecheck_options if find_versions_parameters.include?(:options)
strategy_args.compact!

strategy_data = strategy.find_versions(**strategy_args, &livecheck_strategy_block)
Expand Down Expand Up @@ -811,7 +832,6 @@ def self.latest_version(
end
version_info[:meta][:url][:final] = strategy_data[:final_url] if strategy_data[:final_url]
version_info[:meta][:url][:options] = livecheck_url_options if livecheck_url_options.present?
version_info[:meta][:url][:homebrew_curl] = homebrew_curl if homebrew_curl.present?
end
version_info[:meta][:strategy] = strategy_name if strategy.present?
version_info[:meta][:strategies] = strategies.map { |s| livecheck_strategy_names[s] } if strategies.present?
Expand Down Expand Up @@ -858,9 +878,10 @@ def self.resource_version(
resource_version_info = {}

livecheck = resource.livecheck
livecheck_options = livecheck.options
livecheck_url_options = livecheck_options.url_options.compact

Check warning on line 882 in Library/Homebrew/livecheck/livecheck.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/livecheck/livecheck.rb#L881-L882

Added lines #L881 - L882 were not covered by tests
livecheck_reference = livecheck.formula
livecheck_url = livecheck.url
livecheck_url_options = livecheck.url_options
livecheck_regex = livecheck.regex
livecheck_strategy = livecheck.strategy
livecheck_strategy_block = livecheck.strategy_block
Expand Down Expand Up @@ -898,8 +919,8 @@ def self.resource_version(
elsif original_url.present? && original_url != "None"
puts "URL: #{original_url}"
end
puts "URL Options: #{livecheck_url_options}" if livecheck_url_options.present?
puts "URL (processed): #{url}" if url != original_url
puts "URL Options: #{livecheck_url_options}" if livecheck_url_options.present?
if strategies.present? && verbose
puts "Strategies: #{strategies.map { |s| livecheck_strategy_names[s] }.join(", ")}"
end
Expand All @@ -922,16 +943,22 @@ def self.resource_version(
puts if debug && strategy.blank? && livecheck_reference != :parent
next if strategy.blank? && livecheck_reference != :parent

if debug && !(livecheck_homebrew_curl = livecheck_options.homebrew_curl).nil?
puts "Homebrew curl?: #{livecheck_homebrew_curl ? "Yes" : "No"}"
end

if livecheck_reference == :parent
match_version_map = { formula_latest => Version.new(formula_latest) }
cached = true
else
strategy_args = {
url:,
regex: livecheck_regex,
url_options: livecheck_url_options,
homebrew_curl: false,
}.compact
# Only use arguments that the strategy's `#find_versions` method
# supports
find_versions_parameters = T.must(livecheck_find_versions_parameters[strategy])
strategy_args = {}

Check warning on line 957 in Library/Homebrew/livecheck/livecheck.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/livecheck/livecheck.rb#L957

Added line #L957 was not covered by tests
strategy_args[:url] = url if find_versions_parameters.include?(:url)
strategy_args[:regex] = livecheck_regex if find_versions_parameters.include?(:regex)
strategy_args[:options] = livecheck_options if find_versions_parameters.include?(:options)
strategy_args.compact!

Check warning on line 961 in Library/Homebrew/livecheck/livecheck.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/livecheck/livecheck.rb#L961

Added line #L961 was not covered by tests

strategy_data = strategy.find_versions(**strategy_args, &livecheck_strategy_block)
match_version_map = strategy_data[:matches]
Expand Down
107 changes: 107 additions & 0 deletions Library/Homebrew/livecheck/options.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
# typed: strict
# frozen_string_literal: true

module Homebrew
module Livecheck
# Options to modify livecheck's behavior. These primarily come from
# `livecheck` blocks but they can also be set by livecheck at runtime.
class Options
# Whether to use brewed curl.
sig { returns(T.nilable(T::Boolean)) }
attr_reader :homebrew_curl

# Form data to use when making a `POST` request.
sig { returns(T.nilable(T::Hash[T.any(String, Symbol), String])) }
attr_reader :post_form

# JSON data to use when making a `POST` request.
sig { returns(T.nilable(T::Hash[T.any(String, Symbol), String])) }
attr_reader :post_json

# @param homebrew_curl whether to use brewed curl
# @param post_form form data to use when making a `POST` request
# @param post_json JSON data to use when making a `POST` request
sig {
params(
homebrew_curl: T.nilable(T::Boolean),
post_form: T.nilable(T::Hash[T.any(String, Symbol), String]),
post_json: T.nilable(T::Hash[T.any(String, Symbol), String]),
).void
}
def initialize(homebrew_curl: nil, post_form: nil, post_json: nil)
@homebrew_curl = homebrew_curl
@post_form = post_form
@post_json = post_json
end

# Returns a `Hash` of options that are provided as arguments to `url`.
sig { returns(T::Hash[Symbol, T.untyped]) }
def url_options
{
homebrew_curl:,
post_form:,
post_json:,
}
end

# Returns a `Hash` of all instance variable values.
sig { returns(T::Hash[Symbol, T.untyped]) }
def to_h
{
homebrew_curl:,
post_form:,
post_json:,
}
end

# Returns a `Hash` of all instance variables, using `String` keys.
sig { returns(T::Hash[String, T.untyped]) }
def to_hash
{
"homebrew_curl" => @homebrew_curl,
"post_form" => @post_form,
"post_json" => @post_json,
}
end

# Returns a new object formed by merging `other` values with a copy of
# `self`.
#
# `nil` values are removed from `other` before merging if it is an
# `Options` object, as these are unitiailized values. This ensures that
# existing values in `self` aren't unexpectedly overwritten with defaults.
sig { params(other: T.any(Options, T::Hash[Symbol, T.untyped])).returns(Options) }
def merge(other)
return dup if other.blank?

this_hash = to_h
other_hash = other.is_a?(Options) ? other.to_h.compact : other
return dup if this_hash == other_hash

new_options = this_hash.merge(other_hash)
Options.new(**new_options)
end

sig { params(other: Options).returns(T::Boolean) }
def ==(other)
instance_of?(other.class) &&
@homebrew_curl == other.homebrew_curl &&
@post_form == other.post_form &&
@post_json == other.post_json
end
alias eql? ==

# Whether the object has only default values.
sig { returns(T::Boolean) }
def empty?
@homebrew_curl.nil? && @post_form.nil? && @post_json.nil?
end

# Whether the object has any non-default values.
sig { returns(T::Boolean) }
def present?
!@homebrew_curl.nil? || !@post_form.nil? || !@post_json.nil?
end
end
end
end
Loading

0 comments on commit f898a0a

Please sign in to comment.