Add uninstall_depends_on_default_install_location caveat

[vitorgalvao]

This adds a uninstall_depends_on_default_install_location caveat, as well as changing three casks to use it. Thought descriptive, it is a verbose name, so ideas to make it shorter without losing clarity are very welcome.

This comes in the context of battle-net.rb and vuze.rb. Those casks use an installer via installer :manual that allows the user to pick an install location. To uninstall these, we need to delete files, but we cannot know where they are if the user picked an installation directory other than the default one. For that we currently use a caveat, but I bet there are many more casks that could benefit from it and do not yet have it, hence creating something specific should be beneficial.

caveats do
  uninstall_depends_on_default_install_location
end

should simply output If you pick an installation directory other than {default_install_path} when installing this cask, you will need to uninstall it manually, with {default_install_path} defaulting to /Applications if no argument is given. Maybe there aren’t cases where /Applications isn’t the default, but it doesn’t hurt to have the option there.

In a way, a better solution would be to automatically detect where an application is by their application bundle, but that would not work in cases like blast2go.rb since it would still leave its directory (and possibly sibling files) intact. It would still be nice to be able to do this on the casks that need nothing more, but that is for another issue.

Naturally, this needs review from the more experienced Ruby developers among you.

[rolandwalker]

Welcome to the core! I have gifted you a purple label.

I don't worry about the length per se, but I do fret about the term "install". It's probably correct here, but I am trying to separate the "install" and "activate" concepts, and something is nagging at me.

Conceptually, this is coupled to uninstall. In addition, a caveat fires a message to all users, including the (majority) defaulters. What about an alternative interface such as

uninstall_preflight do
  warn_unless_exists '/Applications/Blast2GO'
end

or even

# warns if delete isn't satisfied
uninstall :delete_warn => '/Applications/Blast2GO'

and by the way, one of the reasons I wanted to move to Ruby 2.0 was to try some nutty things along the lines of

uninstall :delete => '/Applications/Blast2GO'.but_warn

Edit: uninstall_preflight, not postflight

[vitorgalvao]

Welcome to the core! I have gifted you a purple label.

Thank you!

In addition, a caveat fires a message to all users, including the (majority) defaulters.

That was actually part of the decision to make it a caveat — make our limitations explicit. Users likely expect to effectively be able to brew cask uninstall something they brew cask installed. Showing a warning only when they try to uninstall is saying “the decision you made in the past just made it impossible for us to help you in the present”, while showing a warning on install says “think well before you make a decision, as it might affect your experience in the future”, and we explain exactly in what manner, possibly even “converting” people that would otherwise pick atypical install locations.

[rolandwalker]

I see. Yet caveats fires even on brew cask info (which might need review, as that is rarely if ever helpful to me.)

Returning to interface, caveats has no context. Inside caveats we are free-floating, building up meaning from scratch, and that's why the long method name stacks up. So I am still inclined to look for a context which fits to your meaning. What about this:

installer :manual => '/Applications/Blast2GO',
          :recommend_defaults => true

[vitorgalvao]

I definitely like that solution best. I’ll likely need help with that, though. I’ll try to take a look at it soon to see; I believe I know where to look, but I’m not yet sure as to how to implement it.

[rolandwalker]

Good. There's another advantage: installer is an artifact, and artifacts know how to respond to both the install and uninstall verbs.

Currently the uninstall verb has no effect for installer :manual, but the hook exists. We could make it give a different notice at uninstall time. Or we could pass the recommended path value instead of true, and make the uninstall-time warning conditional on the existence of the path.

The install/uninstall implementation is entirely contained in lib/cask/artifact/installer.rb. We would also have to add the desired :key_name here.

[rolandwalker]

How can I help you move forward here? Should I update the DSL and put a comment where the new logic should live?

[vitorgalvao]

I haven’t forgotten about this, but I haven’t looked into it like I should mainly on account of the holidays (have a visiting friend) and I’ll need to sit down a bit with the issue, since it’s somewhat new territory.

However, since the feature is non-critical and it should be contained in those two relatively small files, I do want to try and take a stab at it. I appreciate your offer for help and I’ll likely need it; I’m just waiting a few more days to settle down and see how far I can get.

[rolandwalker]

No hurry of course, just wanted to make sure I'm not blocking. Grab me on IRC any time.

[vitorgalvao]

No longer relevant due to #13201.