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

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    git-fastclone (1.4.5)
+    git-fastclone (1.5.0)
       colorize
 
 GEM

README.md

@@ -43,17 +43,29 @@ Usage
     gem install git-fastclone
     git fastclone [options] <git-repo-url>
 
-    -b, --branch <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:

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] <git-url> [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

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

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