Homebrew-cask: New strategy for `.metadata`

[vitorgalvao]

When we install a cask, we keep a copy of it at that moment in a .metadata directory. This method causes problems when we remove a feature from the core, as the old casks throw errors when read back. An example of widespread breakage due to this method is https://github.com/Homebrew/homebrew-cask/issues/58046.

https://github.com/Homebrew/brew/pull/4730#issuecomment-415322677 suggests a few alternative to the current method. The first one is the most viable: when reading a cask in .metadata, if it gives an error, ignore it and read the current cask.

[reitermarkus]

The only thing we need from the cask copy is the uninstall logic.

Actually, we need everything. In case an upgrade fails, we should still be able to reinstall the old version.

[vitorgalvao]

Actually, we need everything. In case an upgrade fails, we should still be able to reinstall the old version.

Edited the top post to only reference the first option.

[vitorgalvao]

I’ve been thinking a bit more about this. The top post include another suggestion:

The only thing we need from the cask copy is the uninstall logic. When installing, instead of keeping a copy of the cask, create an uninstall bash script. We already run system commands via ruby to perform the uninstallation anyway, so might as well save those commands.

That was removed due to the point that we need everything, in the case of an upgrade fail. But how often does that happen? We could just as well do both. We always uninstall from the shell script, and only reference the .metadata cask if an upgrade fails. At which point, if that fails we’re back where we started, but in the way we’ve already got rid of most errors by offloading uninstall to script.

[MikeMcQuaid]

Homebrew/brew#4730 (comment) suggests a few alternative to the current method. The first one is the most viable: when reading a cask in .metadata, if it gives an error, ignore it and read the current cask.

I think we should do this. We're getting more and more reports of this and the status-quo is pretty untenable and undesirable. Any objections?

[vitorgalvao]

I think we should do this. We're getting more and more reports of this and the status-quo is pretty untenable and undesirable. Any objections?

None whatsoever. We should also think about what to do if the failing cask no longer exists. My suggestion would be to inform the user they’ll need to uninstall it manually, cat the cask, and remove its directory from the Caskroom (so it no longer shows as installed).

[cpcallen]

Pending resolution, might it be worth retitling this issue so that people looking for bugs related to Error: Cask '…' is unreadable: undefined method `undent' can find it more easily?

[vitorgalvao]

Pending resolution, might it be worth retitling this issue so that people looking for bugs related to Error: Cask '…' is unreadable: undefined method `undent' can find it more easily?

That’s a prevalent issue arising from this, but far from the only one. Unexpected method 'license' called on Cask is another one.

[cpcallen]

That’s _a_ prevalent issue arising from this, but far from the only one.

Sure, I understand. The more the merrier.