Skip to content

Commit

Permalink
-I should always win over gems
Browse files Browse the repository at this point in the history
If there were gems in the "unresolved" list, then the files in that gem
would be preferred over files that included in -I.  For example, if you
had a depdedncy tree that looks like this:

```dot
digraph g {
  a -> b [label="version: = 1"];
  b -> c [label="version: > 0"];
}
```

and the installed gems were like this:

a = 1,
b = 1,
c = 1,
c = 2,

When the user requires 'a', then 'b' will be immediately activated
because there is only one decision to make.  However, 'c' will be added
to the `unresloved_deps` list.  Having anything in that list will cause
Rubygems [to skip checking if Ruby's `require` can handle the file](https://github.com/rubygems/rubygems/blob/07f1a83fe095e2b9b09e2f6b9661aed326b2de6a/lib/rubygems/core_ext/kernel_require.rb#L52-55)
If the user provides any paths to `-I`
that contain files that are also in an activated gem (like `b` in this
case), then Rubygems will prefer the file from the gem rather than the
file provided by -I.

This commit fixes it so that -I will win over gems.  e2935c8 introduced
the incorrect behavior.
  • Loading branch information
tenderlove committed May 26, 2015
1 parent 07f1a83 commit 65ab980
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 1 deletion.
2 changes: 1 addition & 1 deletion lib/rubygems/core_ext/kernel_require.rb
Expand Up @@ -66,7 +66,7 @@ def require path

begin
RUBYGEMS_ACTIVATION_MONITOR.exit
return gem_original_require(spec.to_fullpath(path) || path)
return gem_original_require(path)
end if spec

# Attempt to find +path+ in any unresolved gems...
Expand Down
30 changes: 30 additions & 0 deletions test/rubygems/test_require.rb
Expand Up @@ -49,6 +49,36 @@ def append_latch spec
end
end

# Providing -I on the commandline should always beat gems
def test_dash_i_beats_gems
a1 = new_spec "a", "1", {"b" => "= 1"}, "lib/test_gem_require_a.rb"
b1 = new_spec "b", "1", {"c" => "> 0"}, "lib/b/c.rb"
c1 = new_spec "c", "1", nil, "lib/c/c.rb"
c2 = new_spec "c", "2", nil, "lib/c/c.rb"

install_specs c1, c2, b1, a1

dir = Dir.mktmpdir
dash_i_arg = File.join Dir.mktmpdir, 'lib'

c_rb = File.join dash_i_arg, 'b', 'c.rb'

FileUtils.mkdir_p File.dirname c_rb
File.open(c_rb, 'w') { |f| f.write "class Object; HELLO = 'world' end" }

lp = $LOAD_PATH.dup

# Pretend to provide a commandline argument that overrides a file in gem b
$LOAD_PATH.unshift dash_i_arg

assert_require 'test_gem_require_a'
assert_require 'b/c' # this should be required from -I
assert_equal "world", ::Object::HELLO
ensure
$LOAD_PATH.replace lp
Object.send :remove_const, :HELLO if Object.const_defined? :HELLO

This comment has been minimized.

Copy link
@simi

simi May 27, 2015

Member

OHAI

end

def test_concurrent_require
Object.const_set :FILE_ENTERED_LATCH, Latch.new(2)
Object.const_set :FILE_EXIT_LATCH, Latch.new(1)
Expand Down

0 comments on commit 65ab980

Please sign in to comment.