Add to_spec to stub_specification

[jules2689]

Fixes #5592

Interestingly enough, I did not hit this case in Shopify core but a smaller app due to a gem's loading of Gems.

I think this is caused by the gem loading the gem after we initialize our caches. This causes the stub to be of type Bundler::StubSpecification, which means we need to also delegate to_spec to the stub's stub.

cc @segiddins

[jules2689]

Not sure how I'd write a spec for this though :/

[segiddins]

Unit test this, asserting that StubSpec#to_spec returns the right thing

[segiddins]

This maybe should return self?

[jules2689]

stub != self though, one is a Bundler::StubSpecification and the other is Gem::StubSpecification.

18:03:57 web.1      | ===
18:03:57 web.1      | Gem::StubSpecification
18:03:57 web.1      | Bundler::StubSpecification
18:03:57 web.1      | ===

From adding

      STDOUT.puts "==="
      STDOUT.puts stub.class.to_s
      STDOUT.puts self.class.to_s
      STDOUT.puts "==="

[jules2689]

Ah yea, normally we do this:

stub = Gem::StubSpecification.gemspec_stub(file, install_path.parent, install_path.parent)
stub.full_gem_path = Pathname.new(file).dirname.expand_path(root).to_s.untaint
StubSpecification.from_stub(stub)

But on second load, we have stub which is not a Gem::StubSpecification

[jules2689]

perhaps another option is

return stub if stub.is_a?(Bundler::StubSpecification)
 spec = new(stub.name, stub.version, stub.platform, nil)
 spec.stub = stub
 spec

[segiddins]

https://travis-ci.org/bundler/bundler/jobs/222969187#L453

[jules2689]

yea, I wasn't sure if it would actually evaluate it. Need a real gem to use now.

[segiddins]

Just stub the stub specification?

[jules2689]

@segiddins got it. 👍 Thanks for the suggestion, got me on the right path.

  let(:with_gem_stub_spec) do
    stub = Gem::Specification.stubs.first
    described_class.from_stub(stub)
  end

  let(:with_bundler_stub_spec) do
    described_class.from_stub(with_gem_stub_spec)
  end

[segiddins]

You'll probably need to restrict this spec based on RubyGems version

[jules2689]

Curious, why?

[segiddins]

Because old versions of RubyGems didn't have stubs

[jules2689]

Do you know which version or where to check?

[segiddins]

2.1

[segiddins]

this should be RSpec.describe Bundler::StubSpecification, :rubygems => ">= 2.1" do

[jules2689]

Thanks! Is there a doc explaining this DSL at all?

[segiddins]

https://github.com/bundler/bundler/blob/f153ef72f8572eee86e8b1672e29a6a206efb91a/spec/spec_helper.rb#L91

[segiddins]

watch out, this is returning nil in certain cases on CI... I'd create a new spec, serialize it to disk, and then create a stub from that known spec

[segiddins]

I dont understand the point of this... the Bundler::StubSpecification should only ever be passed a Gem::StubSpecification

[jules2689]

The issue is that in some cases stub ends up being Bundler::StubSpecification. When someone loads a gem with Gem::Specification.find_by_name(*args) after require 'bundler/setup', it loads it again with a cache (it seems).

[segiddins]

ah

[segiddins]

in that case, we should have a test that Gem::Specification.find_by_name also works

[segiddins]

why the install? since this is a unit test, it shouldn't be invoking anything

[jules2689]

I was trying to address #5593 (comment)

I'm not familiar with the DSL used in the tests :/

[segiddins]

diff --git a/spec/bundler/stub_specification_spec.rb b/spec/bundler/stub_specification_spec.rb
index b0fec7570..81e601405 100644
--- a/spec/bundler/stub_specification_spec.rb
+++ b/spec/bundler/stub_specification_spec.rb
@@ -1,22 +1,43 @@
 # frozen_string_literal: true
 require "spec_helper"
 
-if Bundler.rubygems.provides?(">= 2.1")
-  RSpec.describe Bundler::StubSpecification do
-    let(:with_gem_stub_spec) do
-      stub = Gem::Specification.stubs.first
-      described_class.from_stub(stub)
+RSpec.describe Bundler::StubSpecification, :rubygems => ">= 2.1" do
+  let(:spec) do
+    build_spec("foo", "1.0").first
+  end
+
+  let(:path) do
+    tmp("foo.gemspec")
+  end
+
+  before do
+    path.open("w") {|f| f << spec.to_ruby }
+  end
+
+  let(:stub) do
+    if Gem::StubSpecification.respond_to?(:gemspec_stub)
+      Gem::StubSpecification.gemspec_stub(path.to_s, path.parent.parent, path.parent.parent)
+    else
+      Gem::StubSpecification.new(path.to_s)
     end
+  end
+
+  subject do
+    described_class.from_stub(stub)
+  end
 
-    let(:with_bundler_stub_spec) do
-      described_class.from_stub(with_gem_stub_spec)
+  describe "#to_spec" do
+    it "returns the Gem::Specification" do
+      expect(subject.to_spec).to eq(spec)
     end
+  end
+
+  context "when the stub is initialized with a #{described_class}" do
+    subject { described_class.from_stub(super()) }
 
     describe "#to_spec" do
-      it "returns a Gem::Specification" do
-        bundle :install
-        expect(with_gem_stub_spec.to_spec).to be_a(Gem::Specification)
-        expect(with_bundler_stub_spec.to_spec).to be_a(Gem::Specification)
+      it "returns the Gem::Specification" do
+        expect(subject.to_spec).to eq(spec)
       end
     end
   end
diff --git a/spec/runtime/setup_spec.rb b/spec/runtime/setup_spec.rb
index 14f973dc9..7a8a9c407 100644
--- a/spec/runtime/setup_spec.rb
+++ b/spec/runtime/setup_spec.rb
@@ -1207,5 +1207,17 @@ end
       RUBY
       expect(out).to eq("rack-1.0.0")
     end
+
+    it "allows Gem::Specification.find_by_name to work" do
+      install_gemfile! <<-G
+        source "file:#{gem_repo1}"
+        gem "rack"
+      G
+      ruby! <<-RUBY
+        require "bundler/setup"
+        puts Gem::Specification.find_by_name("rack").full_name
+      RUBY
+      expect(out).to eq("rack-1.0.0")
+    end
   end
 end

But be aware, the tests seem to pass without your patch as well

[jules2689]

Update: I'll continue poking at this soon, I'm going to try and get a failing test case first though. However, it seems that foreman was doing something weird with how this worked, so I will be focussing on threading and loading using that as a basis.

[segiddins]

@jules2689 are you OK with me de-scoping this from 1.15.0.pre.2, given it doesn't seem to be as severe as initially suspected?

[jules2689]

@segiddins yea, I need to investigate more anyways and we have a work around

[jules2689]

Superseded by #5630