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

Fix uninstall not working when version changes. #23328

Closed
wants to merge 12 commits into from
Closed
4 changes: 2 additions & 2 deletions lib/hbc/cask.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def initialize(token, sourcefile_path: nil, dsl: nil, &block)
METADATA_SUBDIR = ".metadata".freeze

def metadata_master_container_path
caskroom_path.join(METADATA_SUBDIR)
@metadata_master_container_path ||= caskroom_path.join(METADATA_SUBDIR)
end

def metadata_versioned_container_path
Expand Down Expand Up @@ -79,7 +79,7 @@ def versions
end

def installed?
staged_path.exist?
!versions.empty?
end

def to_s
Expand Down
23 changes: 23 additions & 0 deletions lib/hbc/cli/uninstall.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,34 @@ def self.run(*args)
cask_tokens = cask_tokens_from(args)
raise Hbc::CaskUnspecifiedError if cask_tokens.empty?
force = args.include? "--force"

cask_tokens.each do |cask_token|
odebug "Uninstalling Cask #{cask_token}"
cask = Hbc.load(cask_token)

raise Hbc::CaskNotInstalledError, cask unless cask.installed? || force

latest_installed_version = cask.timestamped_versions.last

unless latest_installed_version.nil?
latest_installed_cask_file = cask.metadata_master_container_path
.join(latest_installed_version.join(File::Separator),
"Casks", "#{cask_token}.rb")

# use the same cask file that was used for installation, if possible
cask = Hbc.load(latest_installed_cask_file) if latest_installed_cask_file.exist?
end

Hbc::Installer.new(cask, force: force).uninstall

next if (versions = cask.versions).empty?

single = versions.count == 1

puts <<-EOF.undent
#{cask_token} #{versions.join(', ')} #{single ? 'is' : 'are'} still installed.
Remove #{single ? 'it' : 'them all'} with `brew cask uninstall --force #{cask_token}`.
EOF
end
end

Expand Down
2 changes: 1 addition & 1 deletion lib/hbc/installer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ def purge_versioned_files
odebug "Purging files for version #{@cask.version} of Cask #{@cask}"

# versioned staged distribution
gain_permissions_remove(@cask.staged_path)
gain_permissions_remove(@cask.staged_path) if !@cask.staged_path.nil? && @cask.staged_path.exist?

# Homebrew-Cask metadata
if @cask.metadata_versioned_container_path.respond_to?(:children) &&
Expand Down
27 changes: 14 additions & 13 deletions lib/hbc/scopes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,23 +42,24 @@ def all_tokens
end

def installed
@installed ||= {}
installed_cask_dirs = Pathname.glob(caskroom.join("*"))
# Hbc.load has some DWIM which is slow. Optimize here
# by spoon-feeding Hbc.load fully-qualified paths.
# TODO: speed up Hbc::Source::Tapped (main perf drag is calling Hbc.all_tokens repeatedly)
# TODO: ability to specify expected source when calling Hbc.load (minor perf benefit)
installed_cask_dirs.map do |install_dir|
cask_token = install_dir.basename.to_s
path_to_cask = all_tapped_cask_dirs.find { |tap_dir|
tap_dir.join("#{cask_token}.rb").exist?
}
@installed[cask_token] ||= if path_to_cask
Hbc.load(path_to_cask.join("#{cask_token}.rb"))
else
Hbc.load(cask_token)
end
end
Pathname.glob(caskroom.join("*"))
.map { |caskroom_path|
token = caskroom_path.basename.to_s

path_to_cask = all_tapped_cask_dirs.find { |tap_dir|
tap_dir.join("#{token}.rb").exist?
}

if path_to_cask
Hbc.load(path_to_cask.join("#{token}.rb"))
else
Hbc.load(token)
end
}
end
end
end
2 changes: 1 addition & 1 deletion lib/hbc/without_source.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ class Hbc::WithoutSource < Hbc::Cask
# Override from `Hbc::DSL` because we don't have a cask source file to work
# with, so we don't know the cask's `version`.
def staged_path
caskroom_path.children.first
(caskroom_path.children - [metadata_master_container_path]).first
end

def to_s
Expand Down
4 changes: 2 additions & 2 deletions test/cask/cli/list_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
casks = %w[local-caffeine local-transmission].map { |c| Hbc.load(c) }

casks.each do |c|
TestHelper.install_without_artifacts(c)
TestHelper.install_with_caskfile(c)
end

lambda {
Expand Down Expand Up @@ -54,7 +54,7 @@
casks = %w[local-caffeine local-transmission].map { |c| Hbc.load(c) }

casks.each do |c|
TestHelper.install_without_artifacts(c)
TestHelper.install_without_artifacts_with_caskfile(c)
end

caffeine, transmission = casks
Expand Down
88 changes: 82 additions & 6 deletions test/cask/cli/uninstall_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,22 +41,98 @@
File.exist?(Hbc.appdir.join("Caffeine.app")).must_equal false
end

describe "when Casks have been renamed" do
describe "when multiple versions of a cask are installed" do
let(:token) { "versioned-cask" }
let(:first_installed_version) { "1.2.3" }
let(:last_installed_version) { "4.5.6" }
let(:timestamped_versions) {
[
[first_installed_version, "123000"],
[last_installed_version, "456000"],
]
}
let(:caskroom_path) { Hbc.caskroom.join(token).tap(&:mkpath) }

before(:each) do
timestamped_versions.each do |timestamped_version|
caskroom_path.join(".metadata", *timestamped_version, "Casks").tap(&:mkpath)
.join("#{token}.rb").open("w") do |caskfile|
caskfile.puts <<-EOF.undent
cask '#{token}' do
version '#{timestamped_version[0]}'
end
EOF
end
caskroom_path.join(timestamped_version[0]).mkpath
end
end

after(:each) do
caskroom_path.rmtree if caskroom_path.exist?
end

it "uninstalls one version at a time" do
shutup do
Hbc::CLI::Uninstall.run("versioned-cask")
end

caskroom_path.join(first_installed_version).must_be :exist?
caskroom_path.join(last_installed_version).wont_be :exist?
caskroom_path.must_be :exist?

shutup do
Hbc::CLI::Uninstall.run("versioned-cask")
end

caskroom_path.join(first_installed_version).wont_be :exist?
caskroom_path.wont_be :exist?
end

it "displays a message when versions remain installed" do
out, err = capture_io do
Hbc::CLI::Uninstall.run("versioned-cask")
end

out.must_match(%r{#{token} #{first_installed_version} is still installed.})
err.must_be :empty?
end
end

describe "when Casks in Taps have been renamed or removed" do
let(:app) { Hbc.appdir.join("ive-been-renamed.app") }
let(:caskroom_path) { Hbc.caskroom.join("ive-been-renamed").tap(&:mkpath) }
let(:saved_caskfile) { caskroom_path.join(".metadata", "latest", "timestamp", "Casks").join("ive-been-renamed.rb") }

before do
@renamed_path = Hbc.caskroom.join("ive-been-renamed", "latest", "Renamed.app").tap(&:mkpath)
@renamed_path.join("Info.plist").open("w") { |f| f.puts "Oh plist" }
app.tap(&:mkpath)
.join("Contents").tap(&:mkpath)
.join("Info.plist").tap(&FileUtils.method(:touch))

caskroom_path.mkpath

saved_caskfile.dirname.mkpath

IO.write saved_caskfile, <<-EOF.undent
cask 'ive-been-renamed' do
version :latest

app 'ive-been-renamed.app'
end
EOF
end

after do
@renamed_path.rmtree if @renamed_path.exist?
app.rmtree if app.exist?
caskroom_path.rmtree if caskroom_path.exist?
end

it "can uninstall non-ruby-backed Casks" do
it "can still uninstall those Casks" do
shutup do
Hbc::CLI::Uninstall.run("ive-been-renamed")
end

@renamed_path.wont_be :exist?
app.wont_be :exist?
caskroom_path.wont_be :exist?
end
end

Expand Down
10 changes: 10 additions & 0 deletions test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,16 @@ def self.install_with_caskfile(cask)
end
end
end

def self.install_without_artifacts_with_caskfile(cask)
Hbc::Installer.new(cask).tap do |i|
shutup do
i.download
i.extract_primary_container
i.save_caskfile
end
end
end
end

# Extend MiniTest API with support for RSpec-style shared examples
Expand Down