Skip to content

Comments

cask: show accurate log messages for in-place upgrades#21568

Open
sensebat wants to merge 2 commits intoHomebrew:mainfrom
sensebat:fix/cask-upgrade-log-messages
Open

cask: show accurate log messages for in-place upgrades#21568
sensebat wants to merge 2 commits intoHomebrew:mainfrom
sensebat:fix/cask-upgrade-log-messages

Conversation

@sensebat
Copy link

  • 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 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. Claude Code (claude-opus-4-6) was used to trace the upgrade code path in cask/artifact/moved.rb and identify the misleading log messages. The actual fix was authored and reviewed by the submitter. Style and type checks were run locally.

What

When Homebrew has App Management or Full Disk Access permissions, cask upgrades use an in-place content replacement strategy (PR #15138, #15483) that preserves the .app directory inode. macOS then retains Dock position, notification settings, and granted permissions.

However, the ohai log messages in Cask::Artifact::Moved#delete and #move were printed unconditionally before the strategy branch, so the output always showed:

==> Removing App '/Applications/Signal.app'
==> Moving App 'Signal.app' to '/Applications/Signal.app'

even when the app was never actually removed.

Fix

Move the ohai calls into the respective branches:

  • In-place (App Management / FDA granted): prints a single Upgrading App '/Applications/Signal.app' in-place
  • Full replacement (no permissions): prints Removing App and Moving App as before — no change in behavior

No tests added — this is a log-message-only change with no logic change. No existing tests cover these messages.

When a terminal has App Management or Full Disk Access permissions,
Homebrew upgrades cask apps in-place by replacing only the contents of
the .app directory, preserving its inode and macOS metadata (Dock
position, notification settings, granted permissions).

However, the log output still printed "Removing App" and "Moving App"
as if the entire bundle were being deleted and reinstalled.

With this change:
- In-place upgrades print a single "Upgrading App in-place" message
- Full replacement upgrades continue to print "Removing App" and
  "Moving App" as before
Copy link
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! Few tweaks needed.

@sensebat sensebat force-pushed the fix/cask-upgrade-log-messages branch from 73906c3 to db32c04 Compare February 13, 2026 17:12
@sensebat sensebat force-pushed the fix/cask-upgrade-log-messages branch from db32c04 to a5b26ce Compare February 13, 2026 17:40
Copy link
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.

Looks good thanks! Some legit test failures need addressed and this should be good to go.

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.

2 participants