Restructure #require, add tests for exceptional cases

lib/rubygems/custom_require.rb

@@ -32,59 +32,70 @@ module Kernel
   # that file has already been loaded is preserved.
 
   def require path
+    # If there are no unresolved deps, then we can use just try
+    # normal require handle loading a gem from the rescue below.
+
     if Gem::Specification.unresolved_deps.empty? then
-      gem_original_require path
+      return gem_original_require(path)
+    end
+
+    # If +path+ is for a gem that has already been loaded, don't
+    # bother trying to find it in an unresolved gem, just go straight
+    # to normal require.
+
+    spec = Gem::Specification.find { |s|
+      s.activated? and s.contains_requirable_file? path
+    }
+
+    return gem_original_require(path) if spec
+
+    # Attempt to find +path+ in any unresolved gems...
+
+    found_specs = Gem::Specification.find_in_unresolved path
+
+    # If there are no directly unresolved gems, then try and find +path+
+    # in any gems that are available via the currently unresolved gems.
+    # For example, given:
+    #
+    #   a => b => c => d
+    #
+    # If a and b are currently active with c being unresolved and d.rb is
+    # requested, then find_in_unresolved_tree will find d.rb in d because
+    # it's a dependency of c.
+    #
+    if found_specs.empty? then
+      found_specs = Gem::Specification.find_in_unresolved_tree path
+
+      found_specs.each do |found_spec|
+        found_spec.activate
+      end
+
+    # We found +path+ directly in an unresolved gem. Now we figure out, of
+    # the possible found specs, which one we should activate.
     else
-      spec = Gem::Specification.find { |s|
-        s.activated? and s.contains_requirable_file? path
-      }
-
-      unless spec then
-        found_specs = Gem::Specification.find_in_unresolved path
-
-        # If there are no directly unresolved gems, then try and find +path+
-        # in any gems that are available via the currently unresolved gems.
-        # For example, given:
-        #
-        #   a => b => c => d
-        #
-        # If a and b are currently active with c being unresolved and d.rb is
-        # requested, then find_in_unresolved_tree will find d.rb in d because
-        # it's a dependency of c.
-        #
-        if found_specs.empty? then
-          found_specs = Gem::Specification.find_in_unresolved_tree path
-
-          found_specs.each do |found_spec|
-            found_spec.activate
-          end
-
-        # We found +path+ directly in an unresolved gem. Now we figure out, of
-        # the possible found specs, which one we should activate.
-        else
-
-          # Check that all the found specs are just different
-          # versions of the same gem
-          names = found_specs.map(&:name).uniq
-
-          if names.size > 1
-            raise Gem::LoadError, "ambigious path (#{path}) found in multiple gems: #{names.join(', ')}"
-          end
-
-          # Ok, now find a gem that has no conflicts, starting
-          # at the highest version.
-          valid = found_specs.select { |s| s.conflicts.empty? }.last
-
-          unless valid
-            raise Gem::LoadError, "unable to find a version of '#{names.first}' to activate"
-          end
-
-          valid.activate
-        end
+
+      # Check that all the found specs are just different
+      # versions of the same gem
+      names = found_specs.map(&:name).uniq
+
+      if names.size > 1 then
+        raise Gem::LoadError, "#{path} found in multiple gems: #{names.join ', '}"
       end
 
-      return gem_original_require path
+      # Ok, now find a gem that has no conflicts, starting
+      # at the highest version.
+      valid = found_specs.select { |s| s.conflicts.empty? }.last
+
+      unless valid then
+        le = Gem::LoadError.new "unable to find a version of '#{names.first}' to activate"
+        le.name = names.first
+        raise le
+      end
+
+      valid.activate
     end
+
+    gem_original_require path
   rescue LoadError => load_error
     if load_error.message.end_with?(path) and Gem.try_activate(path) then
       return gem_original_require(path)

test/rubygems/test_require.rb

@@ -102,6 +102,56 @@ def test_already_activated_direct_conflict
     end
   end
 
+  def test_multiple_gems_with_the_same_path
+    save_loaded_features do
+      a1 = new_spec "a", "1", { "b" => "> 0", "x" => "> 0" }
+      b1 = new_spec "b", "1", { "c" => ">= 1" }, "lib/ib.rb"
+      b2 = new_spec "b", "2", { "c" => ">= 2" }, "lib/ib.rb"
+      x1 = new_spec "x", "1", nil, "lib/ib.rb"
+      x2 = new_spec "x", "2", nil, "lib/ib.rb"
+      c1 = new_spec "c", "1", nil, "lib/d.rb"
+      c2 = new_spec("c", "2", nil, "lib/d.rb")
+
+      install_specs a1, b1, b2, c1, c2, x1, x2
+
+      a1.activate
+      c1.activate
+      assert_equal %w(a-1 c-1), loaded_spec_names
+      assert_equal ["b (> 0)", "x (> 0)"], unresolved_names
+
+      e = assert_raises(Gem::LoadError) do
+        require("ib")
+      end
+
+      assert_equal "ib found in multiple gems: b, x", e.message
+    end
+  end
+
+  def test_unable_to_find_good_unresolved_version
+    save_loaded_features do
+      a1 = new_spec "a", "1", { "b" => "> 0" }
+      b1 = new_spec "b", "1", { "c" => ">= 2" }, "lib/ib.rb"
+      b2 = new_spec "b", "2", { "c" => ">= 3" }, "lib/ib.rb"
+
+      c1 = new_spec "c", "1", nil, "lib/d.rb"
+      c2 = new_spec "c", "2", nil, "lib/d.rb"
+      c3 = new_spec "c", "3", nil, "lib/d.rb"
+
+      install_specs a1, b1, b2, c1, c2, c3
+
+      a1.activate
+      c1.activate
+      assert_equal %w(a-1 c-1), loaded_spec_names
+      assert_equal ["b (> 0)"], unresolved_names
+
+      e = assert_raises(Gem::LoadError) do
+        require("ib")
+      end
+
+      assert_equal "unable to find a version of 'b' to activate", e.message
+    end
+  end
+
   def loaded_spec_names
     Gem.loaded_specs.values.map(&:full_name).sort
   end