Revert "Add a timeout for git commands (#692)"

jcouball · web-flow · commit 8da29c661483 · 2024-03-19T15:53:18.000-07:00
This reverts commit ab0f757.
diff --git a/README.md b/README.md
@@ -11,18 +11,6 @@
 [![Build Status](https://github.com/ruby-git/ruby-git/workflows/CI/badge.svg?branch=master)](https://github.com/ruby-git/ruby-git/actions?query=workflow%3ACI)
 [![Code Climate](https://codeclimate.com/github/ruby-git/ruby-git.png)](https://codeclimate.com/github/ruby-git/ruby-git)
 
-* [Summary](#summary)
-* [v2.0.0 pre-release](#v200-pre-release)
-* [Install](#install)
-* [Major Objects](#major-objects)
-* [Errors Raised By This Gem](#errors-raised-by-this-gem)
-* [Specifying And Handling Timeouts](#specifying-and-handling-timeouts)
-* [Examples](#examples)
-* [Ruby version support policy](#ruby-version-support-policy)
-* [License](#license)
-
-## Summary
-
 The [git gem](https://rubygems.org/gems/git) provides an API that can be used to
 create, read, and manipulate Git repositories by wrapping system calls to the `git`
 command line. The API can be used for working with Git in complex interactions
@@ -152,60 +140,6 @@ rescue Git::TimeoutError => e # Catch the more specific error first!
   puts "Git clone took too long and timed out #{e}"
 rescue Git::Error => e
   puts "Received the following error: #{e}"
-```
-
-## Specifying And Handling Timeouts
-
-The timeout feature was added in git gem version `2.0.0`.
-
-A timeout for git operations can be set either globally or for specific method calls
-that accept a `:timeout` parameter.
-
-The timeout value must be a real, non-negative `Numeric` value that specifies a
-number of seconds a `git` command will be given to complete before being sent a KILL
-signal. This library may hang if the `git` command does not terminate after receiving
-the KILL signal.
-
-When a command times out, a `Git::TimeoutError` is raised.
-
-If the timeout value is `0` or `nil`, no timeout will be enforced.
-
-If a method accepts a `:timeout` parameter and a receives a non-nil value, it will
-override the global timeout value. In this context, a value of `nil` (which is
-usually the default) will use the global timeout value and a value of `0` will turn
-off timeout enforcement for that method call no matter what the global value is.
-
-To set a global timeout, use the `Git.config` object:
-
-```ruby
-Git.config.timeout = nil # a value of nil or 0 means no timeout is enforced
-Git.config.timeout = 1.5 # can be any real, non-negative Numeric interpreted as number of seconds
-```
-
-The global timeout can be overridden for a specific method if the method accepts a
-`:timeout` parameter:
-
-```ruby
-repo_url = 'https://github.com/ruby-git/ruby-git.git'
-Git.clone(repo_url) # Use the global timeout value
-Git.clone(repo_url, timeout: nil) # Also uses the global timeout value
-Git.clone(repo_url, timeout: 0) # Do not enforce a timeout
-Git.clone(repo_url, timeout: 10.5)  # Timeout after 10.5 seconds raising Git::SignaledError
-```
-
-If the command takes too long, a `Git::SignaledError` will be raised:
-
-```ruby
-begin
-  Git.clone(repo_url, timeout: 10)
-rescue Git::TimeoutError => e
-  result = e.result
-  result.class #=> Git::CommandLineResult
-  result.status #=> #<Process::Status: pid 62173 SIGKILL (signal 9)>
-  result.status.timeout? #=> true
-  result.git_cmd # The git command ran as an array of strings
-  result.stdout # The command's output to stdout until it was terminated
-  result.stderr # The command's output to stderr until it was terminated
 end
 ```
 
diff --git a/bin/command_line_test b/bin/command_line_test
@@ -35,11 +35,10 @@ require 'optparse'
 class CommandLineParser
   def initialize
     @option_parser = OptionParser.new
-    @duration = 0
     define_options
   end
 
-  attr_reader :duration, :stdout, :stderr, :exitstatus, :signal
+  attr_reader :stdout, :stderr, :exitstatus, :signal
 
   # Parse the command line arguements returning the options
   #
@@ -85,7 +84,7 @@ class CommandLineParser
     option_parser.separator 'Options:'
     %i[
       define_help_option define_stdout_option define_stderr_option
-      define_exitstatus_option define_signal_option define_duration_option
+      define_exitstatus_option define_signal_option
     ].each { |m| send(m) }
   end
 
@@ -136,15 +135,6 @@ class CommandLineParser
     end
   end
 
-  # Define the duration option
-  # @return [void]
-  # @api private
-  def define_duration_option
-    option_parser.on('--duration=0', 'The number of seconds the command should take') do |duration|
-      @duration = Integer(duration)
-    end
-  end
-
   # Define the help option
   # @return [void]
   # @api private
@@ -186,6 +176,5 @@ options = CommandLineParser.new.parse(*ARGV)
 
 STDOUT.puts options.stdout if options.stdout
 STDERR.puts options.stderr if options.stderr
-sleep options.duration unless options.duration.zero?
 Process.kill(options.signal, Process.pid) if options.signal
 exit(options.exitstatus) if options.exitstatus
diff --git a/git.gemspec b/git.gemspec
@@ -28,7 +28,7 @@ Gem::Specification.new do |s|
   s.requirements = ['git 2.28.0 or greater']
 
   s.add_runtime_dependency 'addressable', '~> 2.8'
-  s.add_runtime_dependency 'process_executer', '~> 1.1'
+  s.add_runtime_dependency 'process_executer', '~> 0.7'
   s.add_runtime_dependency 'rchardet', '~> 1.8'
 
   s.add_development_dependency 'minitar', '~> 0.9'
diff --git a/lib/git.rb b/lib/git.rb
@@ -7,13 +7,11 @@
 require 'git/base'
 require 'git/branch'
 require 'git/branches'
-require 'git/command_line_error'
 require 'git/command_line_result'
 require 'git/command_line'
 require 'git/config'
 require 'git/diff'
 require 'git/encoding_utils'
-require 'git/error'
 require 'git/escaped_path'
 require 'git/failed_error'
 require 'git/git_execute_error'
@@ -26,6 +24,7 @@
 require 'git/repository'
 require 'git/signaled_error'
 require 'git/status'
+require 'git/signaled_error'
 require 'git/stash'
 require 'git/stashes'
 require 'git/timeout_error'
diff --git a/lib/git/command_line.rb b/lib/git/command_line.rb
@@ -166,30 +166,21 @@ def initialize(env, binary_path, global_opts, logger)
     # @param merge [Boolean] whether to merge stdout and stderr in the string returned
     # @param chdir [String] the directory to run the command in
     #
-    # @param timeout [Numeric, nil] the maximum seconds to wait for the command to complete
-    #
-    #   If timeout is zero or nil, the command will not time out. If the command
-    #   times out, it is killed via a SIGKILL signal and `Git::TimeoutError` is raised.
-    #
-    #   If the command does not respond to SIGKILL, it will hang this method.
-    #
     # @return [Git::CommandLineResult] the output of the command
     #
     #   This result of running the command.
     #
     # @raise [ArgumentError] if `args` is not an array of strings
     # @raise [Git::SignaledError] if the command was terminated because of an uncaught signal
     # @raise [Git::FailedError] if the command returned a non-zero exitstatus
-    # @raise [Git::GitExecuteError] if an exception was raised while collecting subprocess output
-    # @raise [Git::TimeoutError] if the command times out
     #
-    def run(*args, out:, err:, normalize:, chomp:, merge:, chdir: nil, timeout: nil)
+    def run(*args, out:, err:, normalize:, chomp:, merge:, chdir: nil)
       git_cmd = build_git_cmd(args)
       out ||= StringIO.new
       err ||= (merge ? out : StringIO.new)
-      status = execute(git_cmd, out, err, chdir: (chdir || :not_set), timeout: timeout)
+      status = execute(git_cmd, out, err, chdir: (chdir || :not_set))
 
-      process_result(git_cmd, status, out, err, normalize, chomp, timeout)
+      process_result(git_cmd, status, out, err, normalize, chomp)
     end
 
     private
@@ -267,24 +258,17 @@ def raise_pipe_error(git_cmd, pipe_name, pipe)
     #
     # @param cmd [Array<String>] the git command to execute
     # @param chdir [String] the directory to run the command in
-    # @param timeout [Float, Integer, nil] the maximum seconds to wait for the command to complete
-    #
-    #   If timeout is zero of nil, the command will not time out. If the command
-    #   times out, it is killed via a SIGKILL signal and `Git::TimeoutError` is raised.
-    #
-    #   If the command does not respond to SIGKILL, it will hang this method.
     #
     # @raise [Git::GitExecuteError] if an exception was raised while collecting subprocess output
-    # @raise [Git::TimeoutError] if the command times out
     #
-    # @return [ProcessExecuter::Status] the status of the completed subprocess
+    # @return [Process::Status] the status of the completed subprocess
     #
     # @api private
     #
-    def spawn(cmd, out_writers, err_writers, chdir:, timeout:)
+    def spawn(cmd, out_writers, err_writers, chdir:)
       out_pipe = ProcessExecuter::MonitoredPipe.new(*out_writers, chunk_size: 10_000)
       err_pipe = ProcessExecuter::MonitoredPipe.new(*err_writers, chunk_size: 10_000)
-      ProcessExecuter.spawn(env, *cmd, out: out_pipe, err: err_pipe, chdir: chdir, timeout: timeout)
+      ProcessExecuter.spawn(env, *cmd, out: out_pipe, err: err_pipe, chdir: chdir)
     ensure
       out_pipe.close
       err_pipe.close
@@ -329,12 +313,11 @@ def writers(out, err)
     #
     # @api private
     #
-    def process_result(git_cmd, status, out, err, normalize, chomp, timeout)
+    def process_result(git_cmd, status, out, err, normalize, chomp)
       out_str, err_str = post_process_all([out, err], normalize, chomp)
       logger.info { "#{git_cmd} exited with status #{status}" }
       logger.debug { "stdout:\n#{out_str.inspect}\nstderr:\n#{err_str.inspect}" }
       Git::CommandLineResult.new(git_cmd, status, out_str, err_str).tap do |result|
-        raise Git::TimeoutError.new(result, timeout) if status.timeout?
         raise Git::SignaledError.new(result) if status.signaled?
         raise Git::FailedError.new(result) unless status.success?
       end
@@ -346,23 +329,14 @@ def process_result(git_cmd, status, out, err, normalize, chomp, timeout)
     # @param out [#write] the object to write stdout to
     # @param err [#write] the object to write stderr to
     # @param chdir [String] the directory to run the command in
-    # @param timeout [Float, Integer, nil] the maximum seconds to wait for the command to complete
-    #
-    #   If timeout is zero of nil, the command will not time out. If the command
-    #   times out, it is killed via a SIGKILL signal and `Git::TimeoutError` is raised.
-    #
-    #   If the command does not respond to SIGKILL, it will hang this method.
-    #
-    # @raise [Git::GitExecuteError] if an exception was raised while collecting subprocess output
-    # @raise [Git::TimeoutError] if the command times out
     #
     # @return [Git::CommandLineResult] the result of the command to return to the caller
     #
     # @api private
     #
-    def execute(git_cmd, out, err, chdir:, timeout:)
+    def execute(git_cmd, out, err, chdir:)
       out_writers, err_writers = writers(out, err)
-      spawn(git_cmd, out_writers, err_writers, chdir: chdir, timeout: timeout)
+      spawn(git_cmd, out_writers, err_writers, chdir: chdir)
     end
   end
 end
diff --git a/lib/git/config.rb b/lib/git/config.rb
@@ -2,12 +2,11 @@ module Git
 
   class Config
 
-    attr_writer :binary_path, :git_ssh, :timeout
+    attr_writer :binary_path, :git_ssh
 
     def initialize
       @binary_path = nil
       @git_ssh = nil
-      @timeout = nil
     end
 
     def binary_path
@@ -18,9 +17,6 @@ def git_ssh
       @git_ssh || ENV['GIT_SSH']
     end
 
-    def timeout
-      @timeout || (ENV['GIT_TIMEOUT'] && ENV['GIT_TIMEOUT'].to_i)
-    end
   end
 
 end
diff --git a/lib/git/lib.rb b/lib/git/lib.rb
@@ -115,7 +115,7 @@ def clone(repository_url, directory, opts = {})
       arr_opts << repository_url
       arr_opts << clone_dir
 
-      command('clone', *arr_opts, timeout: opts[:timeout])
+      command('clone', *arr_opts)
 
       return_base_opts_from_clone(clone_dir, opts)
     end
@@ -1191,48 +1191,8 @@ def command_line
         Git::CommandLine.new(env_overrides, Git::Base.config.binary_path, global_opts, @logger)
     end
 
-    # Runs a git command and returns the output
-    #
-    # @param args [Array] the git command to run and its arguments
-    #
-    #   This should exclude the 'git' command itself and global options.
-    #
-    #   For example, to run `git log --pretty=oneline`, you would pass `['log',
-    #   '--pretty=oneline']`
-    #
-    # @param out [String, nil] the path to a file or an IO to write the command's
-    #   stdout to
-    #
-    # @param err [String, nil] the path to a file or an IO to write the command's
-    #   stdout to
-    #
-    # @param normalize [Boolean] true to normalize the output encoding
-    #
-    # @param chomp [Boolean] true to remove trailing newlines from the output
-    #
-    # @param merge [Boolean] true to merge stdout and stderr
-    #
-    # @param chdir [String, nil] the directory to run the command in
-    #
-    # @param timeout [Numeric, nil] the maximum time to wait for the command to
-    # complete
-    #
-    # @see Git::CommandLine#run
-    #
-    # @return [String] the command's stdout (or merged stdout and stderr if `merge`
-    # is true)
-    #
-    # @raise [Git::GitExecuteError] if the command fails
-    #
-    #   The exception's `result` attribute is a {Git::CommandLineResult} which will
-    #   contain the result of the command including the exit status, stdout, and
-    #   stderr.
-    #
-    # @api private
-    #
-    def command(*args, out: nil, err: nil, normalize: true, chomp: true, merge: false, chdir: nil, timeout: nil)
-      timeout = timeout || Git.config.timeout
-      result = command_line.run(*args, out: out, err: err, normalize: normalize, chomp: chomp, merge: merge, chdir: chdir, timeout: timeout)
+    def command(*args, out: nil, err: nil, normalize: true, chomp: true, merge: false, chdir: nil)
+      result = command_line.run(*args, out: out, err: err, normalize: normalize, chomp: chomp, merge: merge, chdir: chdir)
       result.stdout
     end
 
diff --git a/tests/units/test_command_line.rb b/tests/units/test_command_line.rb
diff --git a/tests/units/test_git_clone.rb b/tests/units/test_git_clone.rb
