From 9876d1733619e37d37e51d4d6229257bfc142bd1 Mon Sep 17 00:00:00 2001 From: Claudia Date: Fri, 20 May 2016 17:32:10 +0200 Subject: [PATCH] Refactor: add options, ivars to `Installer`, `Download` In preparation for upcoming changes, this commit cleans up some code. The commit includes: - In order to reduce unnecessary object passing, make both the `force` and `skip_cask_deps` option into instance variables of the `Installer` class - Introduce options hashes to initializers of both the `Installer` and `Download` class - When the `install --force` command enters the fetch phase, make it explicit in the code that fetching is never enforced in that case. - Update tests --- lib/hbc/cli/fetch.rb | 2 +- lib/hbc/cli/install.rb | 2 +- lib/hbc/cli/uninstall.rb | 6 ++++-- lib/hbc/download.rb | 4 ++-- lib/hbc/installer.rb | 25 +++++++++++++++---------- test/cask/accessibility_test.rb | 3 ++- test/cask/installer_test.rb | 17 ++++++++--------- 7 files changed, 33 insertions(+), 26 deletions(-) diff --git a/lib/hbc/cli/fetch.rb b/lib/hbc/cli/fetch.rb index d64552ef7f02..8a46f9d7dd5b 100644 --- a/lib/hbc/cli/fetch.rb +++ b/lib/hbc/cli/fetch.rb @@ -7,7 +7,7 @@ def self.run(*args) cask_tokens.each do |cask_token| ohai "Downloading external files for Cask #{cask_token}" cask = Hbc.load(cask_token) - downloaded_path = Hbc::Download.new(cask, force).perform + downloaded_path = Hbc::Download.new(cask, force: force).perform Hbc::Verify.all(cask, downloaded_path) ohai "Success! Downloaded to -> #{downloaded_path}" end diff --git a/lib/hbc/cli/install.rb b/lib/hbc/cli/install.rb index b54494e706ec..9428bf39ded7 100644 --- a/lib/hbc/cli/install.rb +++ b/lib/hbc/cli/install.rb @@ -17,7 +17,7 @@ def self.install_casks(cask_tokens, force) cask_tokens.each do |cask_token| begin cask = Hbc.load(cask_token) - Hbc::Installer.new(cask).install(force) + Hbc::Installer.new(cask, force: force).install count += 1 rescue Hbc::CaskAlreadyInstalledError => e opoo e.message diff --git a/lib/hbc/cli/uninstall.rb b/lib/hbc/cli/uninstall.rb index bef08b2ad72b..6f0523a86937 100644 --- a/lib/hbc/cli/uninstall.rb +++ b/lib/hbc/cli/uninstall.rb @@ -6,8 +6,10 @@ def self.run(*args) cask_tokens.each do |cask_token| odebug "Uninstalling Cask #{cask_token}" cask = Hbc.load(cask_token) - raise Hbc::CaskNotInstalledError.new(cask) unless cask.installed? or force - Hbc::Installer.new(cask).uninstall(force) + unless cask.installed? || force + raise Hbc::CaskNotInstalledError.new(cask) + end + Hbc::Installer.new(cask, force: force).uninstall end end diff --git a/lib/hbc/download.rb b/lib/hbc/download.rb index 7a618a50e828..1769f268cec2 100644 --- a/lib/hbc/download.rb +++ b/lib/hbc/download.rb @@ -4,9 +4,9 @@ class Hbc::Download attr_reader :cask - def initialize(cask, force=false) + def initialize(cask, options={}) @cask = cask - @force = force + @force = options.fetch(:force, false) end def perform diff --git a/lib/hbc/installer.rb b/lib/hbc/installer.rb index 38d6343abd03..3b70ac1754f0 100644 --- a/lib/hbc/installer.rb +++ b/lib/hbc/installer.rb @@ -14,11 +14,15 @@ class Hbc::Installer include Hbc::Staged include Hbc::Verify + attr_reader :force, :skip_cask_deps + PERSISTENT_METADATA_SUBDIRS = [ 'gpg' ] - def initialize(cask, command=Hbc::SystemCommand) + def initialize(cask, options={}) @cask = cask - @command = command + @command = options.fetch(:command, Hbc::SystemCommand) + @force = options.fetch(:force, false) + @skip_cask_deps = options.fetch(:skip_cask_deps, false) end def self.print_caveats(cask) @@ -50,7 +54,7 @@ def self.capture_output(&block) output end - def install(force=false, skip_cask_deps=false) + def install odebug "Hbc::Installer.install" if @cask.installed? && @cask.auto_updates && !force @@ -64,12 +68,12 @@ def install(force=false, skip_cask_deps=false) print_caveats begin - satisfy_dependencies(skip_cask_deps) + satisfy_dependencies download verify extract_primary_container install_artifacts - save_caskfile force + save_caskfile enable_accessibility_access rescue StandardError => e purge_versioned_files @@ -90,7 +94,7 @@ def summary def download odebug "Downloading" - download = Hbc::Download.new(@cask) + download = Hbc::Download.new(@cask, force: false) @downloaded_path = download.perform odebug "Downloaded to -> #{@downloaded_path}" @downloaded_path @@ -128,7 +132,7 @@ def install_artifacts # todo move dependencies to a separate class # dependencies should also apply for "brew cask stage" # override dependencies with --force or perhaps --force-deps - def satisfy_dependencies(skip_cask_deps=false) + def satisfy_dependencies if @cask.depends_on ohai 'Satisfying dependencies' macos_dependencies @@ -205,7 +209,8 @@ def cask_dependencies if dep.installed? puts "already installed" else - Hbc::Installer.new(dep).install(false, true) + Hbc::Installer.new(dep, + force: false, skip_cask_deps: true).install puts "done" end end @@ -258,7 +263,7 @@ def disable_accessibility_access end end - def save_caskfile(force=false) + def save_caskfile timestamp = :now create = true savedir = @cask.metadata_subdir('Casks', timestamp, create) @@ -274,7 +279,7 @@ def save_caskfile(force=false) FileUtils.copy(@cask.sourcefile_path, savedir) if @cask.sourcefile_path end - def uninstall(force=false) + def uninstall odebug "Hbc::Installer.uninstall" disable_accessibility_access uninstall_artifacts diff --git a/test/cask/accessibility_test.rb b/test/cask/accessibility_test.rb index 7b3d9823be7d..cfe769c65610 100644 --- a/test/cask/accessibility_test.rb +++ b/test/cask/accessibility_test.rb @@ -5,7 +5,8 @@ describe "Accessibility Access" do before do cask = Hbc.load('with-accessibility-access') - @installer = Hbc::Installer.new(cask, Hbc::FakeSystemCommand) + with_fake_command = { command: Hbc::FakeSystemCommand } + @installer = Hbc::Installer.new(cask, with_fake_command) end describe "install" do diff --git a/test/cask/installer_test.rb b/test/cask/installer_test.rb index 66dfb8804d9e..baa8065bb639 100644 --- a/test/cask/installer_test.rb +++ b/test/cask/installer_test.rb @@ -220,11 +220,11 @@ it "allows already-installed Casks which auto-update to be installed if force is provided" do auto_updates = Hbc.load('auto-updates') auto_updates.installed?.must_equal false - installer = Hbc::Installer.new(auto_updates) - shutup { installer.install } + shutup { Hbc::Installer.new(auto_updates).install } + shutup { - installer.install(true) + Hbc::Installer.new(auto_updates, force: true).install } # wont_raise end @@ -243,11 +243,11 @@ it "allows already-installed Casks to be installed if force is provided" do transmission = Hbc.load('local-transmission') transmission.installed?.must_equal false - installer = Hbc::Installer.new(transmission) - shutup { installer.install } + shutup { Hbc::Installer.new(transmission).install } + shutup { - installer.install(true) + Hbc::Installer.new(transmission, force: true).install } # wont_raise end @@ -329,11 +329,10 @@ it "uninstalls all versions if force is set" do caffeine = Hbc.load('local-caffeine') - installer = Hbc::Installer.new(caffeine) mutated_version = caffeine.version + '.1' shutup do - installer.install + Hbc::Installer.new(caffeine).install end Hbc.caskroom.join('local-caffeine',caffeine.version).must_be :directory? @@ -343,7 +342,7 @@ Hbc.caskroom.join('local-caffeine',mutated_version).must_be :directory? shutup do - installer.uninstall(true) + Hbc::Installer.new(caffeine, force: true).uninstall end Hbc.caskroom.join('local-caffeine',caffeine.version).wont_be :directory?