-
-
Notifications
You must be signed in to change notification settings - Fork 925
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
load current directory file #3126
Comments
oho
but
the latter is wanted. the first one not working with jruby is a bug. |
That's weird. I'm not sure why MRI loads from CWD for "load" but not for "require" we need to clarify this behavior with MRI. |
This breaks Sequel's specs (and many uses of Sequel migrations). The historical and I believe expected behavior for all ruby versions (and JRuby versions prior to rc2) is for |
This is a pretty serious bug that is likely to occur in many apps. It currently breaks our Capistrano deployment. Will it force a JRuby 9.0.0.0.rc3 release? |
@donv @jeremyevans I think we will just make sure both your use cases work and still try for final. So long as we don't realize the semantics are weirder than reported... |
wowsers. The semantics of this are interesting and it reflects yet another hard to believe lack of understanding on my part within one week (to_ary/to_a in masgn being the other -- love random trivia in an issue comment). If I have CWD like this:
Invocations of MRI:
So if if it a qualified path we load that. If it is an unqualified path we try to load from LOAD_PATH and failing that we load from CWD. |
I believe this logic is in path = rb_find_file(fname);
if (!path) {
if (!rb_file_load_ok(RSTRING_PTR(fname)))
load_failed(orig_fname);
path = fname;
}
rb_load_internal(path, RTEST(wrap)); Seems simple enough to add to our load logic, yes? |
@headius please review this for me. I think it is what we discovered this morning but the actual code and where I injected this potentially could be wrong. @donv @jeremyevans could I trouble you guys to test HEAD to make sure you see no more problems. This turns out to be not too risky because it comes after normal load semantics and MRI definitely has this additional check so we now line up with it. |
The fix looks right to me. |
I'm currently at an OpenBSD hackathon, and won't be able to test this until mid-next week. I don't ever build JRuby from source, but I suppose I could try jruby-head in RVM in a VM next week and see if it works there. |
My use case with Capistrano that failed earlier work fine now. 😄 👍 |
@jeremyevans I have sequel cloned and I was able to get a full green run doing a 'rake spec' with HEAD. I confirmed it was broken before this fix so I think we are golden. |
@headius
Jruby 1.7.21$ jruby -v -e "require 'test.rb' ; puts 'OK'"
jruby 1.7.21 (1.9.3p551) 2015-07-07 a741a82 on Java HotSpot(TM) 64-Bit Server VM 1.8.0_45-b15 +jit [Windows 7-amd64]
io/console not supported; tty will not be manipulated
OK
Jruby 9.2.6.0$ jruby -v -e "require 'test.rb' ; puts 'OK'"
jruby 9.2.6.0 (2.5.3) 2019-02-11 15ba00b Java HotSpot(TM) 64-Bit Server VM 25.45-b02 on 1.8.0_45-b15 +jit [mswin32-x86_64]
LoadError: no such file to load -- test
require at org/jruby/RubyKernel.java:984
require at C:/JRuby/jruby-9.2.6.0/lib/ruby/stdlib/rubygems/core_ext/kernel_require.rb:59
<main> at -e:1 |
jruby 9k rc2 can't load a file in current directory.
The text was updated successfully, but these errors were encountered: