Merge pull request #434 from hunner/add_template_ref

(PDK-718) Add --template-ref argument for upstream template repo tags

lib/pdk/cli.rb

@@ -39,11 +39,15 @@ def self.run(args)
   end
 
   def self.template_url_option(dsl)
-    desc = _('Specifies the URL to the template to use when creating new modules or classes. (default: %{default_url})') % { default_url: PDK::Util.default_template_url }
+    desc = _('Specifies the URL to the template to use when creating new modules or classes. (default: %{default_url})') % { default_url: PDK::Util::TemplateURI.default_template_uri }
 
     dsl.option nil, 'template-url', desc, argument: :required
   end
 
+  def self.template_ref_option(dsl)
+    dsl.option nil, 'template-ref', _('Specifies the template git branch or tag to use when creating new modules or classes.'), argument: :required
+  end
+
   def self.skip_interview_option(dsl)
     dsl.option nil, 'skip-interview', _('When specified, skips interactive querying of metadata.')
   end

lib/pdk/cli/convert.rb

@@ -7,6 +7,7 @@ module PDK::CLI
     summary _('Convert an existing module to be compatible with the PDK.')
 
     PDK::CLI.template_url_option(self)
+    PDK::CLI.template_ref_option(self)
     PDK::CLI.skip_interview_option(self)
     PDK::CLI.full_interview_option(self)
     flag nil, :noop, _('Do not convert the module, just output what would be done.')
@@ -21,6 +22,8 @@ module PDK::CLI
         log_level:           :info,
       )
 
+      PDK::CLI::Util.validate_template_opts(opts)
+
       if opts[:noop] && opts[:force]
         raise PDK::CLI::ExitWithError, _('You can not specify --noop and --force when converting a module')
       end

lib/pdk/cli/module/generate.rb

@@ -7,6 +7,7 @@ module PDK::CLI
     summary _('This command is now \'pdk new module\'.')
 
     PDK::CLI.template_url_option(self)
+    PDK::CLI.template_ref_option(self)
     PDK::CLI.skip_interview_option(self)
 
     run do |opts, args, _cmd|
@@ -19,6 +20,8 @@ module PDK::CLI
         exit 1
       end
 
+      PDK::CLI::Util.validate_template_opts(opts)
+
       PDK.logger.info(_('New modules are created using the ‘pdk new module’ command.'))
       prompt = TTY::Prompt.new(help_color: :cyan)
       redirect = PDK::CLI::Util::CommandRedirector.new(prompt)

lib/pdk/cli/new/class.rb

@@ -4,8 +4,6 @@ module PDK::CLI
     usage _('class [options] <class_name>')
     summary _('Create a new class named <class_name> using given options')
 
-    PDK::CLI.template_url_option(self)
-
     run do |opts, args, _cmd|
       require 'pdk/generate/puppet_class'
 

lib/pdk/cli/new/defined_type.rb

@@ -4,8 +4,6 @@ module PDK::CLI
     usage _('defined_type [options] <name>')
     summary _('Create a new defined type named <name> using given options')
 
-    PDK::CLI.template_url_option(self)
-
     run do |opts, args, _cmd|
       PDK::CLI::Util.ensure_in_module!(
         message: _('Defined types can only be created from inside a valid module directory.'),

lib/pdk/cli/new/module.rb

@@ -5,6 +5,7 @@ module PDK::CLI
     summary _('Create a new module named [module_name] using given options')
 
     PDK::CLI.template_url_option(self)
+    PDK::CLI.template_ref_option(self)
     PDK::CLI.skip_interview_option(self)
     PDK::CLI.full_interview_option(self)
 
@@ -18,6 +19,8 @@ module PDK::CLI
       module_name = args[0]
       target_dir = args[1]
 
+      PDK::CLI::Util.validate_template_opts(opts)
+
       if opts[:'skip-interview'] && opts[:'full-interview']
         PDK.logger.info _('Ignoring --full-interview and continuing with --skip-interview.')
         opts[:'full-interview'] = false

lib/pdk/cli/new/provider.rb

@@ -4,8 +4,6 @@ module PDK::CLI
     usage _('provider [options] <name>')
     summary _('[experimental] Create a new ruby provider named <name> using given options')
 
-    PDK::CLI.template_url_option(self)
-
     run do |opts, args, _cmd|
       PDK::CLI::Util.ensure_in_module!
 

lib/pdk/cli/new/task.rb

@@ -4,7 +4,6 @@ module PDK::CLI
     usage _('task [options] <name>')
     summary _('Create a new task named <name> using given options')
 
-    PDK::CLI.template_url_option(self)
     option nil, :description, _('A short description of the purpose of the task'), argument: :required
 
     run do |opts, args, _cmd|

lib/pdk/cli/update.rb

@@ -10,6 +10,8 @@ module PDK::CLI
     flag nil, :noop, _('Do not update the module, just output what would be done.')
     flag nil, :force, _('Update the module automatically, with no prompts.')
 
+    PDK::CLI.template_ref_option(self)
+
     run do |opts, _args, _cmd|
       require 'pdk/module/update'
 

lib/pdk/cli/util.rb

@@ -188,6 +188,16 @@ def validate_puppet_version_opts(opts)
         end
       end
       module_function :validate_puppet_version_opts
+
+      def validate_template_opts(opts)
+        if opts[:'template-ref'] && opts[:'template-url'].nil?
+          raise PDK::CLI::ExitWithError, _('--template-ref requires --template-url to also be specified.')
+        end
+
+        return unless opts[:'template-url'] && opts[:'template-url'].include?('#')
+        raise PDK::CLI::ExitWithError, _('--template-url may not be used to specify paths containing #\'s.')
+      end
+      module_function :validate_template_opts
     end
   end
 end

lib/pdk/generate/module.rb

@@ -54,10 +54,10 @@ def self.invoke(opts = {})
 
         prepare_module_directory(temp_target_dir)
 
-        template_url = opts.fetch(:'template-url', PDK::Util.default_template_url)
+        template_uri = PDK::Util::TemplateURI.new(opts)
 
         begin
-          PDK::Module::TemplateDir.new(template_url, metadata.data, true) do |templates|
+          PDK::Module::TemplateDir.new(template_uri, metadata.data, true) do |templates|
             templates.render do |file_path, file_content|
               file = Pathname.new(temp_target_dir) + file_path
               file.dirname.mkpath
@@ -74,21 +74,23 @@ def self.invoke(opts = {})
           raise PDK::CLI::ExitWithError, e
         end
 
-        if template_url == PDK::Util.puppetlabs_template_url
-          # If the user specifies our template via the command line, remove the
-          # saved template-url answer.
+        # Only update the answers files after metadata has been written.
+        if template_uri.default?
+          # If the user specifies our default template url via the command
+          # line, remove the saved template-url answer so that the template_uri
+          # resolution can find new default URLs in the future.
           PDK.answers.update!('template-url' => nil) if opts.key?(:'template-url')
         else
-          # Save the template-url answer if the module was generated using
-          # a template other than ours.
-          PDK.answers.update!('template-url' => template_url)
+          # Save the template-url answers if the module was generated using a
+          # template/reference other than ours.
+          PDK.answers.update!('template-url' => template_uri.metadata_format)
         end
 
         begin
           if FileUtils.mv(temp_target_dir, target_dir)
             Dir.chdir(target_dir) { PDK::Util::Bundler.ensure_bundle! } unless opts[:'skip-bundle-install']
 
-            PDK.logger.info(_('Module \'%{name}\' generated at path \'%{path}\', from template \'%{template_url}\'.') % { name: opts[:module_name], path: target_dir, template_url: template_url })
+            PDK.logger.info _('Module \'%{name}\' generated at path \'%{path}\', from template \'%{url}\'.') % { name: opts[:module_name], path: target_dir, url: template_uri.git_remote }
             PDK.logger.info(_('In your module directory, add classes with the \'pdk new class\' command.'))
           end
         rescue Errno::EACCES => e

lib/pdk/generate/puppet_object.rb

@@ -29,7 +29,7 @@ class PuppetObject
       # @param options [Hash{Symbol => Object}]
       #
       # @api public
-      def initialize(module_dir, object_name, options = {})
+      def initialize(module_dir, object_name, options)
         @module_dir = module_dir
         @options = options
         @object_name = object_name
@@ -205,12 +205,12 @@ def write_file(dest_path)
       # @api private
       def with_templates
         templates.each do |template|
-          if template[:url].nil?
-            PDK.logger.debug(_('No %{dir_type} template specified; trying next template directory.') % { dir_type: template[:type] })
+          if template[:uri].nil?
+            PDK.logger.debug(_('No %{dir_type} template found; trying next template directory.') % { dir_type: template[:type] })
             next
           end
 
-          PDK::Module::TemplateDir.new(template[:url]) do |template_dir|
+          PDK::Module::TemplateDir.new(PDK::Util::TemplateURI.new(template[:uri])) do |template_dir|
             template_paths = template_dir.object_template_for(object_type)
 
             if template_paths
@@ -250,11 +250,7 @@ def with_templates
       #
       # @api private
       def templates
-        @templates ||= [
-          { type: 'CLI', url: @options[:'template-url'], allow_fallback: false },
-          { type: 'metadata', url: module_metadata.data['template-url'], allow_fallback: true },
-          { type: 'default', url: PDK::Util.default_template_url, allow_fallback: false },
-        ]
+        @templates ||= PDK::Util::TemplateURI.templates(@options)
       end
 
       # Retrieves the name of the module (without the forge username) from the

lib/pdk/module/convert.rb

@@ -68,7 +68,7 @@ def needs_bundle_update?
       def stage_changes!
         metadata_path = 'metadata.json'
 
-        PDK::Module::TemplateDir.new(template_url, nil, false) do |templates|
+        PDK::Module::TemplateDir.new(template_uri, nil, false) do |templates|
           new_metadata = update_metadata(metadata_path, templates.metadata)
           templates.module_metadata = new_metadata.data unless new_metadata.nil?
 
@@ -102,8 +102,8 @@ def update_manager
         @update_manager ||= PDK::Module::UpdateManager.new
       end
 
-      def template_url
-        @template_url ||= options.fetch(:'template-url', PDK::Util.default_template_url)
+      def template_uri
+        @template_uri ||= PDK::Util::TemplateURI.new(options)
       end
 
       def update_metadata(metadata_path, template_metadata)

lib/pdk/module/templatedir.rb

@@ -16,8 +16,8 @@ class TemplateDir
       # The template directory is only guaranteed to be available on disk
       # within the scope of the block passed to this method.
       #
-      # @param path_or_url [String] The path to a directory to use as the
-      # template or a URL to a git repository.
+      # @param uri [PDK::Util::TemplateURI] The path to a directory to use as the
+      # template or a URI to a git repository.
       # @param module_metadata [Hash] A Hash containing the module metadata.
       # Defaults to an empty Hash.
       # @yieldparam self [PDK::Module::TemplateDir] The initialised object with
@@ -37,17 +37,31 @@ class TemplateDir
       # @raise [ArgumentError] (see #validate_module_template!)
       #
       # @api public
-      def initialize(path_or_url, module_metadata = {}, init = false)
+      def initialize(uri, module_metadata = {}, init = false)
         unless block_given?
           raise ArgumentError, _('%{class_name} must be initialized with a block.') % { class_name: self.class.name }
         end
+        unless uri.is_a? PDK::Util::TemplateURI
+          raise ArgumentError, _('PDK::Module::TemplateDir.new must be initialized with a PDK::Util::TemplateURI, got a %{uri_type}') % { uri_type: uri.class }
+        end
 
-        if PDK::Util::Git.repo?(path_or_url)
-          @path = self.class.clone_template_repo(path_or_url)
-          @repo = path_or_url
+        if PDK::Util::Git.repo?(uri.git_remote)
+          # This is either a bare local repo or a remote. either way it needs cloning.
+          @path = clone_template_repo(uri)
+          temp_dir_clone = true
         else
-          @path = path_or_url
+          # if it is a local path & non-bare repo then we can use it directly.
+          # Still have to check the branch.
+          @path = uri.shell_path
+          # We don't do a checkout of local-path repos. There are lots of edge
+          # cases or user un-expectations.
+          if PDK::Util::Git.work_tree?(@path)
+            PDK.logger.warn _("Repository '%{repo}' has a work-tree; skipping git reset.") % {
+              repo: @path,
+            }
+          end
         end
+        @cloned_from = uri.metadata_format
 
         @init = init
         @moduleroot_dir = File.join(@path, 'moduleroot')
@@ -64,7 +78,7 @@ def initialize(path_or_url, module_metadata = {}, init = false)
       ensure
         # If we cloned a git repo to get the template, remove the clone once
         # we're done with it.
-        if @repo
+        if temp_dir_clone
           FileUtils.remove_dir(@path)
         end
       end
@@ -78,16 +92,11 @@ def initialize(path_or_url, module_metadata = {}, init = false)
       #
       # @api public
       def metadata
-        result = {
-          'pdk-version' => PDK::Util::Version.version_string,
+        {
+          'pdk-version'  => PDK::Util::Version.version_string,
+          'template-url' => @cloned_from,
+          'template-ref' => cache_template_ref(@path),
         }
-
-        result['template-url'] = @repo ? @repo : @path
-
-        ref_result = PDK::Util::Git.git('--git-dir', File.join(@path, '.git'), 'describe', '--all', '--long', '--always')
-        result['template-ref'] = ref_result[:stdout].strip if ref_result[:exit_code].zero?
-
-        result
       end
 
       # Loop through the files in the template, yielding each rendered file to
@@ -118,11 +127,11 @@ def render
           else
             begin
               dest_content = PDK::TemplateFile.new(File.join(template_loc, template_file), configs: config, template_dir: self).render
-            rescue => e
+            rescue => error
               error_msg = _(
                 "Failed to render template '%{template}'\n" \
                 '%{exception}: %{message}',
-              ) % { template: template_file, exception: e.class, message: e.message }
+              ) % { template: template_file, exception: error.class, message: error.message }
               raise PDK::CLI::FatalError, error_msg
             end
           end
@@ -290,24 +299,18 @@ def read_config(loc)
       # @raise [PDK::CLI::FatalError] If reset HEAD of the cloned repo to desired ref.
       #
       # @api private
-      def self.clone_template_repo(origin_repo)
+      def clone_template_repo(uri)
         # @todo When switching this over to using rugged, cache the cloned
         # template repo in `%AppData%` or `$XDG_CACHE_DIR` and update before
         # use.
         temp_dir = PDK::Util.make_tmpdir_name('pdk-templates')
-        git_ref = (origin_repo == PDK::Util.default_template_url) ? PDK::Util.default_template_ref : 'origin/master'
+        origin_repo = uri.git_remote
+        git_ref = uri.git_ref
 
         clone_result = PDK::Util::Git.git('clone', origin_repo, temp_dir)
 
         if clone_result[:exit_code].zero?
-          Dir.chdir(temp_dir) do
-            reset_result = PDK::Util::Git.git('reset', '--hard', git_ref)
-            unless reset_result[:exit_code].zero?
-              PDK.logger.error reset_result[:stdout]
-              PDK.logger.error reset_result[:stderr]
-              raise PDK::CLI::FatalError, _("Unable to set HEAD of git repository at '%{repo}' to ref:'%{ref}'.") % { repo: temp_dir, ref: git_ref }
-            end
-          end
+          checkout_template_ref(temp_dir, git_ref)
         else
           PDK.logger.error clone_result[:stdout]
           PDK.logger.error clone_result[:stderr]
@@ -316,6 +319,28 @@ def self.clone_template_repo(origin_repo)
 
         PDK::Util.canonical_path(temp_dir)
       end
+
+      # @api private
+      def checkout_template_ref(path, ref)
+        if PDK::Util::Git.work_dir_clean?(path)
+          Dir.chdir(path) do
+            full_ref = PDK::Util::Git.ls_remote(path, ref)
+            cache_template_ref(path, full_ref)
+            reset_result = PDK::Util::Git.git('reset', '--hard', full_ref)
+            return if reset_result[:exit_code].zero?
+
+            PDK.logger.error reset_result[:stdout]
+            PDK.logger.error reset_result[:stderr]
+            raise PDK::CLI::FatalError, _("Unable to set HEAD of git repository at '%{repo}' to ref:'%{ref}'.") % { repo: path, ref: ref }
+          end
+        else
+          PDK.logger.warn _("Uncommitted changes found when attempting to set HEAD of git repository at '%{repo}' to ref '%{ref}'; skipping git reset.") % { repo: path, ref: ref }
+        end
+      end
+
+      def cache_template_ref(path, ref = nil)
+        @template_ref ||= PDK::Util::Git.describe(File.join(path, '.git'), ref)
+      end
     end
   end
 end