Skip to content

Commit b0d89ac

Browse files
fxposterfxposter
authored
Remove calls to Dir.chdir (#673)
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 e64c2f6 commit b0d89ac

File tree

6 files changed

+40
-46
lines changed

6 files changed

+40
-46
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 & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
require 'git/base/factory'
22
require 'logger'
3+
require 'open3'
34

45
module Git
56
# Git::Base is the main public interface for interacting with Git commands.
@@ -66,11 +67,11 @@ def self.init(directory = '.', options = {})
6667
def self.root_of_worktree(working_dir)
6768
result = working_dir
6869
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 = $?
73-
end
70+
71+
git_cmd = "#{Git::Base.config.binary_path} -c core.quotePath=true -c color.ui=false rev-parse --show-toplevel 2>&1"
72+
result, status = Open3.capture2(git_cmd, chdir: File.expand_path(working_dir))
73+
result = result.chomp
74+
7475
raise ArgumentError, "'#{working_dir}' is not in a git working tree" unless status.success?
7576
result
7677
end

‎lib/git/lib.rb

Lines changed: 21 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
require 'logger'
33
require 'tempfile'
44
require 'zlib'
5+
require 'open3'
56

67
module Git
78
class Lib
@@ -441,7 +442,10 @@ def worktree_prune
441442
def list_files(ref_dir)
442443
dir = File.join(@git_dir, 'refs', ref_dir)
443444
files = []
444-
Dir.chdir(dir) { files = Dir.glob('**/*').select { |f| File.file?(f) } } rescue nil
445+
begin
446+
files = Dir.glob('**/*', base: dir).select { |f| File.file?(File.join(dir, f)) }
447+
rescue
448+
end
445449
files
446450
end
447451

@@ -579,31 +583,15 @@ def config_remote(name)
579583
end
580584

581585
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
586+
command('config', '--get', name, chdir: @git_dir)
591587
end
592588

593589
def global_config_get(name)
594590
command('config', '--global', '--get', name)
595591
end
596592

597593
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
594+
parse_config_list command_lines('config', '--list', chdir: @git_dir)
607595
end
608596

609597
def global_config_list
@@ -1148,8 +1136,8 @@ def self.warn_if_old_command(lib)
11481136
# @return [<String>] the names of the EVN variables involved in the git commands
11491137
ENV_VARIABLE_NAMES = ['GIT_DIR', 'GIT_WORK_TREE', 'GIT_INDEX_FILE', 'GIT_SSH']
11501138

1151-
def command_lines(cmd, *opts)
1152-
cmd_op = command(cmd, *opts)
1139+
def command_lines(cmd, *opts, chdir: nil)
1140+
cmd_op = command(cmd, *opts, chdir: chdir)
11531141
if cmd_op.encoding.name != "UTF-8"
11541142
op = cmd_op.encode("UTF-8", "binary", :invalid => :replace, :undef => :replace)
11551143
else
@@ -1195,7 +1183,7 @@ def with_custom_env_variables(&block)
11951183
restore_git_system_env_variables()
11961184
end
11971185

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

12011189
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)
12201208

12211209
with_custom_env_variables do
12221210
command_thread = Thread.new do
1223-
output = run_command(git_cmd, &block)
1224-
status = $?
1211+
output, status = run_command(git_cmd, chdir, &block)
12251212
end
12261213
command_thread.join
12271214
end
@@ -1303,10 +1290,17 @@ def log_path_options(opts)
13031290
arr_opts
13041291
end
13051292

1306-
def run_command(git_cmd, &block)
1307-
return IO.popen(git_cmd, &block) if block_given?
1293+
def run_command(git_cmd, chdir=nil, &block)
1294+
block ||= Proc.new do |io|
1295+
io.readlines.map { |l| Git::EncodingUtils.normalize_encoding(l) }.join
1296+
end
1297+
1298+
opts = {}
1299+
opts[:chdir] = File.expand_path(chdir) if chdir
13081300

1309-
`#{git_cmd}`.lines.map { |l| Git::EncodingUtils.normalize_encoding(l) }.join
1301+
Open3.popen2(git_cmd, opts) do |stdin, stdout, wait_thr|
1302+
[block.call(stdout), wait_thr.value]
1303+
end
13101304
end
13111305

13121306
def escape(s)

‎lib/git/status.rb

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -172,13 +172,12 @@ def construct_status
172172
def fetch_untracked
173173
ignore = @base.lib.ignored_files
174174

175-
Dir.chdir(@base.dir.path) do
176-
Dir.glob('**/*', File::FNM_DOTMATCH) do |file|
177-
next if @files[file] || File.directory?(file) ||
178-
ignore.include?(file) || file =~ %r{^.git\/.+}
175+
root_dir = @base.dir.path
176+
Dir.glob('**/*', File::FNM_DOTMATCH, base: root_dir) do |file|
177+
next if @files[file] || File.directory?(File.join(root_dir, file)) ||
178+
ignore.include?(file) || file =~ %r{^.git\/.+}
179179

180-
@files[file] = { path: file, untracked: true }
181-
end
180+
@files[file] = { path: file, untracked: true }
182181
end
183182
end
184183

‎tests/units/test_commit_with_gpg.rb

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,9 @@ 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
16-
`true`
16+
[`true`, $?]
1717
end
1818
message = 'My commit message'
1919
git.commit(message, gpg_sign: true)
@@ -25,9 +25,9 @@ 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
30-
`true`
30+
[`true`, $?]
3131
end
3232
message = 'My commit message'
3333
git.commit(message, gpg_sign: 'keykeykey')
@@ -39,9 +39,9 @@ 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
44-
`true`
44+
[`true`, $?]
4545
end
4646
message = 'My commit message'
4747
git.commit(message, no_gpg_sign: true)

‎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)