Merge pull request #3422 from MikeMcQuaid/null-versions

mac/xcode: compare with existing Version class.

8 files changed, 68 insertions, 54 deletions

diff --git a/Library/Homebrew/compat/macos.rb b/Library/Homebrew/compat/macos.rb
index 3acd2f9ef..1f2c651c5 100644
--- a/Library/Homebrew/compat/macos.rb
+++ b/Library/Homebrew/compat/macos.rb
@@ -1,5 +1,10 @@
 require "development_tools"
 
+if OS.mac?
+  MACOS_FULL_VERSION = OS::Mac.full_version.to_s.freeze
+  MACOS_VERSION = OS::Mac.version.to_s.freeze
+end
+
 module OS
   module Mac
     module_function
diff --git a/Library/Homebrew/extend/os/mac/diagnostic.rb b/Library/Homebrew/extend/os/mac/diagnostic.rb
index be0ce9351..86abfab87 100644
--- a/Library/Homebrew/extend/os/mac/diagnostic.rb
+++ b/Library/Homebrew/extend/os/mac/diagnostic.rb
@@ -50,7 +50,6 @@ module Homebrew
       end
 
       def check_xcode_up_to_date
-        return unless MacOS::Xcode.installed?
         return unless MacOS::Xcode.outdated?
 
         # Travis CI images are going to end up outdated so don't complain when
@@ -78,7 +77,6 @@ module Homebrew
       end
 
       def check_clt_up_to_date
-        return unless MacOS::CLT.installed?
         return unless MacOS::CLT.outdated?
 
         # Travis CI images are going to end up outdated so don't complain when
@@ -108,7 +106,6 @@ module Homebrew
       end
 
       def check_xcode_minimum_version
-        return unless MacOS::Xcode.installed?
         return unless MacOS::Xcode.below_minimum_version?
 
         <<~EOS
@@ -119,7 +116,6 @@ module Homebrew
       end
 
       def check_clt_minimum_version
-        return unless MacOS::CLT.installed?
         return unless MacOS::CLT.below_minimum_version?
 
         <<~EOS
@@ -281,13 +277,8 @@ module Homebrew
         EOS
       end
 
-      def check_for_latest_xquartz
-        return unless MacOS::XQuartz.version
-        return if MacOS::XQuartz.provided_by_apple?
-
-        installed_version = Version.create(MacOS::XQuartz.version)
-        latest_version = Version.create(MacOS::XQuartz.latest_version)
-        return if installed_version >= latest_version
+      def check_xquartz_up_to_date
+        return unless MacOS::XQuartz.outdated?
 
         <<~EOS
           Your XQuartz (#{installed_version}) is outdated.
@@ -298,8 +289,7 @@ module Homebrew
       end
 
       def check_for_beta_xquartz
-        return unless MacOS::XQuartz.version
-        return unless MacOS::XQuartz.version.include? "beta"
+        return unless MacOS::XQuartz.version.to_s.include?("beta")
 
         <<~EOS
           The following beta release of XQuartz is installed: #{MacOS::XQuartz.version}
diff --git a/Library/Homebrew/extend/os/mac/extend/ENV/super.rb b/Library/Homebrew/extend/os/mac/extend/ENV/super.rb
index 5872c2264..ee0d2343c 100644
--- a/Library/Homebrew/extend/os/mac/extend/ENV/super.rb
+++ b/Library/Homebrew/extend/os/mac/extend/ENV/super.rb
@@ -91,18 +91,18 @@ module Superenv
     generic_setup_build_environment(formula)
     self["HOMEBREW_SDKROOT"] = effective_sysroot
 
-    if MacOS::Xcode.without_clt? || (MacOS::Xcode.installed? && MacOS::Xcode.version.to_i >= 7)
+    if MacOS::Xcode.without_clt? || MacOS::Xcode.version.to_i >= 7
       self["MACOSX_DEPLOYMENT_TARGET"] = MacOS.version.to_s
       self["SDKROOT"] = MacOS.sdk_path
     end
 
     # Filter out symbols known not to be defined since GNU Autotools can't
     # reliably figure this out with Xcode 8 and above.
-    if MacOS.version == "10.12" && MacOS::Xcode.installed? && MacOS::Xcode.version >= "9.0"
+    if MacOS.version == "10.12" && MacOS::Xcode.version >= "9.0"
       %w[fmemopen futimens open_memstream utimensat].each do |s|
         ENV["ac_cv_func_#{s}"] = "no"
       end
-    elsif MacOS.version == "10.11" && MacOS::Xcode.installed? && MacOS::Xcode.version >= "8.0"
+    elsif MacOS.version == "10.11" && MacOS::Xcode.version >= "8.0"
       %w[basename_r clock_getres clock_gettime clock_settime dirname_r
          getentropy mkostemp mkostemps timingsafe_bcmp].each do |s|
         ENV["ac_cv_func_#{s}"] = "no"
diff --git a/Library/Homebrew/extend/os/mac/system_config.rb b/Library/Homebrew/extend/os/mac/system_config.rb
index acee50ad0..796134781 100644
--- a/Library/Homebrew/extend/os/mac/system_config.rb
+++ b/Library/Homebrew/extend/os/mac/system_config.rb
@@ -4,7 +4,7 @@ class SystemConfig
       if instance_variable_defined?(:@xcode)
         @xcode
       elsif MacOS::Xcode.installed?
-        @xcode = MacOS::Xcode.version
+        @xcode = MacOS::Xcode.version.to_s
         @xcode += " => #{MacOS::Xcode.prefix}" unless MacOS::Xcode.default_prefix?
         @xcode
       end
diff --git a/Library/Homebrew/os.rb b/Library/Homebrew/os.rb
index 2867d8864..d35a49dd7 100644
--- a/Library/Homebrew/os.rb
+++ b/Library/Homebrew/os.rb
@@ -20,9 +20,6 @@ module OS
       ISSUES_URL = "https://docs.brew.sh/Troubleshooting.html".freeze
     end
     PATH_OPEN = "/usr/bin/open".freeze
-    # compatibility
-    ::MACOS_FULL_VERSION = OS::Mac.full_version.to_s.freeze
-    ::MACOS_VERSION = OS::Mac.version.to_s.freeze
   elsif OS.linux?
     ISSUES_URL = "https://github.com/Linuxbrew/brew/wiki/troubleshooting".freeze
     PATH_OPEN = "xdg-open".freeze
diff --git a/Library/Homebrew/os/mac/xcode.rb b/Library/Homebrew/os/mac/xcode.rb
index 81149fb4c..4382b505c 100644
--- a/Library/Homebrew/os/mac/xcode.rb
+++ b/Library/Homebrew/os/mac/xcode.rb
@@ -36,15 +36,17 @@ module OS
       end
 
       def below_minimum_version?
+        return false unless installed?
         version < minimum_version
       end
 
       def outdated?
-        Version.new(version) < latest_version
+        return false unless installed?
+        version < latest_version
       end
 
       def without_clt?
-        installed? && Version.new(version) >= "4.3" && !MacOS::CLT.installed?
+        version >= "4.3" && !MacOS::CLT.installed?
       end
 
       # Returns a Pathname object corresponding to Xcode.app's Developer
@@ -65,8 +67,7 @@ module OS
       end
 
       def toolchain_path
-        return unless installed?
-        return if Version.new(version) < "4.3"
+        return if version < "4.3"
         Pathname.new("#{prefix}/Toolchains/XcodeDefault.xctoolchain")
       end
 
@@ -101,16 +102,17 @@ module OS
         # may return a version string
         # that is guessed based on the compiler, so do not
         # use it in order to check if Xcode is installed.
-        @version ||= uncached_version
+        if @version ||= detect_version
+          ::Version.new @version
+        else
+          ::Version::NULL
+        end
       end
 
-      def uncached_version
+      def detect_version
         # This is a separate function as you can't cache the value out of a block
         # if return is used in the middle, which we do many times in here.
-
-        return "0" unless OS.mac?
-
-        return nil if !MacOS::Xcode.installed? && !MacOS::CLT.installed?
+        return if !MacOS::Xcode.installed? && !MacOS::CLT.installed?
 
         %W[
           #{prefix}/usr/bin/xcodebuild
@@ -160,26 +162,20 @@ module OS
       end
 
       def provides_gcc?
-        installed? && Version.new(version) < "4.3"
+        version < "4.3"
       end
 
       def provides_cvs?
-        installed? && Version.new(version) < "5.0"
+        version < "5.0"
       end
 
       def default_prefix?
-        if Version.new(version) < "4.3"
+        if version < "4.3"
           prefix.to_s.start_with? "/Developer"
         else
           prefix.to_s == "/Applications/Xcode.app/Contents/Developer"
         end
       end
-
-      class Version < ::Version
-        def <=>(other)
-          super(Version.new(other))
-        end
-      end
     end
 
     module CLT
@@ -194,7 +190,7 @@ module OS
       # Returns true even if outdated tools are installed, e.g.
       # tools from Xcode 4.x on 10.9
       def installed?
-        !detect_version.nil?
+        !version.null?
       end
 
       def update_instructions
@@ -238,43 +234,55 @@ module OS
       def below_minimum_version?
         # Lion was the first version of OS X to ship with a CLT
         return false if MacOS.version < :lion
+        return false unless installed?
         version < minimum_version
       end
 
       def outdated?
+        clang_version = detect_clang_version
+        return false unless clang_version
+        ::Version.new(clang_version) < latest_version
+      end
+
+      def detect_clang_version
         # Lion was the first version of OS X to ship with a CLT
-        return false if MacOS.version < :lion
+        return if MacOS.version < :lion
 
-        if MacOS.version >= :mavericks
-          version = Utils.popen_read("#{PKG_PATH}/usr/bin/clang --version")
+        path = if MacOS.version >= :mavericks
+          "#{PKG_PATH}/usr/bin/clang"
         else
-          version = Utils.popen_read("/usr/bin/clang --version")
+          "/usr/bin/clang"
         end
-        version = version[/clang-(\d+\.\d+\.\d+(\.\d+)?)/, 1] || "0"
-        Xcode::Version.new(version) < latest_version
+
+        version_output = Utils.popen_read("#{path} --version")
+        version_output[/clang-(\d+\.\d+\.\d+(\.\d+)?)/, 1]
       end
 
       # Version string (a pretty long one) of the CLT package.
       # Note, that different ways to install the CLTs lead to different
       # version numbers.
       def version
-        @version ||= detect_version
+        if @version ||= detect_version
+          ::Version.new @version
+        else
+          ::Version::NULL
+        end
       end
 
       def detect_version
         # CLT isn't a distinct entity pre-4.3, and pkgutil doesn't exist
         # at all on Tiger, so just count it as installed if Xcode is installed
-        if MacOS::Xcode.installed? && Xcode::Version.new(MacOS::Xcode.version) < "3.0"
-          return MacOS::Xcode.version
-        end
+        return MacOS::Xcode.version if MacOS::Xcode.version < "3.0"
 
-        [MAVERICKS_PKG_ID, MAVERICKS_NEW_PKG_ID, STANDALONE_PKG_ID, FROM_XCODE_PKG_ID].find do |id|
+        version = nil
+        [MAVERICKS_PKG_ID, MAVERICKS_NEW_PKG_ID, STANDALONE_PKG_ID, FROM_XCODE_PKG_ID].each do |id|
           if MacOS.version >= :mavericks
             next unless File.exist?("#{PKG_PATH}/usr/bin/clang")
           end
           version = MacOS.pkgutil_info(id)[/version: (.+)$/, 1]
-          return version if version
+          break if version
         end
+        version
       end
     end
   end
diff --git a/Library/Homebrew/os/mac/xquartz.rb b/Library/Homebrew/os/mac/xquartz.rb
index ddb2aa4fa..efbb5b738 100644
--- a/Library/Homebrew/os/mac/xquartz.rb
+++ b/Library/Homebrew/os/mac/xquartz.rb
@@ -34,7 +34,11 @@ module OS
       # The X11.app distributed by Apple is also XQuartz, and therefore covered
       # by this method.
       def version
-        @version ||= detect_version
+        if @version ||= detect_version
+          ::Version.new @version
+        else
+          ::Version::NULL
+        end
       end
 
       def detect_version
@@ -115,7 +119,13 @@ module OS
       end
 
       def installed?
-        !version.nil? && !prefix.nil?
+        !version.null? && !prefix.nil?
+      end
+
+      def outdated?
+        return false unless installed?
+        return false if provided_by_apple?
+        version < latest_version
       end
 
       # If XQuartz and/or the CLT are installed, headers will be found under
diff --git a/Library/Homebrew/version.rb b/Library/Homebrew/version.rb
index d43e0c665..c2635f18c 100644
--- a/Library/Homebrew/version.rb
+++ b/Library/Homebrew/version.rb
@@ -431,6 +431,10 @@ class Version
     version.to_f
   end
 
+  def to_i
+    version.to_i
+  end
+
   def to_s
     version.dup
   end