pdk update and convert fixes

lib/pdk/module/convert.rb

@@ -133,11 +133,15 @@ def update_metadata(metadata_path, template_metadata)
       def summary
         summary = {}
         update_manager.changes.each do |category, update_category|
-          updated_files = if update_category.respond_to?(:keys)
-                            update_category.keys
-                          else
-                            update_category.map { |file| file[:path] }
-                          end
+          if update_category.respond_to?(:keys)
+            updated_files = update_category.keys
+          else
+            begin
+              updated_files = update_category.map { |file| file[:path] }
+            rescue TypeError
+              updated_files = update_category.to_a
+            end
+          end
 
           summary[category] = updated_files
         end

lib/pdk/module/templatedir.rb

@@ -48,7 +48,7 @@ def initialize(path_or_url, module_metadata = {}, init = false)
           # template repo in `%AppData%` or `$XDG_CACHE_DIR` and update before
           # use.
           temp_dir = PDK::Util.make_tmpdir_name('pdk-templates')
-          git_ref = PDK::Util.default_template_ref
+          git_ref = (path_or_url == PDK::Util.default_template_url) ? PDK::Util.default_template_ref : 'origin/master'
 
           clone_result = PDK::Util::Git.git('clone', path_or_url, temp_dir)
 

lib/pdk/module/update.rb

@@ -8,18 +8,22 @@ class Update < Convert
       def run
         stage_changes!
 
+        unless update_manager.changes?
+          if current_version == new_version
+            PDK.logger.info _('This module is already up to date with version %{version} of the template.') % {
+              version: new_version,
+            }
+          else
+            PDK::Report.default_target.puts(_('No changes required.'))
+          end
+          return
+        end
+
         PDK.logger.info(update_message)
 
         print_summary
         full_report('update_report.txt') unless update_manager.changes[:modified].empty?
 
-        if !update_manager.changes? && get_sha(current_version) == get_sha(new_version)
-          PDK.logger.info _('This module is already up to date with version %{version} of the template.') % {
-            version: new_version,
-          }
-          return
-        end
-
         return if noop?
 
         unless force?
@@ -59,10 +63,6 @@ def new_version
 
       private
 
-      def get_sha(version_ref)
-        version_ref.split('@')[-1]
-      end
-
       def current_template_version
         @current_template_version ||= module_metadata.data['template-ref']
       end
@@ -74,8 +74,6 @@ def describe_ref_to_s(describe_ref)
 
         if data[:base].start_with?('heads/')
           "#{data[:base].gsub(%r{^heads/}, '')}@#{data[:sha]}"
-        elsif data[:sha]
-          "#{data[:base]}@#{data[:sha]}"
         else
           data[:base]
         end
@@ -86,12 +84,10 @@ def new_template_version
       end
 
       def fetch_remote_version(version)
-        data = GIT_DESCRIBE_PATTERN.match(current_template_version)
-
-        return version if data.nil?
+        return version unless version.include?('/')
 
-        branch = version.partition('/').last if version.include?('/')
-        sha_length = data[:sha].length - 1
+        branch = version.partition('/').last
+        sha_length = GIT_DESCRIBE_PATTERN.match(current_template_version)[:sha].length - 1
         "#{branch}@#{PDK::Util::Git.ls_remote(template_url, "refs/heads/#{branch}")[0..sha_length]}"
       end
 

spec/acceptance/update_spec.rb

@@ -13,8 +13,8 @@
 
     describe command('pdk update') do
       its(:exit_status) { is_expected.to eq(0) }
-      its(:stdout) { is_expected.to match(no_output) }
-      its(:stderr) { is_expected.to match(%r{already up to date}i) }
+      its(:stdout) { is_expected.to match(%r{No changes required}i) }
+      its(:stderr) { is_expected.to match(no_output) }
     end
 
     describe file('update_report.txt') do

spec/unit/pdk/module/template_dir_spec.rb

@@ -233,6 +233,7 @@
     before(:each) do
       allow(File).to receive(:directory?).with(anything).and_return(true)
       allow(File).to receive(:directory?).with(path_or_url).and_return(false)
+      allow(PDK::Util).to receive(:default_template_url).and_return(path_or_url)
       allow(PDK::Util).to receive(:default_template_ref).and_return('default-ref')
       allow(PDK::Util).to receive(:make_tmpdir_name).with('pdk-templates').and_return(tmp_path)
       allow(PDK::Util::Git).to receive(:git).with('clone', path_or_url, tmp_path).and_return(exit_code: 0)
@@ -249,4 +250,26 @@
       end
     end
   end
+
+  describe 'custom template' do
+    before(:each) do
+      allow(File).to receive(:directory?).with(anything).and_return(true)
+      allow(File).to receive(:directory?).with(path_or_url).and_return(false)
+      allow(PDK::Util).to receive(:default_template_url).and_return('default-url')
+      allow(PDK::Util).to receive(:default_template_ref).and_return('default-ref')
+      allow(PDK::Util).to receive(:make_tmpdir_name).with('pdk-templates').and_return(tmp_path)
+      allow(PDK::Util::Git).to receive(:git).with('clone', path_or_url, tmp_path).and_return(exit_code: 0)
+      allow(PDK::Util::Git).to receive(:git).with('-C', tmp_path, 'reset', '--hard', 'origin/master').and_return(exit_code: 0)
+      allow(FileUtils).to receive(:remove_dir).with(tmp_path)
+      allow(PDK::Util::Git).to receive(:git).with('--git-dir', anything, 'describe', '--all', '--long', '--always').and_return(exit_code: 0, stdout: '1234abcd')
+      allow(PDK::Util::Version).to receive(:version_string).and_return('0.0.0')
+      allow(PDK::Util).to receive(:canonical_path).with(tmp_path).and_return(tmp_path)
+    end
+
+    context 'pdk data' do
+      it 'includes the PDK version and template info' do
+        expect(template_dir.metadata).to include('pdk-version' => '0.0.0', 'template-url' => path_or_url, 'template-ref' => '1234abcd')
+      end
+    end
+  end
 end

spec/unit/pdk/module/update_spec.rb

@@ -211,7 +211,7 @@
       let(:template_ref) { '1.3.2-0-g07678c8' }
 
       it 'returns the tag name' do
-        is_expected.to eq('1.3.2@07678c8')
+        is_expected.to eq('1.3.2')
       end
     end