diff --git a/Library/Homebrew/cask/lib/hbc/cli/upgrade.rb b/Library/Homebrew/cask/lib/hbc/cli/upgrade.rb index 90da6f325c..94fe56a47f 100644 --- a/Library/Homebrew/cask/lib/hbc/cli/upgrade.rb +++ b/Library/Homebrew/cask/lib/hbc/cli/upgrade.rb @@ -41,6 +41,9 @@ module Hbc require_sha: require_sha?, upgrade: true) + started_upgrade = false + new_artifacts_installed = false + begin # Start new Cask's installation steps new_cask_installer.check_conflicts @@ -51,9 +54,12 @@ module Hbc # Move the old Cask's artifacts back to staging old_cask_installer.start_upgrade + # And flag it so in case of error + started_upgrade = true # Install the new Cask new_cask_installer.install_artifacts + new_artifacts_installed = true new_cask_installer.enable_accessibility_access @@ -61,8 +67,9 @@ module Hbc old_cask_installer.finalize_upgrade rescue CaskError => e opoo e.message - new_cask_installer.uninstall - old_cask_installer.revert_upgrade + new_cask_installer.uninstall_artifacts if new_artifacts_installed + new_cask_installer.purge_versioned_files + old_cask_installer.revert_upgrade if started_upgrade end end end diff --git a/Library/Homebrew/test/cask/cli/upgrade_spec.rb b/Library/Homebrew/test/cask/cli/upgrade_spec.rb index 38e57fc5e0..ee7ef22614 100644 --- a/Library/Homebrew/test/cask/cli/upgrade_spec.rb +++ b/Library/Homebrew/test/cask/cli/upgrade_spec.rb @@ -3,21 +3,39 @@ require_relative "shared_examples/invalid_option" describe Hbc::CLI::Upgrade, :cask do it_behaves_like "a command that handles invalid options" - before(:example) do - installed = - [ - "outdated/local-caffeine", - "outdated/local-transmission", - "outdated/auto-updates", - ] + shared_context "Proper Casks" do + before(:example) do + installed = + [ + "outdated/local-caffeine", + "outdated/local-transmission", + "outdated/auto-updates", + ] - installed.each { |cask| Hbc::CLI::Install.run(cask) } + installed.each { |cask| Hbc::CLI::Install.run(cask) } - allow_any_instance_of(described_class).to receive(:verbose?).and_return(true) + allow_any_instance_of(described_class).to receive(:verbose?).and_return(true) + end + end + + shared_context "Casks that will fail upon upgrade" do + before(:example) do + installed = + [ + "outdated/bad-checksum", + "outdated/will-fail-if-upgraded", + ] + + installed.each { |cask| Hbc::CLI::Install.run(cask) } + + allow_any_instance_of(described_class).to receive(:verbose?).and_return(true) + end end describe 'without --greedy it ignores the Casks with "version latest" or "auto_updates true"' do - it "updates all the installed Casks when no token is provided" do + include_context "Proper Casks" + + it "and updates all the installed Casks when no token is provided" do expect(Hbc::CaskLoader.load("local-caffeine")).to be_installed expect(Hbc.appdir.join("Caffeine.app")).to be_a_directory expect(Hbc::CaskLoader.load("local-caffeine").versions).to include("1.2.2") @@ -37,7 +55,7 @@ describe Hbc::CLI::Upgrade, :cask do expect(Hbc::CaskLoader.load("local-transmission").versions).to include("2.61") end - it "updates only the Casks specified in the command line" do + it "and updates only the Casks specified in the command line" do expect(Hbc::CaskLoader.load("local-caffeine")).to be_installed expect(Hbc.appdir.join("Caffeine.app")).to be_a_directory expect(Hbc::CaskLoader.load("local-caffeine").versions).to include("1.2.2") @@ -58,7 +76,7 @@ describe Hbc::CLI::Upgrade, :cask do expect(Hbc::CaskLoader.load("local-transmission").versions).to include("2.60") end - it 'ignores "auto_updates" and "latest" Casks even when their tokens are provided in the command line' do + it 'and ignores "auto_updates" and "latest" Casks even when their tokens are provided in the command line' do expect(Hbc::CaskLoader.load("local-caffeine")).to be_installed expect(Hbc.appdir.join("Caffeine.app")).to be_a_directory expect(Hbc::CaskLoader.load("local-caffeine").versions).to include("1.2.2") @@ -80,6 +98,8 @@ describe Hbc::CLI::Upgrade, :cask do end describe "with --greedy it checks additional Casks" do + include_context "Proper Casks" + it 'includes the Casks with "auto_updates true" or "version latest" with --greedy' do expect(Hbc::CaskLoader.load("auto-updates")).to be_installed expect(Hbc.appdir.join("MyFancyApp.app")).to be_a_directory @@ -124,4 +144,42 @@ describe Hbc::CLI::Upgrade, :cask do expect(Hbc::CaskLoader.load("auto-updates").versions).to include("2.61") end end + + describe "handles borked upgrades" do + include_context "Casks that will fail upon upgrade" + + output_reverted = Regexp.new <<~EOS + Warning: Reverting upgrade for Cask .* + EOS + + it "by restoring the old Cask if the upgrade's install failed" do + expect(Hbc::CaskLoader.load("will-fail-if-upgraded")).to be_installed + expect(Hbc.appdir.join("container")).to be_a_file + expect(Hbc::CaskLoader.load("will-fail-if-upgraded").versions).to include("1.2.2") + + expect { + described_class.run("will-fail-if-upgraded") + }.to output(output_reverted).to_stderr + + expect(Hbc::CaskLoader.load("will-fail-if-upgraded")).to be_installed + expect(Hbc.appdir.join("container")).to be_a_file + expect(Hbc::CaskLoader.load("will-fail-if-upgraded").versions).to include("1.2.2") + expect(Hbc::CaskLoader.load("will-fail-if-upgraded").staged_path).to_not exist + end + + it "by not restoring the old Cask if the upgrade failed pre-install" do + expect(Hbc::CaskLoader.load("bad-checksum")).to be_installed + expect(Hbc.appdir.join("Caffeine.app")).to be_a_directory + expect(Hbc::CaskLoader.load("bad-checksum").versions).to include("1.2.2") + + expect { + described_class.run("bad-checksum") + }.to_not output(output_reverted).to_stderr + + expect(Hbc::CaskLoader.load("bad-checksum")).to be_installed + expect(Hbc.appdir.join("Caffeine.app")).to be_a_directory + expect(Hbc::CaskLoader.load("bad-checksum").versions).to include("1.2.2") + expect(Hbc::CaskLoader.load("bad-checksum").staged_path).to_not exist + end + end end diff --git a/Library/Homebrew/test/support/fixtures/cask/Casks/outdated/bad-checksum.rb b/Library/Homebrew/test/support/fixtures/cask/Casks/outdated/bad-checksum.rb new file mode 100644 index 0000000000..5e7e744830 --- /dev/null +++ b/Library/Homebrew/test/support/fixtures/cask/Casks/outdated/bad-checksum.rb @@ -0,0 +1,9 @@ +cask 'bad-checksum' do + version '1.2.2' + sha256 '67cdb8a02803ef37fdbf7e0be205863172e41a561ca446cd84f0d7ab35a99d94' + + url "file://#{TEST_FIXTURE_DIR}/cask/caffeine.zip" + homepage 'http://example.com/local-caffeine' + + app 'Caffeine.app' +end diff --git a/Library/Homebrew/test/support/fixtures/cask/Casks/outdated/will-fail-if-upgraded.rb b/Library/Homebrew/test/support/fixtures/cask/Casks/outdated/will-fail-if-upgraded.rb new file mode 100644 index 0000000000..7735ffa848 --- /dev/null +++ b/Library/Homebrew/test/support/fixtures/cask/Casks/outdated/will-fail-if-upgraded.rb @@ -0,0 +1,9 @@ +cask 'will-fail-if-upgraded' do + version '1.2.2' + sha256 'fab685fabf73d5a9382581ce8698fce9408f5feaa49fa10d9bc6c510493300f5' + + url "file://#{TEST_FIXTURE_DIR}/cask/container.tar.gz" + homepage 'https://example.com/container-tar-gz' + + app 'container' +end diff --git a/Library/Homebrew/test/support/fixtures/cask/Casks/will-fail-if-upgraded.rb b/Library/Homebrew/test/support/fixtures/cask/Casks/will-fail-if-upgraded.rb new file mode 100644 index 0000000000..99ed4b87cf --- /dev/null +++ b/Library/Homebrew/test/support/fixtures/cask/Casks/will-fail-if-upgraded.rb @@ -0,0 +1,9 @@ +cask 'will-fail-if-upgraded' do + version '1.2.3' + sha256 'e44ffa103fbf83f55c8d0b1bea309a43b2880798dae8620b1ee8da5e1095ec68' + + url "file://#{TEST_FIXTURE_DIR}/cask/transmission-2.61.dmg" + homepage 'http://example.com/local-transmission' + + app 'container' +end