Skip to content

Unify method to detect Gemfile name #538

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

Closed
wants to merge 1 commit into from

Conversation

mizalewski
Copy link

Background
Bundler 1.9 added support for changed Gemfile name: gems.rb. (related bundler issue)

Spring use Bundler method to find Gemfile in binstub but in configuration it use custom method, which don't cover new Gemfile name (gems.rb).

Pull request
This pull request unify Gemfile name detection by using Bundler method in both places. Old behavior still works like expected and now support also gems.rb file.

@rails-bot
Copy link

Thanks for the pull request, and welcome! The Rails team is excited to review your changes, and you should hear from @rafaelfranca (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.

Please see the contribution instructions for more information.

@sergey-alekseev
Copy link

I used it in my gems.rb (gem 'spring', github: 'mizalewski/spring') and got Illformed requirement [">= 1.2, < 3.0"] (Gem::Requirement::BadRequirementError).

@mizalewski
Copy link
Author

Hi sergey,

Please let know what version of bundler and gem (gem -v) do you have.
Do problem occur when you add spring from master branch (gem 'spring', github: 'rails/spring')?

@JuPlutonic
Copy link

JuPlutonic commented Sep 9, 2017

(spring init says `find_project_root': Spring was unable to locate the root of your project. There was no Gemfile present .......... )

Application root files: gems.rb gems.locked package.json Rakefile config.ru README.md

gem -v 2.6.13
bundle -v Bundler version 1.15.4
gist with mizalewski/spring
gist with rails/spring

@sergey-alekseev
Copy link

Do problem occur when you add spring from master branch (gem 'spring', github: 'rails/spring')?

Yeah, same error.

Do:

  1. git clone https://github.com/sergey-alekseev/test-spring
  2. cd test-spring
  3. bundle
  4. rails s

@mizalewski
Copy link
Author

mizalewski commented Sep 12, 2017

Thank you for sample app!

I checked app and your gists. It looks like application load spring 2.0.2 from your system instead of version downloaded from github:
/Users/sergey/.rvm/gems/ruby-2.3.4@test/gems/spring-2.0.2/

Please try to run application using bundle exec. It should load correct version of spring gem:
bundle exec rails s

@JuPlutonic
Copy link

JuPlutonic commented Sep 13, 2017

bundle exec rails s Or bundle exec spring init
No errors on last branches of
Using spring 2.0.2 from https://github.com/rails/spring.git (at master@f0d2d65)
Or
Using spring 2.0.2 from https://github.com/mizalewski/spring.git (at master@6e388aa)

Support was added by using Gemfile file name detection from Bundler gem.
@sergey-alekseev
Copy link

Confirming bundle exec rails c works from https://github.com/rails/spring.git (at master@f0d2d65). However rails c doesn't work as noted.

@remomueller
Copy link
Contributor

Any update on this issue? I was able to insert the above changes into the spring 2.0.2 gem on my system, and both rails test and bundle exec rails test worked as expected.

Specifically, I made the changes to lib/spring/configuration.rb and lib/spring/test/application.rb. It'd be helpful to have this for Rails apps adopting the "gems.rb" format that are also using spring. Thanks!

@n-rodriguez
Copy link

Hi there! Any news?

@JuPlutonic
Copy link

JuPlutonic commented Jan 8, 2019

When run rails c:

Traceback.txt
Spring was unable to locate the root of your project. There was no Gemfile present in the current
directory (/home/XXX/rails_starter/rails_starter) or any of the parent directories.
(Spring::UnknownProject)…
spring/lib/spring/configuration.rb: Needs to be refactored!

However bundle exec rails c works fine!

Copy link

@JuPlutonic JuPlutonic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spring/lib/spring/configuration.rb changes doesn't work

@remomueller
Copy link
Contributor

The rails c command fails because it's loading the binary/binstub of a version of spring that doesn't have the changes present. bundle exec seems to skip loading the binary/binstub directly and instead uses the modified code. To test, you'd have to uninstall all existing versions of spring, or change the latest version of spring gem directly on the filesystem, since the older versions will interfere with testing these changes.

The changes in this pull request work fine in isolation with both rails c or bundle exec rails c.

@JuPlutonic
Copy link

JuPlutonic commented Jan 8, 2019

Yes trace shows me binstub.rb:31:in <top (required)>`
Yes I need to change this:

group :development do
  gem 'spring', git: 'git://github.com/JuPlutonic/spring.git', branch: 'JuPlutonic-patch-1')
end

to something and when gem uninstall spring; gem install --local XXXPATH/myspring.gem?

@JuPlutonic
Copy link

Using spring 2.0.2 from source ../../../../var/lib/gems/2.6.0/bundler/gems/spring-0cfe81f76cc3 (I know about, secure paths with #join method).
rails c
/home/XXX/rails_starter/rails_starter/Gemfile not found
There's not any other versions of Spring… So I lose @remomueller

@remomueller
Copy link
Contributor

@JuPlutonic I'll try it again soon on my end, see if I get the same error you're seeing.

@remomueller
Copy link
Contributor

So I did the following to make this work:

Using an app with spring installed (2.0.2), go to the root directory of the app. Change Gemfile to gems.rb, and delete Gemfile.lock (and run bundle install or bundle update if gems.locked doesn't show up).

Running bundle exec rails c will work (as you saw), and rails c will generate an error "Spring was unable to locate the root of your project. There was no Gemfile present in the current directory (_/rails_starter) or any of the parent directories. (Spring::UnknownProject)".

Next, in the root directory type: bundle open spring. This will open the source directory of the main spring gem installed in your editor. You can also type bundle show spring to see the directory it's using as well. Mine for example is: /usr/local/rvm/gems/ruby-2.6.0/gems/spring-2.0.2

Next, modify /usr/local/rvm/gems/ruby-2.6.0/gems/spring-2.0.2/lib/spring/configuration.rb to be:

    def gemfile
      # ENV['BUNDLE_GEMFILE'] || "Gemfile"
      Bundler.default_gemfile
    end

You can also modify /usr/local/rvm/gems/ruby-2.6.0/gems/spring-2.02/lib/spring/test/application.rb to include the following methods:

      def gemfile_lock
        path "Gemfile.lock"
      end

      def gems_rb
        path "gems.rb"
      end

      def gems_locked
        path "gems.locked"
      end

At this point, your main spring gem is as if the changes from this pull request were already in a mainline release.

Go back to your Rails root directory, and now rails c (as well as bundle exec rails c should correctly load the console.

rails_starter remo$ rails c
Running via Spring preloader in process 67264
Loading development environment (Rails 5.2.2)
2.6.0 :001 > Bundler.default_gemfile
 => #<Pathname:/Users/remo/code/ruby/lofthf.study/gems.rb> 

Note, I don't recommend changing gems on the filesystem in this manner for any long term solution, but it's to show that the changes from this pull request do work with and without bundle exec.

Let me know if that works, or if something doesn't make sense that I can clarify. Thanks!

@n-rodriguez
Copy link

n-rodriguez commented Jan 8, 2019

Next, modify /usr/local/rvm/gems/ruby-2.6.0/gems/spring-2.0.2/lib/spring/configuration.rb to be:

I did the same test and it's sufficient to make it works.

Actually I don't see why this PR is not merged yet since Bundler.default_gemfile does exactly the same thing than ENV['BUNDLE_GEMFILE'] || "Gemfile" in a much cleaner way : https://github.com/bundler/bundler/blob/7372d75243e6179f6258e2b5148fa89a7b64df8b/lib/bundler/shared_helpers.rb#L247

Any hints?

BTW, it should not break : https://github.com/bundler/bundler/blame/7372d75243e6179f6258e2b5148fa89a7b64df8b/lib/bundler.rb#L351

@JuPlutonic
Copy link

JuPlutonic commented Jan 8, 2019

All right. Bundle exec rails c / rails c
Bundler.default_gemfile => #<Pathname:/home/salessionato/rails_starter/rails_starter/gems.rb>
For push PR through TravisCI maybe change @mizalewski commit to:

    if /\s1.9.[0-9]/ ===  Bundler.ruby_scope.gsub(/[\/\s]+/,'')
      ENV['BUNDLE_GEMFILE'] || "Gemfile"
    else
      Bundler.default_gemfile
    end

Hmm:

WARNING: ruby-1.9.3-p551 is past its end of life and is now unsupported.

@JuPlutonic
Copy link

JuPlutonic commented Jan 8, 2019

Conflicting files
CHANGELOG.md

nothing to change -ff

lib/spring/test/acceptance_test.rb

??
@rafaelfranca Hello, excuse me can I know if above PR and example code in comment #538 (comment) (tested with Ruby 1.9.3 Spring 2.0.2) appropriate to be merged. Or please address PR to smb experienced in Rails development.

@remomueller
Copy link
Contributor

@rafaelfranca Any actions to be taken on this? I can help recreate a new pull request with @JuPlutonic's changes incorporated if needed (or help in any other way to push this forward). It'd be great to see this ready in time for Rails 6.0.0 release!

I also looked here for Ruby support of Rails, and it looks like Rails 5 (Ruby 2.2.2+) and Rails 6 (Ruby 2.5.0+) don't need the patch for Ruby 1.9.3 support, only Rails 4.2 would need this. And given that Rails 6 is out in a few months, this patch may only be needed temporarily then (as I'm guessing spring follows Rails End-Of-Life cycle) and support would be dropped for Rails 4.2 (Ruby 1.9.3).

@n-rodriguez
Copy link

@rafaelfranca fixed by 647b8c3

Thank you! 👍

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