Homebrew / homebrew-cask

🍻 A CLI workflow for the administration of macOS applications distributed as binaries
https://brew.sh
BSD 2-Clause "Simplified" License
20.94k stars 10.71k forks source link

`brew cask upgrade` outline #29301

Closed vitorgalvao closed 6 years ago

vitorgalvao commented 7 years ago

This issue is meant to outline the desired behaviour of upgrade (and outdated). Marking as ready to implement but discussion is encouraged. Will keep this top post updated with the final decisions of the discussion.

outdated should be implemented first, to make sure we have a solid foundation. upgrade should be built on top of it

Behaviour of brew cask outdated:

  1. If given arguments, act on those casks. If not, act on all installed casks.
  2. If called with --greedy, jump directly to step 5.
  3. Ignore the ones that have auto_updates true.
  4. Ignore the ones that have version :latest.
  5. For the rest, compare the installed version to the latest one.
  6. Output outdated casks with the same format as Homebrew:
aria2 (1.30.0) < 1.31.0
cmake (3.7.1) < 3.7.2

Behaviour for brew cask upgrade:

Same steps 1–5.

  1. For the ones that differ, run the uninstall and install steps. Uninstalling is necessary to avoid leaving old traces of it when uninstallation procedures change.
  2. If install fails (after uninstall), revert to previous version.

For those interested in giving a go at this, take a look at https://github.com/Homebrew/brew/pull/1523. It should give you a starting point (but don’t feel obligated to use it). That specific PR has things that would need to be removed (pin/unpin) and added (take auto_updates into account). If working on it, ideally you could commit to provide some further support for the feature for a little while (just enough to make sure the code itself is stable, not be tied to it ad eternum).

There was an important consideration when thinking about this feature:

You have a commercial app at version 3.2. The developer releases a paid upgrade: 4.0, the cask is updated, and you upgrade. However, you did not want to pay for version 4.0. Now you’re stuck with a version you do not want. Not only is that a bad experience for you as a user, now you’ll likely introduce that last version into caskroom/versions, further filling the repo.

Some solutions were discussed, but ultimately it was argued they might not be needed or be detrimental at this stage, before we have a better understanding on how best to proceed:

(…) we have auto_updates (not yet taken into account in this PR) and I’m thinking: “are there any commercial apps that do not auto-update”? I’m betting those might very well be the minority if they exist (after all, if you ask for more money for upgrades, you want to auto-update so your users get notified).


Pinging @caskroom/maintainers. Any user is welcome to chime in.

miccal commented 7 years ago

This looks promising - may I suggest, though, that as a first step in this and to provide some testing, that brew cask outdated gets done and pushed before brew cask upgrade.

The reason I suggest this is that it seems to me the proposal for brew cask outdated doesn't actually do anything but list outdated Casks. Implementing this first will not interfere with user's existing Cask installs, and may then highlight any potential problems that we can tackle before implementing brew cask upgrade.

vitorgalvao commented 7 years ago

@miccal No objections. Ideally they’ll be built at the same time anyway, and upgrade should lean on outdated.

reitermarkus commented 7 years ago

This looks promising - may I suggest, though, that as a first step in this and to provide some testing, that brew cask outdated gets done and pushed before brew cask upgrade.

I second that, as we first need to figure out what outdated means exactly.

vitorgalvao commented 7 years ago

Edited top post to give precedence to outdated. Also added another step: “ignore the ones that have version :latest”.

mwean commented 7 years ago

I think this is still needed. I've had several issues with the Slack app where the auto-update showed no updates, but there was an updated cask. I'm pretty busy, but I might be able to look into this next weekend.

reitermarkus commented 7 years ago

You should still be able to force updates for :latest casks, at least.

miccal commented 7 years ago

I think this is still needed. I've had several issues with the Slack app where the auto-update showed no updates, but there was an updated cask. I'm pretty busy, but I might be able to look into this next weekend.

I have a similar issue with Dropbox, so I run cd /usr/local/Caskroom/; rm -r dropbox; cd so that Dropbox does not appear on brew cask list and therefore will not be updated whenever the Cask is updated.

I have always had issues whenever I force Dropbox to update.

vitorgalvao commented 7 years ago

so I run cd /usr/local/Caskroom/; rm -r dropbox; cd

Small note: cd - changes to the previous working directory. So cd /usr/local/Caskroom/; rm -r dropbox; cd - will get you back to wherever you were before. But why not just rm -r /usr/local/Caskroom/dropbox directly?

Even in those cases cases of Dropbox and Slack, presumably they’d still be auto-updated at some point and it might be that the apps are themselves delaying it on purpose (to prevent everyone contacting the server at once?). With that in mind, I don’t think we need to by default force those to update when the cask does. We might still ignore it. If a user knows an update was released and wants it right now, reinstall suffices.

miccal commented 7 years ago

Small note: cd - changes to the previous working directory. So cd /usr/local/Caskroom/; rm -r dropbox; cd - will get you back to wherever you were before. But why not just rm -r /usr/local/Caskroom/dropbox directly?

I like to be in the directory I am working in - don't ask me why, because I don't have an answer!

leipert commented 7 years ago

I am using buo/homebrew-cask-upgrade successfully for over 6 months.

For me it seems like brew cu --dry-run behaves like the wanted brew cask outdated, with the exceptions

  1. you just can check all casks
  2. auto_updates casks are not filtered out
  3. the output format is different from brew

brew cu behaves like the wanted brew cask upgrade, except it does not do rollback or the uninstall routines.

I just wanted to link to this project, as it is already written in ruby and maybe some of the code/ideas can be reused.

/cc https://github.com/buo/homebrew-cask-upgrade/issues/23

vitorgalvao commented 7 years ago

@leipert As has been repeated ad nauseam and is even stated in that repo, those scripts are fragile and rely on unreliable features. By your own admission, that one is missing pretty major stuff.

Lets please focus on building this right and natively, not retrofitting incomplete scripts that use user-facing features. Those are slow and can’t be trusted entirely.

claui commented 7 years ago

While I fully agree with what @vitorgalvao said, thank you @leipert for the heads up 👍

colindunn commented 7 years ago

Would love to see a flag for both outdated and upgrade to show auto_updates casks anyway. Just in case there was an error in the cask, or you have the auto update feature in the app disabled for some reason.

vitorgalvao commented 7 years ago

or you have the auto update feature in the app disabled for some reason

How?

toonetown commented 7 years ago

I agree. It would be nice to be able to force the update and outdated checks… For example, most apps that do auto update, have the option to opt in to automatic updates. Someone may turn that off in the app, and rely instead on hbc to do the updates.

I don't know why I would want to do that… But it might be a use case to consider.

vitorgalvao commented 7 years ago

With multiple maintainers and users asking for it, --force has now been added to the top post.

vitorgalvao commented 7 years ago

Changed --force to --extensive (seems more accurate/clearer).

mwean commented 7 years ago

What does --extensive mean? --force makes more sense to me

toonetown commented 7 years ago

I think within the context of "outdated" "--extensive" makes more sense (i.e. brew cask outdated --extensive) - but within the context of "upgrade", "--force" makes more sense (i.e. brew cask upgrade --force).

However, it gets confusing when there are two different options with basically do the same thing.

Personally, I prefer "--force" (even brew cask outdated --force makes a little bit of sense)...but I really don't care what the option is called.

toonetown commented 7 years ago

And a minor nitpick - for "Behavior for brew cask upgrade", it states "Same steps 1–5." and then it starts with step "5" again.

mwean commented 7 years ago

What about --all?

vitorgalvao commented 7 years ago

And a minor nitpick - for "Behavior for brew cask upgrade", it states "Same steps 1–5." and then it starts with step "5" again.

Thank you. Fixed.

but within the context of "upgrade", "--force" makes more sense (i.e. brew cask upgrade --force).

I feel --force is inadequate because it looks like we’re doing a force reinstall, and that is not the case at all. I’d rather we save the keyword for if we actually need it later.

What about --all?

I fear that sends the wrong message. Just as people ran brew update && brew cask update (which we deprecated it exactly because it was being used wrongly), --all is “what do you mean “all”? Of course I want all to be updated, that should be the default”. I’m betting we’d see that option used in a majority of the time when it doesn’t make sense.

--extensive to me means “do more than the recommended”. Whatever the exact word is, that’s the message I think it should convey: “you should not be using this flag most of the time, and it does more than the recommended”. It also indicates the operation will take longer (which it will).

I think it’s more beneficial to have a word users aren’t absolutely clear about and as such will need to read the docs, than one they think they are clear about when it actually means something else.

leipert commented 7 years ago

Regarding those two statements:

or you have the auto update feature in the app disabled for some reason

How?

I don't know why I would want to do that… But it might be a use case to consider.

I actually disable all autoupdates by blocking http(s) calls with Little Snitch for most apps and/or disabling the auto checks in the options, because I do not want them to send more information "home" than necessary and do not want to handle constant nagging/auto-downloads. I'd rather have manual updates with the help of brew cask, so I have control over when an updates happen and which apps will get updates.

This brings me to my question:

Would it make sense to have two different flags for auto_updates true and version :latest? Because on a normal day I just want nicely versioned upgrades for e.g. firefox and not for version: latest casks.

Sorry for hijacking the discussion again and a big 👍 for all your work, you lovely maintainers :)

vitorgalvao commented 7 years ago

@leipert That setup makes you an outlier.

Just like changing the install directory to /Applications HBC is a slight inconvenience to users who run on non-admin accounts, or how HB does not support installing anywhere other than /usr/local, even though that is technically possible, #13201 is clear: workarounds in the code make a mess, and our procedure should seek to emulate normal manual usage.

Blocking all auto-upgrades and upgrading manually is not common enough to be supported as a specific use-case and complicating the options.

We might eventually reach the conclusion it is indeed worth it, but at this point I don’t see it.

leipert commented 7 years ago

@vitorgalvao I know it makes me an outlier and I demand no support for my setup. Just shared it, so you maintainers have a better understanding how HBC is used "in the wild".

claui commented 7 years ago

Good suggestions so far! While --force seems obvious at a glance, I don’t see how we can make it work well in practice, given the commitment we already have made in too many places that whenever we say --force, we mean “do whatever it takes to make this work” and that is a whole different thing.

With upgrade --all, we would give users too many ways to read either less or more into it than it actually is (cf. @vitorgalvao’s example).

Regarding --extensive: of the suggestions on the table so far, I can see it work best; yet, it feels just a tad too smart, overly far removed from our everyday language. (Let’s say your spouse just returned from work. Would you, in real life, greet him/her with the words, “Howdy love! Would you mind taking off your shoes because I just gave the house an extensive vacuuming?”)

Let’s consider simple words, like --greedy. I’d personally prefer --greedy; it feels more in touch with the language we actually use in our lives. It’s quite vivid so users might have an easier time to memorize it; that gives it a practical value. It’s already being used in the wild as a configuration option (although this is true for the other suggestions, too). Programmers will probably recall greedy from yet another, not too unfamiliar domain; and while we’re there, one of its antonyms happens to be surprisingly consistent with what we already have in mind for the default, non--greedy behavior: “try to do as little work as possible.”

vitorgalvao commented 7 years ago

@claui I’m completely down with --greedy! Clear, easy to memorise, write, and understand. Also gives the right mood for the action. I’m convinced.

reitermarkus commented 7 years ago

Yes, --greedy seems like a better idea than --brute-force. 😜

Saklad5 commented 7 years ago

I don’t suppose we could add a command or flag that directly checks appcasts for Casks with version :latest and outputs the results? That would be extremely helpful for manual usage. I understand there is already a similar script in Homebrew-Cask, but that is not intended for non-maintainers to use.

vitorgalvao commented 7 years ago

I don’t suppose we could add a command or flag that directly checks appcasts for Casks with version :latest and outputs the results?

There are no casks with version :latest and an appcast. If you found one, it needs to be fixed. All casks with appcasts get a specific version, even if the url is unversioned.

colindunn commented 7 years ago

There were 5 still with :latest and an appcast @vitorgalvao. Fixed in #29756

What exactly are you after @Saklad5 ? brew cask list --versions will show you the installed casks with version latest. You're not going to be able to get any available update information though.

Saklad5 commented 7 years ago

I was hoping for a way to narrow down which programs may be outdated manually.

ward commented 7 years ago

Google still ranks https://github.com/caskroom/homebrew-cask/issues/4678 pretty high up when searching for brew cask upgrade. Perhaps explicitly mention this issue in there? (Though I guess due to the way GitHub works, that may be done automatically after I submit this comment)

joshka commented 7 years ago

One of the issues that is mentioned in this and #1523 is the problem of how to handle several major versions. Homebrew core recently changed their versioning strategy from a repo approach to a file approach. Documented at http://docs.brew.sh/Versions.html and https://github.com/Homebrew/brew/issues/620. I think it would be a good solution to the problem to do something similar in HBC.

I'll use a real example. Recently Bitwig released Bitwig Studio (BWS) 2.0, a paid upgrade. I anticipate continuing to use BWS 1 for a while. I'd like to see the following casks in this repo: bitwig-studio@1.3, bitwig-studio@2.

toonetown commented 7 years ago

@joshka Homebrew core recently changed their versioning strategy from a repo approach to a file approach.

This actually already works with casks too - I have created a cask for kindle that pins the version at 1.17 (https://github.com/toonetown/homebrew-extras/blob/master/Casks/kindle%401.17.rb)

I don't know how "supported" it is...but at least all the under-the-scenes plumbing is in place to where that is an option.

vitorgalvao commented 7 years ago

There’s nothing about that approach that would make implementing upgrade easier. If you’ll continue to use an old version of an app, use the one in caskroom/versions. “What about if we ever remove that version?”. Then we would’ve removed the cask with the version in the name as well.

joshka commented 7 years ago

I like the idea of tackling this a small piece at a time per https://github.com/caskroom/homebrew-cask/issues/29301#issuecomment-274372842. I think there could be a few small steps between brew cask outdated and brew cask upgrade that would be helpful.

@vitorgalvao you mentioned that uninstall was necessary in case the uninstallation procedures change. I think you miss that if the uninstallation procedures changed between versions, the valid uninstallation procedure for the later version of an app should very likely be those of the later version not the earlier. If I update in-app, it would be also be nice to be able to mark the app as updated in the caskroom rather than reinstalling. I'm assuming this is a place where that's a general rule, but one that has some complexity and exceptions? This is just my guess though.

So I think we could use a few more commands prior to implementing brew cask update.

  1. brew cask check-version --appcast: takes a cask and checks the installed version / latest version against the app's appcast. The output of this command would be useful for determining whether the cask is out of date (and the app if it's not yet been updated).

I imagine this being used to automatically ping a particular maintainer to update an app's cask to the latest version whenever the appcast gets an update.

  1. brew cask check-version --plist: takes a cask and checks the CFBundleShortVersionString from `Abc.App/Contents/Info.plist'. This would be useful in determining whether the app is newer than the installed version / cask.

I'm guessing that these would very likely cover a large amount of cases, and allow us to know when to update the app / cask / both. However we might need custom version sensing for some auto_updates / latest casks that don't install an app, or those that break this assumption.

  1. A brew cask mark-updated command might be useful to mark the installed app's version as the installed version. E.g. brew thinks v1.0 is installed, but I've updated to v1.1, brew cask mark-updated picks up the plist or appcast version (or other custom mechanism).
muescha commented 7 years ago

for 2)

you get with mdls -name kMDItemVersion <path to file> also the version. what is better?

(example mdls -name kMDItemVersion /Applications/iPhoto.app)

vitorgalvao commented 7 years ago

you mentioned that uninstall was necessary in case the uninstallation procedures change. I think you miss that if the uninstallation procedures changed between versions, the valid uninstallation procedure for the later version of an app should very likely be those of the later version not the earlier.

Not at all. That is precisely why we have a .metadata directory that saves a copy of the cask at the moment you installed. When uninstalling, that is the used procedure. By your logic, casks would already be breaking, upgrade or not.

If I update in-app, it would be also be nice to be able to mark the app as updated in the caskroom rather than reinstalling.

There is no way to do that accurately. auto_updates exists for that very reason. If that’s important to you, you’ll need to reinstall.

  1. brew cask check-version --appcast: takes a cask and checks the installed version / latest version against the app's appcast. The output of this command would be useful for determining whether the cask is out of date (and the app if it's not yet been updated).

No. Appcasts are mostly different, even amongst themselves (github, sparkle), and there’s no reliable way to get versions from them without working mostly case-by-case.

I imagine this being used to automatically ping a particular maintainer to update an app's cask to the latest version whenever the appcast gets an update.

There were already scripts done for that, and more are in the works. That’s the whole goal of appcast, they were always built with that in mind.

  1. brew cask check-version --plist: takes a cask and checks the CFBundleShortVersionString from `Abc.App/Contents/Info.plist'. This would be useful in determining whether the app is newer than the installed version / cask.

Also no. There are a ton of casks that aren’t apps, so this wouldn’t work.

However we might need custom version sensing for some auto_updates / latest casks that don't install an app, or those that break this assumption.

Which means more ugly workarounds and a new mess to handle. The very thing #13201 aims to fix.

  1. A brew cask mark-updated command might be useful to mark the installed app's version as the installed version. E.g. brew thinks v1.0 is installed, but I've updated to v1.1, brew cask mark-updated picks up the plist or appcast version (or other custom mechanism).

Again, you can’t rely on the plist. At the most all this command could do is say “replace the old .metadata with the latest version of the cask”. That’s an ugly manual process, though. It’d make more sense to add a new flag to uninstall to say “ignore the .metadata version and use the current one”, or just make all auto_updates cask do that by default.

Saklad5 commented 7 years ago

Since it seems like a lot of the obstacles consist of dealing with vastly different types of Casks, I am starting to think it would be wise to split some types into entirely new Homebrew projects. I don’t see how simplicity and functionality can be preserved otherwise.

vitorgalvao commented 7 years ago

Since it seems like a lot of the obstacles are dealing with vastly different types of Casks

Not at all. Even similar types of casks (apps, fonts, pkgs) are different amongst themselves. Splitting them into new homebrew projects would be nonsensical. A bunch of the logic would be repeated, and problems would multiply instead or being reduced.

The reason #13201 came to be was precisely because it was thought that all apps could be dealt in the same way, and all pkgs in the same different way, and so on. But they can’t. Just because apps share a type, it does not mean they can be dealt with using the exact same methods. Exceptions always occur.

Saklad5 commented 7 years ago

The code could easily be shared, in the same fashion that Homebrew-Cask shares functionality with Homebrew proper already. And individual projects could still include workarounds and exceptions, though they would need fewer for obvious reasons.

I’m not saying it is a good idea. However, ugly workarounds seem like the only other option, no?

vitorgalvao commented 7 years ago

However, ugly workarounds seem like the only other option, no?

No. We’re reducing those, not adding them, and it’s working well. Again, read #13201.

joshka commented 7 years ago

Following on from https://github.com/Homebrew/brew/pull/2250#discussion_r104303535, how should Casks that aren't in a tap show in outdated? E.g.:

$ brew cask outdated --greedy
alfred: (3.1_718) < 3.3.1_806
arturia-analog-lab: (1.2.4.126) <
arturia-mini-v: (2.7.0.76) <
arturia-software-center: (1.1.3.145) <
atom: (1.8.0) < 1.14.4
...

I wrote the arturia Casks some time ago, submitted as a PR, but neglected to fix up the uninstall appropriately to make them acceptable.

toonetown commented 7 years ago

I would say they show the same way that formulas that aren't in a tap show for brew outdated

joshka commented 7 years ago

@toonetown did you perhaps mean brew cask list?

Edit: ah - just realized you said brew not brew cask ;)

reitermarkus commented 7 years ago

how should Casks that aren't in a tap show in outdated?

What does that even mean? If they aren't in a Tap, they shouldn't be outdated in the first place, since there is no way to tell, right?

toonetown commented 7 years ago

Oh - I see what you mean now...there actually is no update for those casks...so they shouldn't be showing up, I would say.

I thought you were talking about how the output of brew cask outdated Should be formatted, and I was just stating that it probably should mirror how brew outdated is formatted.

vitorgalvao commented 7 years ago

If they aren't in a Tap, they shouldn't be outdated in the first place, since there is no way to tell

Agreed.

how should Casks that aren't in a tap show in outdated?

They should never appear in outdated.

joshka commented 7 years ago

Agreed - I left the nil check in to ensure these types of Casks don't show up. https://github.com/Homebrew/brew/pull/2250/commits/bc3bc1a4d6776b3675a1a365e20b5931d5bfdf50#diff-f85d4948a976b9727959680dbfe1118aR94