Skip to content
This repository has been archived by the owner on Apr 14, 2021. It is now read-only.

Error in Bundler 1.13 with JRuby 1.7.x #4975

Closed
petr666 opened this issue Sep 13, 2016 · 22 comments
Closed

Error in Bundler 1.13 with JRuby 1.7.x #4975

petr666 opened this issue Sep 13, 2016 · 22 comments

Comments

@petr666
Copy link

petr666 commented Sep 13, 2016

Calling

Bundler.require

fails with

Gem Load Error is: private method require' called for Kernel:Module Backtrace for gem load error is: /home/test/.gem/gems/bundler-1.13.0/lib/bundler/runtime.rb:91:inrequire'
org/jruby/RubyArray.java:1613:in each' /home/test/.gem/gems/bundler-1.13.0/lib/bundler/runtime.rb:86:inrequire'
org/jruby/RubyArray.java:1613:in each' /home/test/.gem/gems/bundler-1.13.0/lib/bundler/runtime.rb:75:inrequire'
/home/test/.gem/gems/bundler-1.13.0/lib/bundler.rb:106:in `require'

The problem is in method reverse_rubygems_kernel_mixin from rubygems_integration.rb which is different from Bundler 1.12. When method implementation from 1.13 is replaced by implementation from 1.12, Bundler is working again.

It might be generic problem with Ruby 1.9 and not specifically JRuby 1.7.x, but I have no environment with MRI 1.9 to test it.

@segiddins
Copy link
Member

What's the output of bundle env?

@petr666
Copy link
Author

petr666 commented Sep 13, 2016

For JRuby 1.7.26 (latest 1.7.x release):

Bundler   1.13.0
Rubygems  2.4.8
Ruby      1.9.3p551 (2016-08-26 revision 48406) [java]
GEM_HOME  /home/test/.gem
GEM_PATH  
Git       2.7.4

I also forgot to mention, than Bundler 1.13 works under JRuby 9k:

Bundler   1.13.0
Rubygems  2.6.6
Ruby      2.3.1p0 (2016-09-07 revision 54768) [java]
GEM_HOME  /home/uni430/.gem
GEM_PATH  
Git       2.7.4

@jayjlawrence
Copy link

jayjlawrence commented Sep 13, 2016

I am experiencing the exact same issue. Manifests in bundler 1.13.1 as well. Dropped back to 1.12.5 and bundler is now functioning correctly.

Here's another stack trace if that is helpful.

Using /var/lib/jenkins/.rvm/gems/jruby-1.7.24
rake aborted!
Bundler::GemRequireError: There was an error while trying to load the gem 'rails'.
Gem Load Error is: private method `require' called for Kernel:Module
Backtrace for gem load error is:
/var/lib/jenkins/.rvm/gems/jruby-1.7.24/gems/bundler-1.13.1/lib/bundler/runtime.rb:91:in `require'
org/jruby/RubyArray.java:1613:in `each'
/var/lib/jenkins/.rvm/gems/jruby-1.7.24/gems/bundler-1.13.1/lib/bundler/runtime.rb:86:in `require'
org/jruby/RubyArray.java:1613:in `each'
/var/lib/jenkins/.rvm/gems/jruby-1.7.24/gems/bundler-1.13.1/lib/bundler/runtime.rb:75:in `require'
/var/lib/jenkins/.rvm/gems/jruby-1.7.24/gems/bundler-1.13.1/lib/bundler.rb:106:in `require'
/var/lib/jenkins/jobs/amp_15/workspace/config/application.rb:15:in `(root)'
org/jruby/RubyKernel.java:1040:in `require'
/var/lib/jenkins/jobs/amp_15/workspace/Rakefile:1:in `(root)'
org/jruby/RubyKernel.java:1059:in `load'
/var/lib/jenkins/jobs/amp_15/workspace/Rakefile:3:in `(root)'
/var/lib/jenkins/.rvm/gems/jruby-1.7.24/gems/rake-11.1.2/lib/rake/rake_module.rb:1:in `(root)'
/var/lib/jenkins/.rvm/gems/jruby-1.7.24/gems/rake-11.1.2/lib/rake/rake_module.rb:28:in `load_rakefile'
/var/lib/jenkins/.rvm/gems/jruby-1.7.24/gems/rake-11.1.2/lib/rake/application.rb:689:in `raw_load_rakefile'
/var/lib/jenkins/.rvm/gems/jruby-1.7.24/gems/rake-11.1.2/lib/rake/application.rb:94:in `load_rakefile'
/var/lib/jenkins/.rvm/gems/jruby-1.7.24/gems/rake-11.1.2/lib/rake/application.rb:176:in `standard_exception_handling'
/var/lib/jenkins/.rvm/gems/jruby-1.7.24/gems/rake-11.1.2/lib/rake/application.rb:93:in `load_rakefile'
/var/lib/jenkins/.rvm/gems/jruby-1.7.24/gems/rake-11.1.2/lib/rake/application.rb:77:in `run'
/var/lib/jenkins/.rvm/gems/jruby-1.7.24/gems/rake-11.1.2/lib/rake/application.rb:176:in `standard_exception_handling'
org/jruby/RubyKernel.java:1059:in `load'
/var/lib/jenkins/.rvm/gems/jruby-1.7.24/gems/rake-11.1.2/lib/rake/application.rb:75:in `run'
org/jruby/RubyKernel.java:1079:in `eval'
/var/lib/jenkins/.rvm/gems/jruby-1.7.24/gems/rake-11.1.2/bin/rake:33:in `(root)'
Bundler Error Backtrace:
/var/lib/jenkins/.rvm/gems/jruby-1.7.24/gems/bundler-1.13.1/lib/bundler/runtime.rb:95:in `require'
/var/lib/jenkins/.rvm/gems/jruby-1.7.24/gems/bundler-1.13.1/lib/bundler/runtime.rb:86:in `require'
/var/lib/jenkins/.rvm/gems/jruby-1.7.24/gems/bundler-1.13.1/lib/bundler/runtime.rb:75:in `require'
/var/lib/jenkins/.rvm/gems/jruby-1.7.24/gems/bundler-1.13.1/lib/bundler.rb:106:in `require'
/var/lib/jenkins/jobs/amp_15/workspace/config/application.rb:15:in `(root)'
/var/lib/jenkins/jobs/amp_15/workspace/Rakefile:1:in `(root)'
/var/lib/jenkins/jobs/amp_15/workspace/Rakefile:3:in `(root)'

And here is top of my bundler env output

Environment

    Bundler   1.12.5
    Rubygems  2.4.8
    Ruby      1.9.3p551 (2016-01-20 revision 48406) [java]
    GEM_HOME  /var/lib/jenkins/.rvm/gems/jruby-1.7.24
    GEM_PATH  /var/lib/jenkins/.rvm/gems/jruby-1.7.24:/var/lib/jenkins/.rvm/gems/jruby-1.7.24@global
    RVM       1.24.4 (master)
    Git       1.7.9.5
    rubygems-bundler (1.4.4)

@segiddins
Copy link
Member

Does that version of jruby have a different ACL level for Kernel.require than everything else?

@jayjlawrence
Copy link

Hi Samuel, I've spend a few minutes in the Jruby source before :) I had a look at the RubyKernel.java class and it looks like Kernel.require is marked as private

    /**
     * Require.
     * MRI allows to require ever .rb files or ruby extension dll (.so or .dll depending on system).
     * we allow requiring either .rb files or jars.
     * @param recv ruby object used to call require (any object will do and it won't be used anyway).
     * @param name the name of the file to require
     **/
    @JRubyMethod(module = true, visibility = PRIVATE, compat = RUBY1_8)
    public static IRubyObject require(IRubyObject recv, IRubyObject name, Block block) {
        return requireCommon(recv.getRuntime(), recv, name, block);
    }

    @JRubyMethod(name = "require", module = true, visibility = PRIVATE, compat = RUBY1_9)
    public static IRubyObject require19(ThreadContext context, IRubyObject recv, IRubyObject name, Block block) {
        Ruby runtime = context.runtime;
        IRubyObject tmp = name.checkStringType();

        if (!tmp.isNil()) return requireCommon(runtime, recv, tmp, block);

        return requireCommon(runtime, recv, RubyFile.get_path(context, name), block);
    }

By my read MRI Kernel.require is public. Do you want me to raise this with the Jruby team?

Regards,
Jay

@jayjlawrence
Copy link

Hi Samuel,

I'm a bit perplexed here. Chatted briefly on IRC and someone mentioned Kernel.public_methods - it does show require. And I confirmed via IRB that I can invoke Kernel.require 'fubar.rb' with no difficultly.

I am wondering if something in Bundler is impacting the Kernel methods? Maybe as Bundler fires up you can isolate the culprit by putting "Kernel.require 'fubar.rb'" statements that will fail at first for file not found and then eventually for private method. Yuck, I know, but may be necessary to hunt down an issue in other people's code.

I think we'll need to bring this to the JRuby maintainers because the cause is not entirely clear.

Your thoughts?
Jay

@petr666
Copy link
Author

petr666 commented Sep 14, 2016

So I've tried MRI 1.9.3 in Docker and Bundler works. So it is really only problem of JRuby 1.7.x. And yes, #jayjlawrence, something in Bundler is impacting Kernel methods, see my first comment ;o) I found it can be simply tested like this:

Kernel.public_methods.include?(:require) # returns true

require 'bundler/rubygems_integration'
Bundler::RubygemsIntegration.new.reverse_rubygems_kernel_mixin

Kernel.public_methods.include?(:require) # now returns false in Jruby 1.7.x

The previous implementation of reverse_rubygems_kernel_mixin used alias_method, while current implementation is done via remove_method/define_method which obviously does not play well with JRuby 1.7.x

@jayjlawrence
Copy link

jayjlawrence commented Sep 14, 2016

A similar issue was reported. Here is a proposed workaround:

Kernel.send(:require, "<name>")

See jruby/jruby#4155

@jayjlawrence
Copy link

jayjlawrence commented Sep 14, 2016

I propose a couple things.

First is to assess if the Kernel.send clears up the issue in bundler and at least release a revision because lots of JRuby users are going to get bit by this issue.

The second is to package up @petr666's minimal example and submit as a jruby bug. Clearly the Kernel.require method is getting 'corrupted' as part of the execution of Bundler::RubygemsIntegration.new.reverse_rubygems_kernel_mixin and maybe this is enough of a smoking gun for the JRuby folks to figure out what went wrong.

I am happy to report the matter to JRuby or please feel free to submit the issue as you see fit.

@jayjlawrence
Copy link

Actually this is being discussed on IRC now. It looks like they might have an idea of what's going wrong...

@olleolleolle
Copy link
Member

@jayjlawrence It's awesome that you keep reporting and compiling the details.

@jayjlawrence
Copy link

@segiddins possibly saving you 5 minutes - I have confirmed that Kernel.send(:require, fname) works. Note that there are two places in runtime.rb where Kernel.require is called.

This was using JRuby 1.7.24 and Bundler 1.13.1.

@jayjlawrence
Copy link

@segiddins I agree with @headius on JRuby team that the best approach is switch to Kernel.send and do a quick release of Bundler. There will be many JRuby deploys that will be impacted by this issue.

Over to you.

@headius
Copy link
Contributor

headius commented Sep 14, 2016

I have a new solution...modify the logic in redefine_method to do an additional k.send(:public, method). The Kerne.send approach works, but it will leave Kernel.require private which we don't want to do.

@headius
Copy link
Contributor

headius commented Sep 14, 2016

To summarize what I posted in jruby/jruby#4155...

In JRuby, method visibility is attached to the method rather than the method table, so pulling a private method off one method table and define_methoding it into another one does not reset the visibility to public. In MRI, where visibility is kept in the method table (i.e. it is an attribute of the association between a name and a method), transplanting a method gives it the default visibility, which is usually public.

This is a bug, and one we've known about...but nobody ever ran into it because they weren't transplanting methods this way (or at least, they didn't require them to become more visible).

This issue could have been avoided in a few ways:

  • Use alias_method to put the original method back where it was. It would retain its public visibility.
  • Use class_eval and define a new public method that calls the private one.
  • Test bundler on JRuby :-)

We will fix the root issue in JRuby, but we can't do anything about past JRuby releases in the wild. Therefore we request that bundler use my send(:public ... fix above to ensure the new method is defined as a public method. It will do no harm on any implementation.

@jayjlawrence
Copy link

From @headius 's recommendation above here is a proposed patch

--- bundler-1.13.1/lib/bundler/rubygems_integration.rb  2016-09-14 11:15:36.000000000 -0400
+++ bundler-copy/lib/bundler/rubygems_integration.rb    2016-09-14 11:15:55.000000000 -0400
@@ -301,6 +301,7 @@
       [kernel, ::Kernel].each do |k|
         if k.private_method_defined?(:gem_original_require)
           redefine_method(k, :require, k.instance_method(:gem_original_require))
+          k.send(:public, :require)
         end
       end
     end

@headius
Copy link
Contributor

headius commented Sep 14, 2016

@jayjlawrence Thank you for confirming and providing a patch!

Given that redefine_method should be producing all public methods, you might want the send(:public ... to be in redefine_method. That will fix any other cases we haven't discovered yet.

@headius
Copy link
Contributor

headius commented Sep 14, 2016

The root issue in JRuby was fixed in jruby/jruby@9b4e97b, which went into 9.1.3.0; versions >= 9.1.3.0 should not be affected. I'm backporting the fix to the 1.7.x line now.

@segiddins
Copy link
Member

@headius thanks for the explanation! We tried to get the test suite running under jruby on travis but given the reliance on subprocess exec'ing, it just wasn't feasible given the JVM boot time. I have an idea for how to fix this, I'll try and pull together a PR now

@segiddins
Copy link
Member

Feedback on #4983 would be appreciated, I don't have the bandwidth right now to download a new ruby

@headius
Copy link
Contributor

headius commented Sep 14, 2016

@segiddins Yeah I'm trying to run the suite locally and it takes a dog's age to finish. We may be able to mitigate that by installing Drip, which will spin up the "next" subprocess JRuby in the background so it's ready when we need it.

@coilysiren coilysiren added this to the 1.13 milestone Sep 14, 2016
rick added a commit to puppetlabs/vmpooler that referenced this issue Sep 14, 2016
homu added a commit that referenced this issue Sep 14, 2016
[RubygemsIntegration] Ensure redefined methods have the same visibility

Closes #4975

\c @indirect @headius

I'd love to get this out as 1.13.2 asap, if it works
homu added a commit that referenced this issue Sep 16, 2016
[RubygemsIntegration] Ensure redefined methods have the same visibility

Closes #4975

\c @indirect @headius

I'd love to get this out as 1.13.2 asap, if it works
yujideveloper added a commit to yujideveloper/health_check that referenced this issue Sep 28, 2016
This is necessary until the bundler + JRuby issues are resolved:

* rubygems/bundler#4975
* bundler/bundler#4984
yujideveloper added a commit to yujideveloper/health_check that referenced this issue Sep 28, 2016
This is necessary until the bundler + JRuby issues are resolved:

* rubygems/bundler#4975
* bundler/bundler#4984
yujideveloper added a commit to yujideveloper/health_check that referenced this issue Sep 28, 2016
This is necessary until the bundler + JRuby issues are resolved:

* rubygems/bundler#4975
* bundler/bundler#4984
yujideveloper added a commit to yujideveloper/health_check that referenced this issue Sep 28, 2016
This is necessary until the bundler + JRuby issues are resolved:

* rubygems/bundler#4975
* bundler/bundler#4984
yujideveloper added a commit to yujideveloper/health_check that referenced this issue Sep 28, 2016
This is necessary until the bundler + JRuby issues are resolved:

* rubygems/bundler#4975
* bundler/bundler#4984
yujideveloper added a commit to yujideveloper/health_check that referenced this issue Sep 28, 2016
This is necessary until the bundler + JRuby issues are resolved:

* rubygems/bundler#4975
* bundler/bundler#4984
yujideveloper added a commit to yujideveloper/health_check that referenced this issue Sep 28, 2016
yujideveloper added a commit to yujideveloper/health_check that referenced this issue Sep 28, 2016
Fix to bypass these bundler + JRuby bugs:

* rubygems/bundler#4975
* bundler/bundler#4984
ianheggie added a commit to Purple-Devs/health_check that referenced this issue Sep 29, 2016
Fix to bypass these bundler + JRuby bugs:

* rubygems/bundler#4975
* bundler/bundler#4984

Conflicts:
	.travis.yml
@synth
Copy link

synth commented Sep 29, 2016

To whom it may concern,

We ran into this issue when running our test suite on Travis-CI. It took many hours of hair pulling, but finally figured out the incantation which I shall leave here for others :)

before_install:
  - gem update --system
  - gem uninstall -i /home/travis/.rvm/gems/jruby-d19@global bundler
  - gem install bundler -v 1.12.5 --no-rdoc --no-ri --no-document

segiddins pushed a commit that referenced this issue Sep 30, 2016
[RubygemsIntegration] Ensure redefined methods have the same visibility

Closes #4975

\c @indirect @headius

I'd love to get this out as 1.13.2 asap, if it works
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants