Skip to content

Commit

Permalink
Remove calls to Dir.chdir (#673)
Browse files Browse the repository at this point in the history
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 <fxposter@gmail.com>
  • Loading branch information
fxposter authored Dec 26, 2023
1 parent e64c2f6 commit b0d89ac
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 46 deletions.
2 changes: 1 addition & 1 deletion lib/git.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 6 additions & 5 deletions lib/git/base.rb
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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
Expand Down
48 changes: 21 additions & 27 deletions lib/git/lib.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
require 'logger'
require 'tempfile'
require 'zlib'
require 'open3'

module Git
class Lib
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 }
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down
11 changes: 5 additions & 6 deletions lib/git/status.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
12 changes: 6 additions & 6 deletions tests/units/test_commit_with_gpg.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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')
Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion tests/units/test_lib.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit b0d89ac

Please sign in to comment.