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

jruby-rack adjust_gem_path has no effect #3828

Closed
mkristian opened this Issue Apr 26, 2016 · 14 comments

Comments

Projects
None yet
4 participants
@mkristian
Member

mkristian commented Apr 26, 2016

Environment

when investigating the broken j2ee tests on travis it shows that the ENV['GEM_PATH'] set by jruby-rack has no effect on Gem.path and so does not show up in the testcase.

Expected Behavior

jruby does not call Gem.path before requiring a file outside the core.

Actual Behavior

[INFO]  uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/rubygems.rb:403:in `path'
[INFO]  uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/rubygems/dependency.rb:316:in `to_specs'
[INFO]  uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/rubygems/dependency.rb:330:in `to_spec'
[INFO]  uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/rubygems/core_ext/kernel_gem.rb:65:in `gem'
[INFO]  uri:classloader:/jruby/kernel/gem_prelude.rb:9:in `<top>'
[INFO]  file:/Users/cmeier/.m2/repository/org/jruby/jruby-core/9.1.0.0-SNAPSHOT/jruby-core-9.1.0.0-SNAPSHOT.jar!/jruby/preludes.rb:1:in `load'
[INFO]  file:/Users/cmeier/.m2/repository/org/jruby/jruby-core/9.1.0.0-SNAPSHOT/jruby-core-9.1.0.0-SNAPSHOT.jar!/jruby/preludes.rb:1:in `<top>'

which gets activated on jruby startup.

commit 4a8a2bc introduce this problem.

@kares

This comment has been minimized.

Show comment
Hide comment
@kares

kares Apr 26, 2016

Member

@mkristian very najs find! wonder if its worth the "startup cost" to issue a gem() call that previously didn't happen. also maybe the if defined?(DidYouMean) could use some explanation where its being defined.

Member

kares commented Apr 26, 2016

@mkristian very najs find! wonder if its worth the "startup cost" to issue a gem() call that previously didn't happen. also maybe the if defined?(DidYouMean) could use some explanation where its being defined.

@mkristian

This comment has been minimized.

Show comment
Hide comment
@mkristian

mkristian Apr 26, 2016

Member

@kares did look a bit further and there is more then this commit to it. first I thought it is the gem() but even the require 'did_you_mean' does trigger a search through all installed gems, which needs the Gem.path to be set. a workaround all this is to Gem.clear_path inside jruby-rack. but currently you can not set gem.path inside the web.xml as it has no effect

Member

mkristian commented Apr 26, 2016

@kares did look a bit further and there is more then this commit to it. first I thought it is the gem() but even the require 'did_you_mean' does trigger a search through all installed gems, which needs the Gem.path to be set. a workaround all this is to Gem.clear_path inside jruby-rack. but currently you can not set gem.path inside the web.xml as it has no effect

@kares

This comment has been minimized.

Show comment
Hide comment
@kares

kares Apr 26, 2016

Member

sounds like a jruby.rack.gem.path env/web.xml setting would be necessary anyway? also this has other consequences to be considered - would anyone want "did_you_mean" to be "messing" on env.production?

Member

kares commented Apr 26, 2016

sounds like a jruby.rack.gem.path env/web.xml setting would be necessary anyway? also this has other consequences to be considered - would anyone want "did_you_mean" to be "messing" on env.production?

@mkristian

This comment has been minimized.

Show comment
Hide comment
@mkristian

mkristian Apr 26, 2016

Member

let's reduce the regression to a simple example:
jruby-9.0.x.0

$ jruby -v -e "ENV['GEM_PATH']='.';p Gem.path"
jruby 9.0.5.0 (2.2.3) 2016-01-26 7bee00d Java HotSpot(TM) 64-Bit Server VM 25.72-b05 on 1.8.0_72-ea-b05 +jit [darwin-x86_64]
[".", "/Users/cmeier/.rbenv/versions/jruby-9.0.5.0/lib/ruby/gems/shared"]

and jruby-9.1.0.0-SNAPSHOT

bin/jruby -v -e "ENV['GEM_PATH']='.';p Gem.path"
jruby 9.1.0.0-SNAPSHOT (2.3.0) 2016-04-26 af9095f Java HotSpot(TM) 64-Bit Server VM 24.79-b02 on 1.7.0_79-b15 +jit [darwin-x86_64]
["/Users/cmeier/.gem/jruby/2.3.0", "/Users/cmeier/projects/active/jruby/lib/ruby/gems/shared"]
Member

mkristian commented Apr 26, 2016

let's reduce the regression to a simple example:
jruby-9.0.x.0

$ jruby -v -e "ENV['GEM_PATH']='.';p Gem.path"
jruby 9.0.5.0 (2.2.3) 2016-01-26 7bee00d Java HotSpot(TM) 64-Bit Server VM 25.72-b05 on 1.8.0_72-ea-b05 +jit [darwin-x86_64]
[".", "/Users/cmeier/.rbenv/versions/jruby-9.0.5.0/lib/ruby/gems/shared"]

and jruby-9.1.0.0-SNAPSHOT

bin/jruby -v -e "ENV['GEM_PATH']='.';p Gem.path"
jruby 9.1.0.0-SNAPSHOT (2.3.0) 2016-04-26 af9095f Java HotSpot(TM) 64-Bit Server VM 24.79-b02 on 1.7.0_79-b15 +jit [darwin-x86_64]
["/Users/cmeier/.gem/jruby/2.3.0", "/Users/cmeier/projects/active/jruby/lib/ruby/gems/shared"]
@kares

This comment has been minimized.

Show comment
Hide comment
@kares

kares Apr 27, 2016

Member

for the record the other DidYouMean part is coming from https://github.com/jruby/jruby/blob/e05ec8/core/src/main/java/org/jruby/Ruby.java#L1303-L1305 ... simply handling did-you-mean being disabled.

Member

kares commented Apr 27, 2016

for the record the other DidYouMean part is coming from https://github.com/jruby/jruby/blob/e05ec8/core/src/main/java/org/jruby/Ruby.java#L1303-L1305 ... simply handling did-you-mean being disabled.

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Apr 28, 2016

Member

Tick tock gentlemen :-)

Member

headius commented Apr 28, 2016

Tick tock gentlemen :-)

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Apr 28, 2016

Member

What if we just make it try to require 'did_you_mean' without activating the gem?

Member

headius commented Apr 28, 2016

What if we just make it try to require 'did_you_mean' without activating the gem?

@mkristian

This comment has been minimized.

Show comment
Hide comment
@mkristian

mkristian Apr 29, 2016

Member

the problem is that when require fails that rubygems will go though
Gem.path and this is where ENV['GEM_PATH'] gets parsed and any
later changes on GEM_PATH gets ignored. so the problem is not the
gem() but the require

as @kares asked before where we need did_you_mean in production, i.e. for
the ScriptingContainer we could have a different default or something in
this direction.

Member

mkristian commented Apr 29, 2016

the problem is that when require fails that rubygems will go though
Gem.path and this is where ENV['GEM_PATH'] gets parsed and any
later changes on GEM_PATH gets ignored. so the problem is not the
gem() but the require

as @kares asked before where we need did_you_mean in production, i.e. for
the ScriptingContainer we could have a different default or something in
this direction.

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Apr 29, 2016

Member

Yeah that is a pain...there's no good option.

I see two paths forward:

  1. We just remove it and it's not a feature of JRuby out of the box.
  2. We make it a default gem so it's always requirable without going through RubyGems.

I'm going to chat with @tenderlove today about options.

Member

headius commented Apr 29, 2016

Yeah that is a pain...there's no good option.

I see two paths forward:

  1. We just remove it and it's not a feature of JRuby out of the box.
  2. We make it a default gem so it's always requirable without going through RubyGems.

I'm going to chat with @tenderlove today about options.

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Apr 29, 2016

Member

We are going to punt this decision to 9.1.1.0.

Member

headius commented Apr 29, 2016

We are going to punt this decision to 9.1.1.0.

@mkristian

This comment has been minimized.

Show comment
Hide comment
@mkristian

mkristian May 2, 2016

Member

@headius with commit 3dc2acf the j2ee tests are green again - we should move them back to the "not-allowed-failures" again ?!

Member

mkristian commented May 2, 2016

@headius with commit 3dc2acf the j2ee tests are green again - we should move them back to the "not-allowed-failures" again ?!

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius May 2, 2016

Member

@mkristian Seems fine to me if they're passing. It's obviously something we'll have to deal with when we do get DYM in place.

Member

headius commented May 2, 2016

@mkristian Seems fine to me if they're passing. It's obviously something we'll have to deal with when we do get DYM in place.

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius May 11, 2016

Member

9.1.1.0 has become a quick regression-patching release, so bumped this to 9.1.2.0.

Member

headius commented May 11, 2016

9.1.1.0 has become a quick regression-patching release, so bumped this to 9.1.2.0.

@enebo enebo modified the milestones: JRuby 9.1.2.0, JRuby 9.1.3.0 May 23, 2016

@mkristian

This comment has been minimized.

Show comment
Hide comment
@mkristian

mkristian Jun 7, 2016

Member

already fixed with #3892 and travis -Pj2ee is also green again.

Member

mkristian commented Jun 7, 2016

already fixed with #3892 and travis -Pj2ee is also green again.

@mkristian mkristian closed this Jun 7, 2016

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