From a49d5b885a62aa9bd3a686d411739723a67c399c Mon Sep 17 00:00:00 2001 From: Michael Stahnke Date: Wed, 22 Jun 2011 14:57:03 -0700 Subject: (#8048) Gem install puppet no longer fails if rdoc enabled. Rdoc wouldn't parse lib/puppet/interface/options.rb The offending code has been removed. This was causing issues for users wishing to upgrade puppet, via gem or puppet. Signed-off-by: Michael Stahnke --- lib/puppet/interface/option.rb | 2 -- 1 file changed, 2 deletions(-) (limited to 'lib/puppet/interface') diff --git a/lib/puppet/interface/option.rb b/lib/puppet/interface/option.rb index b68bdeb12..3cd930acf 100644 --- a/lib/puppet/interface/option.rb +++ b/lib/puppet/interface/option.rb @@ -2,8 +2,6 @@ require 'puppet/interface' class Puppet::Interface::Option include Puppet::Interface::TinyDocs - # For compatibility, deprecated, and should go fairly soon... - ['', '='].each { |x| alias :"desc#{x}" :"description#{x}" } def initialize(parent, *declaration, &block) @parent = parent -- cgit From ae3ef423c03b7ef27f975dadfb67bf77ca481503 Mon Sep 17 00:00:00 2001 From: Dan Bode Date: Wed, 6 Jul 2011 21:59:05 -0700 Subject: (#7699) - Help should only show options once puppet help was reprinting every option once for every alias that is had. This fix involves only storing the option.name in the @options instance var for both face and actions options. The @options_hash still maintains the list of options and aliases as its keys. Reviewed-by: Daniel Pittman (puppet-dev list) --- lib/puppet/interface/action.rb | 3 ++- lib/puppet/interface/option_manager.rb | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) (limited to 'lib/puppet/interface') diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb index 185302b07..fe77a9658 100644 --- a/lib/puppet/interface/action.rb +++ b/lib/puppet/interface/action.rb @@ -227,8 +227,9 @@ WRAPPER end end + @options << option.name + option.aliases.each do |name| - @options << name @options_hash[name] = option end diff --git a/lib/puppet/interface/option_manager.rb b/lib/puppet/interface/option_manager.rb index 326a91d92..a1f300e8e 100644 --- a/lib/puppet/interface/option_manager.rb +++ b/lib/puppet/interface/option_manager.rb @@ -26,8 +26,9 @@ module Puppet::Interface::OptionManager end end + @options << option.name + option.aliases.each do |name| - @options << name @options_hash[name] = option end -- cgit From 1feccc3f2db29d112308a55032e7f93ca44b45aa Mon Sep 17 00:00:00 2001 From: Jeff McCune Date: Sun, 10 Jul 2011 13:51:31 -0700 Subject: Revert "Merge branch 'ticket/2.7.x/7699_fix_help_listing_options_twice' into 2.7.x" This reverts commit b7ee0258ab40478329c20177eda9b250f27ede18, reversing changes made to 8fe2e555ac3d57f5b6503ffe1a5466db8d6e190a. --- lib/puppet/interface/action.rb | 3 +-- lib/puppet/interface/option_manager.rb | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) (limited to 'lib/puppet/interface') diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb index fe77a9658..185302b07 100644 --- a/lib/puppet/interface/action.rb +++ b/lib/puppet/interface/action.rb @@ -227,9 +227,8 @@ WRAPPER end end - @options << option.name - option.aliases.each do |name| + @options << name @options_hash[name] = option end diff --git a/lib/puppet/interface/option_manager.rb b/lib/puppet/interface/option_manager.rb index a1f300e8e..326a91d92 100644 --- a/lib/puppet/interface/option_manager.rb +++ b/lib/puppet/interface/option_manager.rb @@ -26,9 +26,8 @@ module Puppet::Interface::OptionManager end end - @options << option.name - option.aliases.each do |name| + @options << name @options_hash[name] = option end -- cgit From b82f29c61aa84a12fc208487e4b049cb24702261 Mon Sep 17 00:00:00 2001 From: Dan Bode Date: Wed, 13 Jul 2011 15:38:32 -0700 Subject: (#7699) Help command should only list options once The problem was caused by the fact that the options method returns a list of options that treated the aliases as seperate options. The fix is to only maintain a list of options and not add all aliases to the options list. --- lib/puppet/interface/action.rb | 3 ++- lib/puppet/interface/option_manager.rb | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) (limited to 'lib/puppet/interface') diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb index 185302b07..fe77a9658 100644 --- a/lib/puppet/interface/action.rb +++ b/lib/puppet/interface/action.rb @@ -227,8 +227,9 @@ WRAPPER end end + @options << option.name + option.aliases.each do |name| - @options << name @options_hash[name] = option end diff --git a/lib/puppet/interface/option_manager.rb b/lib/puppet/interface/option_manager.rb index 326a91d92..a1f300e8e 100644 --- a/lib/puppet/interface/option_manager.rb +++ b/lib/puppet/interface/option_manager.rb @@ -26,8 +26,9 @@ module Puppet::Interface::OptionManager end end + @options << option.name + option.aliases.each do |name| - @options << name @options_hash[name] = option end -- cgit From 72abe6ce7192bba0b295a8a83483668d21050624 Mon Sep 17 00:00:00 2001 From: Pieter van de Bruggen Date: Tue, 19 Jul 2011 10:55:26 -0700 Subject: (#7204) Consolidate Semantic Versioning code. This introduces a class representing a semantic version, and implementing a few of the most common uses of them: validation, comparison, and finding the greatest available version matching a range. This refactoring also allows us to easily expand our matching of version ranges in the future, which is a big plus. Reviewed-By: Daniel Pittman --- lib/puppet/interface/face_collection.rb | 49 ++++----------------------------- 1 file changed, 5 insertions(+), 44 deletions(-) (limited to 'lib/puppet/interface') diff --git a/lib/puppet/interface/face_collection.rb b/lib/puppet/interface/face_collection.rb index 12d3c56b1..4522824fd 100644 --- a/lib/puppet/interface/face_collection.rb +++ b/lib/puppet/interface/face_collection.rb @@ -1,8 +1,6 @@ require 'puppet/interface' module Puppet::Interface::FaceCollection - SEMVER_VERSION = /^(\d+)\.(\d+)\.(\d+)([A-Za-z][0-9A-Za-z-]*|)$/ - @faces = Hash.new { |hash, key| hash[key] = {} } def self.faces @@ -17,55 +15,18 @@ module Puppet::Interface::FaceCollection @faces.keys.select {|name| @faces[name].length > 0 } end - def self.validate_version(version) - !!(SEMVER_VERSION =~ version.to_s) - end - - def self.semver_to_array(v) - parts = SEMVER_VERSION.match(v).to_a[1..4] - parts[0..2] = parts[0..2].map { |e| e.to_i } - parts - end - - def self.cmp_semver(a, b) - a, b = [a, b].map do |x| semver_to_array(x) end - - cmp = a[0..2] <=> b[0..2] - if cmp == 0 - cmp = a[3] <=> b[3] - cmp = +1 if a[3].empty? && !b[3].empty? - cmp = -1 if b[3].empty? && !a[3].empty? - end - cmp - end - - def self.prefix_match?(desired, target) - # Can't meaningfully do a prefix match with current on either side. - return false if desired == :current - return false if target == :current - - # REVISIT: Should probably fail if the matcher is not valid. - prefix = desired.split('.').map {|x| x =~ /^\d+$/ and x.to_i } - have = semver_to_array(target) - - while want = prefix.shift do - return false unless want == have.shift - end - return true - end - def self.[](name, version) name = underscorize(name) get_face(name, version) or load_face(name, version) end # get face from memory, without loading. - def self.get_face(name, desired_version) + def self.get_face(name, pattern) return nil unless @faces.has_key? name + return @faces[name][:current] if pattern == :current - return @faces[name][:current] if desired_version == :current - - found = @faces[name].keys.select {|v| prefix_match?(desired_version, v) }.sort.last + versions = @faces[name].keys - [ :current ] + found = SemVer.find_matching(pattern, versions) return @faces[name][found] end @@ -108,7 +69,7 @@ module Puppet::Interface::FaceCollection # versions here and return the last item in that set. # # --daniel 2011-04-06 - latest_ver = @faces[name].keys.sort {|a, b| cmp_semver(a, b) }.last + latest_ver = @faces[name].keys.sort.last @faces[name][:current] = @faces[name][latest_ver] end rescue LoadError => e -- cgit From b75b1c19ecf6c278b065d203ac8486fa598caa8b Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Wed, 20 Jul 2011 11:58:55 -0700 Subject: (#6787) Add `default_to` for options. This implement support for options with default values, allowing faces to set those values when not invoked. This can eliminate substantial duplicate code from actions, especially when there are face-level options in use. Reviewed-By: Pieter van de Bruggen --- lib/puppet/interface/action.rb | 9 +++++++++ lib/puppet/interface/option.rb | 19 +++++++++++++++++++ lib/puppet/interface/option_builder.rb | 13 +++++++++++++ 3 files changed, 41 insertions(+) (limited to 'lib/puppet/interface') diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb index fe77a9658..fc1121eb6 100644 --- a/lib/puppet/interface/action.rb +++ b/lib/puppet/interface/action.rb @@ -199,6 +199,7 @@ def #{@name}(#{decl.join(", ")}) options = args.last action = get_action(#{name.inspect}) + action.add_default_args(args) action.validate_args(args) __invoke_decorations(:before, action, args, options) rval = self.__send__(#{internal_name.inspect}, *args) @@ -252,6 +253,14 @@ WRAPPER option end + def add_default_args(args) + options.map {|x| get_option(x) }.each do |option| + if option.has_default? and not option.aliases.any? {|x| args.last.has_key? x} + args.last[option.name] = option.default + end + end + end + def validate_args(args) # Check for multiple aliases for the same option... args.last.keys.each do |name| diff --git a/lib/puppet/interface/option.rb b/lib/puppet/interface/option.rb index 3cd930acf..01f6f2307 100644 --- a/lib/puppet/interface/option.rb +++ b/lib/puppet/interface/option.rb @@ -6,6 +6,7 @@ class Puppet::Interface::Option def initialize(parent, *declaration, &block) @parent = parent @optparse = [] + @default = nil # Collect and sort the arguments in the declaration. dups = {} @@ -81,8 +82,26 @@ class Puppet::Interface::Option !!@required end + def has_default? + !!@default + end + + def default=(proc) + required and raise ArgumentError, "#{self} can't be optional and have a default value" + proc.is_a? Proc or raise ArgumentError, "default value for #{self} is a #{proc.class.name.inspect}, not a proc" + @default = proc + end + + def default + @default and @default.call + end + attr_reader :parent, :name, :aliases, :optparse attr_accessor :required + def required=(value) + has_default? and raise ArgumentError, "#{self} can't be optional and have a default value" + @required = value + end attr_accessor :before_action def before_action=(proc) diff --git a/lib/puppet/interface/option_builder.rb b/lib/puppet/interface/option_builder.rb index 5676ec977..c87adc2c0 100644 --- a/lib/puppet/interface/option_builder.rb +++ b/lib/puppet/interface/option_builder.rb @@ -51,4 +51,17 @@ class Puppet::Interface::OptionBuilder def required(value = true) @option.required = value end + + def default_to(&block) + block or raise ArgumentError, "#{@option} default_to requires a block" + if @option.has_default? + raise ArgumentError, "#{@option} already has a default value" + end + # Ruby 1.8 treats a block without arguments as accepting any number; 1.9 + # gets this right, so we work around it for now... --daniel 2011-07-20 + unless block.arity == 0 or (RUBY_VERSION =~ /^1\.8/ and block.arity == -1) + raise ArgumentError, "#{@option} default_to block should not take any arguments" + end + @option.default = block + end end -- cgit From fd6a653cb32cb03e339655862c526fd5dccbfcf0 Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Wed, 20 Jul 2011 12:35:22 -0700 Subject: (#7123) Support runtime setting of 'default' on actions. Given the inheritance model for actions, we are sometimes going to need to set them to 'default' at runtime, rather than during their static declaration. Add tests to verify that this works correctly, and update the code to ensure that happens. This gives up caching of the default action, but this should be an extremely rare operation - pretty much only CLI invocation, really. Reviewed-By: Pieter van de Bruggen --- lib/puppet/interface/action_manager.rb | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) (limited to 'lib/puppet/interface') diff --git a/lib/puppet/interface/action_manager.rb b/lib/puppet/interface/action_manager.rb index fbf588d7d..5c9af4f96 100644 --- a/lib/puppet/interface/action_manager.rb +++ b/lib/puppet/interface/action_manager.rb @@ -7,13 +7,14 @@ module Puppet::Interface::ActionManager require 'puppet/interface/action_builder' @actions ||= {} - @default_action ||= nil raise "Action #{name} already defined for #{self}" if action?(name) + action = Puppet::Interface::ActionBuilder.build(self, name, &block) - if action.default - raise "Actions #{@default_action.name} and #{name} cannot both be default" if @default_action - @default_action = action + + if action.default and current = get_default_action + raise "Actions #{current.name} and #{name} cannot both be default" end + @actions[action.name] = action end @@ -61,7 +62,11 @@ module Puppet::Interface::ActionManager end def get_default_action - @default_action + default = actions.map {|x| get_action(x) }.select {|x| x.default } + if default.length > 1 + raise "The actions #{default.map(&:name).join(", ")} cannot all be default" + end + default.first end def action?(name) -- cgit From 1e0655e6bdbc872014abdffa5deacb334616e826 Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Thu, 21 Jul 2011 15:34:52 -0700 Subject: (#7184) Centralize "find action for face" into Puppet::Face As part of moving to load actions first, and their associated face, when invoked from the command line, it makes sense to push the logic for finding the action and face down into the Puppet::Face implementation. This means that we can change the logic there without needing to update the public part of the CLI implementation, and that any further facades can use the same, correct, logic to locate the action for the face. Reviewed-By: Pieter van de Bruggen --- lib/puppet/interface/action.rb | 1 + lib/puppet/interface/face_collection.rb | 11 +++++++++++ 2 files changed, 12 insertions(+) (limited to 'lib/puppet/interface') diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb index fc1121eb6..ce9c60b49 100644 --- a/lib/puppet/interface/action.rb +++ b/lib/puppet/interface/action.rb @@ -38,6 +38,7 @@ class Puppet::Interface::Action def to_s() "#{@face}##{@name}" end attr_reader :name + attr_reader :face attr_accessor :default def default? !!@default diff --git a/lib/puppet/interface/face_collection.rb b/lib/puppet/interface/face_collection.rb index 4522824fd..ddc66f583 100644 --- a/lib/puppet/interface/face_collection.rb +++ b/lib/puppet/interface/face_collection.rb @@ -20,6 +20,17 @@ module Puppet::Interface::FaceCollection get_face(name, version) or load_face(name, version) end + def self.get_action_for_face(face_name, action_name, version) + # If the version they request specifically doesn't exist, don't search + # elsewhere. Usually this will start from :current and all... + return nil unless face = self[face_name, version] + unless action = face.get_action(action_name) + # ...we need to search for it bound to an o{lder,ther} version. + end + + return action + end + # get face from memory, without loading. def self.get_face(name, pattern) return nil unless @faces.has_key? name -- cgit From 2cd3bc47993fbd32a77ca9dfdd51353f2dfbcb58 Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Thu, 21 Jul 2011 16:34:20 -0700 Subject: (#7184) Find actions bound to other versions of Faces. When we first touch a Face, we load all the available Actions from disk. Given they define themselves against a specific version of a Face, they are automatically available tied to the correct version; this makes it trivially possible to locate those on demand and return them. Now, we have the ability to find and, consequently, invoke Actions on older versions of Faces. We don't load enough context, though: the older face will only have external Actions defined, not anything core. Reviewed-By: Pieter van de Bruggen --- lib/puppet/interface/face_collection.rb | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) (limited to 'lib/puppet/interface') diff --git a/lib/puppet/interface/face_collection.rb b/lib/puppet/interface/face_collection.rb index ddc66f583..868997b67 100644 --- a/lib/puppet/interface/face_collection.rb +++ b/lib/puppet/interface/face_collection.rb @@ -20,12 +20,19 @@ module Puppet::Interface::FaceCollection get_face(name, version) or load_face(name, version) end - def self.get_action_for_face(face_name, action_name, version) + def self.get_action_for_face(name, action_name, version) + name = underscorize(name) + # If the version they request specifically doesn't exist, don't search # elsewhere. Usually this will start from :current and all... - return nil unless face = self[face_name, version] + return nil unless face = self[name, version] unless action = face.get_action(action_name) - # ...we need to search for it bound to an o{lder,ther} version. + # ...we need to search for it bound to an o{lder,ther} version. Since + # we load all actions when the face is first references, this will be in + # memory in the known set of versions of the face. + (@faces[name].keys - [ :current ]).sort.reverse.each do |version| + break if action = @faces[name][version].get_action(action_name) + end end return action -- cgit From 532c4f37e4f8289cf4a9871ebc0cb5086c2ba26a Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Thu, 21 Jul 2011 16:43:16 -0700 Subject: (#7184) Load the core of obsolete versions of Faces. When we define an action on an older version of a Face, we must be sure to directly load the core of that version, not just define it with the external Action(s) that it had. Otherwise we break our contract, which is that any core Actions for a specific version will be available to your external Action for as long as we support that core version. Reviewed-By: Pieter van de Bruggen --- lib/puppet/interface/face_collection.rb | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) (limited to 'lib/puppet/interface') diff --git a/lib/puppet/interface/face_collection.rb b/lib/puppet/interface/face_collection.rb index 868997b67..b1f6ba398 100644 --- a/lib/puppet/interface/face_collection.rb +++ b/lib/puppet/interface/face_collection.rb @@ -56,9 +56,7 @@ module Puppet::Interface::FaceCollection # # We use require to avoid executing the code multiple times, like any # other Ruby library that we might want to use. --daniel 2011-04-06 - begin - require "puppet/face/#{name}" - + if safely_require name then # If we wanted :current, we need to index to find that; direct version # requests just work™ as they go. --daniel 2011-04-06 if version == :current then @@ -90,18 +88,32 @@ module Puppet::Interface::FaceCollection latest_ver = @faces[name].keys.sort.last @faces[name][:current] = @faces[name][latest_ver] end - rescue LoadError => e - raise unless e.message =~ %r{-- puppet/face/#{name}$} - # ...guess we didn't find the file; return a much better problem. - rescue SyntaxError => e - raise unless e.message =~ %r{puppet/face/#{name}\.rb:\d+: } - Puppet.err "Failed to load face #{name}:\n#{e}" - # ...but we just carry on after complaining. + end + + unless version == :current or get_face(name, version) + # Try an obsolete version of the face, if needed, to see if that helps? + safely_require name, version end return get_face(name, version) end + def self.safely_require(name, version = nil) + path = File.join 'puppet' ,'face', version.to_s, name.to_s + require path + true + + rescue LoadError => e + raise unless e.message =~ %r{-- #{path}$} + # ...guess we didn't find the file; return a much better problem. + nil + rescue SyntaxError => e + raise unless e.message =~ %r{#{path}\.rb:\d+: } + Puppet.err "Failed to load face #{name}:\n#{e}" + # ...but we just carry on after complaining. + nil + end + def self.register(face) @faces[underscorize(face.name)][face.version] = face end -- cgit From 77441be2299bbb96ab9f048855b0fd4c16eb7b5a Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Fri, 22 Jul 2011 14:32:00 -0700 Subject: (#8561) Unify validation and modification of action arguments. Rather than having multiple, separate operations that modify and validate the arguments to an action, a single pass makes sense. This also means less walks across the set of data, and a few less expensive method calls in Ruby. Additionally, we work on a duplicate of the arguments hash rather than directly modifying the original. Because everything we do is at the top level key/value mapping, this is sufficient to isolate the original. While mostly theoretical, we now don't mutilate the hash passed in, so the user won't get nastily surprised by the fact that we could have done so. Reviewed-By: Pieter van de Bruggen --- lib/puppet/interface/action.rb | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) (limited to 'lib/puppet/interface') diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb index ce9c60b49..2a2f48e03 100644 --- a/lib/puppet/interface/action.rb +++ b/lib/puppet/interface/action.rb @@ -200,8 +200,7 @@ def #{@name}(#{decl.join(", ")}) options = args.last action = get_action(#{name.inspect}) - action.add_default_args(args) - action.validate_args(args) + args = action.validate_and_clean(args) __invoke_decorations(:before, action, args, options) rval = self.__send__(#{internal_name.inspect}, *args) __invoke_decorations(:after, action, args, options) @@ -254,15 +253,19 @@ WRAPPER option end - def add_default_args(args) + def validate_and_clean(original_args) + # Make a shallow copy, so as not to surprise callers. We only modify the + # top level keys and all, so this should be sufficient without being too + # costly overall. + args = original_args.dup + + # Inject default arguments. options.map {|x| get_option(x) }.each do |option| if option.has_default? and not option.aliases.any? {|x| args.last.has_key? x} args.last[option.name] = option.default end end - end - def validate_args(args) # Check for multiple aliases for the same option... args.last.keys.each do |name| # #7290: If this isn't actually an option, ignore it for now. We should @@ -281,8 +284,12 @@ WRAPPER get_option(name) end.select(&:required?).collect(&:name) - args.last.keys - return if required.empty? - raise ArgumentError, "The following options are required: #{required.join(', ')}" + unless required.empty? + raise ArgumentError, "The following options are required: #{required.join(', ')}" + end + + # All done. + return args end ######################################################################## -- cgit From 82e5fa9561e2d4cb1d699a41c14f50953d8f2d97 Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Fri, 22 Jul 2011 15:15:38 -0700 Subject: (#8561, #7290) Implement the option contract fully. Rewrite the process of validating and updating the options to fully reflect the contract - we fail if there are unknown options passed, report nicely errors of duplicate names, pass only the canonical names to the action code, and generally enforce nicely. Reviewed-By: Pieter van de Bruggen --- lib/puppet/interface/action.rb | 84 ++++++++++++++++++++++++------------------ 1 file changed, 49 insertions(+), 35 deletions(-) (limited to 'lib/puppet/interface') diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb index 2a2f48e03..bd47a36ea 100644 --- a/lib/puppet/interface/action.rb +++ b/lib/puppet/interface/action.rb @@ -196,14 +196,12 @@ class Puppet::Interface::Action wrapper = <