Merge pull request #3631 from amyspark/doctor

Let cask doctor exit with 1

1 files changed, 116 insertions, 7 deletions

diff --git a/Library/Homebrew/cask/lib/hbc/cli/doctor.rb b/Library/Homebrew/cask/lib/hbc/cli/doctor.rb
index 5aef2c420..a586a00ab 100644
--- a/Library/Homebrew/cask/lib/hbc/cli/doctor.rb
+++ b/Library/Homebrew/cask/lib/hbc/cli/doctor.rb
@@ -1,25 +1,115 @@
 require "system_config"
+require "hbc/checkable"
 
 module Hbc
   class CLI
     class Doctor < AbstractCommand
+      include Checkable
+
       def initialize(*)
         super
         return if args.empty?
         raise ArgumentError, "#{self.class.command_name} does not take arguments."
       end
 
+      def success?
+        !(errors? || warnings?)
+      end
+
+      def summary_header
+        "Cask's Doctor Checkup"
+      end
+
       def run
+        check_software_versions
+        check_install_location
+        check_staging_location
+        check_taps
+        check_load_path
+        check_environment_variables
+
+        puts summary unless success?
+        raise CaskError, "There are some problems with your setup." unless success?
+      end
+
+      def check_software_versions
         ohai "Homebrew-Cask Version", Hbc.full_version
         ohai "macOS", MacOS.full_version
         ohai "SIP", self.class.check_sip
         ohai "Java", SystemConfig.describe_java
-        ohai "Homebrew-Cask Install Location", self.class.render_install_location
-        ohai "Homebrew-Cask Staging Location", self.class.render_staging_location(Hbc.caskroom)
-        ohai "Homebrew-Cask Cached Downloads", self.class.render_cached_downloads
+      end
+
+      # This could be done by calling into Homebrew, but the situation
+      # where "doctor" is needed is precisely the situation where such
+      # things are less dependable.
+      def check_install_location
+        ohai "Homebrew-Cask Install Location"
+
+        locations = Dir.glob(HOMEBREW_CELLAR.join("brew-cask", "*")).reverse
+        if locations.empty?
+          puts self.class.none_string
+        else
+          locations.collect do |l|
+            add_error "Legacy install at #{l}. Run \"brew uninstall --force brew-cask\"."
+            puts l
+          end
+        end
+      end
+
+      def check_staging_location
+        ohai "Homebrew-Cask Staging Location"
+
+        path = Pathname.new(user_tilde(Hbc.caskroom.to_s))
+
+        if !path.exist?
+          add_error "The staging path #{path} does not exist."
+        elsif !path.writable?
+          add_error "The staging path #{path} is not writable by the current user."
+        end
+
+        puts path
+      end
+
+      def check_cached_downloads
+        ohai "Homebrew-Cask Cached Downloads"
+
+        cleanup = CLI::Cleanup.new
+        count = cleanup.cache_files.count
+        size = cleanup.disk_cleanup_size
+        msg = user_tilde(Hbc.cache.to_s)
+        msg << " (#{number_readable(count)} files, #{disk_usage_readable(size)})" unless count.zero?
+        puts msg
+      end
+
+      def check_taps
         ohai "Homebrew-Cask Taps:"
-        puts self.class.render_taps(Hbc.default_tap, *self.class.alt_taps)
-        ohai "Contents of $LOAD_PATH", self.class.render_load_path($LOAD_PATH)
+
+        default_tap = [Hbc.default_tap]
+
+        alt_taps = Tap.select { |t| t.cask_dir.exist? && t != Hbc.default_tap }
+
+        (default_tap + alt_taps).each do |tap|
+          if tap.path.nil? || tap.path.to_s.empty?
+            puts none_string
+          else
+            puts "#{tap.path} (#{cask_count_for_tap(tap)})"
+          end
+        end
+      end
+
+      def check_load_path
+        ohai "Contents of $LOAD_PATH"
+        paths = $LOAD_PATH.map(&method(:user_tilde))
+
+        if paths.empty?
+          puts none_string
+          add_error "$LOAD_PATH is empty"
+        else
+          puts paths
+        end
+      end
+
+      def check_environment_variables
         ohai "Environment Variables"
 
         environment_variables = %w[
@@ -35,7 +125,25 @@ module Hbc
           SHELL
         ]
 
-        (self.class.locale_variables + environment_variables).sort.each(&self.class.method(:render_env_var))
+        locale_variables = ENV.keys.grep(/^(?:LC_\S+|LANG|LANGUAGE)\Z/).sort
+
+        (locale_variables + environment_variables).sort.each(&method(:render_env_var))
+      end
+
+      def user_tilde(path)
+        self.class.user_tilde(path)
+      end
+
+      def cask_count_for_tap(tap)
+        self.class.cask_count_for_tap(tap)
+      end
+
+      def none_string
+        self.class.none_string
+      end
+
+      def render_env_var(var)
+        self.class.render_env_var(var)
       end
 
       def self.check_sip
@@ -71,7 +179,8 @@ module Hbc
       def self.cask_count_for_tap(tap)
         Formatter.pluralize(tap.cask_files.count, "cask")
       rescue StandardError
-        "0 #{error_string "error reading #{tap.path}"}"
+        add_error "Unable to read from Tap: #{tap.path}"
+        "0"
       end
 
       def self.render_taps(*taps)