The `Bitbucket` strategy checks download or tag pages but the content
is now fetched separately on page load, so the strategy is failing for
all related formulae. This updates the generated strategy URLs to
fetch the page content instead, which works as expected.
Some servers will return an error response if a `Content-Length`
header isn't included in a `POST` request, so this adds it to the
`post_args` array when `post_form` or `post_json` are used.
The `Xorg.find_versions` method was recently updated to replace
`match_data[:content].blank?` with `match_data[:content].empty?` but
this is producing an `undefined method 'empty?' for nil` error, as
`:content` is not present when `PageMatch.find_versions` uses cached
content. This updates `Xorg.find_versions` to handle nil `:content`
values in a way that's similar to other `find_versions` methods.
This enables `typed: strong` in `Options` and resolves the related
errors.
I annotated the return value of `serialize` in `#to_hash` to resolve
a type error and then updated other methods to use `to_hash` instead.
This approach resolves similar type errors without duplicating the
same `serialize` annotation.
For `#==`, I switched `instance_of?(other.class)` to
`other.is_a?(Options)`, as Sorbet understands that `is_a?` ensures
`other` is an `Options` object but doesn't seem to understand
that `instance_of?` was doing the same thing. The tests continue to
pass with these changes, so hopefully this is fine.
As suggested, this reworks `Options` to subclass `T::Struct`, which
simplifies the implementation and makes it easier to maintain.
One noteworthy difference in switching to `T::Struct` is that
`#serialize` omits `nil` values but I don't _think_ this should be a
problem for us. In terms of changes, I modified `#merge` to remove a
now-unnecessary `compact` call and updated related tests.
Co-authored-by: Douglas Eichelberger <697964+dduugg@users.noreply.github.com>
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 `livecheck` block.
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 the aforementioned `find_versions` parameters with `Options`
ensures that the remaining parameters are fairly 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`.
I initially set the type for livecheck's `post_form` and `post_json`
hashes to allow either a string or symbol key. I used string keys in
the documentation, as there will inevitably be some form field names
that would pose a problem for symbols (e.g., `E-mail` uses a hyphen,
`1twothree` starts with a digit, etc.). However, I remembered that we
can simply use quote symbols like `:"E-mail"` to handle these
situations, as they have the flexibility of a string while still being
a symbol.
With that in mind, this updates related type signatures to only allow
symbol keys and updates documentation and tests accordingly. The
documentation example contains a hyphenated form field, so it
demonstrates how to handle names that don't work as a bare symbol.
The existing code for handling a `HEAD`-only formula involves two
return values that can be `nil` but this isn't apparent because the
related methods aren't typed. This adds type signatures to the
methods and updates the livecheck code to account for `nil` return
values (making it clear which methods can return `nil`).
Co-authored-by: Mike McQuaid <mike@mikemcquaid.com>
`Sparkle` is the only strategy with a `find_versions` method that
calls `Strategy::page_content` (or `::page_headers`) and doesn't have
a `homebrew_curl` parameter. This adds the missing parameter and
passes the value to `page_content`, which brings it in line with the
other strategies.
livecheck currently doesn't support `POST` requests but it wasn't
entirely clear how best to handle that. I initially approached it as
a `Post` strategy but unfortunately that would have required us to
handle response body parsing (e.g., JSON, XML, etc.) in some fashion.
We could borrow some of the logic from related strategies but we would
still be stuck having to update `Post` whenever we add a strategy for
a new format.
Instead, this implements `POST` support by borrowing ideas from the
`using: :post` and `data` `url` options found in formulae. This uses
a `post_form` option to handle form data and `post_json` to handle
JSON data, encoding the hash argument for each into the appropriate
format. The presence of either option means that curl will use a
`POST` request.
With this approach, we can make a `POST` request using any strategy
that calls `Strategy::page_headers` or `::page_content` (directly or
indirectly) and everything else works the same as usual. The only
change needed in related strategies was to pass the options through
to the `Strategy` methods.
For example, if we need to parse a JSON response from a `POST`
request, we add a `post_data` or `post_json` hash to the `livecheck`
block `url` and use `strategy :json` with a `strategy` block. This
leans on existing patterns that we're already familiar with and
shouldn't require any notable maintenance burden when adding new
strategies, so it seems like a better approach than a `Post` strategy.
The `mesa` formula currently uses a mesa.freedesktop.org/archive/
`stable` URL but it redirects to archive.mesa3d.org. Upstream links
to archive.mesa3d.org as the location to find Mesa releases, so we
should update the formula URLs accordingly.
This updates the `Xorg` strategy to be able to handle
archive.mesa3d.org URLs, so livecheck will continue to be able to
check `mesa` without needing a one-off `livecheck` block. [This would
also work for `mesalib-glw` (which has an archive.mesa3d.org `stable`
URL) but that formula is deprecated.]
We recently updated the `Pypi` strategy to use the PyPI JSON API and
the default strategy behavior no longer relies on a regex, so the initial implementation didn't include regex handling. This restores
support for a `livecheck` block regex by updating the `DEFAULT_BLOCK`
logic to handle an optional regex. This allows us to use a regex to
omit parts of the `info.version` value without having to duplicate
the default block logic in a `strategy` block only to use a regex.
This isn't currently necessary for any existing formulae using the
`Pypi` strategy but we have a few that needed a custom regex with
the previous strategy approach, so they may need this functionality
in the future. Besides that, restoring regex support to `Pypi`
ensures that `livecheck`/`strategy` blocks work in a fairly
consistent manner across strategies.
This updates the block-handling logic in `Json::versions_from_content`
to naively pass the regex value when the block has two parameters. Up
to now, we have been ensuring that `regex` is not `nil` and this
makes sense with existing usage (the `Crate` strategy's default
block, formulae/cask `strategy` blocks). However, we need to allow a
`nil` `regex` value to make it possible to add an optional `regex`
parameter in the `Pypi::DEFAULT_BLOCK` Proc. This is necessary to
allow the `Pypi` strategy to work with an optional regex from a
`livecheck` block again [without creating an additional
`DEFAULT_BLOCK` variant with a regex parameter].
This reworks the new `Pypi` JSON API implementation to use
`Json::find_versions` in `Pypi::find_versions`, borrowing some of the
approach from the `Crate` strategy.
Besides that, this pares down the fields in the
`::generate_input_values` return hash to only `:url`, as we're not
using a generated regex to match version information in this setup.
This adds a `provided_content` parameter to `::find_versions` as part
of this process and I will expand the `Pypi` tests to increase
coverage (like the `Crates` tests) in a later PR. 75% of `Pypi` checks
are failing at the moment (with some returning inaccurate version
information), so the current priority is getting this fix merged in
the short-term.
Formulae, casks, and resources have a `#livecheckable?` method that
indicates whether they contain a `livecheck` block. This is intended
to be read as "has a livecheckable?", not "is livecheckable?" (as
livecheck can find versions for some packages/resources without a
`livecheck` block). Unfortunately, correct understanding of this
method's behavior [outside of documentation] relies on historical
knowledge that few people possess, so this is often confusing to
anyone who hasn't been working on livecheck since 2020.
In the olden days, a "livecheckable" was a Ruby file containing a
`livecheck` block (originally a hash) with a filename that
corresponded to a related formula. The `livecheck` blocks in
livecheckable files were integrated into their respective formulae in
August 2020, so [first-party] livecheckables ceased to exist at that
time. From that point forward, we simply referred to these as
`livecheck` blocks.
With that in mind, this clarifies the situation by replacing
"livecheckable" language. This includes renaming `#livecheckable?` to
`#livecheck_defined?`, replacing usage of "livecheckable" as a noun
with "`livecheck` block", replacing "livecheckable" as a boolean with
"livecheck_defined", and replacing incorrect usage of "livecheckable"
as an adjective with "checkable".
Up to now, we haven't been accounting for `#url` symbol arguments in
`livecheck` blocks that don't reference a checkable URL. This can
either be an invalid symbol (e.g., using the `:stable` formula symbol
in a cask) or a valid symbol where the referenced URL doesn't exist
(e.g., using `:head` when there's no `head` URL). [Almost all of the
valid symbols are required URLs but `head` is optional.]
Over the years, we've had a handful of slips where we've used `:url`
in formulae (when it's only valid in casks) and `:stable` in casks
(when it's only valid in formulae). In this scenario,
`livecheck_url_string` is `nil`, so livecheck falls back to
`#checkable_urls`. In this scenario, `stable` and `url` are the first
checkable URLs for formulae and casks (respectively), so the checks
ended up working as expected merely by chance. This isn't obvious in
the output and even the debug output looks normal. It only becomes
apparent that livecheck isn't working as expected if it iterates
through more than one checkable URL before reaching one that works
(not the case in those instances).
With that in mind, this adds an error when a `#url` symbol is used
but it doesn't correspond to a checkable URL. This will account for
both of the mentioned scenarios (invalid symbols and valid ones
referencing a non-existent URL) and prevent livecheck from quietly
proceeding in an unexpected manner.
`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.
The `url_provided` parameter of the `Strategy#from_url` method was
originally introduced in #9529 but I removed it in a later commit in
that PR in favor of a different approach. Unfortunately, I forgot to
remove the `url_provided` parameter, as it was no longer needed after
that change. This removes the parameter and updates `#from_url` calls
accordingly.
I refactored the `Git` strategy to use `SystemCommand` instead of
`Open3#capture3` in #13387 but I forgot to remove `require "open3"`
at the time. `Git` doesn't use `open3` now, so this removes the
unused `require`.
`URI#parse` was originally added in #9074 and replaced with
`Addressable::URI#parse` in #13306 but `require "uri"` wasn't removed
at the time. livecheck doesn't use `URI` now, so this removes the
unused `require`.
`brew livecheck` currently gives a Sorbet type error when run on a
HEAD-only formula: `Parameter 'version': Expected type Version, got
type String with value "c06c10d"`. This happens because the `current`
and `latest` values are strings but `LivecheckVersion#create` expects
a `Version` object.
This addresses the issue by creating a `Version` object from the
related commit strings. This ensures that the type of these variables
is more uniform, which makes them easier to reason about.
This is primarily intended to resolve the `uninitialized constant
Utils::Backtrace` error in `formula_versions.rb:60` but I expanded it
to try to cover all existing usage of `Utils::Backtrace`.
I've followed the existing pattern, where `utils/backtrace` is
required in the context of where it's used. Many of these cases use
`Backtrace` in a conditional manner, so I've tried to ensure that the
`require` follows suit.
This improves the load time of most brew commands. For an example of
one of the simplest commands this speeds up:
Without Bootsnap:
```
$ hyperfine 'git checkout master; brew help' 'git checkout optimise_requires; brew help'
Benchmark 1: git checkout master; brew help
Time (mean ± σ): 525.0 ms ± 35.8 ms [User: 229.9 ms, System: 113.1 ms]
Range (min … max): 465.3 ms … 576.6 ms 10 runs
Benchmark 2: git checkout optimise_requires; brew help
Time (mean ± σ): 383.3 ms ± 25.1 ms [User: 133.0 ms, System: 72.1 ms]
Range (min … max): 353.0 ms … 443.6 ms 10 runs
Summary
git checkout optimise_requires; brew help ran
1.37 ± 0.13 times faster than git checkout master; brew help
```
With Bootsnap:
```
$ hyperfine 'git checkout master; brew help' 'git checkout optimise_requires; brew help'
Benchmark 1: git checkout master; brew help
Time (mean ± σ): 386.0 ms ± 30.9 ms [User: 130.2 ms, System: 93.8 ms]
Range (min … max): 359.5 ms … 469.3 ms 10 runs
Benchmark 2: git checkout optimise_requires; brew help
Time (mean ± σ): 330.2 ms ± 32.4 ms [User: 93.4 ms, System: 73.0 ms]
Range (min … max): 302.9 ms … 413.9 ms 10 runs
Summary
git checkout optimise_requires; brew help ran
1.17 ± 0.15 times faster than git checkout master; brew help
```
I previously expanded use of `typed: strict` in livecheck files but
the exception was `livecheck/strategy.rb`. This addresses the
`@strategies` type errors in that file and upgrades it to
`typed: strict`.
Co-authored-by: apainintheneck <apainintheneck@gmail.com>
