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

activation of jar-dependencies in bundler clashes with shipped gem #4740

Closed
jsvd opened this Issue Aug 16, 2017 · 10 comments

Comments

Projects
None yet
3 participants
@jsvd

jsvd commented Aug 16, 2017

Environment

  • jruby 9.1.10.0 (2.3.3) 2017-05-25 b09c48a Java HotSpot(TM) 64-Bit Server VM 25.111-b14 on 1.8.0_111-b14 +jit [darwin-x86_64]
  • Darwin Joaos-MBP-5.lan 16.6.0 Darwin Kernel Version 16.6.0: Fri Apr 14 16:21:16 PDT 2017; root:xnu-3789.60.24~6/RELEASE_X86_64 x86_64

Gemfile:

source "https://rubygems.org"

gem "jar-dependencies"

Existing jar-dependencies:

/tmp/jars_deps % gem list -a jar-dependencies

*** LOCAL GEMS ***

jar-dependencies (default: 0.3.9)

Expected Behavior

  • Doing a localized bundle install and running bundle exec rspec should work

Actual Behavior

/tmp/jars_deps % bundle install --path=vendor
Fetching gem metadata from https://rubygems.org/.......
Fetching version metadata from https://rubygems.org/.
Resolving dependencies...
Using bundler 1.15.3
Fetching jar-dependencies 0.3.11
Installing jar-dependencies 0.3.11
Bundle complete! 1 Gemfile dependency, 2 gems now installed.
Bundled gems are installed into ./vendor.
Post-install message from jar-dependencies:

if you want to use the executable lock_jars then install ruby-maven gem before using lock_jars 

   $ gem install ruby-maven -v '~> 3.3.11'

or add it as a development dependency to your Gemfile

   gem 'ruby-maven', '~> 3.3.11'

/tmp/jars_deps % bundle exec rspec
bundler: failed to load command: rspec (/Users/joaoduarte/.rvm/gems/jruby-9.1.10.0/bin/rspec)
Gem::LoadError: You have already activated jar-dependencies 0.3.9, but your Gemfile requires jar-dependencies 0.3.11. Since jar-dependencies is a default gem, you can either remove your dependency on it or try updating to a newer version of bundler that supports jar-dependencies as a default gem.
  /Users/joaoduarte/.rvm/gems/jruby-9.1.10.0/gems/bundler-1.15.3/lib/bundler/runtime.rb:317:in `check_for_activated_spec!'
  /Users/joaoduarte/.rvm/gems/jruby-9.1.10.0/gems/bundler-1.15.3/lib/bundler/runtime.rb:32:in `block in setup'
  org/jruby/RubyArray.java:1734:in `each'
  org/jruby/RubyEnumerable.java:830:in `map'
  /Users/joaoduarte/.rvm/gems/jruby-9.1.10.0/gems/bundler-1.15.3/lib/bundler/runtime.rb:27:in `setup'
  /Users/joaoduarte/.rvm/gems/jruby-9.1.10.0/gems/bundler-1.15.3/lib/bundler.rb:101:in `setup'
  /Users/joaoduarte/.rvm/gems/jruby-9.1.10.0/gems/bundler-1.15.3/lib/bundler/setup.rb:9:in `<main>'
  org/jruby/RubyKernel.java:961:in `require'
  /Users/joaoduarte/.rvm/rubies/jruby-9.1.10.0/lib/ruby/stdlib/rubygems/core_ext/kernel_require.rb:54:in `require'

This may be related to mkristian/jar-dependencies#52, but I can't reproduce the issue on jruby 1.7.x, leading me to think it is a 9k problem.

@mkristian

This comment has been minimized.

Show comment
Hide comment
@mkristian

mkristian Aug 20, 2017

Member

@jsvd since I am basically offline until end of August - I can not have a look at until I am back to internet land.

Member

mkristian commented Aug 20, 2017

@jsvd since I am basically offline until end of August - I can not have a look at until I am back to internet land.

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Nov 10, 2017

Member

@mkristian Can you look at this now? :-)

Member

headius commented Nov 10, 2017

@mkristian Can you look at this now? :-)

@mkristian

This comment has been minimized.

Show comment
Hide comment
@mkristian

mkristian Nov 11, 2017

Member

@headius I got the reminder yesterday through the other issue already

Member

mkristian commented Nov 11, 2017

@headius I got the reminder yesterday through the other issue already

@mkristian

This comment has been minimized.

Show comment
Hide comment
@mkristian

mkristian Nov 11, 2017

Member

@headius so the problem is that bundler requires 'readline' inside its vendor thor, in the end it is never ever used but readline needs jar-dependencies to load its jars.

possible fixes:

  • bundler shall require its Thor::LineEditor lazy inside the Thor::Shell::Basic: but that is unlikely since it it needs to wait for the require until it is really needed - not sure if such a patch gets excepted and it is inside a vendor gem which is most likely unpatched (not sure here)
  • the jruby readline needs jline-2.11.jar we could vendor this jline jar an repakcing it under org.jruby then readline behaves more like MRI, i.e. not depending on other gems. it also comes with the advantage that we do not "enforce" our jline version to be used by other dependencies on the JRubyClassloader.

I would go for the second option as it is in our domain and its gives IMO improvement onjline issue we saw in the past.

it also relates to #3681 where we also had two solutions:

  • shading jzlib jar
  • make an extension gem out of zlib like readline

if am already repacking/shading things then I will also address #3821 in the same PR

my proposed actions would be:

  • get readline back into jruby codebase
  • repack all jars which are shaded into the jruby-core and do not have org.jruby package, i.e. we only want org.jruby classes in our jruby-core !

/cc @enebo

Member

mkristian commented Nov 11, 2017

@headius so the problem is that bundler requires 'readline' inside its vendor thor, in the end it is never ever used but readline needs jar-dependencies to load its jars.

possible fixes:

  • bundler shall require its Thor::LineEditor lazy inside the Thor::Shell::Basic: but that is unlikely since it it needs to wait for the require until it is really needed - not sure if such a patch gets excepted and it is inside a vendor gem which is most likely unpatched (not sure here)
  • the jruby readline needs jline-2.11.jar we could vendor this jline jar an repakcing it under org.jruby then readline behaves more like MRI, i.e. not depending on other gems. it also comes with the advantage that we do not "enforce" our jline version to be used by other dependencies on the JRubyClassloader.

I would go for the second option as it is in our domain and its gives IMO improvement onjline issue we saw in the past.

it also relates to #3681 where we also had two solutions:

  • shading jzlib jar
  • make an extension gem out of zlib like readline

if am already repacking/shading things then I will also address #3821 in the same PR

my proposed actions would be:

  • get readline back into jruby codebase
  • repack all jars which are shaded into the jruby-core and do not have org.jruby package, i.e. we only want org.jruby classes in our jruby-core !

/cc @enebo

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Nov 22, 2017

Member

@mkristian Ok so my main concern here is that Ruby 2.5 is gemifying basically everything, which means we're going to need to essentially follow that pattern. We'll want to figure out which gems we can just use, which need some JRuby patches, and which we'll have to replace altogether (probably still needing to push to those names). Moving readline back into core now means we'll just be moving it back out again.

I do agree we should probably start shading everything we ship and do not intend to export.

Member

headius commented Nov 22, 2017

@mkristian Ok so my main concern here is that Ruby 2.5 is gemifying basically everything, which means we're going to need to essentially follow that pattern. We'll want to figure out which gems we can just use, which need some JRuby patches, and which we'll have to replace altogether (probably still needing to push to those names). Moving readline back into core now means we'll just be moving it back out again.

I do agree we should probably start shading everything we ship and do not intend to export.

@mkristian

This comment has been minimized.

Show comment
Hide comment
@mkristian

mkristian Nov 22, 2017

Member

@headius OK then it is better we leave the readline gem as gem and just shade/repackage the jline and require it without the jar-dependencies. in general it is nice to share even the jars via jar-dependencies but jline was problematic in the past as well, and with this bundler issue, we just repackage it.

Member

mkristian commented Nov 22, 2017

@headius OK then it is better we leave the readline gem as gem and just shade/repackage the jline and require it without the jar-dependencies. in general it is nice to share even the jars via jar-dependencies but jline was problematic in the past as well, and with this bundler issue, we just repackage it.

mkristian added a commit that referenced this issue Nov 27, 2017

pick new jruby-readline
this will include the jline shaded and relocated in the readline.jar

fixes #4740

@headius headius added this to the JRuby 9.1.15.0 milestone Nov 29, 2017

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Nov 29, 2017

Member

This is good and safe for 9.1.15, right?

Member

headius commented Nov 29, 2017

This is good and safe for 9.1.15, right?

@mkristian

This comment has been minimized.

Show comment
Hide comment
@mkristian

mkristian Nov 29, 2017

Member

@headius if the build is green then it is good to go.

Member

mkristian commented Nov 29, 2017

@headius if the build is green then it is good to go.

mkristian added a commit that referenced this issue Dec 4, 2017

pick new jruby-readline
this will include the jline shaded and relocated in the readline.jar

fixes #4740
@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Dec 5, 2017

Member

Ship it!

Member

headius commented Dec 5, 2017

Ship it!

headius added a commit that referenced this issue Dec 5, 2017

pick new jruby-readline
this will include the jline shaded and relocated in the readline.jar

fixes #4740

@headius headius closed this Dec 5, 2017

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Dec 5, 2017

Member

@mkristian Please re-comment the staging repo once you've gotten the release out.

Member

headius commented Dec 5, 2017

@mkristian Please re-comment the staging repo once you've gotten the release out.

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