Skip to content

[BudlerVersionFinder] set .filter! and .compatible? to match only on major versions#2515

Merged
1 commit merged intomasterfrom
colby/bundler-finder-major-version
Dec 11, 2018
Merged

[BudlerVersionFinder] set .filter! and .compatible? to match only on major versions#2515
1 commit merged intomasterfrom
colby/bundler-finder-major-version

Conversation

@colby-swandale
Copy link
Copy Markdown
Member

Description:

This is a complicated problem so I will do my best to explain it well. When people use Bundler, there is a bit of functionality in RubyGems called the BundlerVersionFinder, that filters the available Bundler specs that RubyGems could activate - depending on the Gemfile.lock.

For a Gemfile.lock that is BUNDLED WITH 1.17.1, the BundlerVersionFinder allows for any version of Bundler 1. But for Bundler 2, only the exact same version is allowed.

https://github.com/rubygems/rubygems/blob/fa9c3b6cc8e8d20b0486524c5eec851768e594fe/lib/rubygems/bundler_version_finder.rb#L40-L43

This functionality was built to support Bundler's postit/trampoline functionality which was removed about a year ago that would automatically install the locked version of Bundler in the Gemfile.lock

This code is an issue now because after Bundler 2 is released, everyone would have to manually install the exact version of Bundler for each Gemfile a person is working on. Going forward, I want to keep the functionality the same as Bundler 1. A Gemfile.lock that that was bundled with 2.0.0 is compatible with 2.1.0, 2.1.1.0, 2.9999, but not 3.0.0. And a Gemfile.lock that that was bundled with 3.0.0 Is compatible with 3.1.0, 3.999 and so on.

Tasks:

  • Describe the problem / feature
  • Write tests
  • Write code to solve the problem
  • Get code review from coworkers / friends

I will abide by the code of conduct.

@indirect
Copy link
Copy Markdown

indirect commented Dec 7, 2018

This seems like the right balance between compatibility and progress to me... feature and patch changes will run automatically, breaking changes will require a manual step to opt in. 👍

@colby-swandale colby-swandale added this to the 2.7.9 milestone Dec 7, 2018
@hsbt
Copy link
Copy Markdown
Member

hsbt commented Dec 7, 2018

When I have a Gemfile.lock with BUNDLED WITH 1.17.1, What's happened with bundled install used by Bundler 2?

@colby-swandale
Copy link
Copy Markdown
Member Author

@hsbt there is 2 possible scenarios.

  1. If you have any version of Bundler 1 installed, RubyGems will activate the latest version of 1.X.X that is available.

  2. If you don't have any Bundler 1 version installed, Bundler will print an error to the user:

You must use Bundler 2 or greater with this lockfile. (Bundler::LockfileError)

@deivid-rodriguez
Copy link
Copy Markdown
Contributor

I also agree. Given that we don't have the trampoline thing, it's better to keep the same behavior for now 👍.

@hsbt
Copy link
Copy Markdown
Member

hsbt commented Dec 7, 2018

2. If you don't have any Bundler 1 version installed, Bundler will print an error to the user:

You must use Bundler 2 or greater with this lockfile. (Bundler::LockfileError)

I also agree with this change. But this change doesn't care users that use Bundler 1. Should we support backward compatibility like this:

  • Gemfile.lock: 1.17.1 and Bundler 2.0.0 -> OK
  • Gemfile.lock: 1.17.1 and Bundler 1.17.1 -> OK
  • Gemfile.lock: 2.0.0 and Bundler 1.17.1 -> NG
  • Gemfile.lock: 2.0.0 and Bundler 2.0.0 -> OK

How about this?

@hsbt
Copy link
Copy Markdown
Member

hsbt commented Dec 7, 2018

@schneems ping

@schneems
Copy link
Copy Markdown
Contributor

schneems commented Dec 7, 2018

@coby-swandale Thanks a ton for taking this on. Sorry for not being more accessible or involved earlier. I appreciate the patch and the explanation. This issue is indeed confusing, so if you need clarification for anything I say, please do not hesitate to ask.

Also, I'm available in the slack room now.

A Gemfile.lock that that was bundled with 2.0.0 is compatible with 2.1.0, 2.1.1.0, 2.9999, but not 3.0.0. And a Gemfile.lock that that was bundled with 3.0.0 Is compatible with 3.1.0, 3.999 and so on.

That makes sense to me. Heroku provides a version of Bundler. Currently, it's 1.15.2 (but I'm looking into upgrading to 1.17.1).

Here's how I understand how the feature in this patch would work:

Scenario 1

If someone deploys a Ruby 2.6 app with:

BUNDLED WITH:
  1.17.0

The built-in bundler looks for a 1.x copy; it will find 1.15.2 and use it. 👍

Scenario 2

If someone deploys a Ruby 2.6 app with:

BUNDLED WITH:
  2.0.x

Then it will use the built-in version that ships with Ruby 2.6 unless a more recent version of 2.x is available. 👍

Next steps for Heroku

If I understood the proposed patch correctly then in the short term we need to do nothing. In the long term we would essentially provide a 1.x version as we currently do and in the future provide a more recent 2.x version.

Bundler removing bundler

One thing that currently happens with the built-in version of bundler 2.0 is that it removes old bundler versions. From the current output https://gist.github.com/schneems/26452540f6e2bbbcf2ea144f45f6b305

remote: Removing bundler (1.15.2)
remote: Bundle completed (45.23s)

I'm not sure if that's addressed in this current patch, or if it needs a separate issue. Please let me know.

@hsbt I'm not sure in each of those scenarios which version of bundler would end up being used for the execution? What does ng indicate?

Thank's everyone for their work here!

@hsbt
Copy link
Copy Markdown
Member

hsbt commented Dec 8, 2018

@schneems Thanks for your explanation. I worried that the this PR's behavior breaks heroku ecosystem. I think we should support backward compatibility with Gemfile.lock and Bundler versions at first because heroku have only 1 version of bundler like bundler 2.

After reading your comment, Because I understood heroku have 2 versions of bundler that are built-in bundler 2 and 1.15.2 installed by heroku platform. I have no concerns about this pull-request.

@colby-swandale I accept with your proposal. After merging this, I'm going to backport this into RubyGems 2 branch and release 2.7.9.

@hsbt
Copy link
Copy Markdown
Member

hsbt commented Dec 10, 2018

I did investigate today. I couldn't reproduce @schneems 's case. I tried with:

  • Ruby 2.6.0-rc1 with Bundler 2 + Bundler 1.15.2
  • Ruby 2.6.0-rc1 with Bundler 2 + Bundler 1.15.2 + This patch

I could invoke bundle install and bundle exec rake -AT with the sample Rails app each case.

I'm not sure if that's addressed in this current patch, or if it needs a separate issue. Please let me know.

I think that it's the root cause of heroku's issue.

@schneems Can you provide the reproduction environment or instructions? I try to investigate it before releasing Ruby 2.6.0.

@colby-swandale
Copy link
Copy Markdown
Member Author

@hsbt deploy https://github.com/schneems/ruby_26_bundle_1-17-1 to heroku and it will fail on the first deploy.

@hsbt
Copy link
Copy Markdown
Member

hsbt commented Dec 10, 2018

@colby-swandale Thanks! I'm okay to merge this without heroku issue.

@segiddins
Copy link
Copy Markdown
Contributor

I don't really like this change. I (personally, perhaps) want to lock an individual version of bundler. The only reason we didn't do that for Bundler < 2 is that it would be a breaking change. As someone who manages some systems that invoke bundler, I want to control what version of bundler is used, exactly the same as I can lock the version of every other gem that is used.

This functionality was built to support Bundler's postit/trampoline functionality which was removed about a year ago that would automatically install the locked version of Bundler in the Gemfile.lock

This assertion is not true. This functionality was build specifically to replace the trampolining functionality, which I removed from Bundler.

This code is an issue now because after Bundler 2 is released, everyone would have to manually install the exact version of Bundler for each Gemfile a person is working on.

That's not an issue to me. That's exactly the point.

@deivid-rodriguez
Copy link
Copy Markdown
Contributor

@segiddins I fully agree with you.

The thing is, this is an unexpected breaking change in bundler 2. We decided to release bundler 2 with the single "breaking change" of dropping support for old rubies in order to ease integration in ruby core. We modified bundler to not introduce any breaking changes, but we oversaw this single breaking change.

So I propose, instead of fully dropping this change, to replace > 2 with > 3.

Regarding the trampoline stuff. In my opinion, in order to ship this change in a smooth manner, we should make it so that bundle install does not fail if the locked bundler version does not match the running bundler version. Instead, it should automatically install the locked bundler version, and exec to it. I thought this was the trampoline stuff that got reverted, and my idea was to give it a try again, since @colby-swandale said that it was reverted because it had bugs and it didn't work as we expected.

@indirect
Copy link
Copy Markdown

Yeah... I think eventually we will need the ability to lock to an exact Bundler version, but I think we also need something completely automated to handle installing the locked version, if we require an exact match.

Since the focus of Bundler 2 has shifted to cleanup with minimal breakage, and we don't have automatic Bundler version management, I think we should continue to allow major version matching for Bundler 2.

Hopefully we can circle back around and find an automatic solution that works for everyone before Bundler 3 next year, and ship exact version locking as part of Bundler 3.

@colby-swandale
Copy link
Copy Markdown
Member Author

@segiddins Thanks for the clarification 👏.

Could we have a config option to enable the version locking in RubyGems if people would like that? We could then get both functions in and operating.

@indirect
Copy link
Copy Markdown

It seems like this change is a blocker for a patch release of rubygems, which is a blocker for Bundler 2.0.0, so I’m going to go ahead and merge it.

I would love to see a future PR that implements fully automated bundler version management via the lockfile, that we can make the default behavior for a version of bundler that enforces and exact version match.

@bundlerbot r+

ghost pushed a commit that referenced this pull request Dec 11, 2018
2515: [BudlerVersionFinder] set .filter! and .compatible? to match only on major versions r=indirect a=colby-swandale

# Description:

This is a complicated problem so I will do my best to explain it well. When people use Bundler, there is a bit of functionality in RubyGems called the `BundlerVersionFinder`, that filters the available Bundler specs that RubyGems could activate - depending on the Gemfile.lock.

For a `Gemfile.lock` that is `BUNDLED WITH` 1.17.1, the `BundlerVersionFinder` allows for any version of Bundler 1. But for Bundler 2, only the exact same version is allowed.

https://github.com/rubygems/rubygems/blob/fa9c3b6cc8e8d20b0486524c5eec851768e594fe/lib/rubygems/bundler_version_finder.rb#L40-L43

This functionality was built to support Bundler's postit/trampoline functionality which was removed about a year ago that would automatically install the locked version of Bundler in the `Gemfile.lock`

This code is an issue now because after Bundler 2 is released, everyone would have to manually install the exact version of Bundler for each `Gemfile` a person is working on. Going forward, I want to keep the functionality the same as Bundler 1. A `Gemfile.lock` that that was bundled with `2.0.0` is compatible with `2.1.0`, `2.1.1.0`, `2.9999`, but not `3.0.0`. And a `Gemfile.lock` that that was bundled with `3.0.0` Is compatible with `3.1.0`, `3.999` and so on.

# Tasks:

- [ ] Describe the problem / feature
- [ ] Write tests
- [ ] Write code to solve the problem
- [ ] Get code review from coworkers / friends

I will abide by the [code of conduct](https://github.com/rubygems/rubygems/blob/master/CODE_OF_CONDUCT.md).

Co-authored-by: Colby Swandale <me@colby.fyi>
@ghost
Copy link
Copy Markdown

ghost commented Dec 11, 2018

Build succeeded

@ghost ghost merged commit 28ed2dd into master Dec 11, 2018
@ghost ghost deleted the colby/bundler-finder-major-version branch December 11, 2018 01:41
matzbot pushed a commit to ruby/ruby that referenced this pull request Dec 12, 2018
  * [BudlerVersionFinder] set .filter! and .compatible? to match only on major versions ruby/rubygems#2515
  * Fix broken symlink that points to ../* ruby/rubygems#2516

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@66347 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
schneems added a commit to heroku/heroku-buildpack-ruby that referenced this pull request Feb 6, 2019
## Problem 1

Bundler 2.x cannot be used with a Gemfile.lock that specifies bundler 1.x:

```
BUNDLED WITH
   1.16.4
```

If you try, then Bundler 2.x will look for an installed bundler 1.x version and try to use it. If no such version exists on the system then an error is thrown:


```
can't find gem bundler (>= 0.a) with executable bundle (Gem::GemNotFoundException)
```


- ruby/rubygems#2426
- ruby/rubygems#2515


## Problem 2

Likewise a version of bundler 1.x cannot be used on a project where a `BUNDLED WITH` specifies 2.x:


```
$ bundle -v
1.17.3
$ cat Gemfile.lock | grep -A 1 BUNDLED WITH
BUNDLED WITH
  2.0.1
$ bundle
Traceback (most recent call last):
	2: from /Users/rschneeman/.gem/ruby/2.6.0/bin/bundle:23:in `<main>'
	1: from /Users/rschneeman/.rubies/ruby-2.6.0/lib/ruby/2.6.0/rubygems.rb:302:in `activate_bin_path'
/Users/rschneeman/.rubies/ruby-2.6.0/lib/ruby/2.6.0/rubygems.rb:283:in `find_spec_for_exe': Could not find 'bundler' (2.0.0) required by your /private/tmp/default_ruby/Gemfile.lock. (Gem::GemNotFoundException)
To update to the latest version installed on your system, run `bundle update --bundler`.
To install the missing version, run `gem install bundler:2.0.0`
```

This is due to a bug in Rubygems 3.x ruby/rubygems#2592


## Proposed Solution

This PR implements a solution where we have a different "blessed" version of bundler for each major version. 

The way this is implemented is to read in a Gemfile.lock and to read in the `BUNDLED WITH` value. Then the major version is pulled out and converted into a "blessed" version.

This allows for multiple major versions of bundler to be used on Heroku without sacrificing stability.
@hsbt hsbt modified the milestones: 2.7.9, 3.0.4 Mar 6, 2019
hsbt pushed a commit that referenced this pull request Mar 6, 2019
2515: [BudlerVersionFinder] set .filter! and .compatible? to match only on major versions r=indirect a=colby-swandale

# Description:

This is a complicated problem so I will do my best to explain it well. When people use Bundler, there is a bit of functionality in RubyGems called the `BundlerVersionFinder`, that filters the available Bundler specs that RubyGems could activate - depending on the Gemfile.lock.

For a `Gemfile.lock` that is `BUNDLED WITH` 1.17.1, the `BundlerVersionFinder` allows for any version of Bundler 1. But for Bundler 2, only the exact same version is allowed.

https://github.com/rubygems/rubygems/blob/fa9c3b6cc8e8d20b0486524c5eec851768e594fe/lib/rubygems/bundler_version_finder.rb#L40-L43

This functionality was built to support Bundler's postit/trampoline functionality which was removed about a year ago that would automatically install the locked version of Bundler in the `Gemfile.lock`

This code is an issue now because after Bundler 2 is released, everyone would have to manually install the exact version of Bundler for each `Gemfile` a person is working on. Going forward, I want to keep the functionality the same as Bundler 1. A `Gemfile.lock` that that was bundled with `2.0.0` is compatible with `2.1.0`, `2.1.1.0`, `2.9999`, but not `3.0.0`. And a `Gemfile.lock` that that was bundled with `3.0.0` Is compatible with `3.1.0`, `3.999` and so on.

# Tasks:

- [ ] Describe the problem / feature
- [ ] Write tests
- [ ] Write code to solve the problem
- [ ] Get code review from coworkers / friends

I will abide by the [code of conduct](https://github.com/rubygems/rubygems/blob/master/CODE_OF_CONDUCT.md).

Co-authored-by: Colby Swandale <me@colby.fyi>
(cherry picked from commit c6bf378)
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants