From 557767b164376d0e61875646715d3ec96401b96c Mon Sep 17 00:00:00 2001 From: Matt Robinson Date: Tue, 19 Apr 2011 12:53:16 -0700 Subject: maint: Remove unused faces code Looks like in renaming faces to face we just missed some files. They got copied, not moved. Paired-with: Max Martin --- lib/puppet/faces/help/action.erb | 3 --- lib/puppet/faces/help/face.erb | 7 ------- lib/puppet/faces/help/global.erb | 20 -------------------- 3 files changed, 30 deletions(-) delete mode 100644 lib/puppet/faces/help/action.erb delete mode 100644 lib/puppet/faces/help/face.erb delete mode 100644 lib/puppet/faces/help/global.erb (limited to 'lib') diff --git a/lib/puppet/faces/help/action.erb b/lib/puppet/faces/help/action.erb deleted file mode 100644 index eaf131464..000000000 --- a/lib/puppet/faces/help/action.erb +++ /dev/null @@ -1,3 +0,0 @@ -Use: puppet <%= face.name %> [options] <%= action.name %> [options] - -Summary: <%= action.summary %> diff --git a/lib/puppet/faces/help/face.erb b/lib/puppet/faces/help/face.erb deleted file mode 100644 index efe5fd809..000000000 --- a/lib/puppet/faces/help/face.erb +++ /dev/null @@ -1,7 +0,0 @@ -Use: puppet <%= face.name %> [options] [options] - -Available actions: -% face.actions.each do |actionname| -% action = face.get_action(actionname) - <%= action.name.to_s.ljust(16) %> <%= action.summary %> -% end diff --git a/lib/puppet/faces/help/global.erb b/lib/puppet/faces/help/global.erb deleted file mode 100644 index e123367a2..000000000 --- a/lib/puppet/faces/help/global.erb +++ /dev/null @@ -1,20 +0,0 @@ -puppet [options] [options] - -Available subcommands, from Puppet Faces: -% Puppet::Faces.faces.sort.each do |name| -% face = Puppet::Faces[name, :current] - <%= face.name.to_s.ljust(16) %> <%= face.summary %> -% end - -% unless legacy_applications.empty? then # great victory when this is true! -Available applications, soon to be ported to Faces: -% legacy_applications.each do |appname| -% summary = horribly_extract_summary_from appname - <%= appname.to_s.ljust(16) %> <%= summary %> -% end -% end - -See 'puppet help ' for help on a specific subcommand action. -See 'puppet help ' for help on a specific subcommand. -See 'puppet man ' for the full man page. -Puppet v<%= Puppet::PUPPETVERSION %> -- cgit From c87d6c98ec1a315d435be38e7eb2258984c7d88c Mon Sep 17 00:00:00 2001 From: James Turnbull Date: Wed, 20 Apr 2011 02:35:40 +1000 Subject: Fixed #7166 - Replaced deprecated stomp "send" method with "publish" The "send" method in the stomp gem has been deprecated since: http://gitorious.org/stomp/mainline/commit/d542a976028cb4c5badcbb69e3383e746721e44c It's been replaced with the "publish" method. Also renamed the send_message method to publish_message more in keeping with language used in queuing. --- lib/puppet/indirector/queue.rb | 2 +- lib/puppet/util/queue.rb | 2 +- lib/puppet/util/queue/stomp.rb | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) (limited to 'lib') diff --git a/lib/puppet/indirector/queue.rb b/lib/puppet/indirector/queue.rb index fd089f431..85ffacacc 100644 --- a/lib/puppet/indirector/queue.rb +++ b/lib/puppet/indirector/queue.rb @@ -36,7 +36,7 @@ class Puppet::Indirector::Queue < Puppet::Indirector::Terminus def save(request) result = nil benchmark :info, "Queued #{indirection.name} for #{request.key}" do - result = client.send_message(queue, request.instance.render(:pson)) + result = client.publish_message(queue, request.instance.render(:pson)) end result rescue => detail diff --git a/lib/puppet/util/queue.rb b/lib/puppet/util/queue.rb index 02357742a..636bdcf2e 100644 --- a/lib/puppet/util/queue.rb +++ b/lib/puppet/util/queue.rb @@ -30,7 +30,7 @@ require 'puppet/util/instance_loader' # # The client plugins are expected to implement an interface similar to that of Stomp::Client: # * new should return a connected, ready-to-go client instance. Note that no arguments are passed in. -# * send_message(queue, message) should send the _message_ to the specified _queue_. +# * publish_message(queue, message) should publish the _message_ to the specified _queue_. # * subscribe(queue) _block_ subscribes to _queue_ and executes _block_ upon receiving a message. # * _queue_ names are simple names independent of the message broker or client library. No "/queue/" prefixes like in Stomp::Client. module Puppet::Util::Queue diff --git a/lib/puppet/util/queue/stomp.rb b/lib/puppet/util/queue/stomp.rb index c18edae6a..cabc56627 100644 --- a/lib/puppet/util/queue/stomp.rb +++ b/lib/puppet/util/queue/stomp.rb @@ -28,8 +28,8 @@ class Puppet::Util::Queue::Stomp end end - def send_message(target, msg) - stomp_client.send(stompify_target(target), msg, :persistent => true) + def publish_message(target, msg) + stomp_client.publish(stompify_target(target), msg, :persistent => true) end def subscribe(target) -- cgit From 33b5580ef6b6c851beb6852e56659afea8bb0b04 Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Tue, 19 Apr 2011 18:22:03 -0700 Subject: maint: move method comments outside the comment. The comment discussing the purpose of the wrapper and related details rightly belongs outside the method; move it there so it doesn't perturb the functional changes that follow. Reviewed-By: Max Martin --- lib/puppet/interface/action.rb | 39 +++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 20 deletions(-) (limited to 'lib') diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb index 23366b407..e644d8999 100644 --- a/lib/puppet/interface/action.rb +++ b/lib/puppet/interface/action.rb @@ -129,27 +129,26 @@ class Puppet::Interface::Action # @face.send(name, *args, &block) # end + + # We need to build an instance method as a wrapper, using normal code, to be + # able to expose argument defaulting between the caller and definer in the + # Ruby API. An extra method is, sadly, required for Ruby 1.8 to work since + # it doesn't expose bind on a block. + # + # Hopefully we can improve this when we finally shuffle off the last of Ruby + # 1.8 support, but that looks to be a few "enterprise" release eras away, so + # we are pretty stuck with this for now. + # + # Patches to make this work more nicely with Ruby 1.9 using runtime version + # checking and all are welcome, provided that they don't change anything + # outside this little ol' bit of code and all. + # + # Incidentally, we though about vendoring evil-ruby and actually adjusting + # the internal C structure implementation details under the hood to make + # this stuff work, because it would have been cleaner. Which gives you an + # idea how motivated we were to make this cleaner. Sorry. + # --daniel 2011-03-31 def when_invoked=(block) - # We need to build an instance method as a wrapper, using normal code, to - # be able to expose argument defaulting between the caller and definer in - # the Ruby API. An extra method is, sadly, required for Ruby 1.8 to work. - # - # In future this also gives us a place to hook in additional behaviour - # such as calling out to the action instance to validate and coerce - # parameters, which avoids any exciting context switching and all. - # - # Hopefully we can improve this when we finally shuffle off the last of - # Ruby 1.8 support, but that looks to be a few "enterprise" release eras - # away, so we are pretty stuck with this for now. - # - # Patches to make this work more nicely with Ruby 1.9 using runtime - # version checking and all are welcome, but they can't actually help if - # the results are not totally hidden away in here. - # - # Incidentally, we though about vendoring evil-ruby and actually adjusting - # the internal C structure implementation details under the hood to make - # this stuff work, because it would have been cleaner. Which gives you an - # idea how motivated we were to make this cleaner. Sorry. --daniel 2011-03-31 internal_name = "#{@name} implementation, required on Ruby 1.8".to_sym file = __FILE__ + "+eval" -- cgit From 5d7ef5caf30a0c5b3253340c5f2722e51c56c75e Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Tue, 19 Apr 2011 20:23:27 -0700 Subject: (#7062) better argument handling in the action wrapper methods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We previously used *args to collect all arguments to the action when_invoked block, then tried vaguely to massage some little bits of them into the right shape. Methods defined with blocks, in Ruby 1.8, also have some fun behaviours. The most special is that if you pass more than one argument to a block defined with only one Ruby will automatically coerce the arguments into an array – and this is preserved when it is bound to a method. This led to routine situations where we would pass the wrong number of arguments to the block because, say, the user gave an extra argument on the command line. Instead of failing this would transmogrify the arguments in counterintuitive ways, and end up with horrible stack traces when that interacted badly with the code as written. Now, instead, we work out the right argument format based on the arguments that the when_invoked block takes. This gives much better (albeit perhaps not so user friendly) behaviour at the interface level. Which is, at least, consistent with other Ruby API. Reviewed-By: Max Martin --- lib/puppet/interface/action.rb | 38 ++++++++++++++++++++++++++++++-------- 1 file changed, 30 insertions(+), 8 deletions(-) (limited to 'lib') diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb index e644d8999..08bc0a345 100644 --- a/lib/puppet/interface/action.rb +++ b/lib/puppet/interface/action.rb @@ -151,15 +151,37 @@ class Puppet::Interface::Action def when_invoked=(block) internal_name = "#{@name} implementation, required on Ruby 1.8".to_sym - file = __FILE__ + "+eval" - line = __LINE__ + 1 + + arity = block.arity + if arity == 0 then + # This will never fire on 1.8.7, which treats no arguments as "*args", + # but will on 1.9.2, which treats it as "no arguments". Which bites, + # because this just begs for us to wind up in the horrible situation + # where a 1.8 vs 1.9 error bites our end users. --daniel 2011-04-19 + raise ArgumentError, "action when_invoked requires at least one argument (options)" + elsif arity > 0 then + range = Range.new(1, arity - 1) + decl = range.map { |x| "arg#{x}" } << "options = {}" + optn = "" + args = "[" + (range.map { |x| "arg#{x}" } << "options").join(", ") + "]" + else + range = Range.new(1, arity.abs - 1) + decl = range.map { |x| "arg#{x}" } << "*rest" + optn = "rest << {} unless rest.last.is_a?(Hash)" + if arity == -1 then + args = "rest" + else + args = "[" + range.map { |x| "arg#{x}" }.join(", ") + "] + rest" + end + end + + file = __FILE__ + "+eval[wrapper]" + line = __LINE__ + 2 # <== points to the same line as 'def' in the wrapper. wrapper = < Date: Tue, 19 Apr 2011 20:52:52 -0700 Subject: maint: fix gratuitous whitespace in the code. We had some stray spacing between variables and the '=' sign from when there was another variable in place; it got deleted, but the code wasn't closed up. Reviewed-By: Max Martin --- lib/puppet/application/face_base.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/puppet/application/face_base.rb b/lib/puppet/application/face_base.rb index 9da48af55..fabe71896 100644 --- a/lib/puppet/application/face_base.rb +++ b/lib/puppet/application/face_base.rb @@ -92,8 +92,8 @@ class Puppet::Application::FaceBase < Puppet::Application # REVISIT: These should be configurable versions, through a global # '--version' option, but we don't implement that yet... --daniel 2011-03-29 - @type = self.class.name.to_s.sub(/.+:/, '').downcase.to_sym - @face = Puppet::Face[@type, :current] + @type = self.class.name.to_s.sub(/.+:/, '').downcase.to_sym + @face = Puppet::Face[@type, :current] # Now, walk the command line and identify the action. We skip over # arguments based on introspecting the action and all, and find the first -- cgit From 379b46d4b9e2a57f954ff178956ca6850c3c56f7 Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Tue, 19 Apr 2011 21:08:05 -0700 Subject: (#7116) Handle application-level options in parse_options We hid another layer of per-application option in the class backing the application, which wasn't correctly handled in the parse_options method. They are now found and handled, so that global flags like --debug work as expected on the left of the action, not just the right. Reviewed-By: Max Martin --- lib/puppet/application/face_base.rb | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) (limited to 'lib') diff --git a/lib/puppet/application/face_base.rb b/lib/puppet/application/face_base.rb index fabe71896..7bebd18bb 100644 --- a/lib/puppet/application/face_base.rb +++ b/lib/puppet/application/face_base.rb @@ -122,6 +122,8 @@ class Puppet::Application::FaceBase < Puppet::Application # a mandatory argument. --daniel 2011-04-05 index += 1 # ...so skip the argument. end + elsif option = find_application_argument(item) then + index += 1 if (option[:argument] and option[:optional]) else raise OptionParser::InvalidOption.new(item.sub(/=.*$/, '')) end @@ -158,6 +160,21 @@ class Puppet::Application::FaceBase < Puppet::Application return nil # nothing found. end + def find_application_argument(item) + self.class.option_parser_commands.each do |options, function| + options.each do |option| + next unless option =~ /^-/ + pattern = /^#{option.sub('[no-]', '').sub(/[ =].*$/, '')}(?:[ =].*)?$/ + next unless pattern.match(item) + return { + :argument => option =~ /[ =]/, + :optional => option =~ /[ =]\[/ + } + end + end + return nil # not found + end + def setup Puppet::Util::Log.newdestination :console -- cgit From eeb82361de00f86f0840c2fcdd30a5e84c49232d Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Wed, 20 Apr 2011 16:56:45 -0700 Subject: maint: better error report for a missing version of a face. We would report this: Could not find version 1.0.0 of Puppet::Face[:version_matching, "2.0.0"] That is not actually so helpful, not least because people wonder why it reports a version number they didn't ask for. Instead, we just report the requested name now. --- lib/puppet/interface.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/puppet/interface.rb b/lib/puppet/interface.rb index 5c8ade749..b48f963ec 100644 --- a/lib/puppet/interface.rb +++ b/lib/puppet/interface.rb @@ -53,7 +53,7 @@ class Puppet::Interface def [](name, version) unless face = Puppet::Interface::FaceCollection[name, version] if current = Puppet::Interface::FaceCollection[name, :current] - raise Puppet::Error, "Could not find version #{version} of #{current}" + raise Puppet::Error, "Could not find version #{version} of #{name}" else raise Puppet::Error, "Could not find Puppet Face #{name.inspect}" end -- cgit From 03bd5595365b7d541d20ed614e9c8be6a7cba9c9 Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Thu, 21 Apr 2011 10:38:25 -0700 Subject: maint: more robust listing of valid faces. We used to treat anything with a top level key in the faces hash as a valid face; it makes more sense to filter that only to things that have at least one implementation. Previously we had to be super-careful not to accidentally touch the top level for an invalid face, which set us up for future failure when someone wasn't careful enough; now we can cope with that. Paired-With: Max Martin --- lib/puppet/interface/face_collection.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/puppet/interface/face_collection.rb b/lib/puppet/interface/face_collection.rb index 591471d4b..49e007ec0 100644 --- a/lib/puppet/interface/face_collection.rb +++ b/lib/puppet/interface/face_collection.rb @@ -24,7 +24,7 @@ module Puppet::Interface::FaceCollection end end end - return @faces.keys + return @faces.keys.select {|name| @faces[name].length > 0 } end def self.validate_version(version) -- cgit From f17f6bba87519db888854acf7017ddff61f635be Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Wed, 20 Apr 2011 15:51:39 -0700 Subject: (#7183) Implement "invisible glob" version matching for faces "Invisible glob", or "prefix", version matching means that when you specify a version string to use you can specify as little as one version number out of the semantic versioning spec. Matching is done on the prefix; an omitted number is treated as "anything" in that slot, and we return the highest matching versioned face by that spec. For example, given the set of versions: 1.0.0, 1.0.1, 1.1.0, 1.1.1, 2.0.0 The following would be matched: input matched 1 1.1.1 1.0 1.0.1 1.0.1 1.0.1 1.0.2 fail - no match 1.1 1.1.1 1.1.1 1.1.1 1.2 fail - no match --- lib/puppet/interface.rb | 13 ++++---- lib/puppet/interface/face_collection.rb | 58 ++++++++++++++++++++------------- 2 files changed, 41 insertions(+), 30 deletions(-) (limited to 'lib') diff --git a/lib/puppet/interface.rb b/lib/puppet/interface.rb index b48f963ec..ced00863d 100644 --- a/lib/puppet/interface.rb +++ b/lib/puppet/interface.rb @@ -26,18 +26,13 @@ class Puppet::Interface Puppet::Interface::FaceCollection.faces end - def face?(name, version) - Puppet::Interface::FaceCollection.face?(name, version) - end - def register(instance) Puppet::Interface::FaceCollection.register(instance) end def define(name, version, &block) - if face?(name, version) - face = Puppet::Interface::FaceCollection[name, version] - else + face = Puppet::Interface::FaceCollection[name, version] + if face.nil? then face = self.new(name, version) Puppet::Interface::FaceCollection.register(face) # REVISIT: Shouldn't this be delayed until *after* we evaluate the @@ -50,6 +45,10 @@ class Puppet::Interface return face end + def face?(name, version) + Puppet::Interface::FaceCollection[name, version] + end + def [](name, version) unless face = Puppet::Interface::FaceCollection[name, version] if current = Puppet::Interface::FaceCollection[name, :current] diff --git a/lib/puppet/interface/face_collection.rb b/lib/puppet/interface/face_collection.rb index 49e007ec0..6e6afc545 100644 --- a/lib/puppet/interface/face_collection.rb +++ b/lib/puppet/interface/face_collection.rb @@ -31,12 +31,14 @@ module Puppet::Interface::FaceCollection !!(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| - parts = SEMVER_VERSION.match(x).to_a[1..4] - parts[0..2] = parts[0..2].map { |e| e.to_i } - parts - end + a, b = [a, b].map do |x| semver_to_array(x) end cmp = a[0..2] <=> b[0..2] if cmp == 0 @@ -47,18 +49,38 @@ module Puppet::Interface::FaceCollection cmp end - def self.[](name, version) - @faces[underscorize(name)][version] if face?(name, version) + 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.face?(name, version) + def self.[](name, version) name = underscorize(name) + get_face(name, version) or load_face(name, version) + end - # Note: be careful not to accidentally create the top level key, either, - # because it will result in confusion when people try to enumerate the - # list of valid faces later. --daniel 2011-04-11 - return true if @faces.has_key?(name) and @faces[name].has_key?(version) + # get face from memory, without loading. + def self.get_face(name, desired_version) + return nil unless @faces.has_key? name + return @faces[name][:current] if desired_version == :current + + found = @faces[name].keys.select {|v| prefix_match?(desired_version, v) }.sort.last + return @faces[name][found] + end + + # try to load the face, and return it. + def self.load_face(name, version) # We always load the current version file; the common case is that we have # the expected version and any compatibility versions in the same file, # the default. Which means that this is almost always the case. @@ -104,17 +126,7 @@ module Puppet::Interface::FaceCollection # ...guess we didn't find the file; return a much better problem. end - # Now, either we have the version in our set of faces, or we didn't find - # the version they were looking for. In the future we will support - # loading versioned stuff from some look-aside part of the Ruby load path, - # but we don't need that right now. - # - # So, this comment is a place-holder for that. --daniel 2011-04-06 - # - # Note: be careful not to accidentally create the top level key, either, - # because it will result in confusion when people try to enumerate the - # list of valid faces later. --daniel 2011-04-11 - return !! (@faces.has_key?(name) and @faces[name].has_key?(version)) + return get_face(name, version) end def self.register(face) -- cgit From a0de3288bad113c9be1190095b03e892e17000d2 Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Thu, 21 Apr 2011 14:46:36 -0700 Subject: (#6928) backport Symbol#to_proc for Ruby < 1.8.7 We use the &:foo symbol-to-proc syntax in some of our code, so to avoid problems on Ruby earlier than 1.8.7 we should backport the support in our monkey-patch file. Reviewed-By: Max Martin --- lib/puppet/util/monkey_patches.rb | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'lib') diff --git a/lib/puppet/util/monkey_patches.rb b/lib/puppet/util/monkey_patches.rb index 10a268409..9ae0ca6a2 100644 --- a/lib/puppet/util/monkey_patches.rb +++ b/lib/puppet/util/monkey_patches.rb @@ -104,3 +104,12 @@ class Array end end unless method_defined? :combination end + + +if Symbol.instance_method(:to_proc).nil? + class Symbol + def to_proc + Proc.new { |*args| args.shift.__send__(self, *args) } + end + end +end -- cgit From de2199f3666ca9e26a0a36ec17b176300a4fa599 Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Thu, 21 Apr 2011 15:01:38 -0700 Subject: (#6928) Don't blow up when the method is undefined... Use the same model for testing instance methods as the rest of the code. Reviewed-By: Max Martin --- lib/puppet/util/monkey_patches.rb | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) (limited to 'lib') diff --git a/lib/puppet/util/monkey_patches.rb b/lib/puppet/util/monkey_patches.rb index 9ae0ca6a2..bd954c665 100644 --- a/lib/puppet/util/monkey_patches.rb +++ b/lib/puppet/util/monkey_patches.rb @@ -106,10 +106,8 @@ class Array end -if Symbol.instance_method(:to_proc).nil? - class Symbol - def to_proc - Proc.new { |*args| args.shift.__send__(self, *args) } - end - end +class Symbol + def to_proc + Proc.new { |*args| args.shift.__send__(self, *args) } + end unless method_defined? :to_proc end -- cgit