Merge pull request #38495 from jonathanhefner/isolate-argv-in-command-invoke

Isolate ARGV in Rails::Command.invoke

railties/lib/rails/command.rb

@@ -40,12 +40,19 @@ def invoke(full_namespace, args = [], **config)
         command_name, namespace = "help", "help" if command_name.blank? || HELP_MAPPINGS.include?(command_name)
         command_name, namespace = "version", "version" if %w( -v --version ).include?(command_name)
 
+        # isolate ARGV to ensure that commands depend only on the args they are given
+        args = args.dup # args might *be* ARGV so dup before clearing
+        old_argv = ARGV.dup
+        ARGV.clear
+
         command = find_by_namespace(namespace, command_name)
         if command && command.all_commands[command_name]
           command.perform(command_name, args, config)
         else
           find_by_namespace("rake").perform(full_namespace, args, config)
         end
+      ensure
+        ARGV.replace(old_argv)
       end
 
       # Rails finds namespaces similar to Thor, it only adds one rule:

railties/lib/rails/commands/db/system/change/change_command.rb

@@ -10,8 +10,13 @@ module System
         class ChangeCommand < Base # :nodoc:
           class_option :to, desc: "The database system to switch to."
 
+          def initialize(positional_args, option_args, *)
+            @argv = positional_args + option_args
+            super
+          end
+
           def perform
-            Rails::Generators::Db::System::ChangeGenerator.start
+            Rails::Generators::Db::System::ChangeGenerator.start(@argv)
           end
         end
       end

railties/lib/rails/commands/test/test_command.rb

@@ -29,8 +29,8 @@ def help
       def perform(*)
         $LOAD_PATH << Rails::Command.root.join("test").to_s
 
-        Rails::TestUnit::Runner.parse_options(ARGV)
-        Rails::TestUnit::Runner.run(ARGV)
+        Rails::TestUnit::Runner.parse_options(args)
+        Rails::TestUnit::Runner.run(args)
       end
     end
   end

railties/lib/rails/generators.rb

@@ -124,9 +124,8 @@ def api_only!
           template_engine: nil
         )
 
-        if ARGV.first == "mailer"
-          options[:rails][:template_engine] = :erb
-        end
+        options[:mailer] ||= {}
+        options[:mailer][:template_engine] ||= :erb
       end
 
       # Returns an array of generator namespaces that are hidden.

railties/test/application/generators_test.rb

@@ -172,20 +172,19 @@ def with_bare_config
       assert File.exist?(File.join(rails_root, "app/views/notifier_mailer/foo.html.erb"))
     end
 
-    test "ARGV is mutated as expected" do
+    test "ARGV is populated" do
       require "#{app_path}/config/environment"
-      require "rails/command"
-      Rails::Command.const_set("APP_PATH", "rails/all")
-
-      FileUtils.cd(rails_root) do
-        ARGV = ["mailer", "notifier", "foo"]
-        Rails::Command.const_set("ARGV", ARGV)
-        quietly { Rails::Command.invoke :generate, ARGV }
+      Rails.application.load_generators
 
-        assert_equal ["notifier", "foo"], ARGV
+      class Rails::Generators::CheckArgvGenerator < Rails::Generators::Base
+        def check_expected
+          raise "ARGV.first is not expected" unless ARGV.first == "expected"
+        end
       end
 
-      Rails::Command.send(:remove_const, "APP_PATH")
+      quietly do
+        Rails::Command.invoke(:generate, ["check_argv", "expected"]) # should not raise
+      end
     end
 
     test "help does not show hidden namespaces and hidden commands" do

railties/test/command/base_test.rb

@@ -12,4 +12,22 @@ class Rails::Command::BaseTest < ActiveSupport::TestCase
     assert_equal %w(secrets:setup secrets:edit secrets:show), Rails::Command::SecretsCommand.printing_commands
     assert_equal %w(db:system:change), Rails::Command::Db::System::ChangeCommand.printing_commands
   end
+
+  test "ARGV is isolated" do
+    class Rails::Command::ArgvCommand < Rails::Command::Base
+      def check_isolation
+        raise "not isolated" unless ARGV.empty?
+        ARGV << "isolate this"
+      end
+    end
+
+    old_argv = ARGV.dup
+    new_argv = ["foo", "bar"]
+    ARGV.replace(new_argv)
+
+    Rails::Command.invoke("argv:check_isolation") # should not raise
+    assert_equal new_argv, ARGV
+  ensure
+    ARGV.replace(old_argv)
+  end
 end