Skip to content

Commit 5d269d8

Browse files
fxposterfxposter
committed
Remove calls to Dir.chdir
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>
1 parent 8481f8c commit 5d269d8

File tree

5 files changed

+27
-35
lines changed

5 files changed

+27
-35
lines changed

‎lib/git.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,7 @@ def self.export(repository, name, options = {})
244244
options.delete(:remote)
245245
repo = clone(repository, name, {:depth => 1}.merge(options))
246246
repo.checkout("origin/#{options[:branch]}") if options[:branch]
247-
Dir.chdir(repo.dir.to_s) { FileUtils.rm_r '.git' }
247+
FileUtils.rm_r File.join(repo.dir.to_s, '.git')
248248
end
249249

250250
# Same as g.config, but forces it to be at the global level

‎lib/git/base.rb

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,11 +66,13 @@ def self.init(directory = '.', options = {})
6666
def self.root_of_worktree(working_dir)
6767
result = working_dir
6868
status = nil
69-
Dir.chdir(working_dir) do
70-
git_cmd = "#{Git::Base.config.binary_path} -c core.quotePath=true -c color.ui=false rev-parse --show-toplevel 2>&1"
71-
result = `#{git_cmd}`.chomp
72-
status = $?
69+
70+
git_cmd = "#{Git::Base.config.binary_path} -c core.quotePath=true -c color.ui=false rev-parse --show-toplevel 2>&1"
71+
IO.popen(git_cmd, :chdir => working_dir) do |io|
72+
status = Process.wait2(io.pid).last
73+
result = io.read.chomp
7374
end
75+
7476
raise ArgumentError, "'#{working_dir}' is not in a git working tree" unless status.success?
7577
result
7678
end

‎lib/git/lib.rb

Lines changed: 16 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -441,7 +441,7 @@ def worktree_prune
441441
def list_files(ref_dir)
442442
dir = File.join(@git_dir, 'refs', ref_dir)
443443
files = []
444-
Dir.chdir(dir) { files = Dir.glob('**/*').select { |f| File.file?(f) } } rescue nil
444+
files = Dir.glob(File.join(dir, '**/*')).select { |f| File.file?(f) } rescue nil
445445
files
446446
end
447447

@@ -579,31 +579,15 @@ def config_remote(name)
579579
end
580580

581581
def config_get(name)
582-
do_get = Proc.new do |path|
583-
command('config', '--get', name)
584-
end
585-
586-
if @git_dir
587-
Dir.chdir(@git_dir, &do_get)
588-
else
589-
do_get.call
590-
end
582+
command('config', '--get', name, chdir: @git_dir)
591583
end
592584

593585
def global_config_get(name)
594586
command('config', '--global', '--get', name)
595587
end
596588

597589
def config_list
598-
build_list = Proc.new do |path|
599-
parse_config_list command_lines('config', '--list')
600-
end
601-
602-
if @git_dir
603-
Dir.chdir(@git_dir, &build_list)
604-
else
605-
build_list.call
606-
end
590+
parse_config_list command_lines('config', '--list', chdir: @git_dir)
607591
end
608592

609593
def global_config_list
@@ -1148,8 +1132,8 @@ def self.warn_if_old_command(lib)
11481132
# @return [<String>] the names of the EVN variables involved in the git commands
11491133
ENV_VARIABLE_NAMES = ['GIT_DIR', 'GIT_WORK_TREE', 'GIT_INDEX_FILE', 'GIT_SSH']
11501134

1151-
def command_lines(cmd, *opts)
1152-
cmd_op = command(cmd, *opts)
1135+
def command_lines(cmd, *opts, chdir: nil)
1136+
cmd_op = command(cmd, *opts, chdir: chdir)
11531137
if cmd_op.encoding.name != "UTF-8"
11541138
op = cmd_op.encode("UTF-8", "binary", :invalid => :replace, :undef => :replace)
11551139
else
@@ -1195,7 +1179,7 @@ def with_custom_env_variables(&block)
11951179
restore_git_system_env_variables()
11961180
end
11971181

1198-
def command(*cmd, redirect: '', chomp: true, &block)
1182+
def command(*cmd, redirect: '', chomp: true, chdir: nil, &block)
11991183
Git::Lib.warn_if_old_command(self)
12001184

12011185
raise 'cmd can not include a nested array' if cmd.any? { |o| o.is_a? Array }
@@ -1220,7 +1204,7 @@ def command(*cmd, redirect: '', chomp: true, &block)
12201204

12211205
with_custom_env_variables do
12221206
command_thread = Thread.new do
1223-
output = run_command(git_cmd, &block)
1207+
output = run_command(git_cmd, chdir, &block)
12241208
status = $?
12251209
end
12261210
command_thread.join
@@ -1303,10 +1287,16 @@ def log_path_options(opts)
13031287
arr_opts
13041288
end
13051289

1306-
def run_command(git_cmd, &block)
1307-
return IO.popen(git_cmd, &block) if block_given?
1290+
def run_command(git_cmd, chdir=nil, &block)
1291+
block ||= Proc.new do |io|
1292+
io.readlines.map { |l| Git::EncodingUtils.normalize_encoding(l) }.join
1293+
end
13081294

1309-
`#{git_cmd}`.lines.map { |l| Git::EncodingUtils.normalize_encoding(l) }.join
1295+
if chdir
1296+
IO.popen(git_cmd, chdir: chdir, &block)
1297+
else
1298+
IO.popen(git_cmd, &block)
1299+
end
13101300
end
13111301

13121302
def escape(s)

‎tests/units/test_commit_with_gpg.rb

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ def test_with_configured_gpg_keyid
1111
Dir.mktmpdir do |dir|
1212
git = Git.init(dir)
1313
actual_cmd = nil
14-
git.lib.define_singleton_method(:run_command) do |git_cmd, &block|
14+
git.lib.define_singleton_method(:run_command) do |git_cmd, chdir, &block|
1515
actual_cmd = git_cmd
1616
`true`
1717
end
@@ -25,7 +25,7 @@ def test_with_specific_gpg_keyid
2525
Dir.mktmpdir do |dir|
2626
git = Git.init(dir)
2727
actual_cmd = nil
28-
git.lib.define_singleton_method(:run_command) do |git_cmd, &block|
28+
git.lib.define_singleton_method(:run_command) do |git_cmd, chdir, &block|
2929
actual_cmd = git_cmd
3030
`true`
3131
end
@@ -39,7 +39,7 @@ def test_disabling_gpg_sign
3939
Dir.mktmpdir do |dir|
4040
git = Git.init(dir)
4141
actual_cmd = nil
42-
git.lib.define_singleton_method(:run_command) do |git_cmd, &block|
42+
git.lib.define_singleton_method(:run_command) do |git_cmd, chdir, &block|
4343
actual_cmd = git_cmd
4444
`true`
4545
end

‎tests/units/test_lib.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ def test_checkout_with_start_point
9191
assert(@lib.reset(nil, hard: true)) # to get around worktree status on windows
9292

9393
actual_cmd = nil
94-
@lib.define_singleton_method(:run_command) do |git_cmd, &block|
94+
@lib.define_singleton_method(:run_command) do |git_cmd, chdir, &block|
9595
actual_cmd = git_cmd
9696
super(git_cmd, &block)
9797
end

0 commit comments

Comments
 (0)