DEV: Allow execute_command to receive a block (#8303)

This makes it easy to run multiple commands with the same keyword arguments. The main use is for using `chdir` across multiple commands. The `Dir.chdir` method is not concurrency safe because it switches the working directory of the entire process.

lib/discourse.rb

@@ -23,20 +23,52 @@ module Discourse
   end
 
   class Utils
-    def self.execute_command(*command, failure_message: "", success_status_codes: [0], chdir: ".")
+    # Usage:
-      stdout, stderr, status = Open3.capture3(*command, chdir: chdir)
+    #   Discourse::Utils.execute_command("pwd", chdir: 'mydirectory')
+    # or with a block
+    #   Discourse::Utils.execute_command(chdir: 'mydirectory') do |runner|
+    #     runner.exec("pwd")
+    #   end
+    def self.execute_command(*command, **args)
+      runner = CommandRunner.new(**args)
 
-      if !status.exited? || !success_status_codes.include?(status.exitstatus)
+      if block_given?
-        failure_message = "#{failure_message}\n" if !failure_message.blank?
+        raise RuntimeError.new("Cannot pass command and block to execute_command") if command.present?
-        raise "#{caller[0]}: #{failure_message}#{stderr}"
+        yield runner
+      else
+        runner.exec(*command)
       end
-
-      stdout
     end
 
     def self.pretty_logs(logs)
       logs.join("\n".freeze)
     end
+
+    private
+
+    class CommandRunner
+      def initialize(**init_params)
+        @init_params = init_params
+      end
+
+      def exec(*command, **exec_params)
+        raise RuntimeError.new("Cannot specify same parameters at block and command level") if (@init_params.keys & exec_params.keys).present?
+        execute_command(*command, **@init_params.merge(exec_params))
+      end
+
+      private
+
+      def execute_command(*command, failure_message: "", success_status_codes: [0], chdir: ".")
+        stdout, stderr, status = Open3.capture3(*command, chdir: chdir)
+
+        if !status.exited? || !success_status_codes.include?(status.exitstatus)
+          failure_message = "#{failure_message}\n" if !failure_message.blank?
+          raise "#{caller[0]}: #{failure_message}#{stderr}"
+        end
+
+        stdout
+      end
+    end
   end
 
   # Log an exception.

spec/components/discourse_spec.rb

@@ -391,4 +391,42 @@ describe Discourse do
     end
   end
 
+  describe "Utils.execute_command" do
+    it "works for individual commands" do
+      expect(Discourse::Utils.execute_command("pwd").strip).to eq(Rails.root.to_s)
+      expect(Discourse::Utils.execute_command("pwd", chdir: "plugins").strip).to eq("#{Rails.root.to_s}/plugins")
+    end
+
+    it "works with a block" do
+      Discourse::Utils.execute_command do |runner|
+        expect(runner.exec("pwd").strip).to eq(Rails.root.to_s)
+      end
+
+      result = Discourse::Utils.execute_command(chdir: "plugins") do |runner|
+        expect(runner.exec("pwd").strip).to eq("#{Rails.root.to_s}/plugins")
+        runner.exec("pwd")
+      end
+
+      # Should return output of block
+      expect(result.strip).to eq("#{Rails.root.to_s}/plugins")
+    end
+
+    it "does not leak chdir between threads" do
+      has_done_chdir = false
+      has_checked_chdir = false
+
+      thread = Thread.new do
+        Discourse::Utils.execute_command(chdir: "plugins") do
+          has_done_chdir = true
+          sleep(0.01) until has_checked_chdir
+        end
+      end
+
+      sleep(0.01) until has_done_chdir
+      expect(Discourse::Utils.execute_command("pwd").strip).to eq(Rails.root.to_s)
+      has_checked_chdir = true
+      thread.join
+    end
+  end
+
 end