Auto merge of #1552 - tenderlove:spec_ordering, r=drbrain

stub ordering should be consistent regardless of how cache is populated

Looks like stub ordering can change depending on how the cache was
populated.  This commit changes cache population to always order spec
stubs consistently: newest first.  This fixes a bug where an older spec
is activated when the newer spec is expected.

lib/rubygems/commands/pristine_command.rb

@@ -92,8 +92,8 @@ def execute
                 spec.extensions and not spec.extensions.empty?
               end
             else
-              get_all_gem_names.map do |gem_name|
-                Gem::Specification.find_all_by_name gem_name, options[:version]
+              get_all_gem_names.sort.map do |gem_name|
+                Gem::Specification.find_all_by_name(gem_name, options[:version]).reverse
               end.flatten
             end
 

lib/rubygems/core_ext/kernel_require.rb

@@ -104,7 +104,7 @@ def require path
 
       # Ok, now find a gem that has no conflicts, starting
       # at the highest version.
-      valid = found_specs.reject { |s| s.has_conflicts? }.last
+      valid = found_specs.reject { |s| s.has_conflicts? }.first
 
       unless valid then
         le = Gem::LoadError.new "unable to find a version of '#{names.first}' to activate"

lib/rubygems/dependency.rb

@@ -286,9 +286,7 @@ def matching_specs platform_only = false
       }
     end
 
-    # `stubs_for` returns oldest first, but `matching_specs` is supposed to
-    # return newest first, so just reverse the list
-    matches.reverse
+    matches
   end
 
   ##

lib/rubygems/specification.rb

@@ -849,7 +849,7 @@ def self.stubs_for name
       pattern = "#{name}-*.gemspec"
       stubs = default_stubs(pattern) + installed_stubs(dirs, pattern)
       stubs = uniq_by(stubs) { |stub| stub.full_name }.group_by(&:name)
-      stubs.each_value { |v| sort_by!(v) { |i| i.version } }
+      stubs.each_value { |v| _resort!(v) }
 
       @@stubs_by_name.merge! stubs
       @@stubs_by_name[name] ||= EMPTY
@@ -1074,7 +1074,7 @@ def self.find_in_unresolved path
   def self.find_in_unresolved_tree path
     specs = unresolved_deps.values.map { |dep| dep.to_specs }.flatten
 
-    specs.reverse_each do |spec|
+    specs.each do |spec|
       spec.traverse do |from_spec, dep, to_spec, trail|
         if to_spec.has_conflicts? || to_spec.conficts_when_loaded_with?(trail)
           :next
@@ -2613,7 +2613,7 @@ def traverse trail = [], visited = {}, &block
     begin
       dependencies.each do |dep|
         next unless dep.runtime?
-        dep.to_specs.reverse_each do |dep_spec|
+        dep.to_specs.each do |dep_spec|
           next if visited.has_key?(dep_spec)
           visited[dep_spec] = true
           trail.push(dep_spec)

test/rubygems/test_gem.rb

@@ -959,6 +959,19 @@ def test_try_activate_returns_true_for_activated_specs
     assert Gem.try_activate('b'), 'try_activate should still return true'
   end
 
+  def test_spec_order_is_consistent
+    b1 = util_spec 'b', '1.0'
+    b2 = util_spec 'b', '2.0'
+    b3 = util_spec 'b', '3.0'
+
+    install_specs b1, b2, b3
+
+    specs1 = Gem::Specification.stubs.find_all { |s| s.name == 'b' }
+    Gem::Specification.reset
+    specs2 = Gem::Specification.stubs_for('b')
+    assert_equal specs1.map(&:version), specs2.map(&:version)
+  end
+
   def test_self_try_activate_missing_dep
     b = util_spec 'b', '1.0'
     a = util_spec 'a', '1.0', 'b' => '>= 1.0'