From bbfb6400c77aeaaf88216263d86491d85a40f8a9 Mon Sep 17 00:00:00 2001
From: Misty De Meo <mistydemeo@gmail.com>
Date: Tue, 2 Oct 2012 13:21:00 -0500
Subject: [PATCH] Manage Requirements using ComparableSet

ComparableSet only allows a single object of a given class,
choosing the object with the greatest value. This was mainly created
for Requirements, so that, e.g., two X11Dependencies of differing
strictness don't both end up in the same requirement set.

Fixes Homebrew/homebrew#15240.
---
 Library/Homebrew/dependencies.rb            | 20 +++++++++--
 Library/Homebrew/extend/set.rb              | 24 +++++++++++++
 Library/Homebrew/formula.rb                 |  7 ++--
 Library/Homebrew/test/test_comparableset.rb | 40 +++++++++++++++++++++
 4 files changed, 85 insertions(+), 6 deletions(-)
 create mode 100644 Library/Homebrew/extend/set.rb
 create mode 100644 Library/Homebrew/test/test_comparableset.rb

diff --git a/Library/Homebrew/dependencies.rb b/Library/Homebrew/dependencies.rb
index 5d5b5ab0d9..7c2ff7d1b0 100644
--- a/Library/Homebrew/dependencies.rb
+++ b/Library/Homebrew/dependencies.rb
@@ -21,7 +21,7 @@ class DependencyCollector
 
   def initialize
     @deps = Dependencies.new
-    @requirements = Set.new
+    @requirements = ComparableSet.new
   end
 
   def add spec
@@ -196,6 +196,9 @@ end
 # This requirement is used to require an X11 implementation,
 # optionally with a minimum version number.
 class X11Dependency < Requirement
+  include Comparable
+  attr_reader :min_version
+
   def initialize min_version=nil
     @min_version = min_version
   end
@@ -217,9 +220,20 @@ class X11Dependency < Requirement
     ENV.x11
   end
 
-  def hash
-    "X11".hash
+  def <=> other
+    unless other.is_a? X11Dependency
+      raise TypeError, "expected X11Dependency"
+    end
+
+    if other.min_version.nil?
+      1
+    elsif @min_version.nil?
+      -1
+    else
+      @min_version <=> other.min_version
+    end
   end
+
 end
 
 
diff --git a/Library/Homebrew/extend/set.rb b/Library/Homebrew/extend/set.rb
new file mode 100644
index 0000000000..b45b85de96
--- /dev/null
+++ b/Library/Homebrew/extend/set.rb
@@ -0,0 +1,24 @@
+require 'set'
+
+class ComparableSet < Set
+  def add new
+    # smileys only
+    return super new unless new.respond_to? :>
+
+    objs = find_all { |o| o.class == new.class }
+    objs.each do |o|
+      return self if o > new
+      delete o
+    end
+    super new
+  end
+
+  alias_method :<<, :add
+
+  # Set#merge bypasses enumerating the set's contents,
+  # so the subclassed #add would never be called
+  def merge enum
+    enum.is_a?(Enumerable) or raise ArgumentError, "value must be enumerable"
+    enum.each { |o| add(o) }
+  end
+end
diff --git a/Library/Homebrew/formula.rb b/Library/Homebrew/formula.rb
index d23aeea714..8f63cd705d 100644
--- a/Library/Homebrew/formula.rb
+++ b/Library/Homebrew/formula.rb
@@ -6,6 +6,7 @@ require 'bottles'
 require 'patches'
 require 'compilers'
 require 'build_environment'
+require 'extend/set'
 
 
 class Formula
@@ -455,9 +456,9 @@ class Formula
   end
 
   def recursive_requirements
-    reqs = recursive_deps.map { |dep| dep.requirements }.to_set
-    reqs << requirements
-    reqs.flatten
+    reqs = ComparableSet.new
+    recursive_deps.each { |dep| reqs.merge dep.requirements }
+    reqs.merge requirements
   end
 
   def to_hash
diff --git a/Library/Homebrew/test/test_comparableset.rb b/Library/Homebrew/test/test_comparableset.rb
new file mode 100644
index 0000000000..d39ad51d6d
--- /dev/null
+++ b/Library/Homebrew/test/test_comparableset.rb
@@ -0,0 +1,40 @@
+require 'testing_env'
+require 'extend/set'
+
+class ComparableSetTests < Test::Unit::TestCase
+  def setup
+    @set = ComparableSet.new
+  end
+
+  def test_merging_multiple_dependencies
+    @set << X11Dependency.new
+    @set << X11Dependency.new
+    assert_equal @set.count, 1
+    @set << Requirement.new
+    assert_equal @set.count, 2
+  end
+
+  def test_comparison_prefers_larger
+    @set << X11Dependency.new
+    @set << X11Dependency.new('2.6')
+    assert_equal @set.count, 1
+    assert_equal @set.to_a, [X11Dependency.new('2.6')]
+  end
+
+  def test_comparison_does_not_merge_smaller
+    @set << X11Dependency.new('2.6')
+    @set << X11Dependency.new
+    assert_equal @set.count, 1
+    assert_equal @set.to_a, [X11Dependency.new('2.6')]
+  end
+
+  def test_merging_sets
+    @set << X11Dependency.new
+    @set << Requirement.new
+    reqs = Set.new [X11Dependency.new('2.6'), Requirement.new]
+    @set.merge reqs
+
+    assert_equal @set.count, 2
+    assert_equal @set.find {|r| r.is_a? X11Dependency}, X11Dependency.new('2.6')
+  end
+end