/jruby

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

bundle install fails since #5215 #5280

Closed
yahonda opened this Issue Aug 14, 2018 · 5 comments

Comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
  JRuby 9.2.1.0
3 participants
@yahonda @headius @ChrisBr
@yahonda
Copy link

yahonda commented Aug 14, 2018 •
edited

Environment

  • JRuby version (jruby -v) and command line (flags, JRUBY_OPTS, etc)
$ jruby -v
jruby 9.2.1.0-SNAPSHOT (2.5.0) 2018-07-31 cbccb8d Java HotSpot(TM) 64-Bit Server VM 25.181-b13 on 1.8.0_181-b13 [linux-x86_64]
  • Operating system and platform (e.g. uname -a)
$ uname -a
Linux li1554-157.members.linode.com 4.15.0-29-generic #31-Ubuntu SMP Tue Jul 17 15:39:52 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux

Other relevant info you may wish to add:

  • Installed or activated gems - Rails
  • Application/framework version (e.g. Rails, Sinatra)
  • Environment variables
$ env |grep RUBY
JRUBY_OPTS=--dev -J-Xmx2048M
RUBY_CONFIGURE_OPTS=--disable-install-doc

Steps to reproduce

  • Create Gemfile
# frozen_string_literal: true

source "https://rubygems.org"

git_source(:github) {|repo_name| "https://github.com/#{repo_name}" }

gem "rails"
  • Execute gem install bundler
$ gem install bundler
  • Execute bundle install
$ bundle install

Expected Behavior

Bundle should pass.

Actual Behavior

NoMethodError: undefined method `dep' for #<Gem::Dependency:0x100c8b75>
Did you mean?  dup
  /home/yahonda/git/jruby/lib/ruby/gems/shared/gems/bundler-1.16.3/lib/bundler/dep_proxy.rb:18:in `=='
  org/jruby/ext/set/RubySet.java:639:in `add?'
  /home/yahonda/git/jruby/lib/ruby/gems/shared/gems/bundler-1.16.3/lib/bundler/spec_set.rb:27:in `block in for'
  org/jruby/RubyKernel.java:1412:in `loop'
  /home/yahonda/git/jruby/lib/ruby/gems/shared/gems/bundler-1.16.3/lib/bundler/spec_set.rb:25:in `for'
  /home/yahonda/git/jruby/lib/ruby/gems/shared/gems/bundler-1.16.3/lib/bundler/definition.rb:810:in `converge_locked_specs'
  /home/yahonda/git/jruby/lib/ruby/gems/shared/gems/bundler-1.16.3/lib/bundler/definition.rb:248:in `resolve'
  /home/yahonda/git/jruby/lib/ruby/gems/shared/gems/bundler-1.16.3/lib/bundler/definition.rb:204:in `missing_specs'
  /home/yahonda/git/jruby/lib/ruby/gems/shared/gems/bundler-1.16.3/lib/bundler/definition.rb:209:in `missing_specs?'
  /home/yahonda/git/jruby/lib/ruby/gems/shared/gems/bundler-1.16.3/lib/bundler/installer.rb:284:in `resolve_if_needed'
  /home/yahonda/git/jruby/lib/ruby/gems/shared/gems/bundler-1.16.3/lib/bundler/installer.rb:83:in `block in run'
  /home/yahonda/git/jruby/lib/ruby/gems/shared/gems/bundler-1.16.3/lib/bundler/process_lock.rb:12:in `block in lock'
  org/jruby/RubyIO.java:1154:in `open'
  /home/yahonda/git/jruby/lib/ruby/gems/shared/gems/bundler-1.16.3/lib/bundler/process_lock.rb:9:in `lock'
  /home/yahonda/git/jruby/lib/ruby/gems/shared/gems/bundler-1.16.3/lib/bundler/installer.rb:72:in `run'
  /home/yahonda/git/jruby/lib/ruby/gems/shared/gems/bundler-1.16.3/lib/bundler/installer.rb:25:in `install'
  /home/yahonda/git/jruby/lib/ruby/gems/shared/gems/bundler-1.16.3/lib/bundler/cli/install.rb:65:in `run'
  /home/yahonda/git/jruby/lib/ruby/gems/shared/gems/bundler-1.16.3/lib/bundler/cli.rb:224:in `block in install'
  /home/yahonda/git/jruby/lib/ruby/gems/shared/gems/bundler-1.16.3/lib/bundler/settings.rb:136:in `temporary'
  /home/yahonda/git/jruby/lib/ruby/gems/shared/gems/bundler-1.16.3/lib/bundler/cli.rb:223:in `install'
  /home/yahonda/git/jruby/lib/ruby/gems/shared/gems/bundler-1.16.3/lib/bundler/vendor/thor/lib/thor/command.rb:27:in `run'
  /home/yahonda/git/jruby/lib/ruby/gems/shared/gems/bundler-1.16.3/lib/bundler/vendor/thor/lib/thor/invocation.rb:126:in `invoke_command'
  /home/yahonda/git/jruby/lib/ruby/gems/shared/gems/bundler-1.16.3/lib/bundler/vendor/thor/lib/thor.rb:387:in `dispatch'
  /home/yahonda/git/jruby/lib/ruby/gems/shared/gems/bundler-1.16.3/lib/bundler/cli.rb:27:in `dispatch'
  /home/yahonda/git/jruby/lib/ruby/gems/shared/gems/bundler-1.16.3/lib/bundler/vendor/thor/lib/thor/base.rb:466:in `start'
  /home/yahonda/git/jruby/lib/ruby/gems/shared/gems/bundler-1.16.3/lib/bundler/cli.rb:18:in `start'
  /home/yahonda/git/jruby/lib/ruby/gems/shared/gems/bundler-1.16.3/exe/bundle:30:in `block in <main>'
  /home/yahonda/git/jruby/lib/ruby/gems/shared/gems/bundler-1.16.3/lib/bundler/friendly_errors.rb:124:in `with_friendly_errors'
  /home/yahonda/git/jruby/lib/ruby/gems/shared/gems/bundler-1.16.3/exe/bundle:22:in `<main>'
  org/jruby/RubyKernel.java:997:in `load'
  /home/yahonda/git/jruby/bin/bundle:1:in `<main>'
@yahonda

This comment has been minimized.

Sign in to view
Copy link
Author

yahonda commented Aug 14, 2018

This issue has been triggered since #5215. Then reverting this merge commit workaround this issue.

$ git revert -m1 e483fb2251845b7bb3ff1ac99aa5760699438ffe

@yahonda yahonda referenced this issue Aug 14, 2018

Closed

Build using jruby-head has been failing `NoMethodError: undefined method `dep' for #<Gem::Dependency:0x7ee955a8>` #9994

@headius

This comment has been minimized.

Sign in to view
Copy link
Member

headius commented Aug 16, 2018

@yahonda Thank you for the report!

@ChrisBr Guess we still have a few kinks to work out.

@headius headius added this to the JRuby 9.2.1.0 milestone Aug 16, 2018

@ChrisBr

This comment has been minimized.

Sign in to view
Copy link
Contributor

ChrisBr commented Aug 20, 2018

Grml 😢 I will look into it @headius ...

@fenrir-naru fenrir-naru referenced this issue Aug 22, 2018

Closed

jruby-head CI tests are sometimes broken #39

ChrisBr added a commit to ChrisBr/bundler that referenced this issue Aug 22, 2018

@ChrisBr
Fix DepProxy == method 
Checking now also that other class is the same as self.
Otherwise it will crash for anything other than DepProxy class
or nil.

Fixes jruby/jruby#5280 travis-ci/travis-ci#9994
Unverified
The committer email address is not verified.
Learn about signing commits
fdd15fa

@ChrisBr ChrisBr referenced this issue Aug 22, 2018

Merged

Fix DepProxy == method #6669

@ChrisBr

This comment has been minimized.

Sign in to view
Copy link
Contributor

ChrisBr commented Aug 22, 2018

Ok, I tracked this down now. This is a combination of two things:

  1. When using the open addressing strategy, we do not cache the hash value of the keys anymore (we only do that for linear search). This means when a collision happens, we do == on the keys.

  2. In bundler the DepProxy class implements == but does not check the class of the other object. This is combination with 1) causes this error.

I already submitted a fix to bundler, see bundler/bundler#6669. However, probably we need to fix this on JRuby side as well as we don't know if bundler accepts the fix and when the will release a new version (and also we can not rely that all our users update to the new bundler version ...) 😢 So the only solution I see for now is that we always cache the hash value now.

cc @headius @enebo what do you think?

ChrisBr added a commit to ChrisBr/bundler that referenced this issue Aug 22, 2018

@ChrisBr
Fix DepProxy#== undefind method error 
DepProxy#== crashed with an undefind method error
for anything other than a DepProxy class or nil
as parameter. This was caused that it was assumed
only DepProxy instances or nil can get passed as
parameter. This commit implements checking the class
as well and returns false if the classes are not
the same.

Fixes jruby/jruby#5280 travis-ci/travis-ci#9994
Unverified
The committer email address is not verified.
Learn about signing commits
b939e3c

ChrisBr added a commit to ChrisBr/bundler that referenced this issue Aug 23, 2018

@ChrisBr
Fix DepProxy#== undefind method error 
DepProxy#== crashed with an undefind method error
for anything other than a DepProxy class or nil
as parameter. This was caused that it was assumed
only DepProxy instances or nil can get passed as
parameter. This commit implements checking the class
as well and returns false if the classes are not
the same.

Fixes jruby/jruby#5280 travis-ci/travis-ci#9994
Unverified
The committer email address is not verified.
Learn about signing commits
fd43f11

ChrisBr added a commit to ChrisBr/jruby that referenced this issue Aug 23, 2018

@ChrisBr
Cache hash values for open addressing 
To save memory we decided to not cache the hash value of the keys anymore
when using open addressing as this was only necessary for a fast bucket
skip when a bin collision happens. With the current approach, when a bin collision
happens we call eql? on the keys. This was reasonable as this only happens
when a collision occours.

However, with this approach we always call eql? when a bin collision happens even
when the hashes are different as we only use the lower bits of the hash
to calculate the bin where the key / value pair is stored. This causes now
that wrong implementation of eql? will crash always on a bin collisions and not
only on a bin AND hash collision.

As this is the case for bundler, there is no other way than caching the hash values
again to reduce the probability that this happens.

Fixes jruby#5280 travis-ci/travis-ci#9994
Unverified
This user has not uploaded their public key yet.
GPG key ID: 168295FC108C44E5 Learn about signing commits
d6fde97

@ChrisBr ChrisBr referenced this issue Aug 23, 2018

Merged

Implement hash cache to fix #5280 #5289

@enebo enebo closed this in #5289 Aug 24, 2018

enebo added a commit that referenced this issue Aug 24, 2018

@enebo
Merge pull request #5289 from ChrisBr/hash/cache-key 
Implement hash cache to fix #5280
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
067b279

@Skarlso Skarlso referenced this issue Aug 24, 2018

Merged

Delete elements with path #95

@yahonda

This comment has been minimized.

Sign in to view
Copy link
Author

yahonda commented Aug 25, 2018

Thanks for the fix. I have confirmed this issue has been addressed by this version.

$ ruby -v
jruby 9.2.1.0-SNAPSHOT (2.5.0) 2018-07-31 067b279 Java HotSpot(TM) 64-Bit Server VM 25.181-b13 on 1.8.0_181-b13 [linux-x86_64]

bundlerbot added a commit to bundler/bundler that referenced this issue Aug 27, 2018

@bundlerbot
Auto merge of #6669 - ChrisBr:fix_dep_proxy, r=segiddins 
Fix DepProxy == method

### What was the end-user problem that led to this PR?
After implementing a new hash table strategy in JRuby, bundle is broken for JRuby. The problem is caused that the ``==`` method in bundler does not check the class of the ``other`` object. This causes problems now when calling ``==`` on object other than DepProxy or nil.

 jruby/jruby#5280 travis-ci/travis-ci#9994

### What was your diagnosis of the problem?
The code crashes for anything other than DepProxy class or nil.

### What is your fix for the problem, implemented in this PR?
Checking now also that other class is the same as self.
Loading status checks…
1856133

colby-swandale added a commit to bundler/bundler that referenced this issue Sep 14, 2018

@bundlerbot @colby-swandale
Auto merge of #6669 - ChrisBr:fix_dep_proxy, r=segiddins 
Fix DepProxy == method

### What was the end-user problem that led to this PR?
After implementing a new hash table strategy in JRuby, bundle is broken for JRuby. The problem is caused that the ``==`` method in bundler does not check the class of the ``other`` object. This causes problems now when calling ``==`` on object other than DepProxy or nil.

 jruby/jruby#5280 travis-ci/travis-ci#9994

### What was your diagnosis of the problem?
The code crashes for anything other than DepProxy class or nil.

### What is your fix for the problem, implemented in this PR?
Checking now also that other class is the same as self.

(cherry picked from commit 1856133)
eb0f3b3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment