Merge pull request #1435 from mistydemeo/dev_tools_version

Add "null version" class, and return compiler versions/build versions as Version objects

9 files changed, 134 insertions, 23 deletions

diff --git a/Library/Homebrew/compilers.rb b/Library/Homebrew/compilers.rb
index 6c3971984..fdfcab817 100644
--- a/Library/Homebrew/compilers.rb
+++ b/Library/Homebrew/compilers.rb
@@ -14,7 +14,14 @@ end
 
 class CompilerFailure
   attr_reader :name
-  attr_rw :version
+
+  def version(val = nil)
+    if val
+      @version = Version.parse(val.to_s)
+    else
+      @version
+    end
+  end
 
   # Allows Apple compiler `fails_with` statements to keep using `build`
   # even though `build` and `version` are the same internally
@@ -45,7 +52,7 @@ class CompilerFailure
 
   def initialize(name, version, &block)
     @name = name
-    @version = version
+    @version = Version.parse(version.to_s)
     instance_eval(&block) if block_given?
   end
 
@@ -115,13 +122,13 @@ class CompilerSelector
         GNU_GCC_VERSIONS.reverse_each do |v|
           name = "gcc-#{v}"
           version = compiler_version(name)
-          yield Compiler.new(name, version) if version
+          yield Compiler.new(name, version) unless version.null?
         end
       when :llvm
         next # no-op. DSL supported, compiler is not.
       else
         version = compiler_version(compiler)
-        yield Compiler.new(compiler, version) if version
+        yield Compiler.new(compiler, version) unless version.null?
       end
     end
   end
diff --git a/Library/Homebrew/development_tools.rb b/Library/Homebrew/development_tools.rb
index 3e1f16d2a..febe04b21 100644
--- a/Library/Homebrew/development_tools.rb
+++ b/Library/Homebrew/development_tools.rb
@@ -44,7 +44,9 @@ class DevelopmentTools
     def gcc_40_build_version
       @gcc_40_build_version ||=
         if (path = locate("gcc-4.0"))
-          `#{path} --version 2>/dev/null`[/build (\d{4,})/, 1].to_i
+          Version.new `#{path} --version 2>/dev/null`[/build (\d{4,})/, 1].to_i
+        else
+          Version::NULL
         end
     end
     alias gcc_4_0_build_version gcc_40_build_version
@@ -54,7 +56,9 @@ class DevelopmentTools
         begin
           gcc = locate("gcc-4.2") || HOMEBREW_PREFIX.join("opt/apple-gcc42/bin/gcc-4.2")
           if gcc.exist? && !gcc.realpath.basename.to_s.start_with?("llvm")
-            `#{gcc} --version 2>/dev/null`[/build (\d{4,})/, 1].to_i
+            Version.new `#{gcc} --version 2>/dev/null`[/build (\d{4,})/, 1]
+          else
+            Version::NULL
           end
         end
     end
@@ -63,14 +67,18 @@ class DevelopmentTools
     def clang_version
       @clang_version ||=
         if (path = locate("clang"))
-          `#{path} --version`[/(?:clang|LLVM) version (\d\.\d)/, 1]
+          Version.new `#{path} --version`[/(?:clang|LLVM) version (\d\.\d)/, 1]
+        else
+          Version::NULL
         end
     end
 
     def clang_build_version
       @clang_build_version ||=
         if (path = locate("clang"))
-          `#{path} --version`[/clang-(\d{2,})/, 1].to_i
+          Version.new `#{path} --version`[/clang-(\d{2,})/, 1]
+        else
+          Version::NULL
         end
     end
 
@@ -78,7 +86,11 @@ class DevelopmentTools
       (@non_apple_gcc_version ||= {}).fetch(cc) do
         path = HOMEBREW_PREFIX.join("opt", "gcc", "bin", cc)
         path = locate(cc) unless path.exist?
-        version = `#{path} --version`[/gcc(?:-\d(?:\.\d)? \(.+\))? (\d\.\d\.\d)/, 1] if path
+        version = if path
+          Version.new(`#{path} --version`[/gcc(?:-\d(?:\.\d)? \(.+\))? (\d\.\d\.\d)/, 1])
+        else
+          Version::NULL
+        end
         @non_apple_gcc_version[cc] = version
       end
     end
diff --git a/Library/Homebrew/resource.rb b/Library/Homebrew/resource.rb
index fe18f14dd..c0e9dbada 100644
--- a/Library/Homebrew/resource.rb
+++ b/Library/Homebrew/resource.rb
@@ -145,7 +145,10 @@ class Resource
   end
 
   def version(val = nil)
-    @version ||= detect_version(val)
+    @version ||= begin
+      version = detect_version(val)
+      version.null? ? nil : version
+    end
   end
 
   def mirror(val)
@@ -155,7 +158,7 @@ class Resource
   private
 
   def detect_version(val)
-    return if val.nil? && url.nil?
+    return Version::NULL if val.nil? && url.nil?
 
     case val
     when nil     then Version.detect(url, specs)
diff --git a/Library/Homebrew/system_config.rb b/Library/Homebrew/system_config.rb
index 555493c0d..9c7a8d1b0 100644
--- a/Library/Homebrew/system_config.rb
+++ b/Library/Homebrew/system_config.rb
@@ -143,9 +143,9 @@ class SystemConfig
       f.puts "HOMEBREW_BOTTLE_DOMAIN: #{BottleSpecification::DEFAULT_DOMAIN}"
       f.puts hardware if hardware
       f.puts "Homebrew Ruby: #{describe_homebrew_ruby}"
-      f.puts "GCC-4.0: build #{gcc_40}" if gcc_40
-      f.puts "GCC-4.2: build #{gcc_42}" if gcc_42
-      f.puts "Clang: #{clang ? "#{clang} build #{clang_build}" : "N/A"}"
+      f.puts "GCC-4.0: build #{gcc_40}" unless gcc_40.null?
+      f.puts "GCC-4.2: build #{gcc_42}" unless gcc_42.null?
+      f.puts "Clang: #{clang.null? ? "N/A" : "#{clang} build #{clang_build}"}"
       f.puts "Git: #{describe_git}"
       f.puts "Perl: #{describe_perl}"
       f.puts "Python: #{describe_python}"
diff --git a/Library/Homebrew/test/test_compiler_selector.rb b/Library/Homebrew/test/test_compiler_selector.rb
index 0363cacd2..b1591bdbe 100644
--- a/Library/Homebrew/test/test_compiler_selector.rb
+++ b/Library/Homebrew/test/test_compiler_selector.rb
@@ -15,15 +15,17 @@ class CompilerSelectorTests < Homebrew::TestCase
       :clang_build_version
 
     def initialize
-      @gcc_4_0_build_version = nil
-      @gcc_build_version = 5666
-      @clang_build_version = 425
+      @gcc_4_0_build_version = Version::NULL
+      @gcc_build_version = Version.create("5666")
+      @llvm_build_version = Version::NULL
+      @clang_build_version = Version.create("425")
     end
 
     def non_apple_gcc_version(name)
       case name
-      when "gcc-4.8" then "4.8.1"
-      when "gcc-4.7" then "4.7.1"
+      when "gcc-4.8" then Version.create("4.8.1")
+      when "gcc-4.7" then Version.create("4.7.1")
+      else Version::NULL
       end
     end
   end
@@ -101,13 +103,13 @@ class CompilerSelectorTests < Homebrew::TestCase
   end
 
   def test_missing_gcc
-    @versions.gcc_build_version = nil
+    @versions.gcc_build_version = Version::NULL
     @f << :clang << :llvm << { gcc: "4.8" } << { gcc: "4.7" }
     assert_raises(CompilerSelectionError) { actual_cc }
   end
 
   def test_missing_llvm_and_gcc
-    @versions.gcc_build_version = nil
+    @versions.gcc_build_version = Version::NULL
     @f << :clang << { gcc: "4.8" } << { gcc: "4.7" }
     assert_raises(CompilerSelectionError) { actual_cc }
   end
diff --git a/Library/Homebrew/test/test_versions.rb b/Library/Homebrew/test/test_versions.rb
index 21bf324a3..a6e922178 100644
--- a/Library/Homebrew/test/test_versions.rb
+++ b/Library/Homebrew/test/test_versions.rb
@@ -30,6 +30,29 @@ class VersionTokenTests < Homebrew::TestCase
   end
 end
 
+class NullVersionTests < Homebrew::TestCase
+  def test_null_version_is_always_smaller
+    assert_operator Version::NULL, :<, version("1")
+  end
+
+  def test_null_version_is_never_greater
+    refute_operator Version::NULL, :>, version("0")
+  end
+
+  def test_null_version_is_not_equal_to_itself
+    refute_eql Version::NULL, Version::NULL
+  end
+
+  def test_null_version_creates_an_empty_string
+    assert_eql "", Version::NULL.to_s
+  end
+
+  def test_null_version_produces_nan_as_a_float
+    # Float::NAN is not equal to itself so compare object IDs
+    assert_eql Float::NAN.object_id, Version::NULL.to_f.object_id
+  end
+end
+
 class VersionNullTokenTests < Homebrew::TestCase
   def test_inspect
     assert_equal "#<Version::NullToken>", Version::NullToken.new.inspect
@@ -122,6 +145,15 @@ class VersionComparisonTests < Homebrew::TestCase
     assert_operator version("2-p194"), :<, version("2.1-p195")
   end
 
+  def test_comparing_against_nil
+    assert_operator version("2.1.0-p194"), :>, nil
+  end
+
+  def test_comparing_against_strings
+    assert_operator version("2.1.0-p194"), :==, "2.1.0-p194"
+    assert_operator version("1"), :==, 1
+  end
+
   def test_comparison_returns_nil_for_non_version
     v = version("1.0")
     assert_nil v <=> Object.new
diff --git a/Library/Homebrew/test/testing_env.rb b/Library/Homebrew/test/testing_env.rb
index b6612535a..3fbf3e4b5 100644
--- a/Library/Homebrew/test/testing_env.rb
+++ b/Library/Homebrew/test/testing_env.rb
@@ -36,7 +36,7 @@ module Homebrew
     end
 
     def assert_version_nil(url)
-      assert_nil Version.parse(url)
+      assert Version.parse(url).null?
     end
   end
 
diff --git a/Library/Homebrew/version.rb b/Library/Homebrew/version.rb
index 60a833609..0f3cbb773 100644
--- a/Library/Homebrew/version.rb
+++ b/Library/Homebrew/version.rb
@@ -1,3 +1,5 @@
+require "version/null"
+
 class Version
   include Comparable
 
@@ -206,7 +208,18 @@ class Version
     false
   end
 
+  def null?
+    false
+  end
+
   def <=>(other)
+    # Needed to retain API compatibility with older string comparisons
+    # for compiler versions, etc.
+    other = Version.new(other) if other.is_a? String
+    # Used by the *_build_version comparisons, which formerly returned Fixnum
+    other = Version.new(other.to_s) if other.is_a? Integer
+    return 1 if other.nil?
+
     return unless other.is_a?(Version)
     return 0 if version == other.version
     return 1 if head? && !other.head?
@@ -247,6 +260,10 @@ class Version
     version.hash
   end
 
+  def to_f
+    version.to_f
+  end
+
   def to_s
     version.dup
   end
@@ -281,7 +298,7 @@ class Version
 
   def self.parse(spec)
     version = _parse(spec)
-    new(version) unless version.nil?
+    version.nil? ? NULL : new(version)
   end
 
   def self._parse(spec)
diff --git a/Library/Homebrew/version/null.rb b/Library/Homebrew/version/null.rb
new file mode 100644
index 000000000..77106bcce
--- /dev/null
+++ b/Library/Homebrew/version/null.rb
@@ -0,0 +1,38 @@
+class Version
+  NULL = Class.new do
+    include Comparable
+
+    def <=>(_other)
+      -1
+    end
+
+    def eql?(_other)
+      # Makes sure that the same instance of Version::NULL
+      # will never equal itself; normally Comparable#==
+      # will return true for this regardless of the return
+      # value of #<=>
+      false
+    end
+
+    def detected_from_url?
+      false
+    end
+
+    def head?
+      false
+    end
+
+    def null?
+      true
+    end
+
+    def to_f
+      Float::NAN
+    end
+
+    def to_s
+      ""
+    end
+    alias_method :to_str, :to_s
+  end.new
+end