Skip to content

Commit

Permalink
livecheck: error on invalid url symbol
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
samford committed Oct 24, 2024
1 parent e8ee210 commit 9eeefc6
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 2 deletions.
7 changes: 6 additions & 1 deletion Library/Homebrew/livecheck/livecheck.rb
Original file line number Diff line number Diff line change
Expand Up @@ -611,7 +611,12 @@ def self.latest_version(

referenced_package = referenced_formula_or_cask || formula_or_cask

livecheck_url_string = livecheck_url_to_string(livecheck_url, referenced_package) if livecheck_url
if livecheck_url
livecheck_url_string = livecheck_url_to_string(livecheck_url, referenced_package)
if livecheck_url.is_a?(Symbol) && !livecheck_url_string
raise ArgumentError, "`url :#{livecheck_url}` does not reference a checkable URL"
end
end

urls = [livecheck_url_string] if livecheck_url_string
urls ||= checkable_urls(referenced_package)
Expand Down
22 changes: 21 additions & 1 deletion Library/Homebrew/test/dev-cmd/livecheck_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,27 @@
.and be_a_success
end

it "gives an error when no arguments are given and there's no watchlist", :integration_test do
it "errors when a `livecheck` block URL symbol does not correspond to a checkable URL", :integration_test do
content = <<~RUBY
desc "Some test"
homepage "https://github.com/Homebrew/brew"
url "https://brew.sh/test-1.0.0.tgz"
# This is deliberately wrong, as `:url` should only be used in casks
# (the equivalent for formulae is `:stable`).
livecheck do
url :url
end
RUBY
setup_test_formula("test", content)

expect { brew "livecheck", "test" }
.to output(/`url :url` does not reference a checkable URL/).to_stderr
.and not_to_output.to_stdout
.and be_a_failure
end

it "errors when no arguments are given and there's no watchlist", :integration_test do
expect { brew "livecheck", "HOMEBREW_LIVECHECK_WATCHLIST" => ".this_should_not_exist" }
.to output(/Invalid usage: A watchlist file is required when no arguments are given\./).to_stderr
.and not_to_output.to_stdout
Expand Down

0 comments on commit 9eeefc6

Please sign in to comment.