livecheck: error on invalid url symbol

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.
This commit is contained in:
Sam Ford 2024-10-24 09:00:18 -04:00
parent bbe5a858d5
commit d7b515bf73
No known key found for this signature in database
GPG Key ID: 7AF5CBEE1DD6F76D
2 changed files with 58 additions and 6 deletions

View File

@ -508,10 +508,10 @@ module Homebrew
params(
livecheck_url: T.any(String, Symbol),
package_or_resource: T.any(Formula, Cask::Cask, Resource),
).returns(T.nilable(String))
).returns(String)
}
def self.livecheck_url_to_string(livecheck_url, package_or_resource)
case livecheck_url
livecheck_url_string = case livecheck_url
when String
livecheck_url
when :url
@ -521,6 +521,12 @@ module Homebrew
when :homepage
package_or_resource.homepage unless package_or_resource.is_a?(Resource)
end
if livecheck_url.is_a?(Symbol) && !livecheck_url_string
raise ArgumentError, "`url #{livecheck_url.inspect}` does not reference a checkable URL"
end
livecheck_url_string
end
# Returns an Array containing the formula/cask/resource URLs that can be used by livecheck.
@ -846,7 +852,7 @@ module Homebrew
livecheck_strategy = livecheck.strategy
livecheck_strategy_block = livecheck.strategy_block
livecheck_url_string = livecheck_url_to_string(livecheck_url, resource)
livecheck_url_string = livecheck_url_to_string(livecheck_url, resource) if livecheck_url
urls = [livecheck_url_string] if livecheck_url_string
urls ||= checkable_urls(resource)

View File

@ -134,6 +134,15 @@ RSpec.describe Homebrew::Livecheck do
end
end
let(:f_stable_url_only) do
stable_url_s = stable_url
formula("test_stable_url_only") do
desc "Test formula with only a stable URL"
url stable_url_s
end
end
let(:r_livecheck_url) { f_livecheck_url.resources.first }
let(:c_livecheck_url) do
@ -149,6 +158,17 @@ RSpec.describe Homebrew::Livecheck do
RUBY
end
let(:c_no_checkable_urls) do
Cask::CaskLoader.load(+<<-RUBY)
cask "test_no_checkable_urls" do
version "1.2.3"
name "Test"
desc "Test cask with no checkable URLs"
end
RUBY
end
it "returns a URL string when given a livecheck_url string for a formula" do
expect(livecheck.livecheck_url_to_string(livecheck_url, f_livecheck_url)).to eq(livecheck_url)
end
@ -167,9 +187,35 @@ RSpec.describe Homebrew::Livecheck do
end
it "returns nil when not given a string or valid symbol" do
expect(livecheck.livecheck_url_to_string(:invalid_symbol, f_livecheck_url)).to be_nil
expect(livecheck.livecheck_url_to_string(:invalid_symbol, c_livecheck_url)).to be_nil
expect(livecheck.livecheck_url_to_string(:invalid_symbol, r_livecheck_url)).to be_nil
error_text = "`url :%<symbol>s` does not reference a checkable URL"
# Invalid symbol in any context
expect { livecheck.livecheck_url_to_string(:invalid_symbol, f_livecheck_url) }
.to raise_error(ArgumentError, format(error_text, symbol: :invalid_symbol))
expect { livecheck.livecheck_url_to_string(:invalid_symbol, c_livecheck_url) }
.to raise_error(ArgumentError, format(error_text, symbol: :invalid_symbol))
expect { livecheck.livecheck_url_to_string(:invalid_symbol, r_livecheck_url) }
.to raise_error(ArgumentError, format(error_text, symbol: :invalid_symbol))
# Valid symbol in provided context but referenced URL is not present
expect { livecheck.livecheck_url_to_string(:head, f_stable_url_only) }
.to raise_error(ArgumentError, format(error_text, symbol: :head))
expect { livecheck.livecheck_url_to_string(:homepage, f_stable_url_only) }
.to raise_error(ArgumentError, format(error_text, symbol: :homepage))
expect { livecheck.livecheck_url_to_string(:homepage, c_no_checkable_urls) }
.to raise_error(ArgumentError, format(error_text, symbol: :homepage))
expect { livecheck.livecheck_url_to_string(:url, c_no_checkable_urls) }
.to raise_error(ArgumentError, format(error_text, symbol: :url))
# Valid symbol but not in the provided context
expect { livecheck.livecheck_url_to_string(:head, c_livecheck_url) }
.to raise_error(ArgumentError, format(error_text, symbol: :head))
expect { livecheck.livecheck_url_to_string(:homepage, r_livecheck_url) }
.to raise_error(ArgumentError, format(error_text, symbol: :homepage))
expect { livecheck.livecheck_url_to_string(:stable, c_livecheck_url) }
.to raise_error(ArgumentError, format(error_text, symbol: :stable))
expect { livecheck.livecheck_url_to_string(:url, f_livecheck_url) }
.to raise_error(ArgumentError, format(error_text, symbol: :url))
end
end