Skip to content

Conversation

bevanjkay
Copy link
Member

@bevanjkay bevanjkay commented Oct 4, 2025

  • 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 style with your changes locally?
  • Have you successfully run brew typecheck with your changes locally?
  • Have you successfully run brew tests with your changes locally?

This is an initial pass at trying to skip the uninstall step when running brew upgrade and brew reinstall.
I have also added a simple dsl uninstall_on_upgrade! that would cause the uninstall to always run, for use in cases where it may be required.

This is currently only accessible behind an ENV variable - HOMEBREW_NO_UNINSTALL_ON_CASK_UPGRADE=1.

I have tested this with simple applications so far and it seems to work ok, but have not completed any testing with pkg or script installers yet.

One blindside we may have here is that we do not test brew upgrade on CI, which means any future issues would only be surfaced by users, unless we had a way of running the upgrade on PRs.

@bevanjkay bevanjkay marked this pull request as draft October 4, 2025 01:25
@bevanjkay bevanjkay force-pushed the no-uninstall-on-cask-upgrade branch 2 times, most recently from 86cdd0b to 79c8f66 Compare October 4, 2025 01:43
@bevanjkay bevanjkay force-pushed the no-uninstall-on-cask-upgrade branch from 79c8f66 to cb0d2cc Compare October 4, 2025 02:34
@bevanjkay
Copy link
Member Author

We will likely need a way to detect changes in artifacts between the old and new versions, and where there are changes remove the old. Otherwise we will end up with multiple versions of applications when paths change, or names change etc...

@bevanjkay
Copy link
Member Author

I wonder if we do a straight comparison to see if any artifacts have changed, and if they have, we then go ahead with the uninstall as we historically have to ensure we clean up correctly?

@MikeMcQuaid
Copy link
Member

Nice idea @bevanjkay! The main problem I was worried about was ensuring that uninstall always worked regardless of the state of the cask file in the Caskroom.

We will likely need a way to detect changes in artifacts between the old and new versions, and where there are changes remove the old. Otherwise we will end up with multiple versions of applications when paths change, or names change etc...

I wonder if there's a programmatic way of us detecting this in CI? 🤔

@bevanjkay
Copy link
Member Author

I wonder if there's a programmatic way of us detecting this in CI? 🤔

We can do this by comparing the installed Caskfile with the one that is being upgraded to. It doesn't help for applications that are modified through auto_update.

We can either do a full uninstall during upgrade if artifacts change, or try to handle the ones that no longer exist specifically.

@MikeMcQuaid
Copy link
Member

We can either do a full uninstall during upgrade if artifacts change, or try to handle the ones that no longer exist specifically.

Good idea. Another option is do the uninstall afterwards, something like:

  • gather list of artifacts to be removed by uninstall
  • gather list of new artifacts to be installed/installed
  • remove those that don't intersect

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