From 5537abbe511b10046adc26ee821d95a977a1efb6 Mon Sep 17 00:00:00 2001 From: Misty De Meo Date: Sat, 28 Sep 2013 12:21:16 -0700 Subject: [PATCH] Adjust fails_with syntax for non-Apple compilers The old version worked like this: fails_with :gcc => '4.8.1' That wasn't really flexible enough, and made it harder to distinguish different releases in the same GCC series. Since no one was really using it yet, this adjusts the syntax to be more similar to the Apple compilers: fails_with :gcc => '4.8' do release '4.8.1' end Like with Apple compilers, omitting `release` blacklists the entire series. This also unifies the `build` and `version` attributes and accessors, and exposes them under both names. --- Library/Homebrew/compilers.rb | 43 +++++++++++++++++++----- Library/Homebrew/formula.rb | 37 ++++++++++++++++---- Library/Homebrew/test/test_fails_with.rb | 7 ++-- 3 files changed, 68 insertions(+), 19 deletions(-) diff --git a/Library/Homebrew/compilers.rb b/Library/Homebrew/compilers.rb index 9561fc0fb5..cb7d74334e 100644 --- a/Library/Homebrew/compilers.rb +++ b/Library/Homebrew/compilers.rb @@ -1,30 +1,55 @@ class Compiler < Struct.new(:name, :priority) - def build - MacOS.send("#{name}_build_version") + # The full version of the compiler for comparison purposes. + def version + if name.is_a? String + MacOS.non_apple_gcc_version(name) + else + MacOS.send("#{name}_build_version") + end end - def version - MacOS.non_apple_gcc_version(name) if name.is_a? String + # This is exposed under the `build` name for compatibility, since + # `fails_with` continues to use `build` in the public API. + # `build` indicates the build number of an Apple compiler. + # This is preferred over version numbers since there are often + # significant differences within the same version, + # e.g. GCC 4.2 build 5553 vs 5666. + # Non-Apple compilers don't have build numbers. + alias_method :build, :version + + # The major version for non-Apple compilers. Used to indicate a compiler + # series; for instance, if the version is 4.8.2, it would return "4.8". + def major_version + version.match(/(\d\.\d)/)[0] if name.is_a? String end end class CompilerFailure - attr_reader :compiler, :version - attr_rw :build, :cause + attr_reader :compiler, :major_version + attr_rw :cause, :version def initialize compiler, &block # Non-Apple compilers are in the format fails_with compiler => version if compiler.is_a? Hash # currently the only compiler for this case is GCC - _, @version = compiler.shift - @compiler = 'gcc-' + @version.match(/(\d\.\d)/)[0] + _, @major_version = compiler.shift + @compiler = 'gcc-' + @major_version else @compiler = compiler end instance_eval(&block) if block_given? - @build = (@build || 9999).to_i unless compiler.is_a? Hash + if !compiler.is_a? Hash + @version = (@version || 9999).to_i + else + # so fails_with :gcc => '4.8' simply marks all 4.8 releases incompatible + @version ||= @major_version + '.999' if compiler.is_a? Hash + end end + + # Allows Apple compiler `fails_with` statements to keep using `build` + # even though `build` and `value` are the same internally + alias_method :build, :version end class CompilerQueue diff --git a/Library/Homebrew/formula.rb b/Library/Homebrew/formula.rb index 672d210567..f9de49adbd 100644 --- a/Library/Homebrew/formula.rb +++ b/Library/Homebrew/formula.rb @@ -241,12 +241,10 @@ class Formula def fails_with? cc cc = Compiler.new(cc) unless cc.is_a? Compiler (self.class.cc_failures || []).any? do |failure| - if cc.version - # non-Apple GCCs don't have builds, just version numbers - failure.compiler == cc.name && failure.version >= cc.version - else - failure.compiler == cc.name && failure.build >= cc.build - end + # Major version check distinguishes between, e.g., + # GCC 4.7.1 and GCC 4.8.2, where a comparison is meaningless + failure.compiler == cc.name && failure.major_version == cc.major_version && \ + failure.version >= cc.version end end @@ -785,6 +783,33 @@ class Formula @keg_only_reason = KegOnlyReason.new(reason, explanation.to_s.chomp) end + # For Apple compilers, this should be in the format: + # fails_with compiler do + # cause "An explanation for why the build doesn't work." + # build "The Apple build number for the newest incompatible release." + # end + # + # The block may be omitted, and if present the build may be omitted; + # if so, then the compiler will be blacklisted for *all* versions. + # + # For GNU GCC compilers, this should be in the format: + # fails_with compiler => major_version do + # cause + # version "The official release number for the latest incompatible + # version, for instance 4.8.1" + # end + # + # `major_version` should be the major release number only, for instance + # '4.8' for the GCC 4.8 series (4.8.0, 4.8.1, etc.). + # If `version` or the block is omitted, then the compiler will be + # blacklisted for all compilers in that series. + # + # For example, if a bug is only triggered on GCC 4.8.1 but is not + # encountered on 4.8.2: + # + # fails_with :gcc => '4.8' do + # version '4.8.1' + # end def fails_with compiler, &block @cc_failures ||= Set.new @cc_failures << CompilerFailure.new(compiler, &block) diff --git a/Library/Homebrew/test/test_fails_with.rb b/Library/Homebrew/test/test_fails_with.rb index 31f318a4cd..642d9c0603 100644 --- a/Library/Homebrew/test/test_fails_with.rb +++ b/Library/Homebrew/test/test_fails_with.rb @@ -3,7 +3,7 @@ require 'test/testball' class FailsWithTests < Test::Unit::TestCase class Double < Compiler - attr_accessor :name, :build, :version + attr_accessor :name, :version end def assert_fails_with(cc) @@ -21,8 +21,7 @@ class FailsWithTests < Test::Unit::TestCase def build_cc(sym, build, version=nil) cc = Double.new cc.name = sym - cc.build = build - cc.version = version + cc.version = version || build cc end @@ -49,7 +48,7 @@ class FailsWithTests < Test::Unit::TestCase end def test_non_apple_gcc_version - fails_with(:gcc => '4.8.2') + fails_with(:gcc => '4.8') cc = build_cc("gcc-4.8", nil, "4.8.1") assert_fails_with cc end