JRuby

.class files are loaded before .rb files ignoring the load path ordering

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: None
  • Fix Version/s: JRuby 1.1.5
  • Component/s: Java Integration
  • Labels:
    None
  • Testcase included:
    yes
  • Number of attachments :
    1

Description

If you have 2 files with the same name both in directories on your load path and one is a .rb file and the other is a .class file, the .class file will be loaded first.

In the example attached I have a file load_test.rb that adds the two sub-directories to the load path

$LOAD_PATH << File.expand_path(File.dirname(_FILE_) + "/dir1")
$LOAD_PATH << File.expand_path(File.dirname(_FILE_) + "/dir2")

dir1 contains a file, target.rb with the code: puts "target.rb in dir1"
dir2 contains a file, target.class which is the output of running a file target.rb through jrubyc. The original target.rb file in dir2 was: puts "target.rb in dir2"

The third line of the load_tes.rb file is then

require 'target'

And the output is: target.rb in dir2

Activity

Hide
David Koontz added a comment -

I should have mentioned that if you remove target.class from dir2 and put target.rb in its place, you get the correct output: target.rb in dir1 so this only affects .rb to .class ordering. Also, if you compile both target files, you will also get the correct dir1 output.

Show
David Koontz added a comment - I should have mentioned that if you remove target.class from dir2 and put target.rb in its place, you get the correct output: target.rb in dir1 so this only affects .rb to .class ordering. Also, if you compile both target files, you will also get the correct dir1 output.
Hide
Charles Oliver Nutter added a comment -

It does appear that the order isn't consistent with Ruby's ordering of .rb and .so:

~/projects/jruby &#10132; cat /tmp/etc.rb
puts 'here'
~/projects/jruby &#10132; ruby -e '$: << "/tmp"; require "etc"'
~/projects/jruby &#10132; ruby -I /tmp -e 'require "etc"'
here

But my guess is that this ordering was put in place to allow precompiling a whole directory's worth of files and having the .class files take precedence over the .rb files.

I think I'll spend my night exploring a cleanup of LoadService, while we discuss what the ordering should be.

Show
Charles Oliver Nutter added a comment - It does appear that the order isn't consistent with Ruby's ordering of .rb and .so:
~/projects/jruby &#10132; cat /tmp/etc.rb
puts 'here'
~/projects/jruby &#10132; ruby -e '$: << "/tmp"; require "etc"'
~/projects/jruby &#10132; ruby -I /tmp -e 'require "etc"'
here
But my guess is that this ordering was put in place to allow precompiling a whole directory's worth of files and having the .class files take precedence over the .rb files. I think I'll spend my night exploring a cleanup of LoadService, while we discuss what the ordering should be.
Hide
Charles Oliver Nutter added a comment -

Ok, I got some really basic refactoring in. I think the logic we want would search each directory first for .class, then for .rb, and then move on to the next directory. Currently it's trying .class everywhere, and then .rb everywhere. I have not been able to come up with a down side to that sequence. I think it's also likely the logic we want for each of the extension suffixes (.so, .jar, whatever) would probably want to follow the same logic.

LoadService is pretty heinous, but I'll keep trying tomorrow.

Show
Charles Oliver Nutter added a comment - Ok, I got some really basic refactoring in. I think the logic we want would search each directory first for .class, then for .rb, and then move on to the next directory. Currently it's trying .class everywhere, and then .rb everywhere. I have not been able to come up with a down side to that sequence. I think it's also likely the logic we want for each of the extension suffixes (.so, .jar, whatever) would probably want to follow the same logic. LoadService is pretty heinous, but I'll keep trying tomorrow.
Hide
Matt Fletcher added a comment -

I think the logic we want would search each directory first for .class, then for .rb, and then move on to the next directory.

Yeah, I've spent some time dealing with class and rb files and I agree with your suggested ordering. I've spent some time down in LoadService too. Have fun

Show
Matt Fletcher added a comment -
I think the logic we want would search each directory first for .class, then for .rb, and then move on to the next directory.
Yeah, I've spent some time dealing with class and rb files and I agree with your suggested ordering. I've spent some time down in LoadService too. Have fun
Hide
Charles Oliver Nutter added a comment -

I'm committing a change that appears to work. The test you (David) provided seems to run correctly:

~/projects/jruby/load_path_bug &#10132; jruby load_path_test.rb
/Users/headius/projects/jruby/lib/ruby/site_ruby/1.8
/Users/headius/projects/jruby/lib/ruby/site_ruby
/Users/headius/projects/jruby/lib/ruby/1.8
/Users/headius/projects/jruby/lib/ruby/1.8/java
lib/ruby/1.8
.
/Users/headius/projects/jruby/lib/ruby/site_ruby/1.8
/Users/headius/projects/jruby/lib/ruby/site_ruby
/Users/headius/projects/jruby/lib/ruby/1.8
/Users/headius/projects/jruby/lib/ruby/1.8/java
lib/ruby/1.8
.
/Users/headius/projects/jruby/load_path_bug/dir1
/Users/headius/projects/jruby/load_path_bug/dir2
target.rb in dir1

I'll try to craft it into a test we can run as part of the build. Looks good to you?

Show
Charles Oliver Nutter added a comment - I'm committing a change that appears to work. The test you (David) provided seems to run correctly:
~/projects/jruby/load_path_bug &#10132; jruby load_path_test.rb
/Users/headius/projects/jruby/lib/ruby/site_ruby/1.8
/Users/headius/projects/jruby/lib/ruby/site_ruby
/Users/headius/projects/jruby/lib/ruby/1.8
/Users/headius/projects/jruby/lib/ruby/1.8/java
lib/ruby/1.8
.
/Users/headius/projects/jruby/lib/ruby/site_ruby/1.8
/Users/headius/projects/jruby/lib/ruby/site_ruby
/Users/headius/projects/jruby/lib/ruby/1.8
/Users/headius/projects/jruby/lib/ruby/1.8/java
lib/ruby/1.8
.
/Users/headius/projects/jruby/load_path_bug/dir1
/Users/headius/projects/jruby/load_path_bug/dir2
target.rb in dir1
I'll try to craft it into a test we can run as part of the build. Looks good to you?
Hide
David Koontz added a comment -

Yes, the code passes the test case for me.

Show
David Koontz added a comment - Yes, the code passes the test case for me.
Hide
Charles Oliver Nutter added a comment -

Still needs a test for 1.1.5, but it's done and confirmed working.

Show
Charles Oliver Nutter added a comment - Still needs a test for 1.1.5, but it's done and confirmed working.
Hide
Charles Oliver Nutter added a comment -

Added a test in r7892.

Show
Charles Oliver Nutter added a comment - Added a test in r7892.

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: