Merge pull request #30332 from y-yagi/fix_29306

Make `restart` and `dev:cache` tasks work when customizing pid file path

railties/lib/rails/commands/server/server_command.rb

@@ -126,6 +126,7 @@ module Rails
         desc: "Specifies the PID file."
       class_option "dev-caching", aliases: "-C", type: :boolean, default: nil,
         desc: "Specifies whether to perform caching in development."
+      class_option "restart", type: :boolean, default: nil, hide: true
 
       def initialize(args = [], local_options = {}, config = {})
         @original_options = local_options
@@ -136,6 +137,7 @@ module Rails
 
       def perform
         set_application_directory!
+        prepare_restart
         Rails::Server.new(server_options).tap do |server|
           # Require application after server sets environment to propagate
           # the --environment option.
@@ -222,7 +224,7 @@ module Rails
         end
 
         def restart_command
-          "bin/rails server #{@server} #{@original_options.join(" ")}"
+          "bin/rails server #{@server} #{@original_options.join(" ")} --restart"
         end
 
         def pid
@@ -232,6 +234,10 @@ module Rails
         def self.banner(*)
           "rails server [puma, thin etc] [options]"
         end
+
+        def prepare_restart
+          FileUtils.rm_f(options[:pid]) if options[:restart]
+        end
     end
   end
 end

railties/lib/rails/dev_caching.rb

@@ -19,7 +19,6 @@ module Rails
         end
 
         FileUtils.touch "tmp/restart.txt"
-        FileUtils.rm_f("tmp/pids/server.pid")
       end
 
       def enable_by_argument(caching)

railties/lib/rails/tasks/restart.rake

@@ -5,6 +5,5 @@ task :restart do
   verbose(false) do
     mkdir_p "tmp"
     touch "tmp/restart.txt"
-    rm_f "tmp/pids/server.pid"
   end
 end

railties/test/application/rake/dev_test.rb

@@ -32,12 +32,16 @@ module ApplicationTests
         end
       end
 
-      test "dev:cache removes server.pid also" do
+      test "dev:cache touches tmp/restart.txt" do
         Dir.chdir(app_path) do
-          FileUtils.mkdir_p("tmp/pids")
-          FileUtils.touch("tmp/pids/server.pid")
           `rails dev:cache`
-          assert_not File.exist?("tmp/pids/server.pid")
+          assert File.exist?("tmp/restart.txt")
+
+          prev_mtime = File.mtime("tmp/restart.txt")
+          sleep(1)
+          `rails dev:cache`
+          curr_mtime = File.mtime("tmp/restart.txt")
+          assert_not_equal prev_mtime, curr_mtime
         end
       end
     end

railties/test/application/rake/restart_test.rb

@@ -35,15 +35,6 @@ module ApplicationTests
           assert File.exist?("tmp/restart.txt")
         end
       end
-
-      test "rails restart removes server.pid also" do
-        Dir.chdir(app_path) do
-          FileUtils.mkdir_p("tmp/pids")
-          FileUtils.touch("tmp/pids/server.pid")
-          `bin/rails restart`
-          assert_not File.exist?("tmp/pids/server.pid")
-        end
-      end
     end
   end
 end

railties/test/application/server_test.rb

@@ -1,12 +1,14 @@
 # frozen_string_literal: true
 
 require "isolation/abstract_unit"
+require "console_helpers"
 require "rails/command"
 require "rails/commands/server/server_command"
 
 module ApplicationTests
   class ServerTest < ActiveSupport::TestCase
     include ActiveSupport::Testing::Isolation
+    include ConsoleHelpers
 
     def setup
       build_app
@@ -29,5 +31,31 @@ module ApplicationTests
       log = File.read(Rails.application.config.paths["log"].first)
       assert_match(/DEPRECATION WARNING: Use `Rails::Application` subclass to start the server is deprecated/, log)
     end
+
+    test "restart rails server with custom pid file path" do
+      skip "PTY unavailable" unless available_pty?
+
+      master, slave = PTY.open
+      pid = nil
+
+      begin
+        pid = Process.spawn("#{app_path}/bin/rails server -P tmp/dummy.pid", in: slave, out: slave, err: slave)
+        assert_output("Listening", master)
+
+        Dir.chdir(app_path) { system("bin/rails restart") }
+
+        assert_output("Restarting", master)
+        assert_output("Inherited", master)
+      ensure
+        kill(pid) if pid
+      end
+    end
+
+    private
+      def kill(pid)
+        Process.kill("TERM", pid)
+        Process.wait(pid)
+      rescue Errno::ESRCH
+      end
   end
 end

railties/test/commands/server_test.rb

@@ -190,7 +190,7 @@ class Rails::ServerTest < ActiveSupport::TestCase
     ARGV.replace args
 
     options = parse_arguments(args)
-    expected = "bin/rails server  -p 4567 -b 127.0.0.1 -c dummy_config.ru -d -e test -P tmp/server.pid -C"
+    expected = "bin/rails server  -p 4567 -b 127.0.0.1 -c dummy_config.ru -d -e test -P tmp/server.pid -C --restart"
 
     assert_equal expected, options[:restart_cmd]
   ensure