Skip to content

cask: bundle_version initial implementation#21882

Open
bevanjkay wants to merge 1 commit intomainfrom
cask-bundle-version
Open

cask: bundle_version initial implementation#21882
bevanjkay wants to merge 1 commit intomainfrom
cask-bundle-version

Conversation

@bevanjkay
Copy link
Copy Markdown
Member


  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests (excluding integration tests) for your changes? Here's an example.
  • Have you successfully run brew lgtm (style, typechecking and tests) with your changes locally?

  • AI was used to generate or assist with generating this PR. Please specify below how you used AI to help you, and what steps you have taken to manually verify the changes.

I used Codex to help plan out and implement the initial version of bundle_version. I have reviewed and tested the functionality, but anticipate that there will be some edge-cases.


This PR introduces an initial implementation of a bundle_version parameter for cask artifacts. In this first iteration, it only applies to app artifacts, which is the primary target for this feature.

The goal is to support metadata-only upgrades when an app has already been updated outside Homebrew, for example by an in-app updater. If a newer cask is available in Homebrew, and the installed app bundle already matches the incoming cask’s version and bundle_version, Homebrew updates its installed cask metadata without fetching or reinstalling the app.

Note that this only applies when the version matches, so brew upgrade <cask> would still run an installation when the installed version is greater than the Homebrew version. A future possible enhancement (out of scope here) could be to handle cases where the installed app is never than the incoming cask version.

I’ve kept this PR scoped to the MVP. For broader adoption, we would also want brew bump and brew bump-cask-pr support to detect and update bundle_version automatically. I intend to work on this as a follow-up, unless there is preference to include it here.


Included in this PR;

  • Initial bundle_version support for app artifacts
  • Metadata-only upgrades when the installed app matches the incoming cask’s version and bundle_version
  • Skipping fetch when a metadata-only upgrade will be performed
  • brew audit --online functionality to check the bundle_version values against the artifact

Follow-up work:

  • bundle_version detection and update support in brew bump and brew bump-cask-pr
  • Potential handling for apps already newer than the incoming cask version

Copilot AI review requested due to automatic review settings April 1, 2026 13:06
@bevanjkay bevanjkay changed the title cask: bundle_version initial implementation cask: bundle_version initial implementation Apr 1, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds initial bundle_version support for cask app artifacts to enable “metadata-only” upgrades when an installed app bundle already matches the incoming cask’s version + bundle_version, avoiding unnecessary downloads/reinstalls. This also extends API serialization/loading and introduces an online audit to validate declared bundle_version values against extracted artifacts.

Changes:

  • Add bundle_version option to app artifacts, including API JSON serialization/loading and artifact matching logic.
  • Implement metadata-only upgrade path (and skip prefetch/fetch where applicable) when installed bundle version matches.
  • Add brew audit --online checks and unit tests covering metadata-only upgrade and bundle version auditing.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
Library/Homebrew/cask/artifact/abstract_artifact.rb Adds default “metadata-upgrade” capability hooks to artifacts.
Library/Homebrew/cask/artifact/app.rb Implements bundle_version option + matching against installed Info.plist.
Library/Homebrew/cask/upgrade.rb Adds metadata-only upgrade flow and fetch filtering for upgrades.
Library/Homebrew/cmd/upgrade.rb Skips prefetching downloads for casks eligible for metadata-only upgrades.
Library/Homebrew/cask/installer.rb Adds write_metadata helper used by metadata-only upgrades.
Library/Homebrew/cask/audit.rb Adds online audit for declared bundle_version vs extracted app bundle version.
Library/Homebrew/test/cask/upgrade_spec.rb Tests metadata-only upgrade behavior and prefetch skipping.
Library/Homebrew/test/cask/audit_spec.rb Tests online audit behavior for bundle version matching/mismatch.
Library/Homebrew/test/cask/artifact/app_spec.rb Tests bundle_version parsing/storage and matching logic on App artifacts.
Library/Homebrew/test/cask/cask_loader/from_api_loader_spec.rb Tests loading bundle_version from JSON and internal JSON API artifacts.
Library/Homebrew/test/api/cask/cask_struct_generator_spec.rb Updates API artifact processing expectations for bundle_version.
Library/Homebrew/test/api/cask_spec.rb Updates expected cask source checksum due to fixture changes.
Library/Homebrew/test/support/fixtures/cask/Casks/everything.rb Adds bundle_version to fixture cask’s app stanza.
Library/Homebrew/test/support/fixtures/cask/everything.json Updates API fixture artifacts to include bundle_version and checksum.
Library/Homebrew/test/support/fixtures/cask/everything.internal.json Updates internal API fixture artifacts to include bundle_version and checksum.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +514 to +515
add_error "Unable to determine bundle version for App '#{artifact.source.basename}'" \
"bundle version should be removed from the artifact if it cannot be determined reliably."
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error message concatenation here is missing punctuation/whitespace: the two adjacent string literals join into "...App 'X'bundle version...". Add a separating space/newline (and ideally a period) so the audit output is readable.

Suggested change
add_error "Unable to determine bundle version for App '#{artifact.source.basename}'" \
"bundle version should be removed from the artifact if it cannot be determined reliably."
add_error "Unable to determine bundle version for App '#{artifact.source.basename}'." \
" Bundle version should be removed from the artifact if it cannot be determined reliably."

Copilot uses AI. Check for mistakes.
Comment on lines +141 to +149
fetch_upgradable_casks = fetch_required_casks(upgradable_casks.map(&:last))

created_download_queue = T.let(false, T::Boolean)
download_queue ||= if !dry_run && !skip_prefetch
created_download_queue = true
Homebrew::DownloadQueue.new(pour: true)
end

if !dry_run && !skip_prefetch
if !dry_run && !skip_prefetch && fetch_upgradable_casks.present?
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fetch_required_casks/metadata_upgrade_available? relies on artifact target paths, which depend on cask.config (e.g., custom --appdir). At this point the new_cask configs haven’t been merged with the installed cask’s config yet (that happens later in upgrade_cask), so this prefetch filtering can be wrong and trigger unnecessary download prefetches. Consider merging each new_cask’s config with the corresponding installed cask’s config before calling metadata_upgrade_available? in the prefetch phase.

Copilot uses AI. Check for mistakes.
@bevanjkay
Copy link
Copy Markdown
Member Author

I think we may also need to add an audit to not allow the use of bundle_version when the download url is unversioned, because the download of a specific version is not guaranteed in that case.

Copy link
Copy Markdown
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR @bevanjkay but I think this needs a different approach.

I think we may also need to add an audit to not allow the use of bundle_version when the download url is unversioned, because the download of a specific version is not guaranteed in that case.

This is revealing because I think, unfortunately, it makes the DSL pointless in that case. The "problem to be solved" also applies when the download URL is unversioned, unless I've missed something.

Basically: we want a way to have brew upgrade --greedy be the default but avoid upgrading things where the bundle version indicates it's already up-to-date. We may still want to update e.g. the caskroom version in that case so we don't keep repeatedly needing to check the same data.

We have a bundle_version attribute in our existing cask JSON API that sniffs this bundle version. That's what's already been used by other tools to do this sort of comparison.

Hope that helps, let me know what you think as I may be totally wrong about some of the above!

"Everything.app"
"Everything.app",
{
"bundle_version": "1234"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's already a bundle_version parameter in this file that's "bundle_version": null. we should use that instead.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation in the PR is a bit deeper as it can work across multiple app stanzas. bundle_version may not always be global to the entire cask, which is why it is a parameter on the artifact instead.

I didn't look too deeply but the API value for the existing bundle_version seems to always return nil, so I assumed it is legacy.

rename "Foo.app", "Bar.app"

app "Everything.app"
app "Everything.app", bundle_version: "1234"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this is the right approach for this DSL/API, sorry.

This seems like we'll just need to adjust all tooling and user behaviour to ensure this is bumped every time the version is?

Instead, I think we should aim to essentially have some sort of "this is how you convert from version to bundle version when they differ" or "the bundle and app version are the same" DSL(s).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll look into this some more, but my concern here is how often the cask "version" may diverge from the "bundle_version".

I'll get some real data and come back around.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @bevanjkay! Yeh, I think data will help here a lot.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, the results from a preliminary analysis;

To be eligible as a target;

  • I had to have the cask installed
  • It has to only have a single app artifact

This resulted in 99 eligible casks.

I then expanded the comparison to check the following;

  • Does version match either CFBundleVersion or CFBundleShortVersionString
  • Does version.csv.any? match either CFBundleVersion or CFBundleShortVersionString
    • Some mild concern matching this naively, because some may only increment the secondary version string for additional builds.

The match rate was 82.8% - which I was actually somewhat surprised by.


In summary, I think that you're right here, using a more fundamental (or naive) matching system is probably the way to go for now as it will work in many cases without any lift. But I can't see this being good enough to make --greedy the default in the short->medium term.

@bevanjkay
Copy link
Copy Markdown
Member Author

This is revealing because I think, unfortunately, it makes the DSL pointless in that case. The "problem to be solved" also applies when the download URL is unversioned, unless I've missed something.

Unversioned downloads are always a worse UX because there's no guarantee the version in the cask will match brew fetch. So perhaps the issue I referred to is not actually any worse than the status quo. If the incoming cask metadata is stale with the actual download contents we just have to naively trust it anyway. Unless we handle a bundle_version comparison at upgrade time (after fetching).

@MikeMcQuaid
Copy link
Copy Markdown
Member

Unless we handle a bundle_version comparison at upgrade time (after fetching).

This seems better than nothing if we can 👍🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants