Homebrew-cask: brew cask remove intellij-idea removes custom unrelated shell skript "idea"

Created on 4 Aug 2017  路  15Comments  路  Source: Homebrew/homebrew-cask

Description of issue

when running brew cask remove intellij-idea a custom shell script called idea residing in ~/bin/ gets deleted too. I wrote that shell script and had to restore it from a backup. I don't see why homebrew cask should ever feel like touching it.

% ls -la ~/bin/idea
-rwxr--r-- 1 phi staff 1813 Jan 31  2016 /Users/phi/bin/idea

% brew cask install intellij-idea
==> Satisfying dependencies
==> Downloading https://download.jetbrains.com/idea/ideaIU-2017.2.1.dmg
Already downloaded: /Users/phi/Library/Caches/Homebrew/Cask/intellij-idea--2017.2.1,172.3544.35.dmg
==> Verifying checksum for Cask intellij-idea
==> Installing Cask intellij-idea
==> Moving App 'IntelliJ IDEA.app' to '/Applications/IntelliJ IDEA.app'.
馃嵑  intellij-idea was successfully installed!

% ls -la ~/bin/idea
-rwxr--r-- 1 phi staff 1813 Jan 31  2016 /Users/phi/bin/idea

% brew cask remove intellij-idea --verbose --debug
==> Uninstalling Cask intellij-idea
==> Uninstalling Cask intellij-idea
==> Un-installing artifacts
==> Determining which artifacts are present in Cask intellij-idea
==> 3 artifact/s defined
#<Hbc::Artifact::App:0x007fc4b381f248>
#<Hbc::Artifact::PostflightBlock:0x007fc4b381ff68>
#<Hbc::Artifact::Zap:0x007fc4b381fec8>
==> Un-installing artifact of class Hbc::Artifact::App
==> Removing App '/Applications/IntelliJ IDEA.app'.
==> Un-installing artifact of class Hbc::Artifact::PostflightBlock
==> Purging files for version 2017.2.1,172.3544.35 of Cask intellij-idea

% ls -la ~/bin/idea
gls: cannot access '/Users/phi/bin/idea': No such file or directory

I'm on Mac OS X 10.10 Yosemite.

Most helpful comment

C: 16:166: [Corrected] Use %r around regular expression.

To think that rule was introduced because of me, and I forgot it!

Do we want to make this consistent for all the Jetbrains casks

Yes. Easier to change it the future, if we need.

All 15 comments

I believe it is due to this part of the Cask:

uninstall_postflight do
    ENV['PATH'].split(File::PATH_SEPARATOR).map { |path| File.join(path, 'idea') }.each { |path| File.delete(path) if File.exist?(path) }
end

Ping @jcbot.

Jetbrains allows users to chose where to place a binary. Should we change the uninstall_postflight on the Jetbrains casks to an uninstall delete: in /usr/local/bin (the Jetbrains default location for binaries) to avoid removing files with the same name in the users PATH?

Ping @vitorgalvao @leipert

It鈥榙 be a shame to lose the flexibility of this because a single user by chance has a personal script that shares a name. Maybe we can refine it to:

ENV['PATH'].split(File::PATH_SEPARATOR).reject { |path| path.start_with?('~') || path.start_with?('/Users') }.map { |path| File.join(path, 'idea') }.each { |path| File.delete(path) if File.exist?(path) }

That way we ignore it if it鈥檚 in the user鈥檚 home dir. It might still defeat the purpose, but I鈥檓 not seeing a much better solution.

The functionality you're referring to ("Tools -> Create Launcher Script") does allow the user to both specify the location and the name of the script. It just defaults to "idea" but may be given any other name by the user.
Thus IMHO it is unwise to just delete everything in $PATH which is called "idea". I would in fact not expect homebrew to deal with this at all and leave it to the user to deal with any manually created launcher script herself.
But that's just my six cents.

It just defaults to "idea" but may be given any other name by the user.

And I bet close to none changes the name.

I would in fact not expect homebrew to deal with this at all and leave it to the user to deal with any manually created launcher script herself.

Most users would expect this to be dealt with. Especially since we have zap that is supposed to deal with every trace of an app. There鈥檚 a reason this removal is thorough: exactly because users care that we remove it.

If one assumes that almost nobody will change it, I'd be in favor of deleting /usr/local/bin/idea after briefly checking it is indeed the file created by IntelliJ IDEA (See below).
Also, would it be unreasonable to move this functionality from brew remove to brew zap? My reasoning is that this file is created outside the installation process. If that's in line with how homebrew does things normally, that is.

There are quite a few people who are using a custom idea launch script.
To prevent the worst I'd therefore suggest to check whether the following line is present in the script:

# see com.intellij.idea.SocketLock for the server side of this interface

or maybe this one:

RUN_PATH = u'/Applications/IntelliJ IDEA.app'

one could even check if it matches the installation path of the "intellij-idea" cask.

I hope these suggestions make sense :)
Cheers!

I'd therefore suggest to check whether the following line is present

I think this would be the best approach.

If one assumes that almost nobody will change it

The _name_, not the location.

would it be unreasonable to move this functionality from brew remove to brew zap?

Not unreasonable, but also not possible. zap does not take complex commands.

I'd therefore suggest to check whether the following line is present

I think this would be the best approach.

Agreed.

Also note we need a solution that鈥檒l work for all Intellij casks.

see com.intellij.idea.SocketLock for the server side of this interface

This line is in all of the Jetbrains binaries I've checked so far.

This should work:

ENV['PATH'].split(File::PATH_SEPARATOR).map { |path| File.join(path, 'idea') }.each { |path| File.delete(path) if File.exist?(path) && File.readlines(path).grep(/# see com.intellij.idea.SocketLock for the server side of this interface/).any? }

That is one hell of a one-liner. 馃槃

== intellij-idea.rb ==
C: 16:166: [Corrected] Use %r around regular expression.
ENV['PATH'].split(File::PATH_SEPARATOR).map { |path| File.join(path, 'idea') }.each { |path| File.delete(path) if File.exist?(path) && File.readlines(path).grep(%r{# see com.intellij.idea.SocketLock for the server side of this interface}).any? }

I'll do this tomorrow if no one else does it first. Do we want to make this consistent for all the Jetbrains casks or just the ones with "generic" binaries (idea, mine, charm)?

C: 16:166: [Corrected] Use %r around regular expression.

To think that rule was introduced because of me, and I forgot it!

Do we want to make this consistent for all the Jetbrains casks

Yes. Easier to change it the future, if we need.

To think that rule was introduced because of me, and I forgot it!

brew cask style is great, I hate to think how many mistakes I would make without it!


https://github.com/caskroom/homebrew-cask/pull/37407 and https://github.com/caskroom/homebrew-versions/pull/4244

Was this page helpful?
0 / 5 - 0 ratings