- Thanks to Markus on Slack for saying "the cop should only apply to the
content of the blocks, or more specifically only to stanzas that are
direct children of cask or on_* blocks", which made me realize that
I was overcomplicating things.
- Now that we detect correct stanza _grouping_ within `on_*` blocks in
Casks (PR 15211), correct stanza _ordering_ in `on_*` blocks was the
next logical step. For example, `url` has to come after `version` and
`sha256` in an `on_macos` or `on_intel` block for consistency with the
top-level stanza order we enforce elsewhere.
- Still not doing the nested `on_os` inside `on_arch`, that felt
excessive for an edge case that isn't present in any actual real
Casks we have. I removed the test with that specific TODO.
This prevents situations where the Intel runners have a 360 minute
timeout but the ARM runners only have 45 minutes.
See Homebrew/homebrew-core#130284.
I've been doing this personally for a few months and not hit any bugs.
We already do this for `brew tests`.
It will allow us to:
- remove manual type checks from all developer commands (Sorbet does a
better job with these)
- better surface bugs
- better surface type signatures
- get closer to being able to enable this by default for everyone
1. Use the maximum timeout possible for GitHub-hosted macOS runners
2. When using a short timeout, use an even shorter timeout on ARM
runners.
Our ARM runners are typically at least twice as fast as our Intel
runners on any given job. So, if it takes an ARM runner over half the
timeout to complete a job, it's almost certain that the Intel runner
will not complete the job within the timeout.
Setting an even shorter timeout on the ARM runners will help us abandon
jobs that are unlikely to be completed within the timeout well before we
hit the requested timeout.
The retry behaviour in `publish_commit_bottles.yml` [1] is often
successful after the second try, so it's likely that we're not waiting
long enough in between retries here.
Let's fix that by retrying with exponential backoff instead of adding a
fixed interval of five seconds after each failure.
[1] 3241035b2a/.github/workflows/publish-commit-bottles.yml (L431-L443)
Erroring out in the middle of uploading multiple bottles results in a
state that is tedious to recover from.
Let's try to avoid these situations by performing checks for all the
bottles first before trying to upload any.
livecheck's `Git` strategy uses `DownloadStrategyDetector#detect`
in its `#match?` method to check if a URL is a Git repository. This
has historically worked fine but I've recently seen a `can't modify
frozen String` error for a few formulae (percona-toolkit,
schroedinger, squid) in relation to the in-place `sub` call in
`BazaarDownloadStrategy`'s initializer.
Other download strategies use a `@url = @url.sub(...)` pattern to
avoid this issue, so this commit resolves the issue by using the same
approach in `BazaarDownloadStrategy`.
- If the user doesn't have `HOMEBREW_DEVELOPER` or
`HOMEBREW_NO_INSTALL_FROM_API` set but does have `homebrew/core` or
`homebrew/cask` taps installed this can cause problems with installing
outdated software.
- Hence, warn them in `brew doctor` if they have either of these taps
installed, with instructions on how to remove them.
Since we use `REXML::Document` in the type signature for `#parse_xml`,
we can encounter an `uninitialized constant
Homebrew::Livecheck::Strategy::Xml::REXML` error in strategies like
`Sparkle` that use `Xml#parse_xml` internally when the Sorbet runtime
is used. Moving the related require outside of the `#parse_xml` method
and into the `Xml` strategy proper resolves this issue.
`content` can be `nil` when a request doesn't succeed but
`#versions_from_content` expects a `String` value, so we need to
guard against a `nil` value like we do in other strategies.
The existing way of passing values to `#find_versions` methods in
strategies leads to type issues when the Sorbet runtime is enabled.
We've also recently talked about moving away from nilable args when
we can specify a default value but this doesn't work if we pass in a
`nil` value (like we're currently doing).
This commit aims to address both of those areas by better controlling
which arguments we're passing to `#find_versions`. This approach
naively handles `cask`/`url` arguments by special-casing
`ExtractPlist`.
However, we should be checking the strategy's `#find_versions`
method for a `cask` or `url` keyword parameter. The issue is that
`strategy.method(:find_versions).parameters` is returning
`[[:rest, :args], [:block, :blk]]` instead of the actual parameters
like `[[:keyreq, :url], [:key, :regex], [:keyrest, :unused],
[:block, :block]]`.
