From 55727b789532fbfa7997929aa0506d7843eda3ce Mon Sep 17 00:00:00 2001 From: "L. E. Segovia" Date: Sun, 29 Oct 2017 17:31:07 -0300 Subject: [PATCH 01/15] Hack a first working version of upgrade --- Library/Homebrew/cask/lib/hbc/cli.rb | 1 + Library/Homebrew/cask/lib/hbc/cli/upgrade.rb | 65 ++++++++++++++++ Library/Homebrew/cask/lib/hbc/installer.rb | 16 +++- .../Homebrew/test/cask/cli/upgrade_spec.rb | 78 +++++++++++++++++++ 4 files changed, 157 insertions(+), 3 deletions(-) create mode 100644 Library/Homebrew/cask/lib/hbc/cli/upgrade.rb create mode 100644 Library/Homebrew/test/cask/cli/upgrade_spec.rb diff --git a/Library/Homebrew/cask/lib/hbc/cli.rb b/Library/Homebrew/cask/lib/hbc/cli.rb index e147c82809c61..215b59843dc73 100644 --- a/Library/Homebrew/cask/lib/hbc/cli.rb +++ b/Library/Homebrew/cask/lib/hbc/cli.rb @@ -21,6 +21,7 @@ require "hbc/cli/search" require "hbc/cli/style" require "hbc/cli/uninstall" +require "hbc/cli/upgrade" require "hbc/cli/--version" require "hbc/cli/zap" diff --git a/Library/Homebrew/cask/lib/hbc/cli/upgrade.rb b/Library/Homebrew/cask/lib/hbc/cli/upgrade.rb new file mode 100644 index 0000000000000..49a538704b7e9 --- /dev/null +++ b/Library/Homebrew/cask/lib/hbc/cli/upgrade.rb @@ -0,0 +1,65 @@ +module Hbc + class CLI + class Upgrade < AbstractCommand + option "--greedy", :greedy, false + option "--quiet", :quiet, false + option "--force", :force, false + option "--force-update", :force_update, false + option "--skip-cask-deps", :skip_cask_deps, false + + def initialize(*) + super + self.verbose = ($stdout.tty? || verbose?) && !quiet? + end + + def run + outdated_casks = casks(alternative: -> { Hbc.installed }).find_all { |cask| cask.outdated?(greedy?) } + + if outdated_casks.empty? + oh1 "No packages to upgrade" + else + oh1 "Upgrading #{Formatter.pluralize(outdated_casks.length, "outdated package")}, with result:" + puts outdated_casks.map { |f| "#{f.full_name} #{f.version}" } * ", " + end + + outdated_casks.each do |old_cask| + odebug "Uninstalling Cask #{old_cask}" + + raise CaskNotInstalledError, old_cask unless old_cask.installed? || force? + + unless old_cask.installed_caskfile.nil? + # use the same cask file that was used for installation, if possible + old_cask = CaskLoader.load(old_cask.installed_caskfile) if old_cask.installed_caskfile.exist? + end + + old_cask_installer = Installer.new(old_cask, binaries: binaries?, verbose: verbose?, force: force?, upgrade: true) + + old_cask_installer.uninstall + + begin + odebug "Installing new version of Cask #{old_cask}" + + new_cask = CaskLoader.load(old_cask.to_s) + + Installer.new(new_cask, binaries: binaries?, + verbose: verbose?, + force: force?, + skip_cask_deps: skip_cask_deps?, + require_sha: require_sha?, + upgrade: true).install + + old_cask_installer.finalize_upgrade + rescue CaskUnavailableError => e + opoo e.message + rescue CaskAlreadyInstalledError => e + opoo e.message + end + end + end + + def self.help + "upgrades all outdated casks" + end + end + end +end diff --git a/Library/Homebrew/cask/lib/hbc/installer.rb b/Library/Homebrew/cask/lib/hbc/installer.rb index 1063f488bcf28..629a20f3107ad 100644 --- a/Library/Homebrew/cask/lib/hbc/installer.rb +++ b/Library/Homebrew/cask/lib/hbc/installer.rb @@ -19,7 +19,7 @@ class Installer PERSISTENT_METADATA_SUBDIRS = ["gpg"].freeze - def initialize(cask, command: SystemCommand, force: false, skip_cask_deps: false, binaries: true, verbose: false, require_sha: false) + def initialize(cask, command: SystemCommand, force: false, skip_cask_deps: false, binaries: true, verbose: false, require_sha: false, upgrade: false) @cask = cask @command = command @force = force @@ -28,6 +28,7 @@ def initialize(cask, command: SystemCommand, force: false, skip_cask_deps: false @verbose = verbose @require_sha = require_sha @reinstall = false + @upgrade = upgrade end attr_predicate :binaries?, :force?, :skip_cask_deps?, :require_sha?, :verbose? @@ -82,7 +83,7 @@ def stage def install odebug "Hbc::Installer#install" - if @cask.installed? && !force? && !@reinstall + if @cask.installed? && !force? && !@reinstall && !@upgrade raise CaskAlreadyInstalledError, @cask end @@ -129,7 +130,7 @@ def uninstall_existing_cask installed_cask = installed_caskfile.exist? ? CaskLoader.load(installed_caskfile) : @cask # Always force uninstallation, ignore method parameter - Installer.new(installed_cask, binaries: binaries?, verbose: verbose?, force: true).uninstall + Installer.new(installed_cask, binaries: binaries?, verbose: verbose?, force: true, upgrade: @upgrade).uninstall end def summary @@ -368,6 +369,15 @@ def uninstall oh1 "Uninstalling Cask #{@cask}" disable_accessibility_access uninstall_artifacts + return if @upgrade + + purge_versioned_files + purge_caskroom_path if force? + end + + def finalize_upgrade + return unless @upgrade + purge_versioned_files purge_caskroom_path if force? end diff --git a/Library/Homebrew/test/cask/cli/upgrade_spec.rb b/Library/Homebrew/test/cask/cli/upgrade_spec.rb new file mode 100644 index 0000000000000..532541ba7a492 --- /dev/null +++ b/Library/Homebrew/test/cask/cli/upgrade_spec.rb @@ -0,0 +1,78 @@ +require_relative "shared_examples/invalid_option" + +describe Hbc::CLI::Upgrade, :cask do + let(:installed) do + [ + Hbc::CaskLoader.load(cask_path("outdated/local-caffeine")), + Hbc::CaskLoader.load(cask_path("outdated/local-transmission")), + Hbc::CaskLoader.load(cask_path("outdated/auto-updates")), + ] + end + + it_behaves_like "a command that handles invalid options" + + before(:example) do + installed.each { |cask| InstallHelper.install_with_caskfile(cask) } + + allow_any_instance_of(described_class).to receive(:verbose?).and_return(true) + 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 + described_class.run + + 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.3") + + expect(Hbc::CaskLoader.load("local-transmission")).to be_installed + expect(Hbc.appdir.join("Transmission.app")).to be_a_directory + expect(Hbc::CaskLoader.load("local-transmission").versions).to include("2.61") + end + + it "updates only the Casks specified in the command line" do + expect(Hbc::CaskLoader.load("local-caffeine").versions).to include("1.2.2") + expect(Hbc::CaskLoader.load("local-transmission").versions).to include("2.60") + + described_class.run("local-caffeine") + + expect(Hbc::CaskLoader.load("local-caffeine").versions).to include("1.2.3") + expect(Hbc::CaskLoader.load("local-caffeine").versions).to_not include("1.2.2") + 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 + expect(Hbc::CaskLoader.load("local-caffeine").versions).to include("1.2.2") + expect(Hbc::CaskLoader.load("auto-updates").versions).to include("2.57") + + described_class.run("local-caffeine", "auto-updates", "version-latest-string") + + expect(Hbc::CaskLoader.load("local-caffeine").versions).to include("1.2.3") + expect(Hbc::CaskLoader.load("auto-updates").versions).to include("2.57") + end + end + + describe "with --greedy it checks additional Casks" do + it 'includes the Casks with "auto_updates true" or "version latest" with --greedy' do + expect(Hbc::CaskLoader.load("auto-updates").versions).to include("2.57") + expect(Hbc::CaskLoader.load("local-caffeine").versions).to include("1.2.2") + expect(Hbc::CaskLoader.load("local-transmission").versions).to include("2.60") + + described_class.run("--greedy") + + expect(Hbc::CaskLoader.load("auto-updates").versions).to include("2.61") + expect(Hbc::CaskLoader.load("local-caffeine").versions).to include("1.2.3") + expect(Hbc::CaskLoader.load("local-transmission").versions).to include("2.61") + end + + it 'does not include the Casks with "auto_updates true" when the version did not change' do + cask = Hbc::CaskLoader.load(cask_path("auto-updates")) + InstallHelper.install_with_caskfile(cask) + expect(Hbc::CaskLoader.load("auto-updates").versions).to include("2.61") + + described_class.run("auto-updates", "--greedy") + + expect(Hbc::CaskLoader.load("auto-updates").versions).to include("2.61") + end + end +end From 7f2e4f583a5789a86712322818612c439307b90d Mon Sep 17 00:00:00 2001 From: "L. E. Segovia" Date: Mon, 30 Oct 2017 23:29:00 -0300 Subject: [PATCH 02/15] Finalize metadata handling and uninstall logic --- Library/Homebrew/cask/lib/hbc/cli/upgrade.rb | 24 ++++++++------- Library/Homebrew/cask/lib/hbc/installer.rb | 31 +++++++++++++------- 2 files changed, 35 insertions(+), 20 deletions(-) diff --git a/Library/Homebrew/cask/lib/hbc/cli/upgrade.rb b/Library/Homebrew/cask/lib/hbc/cli/upgrade.rb index 49a538704b7e9..e68fc4e53dfd2 100644 --- a/Library/Homebrew/cask/lib/hbc/cli/upgrade.rb +++ b/Library/Homebrew/cask/lib/hbc/cli/upgrade.rb @@ -34,25 +34,29 @@ def run old_cask_installer = Installer.new(old_cask, binaries: binaries?, verbose: verbose?, force: force?, upgrade: true) - old_cask_installer.uninstall - - begin - odebug "Installing new version of Cask #{old_cask}" - - new_cask = CaskLoader.load(old_cask.to_s) + new_cask = CaskLoader.load(old_cask.to_s) + new_cask_installer = Installer.new(new_cask, binaries: binaries?, verbose: verbose?, force: force?, skip_cask_deps: skip_cask_deps?, require_sha: require_sha?, - upgrade: true).install + upgrade: true) + + begin + # purge artifacts BUT keep metadata aside + old_cask_installer.start_upgrade + + # install BUT do not yet save metadata + new_cask_installer.install + + # if successful, remove old metadata and install new old_cask_installer.finalize_upgrade - rescue CaskUnavailableError => e - opoo e.message - rescue CaskAlreadyInstalledError => e + rescue CaskError => e opoo e.message + old_cask_installer.revert_upgrade end end end diff --git a/Library/Homebrew/cask/lib/hbc/installer.rb b/Library/Homebrew/cask/lib/hbc/installer.rb index 629a20f3107ad..80dd2ca4cc0e8 100644 --- a/Library/Homebrew/cask/lib/hbc/installer.rb +++ b/Library/Homebrew/cask/lib/hbc/installer.rb @@ -83,8 +83,8 @@ def stage def install odebug "Hbc::Installer#install" - if @cask.installed? && !force? && !@reinstall && !@upgrade - raise CaskAlreadyInstalledError, @cask + if @cask.installed? && !force? && !@reinstall + raise CaskAlreadyInstalledError, @cask unless @upgrade end check_conflicts @@ -369,17 +369,28 @@ def uninstall oh1 "Uninstalling Cask #{@cask}" disable_accessibility_access uninstall_artifacts - return if @upgrade - purge_versioned_files purge_caskroom_path if force? end - def finalize_upgrade + def start_upgrade return unless @upgrade + oh1 "Starting upgrade for Cask #{@cask}" - purge_versioned_files - purge_caskroom_path if force? + disable_accessibility_access + uninstall_artifacts + end + + def revert_upgrade + return unless @upgrade + opoo "Reverting upgrade for Cask #{@cask}" + reinstall + end + + def finalize_upgrade + return unless @upgrade + purge_versioned_files(upgrade: true) + oh1 "Cask #{@cask} was successfully upgraded!" end def uninstall_artifacts @@ -414,7 +425,7 @@ def gain_permissions_remove(path) Utils.gain_permissions_remove(path, command: @command) end - def purge_versioned_files + def purge_versioned_files(upgrade: false) odebug "Purging files for version #{@cask.version} of Cask #{@cask}" # versioned staged distribution @@ -430,10 +441,10 @@ def purge_versioned_files end end @cask.metadata_versioned_path.rmdir_if_possible - @cask.metadata_master_container_path.rmdir_if_possible + @cask.metadata_master_container_path.rmdir_if_possible unless upgrade # toplevel staged distribution - @cask.caskroom_path.rmdir_if_possible + @cask.caskroom_path.rmdir_if_possible unless upgrade end def purge_caskroom_path From 84c128411f192b503bee9c7f7595ba1c43b25529 Mon Sep 17 00:00:00 2001 From: "L. E. Segovia" Date: Mon, 6 Nov 2017 18:33:29 -0300 Subject: [PATCH 03/15] Fix style issues --- Library/Homebrew/cask/lib/hbc/cli/upgrade.rb | 12 +++++------- Library/Homebrew/cask/lib/hbc/installer.rb | 12 ++++++------ 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/Library/Homebrew/cask/lib/hbc/cli/upgrade.rb b/Library/Homebrew/cask/lib/hbc/cli/upgrade.rb index e68fc4e53dfd2..1165ce6fc38fb 100644 --- a/Library/Homebrew/cask/lib/hbc/cli/upgrade.rb +++ b/Library/Homebrew/cask/lib/hbc/cli/upgrade.rb @@ -13,14 +13,12 @@ def initialize(*) end def run - outdated_casks = casks(alternative: -> { Hbc.installed }).find_all { |cask| cask.outdated?(greedy?) } + outdated_casks = casks(alternative: -> { Hbc.installed }).select { |cask| cask.outdated?(greedy?) } - if outdated_casks.empty? - oh1 "No packages to upgrade" - else - oh1 "Upgrading #{Formatter.pluralize(outdated_casks.length, "outdated package")}, with result:" - puts outdated_casks.map { |f| "#{f.full_name} #{f.version}" } * ", " - end + return if outdated_casks.empty? + + oh1 "Upgrading #{Formatter.pluralize(outdated_casks.length, "outdated package")}, with result:" + puts outdated_casks.map { |f| "#{f.full_name} #{f.version}" } * ", " outdated_casks.each do |old_cask| odebug "Uninstalling Cask #{old_cask}" diff --git a/Library/Homebrew/cask/lib/hbc/installer.rb b/Library/Homebrew/cask/lib/hbc/installer.rb index 80dd2ca4cc0e8..e2f7714aeaaef 100644 --- a/Library/Homebrew/cask/lib/hbc/installer.rb +++ b/Library/Homebrew/cask/lib/hbc/installer.rb @@ -31,7 +31,7 @@ def initialize(cask, command: SystemCommand, force: false, skip_cask_deps: false @upgrade = upgrade end - attr_predicate :binaries?, :force?, :skip_cask_deps?, :require_sha?, :verbose? + attr_predicate :binaries?, :force?, :skip_cask_deps?, :require_sha?, :upgrade?, :verbose? def self.print_caveats(cask) odebug "Printing caveats" @@ -84,7 +84,7 @@ def install odebug "Hbc::Installer#install" if @cask.installed? && !force? && !@reinstall - raise CaskAlreadyInstalledError, @cask unless @upgrade + raise CaskAlreadyInstalledError, @cask unless upgrade? end check_conflicts @@ -130,7 +130,7 @@ def uninstall_existing_cask installed_cask = installed_caskfile.exist? ? CaskLoader.load(installed_caskfile) : @cask # Always force uninstallation, ignore method parameter - Installer.new(installed_cask, binaries: binaries?, verbose: verbose?, force: true, upgrade: @upgrade).uninstall + Installer.new(installed_cask, binaries: binaries?, verbose: verbose?, force: true, upgrade: upgrade?).uninstall end def summary @@ -374,7 +374,7 @@ def uninstall end def start_upgrade - return unless @upgrade + return unless upgrade? oh1 "Starting upgrade for Cask #{@cask}" disable_accessibility_access @@ -382,13 +382,13 @@ def start_upgrade end def revert_upgrade - return unless @upgrade + return unless upgrade? opoo "Reverting upgrade for Cask #{@cask}" reinstall end def finalize_upgrade - return unless @upgrade + return unless upgrade? purge_versioned_files(upgrade: true) oh1 "Cask #{@cask} was successfully upgraded!" end From 94d266e2d6c2e7c8c14dc9c6f117871e33be3133 Mon Sep 17 00:00:00 2001 From: "L. E. Segovia" Date: Mon, 6 Nov 2017 21:27:02 -0300 Subject: [PATCH 04/15] Rework uninstallation step Now the artifacts get re-staged, and upon an uninstall/finalize_upgrade they are deleted by purge_versioned_files instead --- Library/Homebrew/cask/lib/hbc/artifact/moved.rb | 10 +++++----- Library/Homebrew/cask/lib/hbc/installer.rb | 8 ++++---- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/Library/Homebrew/cask/lib/hbc/artifact/moved.rb b/Library/Homebrew/cask/lib/hbc/artifact/moved.rb index ba1c8e9072003..a2f890e4cea83 100644 --- a/Library/Homebrew/cask/lib/hbc/artifact/moved.rb +++ b/Library/Homebrew/cask/lib/hbc/artifact/moved.rb @@ -8,11 +8,11 @@ def self.english_description end def install_phase(**options) - move(**options) + move(source, target, **options) end def uninstall_phase(**options) - delete(**options) + move(target, source, **options) end def summarize_installed @@ -25,12 +25,12 @@ def summarize_installed private - def move(force: false, command: nil, **options) + def move(source, target, force: false, command: nil, **options) if Utils.path_occupied?(target) message = "It seems there is already #{self.class.english_article} #{self.class.english_name} at '#{target}'" raise CaskError, "#{message}." unless force opoo "#{message}; overwriting." - delete(force: force, command: command, **options) + delete(target, force: force, command: command, **options) end unless source.exist? @@ -49,7 +49,7 @@ def move(force: false, command: nil, **options) add_altname_metadata(target, source.basename, command: command) end - def delete(force: false, command: nil, **_) + def delete(target, force: false, command: nil, **_) ohai "Removing #{self.class.english_name} '#{target}'." raise CaskError, "Cannot remove undeletable #{self.class.english_name}." if MacOS.undeletable?(target) diff --git a/Library/Homebrew/cask/lib/hbc/installer.rb b/Library/Homebrew/cask/lib/hbc/installer.rb index e2f7714aeaaef..cf71639efa1c3 100644 --- a/Library/Homebrew/cask/lib/hbc/installer.rb +++ b/Library/Homebrew/cask/lib/hbc/installer.rb @@ -136,7 +136,7 @@ def uninstall_existing_cask def summary s = "" s << "#{Emoji.install_badge} " if Emoji.enabled? - s << "#{@cask} was successfully installed!" + s << "#{@cask} was successfully #{upgrade? ? "upgraded" : "installed"}!" end def download @@ -384,13 +384,13 @@ def start_upgrade def revert_upgrade return unless upgrade? opoo "Reverting upgrade for Cask #{@cask}" - reinstall + install_artifacts + enable_accessibility_access end def finalize_upgrade return unless upgrade? purge_versioned_files(upgrade: true) - oh1 "Cask #{@cask} was successfully upgraded!" end def uninstall_artifacts @@ -426,7 +426,7 @@ def gain_permissions_remove(path) end def purge_versioned_files(upgrade: false) - odebug "Purging files for version #{@cask.version} of Cask #{@cask}" + ohai "Purging files for version #{@cask.version} of Cask #{@cask}" unless upgrade? # versioned staged distribution gain_permissions_remove(@cask.staged_path) if !@cask.staged_path.nil? && @cask.staged_path.exist? From 522a229dbb0e558b8898df60095527e081500655 Mon Sep 17 00:00:00 2001 From: "L. E. Segovia" Date: Tue, 7 Nov 2017 22:27:04 -0300 Subject: [PATCH 05/15] Fix upgrade tests and some upgrade? leftovers --- Library/Homebrew/cask/lib/hbc/installer.rb | 6 +- .../Homebrew/test/cask/cli/upgrade_spec.rb | 73 ++++++++++++++++--- .../fixtures/cask/Casks/auto-updates.rb | 8 +- .../cask/Casks/outdated/auto-updates.rb | 8 +- 4 files changed, 72 insertions(+), 23 deletions(-) diff --git a/Library/Homebrew/cask/lib/hbc/installer.rb b/Library/Homebrew/cask/lib/hbc/installer.rb index cf71639efa1c3..88b77c62c73f8 100644 --- a/Library/Homebrew/cask/lib/hbc/installer.rb +++ b/Library/Homebrew/cask/lib/hbc/installer.rb @@ -426,7 +426,7 @@ def gain_permissions_remove(path) end def purge_versioned_files(upgrade: false) - ohai "Purging files for version #{@cask.version} of Cask #{@cask}" unless upgrade? + odebug "Purging files for version #{@cask.version} of Cask #{@cask}" unless upgrade? # versioned staged distribution gain_permissions_remove(@cask.staged_path) if !@cask.staged_path.nil? && @cask.staged_path.exist? @@ -441,10 +441,10 @@ def purge_versioned_files(upgrade: false) end end @cask.metadata_versioned_path.rmdir_if_possible - @cask.metadata_master_container_path.rmdir_if_possible unless upgrade + @cask.metadata_master_container_path.rmdir_if_possible unless upgrade? # toplevel staged distribution - @cask.caskroom_path.rmdir_if_possible unless upgrade + @cask.caskroom_path.rmdir_if_possible unless upgrade? end def purge_caskroom_path diff --git a/Library/Homebrew/test/cask/cli/upgrade_spec.rb b/Library/Homebrew/test/cask/cli/upgrade_spec.rb index 532541ba7a492..38e57fc5e0aa7 100644 --- a/Library/Homebrew/test/cask/cli/upgrade_spec.rb +++ b/Library/Homebrew/test/cask/cli/upgrade_spec.rb @@ -1,24 +1,31 @@ require_relative "shared_examples/invalid_option" describe Hbc::CLI::Upgrade, :cask do - let(:installed) do - [ - Hbc::CaskLoader.load(cask_path("outdated/local-caffeine")), - Hbc::CaskLoader.load(cask_path("outdated/local-transmission")), - Hbc::CaskLoader.load(cask_path("outdated/auto-updates")), - ] - end - it_behaves_like "a command that handles invalid options" before(:example) do - installed.each { |cask| InstallHelper.install_with_caskfile(cask) } + installed = + [ + "outdated/local-caffeine", + "outdated/local-transmission", + "outdated/auto-updates", + ] + + installed.each { |cask| Hbc::CLI::Install.run(cask) } allow_any_instance_of(described_class).to receive(:verbose?).and_return(true) 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 + 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") + + expect(Hbc::CaskLoader.load("local-transmission")).to be_installed + expect(Hbc.appdir.join("Transmission.app")).to be_a_directory + expect(Hbc::CaskLoader.load("local-transmission").versions).to include("2.60") + described_class.run expect(Hbc::CaskLoader.load("local-caffeine")).to be_installed @@ -31,47 +38,89 @@ end it "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") + + expect(Hbc::CaskLoader.load("local-transmission")).to be_installed + expect(Hbc.appdir.join("Transmission.app")).to be_a_directory expect(Hbc::CaskLoader.load("local-transmission").versions).to include("2.60") described_class.run("local-caffeine") + 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.3") expect(Hbc::CaskLoader.load("local-caffeine").versions).to_not include("1.2.2") + + expect(Hbc::CaskLoader.load("local-transmission")).to be_installed + expect(Hbc.appdir.join("Transmission.app")).to be_a_directory 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 + 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") + + expect(Hbc::CaskLoader.load("auto-updates")).to be_installed + expect(Hbc.appdir.join("MyFancyApp.app")).to be_a_directory expect(Hbc::CaskLoader.load("auto-updates").versions).to include("2.57") - described_class.run("local-caffeine", "auto-updates", "version-latest-string") + described_class.run("local-caffeine", "auto-updates") + 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.3") + + expect(Hbc::CaskLoader.load("auto-updates")).to be_installed + expect(Hbc.appdir.join("MyFancyApp.app")).to be_a_directory expect(Hbc::CaskLoader.load("auto-updates").versions).to include("2.57") end end describe "with --greedy it checks additional Casks" do 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 expect(Hbc::CaskLoader.load("auto-updates").versions).to include("2.57") + + 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") + expect(Hbc::CaskLoader.load("local-transmission").versions).to include("2.60") described_class.run("--greedy") + expect(Hbc::CaskLoader.load("auto-updates")).to be_installed + expect(Hbc.appdir.join("MyFancyApp.app")).to be_a_directory expect(Hbc::CaskLoader.load("auto-updates").versions).to include("2.61") + + 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.3") + + expect(Hbc::CaskLoader.load("local-transmission")).to be_installed + expect(Hbc.appdir.join("Transmission.app")).to be_a_directory expect(Hbc::CaskLoader.load("local-transmission").versions).to include("2.61") end it 'does not include the Casks with "auto_updates true" when the version did not change' do - cask = Hbc::CaskLoader.load(cask_path("auto-updates")) - InstallHelper.install_with_caskfile(cask) + expect(Hbc::CaskLoader.load("auto-updates")).to be_installed + expect(Hbc.appdir.join("MyFancyApp.app")).to be_a_directory + expect(Hbc::CaskLoader.load("auto-updates").versions).to include("2.57") + + described_class.run("auto-updates", "--greedy") + + expect(Hbc::CaskLoader.load("auto-updates")).to be_installed + expect(Hbc.appdir.join("MyFancyApp.app")).to be_a_directory expect(Hbc::CaskLoader.load("auto-updates").versions).to include("2.61") described_class.run("auto-updates", "--greedy") + expect(Hbc::CaskLoader.load("auto-updates")).to be_installed + expect(Hbc.appdir.join("MyFancyApp.app")).to be_a_directory expect(Hbc::CaskLoader.load("auto-updates").versions).to include("2.61") end end diff --git a/Library/Homebrew/test/support/fixtures/cask/Casks/auto-updates.rb b/Library/Homebrew/test/support/fixtures/cask/Casks/auto-updates.rb index 1cada2561cfb5..4f61455bb1bb6 100644 --- a/Library/Homebrew/test/support/fixtures/cask/Casks/auto-updates.rb +++ b/Library/Homebrew/test/support/fixtures/cask/Casks/auto-updates.rb @@ -1,11 +1,11 @@ cask 'auto-updates' do version '2.61' - sha256 'e44ffa103fbf83f55c8d0b1bea309a43b2880798dae8620b1ee8da5e1095ec68' + sha256 '5633c3a0f2e572cbf021507dec78c50998b398c343232bdfc7e26221d0a5db4d' - url "file://#{TEST_FIXTURE_DIR}/cask/transmission-2.61.dmg" - homepage 'http://example.com/auto-updates' + url "file://#{TEST_FIXTURE_DIR}/cask/MyFancyApp.zip" + homepage 'http://example.com/MyFancyApp' auto_updates true - app 'Transmission.app' + app 'MyFancyApp/MyFancyApp.app' end diff --git a/Library/Homebrew/test/support/fixtures/cask/Casks/outdated/auto-updates.rb b/Library/Homebrew/test/support/fixtures/cask/Casks/outdated/auto-updates.rb index e202f5a1632e7..5844a0762cc7f 100644 --- a/Library/Homebrew/test/support/fixtures/cask/Casks/outdated/auto-updates.rb +++ b/Library/Homebrew/test/support/fixtures/cask/Casks/outdated/auto-updates.rb @@ -1,11 +1,11 @@ cask 'auto-updates' do version '2.57' - sha256 'e44ffa103fbf83f55c8d0b1bea309a43b2880798dae8620b1ee8da5e1095ec68' + sha256 '5633c3a0f2e572cbf021507dec78c50998b398c343232bdfc7e26221d0a5db4d' - url "file://#{TEST_FIXTURE_DIR}/cask/transmission-2.61.dmg" - homepage 'http://example.com/auto-updates' + url "file://#{TEST_FIXTURE_DIR}/cask/MyFancyApp.zip" + homepage 'http://example.com/MyFancyApp' auto_updates true - app 'Transmission.app' + app 'MyFancyApp/MyFancyApp.app' end From 72e673afaeec817c38465dcb9c652ef7556d0b52 Mon Sep 17 00:00:00 2001 From: "L. E. Segovia" Date: Fri, 10 Nov 2017 10:05:18 -0300 Subject: [PATCH 06/15] Fix reinstall/uninstall tests --- Library/Homebrew/cask/lib/hbc/artifact/moved.rb | 4 +++- Library/Homebrew/cask/lib/hbc/installer.rb | 12 ++++++------ Library/Homebrew/test/cask/cli/reinstall_spec.rb | 3 ++- Library/Homebrew/test/cask/cli/uninstall_spec.rb | 3 ++- 4 files changed, 13 insertions(+), 9 deletions(-) diff --git a/Library/Homebrew/cask/lib/hbc/artifact/moved.rb b/Library/Homebrew/cask/lib/hbc/artifact/moved.rb index a2f890e4cea83..dacbe20b739f5 100644 --- a/Library/Homebrew/cask/lib/hbc/artifact/moved.rb +++ b/Library/Homebrew/cask/lib/hbc/artifact/moved.rb @@ -25,7 +25,7 @@ def summarize_installed private - def move(source, target, force: false, command: nil, **options) + def move(source, target, skip: false, force: false, command: nil, **options) if Utils.path_occupied?(target) message = "It seems there is already #{self.class.english_article} #{self.class.english_name} at '#{target}'" raise CaskError, "#{message}." unless force @@ -34,6 +34,8 @@ def move(source, target, force: false, command: nil, **options) end unless source.exist? + return if skip + raise CaskError, "It seems the #{self.class.english_name} source '#{source}' is not there." end diff --git a/Library/Homebrew/cask/lib/hbc/installer.rb b/Library/Homebrew/cask/lib/hbc/installer.rb index 88b77c62c73f8..03f2c6570b4d9 100644 --- a/Library/Homebrew/cask/lib/hbc/installer.rb +++ b/Library/Homebrew/cask/lib/hbc/installer.rb @@ -368,7 +368,7 @@ def save_caskfile def uninstall oh1 "Uninstalling Cask #{@cask}" disable_accessibility_access - uninstall_artifacts + uninstall_artifacts(clear: true) purge_versioned_files purge_caskroom_path if force? end @@ -390,10 +390,10 @@ def revert_upgrade def finalize_upgrade return unless upgrade? - purge_versioned_files(upgrade: true) + purge_versioned_files end - def uninstall_artifacts + def uninstall_artifacts(clear: false) odebug "Un-installing artifacts" artifacts = @cask.artifacts @@ -402,7 +402,7 @@ def uninstall_artifacts artifacts.each do |artifact| next unless artifact.respond_to?(:uninstall_phase) odebug "Un-installing artifact of class #{artifact.class}" - artifact.uninstall_phase(command: @command, verbose: verbose?, force: force?) + artifact.uninstall_phase(command: @command, verbose: verbose?, skip: clear, force: force?) end end @@ -425,8 +425,8 @@ def gain_permissions_remove(path) Utils.gain_permissions_remove(path, command: @command) end - def purge_versioned_files(upgrade: false) - odebug "Purging files for version #{@cask.version} of Cask #{@cask}" unless upgrade? + def purge_versioned_files + ohai "Purging files for version #{@cask.version} of Cask #{@cask}" # versioned staged distribution gain_permissions_remove(@cask.staged_path) if !@cask.staged_path.nil? && @cask.staged_path.exist? diff --git a/Library/Homebrew/test/cask/cli/reinstall_spec.rb b/Library/Homebrew/test/cask/cli/reinstall_spec.rb index 5e551e5b5a2fb..1df8147159a2f 100644 --- a/Library/Homebrew/test/cask/cli/reinstall_spec.rb +++ b/Library/Homebrew/test/cask/cli/reinstall_spec.rb @@ -13,7 +13,8 @@ Already downloaded: .*local-caffeine--1.2.3.zip ==> Verifying checksum for Cask local-caffeine ==> Uninstalling Cask local-caffeine - ==> Removing App '.*Caffeine.app'. + ==> Moving App 'Caffeine.app' to '.*Caffeine.app'. + ==> Purging files for version 1.2.3 of Cask local-caffeine ==> Installing Cask local-caffeine ==> Moving App 'Caffeine.app' to '.*Caffeine.app'. .*local-caffeine was successfully installed! diff --git a/Library/Homebrew/test/cask/cli/uninstall_spec.rb b/Library/Homebrew/test/cask/cli/uninstall_spec.rb index 1ab8f7e4da89a..578c0d43a7cd9 100644 --- a/Library/Homebrew/test/cask/cli/uninstall_spec.rb +++ b/Library/Homebrew/test/cask/cli/uninstall_spec.rb @@ -12,7 +12,8 @@ output = Regexp.new <<~EOS ==> Uninstalling Cask local-caffeine - ==> Removing App '.*Caffeine.app'. + ==> Moving App 'Caffeine.app' to '.*Caffeine.app'. + ==> Purging files for version 1.2.3 of Cask local-caffeine EOS expect { From 8cc1aea5f07d1fdfb9cf8ae9a5687c571a856312 Mon Sep 17 00:00:00 2001 From: "L. E. Segovia" Date: Sat, 11 Nov 2017 17:21:13 -0300 Subject: [PATCH 07/15] Implement @reitermarkus's upgrade flow --- Library/Homebrew/cask/lib/hbc/cli/upgrade.rb | 20 ++++++++++++++------ Library/Homebrew/cask/lib/hbc/installer.rb | 2 ++ 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/Library/Homebrew/cask/lib/hbc/cli/upgrade.rb b/Library/Homebrew/cask/lib/hbc/cli/upgrade.rb index 1165ce6fc38fb..90da6f325c0aa 100644 --- a/Library/Homebrew/cask/lib/hbc/cli/upgrade.rb +++ b/Library/Homebrew/cask/lib/hbc/cli/upgrade.rb @@ -21,8 +21,7 @@ def run puts outdated_casks.map { |f| "#{f.full_name} #{f.version}" } * ", " outdated_casks.each do |old_cask| - odebug "Uninstalling Cask #{old_cask}" - + odebug "Started upgrade process for Cask #{old_cask}" raise CaskNotInstalledError, old_cask unless old_cask.installed? || force? unless old_cask.installed_caskfile.nil? @@ -43,17 +42,26 @@ def run upgrade: true) begin - # purge artifacts BUT keep metadata aside + # Start new Cask's installation steps + new_cask_installer.check_conflicts + + new_cask_installer.fetch + + new_cask_installer.stage + + # Move the old Cask's artifacts back to staging old_cask_installer.start_upgrade - # install BUT do not yet save metadata + # Install the new Cask + new_cask_installer.install_artifacts - new_cask_installer.install + new_cask_installer.enable_accessibility_access - # if successful, remove old metadata and install new + # If successful, wipe the old Cask from staging old_cask_installer.finalize_upgrade rescue CaskError => e opoo e.message + new_cask_installer.uninstall old_cask_installer.revert_upgrade end end diff --git a/Library/Homebrew/cask/lib/hbc/installer.rb b/Library/Homebrew/cask/lib/hbc/installer.rb index 03f2c6570b4d9..aa6f04dbe55fd 100644 --- a/Library/Homebrew/cask/lib/hbc/installer.rb +++ b/Library/Homebrew/cask/lib/hbc/installer.rb @@ -391,6 +391,8 @@ def revert_upgrade def finalize_upgrade return unless upgrade? purge_versioned_files + + puts summary end def uninstall_artifacts(clear: false) From 36fe355159385669fbd5c294b901ed1df5bc745f Mon Sep 17 00:00:00 2001 From: "L. E. Segovia" Date: Sun, 12 Nov 2017 09:25:15 -0300 Subject: [PATCH 08/15] Add tests for upgrade recovery --- Library/Homebrew/cask/lib/hbc/cli/upgrade.rb | 11 ++- .../Homebrew/test/cask/cli/upgrade_spec.rb | 82 ++++++++++++++++--- .../cask/Casks/outdated/bad-checksum.rb | 9 ++ .../Casks/outdated/will-fail-if-upgraded.rb | 9 ++ .../cask/Casks/will-fail-if-upgraded.rb | 9 ++ 5 files changed, 106 insertions(+), 14 deletions(-) create mode 100644 Library/Homebrew/test/support/fixtures/cask/Casks/outdated/bad-checksum.rb create mode 100644 Library/Homebrew/test/support/fixtures/cask/Casks/outdated/will-fail-if-upgraded.rb create mode 100644 Library/Homebrew/test/support/fixtures/cask/Casks/will-fail-if-upgraded.rb diff --git a/Library/Homebrew/cask/lib/hbc/cli/upgrade.rb b/Library/Homebrew/cask/lib/hbc/cli/upgrade.rb index 90da6f325c0aa..94fe56a47fd4a 100644 --- a/Library/Homebrew/cask/lib/hbc/cli/upgrade.rb +++ b/Library/Homebrew/cask/lib/hbc/cli/upgrade.rb @@ -41,6 +41,9 @@ def run 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 @@ def run # 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 @@ def run 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 38e57fc5e0aa7..ee7ef2261441b 100644 --- a/Library/Homebrew/test/cask/cli/upgrade_spec.rb +++ b/Library/Homebrew/test/cask/cli/upgrade_spec.rb @@ -3,21 +3,39 @@ 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) } + + 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) } + 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 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 @@ 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 @@ 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 @@ 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 @@ 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 0000000000000..5e7e744830225 --- /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 0000000000000..7735ffa848a8a --- /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 0000000000000..99ed4b87cfd08 --- /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 From 8ee6ac26130c74fd102d4e77e24f2c3b6aa86adc Mon Sep 17 00:00:00 2001 From: "L. E. Segovia" Date: Thu, 16 Nov 2017 10:40:32 -0300 Subject: [PATCH 09/15] Implement @reitermarkus's comments - Split move into a move_back (and clarify when it is used) - Remove unused flags - Raise error if installed Caskfile not found - Error out if an upgrade fails - Remove some defensive programming checks --- .../Homebrew/cask/lib/hbc/artifact/moved.rb | 33 ++++++++++++++++--- Library/Homebrew/cask/lib/hbc/cli/upgrade.rb | 10 +++--- Library/Homebrew/cask/lib/hbc/installer.rb | 7 ++-- .../Homebrew/test/cask/cli/reinstall_spec.rb | 2 +- .../Homebrew/test/cask/cli/uninstall_spec.rb | 2 +- .../Homebrew/test/cask/cli/upgrade_spec.rb | 8 +++-- 6 files changed, 42 insertions(+), 20 deletions(-) diff --git a/Library/Homebrew/cask/lib/hbc/artifact/moved.rb b/Library/Homebrew/cask/lib/hbc/artifact/moved.rb index dacbe20b739f5..11e019af2c4f7 100644 --- a/Library/Homebrew/cask/lib/hbc/artifact/moved.rb +++ b/Library/Homebrew/cask/lib/hbc/artifact/moved.rb @@ -8,11 +8,11 @@ def self.english_description end def install_phase(**options) - move(source, target, **options) + move(**options) end def uninstall_phase(**options) - move(target, source, **options) + move_back(**options) end def summarize_installed @@ -25,7 +25,7 @@ def summarize_installed private - def move(source, target, skip: false, force: false, command: nil, **options) + def move(force: false, command: nil, **options) if Utils.path_occupied?(target) message = "It seems there is already #{self.class.english_article} #{self.class.english_name} at '#{target}'" raise CaskError, "#{message}." unless force @@ -34,8 +34,6 @@ def move(source, target, skip: false, force: false, command: nil, **options) end unless source.exist? - return if skip - raise CaskError, "It seems the #{self.class.english_name} source '#{source}' is not there." end @@ -51,6 +49,31 @@ def move(source, target, skip: false, force: false, command: nil, **options) add_altname_metadata(target, source.basename, command: command) end + def move_back(skip: false, force: false, command: nil, **options) + if Utils.path_occupied?(source) + message = "It seems there is already #{self.class.english_article} #{self.class.english_name} at '#{source}'" + raise CaskError, "#{message}." unless force + opoo "#{message}; overwriting." + delete(source, force: force, command: command, **options) + end + + unless target.exist? + return if skip + raise CaskError, "It seems the #{self.class.english_name} source '#{target}' is not there." + end + + ohai "Moving #{self.class.english_name} '#{target.basename}' back to '#{source}'." + source.dirname.mkpath + + if source.parent.writable? + FileUtils.move(target, source) + else + command.run("/bin/mv", args: [target, source], sudo: true) + end + + add_altname_metadata(target, source.basename, command: command) + end + def delete(target, force: false, command: nil, **_) ohai "Removing #{self.class.english_name} '#{target}'." raise CaskError, "Cannot remove undeletable #{self.class.english_name}." if MacOS.undeletable?(target) diff --git a/Library/Homebrew/cask/lib/hbc/cli/upgrade.rb b/Library/Homebrew/cask/lib/hbc/cli/upgrade.rb index 94fe56a47fd4a..993ec6ac657fc 100644 --- a/Library/Homebrew/cask/lib/hbc/cli/upgrade.rb +++ b/Library/Homebrew/cask/lib/hbc/cli/upgrade.rb @@ -4,7 +4,6 @@ class Upgrade < AbstractCommand option "--greedy", :greedy, false option "--quiet", :quiet, false option "--force", :force, false - option "--force-update", :force_update, false option "--skip-cask-deps", :skip_cask_deps, false def initialize(*) @@ -24,10 +23,9 @@ def run odebug "Started upgrade process for Cask #{old_cask}" raise CaskNotInstalledError, old_cask unless old_cask.installed? || force? - unless old_cask.installed_caskfile.nil? - # use the same cask file that was used for installation, if possible - old_cask = CaskLoader.load(old_cask.installed_caskfile) if old_cask.installed_caskfile.exist? - end + raise CaskUnavailableError.new(old_cask, "The Caskfile is missing!") if old_cask.installed_caskfile.nil? + + old_cask = CaskLoader.load(old_cask.installed_caskfile) old_cask_installer = Installer.new(old_cask, binaries: binaries?, verbose: verbose?, force: force?, upgrade: true) @@ -66,10 +64,10 @@ def run # If successful, wipe the old Cask from staging old_cask_installer.finalize_upgrade rescue CaskError => e - opoo e.message new_cask_installer.uninstall_artifacts if new_artifacts_installed new_cask_installer.purge_versioned_files old_cask_installer.revert_upgrade if started_upgrade + raise e end end end diff --git a/Library/Homebrew/cask/lib/hbc/installer.rb b/Library/Homebrew/cask/lib/hbc/installer.rb index aa6f04dbe55fd..6056250fc05ce 100644 --- a/Library/Homebrew/cask/lib/hbc/installer.rb +++ b/Library/Homebrew/cask/lib/hbc/installer.rb @@ -83,8 +83,8 @@ def stage def install odebug "Hbc::Installer#install" - if @cask.installed? && !force? && !@reinstall - raise CaskAlreadyInstalledError, @cask unless upgrade? + if @cask.installed? && !force? && !@reinstall && !upgrade? + raise CaskAlreadyInstalledError, @cask end check_conflicts @@ -374,7 +374,6 @@ def uninstall end def start_upgrade - return unless upgrade? oh1 "Starting upgrade for Cask #{@cask}" disable_accessibility_access @@ -382,14 +381,12 @@ def start_upgrade end def revert_upgrade - return unless upgrade? opoo "Reverting upgrade for Cask #{@cask}" install_artifacts enable_accessibility_access end def finalize_upgrade - return unless upgrade? purge_versioned_files puts summary diff --git a/Library/Homebrew/test/cask/cli/reinstall_spec.rb b/Library/Homebrew/test/cask/cli/reinstall_spec.rb index 1df8147159a2f..3737a7a701357 100644 --- a/Library/Homebrew/test/cask/cli/reinstall_spec.rb +++ b/Library/Homebrew/test/cask/cli/reinstall_spec.rb @@ -13,7 +13,7 @@ Already downloaded: .*local-caffeine--1.2.3.zip ==> Verifying checksum for Cask local-caffeine ==> Uninstalling Cask local-caffeine - ==> Moving App 'Caffeine.app' to '.*Caffeine.app'. + ==> Moving App 'Caffeine.app' back to '.*Caffeine.app'. ==> Purging files for version 1.2.3 of Cask local-caffeine ==> Installing Cask local-caffeine ==> Moving App 'Caffeine.app' to '.*Caffeine.app'. diff --git a/Library/Homebrew/test/cask/cli/uninstall_spec.rb b/Library/Homebrew/test/cask/cli/uninstall_spec.rb index 578c0d43a7cd9..345e1b9f230a6 100644 --- a/Library/Homebrew/test/cask/cli/uninstall_spec.rb +++ b/Library/Homebrew/test/cask/cli/uninstall_spec.rb @@ -12,7 +12,7 @@ output = Regexp.new <<~EOS ==> Uninstalling Cask local-caffeine - ==> Moving App 'Caffeine.app' to '.*Caffeine.app'. + ==> Moving App 'Caffeine.app' back to '.*Caffeine.app'. ==> Purging files for version 1.2.3 of Cask local-caffeine EOS diff --git a/Library/Homebrew/test/cask/cli/upgrade_spec.rb b/Library/Homebrew/test/cask/cli/upgrade_spec.rb index ee7ef2261441b..5c08a4728ee4d 100644 --- a/Library/Homebrew/test/cask/cli/upgrade_spec.rb +++ b/Library/Homebrew/test/cask/cli/upgrade_spec.rb @@ -158,7 +158,9 @@ expect(Hbc::CaskLoader.load("will-fail-if-upgraded").versions).to include("1.2.2") expect { - described_class.run("will-fail-if-upgraded") + expect { + described_class.run("will-fail-if-upgraded") + }.to raise_error(Hbc::CaskError) }.to output(output_reverted).to_stderr expect(Hbc::CaskLoader.load("will-fail-if-upgraded")).to be_installed @@ -173,7 +175,9 @@ expect(Hbc::CaskLoader.load("bad-checksum").versions).to include("1.2.2") expect { - described_class.run("bad-checksum") + expect { + described_class.run("bad-checksum") + }.to raise_error(Hbc::CaskSha256MismatchError) }.to_not output(output_reverted).to_stderr expect(Hbc::CaskLoader.load("bad-checksum")).to be_installed From c9b5de4cabac58b64bf14537d8281f00a540767a Mon Sep 17 00:00:00 2001 From: "L. E. Segovia" Date: Wed, 22 Nov 2017 00:45:29 +0000 Subject: [PATCH 10/15] Output message if no Casks are outdated --- Library/Homebrew/cask/lib/hbc/cli/upgrade.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Library/Homebrew/cask/lib/hbc/cli/upgrade.rb b/Library/Homebrew/cask/lib/hbc/cli/upgrade.rb index 993ec6ac657fc..087a30c03c713 100644 --- a/Library/Homebrew/cask/lib/hbc/cli/upgrade.rb +++ b/Library/Homebrew/cask/lib/hbc/cli/upgrade.rb @@ -14,7 +14,10 @@ def initialize(*) def run outdated_casks = casks(alternative: -> { Hbc.installed }).select { |cask| cask.outdated?(greedy?) } - return if outdated_casks.empty? + if outdated_casks.empty? + oh1 "No Casks to upgrade" + return + end oh1 "Upgrading #{Formatter.pluralize(outdated_casks.length, "outdated package")}, with result:" puts outdated_casks.map { |f| "#{f.full_name} #{f.version}" } * ", " From 7ce43190129378ebb6f46d0a77bd1891bef8c9ad Mon Sep 17 00:00:00 2001 From: "L. E. Segovia" Date: Wed, 22 Nov 2017 16:27:13 +0000 Subject: [PATCH 11/15] Upgrade an outdated Cask just by name, no need for --greedy --- Library/Homebrew/cask/lib/hbc/cli/upgrade.rb | 2 +- Library/Homebrew/test/cask/cli/upgrade_spec.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Library/Homebrew/cask/lib/hbc/cli/upgrade.rb b/Library/Homebrew/cask/lib/hbc/cli/upgrade.rb index 087a30c03c713..a6c679142ff1f 100644 --- a/Library/Homebrew/cask/lib/hbc/cli/upgrade.rb +++ b/Library/Homebrew/cask/lib/hbc/cli/upgrade.rb @@ -12,7 +12,7 @@ def initialize(*) end def run - outdated_casks = casks(alternative: -> { Hbc.installed }).select { |cask| cask.outdated?(greedy?) } + outdated_casks = casks(alternative: -> { Hbc.installed }).select { |cask| cask.outdated?(greedy?) || (args.include?(cask.token) && cask.outdated?(true)) } if outdated_casks.empty? oh1 "No Casks to upgrade" diff --git a/Library/Homebrew/test/cask/cli/upgrade_spec.rb b/Library/Homebrew/test/cask/cli/upgrade_spec.rb index 5c08a4728ee4d..435bc38c74a9c 100644 --- a/Library/Homebrew/test/cask/cli/upgrade_spec.rb +++ b/Library/Homebrew/test/cask/cli/upgrade_spec.rb @@ -76,7 +76,7 @@ expect(Hbc::CaskLoader.load("local-transmission").versions).to include("2.60") end - it 'and ignores "auto_updates" and "latest" Casks even when their tokens are provided in the command line' do + it 'but updates "auto_updates" and "latest" Casks 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") @@ -93,7 +93,7 @@ expect(Hbc::CaskLoader.load("auto-updates")).to be_installed expect(Hbc.appdir.join("MyFancyApp.app")).to be_a_directory - expect(Hbc::CaskLoader.load("auto-updates").versions).to include("2.57") + expect(Hbc::CaskLoader.load("auto-updates").versions).to include("2.61") end end From 7ee98eb421380afd45144ef8df1656cb22d4bb66 Mon Sep 17 00:00:00 2001 From: "L. E. Segovia" Date: Fri, 24 Nov 2017 00:48:14 +0000 Subject: [PATCH 12/15] Implement more of @reitermarkus's comments - Simplify outdated Casks checks - Make use of RSpec's let(:) and .and syntax --- Library/Homebrew/cask/lib/hbc/cli/upgrade.rb | 3 +- .../Homebrew/test/cask/cli/upgrade_spec.rb | 44 +++++++++---------- 2 files changed, 23 insertions(+), 24 deletions(-) diff --git a/Library/Homebrew/cask/lib/hbc/cli/upgrade.rb b/Library/Homebrew/cask/lib/hbc/cli/upgrade.rb index a6c679142ff1f..b567f6d20e3a8 100644 --- a/Library/Homebrew/cask/lib/hbc/cli/upgrade.rb +++ b/Library/Homebrew/cask/lib/hbc/cli/upgrade.rb @@ -12,7 +12,8 @@ def initialize(*) end def run - outdated_casks = casks(alternative: -> { Hbc.installed }).select { |cask| cask.outdated?(greedy?) || (args.include?(cask.token) && cask.outdated?(true)) } + outdated_casks = casks(alternative: -> { Hbc.installed.select { |cask| + cask.outdated?(greedy?) } }).select { |cask| cask.outdated?(true) } if outdated_casks.empty? oh1 "No Casks to upgrade" diff --git a/Library/Homebrew/test/cask/cli/upgrade_spec.rb b/Library/Homebrew/test/cask/cli/upgrade_spec.rb index 435bc38c74a9c..fa2a2361cfd8d 100644 --- a/Library/Homebrew/test/cask/cli/upgrade_spec.rb +++ b/Library/Homebrew/test/cask/cli/upgrade_spec.rb @@ -4,14 +4,15 @@ it_behaves_like "a command that handles invalid options" shared_context "Proper Casks" do - before(:example) do - installed = - [ - "outdated/local-caffeine", - "outdated/local-transmission", - "outdated/auto-updates", - ] + let(:installed) { + [ + "outdated/local-caffeine", + "outdated/local-transmission", + "outdated/auto-updates", + ] + } + before(:example) do installed.each { |cask| Hbc::CLI::Install.run(cask) } allow_any_instance_of(described_class).to receive(:verbose?).and_return(true) @@ -19,13 +20,14 @@ end shared_context "Casks that will fail upon upgrade" do - before(:example) do - installed = - [ - "outdated/bad-checksum", - "outdated/will-fail-if-upgraded", - ] + let(:installed) { + [ + "outdated/bad-checksum", + "outdated/will-fail-if-upgraded", + ] + } + before(:example) do installed.each { |cask| Hbc::CLI::Install.run(cask) } allow_any_instance_of(described_class).to receive(:verbose?).and_return(true) @@ -76,7 +78,7 @@ expect(Hbc::CaskLoader.load("local-transmission").versions).to include("2.60") end - it 'but updates "auto_updates" and "latest" Casks when their tokens are provided in the command line' do + it 'updates "auto_updates" and "latest" Casks 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") @@ -152,16 +154,14 @@ Warning: Reverting upgrade for Cask .* EOS - it "by restoring the old Cask if the upgrade's install failed" do + it "restores the old Cask if the upgrade 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 { - expect { - described_class.run("will-fail-if-upgraded") - }.to raise_error(Hbc::CaskError) - }.to output(output_reverted).to_stderr + described_class.run("will-fail-if-upgraded") + }.to raise_error(Hbc::CaskError).and 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 @@ -175,10 +175,8 @@ expect(Hbc::CaskLoader.load("bad-checksum").versions).to include("1.2.2") expect { - expect { - described_class.run("bad-checksum") - }.to raise_error(Hbc::CaskSha256MismatchError) - }.to_not output(output_reverted).to_stderr + described_class.run("bad-checksum") + }.to raise_error(Hbc::CaskSha256MismatchError).and (not_to_output output_reverted).to_stderr expect(Hbc::CaskLoader.load("bad-checksum")).to be_installed expect(Hbc.appdir.join("Caffeine.app")).to be_a_directory From e0be066f8b5f4ccb313ddcf6bba1d6fed3549b69 Mon Sep 17 00:00:00 2001 From: "L. E. Segovia" Date: Fri, 24 Nov 2017 01:21:30 +0000 Subject: [PATCH 13/15] Load Casks only once (and fix style issues) --- Library/Homebrew/cask/lib/hbc/cli/upgrade.rb | 7 +- .../Homebrew/test/cask/cli/upgrade_spec.rb | 186 ++++++++++-------- 2 files changed, 114 insertions(+), 79 deletions(-) diff --git a/Library/Homebrew/cask/lib/hbc/cli/upgrade.rb b/Library/Homebrew/cask/lib/hbc/cli/upgrade.rb index b567f6d20e3a8..bd80ad6905d63 100644 --- a/Library/Homebrew/cask/lib/hbc/cli/upgrade.rb +++ b/Library/Homebrew/cask/lib/hbc/cli/upgrade.rb @@ -12,8 +12,11 @@ def initialize(*) end def run - outdated_casks = casks(alternative: -> { Hbc.installed.select { |cask| - cask.outdated?(greedy?) } }).select { |cask| cask.outdated?(true) } + outdated_casks = casks(alternative: lambda { + Hbc.installed.select do |cask| + cask.outdated?(greedy?) + end + }).select { |cask| cask.outdated?(true) } if outdated_casks.empty? oh1 "No Casks to upgrade" diff --git a/Library/Homebrew/test/cask/cli/upgrade_spec.rb b/Library/Homebrew/test/cask/cli/upgrade_spec.rb index fa2a2361cfd8d..0f15c2d5ba4b9 100644 --- a/Library/Homebrew/test/cask/cli/upgrade_spec.rb +++ b/Library/Homebrew/test/cask/cli/upgrade_spec.rb @@ -38,64 +38,78 @@ 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") + local_caffeine = Hbc::CaskLoader.load("local-caffeine") + local_caffeine_route = Hbc.appdir.join("Caffeine.app") + local_transmission = Hbc::CaskLoader.load("local-transmission") + local_transmission_route = Hbc.appdir.join("Transmission.app") - expect(Hbc::CaskLoader.load("local-transmission")).to be_installed - expect(Hbc.appdir.join("Transmission.app")).to be_a_directory - expect(Hbc::CaskLoader.load("local-transmission").versions).to include("2.60") + expect(local_caffeine).to be_installed + expect(local_caffeine_route).to be_a_directory + expect(local_caffeine.versions).to include("1.2.2") + + expect(local_transmission).to be_installed + expect(local_transmission_route).to be_a_directory + expect(local_transmission.versions).to include("2.60") described_class.run - 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.3") + expect(local_caffeine).to be_installed + expect(local_caffeine_route).to be_a_directory + expect(local_caffeine.versions).to include("1.2.3") - expect(Hbc::CaskLoader.load("local-transmission")).to be_installed - expect(Hbc.appdir.join("Transmission.app")).to be_a_directory - expect(Hbc::CaskLoader.load("local-transmission").versions).to include("2.61") + expect(local_transmission).to be_installed + expect(local_transmission_route).to be_a_directory + expect(local_transmission.versions).to include("2.61") end 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") + local_caffeine = Hbc::CaskLoader.load("local-caffeine") + local_caffeine_route = Hbc.appdir.join("Caffeine.app") + local_transmission = Hbc::CaskLoader.load("local-transmission") + local_transmission_route = Hbc.appdir.join("Transmission.app") + + expect(local_caffeine).to be_installed + expect(local_caffeine_route).to be_a_directory + expect(local_caffeine.versions).to include("1.2.2") - expect(Hbc::CaskLoader.load("local-transmission")).to be_installed - expect(Hbc.appdir.join("Transmission.app")).to be_a_directory - expect(Hbc::CaskLoader.load("local-transmission").versions).to include("2.60") + expect(local_transmission).to be_installed + expect(local_transmission_route).to be_a_directory + expect(local_transmission.versions).to include("2.60") described_class.run("local-caffeine") - 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.3") - expect(Hbc::CaskLoader.load("local-caffeine").versions).to_not include("1.2.2") + expect(local_caffeine).to be_installed + expect(local_caffeine_route).to be_a_directory + expect(local_caffeine.versions).to include("1.2.3") - expect(Hbc::CaskLoader.load("local-transmission")).to be_installed - expect(Hbc.appdir.join("Transmission.app")).to be_a_directory - expect(Hbc::CaskLoader.load("local-transmission").versions).to include("2.60") + expect(local_transmission).to be_installed + expect(local_transmission_route).to be_a_directory + expect(local_transmission.versions).to include("2.60") end it 'updates "auto_updates" and "latest" Casks 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") + local_caffeine = Hbc::CaskLoader.load("local-caffeine") + local_caffeine_route = Hbc.appdir.join("Caffeine.app") + auto_updates = Hbc::CaskLoader.load("auto-updates") + auto_updates_path = Hbc.appdir.join("MyFancyApp.app") - expect(Hbc::CaskLoader.load("auto-updates")).to be_installed - expect(Hbc.appdir.join("MyFancyApp.app")).to be_a_directory - expect(Hbc::CaskLoader.load("auto-updates").versions).to include("2.57") + expect(local_caffeine).to be_installed + expect(local_caffeine_route).to be_a_directory + expect(local_caffeine.versions).to include("1.2.2") + + expect(auto_updates).to be_installed + expect(auto_updates_path).to be_a_directory + expect(auto_updates.versions).to include("2.57") described_class.run("local-caffeine", "auto-updates") - 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.3") + expect(local_caffeine).to be_installed + expect(local_caffeine_route).to be_a_directory + expect(local_caffeine.versions).to include("1.2.3") - expect(Hbc::CaskLoader.load("auto-updates")).to be_installed - expect(Hbc.appdir.join("MyFancyApp.app")).to be_a_directory - expect(Hbc::CaskLoader.load("auto-updates").versions).to include("2.61") + expect(auto_updates).to be_installed + expect(auto_updates_path).to be_a_directory + expect(auto_updates.versions).to include("2.61") end end @@ -103,47 +117,59 @@ 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 - expect(Hbc::CaskLoader.load("auto-updates").versions).to include("2.57") + local_caffeine = Hbc::CaskLoader.load("local-caffeine") + local_caffeine_route = Hbc.appdir.join("Caffeine.app") + auto_updates = Hbc::CaskLoader.load("auto-updates") + auto_updates_path = Hbc.appdir.join("MyFancyApp.app") + local_transmission = Hbc::CaskLoader.load("local-transmission") + local_transmission_route = Hbc.appdir.join("Transmission.app") + + expect(local_caffeine).to be_installed + expect(local_caffeine_route).to be_a_directory + expect(local_caffeine.versions).to include("1.2.2") - 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") + expect(auto_updates).to be_installed + expect(auto_updates_path).to be_a_directory + expect(auto_updates.versions).to include("2.57") - expect(Hbc::CaskLoader.load("local-transmission").versions).to include("2.60") + expect(local_transmission).to be_installed + expect(local_transmission_route).to be_a_directory + expect(local_transmission.versions).to include("2.60") described_class.run("--greedy") - expect(Hbc::CaskLoader.load("auto-updates")).to be_installed - expect(Hbc.appdir.join("MyFancyApp.app")).to be_a_directory - expect(Hbc::CaskLoader.load("auto-updates").versions).to include("2.61") + expect(local_caffeine).to be_installed + expect(local_caffeine_route).to be_a_directory + expect(local_caffeine.versions).to include("1.2.3") - 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.3") + expect(auto_updates).to be_installed + expect(auto_updates_path).to be_a_directory + expect(auto_updates.versions).to include("2.61") - expect(Hbc::CaskLoader.load("local-transmission")).to be_installed - expect(Hbc.appdir.join("Transmission.app")).to be_a_directory - expect(Hbc::CaskLoader.load("local-transmission").versions).to include("2.61") + expect(local_transmission).to be_installed + expect(local_transmission_route).to be_a_directory + expect(local_transmission.versions).to include("2.61") end it 'does not include the Casks with "auto_updates true" when the version did not change' do - expect(Hbc::CaskLoader.load("auto-updates")).to be_installed - expect(Hbc.appdir.join("MyFancyApp.app")).to be_a_directory - expect(Hbc::CaskLoader.load("auto-updates").versions).to include("2.57") + auto_updates = Hbc::CaskLoader.load("auto-updates") + auto_updates_path = Hbc.appdir.join("MyFancyApp.app") + + expect(auto_updates).to be_installed + expect(auto_updates_path).to be_a_directory + expect(auto_updates.versions).to include("2.57") described_class.run("auto-updates", "--greedy") - expect(Hbc::CaskLoader.load("auto-updates")).to be_installed - expect(Hbc.appdir.join("MyFancyApp.app")).to be_a_directory - expect(Hbc::CaskLoader.load("auto-updates").versions).to include("2.61") + expect(auto_updates).to be_installed + expect(auto_updates_path).to be_a_directory + expect(auto_updates.versions).to include("2.61") described_class.run("auto-updates", "--greedy") - expect(Hbc::CaskLoader.load("auto-updates")).to be_installed - expect(Hbc.appdir.join("MyFancyApp.app")).to be_a_directory - expect(Hbc::CaskLoader.load("auto-updates").versions).to include("2.61") + expect(auto_updates).to be_installed + expect(auto_updates_path).to be_a_directory + expect(auto_updates.versions).to include("2.61") end end @@ -155,33 +181,39 @@ EOS it "restores the old Cask if the upgrade 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") + will_fail_if_upgraded = Hbc::CaskLoader.load("will-fail-if-upgraded") + will_fail_if_upgraded_path = Hbc.appdir.join("container") + + expect(will_fail_if_upgraded).to be_installed + expect(will_fail_if_upgraded_path).to be_a_file + expect(will_fail_if_upgraded.versions).to include("1.2.2") expect { described_class.run("will-fail-if-upgraded") }.to raise_error(Hbc::CaskError).and 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 + expect(will_fail_if_upgraded).to be_installed + expect(will_fail_if_upgraded_path).to be_a_file + expect(will_fail_if_upgraded.versions).to include("1.2.2") + expect(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") + bad_checksum = Hbc::CaskLoader.load("bad-checksum") + bad_checksum_path = Hbc.appdir.join("Caffeine.app") + + expect(bad_checksum).to be_installed + expect(bad_checksum_path).to be_a_directory + expect(bad_checksum.versions).to include("1.2.2") expect { described_class.run("bad-checksum") - }.to raise_error(Hbc::CaskSha256MismatchError).and (not_to_output output_reverted).to_stderr + }.to raise_error(Hbc::CaskSha256MismatchError).and(not_to_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 + expect(bad_checksum).to be_installed + expect(bad_checksum_path).to be_a_directory + expect(bad_checksum.versions).to include("1.2.2") + expect(bad_checksum.staged_path).to_not exist end end end From a2730c8618ebb512cf00318af6d4987dbee1efe5 Mon Sep 17 00:00:00 2001 From: "L. E. Segovia" Date: Mon, 27 Nov 2017 01:29:08 +0000 Subject: [PATCH 14/15] Implement more of @reitermarkus's comments - Include tests in context - replace 'route' with 'path' - more assorted fixes --- .../Homebrew/test/cask/cli/upgrade_spec.rb | 268 +++++++++--------- 1 file changed, 130 insertions(+), 138 deletions(-) diff --git a/Library/Homebrew/test/cask/cli/upgrade_spec.rb b/Library/Homebrew/test/cask/cli/upgrade_spec.rb index 0f15c2d5ba4b9..a9959bf6f8d37 100644 --- a/Library/Homebrew/test/cask/cli/upgrade_spec.rb +++ b/Library/Homebrew/test/cask/cli/upgrade_spec.rb @@ -3,7 +3,7 @@ describe Hbc::CLI::Upgrade, :cask do it_behaves_like "a command that handles invalid options" - shared_context "Proper Casks" do + context "successful upgrade" do let(:installed) { [ "outdated/local-caffeine", @@ -17,164 +17,156 @@ allow_any_instance_of(described_class).to receive(:verbose?).and_return(true) end - end - - shared_context "Casks that will fail upon upgrade" do - let(:installed) { - [ - "outdated/bad-checksum", - "outdated/will-fail-if-upgraded", - ] - } - - before(:example) do - installed.each { |cask| Hbc::CLI::Install.run(cask) } - allow_any_instance_of(described_class).to receive(:verbose?).and_return(true) + 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 + local_caffeine = Hbc::CaskLoader.load("local-caffeine") + local_caffeine_path = Hbc.appdir.join("Caffeine.app") + local_transmission = Hbc::CaskLoader.load("local-transmission") + local_transmission_path = Hbc.appdir.join("Transmission.app") + + expect(local_caffeine).to be_installed + expect(local_caffeine_path).to be_a_directory + expect(local_caffeine.versions).to include("1.2.2") + + expect(local_transmission).to be_installed + expect(local_transmission_path).to be_a_directory + expect(local_transmission.versions).to include("2.60") + + described_class.run + + expect(local_caffeine).to be_installed + expect(local_caffeine_path).to be_a_directory + expect(local_caffeine.versions).to include("1.2.3") + + expect(local_transmission).to be_installed + expect(local_transmission_path).to be_a_directory + expect(local_transmission.versions).to include("2.61") + end + + it "updates only the Casks specified in the command line" do + local_caffeine = Hbc::CaskLoader.load("local-caffeine") + local_caffeine_path = Hbc.appdir.join("Caffeine.app") + local_transmission = Hbc::CaskLoader.load("local-transmission") + local_transmission_path = Hbc.appdir.join("Transmission.app") + + expect(local_caffeine).to be_installed + expect(local_caffeine_path).to be_a_directory + expect(local_caffeine.versions).to include("1.2.2") + + expect(local_transmission).to be_installed + expect(local_transmission_path).to be_a_directory + expect(local_transmission.versions).to include("2.60") + + described_class.run("local-caffeine") + + expect(local_caffeine).to be_installed + expect(local_caffeine_path).to be_a_directory + expect(local_caffeine.versions).to include("1.2.3") + + expect(local_transmission).to be_installed + expect(local_transmission_path).to be_a_directory + expect(local_transmission.versions).to include("2.60") + end + + it 'updates "auto_updates" and "latest" Casks when their tokens are provided in the command line' do + local_caffeine = Hbc::CaskLoader.load("local-caffeine") + local_caffeine_path = Hbc.appdir.join("Caffeine.app") + auto_updates = Hbc::CaskLoader.load("auto-updates") + auto_updates_path = Hbc.appdir.join("MyFancyApp.app") + + expect(local_caffeine).to be_installed + expect(local_caffeine_path).to be_a_directory + expect(local_caffeine.versions).to include("1.2.2") + + expect(auto_updates).to be_installed + expect(auto_updates_path).to be_a_directory + expect(auto_updates.versions).to include("2.57") + + described_class.run("local-caffeine", "auto-updates") + + expect(local_caffeine).to be_installed + expect(local_caffeine_path).to be_a_directory + expect(local_caffeine.versions).to include("1.2.3") + + expect(auto_updates).to be_installed + expect(auto_updates_path).to be_a_directory + expect(auto_updates.versions).to include("2.61") + end end - end - - describe 'without --greedy it ignores the Casks with "version latest" or "auto_updates true"' do - include_context "Proper Casks" - it "and updates all the installed Casks when no token is provided" do - local_caffeine = Hbc::CaskLoader.load("local-caffeine") - local_caffeine_route = Hbc.appdir.join("Caffeine.app") - local_transmission = Hbc::CaskLoader.load("local-transmission") - local_transmission_route = Hbc.appdir.join("Transmission.app") + describe "with --greedy it checks additional Casks" do + it 'includes the Casks with "auto_updates true" or "version latest" with --greedy' do + local_caffeine = Hbc::CaskLoader.load("local-caffeine") + local_caffeine_path = Hbc.appdir.join("Caffeine.app") + auto_updates = Hbc::CaskLoader.load("auto-updates") + auto_updates_path = Hbc.appdir.join("MyFancyApp.app") + local_transmission = Hbc::CaskLoader.load("local-transmission") + local_transmission_path = Hbc.appdir.join("Transmission.app") - expect(local_caffeine).to be_installed - expect(local_caffeine_route).to be_a_directory - expect(local_caffeine.versions).to include("1.2.2") + expect(local_caffeine).to be_installed + expect(local_caffeine_path).to be_a_directory + expect(local_caffeine.versions).to include("1.2.2") - expect(local_transmission).to be_installed - expect(local_transmission_route).to be_a_directory - expect(local_transmission.versions).to include("2.60") + expect(auto_updates).to be_installed + expect(auto_updates_path).to be_a_directory + expect(auto_updates.versions).to include("2.57") - described_class.run + expect(local_transmission).to be_installed + expect(local_transmission_path).to be_a_directory + expect(local_transmission.versions).to include("2.60") - expect(local_caffeine).to be_installed - expect(local_caffeine_route).to be_a_directory - expect(local_caffeine.versions).to include("1.2.3") + described_class.run("--greedy") - expect(local_transmission).to be_installed - expect(local_transmission_route).to be_a_directory - expect(local_transmission.versions).to include("2.61") - end - - it "and updates only the Casks specified in the command line" do - local_caffeine = Hbc::CaskLoader.load("local-caffeine") - local_caffeine_route = Hbc.appdir.join("Caffeine.app") - local_transmission = Hbc::CaskLoader.load("local-transmission") - local_transmission_route = Hbc.appdir.join("Transmission.app") + expect(local_caffeine).to be_installed + expect(local_caffeine_path).to be_a_directory + expect(local_caffeine.versions).to include("1.2.3") - expect(local_caffeine).to be_installed - expect(local_caffeine_route).to be_a_directory - expect(local_caffeine.versions).to include("1.2.2") + expect(auto_updates).to be_installed + expect(auto_updates_path).to be_a_directory + expect(auto_updates.versions).to include("2.61") - expect(local_transmission).to be_installed - expect(local_transmission_route).to be_a_directory - expect(local_transmission.versions).to include("2.60") + expect(local_transmission).to be_installed + expect(local_transmission_path).to be_a_directory + expect(local_transmission.versions).to include("2.61") + end - described_class.run("local-caffeine") + it 'does not include the Casks with "auto_updates true" when the version did not change' do + cask = Hbc::CaskLoader.load("auto-updates") + cask_path = Hbc.appdir.join("MyFancyApp.app") - expect(local_caffeine).to be_installed - expect(local_caffeine_route).to be_a_directory - expect(local_caffeine.versions).to include("1.2.3") + expect(cask).to be_installed + expect(cask_path).to be_a_directory + expect(cask.versions).to include("2.57") - expect(local_transmission).to be_installed - expect(local_transmission_route).to be_a_directory - expect(local_transmission.versions).to include("2.60") - end + described_class.run("auto-updates", "--greedy") - it 'updates "auto_updates" and "latest" Casks when their tokens are provided in the command line' do - local_caffeine = Hbc::CaskLoader.load("local-caffeine") - local_caffeine_route = Hbc.appdir.join("Caffeine.app") - auto_updates = Hbc::CaskLoader.load("auto-updates") - auto_updates_path = Hbc.appdir.join("MyFancyApp.app") + expect(cask).to be_installed + expect(cask_path).to be_a_directory + expect(cask.versions).to include("2.61") - expect(local_caffeine).to be_installed - expect(local_caffeine_route).to be_a_directory - expect(local_caffeine.versions).to include("1.2.2") + described_class.run("auto-updates", "--greedy") - expect(auto_updates).to be_installed - expect(auto_updates_path).to be_a_directory - expect(auto_updates.versions).to include("2.57") - - described_class.run("local-caffeine", "auto-updates") - - expect(local_caffeine).to be_installed - expect(local_caffeine_route).to be_a_directory - expect(local_caffeine.versions).to include("1.2.3") - - expect(auto_updates).to be_installed - expect(auto_updates_path).to be_a_directory - expect(auto_updates.versions).to include("2.61") + expect(cask).to be_installed + expect(cask_path).to be_a_directory + expect(cask.versions).to include("2.61") + end end 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 - local_caffeine = Hbc::CaskLoader.load("local-caffeine") - local_caffeine_route = Hbc.appdir.join("Caffeine.app") - auto_updates = Hbc::CaskLoader.load("auto-updates") - auto_updates_path = Hbc.appdir.join("MyFancyApp.app") - local_transmission = Hbc::CaskLoader.load("local-transmission") - local_transmission_route = Hbc.appdir.join("Transmission.app") - - expect(local_caffeine).to be_installed - expect(local_caffeine_route).to be_a_directory - expect(local_caffeine.versions).to include("1.2.2") - - expect(auto_updates).to be_installed - expect(auto_updates_path).to be_a_directory - expect(auto_updates.versions).to include("2.57") - - expect(local_transmission).to be_installed - expect(local_transmission_route).to be_a_directory - expect(local_transmission.versions).to include("2.60") - - described_class.run("--greedy") - - expect(local_caffeine).to be_installed - expect(local_caffeine_route).to be_a_directory - expect(local_caffeine.versions).to include("1.2.3") - - expect(auto_updates).to be_installed - expect(auto_updates_path).to be_a_directory - expect(auto_updates.versions).to include("2.61") - - expect(local_transmission).to be_installed - expect(local_transmission_route).to be_a_directory - expect(local_transmission.versions).to include("2.61") - end - - it 'does not include the Casks with "auto_updates true" when the version did not change' do - auto_updates = Hbc::CaskLoader.load("auto-updates") - auto_updates_path = Hbc.appdir.join("MyFancyApp.app") - - expect(auto_updates).to be_installed - expect(auto_updates_path).to be_a_directory - expect(auto_updates.versions).to include("2.57") - - described_class.run("auto-updates", "--greedy") - - expect(auto_updates).to be_installed - expect(auto_updates_path).to be_a_directory - expect(auto_updates.versions).to include("2.61") + context "failed upgrade" do + let(:installed) { + [ + "outdated/bad-checksum", + "outdated/will-fail-if-upgraded", + ] + } - described_class.run("auto-updates", "--greedy") + before(:example) do + installed.each { |cask| Hbc::CLI::Install.run(cask) } - expect(auto_updates).to be_installed - expect(auto_updates_path).to be_a_directory - expect(auto_updates.versions).to include("2.61") + allow_any_instance_of(described_class).to receive(:verbose?).and_return(true) 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 .* @@ -198,7 +190,7 @@ expect(will_fail_if_upgraded.staged_path).to_not exist end - it "by not restoring the old Cask if the upgrade failed pre-install" do + it "does not restore the old Cask if the upgrade failed pre-install" do bad_checksum = Hbc::CaskLoader.load("bad-checksum") bad_checksum_path = Hbc.appdir.join("Caffeine.app") From 8abe60d2dc3f010112aa671a82e4dceb22c11e5b Mon Sep 17 00:00:00 2001 From: "L. E. Segovia" Date: Mon, 27 Nov 2017 10:15:13 +0000 Subject: [PATCH 15/15] Remove redundant --greedy --- Library/Homebrew/test/cask/cli/upgrade_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Library/Homebrew/test/cask/cli/upgrade_spec.rb b/Library/Homebrew/test/cask/cli/upgrade_spec.rb index a9959bf6f8d37..5f389d695019c 100644 --- a/Library/Homebrew/test/cask/cli/upgrade_spec.rb +++ b/Library/Homebrew/test/cask/cli/upgrade_spec.rb @@ -96,7 +96,7 @@ end describe "with --greedy it checks additional Casks" do - it 'includes the Casks with "auto_updates true" or "version latest" with --greedy' do + it 'includes the Casks with "auto_updates true" or "version latest"' do local_caffeine = Hbc::CaskLoader.load("local-caffeine") local_caffeine_path = Hbc.appdir.join("Caffeine.app") auto_updates = Hbc::CaskLoader.load("auto-updates")