This repository has been archived by the owner on Apr 14, 2021. It is now read-only.
Return Bundler::StubSpec if stub is a Bundler::StubSpec #5630
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I'd like us to add an integration test as well, since in theory other weird things can happen when |
segiddins
reviewed
May 2, 2017
| if Bundler.rubygems.provides?(">= 2.1") | ||
| RSpec.describe Bundler::StubSpecification do | ||
| let(:with_gem_stub_spec) do | ||
| stub = Gem::Specification.stubs.first |
There was a problem hiding this comment.
this can return nil on old rubies on CI
segiddins
reviewed
May 2, 2017
| @@ -4,6 +4,7 @@ | |||
| module Bundler | |||
| class StubSpecification < RemoteSpecification | |||
| def self.from_stub(stub) | |||
| return stub if stub.is_a?(Bundler::StubSpecification) | |||
There was a problem hiding this comment.
with this, we should be able to delete the conditional at line 91
jules2689
force-pushed
the
jules2689-bundler-stub-spec
branch
from
f771cf5
to
c049b09
Compare
|
@segiddins removing the conditional on line 91 caused everything to break ( see the red failures on the commits ). I'm not able to investigate the failures in time for 1.15, would you be able to look at that and take this over if we want it for 1.15? (I'm about to be internetless for 2 weeks on vacation). Or, since @indirect is cool with the unit tests, we could continue with this as is, since it passes with the conditional |
|
👍 |
|
@bundlerbot r+ |
|
📌 Commit a542e2b has been approved by |
bundlerbot
added a commit
that referenced
this pull request
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
|
☀️ Test successful - status-travis |
segiddins
pushed a commit
that referenced
this pull request
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)
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Supersedes #5593
Fixes #5592
Explanation
In some cases the
Gem::Specification.stubscall in this method in the rubygems integration returns a mixed bag ofBundler::StubSpecificationandGem::StubSpecificationobjects. We then instantiateBundler::StubSpecificationobjects and set thestubto be bothGem::StubSpecificationandBundler::StubSpecificationobjects.This happens after we tell rubygems to use our overrides here.
A
Bundler::StubSpecificationdoes not defineto_specwhereGem::StubSpecificationdoes. InBundler::StubSpecificationwe assume thestubto be aGem::StubSpecificationrather than theto_spec-lessBundler::StubSpecification. This means that in_remote_specification, the call toto_spechere fails. This falls back tomethod_missinghere 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 toGem::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 execresulted in the issue being exasperated.You can however replicate with this:
We basically got to a point where we tried calling a method that doesn't exist on a
Bundler::StubSpecification, so_remote_specificationwas 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
to_speconBundler::StubSpecification, as is done in Add to_spec to stub_specification #5593stubis aGem::Specification. Therefore if we try to create aBundler::StubSpecificationwith the stub being aBundler::StubSpecification, we simply return that stub we already made instead.Thoughts
Bundler::StubSpecificationswhere you can followstuball the way up until it's no longer aBundler::StubSpecification. This means that the implementation is an accidental fix asto_specin Add to_spec to stub_specification #5593 actually just callsstub.to_spec- which, if the stub is aBundler:StubSpecification, would call thatBundler::StubSpecification, following the list up until we find aGem::StubSpecification.stub.to_specis called in_remote_specification, we always know it is aGem::StubSpecificationwhich has it defined.cc @segiddins