This adds tests to cover all of the strategy outside of the
`#find_versions` method, which we don't currently test because it
involves a network request.
`GithubLatest` was updated to use parts of `GithubRelease` and this
works fine within `brew livecheck` (since we `require` all the
strategies) but we need to explicitly `require` `GithubRelease` in
the `GithubLatest` test file now. Without this, we encounter errors
in parts of `GithubLatest` where `GithubRelease` is referenced.
This updates `ElectronBuilder` to use `Yaml#find_versions`, as the
only code unique to that strategy is to restrict regex usage and
use default version-finding logic when a `strategy` block isn't
provided. This is similar to how we have various strategies that
use `PageMatch#find_versions` internally.
This allows us to remove `ElectronBuilder#versions_from_content`
entirely, along with the related tests. I've added support for
`provided_content` to `ElectronBuilder#find_versions` as a way of
adding tests and maintaining code coverage.
This adds a generic `Yaml` strategy to livecheck that requires a
`strategy` block to operate. The YAML-parsing code is taken from the
existing approach in the `ElectronBuilder` strategy.
We don't currently have any `strategy` blocks in first-party taps
that manually parse YAML. However, creating a generic `Yaml` strategy
allows us to simplify `ElectronBuilder` (and any future strategy
that works with YAML) while making it easy to create custom `Yaml`
`strategy` blocks in formulae/casks as needed.
This renames `simple_content` to `content_simple`, which makes the
`content_*` variable name convention more consistent (as is in the
`Xml` strategy tests).
This setup mimics the `#parse_xml` method that was implemented in the
`Xml` strategy. Isolating the parsing code means that other strategies
can take only what they need from `Json` (i.e., it's not required for
them to use `Json#find_versions`).
This adds a generic `Xml` strategy to livecheck that requires a
`strategy` block to operate. The XML-parsing code is taken from the
existing approach in the `Sparkle` strategy. As such, `Sparkle` has
been updated to use the `Xml#parse_xml` method instead.
Unlike the `Json` strategy, we don't currently have any `strategy`
blocks in first-party taps that manually parse XML. However, we had a
user request support for something like this and I was already working
on an `Xml` strategy (as a way of extracting the XML-parsing code
from `Sparkle` into something general-purpose), so here we are.
Future strategies that parse simple XML data can potentially use the
`Xml#find_versions` method (similar to how we have strategies that
leverage `PageMatch#find_versions`) instead of having to implement
something bespoke like `Sparkle`.
This adds a generic `Json` strategy to livecheck that requires a
`strategy` block to operate. This is primarily intended as a
replacement for existing `strategy` blocks in formulae/casks that
use `JSON#parse`, as it allows us to internalize/standardize that
boilerplate while improving error-handling.
Additionally, future strategies that parse JSON data can use the
`Json#find_versions` method instead of having to reinvent the wheel
(similar to how we currently have a number of strategies that
leverage `PageMatch#find_versions`).
The `ElectronBuilder` strategy uses `YAML#safe_load` to parse YAML
content and this limits deserialization to appropriate classes. We
recently encountered a `Tried to load unspecified class: Time` error
when using the `ElectronBuilder` strategy on a `latest-mac.yml` file
containing `releaseDate: 2022-12-01T02:02:46.419Z`.
The electron-builder YAML files we usually encounter use single
quotes around the `releaseDate` value to ensure it's treated as a
string (e.g., `releaseDate: '2022-10-12T17:55:26.718Z'`) and this is
what we do in `electron_builder_spec.rb`. The aforementioned YAML
file doesn't use single quotes around the value, so it's treated as
a timestamp and apparently this makes Psych use `Time` (which
`#safe_load` doesn't allow by default).
Seeing as we can't control the YAML content and there's a chance we
may encounter other files like this in the future, this commit
modifies the related `#safe_load` call to allow `Time` (and `Date`
for good measure). This will resolve the aforementioned error and
allow the `ElectronBuilder` strategy to work as expected in this
scenario.
The `check_for_resources` code arguably speaks for itself, so it
doesn't feel like a necessary addition. Besides that, this cleans up
some other comments and text to better align with existing examples.
The `Gnome` strategy's default regex uses the `+` form of the standard
regex for matching versions like 1.2.3. However, with the switch to
the new version scheme, some packages had a release that omits a
minor and patch (i.e., `40` instead of `40.0`). The default regex
fails to match versions like this but the looser `*` form will match
both. [When creating regexes, we generally start with the `+` form
and only switch to the looser `*` form when it's necessary and
contextually-appropriate.]
This also updates the default version filtering logic that's applied
to versions using the old GNOME version scheme (below version 40).
Outside of the refactoring changes, this also filters out versions
where the patch number is 90+, as these are also unstable.
Making `channel` information available in the `Item` is necessary
to be able to filter out unstable items using a `strategy` block. If
an item doesn't specify a channel, then it uses the default channel
(this is what Sparkle itself uses for updates). Channels like `beta`
are something we want to avoid for stable casks and this allows for
that type of [cask-specific] filtering.
It's technically possible to automatically filter out items that
aren't using the default channel (i.e., `channel != nil`) in
`#items_from_content` but some casks use an unstable version, so we
can't do this internally. That is to say, we wouldn't be able to
override internal filtering in a `strategy` block, as any omitted
items wouldn't be provided to the block. Conversely, if we pass all
items to a `strategy` block, we can easily filter by channel there.
We haven't been filtering by channel internally and we've only found
one cask where this has been a problem, so it seems fine for now.
It's sometimes necessary to work with all the items in a Sparkle feed
to be able to correctly identify the newest version but livecheck's
`Sparkle` strategy only passes the `item` it views as newest into a
`strategy` block.
This updates the `Sparkle` strategy to optionally pass all items into
a `strategy` block, so we can manipulate them (e.g., filtering,
sorting). This is enabled by naming the first argument of the
strategy block `items` instead of `item`. `Sparkle` `strategy` blocks
where the first argument is `item` will continue to work as expected.
This necessarily updates `#item_from_content` (now
`items_from_content`) to return all items. I've decided to move the
sorting out of `#items_from_content`, so it simply returns the items
in the order they appear. If there is ever an exceptional situation
where we need the original order, this will technically allow for it.
The sorting has instead been moved into the `#versions_from_content`
method, to maintain the existing behavior. I thought about passing
the items into the `strategy` block in their original order but it
feels like sorting by default is the better approach for now (partly
from the perspective of maintaining existing behavior) and we can
always revisit this in the future if a cask ever requires the
original order.
Lastly, this expands the `Sparkle` tests to increase coverage. The
only untested parts are `#find_versions` (which currently
requires a network request) and a couple safeguard `raise` calls
when there's a `REXML::UndefinedNamespaceException` (which shouldn't
be encountered unless something is broken).
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.
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.
Some formulae/casks contain duplicate checkable URLs or contain
URLs that become duplicates after `#preprocess_url` is used. With
the former, a formula's `stable` and `head` URLs can be the same if
they both use the Git repository. With the latter, a formula's
`homepage` and `stable` URLs are different but `#preprocess_url` can
return the same Git repository URL for each (which can also be
a duplicate of the `head` URL).
The `fabric-completion` formula is a worst case scenario, as it
contains both of these conditions but the repository has no tags.
By default, livecheck would needlessly check the repository three
times (i.e., no versions are found so livecheck tries the next URL,
which happens to be the same URL).
This commit avoids duplicate URLs in `#checkable_urls` and keeps
track of checked URLs in `#latest_version` to avoid a duplicate
caused by `#preprocess_url`. This should effectively resolve the
issue of checking the same URL more than once for a given
formula/cask. Checking the same URL only once across all the
formulae/casks in a given livecheck run will be handled in a later
commit (though I've done most of the groundwork already in previous
PRs).
This modifies cask-related livecheck strategies to allow passing a
regex into a `strategy` block, when appropriate. These strategies
were outliers that explicitly rejected a regex even if a `strategy`
block was used, forcing any regex to be inlined in the `strategy`
block (instead of being defined using `#regex`).
With these changes, all `strategy` blocks will be able to accept a
regex, further simplifying the mental model. This also helps to
better align the various `find_versions` and `versions_from_*`
methods across strategies.
