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

Run uninstall rmdir: after uninstalling artifacts. #5685

Merged
merged 1 commit into from
Feb 8, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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