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

require "spring/commands" after boot_server #460

Closed

Conversation

mgi166
Copy link

@mgi166 mgi166 commented Dec 25, 2015

What is PR

require "spring/commands" calls after boot_server, but calls before run_command.

Related to #458, #453, #456

Environments

I experimented this environments.

  • ruby-2.2.3
  • spring
    • spring-1.6.0
    • spring-1.6.1
  • bundler
    • bundler-1.11.0
    • bundler-1.11.2

Problem

Use spring-1.6.x and bundler-1.11.x combination, spring command failed with follow error.

% spring stop; rails c
Spring stopped.

/Users/hiromu/.rbenv/versions/2.2.3/lib/ruby/site_ruby/2.2.0/rubygems/core_ext/kernel_require.rb:54:in `require': cannot load such file -- bundler/setup (LoadError)
        from /Users/hiromu/.rbenv/versions/2.2.3/lib/ruby/site_ruby/2.2.0/rubygems/core_ext/kernel_require.rb:54:in `require'
        from /Users/hiromu/work/my-project/vendor/bundle/gems/spring-1.6.0/lib/spring/commands.rb:33:in `<module:Spring>'
        from /Users/hiromu/work/my-project/vendor/bundle/gems/spring-1.6.0/lib/spring/commands.rb:4:in `<top (required)>'
        from /Users/hiromu/.rbenv/versions/2.2.3/lib/ruby/site_ruby/2.2.0/rubygems/core_ext/kernel_require.rb:54:in `require'
        from /Users/hiromu/.rbenv/versions/2.2.3/lib/ruby/site_ruby/2.2.0/rubygems/core_ext/kernel_require.rb:54:in `require'
        from /Users/hiromu/work/my-project/vendor/bundle/gems/spring-1.6.0/lib/spring/application.rb:77:in `preload'
        from /Users/hiromu/work/my-project/vendor/bundle/gems/spring-1.6.0/lib/spring/application.rb:143:in `serve'
        from /Users/hiromu/work/my-project/vendor/bundle/gems/spring-1.6.0/lib/spring/application.rb:131:in `block in run'
        from /Users/hiromu/work/my-project/vendor/bundle/gems/spring-1.6.0/lib/spring/application.rb:125:in `loop'
        from /Users/hiromu/work/my-project/vendor/bundle/gems/spring-1.6.0/lib/spring/application.rb:125:in `run'
        from /Users/hiromu/work/my-project/vendor/bundle/gems/spring-1.6.0/lib/spring/application/boot.rb:18:in `<top (required)>'
        from /Users/hiromu/.rbenv/versions/2.2.3/lib/ruby/site_ruby/2.2.0/rubygems/core_ext/kernel_require.rb:54:in `require'
        from /Users/hiromu/.rbenv/versions/2.2.3/lib/ruby/site_ruby/2.2.0/rubygems/core_ext/kernel_require.rb:54:in `require'
        from -e:1:in `<main>'

I think, there is same issue #458, #453, #456

Reason

There is related two reason, this problem is complicative.
The one is that Bundler.with_clean_env problem refs with_clean_env does not replace original RUBYLIB environmental variable · Issue #3982 · bundler/bundler.
The two is that Spring::Client::Run#boot_server receive GEM_PATH env, but if require "bundler/setup" overrides Gem.path, Spring::Client::Run#boot_server receives overrided env. Therefore child process does not found bundler/setup.

Summary is follow this.

  • bundler-1.10.x
    • spring-1.5.0
      • RUBYLIB does not replace in Bundler.with_clean_env, therefore successes require "bundler/setup"
    • spring-1.6.x
      • RUBYLIB does not replace in Bundler.with_clean_env, therefore successes require "bundler/setup"
  • bundler-1.11.x
    • spring-1.5.0
      • RUBYLIB replaced in Bundler.with_clean_env, BUT Gem.path does not override Gem.path because does not exist require "spring/commands", therefore successes require "bundler/setup"
    • spring-1.6.x
      • RUBYLIB replaced in Bundler.with_clean_env, AND Gem.path overrides Gem.path because exist require "spring/commands", therefore failed to require "bundler/setup"

Feedback

What do you think this conclusion? Please let me any feedbacks.

* Calls `require "bundler/setup"` in `require "spring/commands"`.
  `require "bundler/setup"` overrides Gem.path, so changes the env
  which pass spawned process and child process does not found bundler.
  Therefore requires "bundler/setup" after calling Spring::Run#boot_server.
@grosser
Copy link
Collaborator

grosser commented Dec 25, 2015

❤️ thats some thoughtful PR :)

inline requires are not great, but we are not really concerned about performance at this point so I'd say 👍

@jonleighton looks good to you ?

@mgi166
Copy link
Author

mgi166 commented Dec 28, 2015

Sorry, I found the mistake that is failure to boot command from bundle exec. (ex. spring stop; bundle exec rails c)
This is because Spring::Client::Run#boot_server receives overrided env.

I think different approach to fix this problem.

Thanks for review!

@sonalkr132
Copy link
Contributor

hi @mgi166
@jonleighton suggested here that require "spring/commands" line can be dropped all together.

@sonalkr132
Copy link
Contributor

Spring.quite can just be defined in config/spring_client.rb. However, I don't know to test proof this bug.

@jonleighton
Copy link
Member

As @sonalkr132 said, I proposed a solution here. Basically I think we should never be requiring spring/commands on the client side of Spring.

@mgi166
Copy link
Author

mgi166 commented Jan 5, 2016

Thank you for feedbacks. close this pull request.
I also see this discussion.

@mgi166 mgi166 closed this Jan 5, 2016
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.

4 participants