Merge pull request glennsarti#38 from beechtom/bug/gclone

Support using commits for `GClone` spec searcher, correctly print errors

Author: glennsarti

lib/puppetfile-resolver/spec_searchers/git/gclone.rb

@@ -12,6 +12,8 @@ module PuppetfileResolver
   module SpecSearchers
     module Git
       module GClone
+        METADATA_FILE = 'metadata.json'
+
         # @summary clones the remote url and reads the metadata file
         # @returns [String] the content of the metadata file
         def self.metadata(puppetfile_module, resolver_ui, config)
@@ -25,8 +27,6 @@ def self.metadata(puppetfile_module, resolver_ui, config)
           return nil if repo_url.nil?
           return nil unless valid_http_url?(repo_url)
 
-          metadata_file = 'metadata.json'
-
           ref = puppetfile_module.ref ||
                 puppetfile_module.tag ||
                 puppetfile_module.commit ||
@@ -35,7 +35,7 @@ def self.metadata(puppetfile_module, resolver_ui, config)
 
           resolver_ui.debug { "Querying git repository #{repo_url}" }
 
-          clone_and_read_file(repo_url, ref, metadata_file, config)
+          clone_and_read_file(repo_url, ref, config)
         end
 
         # @summary clones the git url and reads the file at the given ref
@@ -44,24 +44,37 @@ def self.metadata(puppetfile_module, resolver_ui, config)
         # @param ref [String] the git ref, branch, commit, tag
         # @param file [String] the file you wish to read
         # @returns [String] the content of the file
-        def self.clone_and_read_file(url, ref, file, config)
-          clone_cmd = ['git', 'clone', '--bare', '--depth=1', '--single-branch']
-          err_msg = ''
-          if config.proxy
-            err_msg += " with proxy #{config.proxy}: "
-            proxy = "--config \"http.proxy=#{config.proxy}\" --config \"https.proxy=#{config.proxy}\""
-            clone_cmd.push(proxy)
-          end
-
+        def self.clone_and_read_file(url, ref, config)
           Dir.mktmpdir(nil, config.clone_dir) do |dir|
-            clone_cmd.push("--branch=#{ref}") if ref != 'HEAD'
-            clone_cmd.push(url, dir)
-            out, err_out, process = ::PuppetfileResolver::Util.run_command(clone_cmd)
-            err_msg += out
-            raise err_msg unless process.success?
+            clone = ['git', 'clone', url, dir]
+            clone += ['--config', "http.proxy=#{config.proxy}", '--config', "https.proxy=#{config.proxy}"] if config.proxy
+
+            bare_clone = clone + ['--bare', '--depth=1']
+            bare_clone.push("--branch=#{ref}") unless ref == 'HEAD'
+
+            # Try to clone a bare repository. If that fails, fall back to a full clone.
+            # Cloning might fail because the repo does not exist or is otherwise
+            # inaccessible, but it can also fail because cloning a bare repository from
+            # a commit/SHA1 fails. Falling back to a full clone ensures that we support
+            # commits/SHA1s like Puppetfile does.
+            _stdout, _stderr, process = ::PuppetfileResolver::Util.run_command(bare_clone)
+
+            unless process.success?
+              _stdout, stderr, process = ::PuppetfileResolver::Util.run_command(clone)
+
+              unless process.success?
+                msg = if config.proxy
+                        "Cloning #{url} with proxy #{config.proxy} failed: #{stderr}"
+                      else
+                        "Cloning #{url} failed: #{stderr}"
+                      end
+                raise msg
+              end
+            end
+
             Dir.chdir(dir) do
-              content, err_out, process = ::PuppetfileResolver::Util.run_command(['git', 'show', "#{ref}:#{file}"])
-              raise 'InvalidContent' unless process.success? && content.length > 2
+              content, stderr, process = ::PuppetfileResolver::Util.run_command(['git', 'show', "#{ref}:#{METADATA_FILE}"])
+              raise "Could not find #{METADATA_FILE} for ref #{ref} at #{url}: #{stderr}" unless process.success?
               return content
             end
           end

spec/unit/puppetfile-resolver/spec_searchers/git/gclone_spec.rb

@@ -23,8 +23,7 @@
       expect(JSON.parse(content)['name']).to eq('puppetlabs-powershell')
     end
 
-    context 'with ref' do
-
+    context 'with tag' do
       let(:puppetfile_module) do
         PuppetfileModule.new(remote: url, ref: '2.1.2')
       end
@@ -34,6 +33,29 @@
         expect(JSON.parse(content)['name']).to eq('puppetlabs-powershell')
       end
     end
+
+    context 'with commit' do
+      let(:puppetfile_module) do
+        PuppetfileModule.new(remote: url, ref: '9276de695798097e8471b877a18df27f764eecda')
+      end
+
+      it 'reads metadata' do
+        content = subject.metadata(puppetfile_module, Logger.new(STDERR), config)
+        expect(JSON.parse(content)['name']).to eq('puppetlabs-powershell')
+      end
+    end
+
+    context 'with invalid ref' do
+      let(:puppetfile_module) do
+        PuppetfileModule.new(remote: url, ref: '8f7d5ea3ef49dadc5e166d5d802d091abc4b02bc')
+      end
+
+      it 'errors gracefully' do
+        expect { subject.metadata(puppetfile_module, Logger.new(STDERR), config) }.to raise_error(
+          /Could not find metadata\.json for ref .* at .*/
+        )
+      end
+    end
   end
 
   context 'invalid url' do