From 9d87ce34f865fa9d7a24ef9f1f1b0d4dedfd3fbf Mon Sep 17 00:00:00 2001 From: Prathamesh Sonpatki Date: Wed, 30 Mar 2016 10:41:41 +0530 Subject: [PATCH] Fix rails restart issue with Puma - We need to pass the restart command to Puma so that it will use it while restarting the server. - Also made sure that all the options passed by user while starting the server are used in the generated restart command so that they will be used while restarting the server. - Besides that we need to remove the server.pid file for the previous running server because otherwise Rack complains about it's presence. - We don't care if the server.pid file does not exist. We only want to delete it if it exists. - This also requires some changes on Puma side which are being tracked here - https://github.com/puma/puma/pull/936. - Fixes #23910. --- railties/CHANGELOG.md | 5 +++++ railties/lib/rails/commands/server.rb | 7 ++++++- railties/lib/rails/dev_caching.rb | 1 + railties/lib/rails/tasks/restart.rake | 1 + railties/test/application/rake/dev_test.rb | 9 +++++++++ railties/test/application/rake/restart_test.rb | 9 +++++++++ railties/test/commands/server_test.rb | 14 ++++++++++++++ 7 files changed, 45 insertions(+), 1 deletion(-) diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index 7193238e83d1e..52c2e0743cf00 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,3 +1,8 @@ +* Make `rails restart` command work with Puma by passing the restart command + which Puma can use to restart rails server. + + *Prathamesh Sonpatki* + * The application generator writes a new file `config/spring.rb`, which tells Spring to watch additional common files. diff --git a/railties/lib/rails/commands/server.rb b/railties/lib/rails/commands/server.rb index c2434d62e2040..7418dff18b1c7 100644 --- a/railties/lib/rails/commands/server.rb +++ b/railties/lib/rails/commands/server.rb @@ -94,7 +94,8 @@ def default_options environment: (ENV['RAILS_ENV'] || ENV['RACK_ENV'] || "development").dup, daemonize: false, caching: nil, - pid: Options::DEFAULT_PID_PATH + pid: Options::DEFAULT_PID_PATH, + restart_cmd: restart_command }) end @@ -130,5 +131,9 @@ def log_to_stdout Rails.logger.extend(ActiveSupport::Logger.broadcast(console)) end end + + def restart_command + "bin/rails server #{ARGV.join(' ')}" + end end end diff --git a/railties/lib/rails/dev_caching.rb b/railties/lib/rails/dev_caching.rb index 4760010851240..3c20164f0f088 100644 --- a/railties/lib/rails/dev_caching.rb +++ b/railties/lib/rails/dev_caching.rb @@ -15,6 +15,7 @@ def enable_by_file end FileUtils.touch 'tmp/restart.txt' + FileUtils.rm_f('tmp/pids/server.pid') end def enable_by_argument(caching) diff --git a/railties/lib/rails/tasks/restart.rake b/railties/lib/rails/tasks/restart.rake index f36c86d81b56f..7e15bb55a1f58 100644 --- a/railties/lib/rails/tasks/restart.rake +++ b/railties/lib/rails/tasks/restart.rake @@ -2,4 +2,5 @@ desc "Restart app by touching tmp/restart.txt" task :restart do FileUtils.mkdir_p('tmp') FileUtils.touch('tmp/restart.txt') + FileUtils.rm_f('tmp/pids/server.pid') end diff --git a/railties/test/application/rake/dev_test.rb b/railties/test/application/rake/dev_test.rb index 59b46c6e79d4e..deb9bc8dee5f6 100644 --- a/railties/test/application/rake/dev_test.rb +++ b/railties/test/application/rake/dev_test.rb @@ -29,6 +29,15 @@ def teardown assert_match(/Development mode is no longer being cached/, output) end end + + test 'dev:cache removes server.pid also' do + Dir.chdir(app_path) do + FileUtils.mkdir_p("tmp/pids") + FileUtils.touch("tmp/pids/server.pid") + `rake dev:cache` + assert_not File.exist?("tmp/pids/server.pid") + end + end end end end diff --git a/railties/test/application/rake/restart_test.rb b/railties/test/application/rake/restart_test.rb index 4cae199e6ba2b..30f662a9be287 100644 --- a/railties/test/application/rake/restart_test.rb +++ b/railties/test/application/rake/restart_test.rb @@ -34,6 +34,15 @@ def teardown assert File.exist?('tmp/restart.txt') end end + + test 'rake restart removes server.pid also' do + Dir.chdir(app_path) do + FileUtils.mkdir_p("tmp/pids") + FileUtils.touch("tmp/pids/server.pid") + `rake restart` + assert_not File.exist?("tmp/pids/server.pid") + end + end end end end diff --git a/railties/test/commands/server_test.rb b/railties/test/commands/server_test.rb index 964b9a44dee3b..38a1605d1f34c 100644 --- a/railties/test/commands/server_test.rb +++ b/railties/test/commands/server_test.rb @@ -118,4 +118,18 @@ def test_default_options assert_equal old_default_options, server.default_options end end + + def test_restart_command_contains_customized_options + original_args = ARGV.dup + args = ["-p", "4567"] + ARGV.replace args + + options = Rails::Server::Options.new.parse! args + server = Rails::Server.new options + expected = "bin/rails server -p 4567" + + assert_equal expected, server.default_options[:restart_cmd] + ensure + ARGV.replace original_args + end end