From 2a21991b1fc8039bfa0c950e439db5a725b62b42 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Thu, 16 Feb 2017 17:10:40 +0100 Subject: [PATCH 1/2] Make sure `uninstall` is called before artifacts are removed. --- Library/Homebrew/cask/lib/hbc/installer.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Library/Homebrew/cask/lib/hbc/installer.rb b/Library/Homebrew/cask/lib/hbc/installer.rb index cafc9d8b9e..b86f3264d2 100644 --- a/Library/Homebrew/cask/lib/hbc/installer.rb +++ b/Library/Homebrew/cask/lib/hbc/installer.rb @@ -318,7 +318,13 @@ module Hbc def uninstall_artifacts odebug "Un-installing artifacts" artifacts = Artifact.for_cask(@cask, command: @command, force: force) + + # Make sure the `uninstall` stanza is run first, as it + # may depend on other artifacts still being installed. + artifacts = artifacts.sort_by { |a| a.is_a?(Artifact::Uninstall) ? -1 : 1 } + odebug "#{artifacts.length} artifact/s defined", artifacts + artifacts.each do |artifact| next unless artifact.respond_to?(:uninstall_phase) odebug "Un-installing artifact of class #{artifact.class}" From ecb17f4f1d265e1625828565ecbbdac6d5f989c6 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Thu, 16 Feb 2017 17:46:23 +0100 Subject: [PATCH 2/2] Add test for `uninstall` before removing artifacts. --- .../cask/spec/cask/cli/uninstall_spec.rb | 19 +++++++++++++++++ .../cask/Casks/with-uninstall-script-app.rb | 21 +++++++++++++++++++ 2 files changed, 40 insertions(+) create mode 100644 Library/Homebrew/test/support/fixtures/cask/Casks/with-uninstall-script-app.rb diff --git a/Library/Homebrew/cask/spec/cask/cli/uninstall_spec.rb b/Library/Homebrew/cask/spec/cask/cli/uninstall_spec.rb index be0aa31b6f..243be49d97 100644 --- a/Library/Homebrew/cask/spec/cask/cli/uninstall_spec.rb +++ b/Library/Homebrew/cask/spec/cask/cli/uninstall_spec.rb @@ -41,6 +41,25 @@ describe Hbc::CLI::Uninstall do expect(Hbc.appdir.join("Caffeine.app")).not_to exist end + it "calls `uninstall` before removing artifacts" do + cask = Hbc::CaskLoader.load_from_file(TEST_FIXTURE_DIR/"cask/Casks/with-uninstall-script-app.rb") + + shutup do + Hbc::Installer.new(cask).install + end + + expect(cask).to be_installed + + expect { + shutup do + Hbc::CLI::Uninstall.run("with-uninstall-script-app") + end + }.not_to raise_error + + expect(cask).not_to be_installed + expect(Hbc.appdir.join("MyFancyApp.app")).not_to exist + end + describe "when multiple versions of a cask are installed" do let(:token) { "versioned-cask" } let(:first_installed_version) { "1.2.3" } diff --git a/Library/Homebrew/test/support/fixtures/cask/Casks/with-uninstall-script-app.rb b/Library/Homebrew/test/support/fixtures/cask/Casks/with-uninstall-script-app.rb new file mode 100644 index 0000000000..f5f3ae5dde --- /dev/null +++ b/Library/Homebrew/test/support/fixtures/cask/Casks/with-uninstall-script-app.rb @@ -0,0 +1,21 @@ +cask 'with-uninstall-script-app' do + version '1.2.3' + sha256 '5633c3a0f2e572cbf021507dec78c50998b398c343232bdfc7e26221d0a5db4d' + + url "file://#{TEST_FIXTURE_DIR}/cask/MyFancyApp.zip" + homepage 'http://example.com/MyFancyApp' + + app 'MyFancyApp/MyFancyApp.app' + + postflight do + IO.write "#{appdir}/MyFancyApp.app/uninstall.sh", <<-EOS.undent + #!/bin/sh + /bin/rm -r "#{appdir}/MyFancyApp.app" + EOS + end + + uninstall script: { + executable: "#{appdir}/MyFancyApp.app/uninstall.sh", + sudo: false + } +end