Skip to content
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

No DidYouMean constants in Travis CI #117

Closed
styd opened this issue Feb 25, 2019 · 14 comments
Closed

No DidYouMean constants in Travis CI #117

styd opened this issue Feb 25, 2019 · 14 comments
Labels

Comments

@styd
Copy link
Contributor

styd commented Feb 25, 2019

Hi @yuki24!

I was trying to use DYM for my gem's KeyError. So, I added this code:

module DidYouMean
  SPELL_CHECKERS.merge!({
    'MyGem::KeyError' => KeyErrorChecker
  })
end

It's working in my local computer, but failed in Travis CI, saying uninitialized constant DidYouMean::SPELL_CHECKERS.
So I added this code to print DidYouMean methods and constants inside the module:

puts "Methods: " + methods.inspect
puts "Constants: " + constants.inspect

To my surprise, it was printing these lines in Travis CI:

Methods: [...] # a lot of methods
Constants: [] # empty

This only happens in CI. It's fine in my local computer.
Do you know what causes this?

@yuki24
Copy link
Member

yuki24 commented Mar 1, 2019

Ok so looking at https://travis-ci.org/styd/smart_kv/jobs/497686270, it's possible that the did_you_mean gem isn't installed. RVM does not install bundled gems for some reason, and it may have to be installed explicitly in Travis. I think adding:

gem 'did_you_mean'

to your Gemfile will fix your failure on CI. This ways bundler will pull in an apprppriate version for the Ruby version so you don't have to worry about installing it in .travis.yml.

Update: I no longer suggest adding did_you_mean to the Gemfile as it causes more problems depending on the version of Rubygems or Bundler. I would recommend manually installing it with e.g. before_install: gem i did_you_mean in Travis or whatever interface is provided by the choice of your CI.

Update 2: Further investigation revealed that the first update may not work in some cases. Please see the comment below for more details.

@styd
Copy link
Contributor Author

styd commented Mar 1, 2019

@yuki24, thank you for looking into my gem's travis log.

I tried your suggestion, but now error happens before running the test.
In ruby 2.5, the error was:

Gem::LoadError: You have already activated did_you_mean 1.2.0, but your Gemfile requires did_you_mean 1.3.0

In ruby 2.6, the error was:

Could not find did_you_mean-1.3.0 in any of the sources
Run `bundle install` to install missing gems.

So, I tried adding bundle install to before_install in travis.yml (which apparently is redundant, because the CI is calling bundle install again after that but with some options/flags).
Surprisingly, my test are now running successfully in ruby 2.5, but ruby 2.6 still outputting the same error.

So, I have two confusions now:

  1. why the gem built successfully in ruby 2.5 just because I added another bundle install (so, now travis is running it twice).
  2. while in ruby 2.6, adding another bundle install doesn't work.

@yuki24
Copy link
Member

yuki24 commented Mar 1, 2019

@styd I'll look into your repository since it's open and I should be able to look into it.

@styd
Copy link
Contributor Author

styd commented Mar 1, 2019

@yuki24, thank you for taking the trouble to help me.

TobyRet added a commit to ministryofjustice/prison-visits-2 that referenced this issue Mar 8, 2019
@yuki24 yuki24 added the Bug label Mar 9, 2019
@jeroenj
Copy link

jeroenj commented Apr 12, 2019

We've been having the same issue since we upgraded to Ruby 2.6 (and thus rubygems 3.0). Basically if the version defined in your Gemfile is the same as the one bundled with Ruby, rubygems 3.0 no longer installs it locally. However with rugyems 2.7.9 it did install it locally. That's also why it didn't appear on Ruby 2.5 (if you didn't upgrade ruygems, since that ships with rubygems 2.7 by default).

The reason you'd want to have it locally installed (e.g. when using bundler's --path option) is because if it doesn't install locally bundler can't find it (even though it's installed on the system with Ruby's bundled gems):

/usr/local/bundle/gems/bundler-2.0.1/lib/bundler/spec_set.rb:87:in `block in materialize': Could not find did_you_mean-1.3.0 in any of the sources (Bundler::GemNotFound)

You can reproduce this issue with the ruby:2.6.2-slim Docker image (which currently contains rubygems 3.0.3 and bundler 1.17.2):

$ docker run -it --rm ruby:2.6.2-slim bash
$ gem list did_you_mean | grep did_you_mean
did_you_mean (1.3.0)
$ printf "source 'https://rubygems.org'\ngem 'did_you_mean', '1.3.0'" > Gemfile
$ bundle install --path bundle
Fetching gem metadata from https://rubygems.org/...
Resolving dependencies...
Using bundler 1.17.2
Using did_you_mean 1.3.0
Bundle complete! 1 Gemfile dependency, 2 gems now installed.
Bundled gems are installed into `./bundle`

As you can see did_you_mean is not being installed but instead used from the system gems.

However if you try this with minitest for example (which just like did_you_mean is just bundled with ruby) it does install it:

$ docker run -it --rm ruby:2.6.2-slim bash
$ gem list minitest | grep minitest
did_you_mean (5.11.3)
$ printf "source 'https://rubygems.org'\ngem 'minitest', '5.11.3'" > Gemfile
$ bundle install --path bundle
Fetching gem metadata from https://rubygems.org/.............
Resolving dependencies...
Using bundler 1.17.2
Fetching minitest 5.11.3
Installing minitest 5.11.3
Bundle complete! 1 Gemfile dependency, 2 gems now installed.
Bundled gems are installed into `./bundle`

If you use bundler 2.0 it also does not install did_you_mean:

$ docker run -it --rm ruby:2.6.2-slim bash
$ gem install bundler -v 2.0.1
Fetching bundler-2.0.1.gem
Successfully installed bundler-2.0.1
1 gem installed
$ gem list did_you_mean | grep did_you_mean
did_you_mean (1.3.0)
$ printf "source 'https://rubygems.org'\ngem 'did_you_mean', '1.3.0'" > Gemfile
$ bundle install --path bundle
Fetching gem metadata from https://rubygems.org/...
Resolving dependencies...
Using bundler 2.0.1
Using did_you_mean 1.3.0
Bundle complete! 1 Gemfile dependency, 2 gems now installed.
Bundled gems are installed into `./bundle`

However, downgrading rubygems to the latest 2.7.x release does make it so that did_you_mean gets installed:

$ docker run -it --rm ruby:2.6.2-slim bash
$ gem update --system 2.7.9
Updating rubygems-update
Fetching rubygems-update-2.7.9.gem
Successfully installed rubygems-update-2.7.9
Installing RubyGems 2.7.9
Bundler 1.16.6 installed
RubyGems 2.7.9 installed
Regenerating binstubs
------------------------------------------------------------------------------
RubyGems installed the following executables:
        /usr/local/bin/gem
        /usr/local/bin/bundle
RubyGems system software updated
$ gem list did_you_mean | grep did_you_mean
did_you_mean (1.3.0)
$ printf "source 'https://rubygems.org'\ngem 'did_you_mean', '1.3.0'" > Gemfile
$ bundle install --path bundle
Fetching gem metadata from https://rubygems.org/...
Resolving dependencies...
Using bundler 1.16.6
Fetching did_you_mean 1.3.0
Installing did_you_mean 1.3.0
Bundle complete! 1 Gemfile dependency, 2 gems now installed.
Bundled gems are installed into `/bundle`

We have been using a downgraded rubygems for a while now. But we switched to this workaround by tricking rubygems into thinking the bundled did_you_mean is not present (apparently for that removing the gemspec is enough):

$ docker run -it --rm ruby:2.6.2-slim bash
$ rm /usr/local/lib/ruby/gems/2.6.0/specifications/did_you_mean-1.3.0.gemspec
$ gem list did_you_mean | grep did_you_mean # returns nothing as it's no longer found
$ printf "source 'https://rubygems.org'\ngem 'did_you_mean', '1.3.0'" > Gemfile
$ bundle install --path bundle
Fetching gem metadata from https://rubygems.org/...
Resolving dependencies...
Using bundler 1.17.2
Fetching did_you_mean 1.3.0
Installing did_you_mean 1.3.0
Bundle complete! 1 Gemfile dependency, 2 gems now installed.
Bundled gems are installed into `./bundle`

To me this looks like an issue with rubygems 3.0. But the weird thing is that it only seems to affect did_you_mean. I have tried the same thing with minitest (see above) and rake and both actually get installed locally (even though they're exactly the same versions as the ones bundled with ruby).

I have yet to try to track down what exactly causes the difference (either in rubygems or in did_you_mean). But for now either downgrading rubygems to 2.7.9 or removing the system installed did_you_mean works around the issue.

@yuki24
Copy link
Member

yuki24 commented Apr 12, 2019

@jeroenj Thanks for reporting. Something is definitely off here and I'll look into it.

@yuki24
Copy link
Member

yuki24 commented Apr 12, 2019

Okay, I thought there were two distinct issues here but turns out it's actually just one issue that awfully affects Bundler's --path bundle option.

  1. Ruby forcefully loads the did_you_mean gem on every boot
  2. When the --path option is specified on bundle install, the CLI sets a new path to Bundler.settings, which puts Bundler in a clean state (== no gems are installed other than the default gems)
  3. However, by the time it tries to install external gems to the specified path, DYM is already loaded (Bundler looks at Gem::Specification.stubs in Rubygems to determine what gems are already loaded). This makes Bundler think that it's a default gem (which is obviously incorrect)
  4. Because of that Bundler thinks DYM is already installed and it skips installing it
  5. When running bundle exec after the completion of bundle install --path ..., Bundler will make all default gems and the gems installed in the path (notice that DYM is missing due to step 4)
  6. Ruby will fail to look up the DidYouMean constant at runtime

This commit in Rubygems rubygems/rubygems@42bba27#diff-05477d38d75c98664b0e7dec6eb73b45R823 seems to have introduced this behavior by accident, but we can't just revert it otherwise it may break other things.

As @jeroenj pointed out, Rubygems 2.7.9 does not have this issue becaue the commit mentioned above is not part of the version 2.7.9.

The other workaround I came up with is to specify RUBYOPT='--disable-did_you_mean' on bundle install --path bundle. This also requires Gemfile to have gem 'did_you_mean'. This way Ruby will temporarily disable DYM and Bundler will install it to the specified path properly:

# In Gemfile
gem 'did_you_mean'
RUBYOPT='--disable-did_you_mean' bundle install --path bundle

I think we should promote DYM up to a default gem to avoid confusions like this. It's very odd that it gets loaded with no clear notice despite the fact that it can be missing at any time.

@jeroenj
Copy link

jeroenj commented Apr 13, 2019

What an investigation. 👏 Thanks for looking into this this quickly.

I’d indeed think promoting did_you_mean to a default gem makes sense here. Now I also understand why minitest didn’t have the issue.

@jeroenj
Copy link

jeroenj commented Jun 21, 2019

Sad to report back here, but bundler 2.0.2 broke the RUBYOPT='--disable-did_you_mean' bundle install --path bundle workaround. 😅

I was able to track it down to this change: https://github.com/bundler/bundler/pull/7067/files#diff-d08082204d5579c541f2c1206ffeb652R4

If I understand this (and the original issue (#117 (comment))) correctly this is caused by (the vendored version of) thor now requiring did_you_mean.

It seems that thor (master) no longer requires did you mean: rails/thor@8c57f1d. So a future version of bundler (with an updated version of thor) might no longer have that issue.

So if I understand it all correctly none of this would be an issue of did_you_mean would be promoted to a default gem, right?

evazion referenced this issue in danbooru/danbooru Aug 15, 2019
Deploying to production failed during `bundle install` because of an
incompatibility between did_you_mean-1.3.0 + ruby-2.6 + rubygems-3.0 +
bundler-2.0.

ref: `https://github.com/yuki24/did_you_mean/issues/117#issuecomment-482565387`
@eregon
Copy link
Member

eregon commented Oct 29, 2019

Agreed having did_you_mean as a default gem would make a lot of sense since it's required by default during startup.
@hsbt What do you think?

@hsbt
Copy link
Member

hsbt commented Oct 29, 2019

I'm also +1 to promote did_you_mean to default gems.

@deivid-rodriguez
Copy link

Me too, for what's it's worth.

kddnewton added a commit to kddnewton/ruby that referenced this issue Oct 29, 2019
At the moment, there are some problems with regard to bundler + did_you_mean because of did_you_mean being a bundled gem. Since the vendored version of thor inside bundler and ruby itself explicitly requires did_you_mean, it can become difficult to load it when using Bundler.setup. See this issue: ruby/did_you_mean#117 (comment) for more details.
@kddnewton
Copy link
Contributor

I opened a PR for it, though I'm not sure if I did it correctly. Feel free to leave comments over there ^^^^

kddnewton added a commit to kddnewton/ruby that referenced this issue Oct 30, 2019
At the moment, there are some problems with regard to bundler + did_you_mean because of did_you_mean being a bundled gem. Since the vendored version of thor inside bundler and ruby itself explicitly requires did_you_mean, it can become difficult to load it when using Bundler.setup. See this issue: ruby/did_you_mean#117 (comment) for more details.
kddnewton added a commit to kddnewton/ruby that referenced this issue Nov 4, 2019
At the moment, there are some problems with regard to bundler + did_you_mean because of did_you_mean being a bundled gem. Since the vendored version of thor inside bundler and ruby itself explicitly requires did_you_mean, it can become difficult to load it when using Bundler.setup. See this issue: ruby/did_you_mean#117 (comment) for more details.
kddnewton added a commit to kddnewton/ruby that referenced this issue Nov 4, 2019
At the moment, there are some problems with regard to bundler + did_you_mean because of did_you_mean being a bundled gem. Since the vendored version of thor inside bundler and ruby itself explicitly requires did_you_mean, it can become difficult to load it when using Bundler.setup. See this issue: ruby/did_you_mean#117 (comment) for more details.
kddnewton added a commit to kddnewton/ruby that referenced this issue Nov 4, 2019
At the moment, there are some problems with regard to bundler + did_you_mean because of did_you_mean being a bundled gem. Since the vendored version of thor inside bundler and ruby itself explicitly requires did_you_mean, it can become difficult to load it when using Bundler.setup. See this issue: ruby/did_you_mean#117 (comment) for more details.
kddnewton added a commit to kddnewton/ruby that referenced this issue Nov 5, 2019
At the moment, there are some problems with regard to bundler + did_you_mean because of did_you_mean being a bundled gem. Since the vendored version of thor inside bundler and ruby itself explicitly requires did_you_mean, it can become difficult to load it when using Bundler.setup. See this issue: ruby/did_you_mean#117 (comment) for more details.
kddnewton added a commit to kddnewton/ruby that referenced this issue Nov 6, 2019
At the moment, there are some problems with regard to bundler + did_you_mean because of did_you_mean being a bundled gem. Since the vendored version of thor inside bundler and ruby itself explicitly requires did_you_mean, it can become difficult to load it when using Bundler.setup. See this issue: ruby/did_you_mean#117 (comment) for more details.
yuki24 pushed a commit to yuki24/ruby that referenced this issue Nov 23, 2019
At the moment, there are some problems with regard to bundler + did_you_mean because of did_you_mean being a bundled gem. Since the vendored version of thor inside bundler and ruby itself explicitly requires did_you_mean, it can become difficult to load it when using Bundler.setup. See this issue: ruby/did_you_mean#117 (comment) for more details.
yuki24 pushed a commit to yuki24/ruby that referenced this issue Nov 24, 2019
At the moment, there are some problems with regard to bundler + did_you_mean because of did_you_mean being a bundled gem. Since the vendored version of thor inside bundler and ruby itself explicitly requires did_you_mean, it can become difficult to load it when using Bundler.setup. See this issue: ruby/did_you_mean#117 (comment) for more details.
yuki24 pushed a commit to yuki24/ruby that referenced this issue Nov 24, 2019
At the moment, there are some problems with regard to bundler + did_you_mean because of did_you_mean being a bundled gem. Since the vendored version of thor inside bundler and ruby itself explicitly requires did_you_mean, it can become difficult to load it when using Bundler.setup. See this issue: ruby/did_you_mean#117 (comment) for more details.
yuki24 pushed a commit to yuki24/ruby that referenced this issue Nov 24, 2019
At the moment, there are some problems with regard to bundler + did_you_mean because of did_you_mean being a bundled gem. Since the vendored version of thor inside bundler and ruby itself explicitly requires did_you_mean, it can become difficult to load it when using Bundler.setup. See this issue: ruby/did_you_mean#117 (comment) for more details.
yuki24 pushed a commit to yuki24/ruby that referenced this issue Nov 29, 2019
At the moment, there are some problems with regard to bundler + did_you_mean because of did_you_mean being a bundled gem. Since the vendored version of thor inside bundler and ruby itself explicitly requires did_you_mean, it can become difficult to load it when using Bundler.setup. See this issue: ruby/did_you_mean#117 (comment) for more details.
yuki24 pushed a commit to yuki24/ruby that referenced this issue Nov 30, 2019
At the moment, there are some problems with regard to bundler + did_you_mean because of did_you_mean being a bundled gem. Since the vendored version of thor inside bundler and ruby itself explicitly requires did_you_mean, it can become difficult to load it when using Bundler.setup. See this issue: ruby/did_you_mean#117 (comment) for more details.
yuki24 pushed a commit to ruby/ruby that referenced this issue Dec 1, 2019
At the moment, there are some problems with regard to bundler + did_you_mean because of did_you_mean being a bundled gem. Since the vendored version of thor inside bundler and ruby itself explicitly requires did_you_mean, it can become difficult to load it when using Bundler.setup. See this issue: ruby/did_you_mean#117 (comment) for more details.
@yuki24
Copy link
Member

yuki24 commented Dec 1, 2019

closing in favor of ruby/ruby#2689.

@yuki24 yuki24 closed this as completed Dec 1, 2019
deivid-rodriguez added a commit to rubygems/rubygems that referenced this issue Mar 30, 2020
…ecification.stubs_for`

The rationale is that:

* The change has caused realworld issues. See for example
ruby/did_you_mean#117 and specifically [this
comment](ruby/did_you_mean#117 (comment))
for a great explanation of the issue it caused for `did_you_mean`.

* The change also causes problems for our development workflows. For
example, because of it, our `bundler` specs cannot currently be run with
`bin/rake` and we have to use `bin/rspec` or `bin/parallel_spec`
directly. The explanation for this is:

  - Our specs install test dependencies to `tmp` before running specs.
  - `rake` is one of these test dependencies.
  - Before installing each test dependency, we check whether it has
  matching installed specs: https://github.com/rubygems/rubygems/blob/2bbcdcde08b90d4ef03da8fb1f7a8a3313e13bb8/bundler/spec/support/rubygems_ext.rb#L109-L114.

  - Normally, if `rake` has not yet been installed to `tmp`, this check
  fails and `rake` is installed, but since the loaded specs are now
  added to `Gem::Specification.stubs` and `rake`'s specification _is_
  loaded because we're running through `bin/rake`, the check incorrectly
  assumes that `rake` is already installed to `tmp` and skips
  installation.
  - At a later point the specs check whether `rake` is actually
  installed and fail if it's not: https://github.com/rubygems/rubygems/blob/2bbcdcde08b90d4ef03da8fb1f7a8a3313e13bb8/bundler/spec/support/builders.rb#L372-L383

Essentially, both of the issues are the same. If at runtime we change
the location of gems, we'll _want_ to not consider loaded specifications
when dealing with the new gem location, because the loaded
specifications have not been loaded from there. Loaded specifications is
something different from installed stub specifications and those should
not be mixed.

The PR still seemed to have fixed an issue, so I did my archaeology job
and investigated the original issue to double check if reverting is ok.
The logs for the original error can be found here:
https://ci.appveyor.com/project/rubygems/rubygems/build/1172/job/ogubyucpljcv22ux.

So I installed ruby 2.4.4, checked out the commit reference before the
offending PR, and the exact error reproduced. 🎉

```
$ rake test
/home/deivid/Code/rubygems/lib/rubygems/resolver.rb:231:in `search_for': Unable to resolve dependency: user requested 'bundler (= 1.16.2)' (Gem::UnsatisfiableDependencyError)
  from /home/deivid/Code/rubygems/lib/rubygems/resolver.rb:283:in `block in sort_dependencies'
  from /home/deivid/Code/rubygems/lib/rubygems/resolver.rb:277:in `each'
  from /home/deivid/Code/rubygems/lib/rubygems/resolver.rb:277:in `sort_by'
  from /home/deivid/Code/rubygems/lib/rubygems/resolver.rb:277:in `with_index'
  from /home/deivid/Code/rubygems/lib/rubygems/resolver.rb:277:in `sort_dependencies'
  from /home/deivid/Code/rubygems/lib/rubygems/resolver/molinillo/lib/molinillo/delegates/specification_provider.rb:52:in `block in sort_dependencies'
  from /home/deivid/Code/rubygems/lib/rubygems/resolver/molinillo/lib/molinillo/delegates/specification_provider.rb:69:in `with_no_such_dependency_error_handling'
  from /home/deivid/Code/rubygems/lib/rubygems/resolver/molinillo/lib/molinillo/delegates/specification_provider.rb:51:in `sort_dependencies'
  from /home/deivid/Code/rubygems/lib/rubygems/resolver/molinillo/lib/molinillo/resolution.rb:165:in `initial_state'
  from /home/deivid/Code/rubygems/lib/rubygems/resolver/molinillo/lib/molinillo/resolution.rb:106:in `start_resolution'
  from /home/deivid/Code/rubygems/lib/rubygems/resolver/molinillo/lib/molinillo/resolution.rb:64:in `resolve'
  from /home/deivid/Code/rubygems/lib/rubygems/resolver/molinillo/lib/molinillo/resolver.rb:42:in `resolve'
  from /home/deivid/Code/rubygems/lib/rubygems/resolver.rb:188:in `resolve'
  from /home/deivid/Code/rubygems/lib/rubygems/request_set.rb:396:in `resolve'
  from /home/deivid/Code/rubygems/lib/rubygems/request_set.rb:408:in `resolve_current'
  from /home/deivid/Code/rubygems/lib/rubygems.rb:243:in `finish_resolve'
  from /home/deivid/Code/rubygems/lib/rubygems/rdoc.rb:13:in `<top (required)>'
  from /home/deivid/Code/rubygems/lib/rubygems/core_ext/kernel_require.rb:54:in `require'
  from /home/deivid/Code/rubygems/lib/rubygems/core_ext/kernel_require.rb:54:in `require'
  from /home/deivid/Code/rubygems/lib/rubygems/test_case.rb:1563:in `<top (required)>'
  from /home/deivid/Code/rubygems/test/rubygems/test_bundled_ca.rb:2:in `require'
  from /home/deivid/Code/rubygems/test/rubygems/test_bundled_ca.rb:2:in `<top (required)>'
  from /home/deivid/.rbenv/versions/2.4.4/lib/ruby/gems/2.4.0/gems/rake-12.0.0/lib/rake/rake_test_loader.rb:15:in `require'
  from /home/deivid/.rbenv/versions/2.4.4/lib/ruby/gems/2.4.0/gems/rake-12.0.0/lib/rake/rake_test_loader.rb:15:in `block in <main>'
  from /home/deivid/.rbenv/versions/2.4.4/lib/ruby/gems/2.4.0/gems/rake-12.0.0/lib/rake/rake_test_loader.rb:4:in `select'
  from /home/deivid/.rbenv/versions/2.4.4/lib/ruby/gems/2.4.0/gems/rake-12.0.0/lib/rake/rake_test_loader.rb:4:in `<main>'
rake aborted!
Command failed with status (1)

Tasks: TOP => test
```

Now the explanation of the error:

* Rubygems base `TestCase` class requires `bundler` because some tests
use `bundler`:
https://github.com/rubygems/rubygems/blob/2bbcdcde08b90d4ef03da8fb1f7a8a3313e13bb8/lib/rubygems/test_case.rb#L26

* That `require` (our custom rubygems require) would activate the
default bundler spec (1.16.1 for ruby 2.4.4) but then overwrite it with
a 1.16.2 version (the locally provided bundler those days) due to [this
old
hack](https://github.com/rubygems/bundler/blob/9f7bf0ac3ab8d995e3a274cec3c292a5203f4534/lib/bundler/version.rb#L7-L23).

* Rubygems base `TestCase` class requires `rubygems/rdoc`:
https://github.com/rubygems/rubygems/blob/2bbcdcde08b90d4ef03da8fb1f7a8a3313e13bb8/lib/rubygems/test_case.rb#L1536

* And that file ends up calling `Gem.finish_resolve`:
https://github.com/rubygems/rubygems/blob/2bbcdcde08b90d4ef03da8fb1f7a8a3313e13bb8/lib/rubygems/rdoc.rb#L13

* `Gem.finish_resolve` adds the currently loaded specs to the
resolution:
https://github.com/rubygems/rubygems/blob/2bbcdcde08b90d4ef03da8fb1f7a8a3313e13bb8/lib/rubygems.rb#L235

* That means it would try to resolve bundler 1.16.2, but no
specification for that version was installed since the default was
1.16.1. That explains why upgrading to rubygems 2.7.7 fixed the issue,
since it provided bundler 1.16.2 by default so there was not bundler
version discrepancy.

After understanding the error, I conclude that:

* Only this part of the original patch was actually needed to resolve
the error, not any of the changes in `Gem::Specification.stubs` and
`Gem::Specification.stubs_for`:

```diff
diff --git a/lib/rubygems/test_case.rb b/lib/rubygems/test_case.rb
index f1cd3d274c..92c848e870 100644
--- a/lib/rubygems/test_case.rb
+++ b/lib/rubygems/test_case.rb
@@ -13,6 +13,15 @@ else
   require 'rubygems'
 end

+# If bundler gemspec exists, add to stubs
+bundler_gemspec = File.expand_path("../../../bundler/bundler.gemspec", __FILE__)
+if File.exist?(bundler_gemspec)
+  Gem::Specification.dirs.unshift File.dirname(bundler_gemspec)
+  Gem::Specification.class_variable_set :@@stubs, nil
+  Gem::Specification.stubs
+  Gem::Specification.dirs.shift
+end
+
 begin
   gem 'minitest'
 rescue Gem::LoadError
```

So, I propose to revert adding loaded specification to
`Gem::Specification.stubs` and `Gem::Specification.stubs_for` because I
think it's safe, it fixes the issues caused by their addition, and it
simplifies `Gem::Specification` code, which is already complicated
enough.
deivid-rodriguez added a commit to rubygems/rubygems that referenced this issue Mar 31, 2020
…ecification.stubs_for`

The rationale is that:

* The change has caused realworld issues. See for example
ruby/did_you_mean#117 and specifically [this
comment](ruby/did_you_mean#117 (comment))
for a great explanation of the issue it caused for `did_you_mean`.

* The change also causes problems for our development workflows. For
example, because of it, our `bundler` specs cannot currently be run with
`bin/rake` and we have to use `bin/rspec` or `bin/parallel_spec`
directly. The explanation for this is:

  - Our specs install test dependencies to `tmp` before running specs.
  - `rake` is one of these test dependencies.
  - Before installing each test dependency, we check whether it has
  matching installed specs: https://github.com/rubygems/rubygems/blob/2bbcdcde08b90d4ef03da8fb1f7a8a3313e13bb8/bundler/spec/support/rubygems_ext.rb#L109-L114.

  - Normally, if `rake` has not yet been installed to `tmp`, this check
  fails and `rake` is installed, but since the loaded specs are now
  added to `Gem::Specification.stubs` and `rake`'s specification _is_
  loaded because we're running through `bin/rake`, the check incorrectly
  assumes that `rake` is already installed to `tmp` and skips
  installation.
  - At a later point the specs check whether `rake` is actually
  installed and fail if it's not: https://github.com/rubygems/rubygems/blob/2bbcdcde08b90d4ef03da8fb1f7a8a3313e13bb8/bundler/spec/support/builders.rb#L372-L383

Essentially, both of the issues are the same. If at runtime we change
the location of gems, we'll _want_ to not consider loaded specifications
when dealing with the new gem location, because the loaded
specifications have not been loaded from there. Loaded specifications is
something different from installed stub specifications and those should
not be mixed.

The PR still seemed to have fixed an issue, so I did my archaeology job
and investigated the original issue to double check if reverting is ok.
The logs for the original error can be found here:
https://ci.appveyor.com/project/rubygems/rubygems/build/1172/job/ogubyucpljcv22ux.

So I installed ruby 2.4.4, checked out the commit reference before the
offending PR, and the exact error reproduced. 🎉

```
$ rake test
/home/deivid/Code/rubygems/lib/rubygems/resolver.rb:231:in `search_for': Unable to resolve dependency: user requested 'bundler (= 1.16.2)' (Gem::UnsatisfiableDependencyError)
  from /home/deivid/Code/rubygems/lib/rubygems/resolver.rb:283:in `block in sort_dependencies'
  from /home/deivid/Code/rubygems/lib/rubygems/resolver.rb:277:in `each'
  from /home/deivid/Code/rubygems/lib/rubygems/resolver.rb:277:in `sort_by'
  from /home/deivid/Code/rubygems/lib/rubygems/resolver.rb:277:in `with_index'
  from /home/deivid/Code/rubygems/lib/rubygems/resolver.rb:277:in `sort_dependencies'
  from /home/deivid/Code/rubygems/lib/rubygems/resolver/molinillo/lib/molinillo/delegates/specification_provider.rb:52:in `block in sort_dependencies'
  from /home/deivid/Code/rubygems/lib/rubygems/resolver/molinillo/lib/molinillo/delegates/specification_provider.rb:69:in `with_no_such_dependency_error_handling'
  from /home/deivid/Code/rubygems/lib/rubygems/resolver/molinillo/lib/molinillo/delegates/specification_provider.rb:51:in `sort_dependencies'
  from /home/deivid/Code/rubygems/lib/rubygems/resolver/molinillo/lib/molinillo/resolution.rb:165:in `initial_state'
  from /home/deivid/Code/rubygems/lib/rubygems/resolver/molinillo/lib/molinillo/resolution.rb:106:in `start_resolution'
  from /home/deivid/Code/rubygems/lib/rubygems/resolver/molinillo/lib/molinillo/resolution.rb:64:in `resolve'
  from /home/deivid/Code/rubygems/lib/rubygems/resolver/molinillo/lib/molinillo/resolver.rb:42:in `resolve'
  from /home/deivid/Code/rubygems/lib/rubygems/resolver.rb:188:in `resolve'
  from /home/deivid/Code/rubygems/lib/rubygems/request_set.rb:396:in `resolve'
  from /home/deivid/Code/rubygems/lib/rubygems/request_set.rb:408:in `resolve_current'
  from /home/deivid/Code/rubygems/lib/rubygems.rb:243:in `finish_resolve'
  from /home/deivid/Code/rubygems/lib/rubygems/rdoc.rb:13:in `<top (required)>'
  from /home/deivid/Code/rubygems/lib/rubygems/core_ext/kernel_require.rb:54:in `require'
  from /home/deivid/Code/rubygems/lib/rubygems/core_ext/kernel_require.rb:54:in `require'
  from /home/deivid/Code/rubygems/lib/rubygems/test_case.rb:1563:in `<top (required)>'
  from /home/deivid/Code/rubygems/test/rubygems/test_bundled_ca.rb:2:in `require'
  from /home/deivid/Code/rubygems/test/rubygems/test_bundled_ca.rb:2:in `<top (required)>'
  from /home/deivid/.rbenv/versions/2.4.4/lib/ruby/gems/2.4.0/gems/rake-12.0.0/lib/rake/rake_test_loader.rb:15:in `require'
  from /home/deivid/.rbenv/versions/2.4.4/lib/ruby/gems/2.4.0/gems/rake-12.0.0/lib/rake/rake_test_loader.rb:15:in `block in <main>'
  from /home/deivid/.rbenv/versions/2.4.4/lib/ruby/gems/2.4.0/gems/rake-12.0.0/lib/rake/rake_test_loader.rb:4:in `select'
  from /home/deivid/.rbenv/versions/2.4.4/lib/ruby/gems/2.4.0/gems/rake-12.0.0/lib/rake/rake_test_loader.rb:4:in `<main>'
rake aborted!
Command failed with status (1)

Tasks: TOP => test
```

Now the explanation of the error:

* Rubygems base `TestCase` class requires `bundler` because some tests
use `bundler`:
https://github.com/rubygems/rubygems/blob/2bbcdcde08b90d4ef03da8fb1f7a8a3313e13bb8/lib/rubygems/test_case.rb#L26

* That `require` (our custom rubygems require) would activate the
default bundler spec (1.16.1 for ruby 2.4.4) but then overwrite it with
a 1.16.2 version (the locally provided bundler those days) due to [this
old
hack](https://github.com/rubygems/bundler/blob/9f7bf0ac3ab8d995e3a274cec3c292a5203f4534/lib/bundler/version.rb#L7-L23).

* Rubygems base `TestCase` class requires `rubygems/rdoc`:
https://github.com/rubygems/rubygems/blob/2bbcdcde08b90d4ef03da8fb1f7a8a3313e13bb8/lib/rubygems/test_case.rb#L1536

* And that file ends up calling `Gem.finish_resolve`:
https://github.com/rubygems/rubygems/blob/2bbcdcde08b90d4ef03da8fb1f7a8a3313e13bb8/lib/rubygems/rdoc.rb#L13

* `Gem.finish_resolve` adds the currently loaded specs to the
resolution:
https://github.com/rubygems/rubygems/blob/2bbcdcde08b90d4ef03da8fb1f7a8a3313e13bb8/lib/rubygems.rb#L235

* That means it would try to resolve bundler 1.16.2, but no
specification for that version was installed since the default was
1.16.1. That explains why upgrading to rubygems 2.7.7 fixed the issue,
since it provided bundler 1.16.2 by default so there was not bundler
version discrepancy.

After understanding the error, I conclude that:

* Only this part of the original patch was actually needed to resolve
the error, not any of the changes in `Gem::Specification.stubs` and
`Gem::Specification.stubs_for`:

```diff
diff --git a/lib/rubygems/test_case.rb b/lib/rubygems/test_case.rb
index f1cd3d274c..92c848e870 100644
--- a/lib/rubygems/test_case.rb
+++ b/lib/rubygems/test_case.rb
@@ -13,6 +13,15 @@ else
   require 'rubygems'
 end

+# If bundler gemspec exists, add to stubs
+bundler_gemspec = File.expand_path("../../../bundler/bundler.gemspec", __FILE__)
+if File.exist?(bundler_gemspec)
+  Gem::Specification.dirs.unshift File.dirname(bundler_gemspec)
+  Gem::Specification.class_variable_set :@@stubs, nil
+  Gem::Specification.stubs
+  Gem::Specification.dirs.shift
+end
+
 begin
   gem 'minitest'
 rescue Gem::LoadError
```

So, I propose to revert adding loaded specification to
`Gem::Specification.stubs` and `Gem::Specification.stubs_for` because I
think it's safe, it fixes the issues caused by their addition, and it
simplifies `Gem::Specification` code, which is already complicated
enough.
hsbt pushed a commit to ruby/ruby that referenced this issue May 8, 2020
….stubs` and `Gem::Specification.stubs_for`

The rationale is that:

* The change has caused realworld issues. See for example
ruby/did_you_mean#117 and specifically [this
comment](ruby/did_you_mean#117 (comment))
for a great explanation of the issue it caused for `did_you_mean`.

* The change also causes problems for our development workflows. For
example, because of it, our `bundler` specs cannot currently be run with
`bin/rake` and we have to use `bin/rspec` or `bin/parallel_spec`
directly. The explanation for this is:

  - Our specs install test dependencies to `tmp` before running specs.
  - `rake` is one of these test dependencies.
  - Before installing each test dependency, we check whether it has
  matching installed specs: https://github.com/rubygems/rubygems/blob/2bbcdcde08b90d4ef03da8fb1f7a8a3313e13bb8/bundler/spec/support/rubygems_ext.rb#L109-L114.

  - Normally, if `rake` has not yet been installed to `tmp`, this check
  fails and `rake` is installed, but since the loaded specs are now
  added to `Gem::Specification.stubs` and `rake`'s specification _is_
  loaded because we're running through `bin/rake`, the check incorrectly
  assumes that `rake` is already installed to `tmp` and skips
  installation.
  - At a later point the specs check whether `rake` is actually
  installed and fail if it's not: https://github.com/rubygems/rubygems/blob/2bbcdcde08b90d4ef03da8fb1f7a8a3313e13bb8/bundler/spec/support/builders.rb#L372-L383

Essentially, both of the issues are the same. If at runtime we change
the location of gems, we'll _want_ to not consider loaded specifications
when dealing with the new gem location, because the loaded
specifications have not been loaded from there. Loaded specifications is
something different from installed stub specifications and those should
not be mixed.

The PR still seemed to have fixed an issue, so I did my archaeology job
and investigated the original issue to double check if reverting is ok.
The logs for the original error can be found here:
https://ci.appveyor.com/project/rubygems/rubygems/build/1172/job/ogubyucpljcv22ux.

So I installed ruby 2.4.4, checked out the commit reference before the
offending PR, and the exact error reproduced. 🎉

```
$ rake test
/home/deivid/Code/rubygems/lib/rubygems/resolver.rb:231:in `search_for': Unable to resolve dependency: user requested 'bundler (= 1.16.2)' (Gem::UnsatisfiableDependencyError)
  from /home/deivid/Code/rubygems/lib/rubygems/resolver.rb:283:in `block in sort_dependencies'
  from /home/deivid/Code/rubygems/lib/rubygems/resolver.rb:277:in `each'
  from /home/deivid/Code/rubygems/lib/rubygems/resolver.rb:277:in `sort_by'
  from /home/deivid/Code/rubygems/lib/rubygems/resolver.rb:277:in `with_index'
  from /home/deivid/Code/rubygems/lib/rubygems/resolver.rb:277:in `sort_dependencies'
  from /home/deivid/Code/rubygems/lib/rubygems/resolver/molinillo/lib/molinillo/delegates/specification_provider.rb:52:in `block in sort_dependencies'
  from /home/deivid/Code/rubygems/lib/rubygems/resolver/molinillo/lib/molinillo/delegates/specification_provider.rb:69:in `with_no_such_dependency_error_handling'
  from /home/deivid/Code/rubygems/lib/rubygems/resolver/molinillo/lib/molinillo/delegates/specification_provider.rb:51:in `sort_dependencies'
  from /home/deivid/Code/rubygems/lib/rubygems/resolver/molinillo/lib/molinillo/resolution.rb:165:in `initial_state'
  from /home/deivid/Code/rubygems/lib/rubygems/resolver/molinillo/lib/molinillo/resolution.rb:106:in `start_resolution'
  from /home/deivid/Code/rubygems/lib/rubygems/resolver/molinillo/lib/molinillo/resolution.rb:64:in `resolve'
  from /home/deivid/Code/rubygems/lib/rubygems/resolver/molinillo/lib/molinillo/resolver.rb:42:in `resolve'
  from /home/deivid/Code/rubygems/lib/rubygems/resolver.rb:188:in `resolve'
  from /home/deivid/Code/rubygems/lib/rubygems/request_set.rb:396:in `resolve'
  from /home/deivid/Code/rubygems/lib/rubygems/request_set.rb:408:in `resolve_current'
  from /home/deivid/Code/rubygems/lib/rubygems.rb:243:in `finish_resolve'
  from /home/deivid/Code/rubygems/lib/rubygems/rdoc.rb:13:in `<top (required)>'
  from /home/deivid/Code/rubygems/lib/rubygems/core_ext/kernel_require.rb:54:in `require'
  from /home/deivid/Code/rubygems/lib/rubygems/core_ext/kernel_require.rb:54:in `require'
  from /home/deivid/Code/rubygems/lib/rubygems/test_case.rb:1563:in `<top (required)>'
  from /home/deivid/Code/rubygems/test/rubygems/test_bundled_ca.rb:2:in `require'
  from /home/deivid/Code/rubygems/test/rubygems/test_bundled_ca.rb:2:in `<top (required)>'
  from /home/deivid/.rbenv/versions/2.4.4/lib/ruby/gems/2.4.0/gems/rake-12.0.0/lib/rake/rake_test_loader.rb:15:in `require'
  from /home/deivid/.rbenv/versions/2.4.4/lib/ruby/gems/2.4.0/gems/rake-12.0.0/lib/rake/rake_test_loader.rb:15:in `block in <main>'
  from /home/deivid/.rbenv/versions/2.4.4/lib/ruby/gems/2.4.0/gems/rake-12.0.0/lib/rake/rake_test_loader.rb:4:in `select'
  from /home/deivid/.rbenv/versions/2.4.4/lib/ruby/gems/2.4.0/gems/rake-12.0.0/lib/rake/rake_test_loader.rb:4:in `<main>'
rake aborted!
Command failed with status (1)

Tasks: TOP => test
```

Now the explanation of the error:

* Rubygems base `TestCase` class requires `bundler` because some tests
use `bundler`:
https://github.com/rubygems/rubygems/blob/2bbcdcde08b90d4ef03da8fb1f7a8a3313e13bb8/lib/rubygems/test_case.rb#L26

* That `require` (our custom rubygems require) would activate the
default bundler spec (1.16.1 for ruby 2.4.4) but then overwrite it with
a 1.16.2 version (the locally provided bundler those days) due to [this
old
hack](https://github.com/rubygems/bundler/blob/9f7bf0ac3ab8d995e3a274cec3c292a5203f4534/lib/bundler/version.rb#L7-L23).

* Rubygems base `TestCase` class requires `rubygems/rdoc`:
https://github.com/rubygems/rubygems/blob/2bbcdcde08b90d4ef03da8fb1f7a8a3313e13bb8/lib/rubygems/test_case.rb#L1536

* And that file ends up calling `Gem.finish_resolve`:
https://github.com/rubygems/rubygems/blob/2bbcdcde08b90d4ef03da8fb1f7a8a3313e13bb8/lib/rubygems/rdoc.rb#L13

* `Gem.finish_resolve` adds the currently loaded specs to the
resolution:
https://github.com/rubygems/rubygems/blob/2bbcdcde08b90d4ef03da8fb1f7a8a3313e13bb8/lib/rubygems.rb#L235

* That means it would try to resolve bundler 1.16.2, but no
specification for that version was installed since the default was
1.16.1. That explains why upgrading to rubygems 2.7.7 fixed the issue,
since it provided bundler 1.16.2 by default so there was not bundler
version discrepancy.

After understanding the error, I conclude that:

* Only this part of the original patch was actually needed to resolve
the error, not any of the changes in `Gem::Specification.stubs` and
`Gem::Specification.stubs_for`:

```diff
diff --git a/lib/rubygems/test_case.rb b/lib/rubygems/test_case.rb
index f1cd3d274c..92c848e870 100644
--- a/lib/rubygems/test_case.rb
+++ b/lib/rubygems/test_case.rb
@@ -13,6 +13,15 @@ else
   require 'rubygems'
 end

+# If bundler gemspec exists, add to stubs
+bundler_gemspec = File.expand_path("../../../bundler/bundler.gemspec", __FILE__)
+if File.exist?(bundler_gemspec)
+  Gem::Specification.dirs.unshift File.dirname(bundler_gemspec)
+  Gem::Specification.class_variable_set :@@stubs, nil
+  Gem::Specification.stubs
+  Gem::Specification.dirs.shift
+end
+
 begin
   gem 'minitest'
 rescue Gem::LoadError
```

So, I propose to revert adding loaded specification to
`Gem::Specification.stubs` and `Gem::Specification.stubs_for` because I
think it's safe, it fixes the issues caused by their addition, and it
simplifies `Gem::Specification` code, which is already complicated
enough.

rubygems/rubygems@5269cd617c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

7 participants