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

Comments

Projects
None yet
3 participants
@lasso

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

This comment has been minimized.

Show comment
Hide comment
@lasso

lasso 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)

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

This comment has been minimized.

Show comment
Hide comment
@kares

kares Aug 8, 2016

Member

@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).

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

This comment has been minimized.

Show comment
Hide comment
@lasso

lasso 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.

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

This comment has been minimized.

Show comment
Hide comment
@headius

headius Aug 10, 2016

Member

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 :-)

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 :-)

@headius headius closed this in 3ba469b Aug 10, 2016

@headius headius added this to the JRuby 9.1.3.0 milestone Aug 10, 2016

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