Skip to content

Commit 8581d85

Browse files
jcouballjcouball
committed
Implement the new timeout feature
1 parent 28400c0 commit 8581d85

File tree

8 files changed

+191
-25
lines changed

8 files changed

+191
-25
lines changed

‎bin/command_line_test

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,11 @@ require 'optparse'
3535
class CommandLineParser
3636
def initialize
3737
@option_parser = OptionParser.new
38+
@duration = 0
3839
define_options
3940
end
4041

41-
attr_reader :stdout, :stderr, :exitstatus, :signal
42+
attr_reader :duration, :stdout, :stderr, :exitstatus, :signal
4243

4344
# Parse the command line arguements returning the options
4445
#
@@ -84,7 +85,7 @@ class CommandLineParser
8485
option_parser.separator 'Options:'
8586
%i[
8687
define_help_option define_stdout_option define_stderr_option
87-
define_exitstatus_option define_signal_option
88+
define_exitstatus_option define_signal_option define_duration_option
8889
].each { |m| send(m) }
8990
end
9091

@@ -135,6 +136,15 @@ class CommandLineParser
135136
end
136137
end
137138

139+
# Define the duration option
140+
# @return [void]
141+
# @api private
142+
def define_duration_option
143+
option_parser.on('--duration=0', 'The number of seconds the command should take') do |duration|
144+
@duration = Integer(duration)
145+
end
146+
end
147+
138148
# Define the help option
139149
# @return [void]
140150
# @api private
@@ -176,5 +186,6 @@ options = CommandLineParser.new.parse(*ARGV)
176186

177187
STDOUT.puts options.stdout if options.stdout
178188
STDERR.puts options.stderr if options.stderr
189+
sleep options.duration unless options.duration.zero?
179190
Process.kill(options.signal, Process.pid) if options.signal
180191
exit(options.exitstatus) if options.exitstatus

‎git.gemspec

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ Gem::Specification.new do |s|
2828
s.requirements = ['git 2.28.0 or greater']
2929

3030
s.add_runtime_dependency 'addressable', '~> 2.8'
31-
s.add_runtime_dependency 'process_executer', '~> 0.7'
31+
s.add_runtime_dependency 'process_executer', '~> 1.1'
3232
s.add_runtime_dependency 'rchardet', '~> 1.8'
3333

3434
s.add_development_dependency 'minitar', '~> 0.9'

‎lib/git.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,13 @@
77
require 'git/base'
88
require 'git/branch'
99
require 'git/branches'
10+
require 'git/command_line_error'
1011
require 'git/command_line_result'
1112
require 'git/command_line'
1213
require 'git/config'
1314
require 'git/diff'
1415
require 'git/encoding_utils'
16+
require 'git/error'
1517
require 'git/escaped_path'
1618
require 'git/failed_error'
1719
require 'git/git_execute_error'
@@ -24,7 +26,6 @@
2426
require 'git/repository'
2527
require 'git/signaled_error'
2628
require 'git/status'
27-
require 'git/signaled_error'
2829
require 'git/stash'
2930
require 'git/stashes'
3031
require 'git/timeout_error'

‎lib/git/command_line.rb

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -166,21 +166,30 @@ def initialize(env, binary_path, global_opts, logger)
166166
# @param merge [Boolean] whether to merge stdout and stderr in the string returned
167167
# @param chdir [String] the directory to run the command in
168168
#
169+
# @param timeout [Float, Integer, nil] the maximum seconds to wait for the command to complete
170+
#
171+
# If timeout is zero of nil, the command will not time out. If the command
172+
# times out, it is killed via a SIGKILL signal and `Timeout::Error` is raised.
173+
#
174+
# If the command does not respond to SIGKILL, it will hang this method.
175+
#
169176
# @return [Git::CommandLineResult] the output of the command
170177
#
171178
# This result of running the command.
172179
#
173180
# @raise [ArgumentError] if `args` is not an array of strings
174181
# @raise [Git::SignaledError] if the command was terminated because of an uncaught signal
175182
# @raise [Git::FailedError] if the command returned a non-zero exitstatus
183+
# @raise [Git::GitExecuteError] if an exception was raised while collecting subprocess output
184+
# @raise [Git::TimeoutError] if the command times out
176185
#
177-
def run(*args, out:, err:, normalize:, chomp:, merge:, chdir: nil)
186+
def run(*args, out:, err:, normalize:, chomp:, merge:, chdir: nil, timeout: nil)
178187
git_cmd = build_git_cmd(args)
179188
out ||= StringIO.new
180189
err ||= (merge ? out : StringIO.new)
181-
status = execute(git_cmd, out, err, chdir: (chdir || :not_set))
190+
status = execute(git_cmd, out, err, chdir: (chdir || :not_set), timeout: timeout)
182191

183-
process_result(git_cmd, status, out, err, normalize, chomp)
192+
process_result(git_cmd, status, out, err, normalize, chomp, timeout)
184193
end
185194

186195
private
@@ -258,17 +267,24 @@ def raise_pipe_error(git_cmd, pipe_name, pipe)
258267
#
259268
# @param cmd [Array<String>] the git command to execute
260269
# @param chdir [String] the directory to run the command in
270+
# @param timeout [Float, Integer, nil] the maximum seconds to wait for the command to complete
271+
#
272+
# If timeout is zero of nil, the command will not time out. If the command
273+
# times out, it is killed via a SIGKILL signal and `Timeout::Error` is raised.
274+
#
275+
# If the command does not respond to SIGKILL, it will hang this method.
261276
#
262277
# @raise [Git::GitExecuteError] if an exception was raised while collecting subprocess output
278+
# @raise [Timeout::Error] if the command times out
263279
#
264-
# @return [Process::Status] the status of the completed subprocess
280+
# @return [ProcessExecuter::Status] the status of the completed subprocess
265281
#
266282
# @api private
267283
#
268-
def spawn(cmd, out_writers, err_writers, chdir:)
284+
def spawn(cmd, out_writers, err_writers, chdir:, timeout:)
269285
out_pipe = ProcessExecuter::MonitoredPipe.new(*out_writers, chunk_size: 10_000)
270286
err_pipe = ProcessExecuter::MonitoredPipe.new(*err_writers, chunk_size: 10_000)
271-
ProcessExecuter.spawn(env, *cmd, out: out_pipe, err: err_pipe, chdir: chdir)
287+
ProcessExecuter.spawn(env, *cmd, out: out_pipe, err: err_pipe, chdir: chdir, timeout: timeout)
272288
ensure
273289
out_pipe.close
274290
err_pipe.close
@@ -313,11 +329,12 @@ def writers(out, err)
313329
#
314330
# @api private
315331
#
316-
def process_result(git_cmd, status, out, err, normalize, chomp)
332+
def process_result(git_cmd, status, out, err, normalize, chomp, timeout)
317333
out_str, err_str = post_process_all([out, err], normalize, chomp)
318334
logger.info { "#{git_cmd} exited with status #{status}" }
319335
logger.debug { "stdout:\n#{out_str.inspect}\nstderr:\n#{err_str.inspect}" }
320336
Git::CommandLineResult.new(git_cmd, status, out_str, err_str).tap do |result|
337+
raise Git::TimeoutError.new(result, timeout) if status.timeout?
321338
raise Git::SignaledError.new(result) if status.signaled?
322339
raise Git::FailedError.new(result) unless status.success?
323340
end
@@ -329,14 +346,23 @@ def process_result(git_cmd, status, out, err, normalize, chomp)
329346
# @param out [#write] the object to write stdout to
330347
# @param err [#write] the object to write stderr to
331348
# @param chdir [String] the directory to run the command in
349+
# @param timeout [Float, Integer, nil] the maximum seconds to wait for the command to complete
350+
#
351+
# If timeout is zero of nil, the command will not time out. If the command
352+
# times out, it is killed via a SIGKILL signal and `Timeout::Error` is raised.
353+
#
354+
# If the command does not respond to SIGKILL, it will hang this method.
355+
#
356+
# @raise [Git::GitExecuteError] if an exception was raised while collecting subprocess output
357+
# @raise [Timeout::Error] if the command times out
332358
#
333359
# @return [Git::CommandLineResult] the result of the command to return to the caller
334360
#
335361
# @api private
336362
#
337-
def execute(git_cmd, out, err, chdir:)
363+
def execute(git_cmd, out, err, chdir:, timeout:)
338364
out_writers, err_writers = writers(out, err)
339-
spawn(git_cmd, out_writers, err_writers, chdir: chdir)
365+
spawn(git_cmd, out_writers, err_writers, chdir: chdir, timeout: timeout)
340366
end
341367
end
342368
end

‎lib/git/config.rb

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,12 @@ module Git
22

33
class Config
44

5-
attr_writer :binary_path, :git_ssh
5+
attr_writer :binary_path, :git_ssh, :timeout
66

77
def initialize
88
@binary_path = nil
99
@git_ssh = nil
10+
@timeout = nil
1011
end
1112

1213
def binary_path
@@ -17,6 +18,9 @@ def git_ssh
1718
@git_ssh || ENV['GIT_SSH']
1819
end
1920

21+
def timeout
22+
@timeout || (ENV['GIT_TIMEOUT'] && ENV['GIT_TIMEOUT'].to_i)
23+
end
2024
end
2125

2226
end

‎lib/git/lib.rb

Lines changed: 43 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ def clone(repository_url, directory, opts = {})
115115
arr_opts << repository_url
116116
arr_opts << clone_dir
117117

118-
command('clone', *arr_opts)
118+
command('clone', *arr_opts, timeout: opts[:timeout])
119119

120120
return_base_opts_from_clone(clone_dir, opts)
121121
end
@@ -1191,8 +1191,48 @@ def command_line
11911191
Git::CommandLine.new(env_overrides, Git::Base.config.binary_path, global_opts, @logger)
11921192
end
11931193

1194-
def command(*args, out: nil, err: nil, normalize: true, chomp: true, merge: false, chdir: nil)
1195-
result = command_line.run(*args, out: out, err: err, normalize: normalize, chomp: chomp, merge: merge, chdir: chdir)
1194+
# Runs a git command and returns the output
1195+
#
1196+
# @param args [Array] the git command to run and its arguments
1197+
#
1198+
# This should exclude the 'git' command itself and global options.
1199+
#
1200+
# For example, to run `git log --pretty=oneline`, you would pass `['log',
1201+
# '--pretty=oneline']`
1202+
#
1203+
# @param out [String, nil] the path to a file or an IO to write the command's
1204+
# stdout to
1205+
#
1206+
# @param err [String, nil] the path to a file or an IO to write the command's
1207+
# stdout to
1208+
#
1209+
# @param normalize [Boolean] true to normalize the output encoding
1210+
#
1211+
# @param chomp [Boolean] true to remove trailing newlines from the output
1212+
#
1213+
# @param merge [Boolean] true to merge stdout and stderr
1214+
#
1215+
# @param chdir [String, nil] the directory to run the command in
1216+
#
1217+
# @param timeout [Numeric, nil] the maximum time to wait for the command to
1218+
# complete
1219+
#
1220+
# @see Git::CommandLine#run
1221+
#
1222+
# @return [String] the command's stdout (or merged stdout and stderr if `merge`
1223+
# is true)
1224+
#
1225+
# @raise [Git::GitExecuteError] if the command fails
1226+
#
1227+
# The exception's `result` attribute is a {Git::CommandLineResult} which will
1228+
# contain the result of the command including the exit status, stdout, and
1229+
# stderr.
1230+
#
1231+
# @api private
1232+
#
1233+
def command(*args, out: nil, err: nil, normalize: true, chomp: true, merge: false, chdir: nil, timeout: nil)
1234+
timeout = timeout || Git.config.timeout
1235+
result = command_line.run(*args, out: out, err: err, normalize: normalize, chomp: chomp, merge: merge, chdir: chdir, timeout: timeout)
11961236
result.stdout
11971237
end
11981238

‎tests/units/test_command_line.rb

Lines changed: 38 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,39 @@ def merge
5454

5555
# END DEFAULT VALUES
5656

57+
sub_test_case "when a timeout is given" do
58+
test 'it should raise an ArgumentError if the timeout is not an Integer, Float, or nil' do
59+
command_line = Git::CommandLine.new(env, binary_path, global_opts, logger)
60+
args = []
61+
error = assert_raise ArgumentError do
62+
command_line.run(*args, out: out_writer, err: err_writer, normalize: normalize, chomp: chomp, merge: merge, timeout: 'not a number')
63+
end
64+
end
65+
66+
test 'it should raise a Git::TimeoutError if the command takes too long' do
67+
command_line = Git::CommandLine.new(env, binary_path, global_opts, logger)
68+
args = ['--duration=5']
69+
70+
error = assert_raise Git::TimeoutError do
71+
command_line.run(*args, out: out_writer, err: err_writer, normalize: normalize, chomp: chomp, merge: merge, timeout: 0.01)
72+
end
73+
end
74+
75+
test 'the error raised should indicate the command timed out' do
76+
command_line = Git::CommandLine.new(env, binary_path, global_opts, logger)
77+
args = ['--duration=5']
78+
79+
# Git::TimeoutError (alone with Git::FailedError and Git::SignaledError) is a
80+
# subclass of Git::GitExecuteError
81+
82+
begin
83+
command_line.run(*args, out: out_writer, err: err_writer, normalize: normalize, chomp: chomp, merge: merge, timeout: 0.01)
84+
rescue Git::GitExecuteError => e
85+
assert_equal(true, e.result.status.timeout?)
86+
end
87+
end
88+
end
89+
5790
test "run should return a result that includes the command ran, its output, and resulting status" do
5891
command_line = Git::CommandLine.new(env, binary_path, global_opts, logger)
5992
args = ['--stdout=stdout output', '--stderr=stderr output']
@@ -62,7 +95,7 @@ def merge
6295
assert_equal(['ruby', 'bin/command_line_test', '--stdout=stdout output', '--stderr=stderr output'], result.git_cmd)
6396
assert_equal('stdout output', result.stdout.chomp)
6497
assert_equal('stderr output', result.stderr.chomp)
65-
assert(result.status.is_a? Process::Status)
98+
assert(result.status.is_a? ProcessExecuter::Status)
6699
assert_equal(0, result.status.exitstatus)
67100
end
68101

@@ -116,10 +149,10 @@ def merge
116149
command_line = Git::CommandLine.new(env, binary_path, global_opts, logger)
117150
args = ['--stdout=stdout output']
118151

119-
def command_line.spawn(cmd, out_writers, err_writers, chdir: nil)
152+
def command_line.spawn(cmd, out_writers, err_writers, chdir: nil, timeout: nil)
120153
out_writers.each { |w| w.write(File.read('tests/files/encoding/test1.txt')) }
121154
`true`
122-
$? # return status
155+
ProcessExecuter::Status.new($?, false) # return status
123156
end
124157

125158
normalize = true
@@ -139,10 +172,10 @@ def command_line.spawn(cmd, out_writers, err_writers, chdir: nil)
139172
command_line = Git::CommandLine.new(env, binary_path, global_opts, logger)
140173
args = ['--stdout=stdout output']
141174

142-
def command_line.spawn(cmd, out_writers, err_writers, chdir: nil)
175+
def command_line.spawn(cmd, out_writers, err_writers, chdir: nil, timeout: nil)
143176
out_writers.each { |w| w.write(File.read('tests/files/encoding/test1.txt')) }
144177
`true`
145-
$? # return status
178+
ProcessExecuter::Status.new($?, false) # return status
146179
end
147180

148181
normalize = false

0 commit comments

Comments
 (0)