From e33236b8b74a3d281dbffedd774c4f4974f21b44 Mon Sep 17 00:00:00 2001 From: Glenn Sarti Date: Wed, 13 Nov 2019 15:31:56 +0800 Subject: [PATCH] (todo) working? --- lib/pdk/generate/module.rb | 2 +- lib/pdk/module/templatedir.rb | 64 ++++++++++------ lib/pdk/module/update.rb | 10 +-- lib/pdk/util/git.rb | 4 + lib/pdk/util/template_uri.rb | 93 ++++++++++++++--------- spec/unit/pdk/module/template_dir_spec.rb | 2 +- spec/unit/pdk/module/update_spec.rb | 18 +++-- spec/unit/pdk/util/template_uri_spec.rb | 32 +++++--- 8 files changed, 144 insertions(+), 81 deletions(-) diff --git a/lib/pdk/generate/module.rb b/lib/pdk/generate/module.rb index a570d4173..c8bf4987b 100644 --- a/lib/pdk/generate/module.rb +++ b/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 diff --git a/lib/pdk/module/templatedir.rb b/lib/pdk/module/templatedir.rb index 568d79396..4c775bc21 100644 --- a/lib/pdk/module/templatedir.rb +++ b/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) diff --git a/lib/pdk/module/update.rb b/lib/pdk/module/update.rb index 52fbbc4c2..b97915eb8 100644 --- a/lib/pdk/module/update.rb +++ b/lib/pdk/module/update.rb @@ -6,7 +6,7 @@ class Update < Convert GIT_DESCRIBE_PATTERN = %r{\A(?.+?)-(?\d+)-g(?.+)\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, } diff --git a/lib/pdk/util/git.rb b/lib/pdk/util/git.rb index 1f4c2136c..0320243ae 100644 --- a/lib/pdk/util/git.rb +++ b/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 diff --git a/lib/pdk/util/template_uri.rb b/lib/pdk/util/template_uri.rb index bf73fed37..0dca66fe2 100644 --- a/lib/pdk/util/template_uri.rb +++ b/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,7 +52,7 @@ 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 @@ -61,28 +60,50 @@ 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 diff --git a/spec/unit/pdk/module/template_dir_spec.rb b/spec/unit/pdk/module/template_dir_spec.rb index 732fe63e9..d19c7fe71 100644 --- a/spec/unit/pdk/module/template_dir_spec.rb +++ b/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 diff --git a/spec/unit/pdk/module/update_spec.rb b/spec/unit/pdk/module/update_spec.rb index eb2812bfa..037adb47c 100644 --- a/spec/unit/pdk/module/update_spec.rb +++ b/spec/unit/pdk/module/update_spec.rb @@ -57,7 +57,7 @@ allow(instance).to receive(:print_summary) allow(instance).to receive(:new_version).and_return('1.4.0') allow(instance).to receive(:print_result) - allow(instance.template_uri).to receive(:ref_is_tag?).and_return(true) + allow(PDK::Util::Git).to receive(:tag?).with(String, String).and_return(true) allow(instance.update_manager).to receive(:sync_changes!) allow(instance.update_manager).to receive(:changes?).and_return(changes) end @@ -272,8 +272,16 @@ include_context 'with mock metadata' - let(:module_template_uri) { instance_double(PDK::Util::TemplateURI, default?: true, ref_is_tag?: false, git_ref: '0.0.1') } - let(:template_url) { 'https://github.com/puppetlabs/pdk-templates#0.0.1' } + let(:module_template_ref) { '0.0.1' } + let(:module_template_uri) do + instance_double( + PDK::Util::TemplateURI, + default?: true, + bare_uri: 'https://github.com/puppetlabs/pdk-templates', + uri_fragment: module_template_ref, + ) + end + let(:template_url) { "https://github.com/puppetlabs/pdk-templates##{module_template_ref}" } before(:each) do allow(PDK::Util::TemplateURI).to receive(:new).and_call_original @@ -296,7 +304,7 @@ context 'and the ref of the template is a tag' do before(:each) do - allow(module_template_uri).to receive(:ref_is_tag?).and_return(true) + allow(PDK::Util::Git).to receive(:tag?).with(String, module_template_ref).and_return(true) end context 'and PDK is running from a package install' do @@ -324,7 +332,7 @@ context 'but the ref of the template is not a tag' do before(:each) do - allow(module_template_uri).to receive(:ref_is_tag?).and_return(false) + allow(PDK::Util::Git).to receive(:tag?).with(String, module_template_ref).and_return(false) end it 'returns the ref from the metadata' do diff --git a/spec/unit/pdk/util/template_uri_spec.rb b/spec/unit/pdk/util/template_uri_spec.rb index 245f64de2..c9fa54b1e 100644 --- a/spec/unit/pdk/util/template_uri_spec.rb +++ b/spec/unit/pdk/util/template_uri_spec.rb @@ -1,5 +1,6 @@ require 'spec_helper' require 'pdk/util/template_uri' +require 'addressable' describe PDK::Util::TemplateURI do subject(:template_uri) do @@ -8,6 +9,7 @@ before(:each) do PDK.answers.update!('template-url' => nil) + # described_class.clear_cached_values end describe '.new' do @@ -179,12 +181,12 @@ end end - describe '.git_remote' do + describe '.bare_uri' do context 'when the uri has a fragment' do let(:opts_or_uri) { 'https://github.com/my/pdk-templates.git#custom' } it 'returns just the url portion' do - expect(template_uri.git_remote).to eq 'https://github.com/my/pdk-templates.git' + expect(template_uri.bare_uri).to eq 'https://github.com/my/pdk-templates.git' end end @@ -192,7 +194,7 @@ let(:opts_or_uri) { 'https://github.com/my/pdk-templates.git' } it 'returns just the url portion' do - expect(template_uri.git_remote).to eq 'https://github.com/my/pdk-templates.git' + expect(template_uri.bare_uri).to eq 'https://github.com/my/pdk-templates.git' end end @@ -202,7 +204,7 @@ it 'returns url portion' do allow(Gem).to receive(:win_platform?).and_return(false) - expect(template_uri.git_remote).to eq '/my/pdk-templates.git' + expect(template_uri.bare_uri).to eq '/my/pdk-templates.git' end end @@ -211,18 +213,18 @@ it 'returns url portion' do allow(Gem).to receive(:win_platform?).and_return(true) - expect(template_uri.git_remote).to eq 'C:/my/pdk-templates.git' + expect(template_uri.bare_uri).to eq 'C:/my/pdk-templates.git' end end end end - describe '.git_ref' do + describe '.uri_fragment' do context 'when the uri has a fragment' do let(:opts_or_uri) { 'https://github.com/my/pdk-templates.git#custom' } - it 'returns just the ref portion' do - expect(template_uri.git_ref).to eq 'custom' + it 'returns just the fragment portion' do + expect(template_uri.uri_fragment).to eq 'custom' end end @@ -230,7 +232,7 @@ let(:opts_or_uri) { 'https://github.com/my/pdk-templates.git' } it 'returns the default ref' do - expect(template_uri.git_ref).to eq(described_class.default_template_ref(opts_or_uri)) + expect(template_uri.uri_fragment).to eq(described_class.default_template_ref(opts_or_uri)) end end end @@ -500,11 +502,12 @@ context 'that is a pdk-default keyword' do let(:metadata_url) { 'pdk-default#master' } + let(:expected_uri) { described_class.default_template_addressable_uri.tap { |obj| obj.fragment = 'master' } } - it 'converts the keyword to the defalut template' do + it 'converts the keyword to the default template' do is_expected.to include( type: 'metadata.json', - uri: described_class.default_template_uri('master').uri, + uri: expected_uri, allow_fallback: true, ) end @@ -575,6 +578,11 @@ allow(PDK::Util::Git).to receive(:repo?).with('/path/to/a/template').and_return(false) end + def allow_template_dir(root, valid) + allow(PDK::Util::Filesystem).to receive(:directory?).with("#{root}/moduleroot").and_return(valid) + allow(PDK::Util::Filesystem).to receive(:directory?).with("#{root}/moduleroot_init").and_return(valid) + end + context 'but does point to a directory' do before(:each) do allow(PDK::Util::Filesystem).to receive(:directory?).with('/path/to/a/template').and_return(true) @@ -582,6 +590,7 @@ context 'that contains a valid template' do before(:each) do + allow_template_dir('/path/to/a/template', true) allow(PDK::Module::TemplateDir).to receive(:new).with('/path/to/a/template').and_yield end @@ -590,6 +599,7 @@ context 'that does not contain a valid template' do before(:each) do + allow_template_dir('/path/to/a/template', false) allow(PDK::Module::TemplateDir).to receive(:new).with('/path/to/a/template').and_raise(ArgumentError) end