use version operators for specifying ruby version in Gemfile #4064
Conversation
* Use `Gem::Requirement` and `Gem::Version` to parse and check ruby version, engine_version and patchlevel requirements. * Added new class `RubyVersionRequirement` to handle this functionality. * Added tests that cover the `RubyVersionRequirement` class.
| @@ -307,6 +307,11 @@ def to_lock | |||
| out << " #{p}\n" | |||
| end | |||
|
|
|||
| if ruby_version | |||
There was a problem hiding this comment.
this will be backwards-incompatible with versions of bundler prior to 1.9.6, I believe
There was a problem hiding this comment.
Do you mean the Gemfile.lock will be incompatible? If so, what is the collision, because wouldn't BUNDLED WITH also be backwards incompatible too?
There was a problem hiding this comment.
@segiddins could you please clarify. If BUNDLED WITH is already backwards incompatible, what is stopping from adding of this feature then.
Signed-off-by: David Morhovich <dmorhovich@pivotal.io>
|
@panthomakos we redid the pull request to merge your PR, so the commit history is maintained. |
|
Bundled with took me three tries to make backwards compatible. To be honest, I feel like, if this is a road we want to go down, it should target the 2.0 branch, as I'm very nervous about changing the lock file for 1.x |
|
@segiddins, Please share what those issues were. If I can work around those problems, there are a series of use cases that this feature help us solve. Would a g-hangout be easier to discuss this? We've only changed the lock file because the original PR have comments from @indirect. We care more about the version operators for the |
|
I think it's possible to take the same approach we took in BUNDLED WITH, and lead with three spaces so old Bundlers will just ignore the line. Keep in mind, though, that using this feature will require everyone on a team and all production boxes to have a version of Bundler that supports this. |
|
@indirect thanks for the input. We'll add the needed spacing to keep backwards compatible. We'll investigate if we might be able to add integration tests for backwards compatibility too. That way if any future changes are made, it will blow up for someone in the test suite. I emphasize, maybe we can do this. :-) |
jtarchie
force-pushed
the
master
branch
2 times, most recently
from
1c71bf2
to
864e752
Compare
jtarchie
changed the title
|
This looks good! One thing I'm not sure about is how you upgrade your locked Ruby version once it's in your lockfile... what if you're locked to 2.2.3 and the Gemfile changes to say "~> 2.3.0"? |
|
Hey @indirect, here are our thoughts on how that behavior would work. Let us know if something looks off. Thanks! |
|
@jtarchie @mavenraven that looks reasonable. I think I would prefer that the user run |
jtarchie
force-pushed
the
master
branch
2 times, most recently
from
0736729
to
b6740f7
Compare
* Adds support for `bundle update --ruby` command to allow for updating ruby version in Gemfile.lock. * Adds support for `bundle install` to allow for adding ruby version in Gemfile.lock. https://www.pivotaltracker.com/story/show/106008092 Signed-off-by: JT Archie <jtarchie@gmail.com>
|
Hey @indirect, we've updated the PR with tests and implementation that cover the above use cases. We also added coverage that uses |
|
@indirect is there anything else you'd need in this pull request? |
|
+1 This would be very helpful. It's not easy to figure out what versions of the ruby buildpack support a specific Ruby version, and manually specifying the the Github url in our manifest has a tendency to break. I would much prefer specifying my Ruby version and not knowing the buildpack details. |
|
@kayline this won't change the Ruby buildpack—Heroku's buildpack does not generally upgrade to new Bundler versions. |
|
@homu r+ |
|
📌 Commit b4a52c3 has been approved by |
| @@ -113,4 +97,32 @@ def patchlevel | |||
| RUBY_PATCHLEVEL.to_s | |||
| end | |||
| end | |||
|
|
|||
| class RubyVersionRequirement < RubyVersion | |||
There was a problem hiding this comment.
requirement should not subclass version
There was a problem hiding this comment.
@segiddins changed, see vmware-archive@9f6ec80
|
☔ The latest upstream changes (presumably #4063) made this pull request unmergeable. Please resolve the merge conflicts. |
Reintroduces test that was removed by 2c8dd13 which prevents a status code regression. https://www.pivotaltracker.com/story/show/106008092
|
Hey @segiddins @indirect @jtarchie @homu We've added the changes that @segiddins suggested, and remerged into master. Can you retry homu when you get a chance please? Thank you! |
|
Hey @indirect, @segiddins can you please take a look at our changes when you get a chance? I'm afraid that they're going to fall out of date again. |
|
☔ The latest upstream changes (presumably #4124) made this pull request unmergeable. Please resolve the merge conflicts. |
|
Don't worry about rebasing again, I believe @indirect said he'd handle getting this merged. |
|
thanks @segiddins! Let me know if you need anything @indirect. |
|
Merged this in the branch 4064, and will fast forward master once the build passes. |
|
Merged, will be released as part of 1.12. 🎉 |
|
hurray! |
|
👍 |
|
Thanks for the help @indirect and @segiddins |
|
Hey @indirect, any idea when 1.12 is going to be released?? |
|
@blackjid The Bundler |
|
thanks @RochesterinNYC ! |
Related to the original PR, we are adding the requested meta data to the
Gemfile. So far we have a proposedRUBY VERSIONthat would store the meta information of the Ruby.You'll be able to support version operators on the version of Ruby you'd like to support.
For example, if you wanted the latest patch release from Ruby 2.2.
This way the Ruby version selected could always be the latest stable. Especially useful for deployments on production environments that are constantly supporting the latest security release of Ruby (eg: Heroku and PWS).