Skip to content

Commit 9d57386

Browse files
jcouballjcouball
committed
Use Open3.capture2e instead of backticks for subprocesses
Signed-off-by: James Couball <jcouball@yahoo.com>
1 parent ff6dcf4 commit 9d57386

File tree

5 files changed

+98
-154
lines changed

5 files changed

+98
-154
lines changed

‎lib/git/lib.rb

Lines changed: 44 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
11
require 'tempfile'
22
require 'zlib'
3+
require 'open3'
34

45
module Git
56

67
class GitExecuteError < StandardError
78
end
89

910
class Lib
10-
11-
@@semaphore = Mutex.new
12-
1311
# The path to the Git working copy. The default is '"./.git"'.
1412
#
1513
# @return [Pathname] the path to the Git working copy.
@@ -785,7 +783,7 @@ def merge(branch, message = nil, opts = {})
785783
arr_opts << '--no-commit' if opts[:no_commit]
786784
arr_opts << '--no-ff' if opts[:no_ff]
787785
arr_opts << '-m' << message if message
788-
arr_opts += [branch]
786+
arr_opts += Array(branch)
789787
command('merge', arr_opts)
790788
end
791789

@@ -1039,11 +1037,6 @@ def self.warn_if_old_command(lib)
10391037

10401038
private
10411039

1042-
# Systen ENV variables involved in the git commands.
1043-
#
1044-
# @return [<String>] the names of the EVN variables involved in the git commands
1045-
ENV_VARIABLE_NAMES = ['GIT_DIR', 'GIT_WORK_TREE', 'GIT_INDEX_FILE', 'GIT_SSH']
1046-
10471040
def command_lines(cmd, *opts)
10481041
cmd_op = command(cmd, *opts)
10491042
if cmd_op.encoding.name != "UTF-8"
@@ -1054,76 +1047,62 @@ def command_lines(cmd, *opts)
10541047
op.split("\n")
10551048
end
10561049

1057-
# Takes the current git's system ENV variables and store them.
1058-
def store_git_system_env_variables
1059-
@git_system_env_variables = {}
1060-
ENV_VARIABLE_NAMES.each do |env_variable_name|
1061-
@git_system_env_variables[env_variable_name] = ENV[env_variable_name]
1050+
def env
1051+
ENV.to_h.tap do |env|
1052+
env['GIT_DIR'] = @git_dir
1053+
env['GIT_WORK_TREE'] = @git_work_dir
1054+
env['GIT_INDEX_FILE'] = @git_index_file
1055+
env['GIT_SSH'] = Git::Base.config.git_ssh
10621056
end
10631057
end
10641058

1065-
# Takes the previously stored git's ENV variables and set them again on ENV.
1066-
def restore_git_system_env_variables
1067-
ENV_VARIABLE_NAMES.each do |env_variable_name|
1068-
ENV[env_variable_name] = @git_system_env_variables[env_variable_name]
1069-
end
1070-
end
1059+
DEFAULT_COMMAND_OPTS = {
1060+
chomp: true,
1061+
redirect: ''
1062+
}
10711063

1072-
# Sets git's ENV variables to the custom values for the current instance.
1073-
def set_custom_git_env_variables
1074-
ENV['GIT_DIR'] = @git_dir
1075-
ENV['GIT_WORK_TREE'] = @git_work_dir
1076-
ENV['GIT_INDEX_FILE'] = @git_index_file
1077-
ENV['GIT_SSH'] = Git::Base.config.git_ssh
1064+
def command_opts(given_command_opts)
1065+
DEFAULT_COMMAND_OPTS.merge(given_command_opts).tap do |command_options|
1066+
command_options.keys.each do |k|
1067+
raise ArgumentError.new("Unsupported command option: #{k}") unless DEFAULT_COMMAND_OPTS.keys.include?(k)
1068+
end
1069+
end
10781070
end
10791071

1080-
# Runs a block inside an environment with customized ENV variables.
1081-
# It restores the ENV after execution.
1082-
#
1083-
# @param [Proc] block block to be executed within the customized environment
1084-
def with_custom_env_variables(&block)
1085-
@@semaphore.synchronize do
1086-
store_git_system_env_variables()
1087-
set_custom_git_env_variables()
1088-
return block.call()
1072+
def global_opts
1073+
Array.new.tap do |global_opts|
1074+
global_opts << "--git-dir=#{@git_dir}" if !@git_dir.nil?
1075+
global_opts << "--work-tree=#{@git_work_dir}" if !@git_work_dir.nil?
1076+
global_opts << %w[-c core.quotePath=true]
1077+
global_opts << %w[-c color.ui=false]
10891078
end
1090-
ensure
1091-
restore_git_system_env_variables()
10921079
end
10931080

10941081
def command(cmd, *opts, &block)
1095-
command_opts = { chomp: true, redirect: '' }
1096-
if opts.last.is_a?(Hash)
1097-
command_opts.merge!(opts.pop)
1098-
end
1099-
command_opts.keys.each do |k|
1100-
raise ArgumentError.new("Unsupported option: #{k}") unless [:chomp, :redirect].include?(k)
1101-
end
1102-
1103-
global_opts = []
1104-
global_opts << "--git-dir=#{@git_dir}" if !@git_dir.nil?
1105-
global_opts << "--work-tree=#{@git_work_dir}" if !@git_work_dir.nil?
1106-
global_opts << %w[-c core.quotePath=true]
1107-
global_opts << %w[-c color.ui=false]
1108-
1109-
opts = [opts].flatten.map {|s| escape(s) }.join(' ')
1110-
1111-
global_opts = global_opts.flatten.map {|s| escape(s) }.join(' ')
1082+
given_command_opts = opts.last.is_a?(Hash) ? opts.pop : {}
1083+
command_opts = command_opts(given_command_opts)
11121084

1113-
git_cmd = "#{Git::Base.config.binary_path} #{global_opts} #{cmd} #{opts} #{command_opts[:redirect]} 2>&1"
1114-
1115-
output = nil
1116-
1117-
command_thread = nil;
1085+
git_cmd = [
1086+
Git::Base.config.binary_path,
1087+
flatten_and_escape(global_opts),
1088+
cmd,
1089+
flatten_and_escape(opts),
1090+
command_opts[:redirect]
1091+
].join(' ')
11181092

11191093
exitstatus = nil
11201094

1121-
with_custom_env_variables do
1122-
command_thread = Thread.new do
1123-
output = run_command(git_cmd, &block)
1095+
output, exitstatus = begin
1096+
if block_given?
1097+
output = IO.popen(env, git_cmd, &block)
11241098
exitstatus = $?.exitstatus
1099+
[output, exitstatus]
1100+
else
1101+
encoded_output, status = Open3.capture2e(env, git_cmd)
1102+
output = encoded_output.lines.map { |l| Git::EncodingUtils.normalize_encoding(l) }.join
1103+
exitstatus = status.exitstatus
1104+
[output, exitstatus]
11251105
end
1126-
command_thread.join
11271106
end
11281107

11291108
if @logger
@@ -1195,10 +1174,8 @@ def log_path_options(opts)
11951174
arr_opts
11961175
end
11971176

1198-
def run_command(git_cmd, &block)
1199-
return IO.popen(git_cmd, &block) if block_given?
1200-
1201-
`#{git_cmd}`.lines.map { |l| Git::EncodingUtils.normalize_encoding(l) }.join
1177+
def flatten_and_escape(string_or_array)
1178+
Array(string_or_array).flatten.map { |s| escape(s) }.join(' ')
12021179
end
12031180

12041181
def escape(s)

‎tests/test_helper.rb

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -83,21 +83,6 @@ def append_file(name, contents)
8383
end
8484
end
8585

86-
# Runs a block inside an environment with customized ENV variables.
87-
# It restores the ENV after execution.
88-
#
89-
# @param [Proc] block block to be executed within the customized environment
90-
#
91-
def with_custom_env_variables(&block)
92-
saved_env = {}
93-
begin
94-
Git::Lib::ENV_VARIABLE_NAMES.each { |k| saved_env[k] = ENV[k] }
95-
return block.call
96-
ensure
97-
Git::Lib::ENV_VARIABLE_NAMES.each { |k| ENV[k] = saved_env[k] }
98-
end
99-
end
100-
10186
# Assert that the expected command line args are generated for a given Git::Lib method
10287
#
10388
# This assertion generates an empty git repository and then runs calls

‎tests/units/test_commit_with_gpg.rb

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -10,42 +10,42 @@ def setup
1010
def test_with_configured_gpg_keyid
1111
Dir.mktmpdir do |dir|
1212
git = Git.init(dir)
13-
actual_cmd = nil
14-
git.lib.define_singleton_method(:run_command) do |git_cmd, &block|
15-
actual_cmd = git_cmd
16-
`true`
13+
actual_opts = nil
14+
git.lib.define_singleton_method(:command) do |cmd, *opts, &block|
15+
actual_opts = opts.flatten
16+
''
1717
end
1818
message = 'My commit message'
1919
git.commit(message, gpg_sign: true)
20-
assert_match(/commit.*--gpg-sign['"]/, actual_cmd)
20+
assert(actual_opts.include?('--gpg-sign'))
2121
end
2222
end
2323

2424
def test_with_specific_gpg_keyid
2525
Dir.mktmpdir do |dir|
2626
git = Git.init(dir)
27-
actual_cmd = nil
28-
git.lib.define_singleton_method(:run_command) do |git_cmd, &block|
29-
actual_cmd = git_cmd
30-
`true`
27+
actual_opts = nil
28+
git.lib.define_singleton_method(:command) do |cmd, *opts, &block|
29+
actual_opts = opts.flatten
30+
''
3131
end
3232
message = 'My commit message'
3333
git.commit(message, gpg_sign: 'keykeykey')
34-
assert_match(/commit.*--gpg-sign=keykeykey['"]/, actual_cmd)
34+
assert(actual_opts.include?('--gpg-sign=keykeykey'))
3535
end
3636
end
3737

3838
def test_disabling_gpg_sign
3939
Dir.mktmpdir do |dir|
4040
git = Git.init(dir)
41-
actual_cmd = nil
42-
git.lib.define_singleton_method(:run_command) do |git_cmd, &block|
43-
actual_cmd = git_cmd
44-
`true`
41+
actual_opts = nil
42+
git.lib.define_singleton_method(:command) do |cmd, *opts, &block|
43+
actual_opts = opts.flatten
44+
''
4545
end
4646
message = 'My commit message'
4747
git.commit(message, no_gpg_sign: true)
48-
assert_match(/commit.*--no-gpg-sign['"]/, actual_cmd)
48+
assert(actual_opts.include?('--no-gpg-sign'))
4949
end
5050
end
5151

‎tests/units/test_config.rb

Lines changed: 23 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -7,18 +7,18 @@ def setup
77
set_file_paths
88
@git = Git.open(@wdir)
99
end
10-
10+
1111
def test_config
1212
c = @git.config
1313
assert_equal('Scott Chacon', c['user.name'])
1414
assert_equal('false', c['core.bare'])
1515
end
16-
16+
1717
def test_read_config
1818
assert_equal('Scott Chacon', @git.config('user.name'))
1919
assert_equal('false', @git.config('core.bare'))
2020
end
21-
21+
2222
def test_set_config
2323
in_temp_dir do |path|
2424
g = Git.clone(@wbare, 'bare')
@@ -42,34 +42,32 @@ def test_set_config_with_custom_file
4242
end
4343

4444
def test_env_config
45-
with_custom_env_variables do
46-
begin
47-
assert_equal(Git::Base.config.binary_path, 'git')
48-
assert_equal(Git::Base.config.git_ssh, nil)
45+
begin
46+
assert_equal(Git::Base.config.binary_path, 'git')
47+
assert_equal(Git::Base.config.git_ssh, nil)
4948

50-
ENV['GIT_PATH'] = '/env/bin'
51-
ENV['GIT_SSH'] = '/env/git/ssh'
49+
ENV['GIT_PATH'] = '/env/bin'
50+
ENV['GIT_SSH'] = '/env/git/ssh'
5251

53-
assert_equal(Git::Base.config.binary_path, '/env/bin/git')
54-
assert_equal(Git::Base.config.git_ssh, '/env/git/ssh')
52+
assert_equal(Git::Base.config.binary_path, '/env/bin/git')
53+
assert_equal(Git::Base.config.git_ssh, '/env/git/ssh')
5554

56-
Git.configure do |config|
57-
config.binary_path = '/usr/bin/git'
58-
config.git_ssh = '/path/to/ssh/script'
59-
end
55+
Git.configure do |config|
56+
config.binary_path = '/usr/bin/git'
57+
config.git_ssh = '/path/to/ssh/script'
58+
end
6059

61-
assert_equal(Git::Base.config.binary_path, '/usr/bin/git')
62-
assert_equal(Git::Base.config.git_ssh, '/path/to/ssh/script')
60+
assert_equal(Git::Base.config.binary_path, '/usr/bin/git')
61+
assert_equal(Git::Base.config.git_ssh, '/path/to/ssh/script')
6362

64-
@git.log
65-
ensure
66-
ENV['GIT_SSH'] = nil
67-
ENV['GIT_PATH'] = nil
63+
@git.log
64+
ensure
65+
ENV['GIT_SSH'] = nil
66+
ENV['GIT_PATH'] = nil
6867

69-
Git.configure do |config|
70-
config.binary_path = nil
71-
config.git_ssh = nil
72-
end
68+
Git.configure do |config|
69+
config.binary_path = nil
70+
config.git_ssh = nil
7371
end
7472
end
7573
end

‎tests/units/test_lib.rb

Lines changed: 16 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -114,39 +114,23 @@ def test_log_commits
114114
assert_equal(20, a.size)
115115
end
116116

117-
def test_environment_reset
118-
with_custom_env_variables do
119-
ENV['GIT_DIR'] = '/my/git/dir'
120-
ENV['GIT_WORK_TREE'] = '/my/work/tree'
121-
ENV['GIT_INDEX_FILE'] = 'my_index'
122-
123-
@lib.log_commits :count => 10
124-
125-
assert_equal(ENV['GIT_DIR'], '/my/git/dir')
126-
assert_equal(ENV['GIT_WORK_TREE'], '/my/work/tree')
127-
assert_equal(ENV['GIT_INDEX_FILE'],'my_index')
128-
end
129-
end
130-
131117
def test_git_ssh_from_environment_is_passed_to_binary
132-
with_custom_env_variables do
133-
begin
134-
Dir.mktmpdir do |dir|
135-
output_path = File.join(dir, 'git_ssh_value')
136-
binary_path = File.join(dir, 'git.bat') # .bat so it works in Windows too
137-
Git::Base.config.binary_path = binary_path
138-
File.open(binary_path, 'w') { |f|
139-
f << "echo \"my/git-ssh-wrapper\" > #{output_path}"
140-
}
141-
FileUtils.chmod(0700, binary_path)
142-
@lib.checkout('something')
143-
assert(File.read(output_path).include?("my/git-ssh-wrapper"))
144-
end
145-
ensure
146-
Git.configure do |config|
147-
config.binary_path = nil
148-
config.git_ssh = nil
149-
end
118+
begin
119+
Dir.mktmpdir do |dir|
120+
output_path = File.join(dir, 'git_ssh_value')
121+
binary_path = File.join(dir, 'git.bat') # .bat so it works in Windows too
122+
Git::Base.config.binary_path = binary_path
123+
File.open(binary_path, 'w') { |f|
124+
f << "echo \"my/git-ssh-wrapper\" > #{output_path}"
125+
}
126+
FileUtils.chmod(0700, binary_path)
127+
@lib.checkout('something')
128+
assert(File.read(output_path).include?("my/git-ssh-wrapper"))
129+
end
130+
ensure
131+
Git.configure do |config|
132+
config.binary_path = nil
133+
config.git_ssh = nil
150134
end
151135
end
152136
end

0 commit comments

Comments
 (0)