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.

3 files changed, 68 insertions, 19 deletions

diff --git a/Library/Homebrew/compilers.rb b/Library/Homebrew/compilers.rb
index 9561fc0fb..cb7d74334 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 672d21056..f9de49adb 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 31f318a4c..642d9c060 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