Update installed caskfiles of modified casks.

[reitermarkus]

• 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 tests with your changes locally?

---

Often casks get updated without a version bump to fix a broken uninstall or add something to zap. This replaces the installed caskfile if it differs from the current one and the version hasn't changed.

[MikeMcQuaid]

Should handle if the write (or perhaps even read) fails here.

[MikeMcQuaid]

What's the overhead on doing this on every brew update?

[vitorgalvao]

Big thumbs up to this. Not only will it fix issues with uninstalls, it should help prevent problems like the undent deprecation (closes Homebrew/homebrew-cask#49716).

[MikeMcQuaid]

it should help prevent problems like the undent deprecation (closes Homebrew/homebrew-cask#49716).

It won't prevent that because if the version of a Cask is updated it'll still have the same problem.

[vitorgalvao]

It won't prevent that because if the version of a Cask is updated it'll still have the same problem.

On the HBC repos, it’s customary that when we introduce a change like the undent deprecation that affects a lot of casks, we make a PR that introduces the change (and nothing else) to all relevant casks at once.

[MikeMcQuaid]

On the HBC repos, it’s customary that when we introduce a change like the undent deprecation that affects a lot of casks, we make a PR that introduces the change (and nothing else) to all relevant casks at once.

Even when you do that: it's still a problem. If you're relying on having ambiguously old Cask files which use Homebrew APIs on them: eventually some of those APIs will be deprecated, disabled and removed and you'll have the same problem again. Assuming that the current version of Cask has infinite backwards compatibility is the root issue here.

[vitorgalvao]

@MikeMcQuaid I agree. I think this change will help prevent those problems, but not outright fix them. With this change in place, your previous suggestion will work even better, since the problems @commitay was envisioning have a lower change of happening.

[reitermarkus]

Yes, I am aware that this won't fix the problem. There really isn't a fix for this, other than never deprecating anything. However, this will prevent deprecation errors for people who run brew update regularly.

Even I had one cask still using .undent, and I run brew update daily.

[MikeMcQuaid]

There really isn't a fix for this, other than never deprecating anything.

I disagree. Several options off the top of my head:

• Catch these exceptions and fall back to the current Cask file's uninstall
• Catch these exceptions and print a warning saying uninstall hooks are not run
• Instead of storing the entire Cask file in the Caskroom store just the part that's needed e.g. the uninstall hook method
• Instead of storing the entire Cask file in the Caskroom store instead store something to define uninstall logic that doesn't rely on Homebrew internals e.g. a Bash script or a JSON file that defines logic

[commitay]

Catch these exceptions and fall back to the current Cask file's uninstall

Homebrew/homebrew-cask#49716 (comment)

[MikeMcQuaid]

@commitay I replied to that comment there. Can you be more explicit on what you're trying to say rather than just dropping a single link to a comment you'd made previously? Would you care to address any of my other options?

To be more explicit on my viewpoint: no-one except me seems to be suggesting solutions to this problem which will not recur next time we remove formerly deprecated then disabled Homebrew APIs. Homebrew formulae have solutions to this problem. I'm not pretending I have all the answers but the current situation of "it'll happen again and there's nothing we can do about it" is defeatist and makes it hard for us to remove any APIs in future.

[vitorgalvao]

Catch these exceptions and fall back to the current Cask file's uninstall

A bunch of casks have changes that make this impractical. E.g., software that was a .app is now a .pkg. Or something that didn’t have an uninstall script, now has one. I don’t have numbers on how often this happens, but it’s not incredibly rare.

Catch these exceptions and print a warning saying uninstall hooks are not run

Worse experience for the user, but I’m not opposed to it. We’d need to document why this happens and explain how to uninstall manually, but it might work.

Instead of storing the entire Cask file in the Caskroom store just the part that's needed e.g. the uninstall hook method

Impractical if the deprecation is on a part needed for the uninstall.

Instead of storing the entire Cask file in the Caskroom store instead store something to define uninstall logic that doesn't rely on Homebrew internals e.g. a Bash script or a JSON file that defines logic

I like this idea.

[MikeMcQuaid]

Worse experience for the user, but I’m not opposed to it. We’d need to document why this happens and explain how to uninstall manually, but it might work.

I think it's a better experience than the current situation of an unhandled but expected error.

Impractical if the deprecation is on a part needed for the uninstall.

Yep but it still reduces the "surface" somewhat.

I like this idea.

🎉

[reitermarkus]

• Catch these exceptions and fall back to the current Cask file's uninstall
• Catch these exceptions and print a warning saying uninstall hooks are not run

Those are not fixes, those are fallbacks.

• Instead of storing the entire Cask file in the Caskroom store just the part that's needed e.g. the uninstall hook method
• Instead of storing the entire Cask file in the Caskroom store instead store something to define uninstall logic that doesn't rely on Homebrew internals e.g. a Bash script or a JSON file that defines logic

This sounds good in theory, but I'm not sure if this isn't just shifting the problem elsewhere and additionally making the uninstall less flexible.

[MikeMcQuaid]

Those are not fixes, those are fallbacks.

Indeed but they are an improvement on the current situation of a known, unhandled exception.

This sounds good in theory, but I'm not sure if this isn't just shifting the problem elsewhere and additionally making the uninstall less flexible.

It will make uninstall less flexible but it's not going to increase the surface area of problems. In the undent case: most of these issues seem to be coming from places other than the uninstall block. Additionally, a bash script would isolate entirely from Homebrew Cask's Ruby (or even Bash) logic as Apple aren't going to be breaking Bash's backwards compatibility any time soon.

[MikeMcQuaid]

@commitay noted in https://github.com/Homebrew/brew/pull/5174/files#r227706037 that this is blocking the removal of Cask DSL (and, more generally, compatibility code in Homebrew/brew). When it gets to Homebrew 2.0.0 (the next major release expected after 1.9.0) around February if we don't have a solution here we just need to remove these DSL regardless.

[MikeMcQuaid]

@Homebrew/cask any thoughts or updates on this? Please note as above these DSLs will be removed in Homebrew 2.0 around February regardless of a PR like this being merged.

[vitorgalvao]

I’m still fine with the idea behind this.

[MikeMcQuaid]

This PR will be closed by #5598 when 2.0.0 is due to be released soon afterwards.