Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Process zap.early_script before uninstall.launchctl #85498

Closed
Saviq opened this issue Jul 7, 2020 · 7 comments
Closed

Process zap.early_script before uninstall.launchctl #85498

Saviq opened this issue Jul 7, 2020 · 7 comments
Labels

Comments

@Saviq
Copy link
Contributor

Saviq commented Jul 7, 2020

Description of feature/enhancement

When using brew cask zap …, uninstall is implicitly called before zap. I believe all uninstall.* stanzas should be merged with the corresponding ones in zap.*, and processed together, in the documented order.

Justification

Consider the case when zapping a cask requires the service to do some initial cleanup. The implicit uninstall.launchctl processing will remove the service before zap.early_script gets invoked.

Example use case

Multipass may optionally use VirtualBox as the VM backend, and brew cask zap multipass should remove those. But the service is stopped through uninstall.launchctl, so even in zap.early_script it's too late.

@miccal
Copy link
Member

miccal commented Jul 8, 2020

As far as I am aware, the zap stanza is used to remove files and folders only and is not used to run any scripts -- can you please explain in more detail the problem you are experiencing?

@Saviq
Copy link
Contributor Author

Saviq commented Jul 10, 2020

Hi, according to the zap docs:

The form of zap stanza follows the uninstall stanza. All of the same directives are available.

I don't know what more I can add, there's two "modes" that you can uninstall Multipass - non-, and -destructive to user data, so that maps quite nicely to uninstall and zap. But as far I can tell, there's currently no way to use it that way, since uninstall is always called before zap.

@miccal
Copy link
Member

miccal commented Jul 10, 2020

So if I understand correctly, you want to call zap without calling uninstall?

@vitorgalvao
Copy link
Member

Consider the case when zapping a cask requires the service to do some initial cleanup.

That cleanup should be done in uninstall, likely.

Multipass may optionally use VirtualBox as the VM backend, and brew cask zap multipass should remove those.

“Those” what? We’re not going to break a Virtualbox installation on the zap of another cask that may optionally have used it. Yes, zap can remove things that were installed by other apps, but within reason.

So if I understand correctly, you want to call zap without calling uninstall?

If that’s what you’re asking, we have no plans to do it.

@vitorgalvao vitorgalvao added the awaiting user reply Issue needs response from a user. label Jul 10, 2020
@Saviq
Copy link
Contributor Author

Saviq commented Jul 10, 2020

So if I understand correctly, you want to call zap without calling uninstall?
If that’s what you’re asking, we have no plans to do it.

No, we'd need things to be called in this order:

  • uninstall.early_script
  • zap.early_script
  • uninstall.launchctl
  • zap.launchctl
  • etc.

So interleave each step of uninstall.* with the corresponding step from zap.*, rather than going through all of uninstall.* first.

That cleanup should be done in uninstall, likely.

I'd say the split between uninstall and zap maps quite nicely to non-destructive and destructive.

“Those” what? We’re not going to break a Virtualbox installation on the zap of another cask that may optionally have used it. Yes, zap can remove things that were installed by other apps, but within reason.

No, the exact case is: we register VMs and VM disks with VirtualBox, brew cask uninstall does not remove them to avoid data loss, but brew cask zap would. It's not just about removing files, for that we'd need the Multipass daemon to still be running, so zap.early_script felt like the right place - but the daemon is already stopped and removed through uninstall.launchctl.

@no-response no-response bot removed the awaiting user reply Issue needs response from a user. label Jul 10, 2020
@vitorgalvao
Copy link
Member

I understand your request, but I also think it’s too specific to bother with officially. Almost no cask would benefit from it.

If you’d be willing to make a PR for it, we can consider it and I’ll ping other maintainers to see if they think the feature has merit and would be willing to review it. But if you’re not willing / able to do the legwork, we can close this now.

@vitorgalvao vitorgalvao added the awaiting user reply Issue needs response from a user. label Jul 10, 2020
@vitorgalvao vitorgalvao removed the awaiting user reply Issue needs response from a user. label Jul 21, 2020
@vitorgalvao
Copy link
Member

Closing for lack of reply.

@Homebrew Homebrew locked as resolved and limited conversation to collaborators Dec 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants