Skip to content

Commit

Permalink
Add print_git_errors mode (#56)
Browse files Browse the repository at this point in the history
  • Loading branch information
polac24 authored Jun 27, 2023
1 parent 1dd8c7d commit aff916d
Show file tree
Hide file tree
Showing 7 changed files with 119 additions and 19 deletions.
2 changes: 1 addition & 1 deletion Gemfile.lock
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
PATH
remote: .
specs:
git-fastclone (1.4.1)
git-fastclone (1.4.2)
colorize

GEM
Expand Down
24 changes: 17 additions & 7 deletions lib/git-fastclone.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ class Runner
DEFAULT_GIT_ALLOW_PROTOCOL = 'file:git:http:https:ssh'

attr_accessor :reference_dir, :prefetch_submodules, :reference_updated, :reference_mutex,
:options, :abs_clone_path, :using_local_repo, :verbose, :color,
:options, :abs_clone_path, :using_local_repo, :verbose, :print_git_errors, :color,
:flock_timeout_secs

def initialize
Expand All @@ -100,6 +100,8 @@ def initialize

self.verbose = false

self.print_git_errors = false

self.color = false

self.flock_timeout_secs = 0
Expand Down Expand Up @@ -133,9 +135,15 @@ def parse_options
end

opts.on('-v', '--verbose', 'Verbose mode') do
puts '--print_git_errors is redundant when using --verbose' if print_git_errors
self.verbose = true
end

opts.on('--print_git_errors', 'Print git output if a command fails') do
puts '--print_git_errors is redundant when using --verbose' if verbose
self.print_git_errors = true
end

opts.on('-c', '--color', 'Display colored output') do
self.color = true
end
Expand Down Expand Up @@ -212,13 +220,14 @@ def clone(url, rev, src_dir, config)
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_on_error(*clone_commands, quiet: !verbose)
fail_on_error(*clone_commands, quiet: !verbose, print_on_failure: print_git_errors)
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
fail_on_error('git', 'checkout', '--quiet', rev.to_s, quiet: !verbose)
fail_on_error('git', 'checkout', '--quiet', rev.to_s, quiet: !verbose,
print_on_failure: print_git_errors)
end
end

Expand All @@ -243,7 +252,8 @@ def update_submodules(pwd, url)
submodule_url_list = []
output = ''
Dir.chdir(File.join(abs_clone_path, pwd).to_s) do
output = fail_on_error('git', 'submodule', 'init', quiet: !verbose)
output = fail_on_error('git', 'submodule', 'init', quiet: !verbose,
print_on_failure: print_git_errors)
end

output.split("\n").each do |line|
Expand All @@ -263,7 +273,7 @@ def thread_update_submodule(submodule_url, submodule_path, threads, pwd)
Dir.chdir(File.join(abs_clone_path, pwd).to_s) do
cmd = ['git', 'submodule', verbose ? nil : '--quiet', 'update', '--reference', mirror.to_s,
submodule_path.to_s].compact
fail_on_error(*cmd, quiet: !verbose)
fail_on_error(*cmd, quiet: !verbose, print_on_failure: print_git_errors)
end
end

Expand Down Expand Up @@ -337,12 +347,12 @@ def prefetch(submodule_file)
def store_updated_repo(url, mirror, repo_name, fail_hard)
unless Dir.exist?(mirror)
fail_on_error('git', 'clone', verbose ? '--verbose' : '--quiet', '--mirror', url.to_s, mirror.to_s,
quiet: !verbose)
quiet: !verbose, print_on_failure: print_git_errors)
end

Dir.chdir(mirror) do
cmd = ['git', 'remote', verbose ? '--verbose' : nil, 'update', '--prune'].compact
fail_on_error(*cmd, quiet: !verbose)
fail_on_error(*cmd, quiet: !verbose, print_on_failure: print_git_errors)
end
reference_updated[repo_name] = true
rescue RunnerExecutionRuntimeError => e
Expand Down
2 changes: 1 addition & 1 deletion lib/git-fastclone/version.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@

# Version string for git-fastclone
module GitFastCloneVersion
VERSION = '1.4.1'
VERSION = '1.4.2'
end
13 changes: 8 additions & 5 deletions lib/runner_execution.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ def initialize(status, command, output = nil)

# Runs a command that fails on error.
# Uses popen2e wrapper. Handles bad statuses with potential for retries.
def fail_on_error(*cmd, stdin_data: nil, binmode: false, quiet: false, **opts)
def fail_on_error(*cmd, stdin_data: nil, binmode: false, quiet: false, print_on_failure: false, **opts)
print_command('Running Shell Safe Command:', [cmd]) unless quiet
shell_safe_cmd = shell_safe(cmd)
retry_times = opts[:retry] || 0
Expand All @@ -39,7 +39,9 @@ def fail_on_error(*cmd, stdin_data: nil, binmode: false, quiet: false, **opts)
end

# Get out with the status, good or bad.
exit_on_status(output, [shell_safe_cmd], [status], quiet: quiet)
# When quiet, we don't need to print the output, as it is already streamed from popen2e_wrapper
needs_print_on_failure = quiet && print_on_failure
exit_on_status(output, [shell_safe_cmd], [status], quiet: quiet, print_on_failure: needs_print_on_failure)
end
module_function :fail_on_error

Expand Down Expand Up @@ -155,20 +157,21 @@ def tee(in_stream, out_stream)
# return code of the first one.
#
# Otherwise returns first argument (output)
def exit_on_status(output, cmd_list, status_list, quiet: false)
def exit_on_status(output, cmd_list, status_list, quiet: false, print_on_failure: false)
status_list.each_index do |index|
status = status_list[index]
cmd = cmd_list[index]
check_status(cmd, status, output: output, quiet: quiet)
check_status(cmd, status, output: output, quiet: quiet, print_on_failure: print_on_failure)
end

output
end
module_function :exit_on_status

def check_status(cmd, status, output: nil, quiet: false)
def check_status(cmd, status, output: nil, quiet: false, print_on_failure: false)
return if status.exited? && status.exitstatus == 0

logger.info(output) if print_on_failure
# If we exited nonzero or abnormally, print debugging info and explode.
if status.exited?
logger.debug("Process Exited normally. Exit status:#{status.exitstatus}") unless quiet
Expand Down
14 changes: 14 additions & 0 deletions script/spec_demo_tool.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#/bin/bash

# This script is a sample script used in integration tests that exits with the code passed as the first argument
# Also, it prints all extra arguments

exit_code="$1"

if [ $# -gt 1 ]; then
# Skip first argument, which is the exit code
shift
echo "$@"
fi

exit $exit_code
25 changes: 20 additions & 5 deletions spec/git_fastclone_runner_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,11 @@ def create_lockfile_double
it 'should clone correctly' do
expect(subject).to receive(:fail_on_error).with(
'git', 'checkout', '--quiet', 'PH',
{ quiet: true }
{ quiet: true, print_on_failure: false }
) { runner_execution_double }
expect(subject).to receive(:fail_on_error).with(
'git', 'clone', '--quiet', '--reference', '/cache', 'PH', '/pwd/.',
{ quiet: true }
{ quiet: true, print_on_failure: false }
) { runner_execution_double }

subject.clone(placeholder_arg, placeholder_arg, '.', nil)
Expand All @@ -113,11 +113,11 @@ def create_lockfile_double
subject.verbose = true
expect(subject).to receive(:fail_on_error).with(
'git', 'checkout', '--quiet', 'PH',
{ quiet: false }
{ quiet: false, print_on_failure: false }
) { runner_execution_double }
expect(subject).to receive(:fail_on_error).with(
'git', 'clone', '--verbose', '--reference', '/cache', 'PH', '/pwd/.',
{ quiet: false }
{ quiet: false, print_on_failure: false }
) { runner_execution_double }

subject.clone(placeholder_arg, placeholder_arg, '.', nil)
Expand All @@ -126,11 +126,26 @@ def create_lockfile_double
it 'should clone correctly with custom configs' do
expect(subject).to receive(:fail_on_error).with(
'git', 'clone', '--quiet', '--reference', '/cache', 'PH', '/pwd/.', '--config', 'config',
{ quiet: true }
{ quiet: true, print_on_failure: false }
) { runner_execution_double }

subject.clone(placeholder_arg, nil, '.', 'config')
end

context 'with printing errors' do
before(:each) do
subject.print_git_errors = true
end

it 'prints failures' do
expect(subject).to receive(:fail_on_error).with(
'git', 'clone', '--quiet', '--reference', '/cache', 'PH', '/pwd/.', '--config', 'config',
{ quiet: true, print_on_failure: true }
) { runner_execution_double }

subject.clone(placeholder_arg, nil, '.', 'config')
end
end
end

describe '.clear_clone_dest_if_needed' do
Expand Down
58 changes: 58 additions & 0 deletions spec/runner_execution_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
# frozen_string_literal: true

# Copyright 2023 Square Inc.

# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at

# http://www.apache.org/licenses/LICENSE-2.0

# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

require 'spec_helper'
require 'git-fastclone'

# Integration tests use real demo_tool.sh to inspect the E2E behavior
describe RunnerExecution do
subject { described_class }
let(:external_tool) { "#{__dir__}/../script/spec_demo_tool.sh" }
let(:logger) { double('logger') }

before do
allow($stdout).to receive(:puts)
allow(logger).to receive(:info)
allow(logger).to receive(:debug)
allow(logger).to receive(:warn)
allow(RunnerExecution).to receive(:logger).and_return(logger)
end

describe '.fail_on_error' do
it 'should log failure info on command error' do
expect(logger).to receive(:info).with("My error output\n")

expect do
described_class.fail_on_error(external_tool, '1', 'My error output', quiet: true,
print_on_failure: true)
end.to raise_error(RunnerExecution::RunnerExecutionRuntimeError)
end

it 'should not log failure output on command success' do
expect($stdout).not_to receive(:info)

described_class.fail_on_error(external_tool, '0', 'My success output', quiet: true,
print_on_failure: true)
end

it 'should not log failure output when not in the quiet mode' do
expect($stdout).not_to receive(:info)

described_class.fail_on_error(external_tool, '0', 'My success output', quiet: false,
print_on_failure: true)
end
end
end

0 comments on commit aff916d

Please sign in to comment.