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

to_spec is broken on 1.15.0.pre.1 in some cases #5592

Closed
jules2689 opened this issue Apr 17, 2017 · 7 comments · Fixed by #5630
Closed

to_spec is broken on 1.15.0.pre.1 in some cases #5592

jules2689 opened this issue Apr 17, 2017 · 7 comments · Fixed by #5630
Assignees

Comments

@jules2689
Copy link
Contributor

https://gist.github.com/jules2689/2e406069d552f0f1f66cfffdeecbfc45

Bundler Version: 1.15.0.pre.1

The issue here seems to be that to_spec is undefined and we get infinite recursion in remote_specification.rb's method_missing method.

I'm going to look into this.

@jules2689 jules2689 self-assigned this Apr 17, 2017
@segiddins
Copy link
Member

bundle env is missing -- particularly which RubyGems version?

@jules2689
Copy link
Contributor Author

@jules2689
Copy link
Contributor Author

@segiddins still trying to replicate on my machine

@jules2689
Copy link
Contributor Author

jules2689 commented Apr 17, 2017

Environment

Bundler   1.15.0.pre.1
Rubygems  2.6.10
Ruby      2.3.3p222 (2016-11-21 revision 56859) [x86_64-darwin16]
GEM_HOME  /Users/joepym/.gem/ruby/2.3.3
GEM_PATH  /Users/joepym/.gem/ruby/2.3.3:/opt/rubies/2.3.3/lib/ruby/gems/2.3.0
Git       2.9.0
Platform  x86_64-darwin-16

Bundler settings

jobs
  Set for your local app (/Users/joepym/code/messenger-commerce/.bundle/config): "4"

@segiddins
Copy link
Member

Think I know the fix for this, I'll check after class

@colby-swandale
Copy link
Member

has this been fixed?

@segiddins
Copy link
Member

We've been unable to reproduce it in a test case

bundlerbot added a commit that referenced this issue May 8, 2017
Return Bundler::StubSpec if stub is a Bundler::StubSpec

Supersedes #5593
Fixes #5592

Explanation
---
In some cases the `Gem::Specification.stubs` call in [this method](https://github.com/bundler/bundler/blob/master/lib/bundler/rubygems_integration.rb#L773-L778) in the rubygems integration returns a mixed bag of `Bundler::StubSpecification` and `Gem::StubSpecification` objects. We then instantiate `Bundler::StubSpecification` objects and set the `stub` to be both `Gem::StubSpecification` and `Bundler::StubSpecification` objects.

This happens after we tell rubygems to use our overrides [here](https://github.com/bundler/bundler/blob/master/lib/bundler/runtime.rb#L21-L24).

A `Bundler::StubSpecification` does not define `to_spec` where `Gem::StubSpecification` does. In `Bundler::StubSpecification` we assume the `stub` to be a `Gem::StubSpecification` rather than the `to_spec`-less `Bundler::StubSpecification`. This means that in `_remote_specification`, the call to `to_spec` [here](https://github.com/bundler/bundler/blob/master/lib/bundler/stub_specification.rb#L88) fails. This falls back to `method_missing` [here](https://github.com/bundler/bundler/blob/master/lib/bundler/remote_specification.rb#L96-L98) which, of course, calls `_remote_specification` (and thus an infinite failing loops occurs).

### Why did this happen in such a weird way?

We needed to use a combination of `foreman`, `unicorn`, and a call to `Gem::Specification.find_by_name(*args)` to replicate.

I suspect this was required because Bundler doesn't call these methods as much. The last call in a doubly nested `bundle exec` resulted in the issue being exasperated.

You can however replicate with this:

```ruby
gem_stub = Gem::Specification.stubs.first
bundler_stub = Bundler::StubSpecification.from_stub(gem_stub)
bundler_stub = Bundler::StubSpecification.from_stub(bundler_stub)
bundler_stub.to_spec
```

We basically got to a point where we tried calling a method that doesn't exist on a `Bundler::StubSpecification`, so `_remote_specification` was called, but that had a method which didn't exist since we had the weirdness going on described here.

It was just a very specific sequence of events that is hard to replicate.

Options
---
1. We implement `to_spec` on `Bundler::StubSpecification`, as is done in #5593
2. We assume that `stub` is a `Gem::Specification`. Therefore if we try to create a `Bundler::StubSpecification` with the stub being a `Bundler::StubSpecification`, we simply return that stub we already made instead.

Thoughts
---
1. This basically ends up making a linked list of `Bundler::StubSpecifications` where you can follow `stub` all the way up until it's no longer a `Bundler::StubSpecification`. This means that the implementation is an accidental fix as `to_spec` in #5593 actually just calls `stub.to_spec` - which, if the stub is a `Bundler:StubSpecification`, would call that `Bundler::StubSpecification`, following the list up until we find a `Gem::StubSpecification`.
2. This is the right solution IMO. This breaks the weird linked list we made by mistake and just returns the object as we'd expect. Then, when `stub.to_spec` is called in `_remote_specification`, we always know it is a `Gem::StubSpecification` which has it defined.

cc @segiddins
segiddins pushed a commit that referenced this issue May 10, 2017
Return Bundler::StubSpec if stub is a Bundler::StubSpec

Supersedes #5593
Fixes #5592

Explanation
---
In some cases the `Gem::Specification.stubs` call in [this method](https://github.com/bundler/bundler/blob/master/lib/bundler/rubygems_integration.rb#L773-L778) in the rubygems integration returns a mixed bag of `Bundler::StubSpecification` and `Gem::StubSpecification` objects. We then instantiate `Bundler::StubSpecification` objects and set the `stub` to be both `Gem::StubSpecification` and `Bundler::StubSpecification` objects.

This happens after we tell rubygems to use our overrides [here](https://github.com/bundler/bundler/blob/master/lib/bundler/runtime.rb#L21-L24).

A `Bundler::StubSpecification` does not define `to_spec` where `Gem::StubSpecification` does. In `Bundler::StubSpecification` we assume the `stub` to be a `Gem::StubSpecification` rather than the `to_spec`-less `Bundler::StubSpecification`. This means that in `_remote_specification`, the call to `to_spec` [here](https://github.com/bundler/bundler/blob/master/lib/bundler/stub_specification.rb#L88) fails. This falls back to `method_missing` [here](https://github.com/bundler/bundler/blob/master/lib/bundler/remote_specification.rb#L96-L98) which, of course, calls `_remote_specification` (and thus an infinite failing loops occurs).

### Why did this happen in such a weird way?

We needed to use a combination of `foreman`, `unicorn`, and a call to `Gem::Specification.find_by_name(*args)` to replicate.

I suspect this was required because Bundler doesn't call these methods as much. The last call in a doubly nested `bundle exec` resulted in the issue being exasperated.

You can however replicate with this:

```ruby
gem_stub = Gem::Specification.stubs.first
bundler_stub = Bundler::StubSpecification.from_stub(gem_stub)
bundler_stub = Bundler::StubSpecification.from_stub(bundler_stub)
bundler_stub.to_spec
```

We basically got to a point where we tried calling a method that doesn't exist on a `Bundler::StubSpecification`, so `_remote_specification` was called, but that had a method which didn't exist since we had the weirdness going on described here.

It was just a very specific sequence of events that is hard to replicate.

Options
---
1. We implement `to_spec` on `Bundler::StubSpecification`, as is done in #5593
2. We assume that `stub` is a `Gem::Specification`. Therefore if we try to create a `Bundler::StubSpecification` with the stub being a `Bundler::StubSpecification`, we simply return that stub we already made instead.

Thoughts
---
1. This basically ends up making a linked list of `Bundler::StubSpecifications` where you can follow `stub` all the way up until it's no longer a `Bundler::StubSpecification`. This means that the implementation is an accidental fix as `to_spec` in #5593 actually just calls `stub.to_spec` - which, if the stub is a `Bundler:StubSpecification`, would call that `Bundler::StubSpecification`, following the list up until we find a `Gem::StubSpecification`.
2. This is the right solution IMO. This breaks the weird linked list we made by mistake and just returns the object as we'd expect. Then, when `stub.to_spec` is called in `_remote_specification`, we always know it is a `Gem::StubSpecification` which has it defined.

cc @segiddins

(cherry picked from commit 47e7dd0)
philipefarias added a commit to dleemoo/rc-images that referenced this issue Jun 12, 2017
Changes since last version used (1.14.6):

== 1.15.1 (2017-06-02)

Bugfixes:

  - `bundle lock --update GEM` will fail gracefully when the gem is not in the lockfile (rubygems/bundler#5693, @segiddins)
  - `bundle init --gemspec` will fail gracefully when the gemspec is invalid (@colby-swandale)
  - `bundle install --force` works when the gemfile contains git gems (rubygems/bundler#5678, @segiddins)
  - `bundle env` will print well-formed markdown when there are no settings (rubygems/bundler#5677, @segiddins)

== 1.15.0 (2017-05-19)

This space intentionally left blank.

== 1.15.0.pre.4 (2017-05-10)

Bugfixes:

  - avoid conflicts when `Gem.finish_resolve` is called after the bundle has been set up (@segiddins)
  - ensure that `Gem::Specification.find_by_name` always returns an object that can have `#to_spec` called on it (rubygems/bundler#5592, @jules2689)

== 1.15.0.pre.3 (2017-04-30)

Bugfixes:

  - avoid redundant blank lines in the readme generated by `bundle gem` (@koic)
  - ensure that `open-uri` is not loaded after `bundle exec` (@segiddins)
  - print a helpful error message when an activated default gem conflicts with
    a gem in the gemfile (@segiddins)
  - only shorten `ref` option for git gems when it is a SHA (rubygems/bundler#5620, @segiddins)

== 1.15.0.pre.2 (2017-04-23)

Bugfixes:

  - ensure pre-existing fit caches are updated from remote sources (rubygems/bundler#5423, @alextaylor000)
  - avoid duplicating specs in the lockfile after updating with the gem uninstalled (rubygems/bundler#5599, @segiddins)
  - ensure git gems have their extensions available at runtime (rubygems/bundler#5594, @jules2689, @segiddins)

== 1.15.0.pre.1 (2017-04-16)

Features:

  - print a notification when a newer version of bundler is available (rubygems/bundler#4683, @segiddins)
  - add man pages for all bundler commands (rubygems/bundler#4988, @feministy)
  - add the `bundle info` command (@fredrb, @colby-swandale)
  - all files created with `bundle gem` comply with the bundler style guide (@zachahn)
  - if installing a gem fails, print out the reason the gem needed to be installed (rubygems/bundler#5078, @segiddins)
  - allow setting `gem.push_key` to set the key used when running `rake release` (@DTrierweiler)
  - print gem versions that are regressing during `bundle update` in yellow (rubygems/bundler#5506, @brchristian)
  - avoid printing extraneous dependencies when the resolver encounters a conflict (@segiddins)
  - add the `bundle issue` command that prints instructions for reporting issues (rubygems/bundler#4871, @jonathanpike)
  - add `--source` and `--group` options to the `bundle inject` command (rubygems/bundler#5452, @Shekharrajak)
  - add the `bundle add` command to add a gem to the gemfile (@denniss)
  - add the `bundle pristine` command to re-install gems from cached `.gem` files (rubygems/bundler#4509, @denniss)
  - add a `--parseable` option for `bundle config` (@JuanitoFatas, @colby-swandale)

Performance:

  - speed up gemfile initialization by storing locked dependencies as a hash (@jules2689)
  - speed up gemfile initialization by making locked dependency comparison lazy, avoiding object allocation (@jules2689)
  - only validate git gems when they are downloaded, instead of every time `Bundler.setup` is run (@segiddins)
  - avoid regenerating the lockfile when nothing has changed (@segiddins)
  - avoid diffing large arrays when no sources in the gemfile have changed (@segiddins)
  - avoid evaluating full gemspecs when running with RubyGems 2.5+ (@segiddins)

Bugfixes:

  - fix cases where `bundle update` would print a resolver conflict instead of updating the selected gems (rubygems/bundler#5031, rubygems/bundler#5095, @segiddins)
  - print out a stack trace after an interrupt when running in debug mode (@segiddins)
  - print out when bundler starts fetching a gem from a remote server (@segiddins)
  - fix `bundle gem` failing when `git` is unavailable (rubygems/bundler#5458, @Shekharrajak, @colby-swandale)
  - suggest the appropriate command to unfreeze a bundle (rubygems/bundler#5009, @denniss)
  - ensure nested calls to `bundle exec` resolve default gems correctly (rubygems/bundler#5500, @segiddins)
  - ensure that a plugin failing to install doesn't uninstall other plugins (@kerrizor, @roseaboveit)
  - ensure `socket` is required before being referenced (rubygems/bundler#5533, @rafaelfranca)
  - allow running `bundle outdated` when gems aren't installed locally (rubygems/bundler#5553, @segiddins)
  - print a helpful error when `bundle exec`ing to a gem that isn't included in the bundle (rubygems/bundler#5487, @segiddins)
  - print an error message when a non-git gem is given a `branch` option (rubygems/bundler#5530, @colby-swandale)
  - allow interrupts to exit the process after gems have been installed (@segiddins)
  - print the underlying error when downloading gem metadata fails (rubygems/bundler#5579, @segiddins)
  - avoid deadlocking when installing with a lockfile that is missing dependencies (rubygems/bundler#5378, rubygems/bundler#5480, rubygems/bundler#5519, rubygems/bundler#5526, rubygems/bundler#5529, rubygems/bundler#5549, rubygems/bundler#5572, @segiddins)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants