Merge pull request #6930 from rubygems/segiddins/remove-chdir

Remove usage of Dir.chdir that only execute a subprocess
rubygems · Sep 21, 2023 · 10b2b5a · 10b2b5a
2 parents 1549d5e + ad5abd6
commit 10b2b5a
Show file tree

Hide file tree

Showing 5 changed files with 13 additions and 16 deletions.
diff --git a/bundler/lib/bundler/build_metadata.rb b/bundler/lib/bundler/build_metadata.rb
@@ -29,7 +29,7 @@ def self.git_commit_sha
       # commit instance variable then we can't determine its commits SHA.
       git_dir = File.expand_path("../../../.git", __dir__)
       if File.directory?(git_dir)
-        return @git_commit_sha = Dir.chdir(git_dir) { `git rev-parse --short HEAD`.strip.freeze }
+        return @git_commit_sha = IO.popen(%w[git rev-parse --short HEAD], { :chdir => git_dir }, &:read).strip.freeze
       end
 
       @git_commit_sha ||= "unknown"

diff --git a/bundler/lib/bundler/cli/gem.rb b/bundler/lib/bundler/cli/gem.rb
@@ -233,9 +233,7 @@ def run
       end
 
       if use_git
-        Dir.chdir(target) do
-          `git add .`
-        end
+        IO.popen(%w[git add .], { :chdir => target }, &:read)
       end
 
       # Open gemspec in editor

diff --git a/bundler/lib/bundler/cli/open.rb b/bundler/lib/bundler/cli/open.rb
@@ -18,13 +18,11 @@ def run
         Bundler.ui.info "Unable to open #{name} because it's a default gem, so the directory it would normally be installed to does not exist."
       else
         root_path = spec.full_gem_path
-        Dir.chdir(root_path) do
-          require "shellwords"
-          command = Shellwords.split(editor) << File.join([root_path, path].compact)
-          Bundler.with_original_env do
-            system(*command)
-          end || Bundler.ui.info("Could not run '#{command.join(" ")}'")
-        end
+        require "shellwords"
+        command = Shellwords.split(editor) << File.join([root_path, path].compact)
+        Bundler.with_original_env do
+          system(*command, { :chdir => root_path })
+        end || Bundler.ui.info("Could not run '#{command.join(" ")}'")
       end
     end
   end

diff --git a/lib/rubygems/commands/open_command.rb b/lib/rubygems/commands/open_command.rb
@@ -70,9 +70,7 @@ def open_gem(name)
   end
 
   def open_editor(path)
-    Dir.chdir(path) do
-      system(*@editor.split(/\s+/) + [path])
-    end
+    system(*@editor.split(/\s+/) + [path], { :chdir => path })
   end
 
   def spec_for(name)

diff --git a/test/rubygems/test_gem_commands_open_command.rb b/test/rubygems/test_gem_commands_open_command.rb
@@ -22,20 +22,23 @@ def gem(name, version = "1.0")
 
   def test_execute
     @cmd.options[:args] = %w[foo]
-    @cmd.options[:editor] = "#{ruby_with_rubygems_in_load_path} -eexit --"
+    @cmd.options[:editor] = (ruby_with_rubygems_in_load_path + ["-e", "puts(ARGV,Dir.pwd)", "--"]).join(" ")
 
     gem "foo", "1.0.0"
     spec = gem "foo", "1.0.1"
 
     assert_nothing_raised Gem::MockGemUi::TermError do
-      Dir.stub(:chdir, spec.full_gem_path) do
+      stdout, stderr = capture_subprocess_io do
         use_ui @ui do
           @cmd.execute
         end
       end
+      assert_equal [spec.full_gem_path, spec.full_gem_path], stdout.split("\n")
+      assert_equal "", stderr
     end
 
     assert_equal "", @ui.error
+    assert_equal "", @ui.output
   end
 
   def test_wrong_version