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

Set Ruby version in Gemfile and .ruby-version by default #30016

Conversation

@albertoalmagro
Copy link
Contributor

@albertoalmagro albertoalmagro commented Jul 31, 2017

Summary

This pull request sets current Ruby version in the Gemfile and .ruby-version by default.

When running rails new my-app the generated application will include by default:

  • A .ruby-version file, containing the default Ruby version.
  • An entry in the Gemfile, with the format ruby '2.x.x' with the same default Ruby version.

Implications

Among other usages:

  • .ruby-version will help ruby version manager tools such as rbenv to determine which Ruby version should be used for the current Rails project.
  • The entry ruby 'version' in the Gemfile may be used by deployment tools (i.e. Heroku) to determine the Ruby version that should be used to run Rails.
@rails-bot
Copy link

@rails-bot rails-bot commented Jul 31, 2017

Thanks for the pull request, and welcome! The Rails team is excited to review your changes, and you should hear from @schneems (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

This repository is being automatically checked for code quality issues using Code Climate. You can see results for this analysis in the PR status below. Newly introduced issues should be fixed before a Pull Request is considered ready to review.

Please see the contribution instructions for more information.

railties/lib/rails/generators/rails/app/templates/Gemfile Outdated
@@ -1,4 +1,5 @@
source 'https://rubygems.org'
ruby <%= "'#{RUBY_VERSION}'" %>

This comment has been minimized.

@rafaelfranca

rafaelfranca Jul 31, 2017
Member

Could you put this after the git_source entry?

@albertoalmagro albertoalmagro force-pushed the albertoalmagro:set-ruby-version-in-gemfile-and-ruby-version-by-default branch Jul 31, 2017
@albertoalmagro
Copy link
Contributor Author

@albertoalmagro albertoalmagro commented Jul 31, 2017

@rafaelfranca Done. As it was a minor change I have already squashed commits. In case requested changes implied more lines, would you prefer first to review the changes? After it is done I would proceed squashing commits.

Copy link
Member

@rafaelfranca rafaelfranca left a comment

That is great! Could you add a CHANGELOG entry too and squash the commits?

@albertoalmagro albertoalmagro force-pushed the albertoalmagro:set-ruby-version-in-gemfile-and-ruby-version-by-default branch to fd7f978 Jul 31, 2017
@albertoalmagro
Copy link
Contributor Author

@albertoalmagro albertoalmagro commented Jul 31, 2017

Copy link
Member

@guilleiguaran guilleiguaran left a comment

Looking good, waiting for CI to merge 😊

@@ -1,5 +1,6 @@
source 'https://rubygems.org'
git_source(:github) { |repo| "https://github.com/#{repo}.git" }
ruby <%= "'#{RUBY_VERSION}'" %>

This comment has been minimized.

@deivid-rodriguez

deivid-rodriguez Jul 31, 2017
Contributor

What about using ruby RUBY_VERSION here? Seems a bit DRYer.

This comment has been minimized.

@deivid-rodriguez

deivid-rodriguez Jul 31, 2017
Contributor

I use this "trick", but I haven't tried it with heroku, so maybe it doesn't work well, not sure.

This comment has been minimized.

@albertoalmagro

albertoalmagro Jul 31, 2017
Author Contributor

Thanks @deivid-rodriguez , I already tried that and generating the app with bundle exec rails new ~/my-test-app --dev caused the following error parsing the Gemfile:

[!] There was an error parsing `Gemfile`: unexpected fraction part after numeric literal - ruby 2.3.1
        ^
/Users/alberto.almagro/my-test-app/Gemfile:3: syntax error, unexpected tIDENTIFIER, expecting end-of-input
git_source(:github) { |repo| "https://github.com/#{repo}.git" }
          ^. Bundler cannot continue.

 #  from /Users/alberto.almagro/my-test-app/Gemfile:2
 #  -------------------------------------------
 #  source 'https://rubygems.org'
 >  ruby 2.3.1
 #  git_source(:github) { |repo| "https://github.com/#{repo}.git" }
 #  -------------------------------------------

I've surrounded it with single quotes, which is what is used in the line above for the source entry.

This comment has been minimized.

@deivid-rodriguez

deivid-rodriguez Aug 9, 2017
Contributor

Sorry @albertoalmagro, totally forgot to answer this. What I meant is not to generate ruby 2.3.1 without quotes, but to generate ruby RUBY_VERSION, which I think would be generated by using ruby RUBY_VERSION (wihout <%=, %> block).

This comment has been minimized.

@schneems

schneems Aug 9, 2017
Member

This defeats the point of putting Ruby version in the Gemfile. Now any version you are using will be valid from 1.8.7 to 2.4.1.

I think we should revert this.

This comment has been minimized.

@greysteil

greysteil Aug 9, 2017
Contributor

@schneems - I can't see how that would happen as long as the .ruby_version file is committed. Am I missing something?

@guilleiguaran guilleiguaran merged commit 0d58e7e into rails:master Jul 31, 2017
2 checks passed
2 checks passed
codeclimate All good!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@greysteil
Copy link
Contributor

@greysteil greysteil commented Aug 9, 2017

Nice! Would you be up for reading the Ruby version out of the .ruby-version file, rather than having it duplicated? For example, like the ossfirday Gemfile does? I've seen applications get out-of-sync before...

Happy to put in a PR if so.

@deivid-rodriguez
Copy link
Contributor

@deivid-rodriguez deivid-rodriguez commented Aug 9, 2017

@greysteil What about ruby RUBY_VERSION? Is it something discouraged or not behaving as expected in some cases?

@greysteil
Copy link
Contributor

@greysteil greysteil commented Aug 9, 2017

@deivid-rodriguez interesting. It makes an assumption about the Ruby manager respecting the .ruby-version file, but that should basically always be the case. Just tested on Heroku and it works there, too, so I'd be 👍 on that change.

@albertoalmagro - any thoughts?

@greysteil
Copy link
Contributor

@greysteil greysteil commented Aug 9, 2017

I suppose one minor objection to the environment variable approach would be that it's less explicit where the Ruby version is being set. Might cause confusion, particularly for a newer developer? And if the .ruby-version file was deleted it would be left as a confusing remnant, rather than erroring.

@robin850
Copy link
Member

@robin850 robin850 commented Aug 9, 2017

ISTM that having ruby RUBY_VERSION isn't really useful.

Locally, hard-coding the Ruby version inside the Gemfile prevents it from resolving if someone is trying to bundle install with the wrong version but ruby RUBY_VERSION will always resolve.

As for Heroku, the ruby call inside the Gemfile allows Heroku to automatically pick the specified version. It ensures that people have a similar set-up between their local environment and the production one.

@greysteil
Copy link
Contributor

@greysteil greysteil commented Aug 9, 2017

It should be useful - RUBY_VERSION will be set to whatever is in .ruby-version before Bundler evaluates the Gemfile. As a result, Bundler will still give us the behaviour we want (install scoped to specified Ruby version) and a test on Heroku shows that it does the same. So we get equivalent behaviour, but without the hardcoding.

Given the confusion, I'm not sure the environment variable approach is clear enough, though - I think I prefer reading the .ruby-version file explicitly in the Gemfile (which DRYs up what we have here, but is still ultra-explicit).

@greysteil
Copy link
Contributor

@greysteil greysteil commented Aug 9, 2017

I'll create a new PR so we're not spamming poor @albertoalmagro. This is still a rad contribution though - nice one @albertoalmagro.

@albertoalmagro
Copy link
Contributor Author

@albertoalmagro albertoalmagro commented Aug 9, 2017

Nice one @greysteil . Opportunities to learn something new and fresh ideas are never spam ;)

@robin850
Copy link
Member

@robin850 robin850 commented Aug 9, 2017

RUBY_VERSION will be set to whatever is in .ruby-version before Bundler evaluates the Gemfile

Not everybody uses a Ruby version manager.

@schneems
Copy link
Member

@schneems schneems commented Aug 9, 2017

I use chruby which does not use the .ruby-version file. It would be better to have rbenv understand how to read the ruby version from Gemfile.lock.

I would like to revert this change.

@schneems
Copy link
Member

@schneems schneems commented Aug 9, 2017

I reverted the change a8f5904.

The good news is you get to keep your contributor points for contributors.rubyonrails.org !! Reverts don't actually remove a commit, it adds an extra "undo" commit that changes back all your changes.

Thanks for the PR, sorry I wasn't paying more attention before. It's always better to have people involved earlier than later. I appreciate all the effort that went into making this.

@albertoalmagro
Copy link
Contributor Author

@albertoalmagro albertoalmagro commented Aug 9, 2017

@schneems Lets get more on my next PR, which is already in CR. The most important thing is to make Rails better, thanks.

@dhh
Copy link
Member

@dhh dhh commented Aug 9, 2017

I think we've crossed wires here a bit. Given that I originally requested this feature, perhaps I was unclear with the rational. I'm fine with the fact that the version is duplicated between .ruby-version and Gemfile. Just as long as they're set by default.

If at some point rbenv learns to read the version from a Gemfile, then we can drop the .ruby-version. But until that I'd like that to stay.

@dhh
Copy link
Member

@dhh dhh commented Aug 9, 2017

Setting the Ruby version in the Gemfile just raises an exception if it doesn't match. You still need to ensure that the correct Ruby version is actually running, which is what .ruby-version supplies for rbenv and rvm.

@dhh
Copy link
Member

@dhh dhh commented Aug 9, 2017

I reverted the revert. Sorry for the confusion! Happy to drop .ruby-version if the day should come that rbenv and rvm support parsing the Gemfile to get the Ruby version. But it seems uncertain whether, and if so, when that would happen. In the mean time this will do.

dhh added a commit that referenced this pull request Aug 9, 2017
…ult""

This reverts commit a8f5904.

See discussion on #30016.
@schneems
Copy link
Member

@schneems schneems commented Aug 9, 2017

Ahh, yes. Sorry. I made the mistake here.

@n-rodriguez
Copy link

@n-rodriguez n-rodriguez commented Mar 21, 2018

@dhh rvm already support Ruby version detection through Gemfile : https://rvm.io/workflow/projects#supported-files

@headius
Copy link
Contributor

@headius headius commented Apr 19, 2018

.ruby-version does not work the same way with JRuby and has led to #32639. It needs to be engine-engine_version to correctly be able to switch to JRuby.

I would strongly advocate against using .ruby-version, however, since we have many users on JRuby that use both JRuby and CRuby interchangeably for local development.

@rafaelfranca
Copy link
Member

@rafaelfranca rafaelfranca commented Apr 19, 2018

Those users can remove the file, if they want. I don't think we should avoid using a good feature that make sure everyone in the same project is using the same version of Ruby that works in 80% of the cases just because it doesn't work for 20%.

@guilleiguaran
Copy link
Member

@guilleiguaran guilleiguaran commented Apr 19, 2018

This .ruby_version is compatible with both CRuby and JRuby: #32649

@headius
Copy link
Contributor

@headius headius commented Apr 19, 2018

@rafaelfranca Fair enough...we'll see if anyone cares.

@rafaelfranca
Copy link
Member

@rafaelfranca rafaelfranca commented Apr 19, 2018

I mean, we should fix, with Guillermo PR, I don't think JRuby should be broken by default. I just think that the cases where you need to have different implementations, which is not common, should be handled differently.

One could say that if people want to use JRuby and CRuby for local development they can use rbenv shell feature (that I think have similars in the other version manager) to override the .ruby-version for the duration of the shell. This is what I do when upgrading Ruby for example in a repository that have a .ruby-version file.

composerinteralia added a commit to composerinteralia/suspenders that referenced this pull request May 2, 2018
Rails 5.2.0 will give us ActiveStorage and all sorts of other goodies:
http://edgeguides.rubyonrails.org/5_2_release_notes.html

rails/rails#30016 introduced `Rails::AppBuilder#ruby_version` to create
a .ruby_version file. To avoid getting stuck in an overwrite prompt
when we try to create our own .ruby_version file based on
`Suspenders::RUBY_VERSION`, I overrode `ruby_version`.

Rails 5.2.0 adds bootsnap to the Gemfile and to config/boot.rb. I added it
to our Gemfile, but we could also `skip_bootsnap` by default if we don't
want to use it.
mike-burns added a commit to thoughtbot/suspenders that referenced this pull request May 25, 2018
Rails 5.2.0 will give us ActiveStorage and all sorts of other goodies:
http://edgeguides.rubyonrails.org/5_2_release_notes.html

rails/rails#30016 introduced `Rails::AppBuilder#ruby_version` to create
a .ruby_version file. To avoid getting stuck in an overwrite prompt
when we try to create our own .ruby_version file based on
`Suspenders::RUBY_VERSION`, I overrode `ruby_version`.

Rails 5.2.0 adds bootsnap to the Gemfile and to config/boot.rb. I added it
to our Gemfile, but we could also `skip_bootsnap` by default if we don't
want to use it.
bogdanvlviv added a commit to bogdanvlviv/rails that referenced this pull request Jun 18, 2018
Since rails#30016 Rails generates `.ruby-version` file
in order to help Ruby version manager tools like `rbenv`, `rvm`
determine which Ruby version should be used for the current Rails
project.

Since rails#32649 Rails sets Ruby version to the file compatible with MRI/JRuby
by default.

Pull Request rails#31496 reports that `.ruby-version` doesn't match ruby version other
than stable version and recommends to use `ENV["RBENV_VERSION"]`, and
`ENV["rvm_ruby_string"]` in order to set correct Ruby version to the file
that `rbenv` or `rvm` can understand.
Also, there is another similar issue that reports the same case if use
JRuby jruby/jruby#5144.

Closes rails#31496, jruby/jruby#5144.
kevin-j-m added a commit to TheGnarCo/gnarails that referenced this pull request Jan 10, 2019
This updates the version of ruby tested in the application to be ruby
2.6.0. This still relies on the `RUBY_VERSION` constant that's defined
in ruby to know what version of ruby to include.

This also removes the code that will add the ruby version to the
Gemfile. Rails 5.2 will add the ruby version to the Gemfile for you,
based on this contribution: rails/rails#30016.
kevin-j-m added a commit to TheGnarCo/gnarails that referenced this pull request Jan 10, 2019
This updates the version of ruby tested in the application to be ruby
2.6.0. This still relies on the `RUBY_VERSION` constant that's defined
in ruby to know what version of ruby to include.

This also removes the code that will add the ruby version to the
Gemfile. Rails 5.2 will add the ruby version to the Gemfile for you,
based on this contribution: rails/rails#30016.
kevin-j-m added a commit to TheGnarCo/gnarails that referenced this pull request Feb 18, 2019
This updates the version of ruby tested in the application to be ruby
2.6.0. This still relies on the `RUBY_VERSION` constant that's defined
in ruby to know what version of ruby to include.

This also removes the code that will add the ruby version to the
Gemfile. Rails 5.2 will add the ruby version to the Gemfile for you,
based on this contribution: rails/rails#30016.
kevin-j-m added a commit to TheGnarCo/gnarails that referenced this pull request Feb 18, 2019
This updates the version of ruby tested in the application to be ruby
2.6.1. This still relies on the `RUBY_VERSION` constant that's defined
in ruby to know what version of ruby to include.

This also removes the code that will add the ruby version to the
Gemfile. Rails 5.2 will add the ruby version to the Gemfile for you,
based on this contribution: rails/rails#30016.

The latest Circle images utilize bundler >= 2, which is causing issues
for the template install process. This circumvents that by modifying the
image to remove the currently-installed version of bundler, and
replacing it with a specific version that's known to work. Additional
effort will be required to support bundler >= 2.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet