From 006f25d8cfa5ffa9e4c531d49dcc99fe312edff6 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Sun, 4 Dec 2016 21:35:43 +0100 Subject: [PATCH 1/4] Use Homebrew to detect if X11 is installed. --- Library/Homebrew/cask/lib/hbc/installer.rb | 2 +- Library/Homebrew/cask/lib/hbc/locations.rb | 8 -------- Library/Homebrew/cask/test/cask/depends_on_test.rb | 5 +++-- 3 files changed, 4 insertions(+), 11 deletions(-) diff --git a/Library/Homebrew/cask/lib/hbc/installer.rb b/Library/Homebrew/cask/lib/hbc/installer.rb index 57efe97e95..afdaa2ec53 100644 --- a/Library/Homebrew/cask/lib/hbc/installer.rb +++ b/Library/Homebrew/cask/lib/hbc/installer.rb @@ -179,7 +179,7 @@ module Hbc def x11_dependencies return unless @cask.depends_on.x11 - raise CaskX11DependencyError, @cask.token if Hbc.x11_libpng.select(&:exist?).empty? + raise CaskX11DependencyError, @cask.token unless MacOS::X11.installed? end def formula_dependencies diff --git a/Library/Homebrew/cask/lib/hbc/locations.rb b/Library/Homebrew/cask/lib/hbc/locations.rb index 7a0bde1b06..f28e84b2ef 100644 --- a/Library/Homebrew/cask/lib/hbc/locations.rb +++ b/Library/Homebrew/cask/lib/hbc/locations.rb @@ -171,14 +171,6 @@ module Hbc def pre_mavericks_accessibility_dotfile @pre_mavericks_accessibility_dotfile ||= Pathname.new("/private/var/db/.AccessibilityAPIEnabled") end - - def x11_executable - @x11_executable ||= Pathname.new("/usr/X11/bin/X") - end - - def x11_libpng - @x11_libpng ||= [Pathname.new("/opt/X11/lib/libpng.dylib"), Pathname.new("/usr/X11/lib/libpng.dylib")] - end end end end diff --git a/Library/Homebrew/cask/test/cask/depends_on_test.rb b/Library/Homebrew/cask/test/cask/depends_on_test.rb index 31e51b5e5e..ce2e541783 100644 --- a/Library/Homebrew/cask/test/cask/depends_on_test.rb +++ b/Library/Homebrew/cask/test/cask/depends_on_test.rb @@ -93,6 +93,7 @@ describe "Satisfy Dependencies and Requirements" do describe "depends_on x11" do it "succeeds when depends_on x11 is satisfied" do x11_cask = Hbc.load("with-depends-on-x11") + MacOS::X11.stubs(:installed?).returns(true) shutup do Hbc::Installer.new(x11_cask).install end @@ -100,7 +101,7 @@ describe "Satisfy Dependencies and Requirements" do it "raises an exception when depends_on x11 is not satisfied" do x11_cask = Hbc.load("with-depends-on-x11") - Hbc.stubs(:x11_libpng).returns([Pathname.new("/usr/path/does/not/exist")]) + MacOS::X11.stubs(:installed?).returns(false) lambda { shutup do Hbc::Installer.new(x11_cask).install @@ -110,7 +111,7 @@ describe "Satisfy Dependencies and Requirements" do it "never raises when depends_on x11: false" do x11_cask = Hbc.load("with-depends-on-x11-false") - Hbc.stubs(:x11_executable).returns(Pathname.new("/usr/path/does/not/exist")) + MacOS::X11.stubs(:installed?).returns(false) lambda do shutup do Hbc::Installer.new(x11_cask).install From 7d7ca0cb1a7d83fe60ec8b7a0598e8f29d2c526d Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Sun, 4 Dec 2016 21:52:30 +0100 Subject: [PATCH 2/4] =?UTF-8?q?Use=20Homebrew=E2=80=99s=20`Emoji`=20class?= =?UTF-8?q?=20in=20cask=20installer.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Library/Homebrew/cask/lib/hbc/installer.rb | 7 ++----- Library/Homebrew/cask/test/cask/cli/install_test.rb | 2 +- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/Library/Homebrew/cask/lib/hbc/installer.rb b/Library/Homebrew/cask/lib/hbc/installer.rb index afdaa2ec53..90e5b1e01d 100644 --- a/Library/Homebrew/cask/lib/hbc/installer.rb +++ b/Library/Homebrew/cask/lib/hbc/installer.rb @@ -82,11 +82,8 @@ module Hbc end def summary - s = if MacOS.version >= :lion && !ENV["HOMEBREW_NO_EMOJI"] - (ENV["HOMEBREW_INSTALL_BADGE"] || "\xf0\x9f\x8d\xba") + " " - else - Formatter.headline("Success! ", color: :blue) - end + s = "" + s << "#{Emoji.install_badge} " if Emoji.enabled? s << "#{@cask} was successfully installed!" end diff --git a/Library/Homebrew/cask/test/cask/cli/install_test.rb b/Library/Homebrew/cask/test/cask/cli/install_test.rb index c774f1fb58..eef3f2e5b0 100644 --- a/Library/Homebrew/cask/test/cask/cli/install_test.rb +++ b/Library/Homebrew/cask/test/cask/cli/install_test.rb @@ -39,7 +39,7 @@ describe Hbc::CLI::Install do lambda { Hbc::CLI::Install.run("local-transmission", "--force") - }.must_output(/==> Success! local-transmission was successfully installed!/) + }.must_output(/local-transmission was successfully installed!/) end it "skips dependencies with --skip-cask-deps" do From e6d9248787b26008b86ecb4dfddba7fb15ba98a5 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Sun, 4 Dec 2016 23:13:39 +0100 Subject: [PATCH 3/4] Separate `fetch` and `stage` steps in `Hbc::Installer`. --- Library/Homebrew/cask/lib/hbc/installer.rb | 50 ++++++++++++++-------- 1 file changed, 32 insertions(+), 18 deletions(-) diff --git a/Library/Homebrew/cask/lib/hbc/installer.rb b/Library/Homebrew/cask/lib/hbc/installer.rb index 90e5b1e01d..df7554cc51 100644 --- a/Library/Homebrew/cask/lib/hbc/installer.rb +++ b/Library/Homebrew/cask/lib/hbc/installer.rb @@ -54,8 +54,27 @@ module Hbc output end + def fetch + odebug "Hbc::Installer#fetch" + + satisfy_dependencies + verify_has_sha if @require_sha && !@force + download + verify + end + + def stage + odebug "Hbc::Installer#stage" + + extract_primary_container + save_caskfile + rescue StandardError => e + purge_versioned_files + raise e + end + def install - odebug "Hbc::Installer.install" + odebug "Hbc::Installer#install" if @cask.installed? && !force raise CaskAlreadyInstalledAutoUpdatesError, @cask if @cask.auto_updates @@ -63,20 +82,10 @@ module Hbc end print_caveats - - begin - satisfy_dependencies - verify_has_sha if @require_sha && !@force - download - verify - extract_primary_container - install_artifacts - save_caskfile - enable_accessibility_access - rescue StandardError => e - purge_versioned_files - raise e - end + fetch + stage + install_artifacts + enable_accessibility_access puts summary end @@ -89,8 +98,7 @@ module Hbc def download odebug "Downloading" - download = Download.new(@cask, force: false) - @downloaded_path = download.perform + @downloaded_path = Download.new(@cask, force: false).perform odebug "Downloaded to -> #{@downloaded_path}" @downloaded_path end @@ -107,15 +115,18 @@ module Hbc def extract_primary_container odebug "Extracting primary container" + FileUtils.mkdir_p @cask.staged_path container = if @cask.container && @cask.container.type Container.from_type(@cask.container.type) else Container.for_path(@downloaded_path, @command) end + unless container raise CaskError, "Uh oh, could not figure out how to unpack '#{@downloaded_path}'" end + odebug "Using container class #{container} for #{@downloaded_path}" container.new(@cask, @downloaded_path, @command).extract end @@ -245,6 +256,9 @@ module Hbc See System Preferences to enable it manually. EOS end + rescue StandardError => e + purge_versioned_files + raise e end def disable_accessibility_access @@ -279,7 +293,7 @@ module Hbc end def uninstall - odebug "Hbc::Installer.uninstall" + odebug "Hbc::Installer#uninstall" disable_accessibility_access uninstall_artifacts purge_versioned_files From 5785f54f4bd48dca58a7550af48217d4c6372ad2 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Sun, 4 Dec 2016 23:14:35 +0100 Subject: [PATCH 4/4] Revert `install_artifacts` if one artifact fails to install. --- Library/Homebrew/cask/lib/hbc/installer.rb | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/Library/Homebrew/cask/lib/hbc/installer.rb b/Library/Homebrew/cask/lib/hbc/installer.rb index df7554cc51..5176143d7a 100644 --- a/Library/Homebrew/cask/lib/hbc/installer.rb +++ b/Library/Homebrew/cask/lib/hbc/installer.rb @@ -132,14 +132,29 @@ module Hbc end def install_artifacts + already_installed_artifacts = [] + options = { command: @command, force: force } + odebug "Installing artifacts" artifacts = Artifact.for_cask(@cask) odebug "#{artifacts.length} artifact/s defined", artifacts + artifacts.each do |artifact| odebug "Installing artifact of class #{artifact}" - options = { command: @command, force: force } + already_installed_artifacts.unshift(artifact) artifact.new(@cask, options).install_phase end + rescue StandardError => e + begin + ofail e.message + already_installed_artifacts.each do |artifact| + odebug "Reverting installation of artifact of class #{artifact}" + artifact.new(@cask, options).uninstall_phase + end + ensure + purge_versioned_files + raise e.class, "An error occured during installation of Cask #{@cask}: #{e.message}" + end end # TODO: move dependencies to a separate class