(todo) working?

lib/pdk/generate/module.rb

@@ -88,7 +88,7 @@ def self.invoke(opts = {})
               end
             end
 
-            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 _('Module \'%{name}\' generated at path \'%{path}\', from template \'%{url}\'.') % { name: opts[:module_name], path: target_dir, url: template_uri.bare_uri }
             PDK.logger.info(_('In your module directory, add classes with the \'pdk new class\' command.'))
           end
         rescue Errno::EACCES => e

lib/pdk/module/templatedir.rb

@@ -6,6 +6,11 @@ class TemplateDir
       attr_accessor :module_metadata
       attr_reader :uri
 
+      attr_reader :template_dir_type
+
+      TEMPLATE_DIR_FILESYSTEM_TYPE ||= :filesystem
+      TEMPLATE_DIR_GIT_TYPE ||= :git
+
       # Initialises the TemplateDir object with the path or URL to the template
       # and the block of code to run to be run while the template is available.
       #
@@ -45,32 +50,37 @@ def initialize(uri, module_metadata = {}, init = false)
           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?(uri.git_remote)
+        if PDK::Util::Git.repo?(uri.bare_uri)
           # This is either a bare local repo or a remote. either way it needs cloning.
+          # A "remote" can also be git repo on the local filsystem.
           @path = clone_template_repo(uri)
           temp_dir_clone = true
+          @template_dir_type = TEMPLATE_DIR_GIT_TYPE
         else
-          # if it is a local path & non-bare repo then we can use it directly.
+          # if it is not git repository a local path & non-bare repo then we can use it directly.
           # Still have to check the branch.
           @path = uri.shell_path
+          @template_dir_type = TEMPLATE_DIR_FILESYSTEM_TYPE
+          # TODO: This whole thing makes no sense. It's not a git repo therefore we never do a checkout!??!?!
+          #
           # 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
+          # if PDK::Util::Git.work_tree?(@path)
+          #   PDK.logger.warn _("Repository '%{repo}' has a work-tree; skipping git reset.") % {
+          #     repo: @path,
+          #   }
+          # end
         end
         @uri = uri
 
         @init = init
-        @moduleroot_dir = File.join(@path, 'moduleroot')
-        @moduleroot_init = File.join(@path, 'moduleroot_init')
+        @moduleroot_dir = self.class.moduleroot_dir(@path)
+        @moduleroot_init = self.class.moduleroot_init(@path)
         @dirs = [@moduleroot_dir]
         @dirs << @moduleroot_init if @init
         @object_dir = File.join(@path, 'object_templates')
 
-        validate_module_template!
+        self.class.validate_module_template!(@path)
 
         @module_metadata = module_metadata
 
@@ -198,6 +208,14 @@ def object_config
         config_for(nil)
       end
 
+      def self.moduleroot_dir(template_root_dir)
+        File.join(template_root_dir, 'moduleroot')
+      end
+
+      def self.moduleroot_init(template_root_dir)
+        File.join(template_root_dir, 'moduleroot_init')
+      end
+
       # Validate the content of the template directory.
       #
       # @raise [ArgumentError] If the specified path is not a directory.
@@ -206,27 +224,27 @@ def object_config
       #
       # @return [void]
       #
-      # @api private
-      def validate_module_template!
+      # @api public
+      def self.validate_module_template!(template_root_dir)
         # rubocop:disable Style/GuardClause
-        unless PDK::Util::Filesystem.directory?(@path)
+        unless PDK::Util::Filesystem.directory?(template_root_dir)
           require 'pdk/util'
 
-          if PDK::Util.package_install? && PDK::Util::Filesystem.fnmatch?(File.join(PDK::Util.package_cachedir, '*'), @path)
+          if PDK::Util.package_install? && PDK::Util::Filesystem.fnmatch?(File.join(PDK::Util.package_cachedir, '*'), template_root_dir)
             raise ArgumentError, _('The built-in template has substantially changed. Please run "pdk convert" on your module to continue.')
           else
-            raise ArgumentError, _("The specified template '%{path}' is not a directory.") % { path: @path }
+            raise ArgumentError, _("The specified template '%{path}' is not a directory.") % { path: template_root_dir }
           end
         end
 
-        unless PDK::Util::Filesystem.directory?(@moduleroot_dir)
-          raise ArgumentError, _("The template at '%{path}' does not contain a 'moduleroot/' directory.") % { path: @path }
+        unless PDK::Util::Filesystem.directory?(moduleroot_dir(template_root_dir))
+          raise ArgumentError, _("The template at '%{path}' does not contain a 'moduleroot/' directory.") % { path: template_root_dir }
         end
 
-        unless PDK::Util::Filesystem.directory?(@moduleroot_init)
+        unless PDK::Util::Filesystem.directory?(moduleroot_init(template_root_dir))
           # rubocop:disable Metrics/LineLength
-          raise ArgumentError, _("The template at '%{path}' does not contain a 'moduleroot_init/' directory, which indicates you are using an older style of template. Before continuing please use the --template-url flag when running the pdk new commands to pass a new style template.") % { path: @path }
-          # rubocop:enable Metrics/LineLength Style/GuardClause
+          raise ArgumentError, _("The template at '%{path}' does not contain a 'moduleroot_init/' directory, which indicates you are using an older style of template. Before continuing please use the --template-url flag when running the pdk new commands to pass a new style template.") % { path: template_root_dir }
+          # rubocop:enable Metrics/LineLength
         end
         # rubocop:enable Style/GuardClause
       end
@@ -344,8 +362,8 @@ def clone_template_repo(uri)
         require 'pdk/util/git'
 
         temp_dir = PDK::Util.make_tmpdir_name('pdk-templates')
-        origin_repo = uri.git_remote
-        git_ref = uri.git_ref
+        origin_repo = uri.bare_uri
+        git_ref = uri.uri_fragment
 
         clone_result = PDK::Util::Git.git('clone', origin_repo, temp_dir)
 
@@ -381,6 +399,8 @@ def checkout_template_ref(path, ref)
       end
 
       def cache_template_ref(path, ref = nil)
+        # Filesystem type template directories have no concept of a ref.
+        return nil if @template_dir_type == TEMPLATE_DIR_FILESYSTEM_TYPE
         require 'pdk/util/git'
 
         @template_ref ||= PDK::Util::Git.describe(File.join(path, '.git'), ref)

lib/pdk/module/update.rb

@@ -6,7 +6,7 @@ class Update < Convert
       GIT_DESCRIBE_PATTERN = %r{\A(?<base>.+?)-(?<additional_commits>\d+)-g(?<sha>.+)\Z}
 
       def run
-        template_uri.git_ref = new_template_version
+        template_uri.uri_fragment = new_template_version
 
         stage_changes!
 
@@ -71,10 +71,10 @@ def new_version
       def new_template_version
         return options[:'template-ref'] if options[:'template-ref']
 
-        if template_uri.default? && template_uri.ref_is_tag? && PDK::Util.package_install?
+        if template_uri.default? && PDK::Util::Git.tag?(template_uri.bare_uri, template_uri.uri_fragment) && PDK::Util.package_install?
           PDK::Util::TemplateURI.default_template_ref
         else
-          template_uri.git_ref
+          template_uri.uri_fragment
         end
       end
 
@@ -101,7 +101,7 @@ def fetch_remote_version(template_ref)
         return template_ref if template_ref == PDK::TEMPLATE_REF
 
         sha_length = GIT_DESCRIBE_PATTERN.match(current_template_version)[:sha].length - 1
-        "#{template_ref}@#{PDK::Util::Git.ls_remote(template_uri.git_remote, template_ref)[0..sha_length]}"
+        "#{template_ref}@#{PDK::Util::Git.ls_remote(template_uri.bare_uri, template_ref)[0..sha_length]}"
       end
 
       def update_message
@@ -113,7 +113,7 @@ def update_message
 
         format_string % {
           module_name:     module_metadata.data['name'],
-          template_url:    template_uri.git_remote,
+          template_url:    template_uri.bare_uri,
           current_version: current_version,
           new_version:     new_version,
         }

lib/pdk/util/git.rb

@@ -128,6 +128,10 @@ def self.describe(path, ref = nil)
         result[:stdout].strip
       end
 
+      def self.tag?(git_remote, tag_name)
+        git('ls-remote', '--tags', '--exit-code', git_remote, tag_name)[:exit_code].zero?
+      end
+
       # Clears any cached information for git queries
       # Should only be used during testing
       # @api private

lib/pdk/util/template_uri.rb

@@ -35,15 +35,14 @@ class TemplateURI
       #
       def initialize(opts_or_uri)
         require 'addressable'
-
         # If a uri string is passed, skip the valid uri finding code.
         @uri = if opts_or_uri.is_a?(self.class)
                  opts_or_uri.uri
                elsif opts_or_uri.is_a?(String)
                  begin
                    uri, ref = opts_or_uri.split('#', 2)
-                   if self.class.packaged_template?(uri)
-                     self.class.default_template_uri(ref).uri
+                   if PDK::Util::TemplateURI.packaged_template?(uri)
+                     PDK::Util::TemplateURI.default_template_addressable_uri.tap { |default| default.fragment = ref unless ref.nil? || ref.empty? }
                    else
                      Addressable::URI.parse(opts_or_uri)
                    end
@@ -53,36 +52,58 @@ def initialize(opts_or_uri)
                elsif opts_or_uri.is_a?(Addressable::URI)
                  opts_or_uri.dup
                else
-                 self.class.first_valid_uri(self.class.templates(opts_or_uri))
+                 PDK::Util::TemplateURI.first_valid_uri(PDK::Util::TemplateURI.templates(opts_or_uri))
                end
       end
 
       def ==(other)
         @uri == other.uri
       end
 
+      def bare_uri
+        PDK::Util::TemplateURI.bare_uri(@uri)
+      end
+
       # This is the URI represented in a format suitable for writing to
       # metadata.
       #
       # @returns String
       def metadata_format
-        if self.class.packaged_template?(git_remote)
-          self.class.human_readable("pdk-default##{git_ref}")
-        else
-          self.class.human_readable(@uri.to_s)
-        end
+        @metadata_format ||= if PDK::Util::TemplateURI.packaged_template?(bare_uri)
+                               PDK::Util::TemplateURI.human_readable("pdk-default##{uri_fragment}")
+                             else
+                               PDK::Util::TemplateURI.human_readable(@uri.to_s)
+                             end
       end
       alias to_s metadata_format
       alias to_str metadata_format
 
-      # This is the url without a fragment, suitable for git clones.
-      #
+      # Returns the fragment of the URI, of the default template's ref if one does not exist
       # @returns String
-      def git_remote
-        self.class.git_remote(@uri)
+      # @api private
+      def uri_fragment
+        @uri.fragment || self.class.default_template_ref(self)
+      end
+
+      def uri_fragment=(fragment)
+        @uri.fragment = fragment
+      end
+
+      def default?
+        bare_uri == PDK::Util::TemplateURI.bare_uri(PDK::Util::TemplateURI.default_template_addressable_uri)
       end
 
-      def self.git_remote(uri)
+      # def ref_is_tag?
+      #   require 'pdk/util/git'
+
+      #   PDK::Util::Git.git('ls-remote', '--tags', '--exit-code', git_remote, git_ref)[:exit_code].zero?
+      # end
+
+      # Class Methods
+
+      # Remove the fragment off of URI. Useful for removing the branch
+      # for Git based URIs
+      def self.bare_uri(uri)
         require 'addressable'
 
         if uri.is_a?(Addressable::URI) && uri.fragment
@@ -98,35 +119,34 @@ def shell_path
         self.class.human_readable(@uri.path)
       end
 
-      # @returns String
-      def git_ref
-        @uri.fragment || self.class.default_template_ref(self)
-      end
+      # # @returns String
+      # def git_ref
+      #   @uri.fragment || self.class.default_template_ref(self)
+      # end
 
-      def git_ref=(ref)
-        @uri.fragment = ref
-      end
+      # def git_ref=(ref)
+      #   @uri.fragment = ref
+      # end
 
       # @returns PDK::Util::TemplateURI
-      def self.default_template_uri(ref = nil)
+      def self.default_template_uri
         require 'pdk/util'
         require 'addressable'
 
-        if PDK::Util.package_install?
-          PDK::Util::TemplateURI.new(Addressable::URI.new(scheme: 'file', host: '', path: File.join(PDK::Util.package_cachedir, 'pdk-templates.git'), fragment: ref))
-        else
-          PDK::Util::TemplateURI.new(Addressable::URI.new(scheme: 'https', host: 'github.com', path: '/puppetlabs/pdk-templates', fragment: ref))
-        end
-      end
-
-      def default?
-        git_remote == self.class.default_template_uri.git_remote
+        PDK::Util::TemplateURI.new(default_template_addressable_uri)
       end
 
-      def ref_is_tag?
-        require 'pdk/util/git'
+      # @returns Addressable::URI
+      # @api private
+      def self.default_template_addressable_uri
+        require 'pdk/util'
+        require 'addressable'
 
-        PDK::Util::Git.git('ls-remote', '--tags', '--exit-code', git_remote, git_ref)[:exit_code].zero?
+        if PDK::Util.package_install?
+          Addressable::URI.new(scheme: 'file', host: '', path: File.join(PDK::Util.package_cachedir, 'pdk-templates.git'))
+        else
+          Addressable::URI.new(scheme: 'https', host: 'github.com', path: '/puppetlabs/pdk-templates')
+        end
       end
 
       # `C:...` urls are not URI-safe. They should be of the form `/C:...` to
@@ -254,12 +274,13 @@ def self.valid_template?(template)
         return false if template.nil? || !template.is_a?(Hash)
         return false if template[:uri].nil? || !template[:uri].is_a?(Addressable::URI)
 
-        return true if PDK::Util::Git.repo?(git_remote(template[:uri]))
+        return true if PDK::Util::Git.repo?(bare_uri(template[:uri]))
 
         path = human_readable(template[:uri].path)
         if PDK::Util::Filesystem.directory?(path)
+          # We know that it's not a git repository, but it's a valid path on disk
           begin
-            PDK::Module::TemplateDir.new(path) {}
+            PDK::Module::TemplateDir.validate_module_template!(path)
             return true
           rescue ArgumentError
             nil

spec/unit/pdk/module/template_dir_spec.rb

@@ -152,7 +152,7 @@
       allow_any_instance_of(described_class).to receive(:clone_template_repo).and_return(path)
       allow(PDK::Util::Git).to receive(:repo?).with(anything).and_return(true)
       allow(PDK::Util::Filesystem).to receive(:rm_rf).with(path)
-      allow_any_instance_of(described_class).to receive(:validate_module_template!)
+      allow(described_class).to receive(:validate_module_template!)
       allow(PDK::Util::Git).to receive(:describe).and_return('git-ref')
       # rubocop:enable RSpec/AnyInstance
     end