Skip to content

Commit

Permalink
Run uninstall rmdir: after uninstalling artifacts.
Browse files Browse the repository at this point in the history
  • Loading branch information
reitermarkus committed Feb 8, 2019
1 parent 7f312ed commit 0fcc62d
Show file tree
Hide file tree
Showing 6 changed files with 95 additions and 43 deletions.
27 changes: 12 additions & 15 deletions Library/Homebrew/cask/artifact/abstract_uninstall.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

require "utils/user"
require "cask/artifact/abstract_artifact"
require "extend/hash_validator"
using HashValidator

module Cask
module Artifact
Expand All @@ -27,6 +29,8 @@ def self.from_args(cask, **directives)
attr_reader :directives

def initialize(cask, directives)
directives.assert_valid_keys!(*ORDERED_DIRECTIVES)

super(cask)
directives[:signal] = [*directives[:signal]].flatten.each_slice(2).to_a
@directives = directives
Expand All @@ -49,30 +53,23 @@ def summarize
private

def dispatch_uninstall_directives(**options)
ohai "Running #{stanza} process for #{@cask}; your password may be necessary"
ORDERED_DIRECTIVES.each do |directive_sym|
dispatch_uninstall_directive(directive_sym, **options)
end
end

warn_for_unknown_directives(directives)
def dispatch_uninstall_directive(directive_sym, **options)
return unless directives.key?(directive_sym)

ORDERED_DIRECTIVES.each do |directive_sym|
next unless directives.key?(directive_sym)
args = directives[directive_sym]

args = directives[directive_sym]
send("uninstall_#{directive_sym}", *(args.is_a?(Hash) ? [args] : args), **options)
end
send("uninstall_#{directive_sym}", *(args.is_a?(Hash) ? [args] : args), **options)
end

def stanza
self.class.dsl_key
end

def warn_for_unknown_directives(directives)
unknown_keys = directives.keys - ORDERED_DIRECTIVES
return if unknown_keys.empty?

opoo "Unknown arguments to #{stanza} -- #{unknown_keys.inspect}. " \
"Running \"brew update; brew cleanup\" will likely fix it."
end

# Preserve prior functionality of script which runs first. Should rarely be needed.
# :early_script should not delete files, better defer that to :script.
# If Cask writers never need :early_script it may be removed in the future.
Expand Down
9 changes: 8 additions & 1 deletion Library/Homebrew/cask/artifact/uninstall.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,14 @@ module Cask
module Artifact
class Uninstall < AbstractUninstall
def uninstall_phase(**options)
dispatch_uninstall_directives(**options)
ORDERED_DIRECTIVES.reject { |directive_sym| directive_sym == :rmdir }
.each do |directive_sym|
dispatch_uninstall_directive(directive_sym, **options)
end
end

def post_uninstall_phase(**options)
dispatch_uninstall_directive(:rmdir, **options)
end
end
end
Expand Down
18 changes: 17 additions & 1 deletion Library/Homebrew/cask/installer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,13 @@ def install_artifacts
odebug "Reverting installation of artifact of class #{artifact.class}"
artifact.uninstall_phase(command: @command, verbose: verbose?, force: force?)
end

already_installed_artifacts.each do |artifact|
next unless artifact.respond_to?(:post_uninstall_phase)

odebug "Reverting installation of artifact of class #{artifact.class}"
artifact.post_uninstall_phase(command: @command, verbose: verbose?, force: force?)
end
ensure
purge_versioned_files
raise e
Expand Down Expand Up @@ -425,9 +432,18 @@ def uninstall_artifacts(clear: false)
artifacts.each do |artifact|
next unless artifact.respond_to?(:uninstall_phase)

odebug "Un-installing artifact of class #{artifact.class}"
odebug "Uninstalling artifact of class #{artifact.class}"
artifact.uninstall_phase(command: @command, verbose: verbose?, skip: clear, force: force?, upgrade: upgrade?)
end

artifacts.each do |artifact|
next unless artifact.respond_to?(:post_uninstall_phase)

odebug "Post-uninstalling artifact of class #{artifact.class}"
artifact.post_uninstall_phase(
command: @command, verbose: verbose?, skip: clear, force: force?, upgrade: upgrade?,
)
end
end

def zap
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -218,32 +218,6 @@
end
end

context "using :rmdir" do
let(:fake_system_command) { NeverSudoSystemCommand }
let(:cask) { Cask::CaskLoader.load(cask_path("with-#{artifact_dsl_key}-rmdir")) }
let(:empty_directory) { Pathname.new("#{TEST_TMPDIR}/empty_directory_path") }
let(:ds_store) { empty_directory.join(".DS_Store") }

before do
empty_directory.mkdir
FileUtils.touch ds_store
end

after do
FileUtils.rm_rf empty_directory
end

it "is supported" do
expect(empty_directory).to exist
expect(ds_store).to exist

subject.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command)

expect(ds_store).not_to exist
expect(empty_directory).not_to exist
end
end

[:script, :early_script].each do |script_type|
context "using #{script_type.inspect}" do
let(:fake_system_command) { NeverSudoSystemCommand }
Expand Down
30 changes: 30 additions & 0 deletions Library/Homebrew/test/cask/artifact/uninstall_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,34 @@
describe "#uninstall_phase" do
include_examples "#uninstall_phase or #zap_phase"
end

describe "#post_uninstall_phase" do
subject(:artifact) { cask.artifacts.find { |a| a.is_a?(described_class) } }

context "using :rmdir" do
let(:fake_system_command) { NeverSudoSystemCommand }
let(:cask) { Cask::CaskLoader.load(cask_path("with-uninstall-rmdir")) }
let(:empty_directory) { Pathname.new("#{TEST_TMPDIR}/empty_directory_path") }
let(:ds_store) { empty_directory.join(".DS_Store") }

before do
empty_directory.mkdir
FileUtils.touch ds_store
end

after do
FileUtils.rm_rf empty_directory
end

it "is supported" do
expect(empty_directory).to exist
expect(ds_store).to exist

artifact.post_uninstall_phase(command: fake_system_command)

expect(ds_store).not_to exist
expect(empty_directory).not_to exist
end
end
end
end
28 changes: 28 additions & 0 deletions Library/Homebrew/test/cask/artifact/zap_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,33 @@
describe Cask::Artifact::Zap, :cask do
describe "#zap_phase" do
include_examples "#uninstall_phase or #zap_phase"

context "using :rmdir" do
subject(:artifact) { cask.artifacts.find { |a| a.is_a?(described_class) } }

let(:fake_system_command) { NeverSudoSystemCommand }
let(:cask) { Cask::CaskLoader.load(cask_path("with-zap-rmdir")) }
let(:empty_directory) { Pathname.new("#{TEST_TMPDIR}/empty_directory_path") }
let(:ds_store) { empty_directory.join(".DS_Store") }

before do
empty_directory.mkdir
FileUtils.touch ds_store
end

after do
FileUtils.rm_rf empty_directory
end

it "is supported" do
expect(empty_directory).to exist
expect(ds_store).to exist

artifact.zap_phase(command: fake_system_command)

expect(ds_store).not_to exist
expect(empty_directory).not_to exist
end
end
end
end

0 comments on commit 0fcc62d

Please sign in to comment.