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

Originally, it hard-coded pid file path. It can not be removed when customizing
pid file path.
But rake task can not get pid file path. Therefore, do not remove file in rake
task, makes it possible to judge whether it is restart from the argument of the
command and removes the file in server command.

Fixes #29306

7 files changed, 44 insertions, 17 deletions

diff --git a/railties/lib/rails/commands/server/server_command.rb b/railties/lib/rails/commands/server/server_command.rb
index 88e5698714..785265d766 100644
--- a/railties/lib/rails/commands/server/server_command.rb
+++ b/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
diff --git a/railties/lib/rails/dev_caching.rb b/railties/lib/rails/dev_caching.rb
index efb523de4c..ff629b2527 100644
--- a/railties/lib/rails/dev_caching.rb
+++ b/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)
diff --git a/railties/lib/rails/tasks/restart.rake b/railties/lib/rails/tasks/restart.rake
index 5b6bb8ddd6..074e3e89a1 100644
--- a/railties/lib/rails/tasks/restart.rake
+++ b/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
diff --git a/railties/test/application/rake/dev_test.rb b/railties/test/application/rake/dev_test.rb
index d56f315455..b25593ee1c 100644
--- a/railties/test/application/rake/dev_test.rb
+++ b/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
diff --git a/railties/test/application/rake/restart_test.rb b/railties/test/application/rake/restart_test.rb
index bf06f4c522..ed96dcb6b1 100644
--- a/railties/test/application/rake/restart_test.rb
+++ b/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
diff --git a/railties/test/application/server_test.rb b/railties/test/application/server_test.rb
index 97b3b4f6b3..6db9a3b9e8 100644
--- a/railties/test/application/server_test.rb
+++ b/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
diff --git a/railties/test/commands/server_test.rb b/railties/test/commands/server_test.rb
index eb739db9d1..556c2289e7 100644
--- a/railties/test/commands/server_test.rb
+++ b/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