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

[Q] __dir__ variable in jar file. #4611

Closed
hiroyuki-sato opened this issue May 17, 2017 · 8 comments
Closed

[Q] __dir__ variable in jar file. #4611

hiroyuki-sato opened this issue May 17, 2017 · 8 comments

Comments

@hiroyuki-sato
Copy link

@hiroyuki-sato hiroyuki-sato commented May 17, 2017

I would like to ask about __dir__ variable in jar archive.

1. environment

  • JRuby: 9.1.7.0
  • OS: macOS 10.12.4
  • java: 1.8.0_112

2. expected and actual behavior

2.1 Expected Behavior

As the following test result, I think __dir__ should return A or B

  • A: uri:classloader:/gems/liquid-4.0.0/lib
  • B: /path/to/jar_file.jar!/gems/liquid-4.0.0/lib

2.2 Actual Behavior

The __dir__ returns strange path like /path/cwd:classloader:/gems/liquid-4.0.0/lib.
/path/cwd is the current working directory which I execute java command.

3. Question

  • Is this JRuby issue or not?

4. Details

This sample project make a single jar file with liquid-4.0.0 gem package.
The liquid package are locate below.

jar tvf ./build/libs/jruby__dir_test-jruby.jar | grep liquid.rb
  3386 Wed May 17 23:56:38 JST 2017 gems/liquid-4.0.0/lib/liquid.rb

When I put the following codes in gems/liquid-4.0.0/lib/liquid.rb
The result were below.

p "                 File.dirname(__FILE__): #{File.dirname(__FILE__)}"
p "File.expand_path(File.dirname(__FILE__): #{File.expand_path(File.dirname(__FILE__))}"
p "                                __dir__: #{__dir__}"
program result
File.dirname(__FILE__) uri:classloader:/gems/liquid-4.0.0/lib
File.expand_path(File.dirname(__FILE__) uri:classloader:/gems/liquid-4.0.0/lib
__dir__ /private/tmp/jruby__dir_test/uri:classloader:/gems/liquid-4.0.0/lib

(/private/tmp/jruby__dir_test is the java execution directory)

And I also test Dir.exist? command.

Dir.exist? result note
uri:classloader:/gems/liquid-4.0.0/lib true
/path/to/jar_file.jar!/gems/liquid-4.0.0/lib true
/path/to/jar_file.jar!uri:classloader:/gems/liquid-4.0.0/lib false
/path/to/project_dir/uri:classloader:/gems/liquid-4.0.0/lib false __dir__ result

5. Test codes.

cd /tmp
git clone https://github.com/hiroyuki-sato/jruby__dir_test
cd jruby__dir_test
./gradlew clean
git checkout -b liquid_4_0_0 liquid_4_0_0
./gradlew jrubyJar
java -jar build/libs/jruby__dir_test-jruby.jar

this java code execute the following Ruby code.

puts JRUBY_VERSION
puts RUBY_VERSION

require 'liquid'
require 'pp'
pp Liquid::Continue

The result is the below.

9.1.7.0
2.3.1
NameError: uninitialized constant Liquid::Continue
Did you mean?  Continuation
  const_missing at org/jruby/RubyModule.java:3344
         <main> at classpath:jar-bootstrap.rb:6

pp Liquid::Continue raise an exception because
lib/liquid.rb in liquid-4.0.0 use the following code and __dir__ return strange path.

Dir["#{__dir__}/liquid/tags/*.rb"].each { |f| require f }

Best regards.

@enebo
Copy link
Member

@enebo enebo commented May 17, 2017

@hiroyuki-sato this is definitely a bug. We should be returing similarly to the FILE.

@hiroyuki-sato
Copy link
Author

@hiroyuki-sato hiroyuki-sato commented May 18, 2017

Hello, @enebo
Thank you for quick replying.
Please let me know if you need more information about this issue.

Best regards.

@kares
Copy link
Member

@kares kares commented Jun 8, 2017

@hiroyuki-sato since the plugin packs in jruby ... any chance you could force it to use latest JRuby 9.1.10?

@kares
Copy link
Member

@kares kares commented Jun 8, 2017

I see now, this simply never worked (the __dir__ version) since JRuby has always being doing a kind of :
new File(__FILE__).getAbsolutePath() in its dir impl ... changing that might have consequences.
basically the notion or uri:classloader: is "special-care" on places such as dirname, missing in __dir__

@headius
Copy link
Member

@headius headius commented Jun 8, 2017

@kares I think we need the same special-case logic here, or better, make this call hopefully-existing logic that does the right thing already.

@hiroyuki-sato
Copy link
Author

@hiroyuki-sato hiroyuki-sato commented Jun 9, 2017

@kares Thank you for your comment.

Yes, I tried it. The latest JRuby (9.1.10.0) got the same error.

Best regards.

Summaries

The Embulk use liquid gem.
Liquid 3.0.6 used File.dirname(__FILE__)
But 4.0.0 use __dir__.
Shopify/liquid@b31df0f#diff-1ac8d79c9833db7b503c16989031b253

Reproduce steps

Build embulk 0.8.20, update

  • Update liquid 3.0.6 -> 4.0.0
  • Update JRuby 9.1.5.0 -> 9.1.10.0
git clone https://github.com/embulk/embulk
cd embulk
git checkout -b v0.8.20 v0.8.20
perl -i -pe 's/9\.1\.5\.0/9.1.10.0/' build.gradle
perl -i -pe 's/3\.0\.6/4.0.0/' embulk-core/build.gradle
./gradlew cli

And test

./pkg/embulk-0.8.20.jar mkbundle dir_test_bundle
./pkg/embulk-0.8.20.jar example dir_test
./pkg/embulk-0.8.20.jar guess dir_test/seed.yml -o config.yml
./pkg/embulk-0.8.20.jar preview -b dir_test_bundle config.yml.liquid

Error.

undefined method `line_number' for #<String:0x652b3733>
    uri:classloader:/liquid/block_body.rb:99:in `block in render'
    org/jruby/RubyArray.java:1734:in `each'
    uri:classloader:/liquid/block_body.rb:75:in `render'
    uri:classloader:/liquid/template.rb:208:in `block in render'
    uri:classloader:/liquid/template.rb:242:in `with_profiling'
    uri:classloader:/liquid/template.rb:207:in `render'
    uri:classloader:/embulk/runner.rb:154:in `run_liquid'
    uri:classloader:/embulk/runner.rb:129:in `read_config'
    uri:classloader:/embulk/runner.rb:32:in `preview'
    uri:classloader:/embulk/command/embulk_run.rb:267:in `run'
    uri:classloader:/embulk/command/embulk_main.rb:2:in `<main>'
    org/jruby/RubyKernel.java:961:in `require'
    file:/private/tmp/embulk/pkg/embulk-0.8.20.jar!/embulk/command/embulk_bundle.rb:30:in `<main>'

Error: undefined method `line_number' for #<String:0x652b3733>

root cause

The 3.0.6 used File.dirname(__FILE__) for load tag classes.
But, 4.0.0 use __dir__. Then it points to another location if -b options use.

Liquid-3.0.6 lib/liquid.rb

Dir[File.dirname(__FILE__) + '/liquid/tags/*.rb'].each { |f| require f }

Liquid-4.0.0 lib/liquid.rb

Dir["#{__dir__}/liquid/tags/*.rb"].each { |f| require f }

Reference.

embulk/embulk#637

kares added a commit to kares/jruby that referenced this issue Jun 9, 2017
)

... if getAbsolutePath is used - which seems like its not necessary
and we can pretty much just do what MRI does File.dirname(__FILE__)
@kares kares added this to the JRuby 9.1.11.0 milestone Jun 11, 2017
kares added a commit to kares/jruby that referenced this issue Jun 11, 2017
)

... if getAbsolutePath is used - which seems like its not necessary
and we can pretty much just do what MRI does File.dirname(__FILE__)
kares added a commit to kares/jruby that referenced this issue Jun 11, 2017
kares added a commit to kares/jruby that referenced this issue Jun 11, 2017
)

... if getAbsolutePath is used - which seems like its not necessary
and we can pretty much just do what MRI does File.dirname(__FILE__)
kares added a commit to kares/jruby that referenced this issue Jun 11, 2017
kares added a commit that referenced this issue Jun 12, 2017
__dir__ won't work with embed paths such as uri:classloader: (#4611)
@kares
Copy link
Member

@kares kares commented Jun 12, 2017

expected to be resolved with #4658

@kares kares closed this Jun 12, 2017
@hiroyuki-sato
Copy link
Author

@hiroyuki-sato hiroyuki-sato commented Jun 12, 2017

@kares Thank you for fixing this issue.

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

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.