Merge pull request #662 from rails/rm-new-format-binstub

Modernize spring binstubs and disable it in production

lib/spring/client/binstub.rb

@@ -11,41 +11,36 @@ class Binstub < Command
       # client is not invoked for whatever reason, then the Kernel.exit won't
       # happen, and so we'll fall back to the lines after this block, which
       # should cause the "unsprung" version of the command to run.
-      LOADER = <<CODE
-begin
-  load File.expand_path('../spring', __FILE__)
-rescue LoadError => e
-  raise unless e.message.include?('spring')
-end
-CODE
+      LOADER = <<~CODE
+        load File.expand_path("spring", __dir__)
+      CODE
 
       # The defined? check ensures these lines don't execute when we load the
       # binstub from the application process. Which means that in the application
       # process we'll execute the lines which come after the LOADER block, which
       # is what we want.
-      SPRING = <<'CODE'
-#!/usr/bin/env ruby
-
-# This file loads Spring without using Bundler, in order to be fast.
-# It gets overwritten when you run the `spring binstub` command.
-
-unless defined?(Spring)
-  require 'rubygems'
-  require 'bundler'
-
-  lockfile = Bundler::LockfileParser.new(Bundler.default_lockfile.read)
-  spring = lockfile.specs.detect { |spec| spec.name == 'spring' }
-  if spring
-    Gem.use_paths Gem.dir, Bundler.bundle_path.to_s, *Gem.path
-    gem 'spring', spring.version
-    require 'spring/binstub'
-  end
-end
-CODE
+      SPRING = <<~CODE
+        #!/usr/bin/env ruby
+
+        # This file loads Spring without using loading other gems in the Gemfile, in order to be fast.
+        # It gets overwritten when you run the `spring binstub` command.
+
+        if !defined?(Spring) && [nil, "development", "test"].include?(ENV["RAILS_ENV"])
+          require "bundler"
+
+          Bundler.locked_gems.specs.find { |spec| spec.name == "spring" }&.tap do |spring|
+            Gem.use_paths Gem.dir, Bundler.bundle_path.to_s, *Gem.path
+            gem "spring", spring.version
+            require "spring/binstub"
+          end
+        end
+      CODE
 
       OLD_BINSTUB = %{if !Process.respond_to?(:fork) || Gem::Specification.find_all_by_name("spring").empty?}
 
       BINSTUB_VARIATIONS = Regexp.union [
+        %{load File.expand_path("spring", __dir__)\n},
+        %{begin\n  load File.expand_path('../spring', __FILE__)\nrescue LoadError => e\n  raise unless e.message.include?('spring')\nend\n},
         %{begin\n  load File.expand_path('../spring', __FILE__)\nrescue LoadError\nend\n},
         %{begin\n  spring_bin_path = File.expand_path('../spring', __FILE__)\n  load spring_bin_path\nrescue LoadError => e\n  raise unless e.message.end_with? spring_bin_path, 'spring/binstub'\nend\n},
         LOADER

test/support/acceptance_test.rb

@@ -281,7 +281,7 @@ def exec_name
       test "binstub when spring binary is missing" do
         begin
           File.rename(app.path("bin/spring"), app.path("bin/spring.bak"))
-          assert_success "bin/rake -T", stdout: "rake db:migrate"
+          assert_failure "bin/rake -T", stderr: "`load': cannot load such file"
         ensure
           File.rename(app.path("bin/spring.bak"), app.path("bin/spring"))
         end
@@ -407,6 +407,65 @@ def exec_name
 
         assert_success "bin/spring binstub rake", stdout: "bin/rake: upgraded"
         assert_equal expected, app.path("bin/rake").read
+
+        # newer variation which checks end of exception message using include
+        File.write(app.path("bin/rake"), <<-RUBY.strip_heredoc)
+          #!/usr/bin/env ruby
+          begin
+            load File.expand_path('../spring', __FILE__)
+          rescue LoadError => e
+            raise unless e.message.include?('spring')
+          end
+          require 'bundler/setup'
+          load Gem.bin_path('rake', 'rake')
+        RUBY
+
+        assert_success "bin/spring binstub rake", stdout: "bin/rake: upgraded"
+        assert_equal expected, app.path("bin/rake").read
+      end
+
+      test "binstub remove with new binstub variations which checks end of the exception message using include" do
+        # newer variation which checks end of exception message using include
+        File.write(app.path("bin/rake"), <<-RUBY.strip_heredoc)
+          #!/usr/bin/env ruby
+          begin
+            load File.expand_path('../spring', __FILE__)
+          rescue LoadError => e
+            raise unless e.message.include?('spring')
+          end
+          require 'bundler/setup'
+          load Gem.bin_path('rake', 'rake')
+        RUBY
+
+        File.write(app.path("bin/rails"), <<-RUBY.strip_heredoc)
+          #!/usr/bin/env ruby
+          begin
+            load File.expand_path('../spring', __FILE__)
+          rescue LoadError => e
+            raise unless e.message.include?('spring')
+          end
+          APP_PATH = File.expand_path('../../config/application',  __FILE__)
+          require_relative '../config/boot'
+          require 'rails/commands'
+        RUBY
+
+        assert_success "bin/spring binstub --remove rake", stdout: "bin/rake: Spring removed"
+        assert_success "bin/spring binstub --remove rails", stdout: "bin/rails: Spring removed"
+
+        expected = <<-RUBY.strip_heredoc
+          #!/usr/bin/env ruby
+          require 'bundler/setup'
+          load Gem.bin_path('rake', 'rake')
+        RUBY
+        assert_equal expected, app.path("bin/rake").read
+
+        expected = <<-RUBY.strip_heredoc
+          #!/usr/bin/env ruby
+          APP_PATH = File.expand_path('../../config/application',  __FILE__)
+          require_relative '../config/boot'
+          require 'rails/commands'
+        RUBY
+        assert_equal expected, app.path("bin/rails").read
       end
 
       test "binstub remove with new binstub variations" do