Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

require_realative calls self.require instead of Kernel.require #4053

Closed
lasso opened this issue Aug 6, 2016 · 4 comments
Closed

require_realative calls self.require instead of Kernel.require #4053

lasso opened this issue Aug 6, 2016 · 4 comments

Comments

@lasso
Copy link

lasso commented Aug 6, 2016

Environment

jruby 9.0.5.0 (2.2.3) 2016-01-26 7bee00d Java HotSpot(TM) 64-Bit Server VM 25.91-b14 on 1.8.0_91-b14 +jit [linux-amd64]
Linux linuxmint 4.4.0-31-generic #50-Ubuntu SMP Wed Jul 13 00:07:12 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux (running in VirtualBox)

Originally discovered by test run on Travis CI: https://travis-ci.org/lasso/racket/jobs/149817357

Expected Behavior

MRI/Rubinius behavior:
When using require_relative in a module Foo that also have a module function named require, require_relative (from Kernel) calls Kernel.require, not Foo.require.

When running main.rb (attached) using MRI, the version string '0.5.0' is printed.

Actual Behavior

JRuby behavior:
When using require_relative in a module Foo that also have a module function named require, require_relative (from Kernel) calls Foo.require, not Kernel.require.

When running main.rb (attached) using JRuby, an error is thrown (because Foo.require is called).

A simple workaround that worked for me was using Kernel.require_relative instead of of require_relative in the Foo.version method, then JRuby uses Kernel.require instead of Foo.require.

In summary MRI (and Rubinius) always calls Kernel.require from Kernel.require_relative, but JRuby seems to use self.require if available.

require-bug.zip

@lasso
Copy link
Author

lasso commented Aug 8, 2016

The main difference between JRuby and other implementations seems to be that JRuby is the only one that is defining require_relative on the "ruby" side (thus following ruby rules). Other implementations have custom (internal) classes/functions to require_files.

MRI uses internal C functions: https://github.com/ruby/ruby/blob/c3ceb1bff26a0dc59d7b93647e3a58c57e7c0440/load.c#L833

Rubinius uses an internal class called CodeLoader: https://github.com/rubinius/rubinius/blob/2bf206d50c0fb1916e17d936de2e7a4c2a42145b/core/code_loader.rb#L138

JRuby uses the public Kernel class:

def require_relative(relative_arg)

@kares
Copy link
Member

kares commented Aug 8, 2016

@lasso sounds like JRuby might do a "better" Ruby job here :) ... suppose you wanted to override require method in your module in MRI there would need to be special handling (re-def) for require_relative as well. maybe should try escalating the issue to MRI itself (they might accept it as a bug for a next release).

@lasso
Copy link
Author

lasso commented Aug 8, 2016

@kares - I agree that JRuby does a better job not "special casing" methods in Kernel, but in my code I overrode require, not require_relative. The docs (http://www.rubydoc.info/stdlib/core/Kernel#require_relative-instance_method) does not say that require_relative is depending on the implementation of require, so I actually think JRuby does the wrong thing here. That dependency is based on implementation and JRuby have chosen a different path here. I'm not saying it's the wrong path, just that it seems to be a less used at the moment.

@headius
Copy link
Member

headius commented Aug 10, 2016

Yeah, we have to fix this. I agree it would be nice to have some uniformity in the redispatch to require, but we can't differ this much :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants