From 9bac107b31f05cca65d8aab23be05e1b867bd289 Mon Sep 17 00:00:00 2001 From: Misty De Meo Date: Thu, 3 Nov 2016 16:28:16 -0700 Subject: [PATCH 1/9] Add Version::NULL singleton --- Library/Homebrew/test/test_versions.rb | 23 ++++++++++++++++ Library/Homebrew/version.rb | 6 ++++ Library/Homebrew/version/null.rb | 38 ++++++++++++++++++++++++++ 3 files changed, 67 insertions(+) create mode 100644 Library/Homebrew/version/null.rb diff --git a/Library/Homebrew/test/test_versions.rb b/Library/Homebrew/test/test_versions.rb index 21bf324a38..6f85fe7a06 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.new.inspect diff --git a/Library/Homebrew/version.rb b/Library/Homebrew/version.rb index 60a8336095..433964dc70 100644 --- a/Library/Homebrew/version.rb +++ b/Library/Homebrew/version.rb @@ -1,3 +1,5 @@ +require "version/null" + class Version include Comparable @@ -206,6 +208,10 @@ class Version false end + def null? + false + end + def <=>(other) return unless other.is_a?(Version) return 0 if version == other.version diff --git a/Library/Homebrew/version/null.rb b/Library/Homebrew/version/null.rb new file mode 100644 index 0000000000..77106bcce2 --- /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 From fbcf500a48bd8a6be2f009c35be5ae971e841b87 Mon Sep 17 00:00:00 2001 From: Misty De Meo Date: Thu, 3 Nov 2016 16:36:20 -0700 Subject: [PATCH 2/9] Version.parse: return Version::NULL for unparseable strings --- Library/Homebrew/test/testing_env.rb | 2 +- Library/Homebrew/version.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Library/Homebrew/test/testing_env.rb b/Library/Homebrew/test/testing_env.rb index 5f98ace15d..4f2619464b 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 433964dc70..57a9c11a50 100644 --- a/Library/Homebrew/version.rb +++ b/Library/Homebrew/version.rb @@ -287,7 +287,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) From b6acb9cb47817631a69fba31a4cab2cae30da01b Mon Sep 17 00:00:00 2001 From: Misty De Meo Date: Thu, 3 Nov 2016 16:43:31 -0700 Subject: [PATCH 3/9] Version: allow comparing against nil --- Library/Homebrew/test/test_versions.rb | 4 ++++ Library/Homebrew/version.rb | 2 ++ 2 files changed, 6 insertions(+) diff --git a/Library/Homebrew/test/test_versions.rb b/Library/Homebrew/test/test_versions.rb index 6f85fe7a06..3e1c97b03a 100644 --- a/Library/Homebrew/test/test_versions.rb +++ b/Library/Homebrew/test/test_versions.rb @@ -145,6 +145,10 @@ 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_comparison_returns_nil_for_non_version v = version("1.0") assert_nil v <=> Object.new diff --git a/Library/Homebrew/version.rb b/Library/Homebrew/version.rb index 57a9c11a50..56ffc9a642 100644 --- a/Library/Homebrew/version.rb +++ b/Library/Homebrew/version.rb @@ -213,6 +213,8 @@ class Version end def <=>(other) + return 1 if other.nil? + return unless other.is_a?(Version) return 0 if version == other.version return 1 if head? && !other.head? From 16529a4de5c24f62574ba0b7dbc033f5323e7afb Mon Sep 17 00:00:00 2001 From: Misty De Meo Date: Thu, 3 Nov 2016 16:47:18 -0700 Subject: [PATCH 4/9] Version: allow coercing non-versions in comparisons These are needed due to the raw string and fixnum comparisons which exist for legacy reasons, for instance compiler version and build comparisons. --- Library/Homebrew/test/test_versions.rb | 5 +++++ Library/Homebrew/version.rb | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/Library/Homebrew/test/test_versions.rb b/Library/Homebrew/test/test_versions.rb index 3e1c97b03a..a6e922178f 100644 --- a/Library/Homebrew/test/test_versions.rb +++ b/Library/Homebrew/test/test_versions.rb @@ -149,6 +149,11 @@ class VersionComparisonTests < Homebrew::TestCase 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/version.rb b/Library/Homebrew/version.rb index 56ffc9a642..f5fdb09669 100644 --- a/Library/Homebrew/version.rb +++ b/Library/Homebrew/version.rb @@ -213,6 +213,11 @@ class Version 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) From 20bbeb5e9c4cdd5fb5b7cc92b46325d86b543cf8 Mon Sep 17 00:00:00 2001 From: Misty De Meo Date: Thu, 3 Nov 2016 16:48:51 -0700 Subject: [PATCH 5/9] Return compiler versions and builds as Versions --- Library/Homebrew/compilers.rb | 11 +++++++++-- Library/Homebrew/development_tools.rb | 22 +++++++++++++++++----- 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/Library/Homebrew/compilers.rb b/Library/Homebrew/compilers.rb index 7503f1d77e..b6c87aeca8 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 diff --git a/Library/Homebrew/development_tools.rb b/Library/Homebrew/development_tools.rb index 3e1f16d2ab..febe04b216 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 From d32a1c4c7da96d5cbf12197be1707364d081fa88 Mon Sep 17 00:00:00 2001 From: Misty De Meo Date: Thu, 3 Nov 2016 16:54:20 -0700 Subject: [PATCH 6/9] Version: add #to_f This is used by things which used to compare against raw strings, for example Xcode.uncached_version --- Library/Homebrew/version.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Library/Homebrew/version.rb b/Library/Homebrew/version.rb index f5fdb09669..0f3cbb7737 100644 --- a/Library/Homebrew/version.rb +++ b/Library/Homebrew/version.rb @@ -260,6 +260,10 @@ class Version version.hash end + def to_f + version.to_f + end + def to_s version.dup end From 4e3d23ad14b29832f14784939acc5afe2f1b28f8 Mon Sep 17 00:00:00 2001 From: Misty De Meo Date: Thu, 3 Nov 2016 16:52:40 -0700 Subject: [PATCH 7/9] Resource: set version to nil if version is null Is this the right fix? This fixes version cascading from the parent. --- Library/Homebrew/resource.rb | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Library/Homebrew/resource.rb b/Library/Homebrew/resource.rb index fe18f14ddc..c0e9dbadae 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) From d8c19fd7d5f658f17f63f1136de6f80a46be0d76 Mon Sep 17 00:00:00 2001 From: Misty De Meo Date: Thu, 3 Nov 2016 16:52:58 -0700 Subject: [PATCH 8/9] SystemConfig: fix version reporting --- Library/Homebrew/system_config.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Library/Homebrew/system_config.rb b/Library/Homebrew/system_config.rb index 555493c0dd..9c7a8d1b01 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}" From c7be025229dbbe86d85982a135c75b04c9ba00f2 Mon Sep 17 00:00:00 2001 From: Misty De Meo Date: Tue, 8 Nov 2016 13:25:44 -0800 Subject: [PATCH 9/9] CompilerSelector: fix null check, tests --- Library/Homebrew/compilers.rb | 4 ++-- Library/Homebrew/test/test_compiler_selector.rb | 16 +++++++++------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/Library/Homebrew/compilers.rb b/Library/Homebrew/compilers.rb index b6c87aeca8..cf614644c0 100644 --- a/Library/Homebrew/compilers.rb +++ b/Library/Homebrew/compilers.rb @@ -122,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 # 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/test/test_compiler_selector.rb b/Library/Homebrew/test/test_compiler_selector.rb index 0363cacd29..b1591bdbec 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