Fix brew outdated --greedy crash on installed casks without a URL#23084
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a regression where brew outdated --greedy could crash for installed :latest casks whose migrated installed JSON metadata omits url, by ensuring API cask struct generation does not produce a blank url("") stanza.
Changes:
- Drop blank
urlvalues when generatingurl_argsfor cask structs, so URL-less installed metadata yieldscask.url == nil. - Add a regression spec that reconstructs URL-less installed
:latestmetadata with an existingLATEST_DOWNLOAD_SHA256and asserts greedy outdated detection returns without raising.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| Library/Homebrew/api/cask/cask_struct_generator.rb | Ensures missing url does not become [""], preventing url("") from being set in the loader path. |
| Library/Homebrew/test/cask/cask_loader/from_api_loader_spec.rb | Adds coverage for the URL-less installed-metadata scenario that previously triggered the downloader crash under --greedy. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
8 tasks
Signed-off-by: Patrick Linnane <patrick@linnane.io>
cede219 to
41cdb6e
Compare
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
After installed cask metadata migration was enabled for all users,
brew outdated --greedybegan failing withError: attempted to use a Downloadable without a URL!on:latestcasks whose minimal installed JSON omitsurl(reported in https://github.com/orgs/Homebrew/discussions/6961).CaskStructGeneratorbuilturl_argsas[hash["url"].to_s], so an absent url became[""]. That one-element array is not blank, so it survivedcompact_blankand reachedurl("")in the loader. Since""is truthy, the DSL wrapped it in a realURLinstead of leaving the url unset, makingchecksumable?true and sending greedy outdated detection into the downloader.Calling
compact_blankon the array drops the blank element, so an absent url yields no arguments,cask.urlisnil, andchecksumable?returns false. Such:latestcasks are still reported outdated under--greedy, now without touching the downloader. This matches the internal API round-trip, which already strips blankurl_argson serialization.The regression test reconstructs URL-less installed
:latestmetadata with an existingLATEST_DOWNLOAD_SHA256and asserts the greedy check returns instead of raising.brewcommands to reproduce the bug?brew lgtm(style, typechecking and tests) locally?Claude Code (Opus 4.8) diagnosed the empty-URL path and wrote the fix and regression test; I reviewed the diff and verified with
brew lgtmplus targetedcask/cask_loader/from_api_loader,api/cask_struct, andapi/cask/cask_struct_generatorspecs.