Skip to content

Commit 72d10e2

Browse files
Check environment before loading Spring in boot.rb
In #39632, `boot.rb` was changed to load `bin/spring`, with the intention of adding a check to Spring itself that would prevent Spring from running in production environments. However, in a production environment, the Spring gem may not be installed. Furthermore, `bin/spring` may raise an error other than `LoadError` if it has been overwritten by e.g. `bundle binstubs` as part of the deployment process. Therefore, this commit adds the environment check to `boot.rb`. This commit also changes the app generator to generate `bin/spring` directly, instead of delegating to `bundle exec spring binstub`. This addresses an issue with the `--skip-bundle` flag. Previously, `--skip-bundle` caused `bin/spring` to not be generated. Thus the user had to manually run `bundle exec spring binstub` later, though that was not documented nor explained. Now, `bin/spring` is always generated. Additionally, by guaranteeing that `bin/stub` is generated, we avoid the need for `rescue LoadError` in `boot.rb`.
1 parent b653778 commit 72d10e2

File tree

5 files changed

+23
-31
lines changed

5 files changed

+23
-31
lines changed

railties/lib/rails/generators/app_base.rb

-6
Original file line numberDiff line numberDiff line change
@@ -441,12 +441,6 @@ def generate_bundler_binstub
441441
end
442442
end
443443

444-
def generate_spring_binstub
445-
if bundle_install? && spring_install?
446-
bundle_command("exec spring binstub")
447-
end
448-
end
449-
450444
def empty_directory_with_keep_file(destination, config = {})
451445
empty_directory(destination, config)
452446
keep_file(destination)

railties/lib/rails/generators/rails/app/app_generator.rb

+4-9
Original file line numberDiff line numberDiff line change
@@ -94,14 +94,13 @@ def bin
9494
"#{shebang}\n" + content
9595
end
9696
chmod "bin", 0755 & ~File.umask, verbose: false
97+
98+
remove_file "bin/spring" unless spring_install?
99+
remove_file "bin/yarn" if options[:skip_javascript]
97100
end
98101

99102
def bin_when_updating
100103
bin
101-
102-
if options[:skip_javascript]
103-
remove_file "bin/yarn"
104-
end
105104
end
106105

107106
def yarn_when_updating
@@ -543,16 +542,12 @@ def delete_new_framework_defaults
543542
end
544543
end
545544

546-
def delete_bin_yarn
547-
remove_file "bin/yarn" if options[:skip_javascript]
548-
end
549-
550545
def finish_template
551546
build(:leftovers)
552547
end
553548

554549
public_task :apply_rails_template, :run_bundle
555-
public_task :generate_bundler_binstub, :generate_spring_binstub
550+
public_task :generate_bundler_binstub
556551
public_task :run_webpack
557552

558553
def run_after_bundle_callbacks
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
# Load Spring without loading other gems in the Gemfile, for speed.
2+
require "bundler"
3+
Bundler.locked_gems.specs.find { |spec| spec.name == "spring" }&.tap do |spring|
4+
Gem.use_paths Gem.dir, Bundler.bundle_path.to_s, *Gem.path
5+
gem "spring", spring.version
6+
require "spring/binstub"
7+
end

railties/lib/rails/generators/rails/app/templates/config/boot.rb.tt

+6-6
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
1-
<% unless options.skip_spring? -%>
2-
begin
1+
# frozen_string_literal: true
2+
3+
ENV['BUNDLE_GEMFILE'] ||= File.expand_path('../Gemfile', __dir__)
4+
<% if spring_install? -%>
5+
6+
if !defined?(Spring) && [nil, "development", "test"].include?(ENV["RAILS_ENV"])
37
load File.expand_path("../bin/spring", __dir__)
4-
rescue LoadError => e
5-
raise unless e.path.end_with?("/bin/spring")
68
end
79
<% end -%>
810

9-
ENV['BUNDLE_GEMFILE'] ||= File.expand_path('../Gemfile', __dir__)
10-
1111
require "bundler/setup" # Set up gems listed in the Gemfile.
1212
<% if depend_on_bootsnap? -%>
1313
require "bootsnap/setup" # Speed up boot time by caching expensive operations.

railties/test/generators/app_generator_test.rb

+6-10
Original file line numberDiff line numberDiff line change
@@ -856,13 +856,17 @@ def test_master_option
856856
end
857857

858858
def test_spring
859+
jruby_skip "spring doesn't run on JRuby"
860+
859861
run_generator
862+
860863
assert_gem "spring"
864+
assert_file "bin/spring", %r{^\s*require "spring/binstub"}
865+
assert_file "config/boot.rb", %r{^\s*load .+\bbin/spring"}
861866
assert_file("config/environments/test.rb") do |contents|
862867
assert_match("config.cache_classes = false", contents)
863868
assert_match("config.action_view.cache_template_loading = true", contents)
864869
end
865-
assert_file "config/boot.rb", %r{^\s*load .+/bin/spring"}
866870
end
867871

868872
def test_bundler_binstub
@@ -871,14 +875,6 @@ def test_bundler_binstub
871875
assert_bundler_command_called("binstubs bundler")
872876
end
873877

874-
def test_spring_binstub
875-
jruby_skip "spring doesn't run on JRuby"
876-
877-
generator([destination_root], skip_webpack_install: true)
878-
879-
assert_bundler_command_called("exec spring binstub")
880-
end
881-
882878
def test_spring_no_fork
883879
respond_to = Process.method(:respond_to?)
884880
respond_to_stub = -> (name) { name != :fork && respond_to[name] }
@@ -898,7 +894,7 @@ def test_skip_spring
898894
assert_match("config.cache_classes = true", contents)
899895
end
900896
assert_file "config/boot.rb" do |contents|
901-
assert_no_match %r{bin/spring}, contents
897+
assert_no_match %r{spring}, contents
902898
end
903899
end
904900

0 commit comments

Comments
 (0)