Remove calls to Dir.chdir

fxposter · jcouball · commit 65c0cc6ccd78 · 2023-12-26T15:26:34.000-08:00
In multithreaded environment Dir.chdir changes current directory of all process' threads, so other threads might
misbehave.

Base#chdir, Base#with_working and Base#with_temp_working were left intact, cause they are not used internally, so it's
an explicit user decision to use them.

Signed-off-by: Pavel Forkert &lt;fxposter@gmail.com&gt;
diff --git a/lib/git.rb b/lib/git.rb
@@ -244,7 +244,7 @@ def self.export(repository, name, options = {})
     options.delete(:remote)
     repo = clone(repository, name, {:depth => 1}.merge(options))
     repo.checkout("origin/#{options[:branch]}") if options[:branch]
-    Dir.chdir(repo.dir.to_s) { FileUtils.rm_r '.git' }
+    FileUtils.rm_r File.join(repo.dir.to_s, '.git')
   end
 
   # Same as g.config, but forces it to be at the global level
diff --git a/lib/git/base.rb b/lib/git/base.rb
@@ -1,5 +1,6 @@
 require 'git/base/factory'
 require 'logger'
+require 'open3'
 
 module Git
   # Git::Base is the main public interface for interacting with Git commands.
@@ -66,11 +67,11 @@ def self.init(directory = '.', options = {})
     def self.root_of_worktree(working_dir)
       result = working_dir
       status = nil
-      Dir.chdir(working_dir) do
-        git_cmd = "#{Git::Base.config.binary_path} -c core.quotePath=true -c color.ui=false rev-parse --show-toplevel 2>&1"
-        result = `#{git_cmd}`.chomp
-        status = $?
-      end
+
+      git_cmd = "#{Git::Base.config.binary_path} -c core.quotePath=true -c color.ui=false rev-parse --show-toplevel 2>&1"
+      result, status = Open3.capture2(git_cmd, chdir: File.expand_path(working_dir))
+      result = result.chomp
+
       raise ArgumentError, "'#{working_dir}' is not in a git working tree" unless status.success?
       result
     end
diff --git a/lib/git/lib.rb b/lib/git/lib.rb
@@ -2,6 +2,7 @@
 require 'logger'
 require 'tempfile'
 require 'zlib'
+require 'open3'
 
 module Git
   class Lib
@@ -441,7 +442,10 @@ def worktree_prune
     def list_files(ref_dir)
       dir = File.join(@git_dir, 'refs', ref_dir)
       files = []
-      Dir.chdir(dir) { files = Dir.glob('**/*').select { |f| File.file?(f) } } rescue nil
+      begin
+        files = Dir.glob('**/*', base: dir).select { |f| File.file?(File.join(dir, f)) }
+      rescue
+      end
       files
     end
 
@@ -579,31 +583,15 @@ def config_remote(name)
     end
 
     def config_get(name)
-      do_get = Proc.new do |path|
-        command('config', '--get', name)
-      end
-
-      if @git_dir
-        Dir.chdir(@git_dir, &do_get)
-      else
-        do_get.call
-      end
+      command('config', '--get', name, chdir: @git_dir)
     end
 
     def global_config_get(name)
       command('config', '--global', '--get', name)
     end
 
     def config_list
-      build_list = Proc.new do |path|
-        parse_config_list command_lines('config', '--list')
-      end
-
-      if @git_dir
-        Dir.chdir(@git_dir, &build_list)
-      else
-        build_list.call
-      end
+      parse_config_list command_lines('config', '--list', chdir: @git_dir)
     end
 
     def global_config_list
@@ -1148,8 +1136,8 @@ def self.warn_if_old_command(lib)
     # @return [<String>] the names of the EVN variables involved in the git commands
     ENV_VARIABLE_NAMES = ['GIT_DIR', 'GIT_WORK_TREE', 'GIT_INDEX_FILE', 'GIT_SSH']
 
-    def command_lines(cmd, *opts)
-      cmd_op = command(cmd, *opts)
+    def command_lines(cmd, *opts, chdir: nil)
+      cmd_op = command(cmd, *opts, chdir: chdir)
       if cmd_op.encoding.name != "UTF-8"
         op = cmd_op.encode("UTF-8", "binary", :invalid => :replace, :undef => :replace)
       else
@@ -1195,7 +1183,7 @@ def with_custom_env_variables(&block)
       restore_git_system_env_variables()
     end
 
-    def command(*cmd, redirect: '', chomp: true, &block)
+    def command(*cmd, redirect: '', chomp: true, chdir: nil, &block)
       Git::Lib.warn_if_old_command(self)
 
       raise 'cmd can not include a nested array' if cmd.any? { |o| o.is_a? Array }
@@ -1220,8 +1208,7 @@ def command(*cmd, redirect: '', chomp: true, &block)
 
       with_custom_env_variables do
         command_thread = Thread.new do
-          output = run_command(git_cmd, &block)
-          status = $?
+          output, status = run_command(git_cmd, chdir, &block)
         end
         command_thread.join
       end
@@ -1303,10 +1290,17 @@ def log_path_options(opts)
       arr_opts
     end
 
-    def run_command(git_cmd, &block)
-      return IO.popen(git_cmd, &block) if block_given?
+    def run_command(git_cmd, chdir=nil, &block)
+      block ||= Proc.new do |io|
+        io.readlines.map { |l| Git::EncodingUtils.normalize_encoding(l) }.join
+      end
+
+      opts = {}
+      opts[:chdir] = File.expand_path(chdir) if chdir
 
-      `#{git_cmd}`.lines.map { |l| Git::EncodingUtils.normalize_encoding(l) }.join
+      Open3.popen2(git_cmd, opts) do |stdin, stdout, wait_thr|
+        [block.call(stdout), wait_thr.value]
+      end
     end
 
     def escape(s)
diff --git a/lib/git/status.rb b/lib/git/status.rb
@@ -172,13 +172,12 @@ def construct_status
     def fetch_untracked
       ignore = @base.lib.ignored_files
 
-      Dir.chdir(@base.dir.path) do
-        Dir.glob('**/*', File::FNM_DOTMATCH) do |file|
-          next if @files[file] || File.directory?(file) ||
-                  ignore.include?(file) || file =~ %r{^.git\/.+}
+      root_dir = @base.dir.path
+      Dir.glob('**/*', File::FNM_DOTMATCH, base: root_dir) do |file|
+        next if @files[file] || File.directory?(File.join(root_dir, file)) ||
+                ignore.include?(file) || file =~ %r{^.git\/.+}
 
-          @files[file] = { path: file, untracked: true }
-        end
+        @files[file] = { path: file, untracked: true }
       end
     end
 
diff --git a/tests/units/test_commit_with_gpg.rb b/tests/units/test_commit_with_gpg.rb
@@ -11,9 +11,9 @@ def test_with_configured_gpg_keyid
     Dir.mktmpdir do |dir|
       git = Git.init(dir)
       actual_cmd = nil
-      git.lib.define_singleton_method(:run_command) do |git_cmd, &block|
+      git.lib.define_singleton_method(:run_command) do |git_cmd, chdir, &block|
         actual_cmd = git_cmd
-        `true`
+        [`true`, $?]
       end
       message = 'My commit message'
       git.commit(message, gpg_sign: true)
@@ -25,9 +25,9 @@ def test_with_specific_gpg_keyid
     Dir.mktmpdir do |dir|
       git = Git.init(dir)
       actual_cmd = nil
-      git.lib.define_singleton_method(:run_command) do |git_cmd, &block|
+      git.lib.define_singleton_method(:run_command) do |git_cmd, chdir, &block|
         actual_cmd = git_cmd
-        `true`
+        [`true`, $?]
       end
       message = 'My commit message'
       git.commit(message, gpg_sign: 'keykeykey')
@@ -39,9 +39,9 @@ def test_disabling_gpg_sign
     Dir.mktmpdir do |dir|
       git = Git.init(dir)
       actual_cmd = nil
-      git.lib.define_singleton_method(:run_command) do |git_cmd, &block|
+      git.lib.define_singleton_method(:run_command) do |git_cmd, chdir, &block|
         actual_cmd = git_cmd
-        `true`
+        [`true`, $?]
       end
       message = 'My commit message'
       git.commit(message, no_gpg_sign: true)
diff --git a/tests/units/test_lib.rb b/tests/units/test_lib.rb
@@ -91,7 +91,7 @@ def test_checkout_with_start_point
     assert(@lib.reset(nil, hard: true)) # to get around worktree status on windows
 
     actual_cmd = nil
-    @lib.define_singleton_method(:run_command) do |git_cmd, &block|
+    @lib.define_singleton_method(:run_command) do |git_cmd, chdir, &block|
       actual_cmd = git_cmd
       super(git_cmd, &block)
     end
