From 55adc9f623730619c06026e82daf4f0836a79a0c Mon Sep 17 00:00:00 2001 From: Bartosz Polaczyk Date: Thu, 22 Feb 2024 15:10:04 -0800 Subject: [PATCH] Add pre-cloning-hook for mirror-prepolulation (#66) * Add pre-cloning-hook * Apply suggestions from code review Styling changes to invoking pre-clone-hook. * Syntax fix for duplicated unless * Revert to the check in a separate method due to cyclomatic complexity --- Gemfile.lock | 2 +- README.md | 22 ++++++++--- lib/git-fastclone.rb | 27 +++++++++---- lib/git-fastclone/version.rb | 2 +- spec/git_fastclone_runner_spec.rb | 66 ++++++++++++++++++++++++++----- 5 files changed, 96 insertions(+), 23 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 647fcf6..5f9dae3 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - git-fastclone (1.4.5) + git-fastclone (1.5.0) colorize GEM diff --git a/README.md b/README.md index 6bf727d..d2d33a3 100644 --- a/README.md +++ b/README.md @@ -43,17 +43,29 @@ Usage gem install git-fastclone git fastclone [options] - -b, --branch Clone a specific branch - -v, --verbose Verbose mode - -c, --color Display colored output - --config CONFIG Git config applied to the cloned repo - --lock-timeout N Timeout in seconds to acquire a lock on any reference repo. + -b, --branch BRANCH Checkout this branch rather than the default + -v, --verbose Verbose mode + --print_git_errors Print git output if a command fails + -c, --color Display colored output + --config CONFIG Git config applied to the cloned repo + --lock-timeout N Timeout in seconds to acquire a lock on any reference repo. + Default is 0 which waits indefinitely. + --pre-clone-hook command An optional command that should be invoked before cloning mirror repo Change the default `REFERENCE_REPO_DIR` environment variable if necessary. Cygwin users need to add `~/bin` to PATH. +Hooks +----- + +- `pre-clone-hook` is invoked right before cloning a new mirror repo, which gives a change to prepopulate git's mirror from a different source. +The hook is invoked with given arguments: +1. cloning repo url +1. path to the repo mirror location +1. attempt number, 0-indexed + How to test? ------------ Manual testing: diff --git a/lib/git-fastclone.rb b/lib/git-fastclone.rb index 18a67a7..3f1520f 100644 --- a/lib/git-fastclone.rb +++ b/lib/git-fastclone.rb @@ -166,6 +166,11 @@ def parse_options Default is 0 which waits indefinitely.') do |timeout_secs| self.flock_timeout_secs = timeout_secs.to_i end + + opts.on('--pre-clone-hook command', + 'An optional command that should be invoked before cloning mirror repo') do |command| + options[:pre_clone_hook] = command + end end.parse! end @@ -324,7 +329,7 @@ def update_submodule_reference(url, submodule_url_list) # overall checkout or not. When we pre-fetch based off of cached information, # fail_hard is false. When we fetch based off info in a repository directly, # fail_hard is true. - def update_reference_repo(url, fail_hard) + def update_reference_repo(url, fail_hard, attempt_number) repo_name = reference_repo_name(url) mirror = reference_repo_dir(url, reference_dir, using_local_repo) @@ -333,24 +338,26 @@ def update_reference_repo(url, fail_hard) submodule_file = reference_repo_submodule_file(url, reference_dir, using_local_repo) # if prefetch is on, then grab children immediately to frontload network requests - prefetch(submodule_file) if File.exist?(submodule_file) && prefetch_submodules + prefetch(submodule_file, attempt_number) if File.exist?(submodule_file) && prefetch_submodules # Store the fact that our repo has been updated if necessary - store_updated_repo(url, mirror, repo_name, fail_hard) unless reference_updated[repo_name] + store_updated_repo(url, mirror, repo_name, fail_hard, attempt_number) unless reference_updated[repo_name] end end # Grab the children in the event of a prefetch - def prefetch(submodule_file) + def prefetch(submodule_file, attempt_number) File.readlines(submodule_file).each do |line| # We don't join these threads explicitly - Thread.new { update_reference_repo(line.strip, false) } + Thread.new { update_reference_repo(line.strip, false, attempt_number) } end end # Creates or updates the mirror repo then stores an indication # that this repo has been updated on this run of fastclone - def store_updated_repo(url, mirror, repo_name, fail_hard) + def store_updated_repo(url, mirror, repo_name, fail_hard, attempt_number) + trigger_pre_clone_hook_if_needed(url, mirror, attempt_number) + # If pre_clone_hook correctly creates a mirror directory, we don't want to clone, but just update it unless Dir.exist?(mirror) fail_on_error('git', 'clone', verbose ? '--verbose' : '--quiet', '--mirror', url.to_s, mirror.to_s, quiet: !verbose, print_on_failure: print_git_errors) @@ -414,7 +421,7 @@ def with_git_mirror(url) retries_allowed ||= 1 attempt_number ||= 0 - update_reference_repo(url, true) + update_reference_repo(url, true, attempt_number) dir = reference_repo_dir(url, reference_dir, using_local_repo) # Sometimes remote updates involve re-packing objects on a different thread @@ -441,5 +448,11 @@ def with_git_mirror(url) def usage 'Usage: git fastclone [options] [path]' end + + private def trigger_pre_clone_hook_if_needed(url, mirror, attempt_number) + return if Dir.exist?(mirror) || !options.include?(:pre_clone_hook) + + popen2e_wrapper(options[:pre_clone_hook], url.to_s, mirror.to_s, attempt_number.to_s, quiet: !verbose) + end end end diff --git a/lib/git-fastclone/version.rb b/lib/git-fastclone/version.rb index c2f01c9..325a079 100644 --- a/lib/git-fastclone/version.rb +++ b/lib/git-fastclone/version.rb @@ -2,5 +2,5 @@ # Version string for git-fastclone module GitFastCloneVersion - VERSION = '1.4.5' + VERSION = '1.5.0' end diff --git a/spec/git_fastclone_runner_spec.rb b/spec/git_fastclone_runner_spec.rb index 1520c05..6c11eb1 100644 --- a/spec/git_fastclone_runner_spec.rb +++ b/spec/git_fastclone_runner_spec.rb @@ -145,6 +145,54 @@ def create_lockfile_double subject.clone(placeholder_arg, nil, '.', 'config') end end + + context 'with pre-clone-hook errors' do + let(:pre_clone_hook) { '/some/command' } + before(:each) do + subject.options[:pre_clone_hook] = pre_clone_hook + subject.reference_dir = placeholder_arg + allow(subject).to receive(:with_git_mirror).and_call_original + allow(subject).to receive(:with_reference_repo_lock) do |_url, &block| + block.call + end + end + + it 'invokes hook command' do + allow(subject).to receive(:fail_on_error) + expect(subject).to receive(:popen2e_wrapper).with( + pre_clone_hook, 'PH', 'PH/PH', '0', + { quiet: true } + ) { runner_execution_double } + + subject.clone(placeholder_arg, nil, '.', 'config') + end + + it 'does not call clone if hook creates mirror' do + allow(subject).to receive(:popen2e_wrapper).with( + pre_clone_hook, 'PH', 'PH/PH', '0', + { quiet: true } + ) do + # Emulate creating mirror dir + allow(Dir).to receive(:exist?).with('PH/PH').and_return(true) + end + allow(subject).to receive(:fail_on_error) + + subject.clone(placeholder_arg, nil, '.', 'config') + end + + it 'does not call pre-clone hook if mirror is already created' do + # Emulate already created mirror dir + allow(Dir).to receive(:exist?).and_call_original + allow(Dir).to receive(:exist?).with('PH/PH').and_return(true) + expect(subject).not_to receive(:popen2e_wrapper).with( + pre_clone_hook, 'PH', 'PH/PH', '0', + { quiet: true } + ) + allow(subject).to receive(:fail_on_error) + + subject.clone(placeholder_arg, nil, '.', 'config') + end + end end describe '.clear_clone_dest_if_needed' do @@ -249,7 +297,7 @@ def create_lockfile_double allow(File).to receive(:exist?) { true } subject.prefetch_submodules = true subject.reference_dir = placeholder_arg - subject.update_reference_repo(test_url_valid, false) + subject.update_reference_repo(test_url_valid, false, 0) end end @@ -262,7 +310,7 @@ def create_lockfile_double allow(File).to receive(:exist?) { true } subject.prefetch_submodules = false subject.reference_dir = placeholder_arg - subject.update_reference_repo(placeholder_arg, false) + subject.update_reference_repo(placeholder_arg, false, 0) end end @@ -277,7 +325,7 @@ def create_lockfile_double allow(subject).to receive(:reference_repo_dir) { placeholder_arg } subject.reference_updated = placeholder_hash subject.prefetch_submodules = false - subject.update_reference_repo(placeholder_arg, false) + subject.update_reference_repo(placeholder_arg, false, 0) end end @@ -291,7 +339,7 @@ def create_lockfile_double subject.reference_updated = placeholder_hash subject.reference_dir = placeholder_arg subject.prefetch_submodules = false - subject.update_reference_repo(placeholder_arg, false) + subject.update_reference_repo(placeholder_arg, false, 0) end end end @@ -302,7 +350,7 @@ def create_lockfile_double allow(File).to receive(:readlines) { %w[1 2 3] } subject.prefetch_submodules = true - subject.prefetch(placeholder_arg) + subject.prefetch(placeholder_arg, 0) end end @@ -315,7 +363,7 @@ def create_lockfile_double allow(subject).to receive(:fail_on_error) { raise ex } expect(FileUtils).to receive(:remove_entry_secure).with(placeholder_arg, force: true) expect do - subject.store_updated_repo(placeholder_arg, placeholder_arg, placeholder_arg, true) + subject.store_updated_repo(placeholder_arg, placeholder_arg, placeholder_arg, true, 0) end.to raise_error(ex) end @@ -327,7 +375,7 @@ def create_lockfile_double allow(subject).to receive(:fail_on_error) { raise ex } expect(FileUtils).to_not receive(:remove_entry_secure).with(placeholder_arg, force: true) expect do - subject.store_updated_repo(placeholder_arg, placeholder_arg, placeholder_arg, true) + subject.store_updated_repo(placeholder_arg, placeholder_arg, placeholder_arg, true, 0) end.to raise_error(ex) end end @@ -340,7 +388,7 @@ def create_lockfile_double allow(subject).to receive(:fail_on_error) { raise ex } expect(FileUtils).to receive(:remove_entry_secure).with(placeholder_arg, force: true) expect do - subject.store_updated_repo(placeholder_arg, placeholder_arg, placeholder_arg, false) + subject.store_updated_repo(placeholder_arg, placeholder_arg, placeholder_arg, false, 0) end.to_not raise_error end end @@ -351,7 +399,7 @@ def create_lockfile_double allow(subject).to receive(:fail_on_error) subject.reference_updated = placeholder_hash - subject.store_updated_repo(placeholder_arg, placeholder_arg, placeholder_arg, false) + subject.store_updated_repo(placeholder_arg, placeholder_arg, placeholder_arg, false, 0) expect(subject.reference_updated).to eq(placeholder_arg => true) end end