Replace Terrapin with BuildExecution to fix process deadlock when out…

…put to stderr is fills buffer (#53)

* Also Fix wrong logic in retriable_error

Gemfile.lock

@@ -1,15 +1,13 @@
 PATH
   remote: .
   specs:
-    git-fastclone (1.3.3)
+    git-fastclone (1.4.0)
       colorize
-      terrapin (~> 0.6.0)
 
 GEM
   remote: https://rubygems.org/
   specs:
     ast (2.4.2)
-    climate_control (0.2.0)
     colorize (0.8.1)
     diff-lcs (1.5.0)
     json (2.6.3)
@@ -46,8 +44,6 @@ GEM
     rubocop-ast (1.24.0)
       parser (>= 3.1.1.0)
     ruby-progressbar (1.11.0)
-    terrapin (0.6.0)
-      climate_control (>= 0.0.3, < 1.0)
     unicode-display_width (2.3.0)
 
 PLATFORMS

git-fastclone.gemspec

@@ -37,6 +37,5 @@ Gem::Specification.new do |gem|
   gem.required_ruby_version = '>= 2.7'
 
   gem.add_runtime_dependency 'colorize'
-  gem.add_runtime_dependency 'terrapin', '~> 0.6.0'
   gem.metadata['rubygems_mfa_required'] = 'true'
 end

lib/git-fastclone.rb

@@ -16,9 +16,8 @@
 
 require 'optparse'
 require 'fileutils'
-require 'logger'
-require 'terrapin'
 require 'timeout'
+require_relative 'runner_execution'
 
 # Contains helper module UrlHelper and execution class GitFastClone::Runner
 module GitFastClone
@@ -68,13 +67,14 @@ class Runner
     require 'colorize'
 
     include GitFastClone::UrlHelper
+    include RunnerExecution
 
     DEFAULT_REFERENCE_REPO_DIR = '/var/tmp/git-fastclone/reference'
 
     DEFAULT_GIT_ALLOW_PROTOCOL = 'file:git:http:https:ssh'
 
     attr_accessor :reference_dir, :prefetch_submodules, :reference_updated, :reference_mutex,
-                  :options, :logger, :abs_clone_path, :using_local_repo, :verbose, :color,
+                  :options, :abs_clone_path, :using_local_repo, :verbose, :color,
                   :flock_timeout_secs
 
     def initialize
@@ -94,8 +94,6 @@ def initialize
 
       self.options = {}
 
-      self.logger = nil # Only set in verbose mode
-
       self.abs_clone_path = Dir.pwd
 
       self.using_local_repo = false
@@ -119,8 +117,7 @@ def run
       end
 
       puts "Cloning #{path_from_git_url(url)} to #{File.join(abs_clone_path, path)}"
-      Terrapin::CommandLine.environment['GIT_ALLOW_PROTOCOL'] =
-        ENV['GIT_ALLOW_PROTOCOL'] || DEFAULT_GIT_ALLOW_PROTOCOL
+      ENV['GIT_ALLOW_PROTOCOL'] ||= DEFAULT_GIT_ALLOW_PROTOCOL
       clone(url, options[:branch], path, options[:config])
     end
 
@@ -137,11 +134,6 @@ def parse_options
 
         opts.on('-v', '--verbose', 'Verbose mode') do
           self.verbose = true
-          self.logger = Logger.new($stdout)
-          logger.formatter = proc do |_severity, _datetime, _progname, msg|
-            "#{msg}\n"
-          end
-          Terrapin::CommandLine.logger = logger
         end
 
         opts.on('-c', '--color', 'Display colored output') do
@@ -217,19 +209,16 @@ def clone(url, rev, src_dir, config)
       with_git_mirror(url) do |mirror, attempt_number|
         clear_clone_dest_if_needed(attempt_number, clone_dest)
 
-        clone_command = '--quiet --reference :mirror :url :path'
-        clone_command += ' --config :config' unless config.nil?
-        Terrapin::CommandLine.new('git clone', clone_command)
-                             .run(mirror: mirror.to_s,
-                                  url: url.to_s,
-                                  path: clone_dest,
-                                  config: config.to_s)
+        clone_commands = ['git', 'clone', verbose ? '--verbose' : '--quiet']
+        clone_commands << '--reference' << mirror.to_s << url.to_s << clone_dest
+        clone_commands << '--config' << config.to_s unless config.nil?
+        fail_pipe_on_error(clone_commands, quiet: !verbose)
       end
 
       # Only checkout if we're changing branches to a non-default branch
       if rev
         Dir.chdir(File.join(abs_clone_path, src_dir)) do
-          Terrapin::CommandLine.new('git checkout', '--quiet :rev').run(rev: rev.to_s)
+          fail_pipe_on_error(['git', 'checkout', '--quiet', rev.to_s], quiet: !verbose)
         end
       end
 
@@ -252,9 +241,12 @@ def update_submodules(pwd, url)
 
       threads = []
       submodule_url_list = []
+      output = ''
+      Dir.chdir(File.join(abs_clone_path, pwd).to_s) do
+        output = fail_on_error('git', 'submodule', 'init', quiet: !verbose)
+      end
 
-      Terrapin::CommandLine.new('cd', ':path; git submodule init 2>&1')
-                           .run(path: File.join(abs_clone_path, pwd)).split("\n").each do |line|
+      output.split("\n").each do |line|
         submodule_path, submodule_url = parse_update_info(line)
         submodule_url_list << submodule_url
 
@@ -268,10 +260,12 @@ def update_submodules(pwd, url)
     def thread_update_submodule(submodule_url, submodule_path, threads, pwd)
       threads << Thread.new do
         with_git_mirror(submodule_url) do |mirror, _|
-          Terrapin::CommandLine.new('cd', ':dir; git submodule update --quiet --reference :mirror :path')
-                               .run(dir: File.join(abs_clone_path, pwd).to_s,
-                                    mirror: mirror.to_s,
-                                    path: submodule_path.to_s)
+          Dir.chdir(File.join(abs_clone_path, pwd).to_s) do
+            fail_pipe_on_error(
+              ['git', 'submodule', verbose ? nil : '--quiet', 'update', '--reference', mirror.to_s,
+               submodule_path.to_s].compact, quiet: !verbose
+            )
+          end
         end
 
         update_submodules(File.join(pwd, submodule_path), submodule_url)
@@ -343,43 +337,54 @@ def prefetch(submodule_file)
     # that this repo has been updated on this run of fastclone
     def store_updated_repo(url, mirror, repo_name, fail_hard)
       unless Dir.exist?(mirror)
-        Terrapin::CommandLine.new('git clone', '--mirror :url :mirror')
-                             .run(url: url.to_s, mirror: mirror.to_s)
+        fail_pipe_on_error(
+          ['git', 'clone', verbose ? '--verbose' : '--quiet', '--mirror', url.to_s,
+           mirror.to_s], quiet: !verbose
+        )
       end
 
-      Terrapin::CommandLine.new('cd', ':path; git remote update --prune').run(path: mirror)
-
+      Dir.chdir(mirror) do
+        cmd = ['git', 'remote', verbose ? '--verbose' : nil, 'update', '--prune'].compact
+        if verbose
+          fail_pipe_on_error(cmd, quiet: !verbose)
+        else
+          # Because above operation might spit out a lot to stderr, we use this to swallow them
+          # and only display if the operation return non 0 exit code
+          fail_on_error(*cmd, quiet: !verbose)
+        end
+      end
       reference_updated[repo_name] = true
-    rescue Terrapin::ExitStatusError => e
+    rescue RunnerExecutionRuntimeError => e
       # To avoid corruption of the cache, if we failed to update or check out we remove
       # the cache directory entirely. This may cause the current clone to fail, but if the
       # underlying error from git is transient it will not affect future clones.
-      FileUtils.remove_entry_secure(mirror, force: true)
+      clear_cache(mirror, url)
       raise e if fail_hard
     end
 
     def retriable_error?(error)
       error_strings = [
-        'fatal: missing blob object',
-        'fatal: remote did not send all necessary objects',
-        /fatal: packed object [a-z0-9]+ \(stored in .*?\) is corrupt/,
-        /fatal: pack has \d+ unresolved delta/,
-        'error: unable to read sha1 file of ',
-        'fatal: did not receive expected object',
+        /^fatal: missing blob object/,
+        /^fatal: remote did not send all necessary objects/,
+        /^fatal: packed object [a-z0-9]+ \(stored in .*?\) is corrupt/,
+        /^fatal: pack has \d+ unresolved delta/,
+        /^error: unable to read sha1 file of /,
+        /^fatal: did not receive expected object/,
         /^fatal: unable to read tree [a-z0-9]+\n^warning: Clone succeeded, but checkout failed/
       ]
-      error.to_s =~ /^STDERR:\n.*^#{Regexp.union(error_strings)}/m
+      error.to_s =~ /.*#{Regexp.union(error_strings)}/m
     end
 
     def print_formatted_error(error)
       indented_error = error.to_s.split("\n").map { |s| ">  #{s}\n" }.join
-      puts "Encountered a retriable error:\n#{indented_error}\n\nRemoving the fastclone cache."
+      puts "[INFO] Encountered a retriable error:\n#{indented_error}\n"
     end
 
     # To avoid corruption of the cache, if we failed to update or check out we remove
     # the cache directory entirely. This may cause the current clone to fail, but if the
     # underlying error from git is transient it will not affect future clones.
     def clear_cache(dir, url)
+      puts "[WARN] Removing the fastclone cache at #{dir}"
       FileUtils.remove_entry_secure(dir, force: true)
       reference_updated.delete(reference_repo_name(url))
     end
@@ -405,9 +410,9 @@ def with_git_mirror(url)
       with_reference_repo_lock(url) do
         yield dir, attempt_number
       end
-    rescue Terrapin::ExitStatusError => e
-      if retriable_error?(e)
-        print_formatted_error(e)
+    rescue RunnerExecutionRuntimeError => e
+      if retriable_error?(e.output)
+        print_formatted_error(e.output)
         clear_cache(dir, url)
 
         if attempt_number < retries_allowed
@@ -416,7 +421,7 @@ def with_git_mirror(url)
         end
       end
 
-      raise
+      raise e
     end
 
     def usage

lib/git-fastclone/version.rb

@@ -2,5 +2,5 @@
 
 # Version string for git-fastclone
 module GitFastCloneVersion
-  VERSION = '1.3.3'
+  VERSION = '1.4.0'
 end