From 63e2e56d3172bdc80aaca5f5ddde5811728e3c76 Mon Sep 17 00:00:00 2001 From: Jesse Wolfe Date: Fri, 30 Apr 2010 14:54:07 -0700 Subject: [PATCH] feature #2276 Single Executable: subcommand method Extract the logic to determine the subcommand name into a method. Signed-off-by: Jesse Wolfe --- bin/puppet | 12 +++---- lib/puppet/application.rb | 2 +- lib/puppet/application/apply.rb | 8 ++--- lib/puppet/application/cert.rb | 2 +- lib/puppet/application/describe.rb | 2 +- lib/puppet/application/doc.rb | 4 +-- lib/puppet/application/filebucket.rb | 15 ++++++--- lib/puppet/application/resource.rb | 7 ++-- lib/puppet/defaults.rb | 15 ++------- lib/puppet/reference/providers.rb | 4 +-- lib/puppet/util/command_line.rb | 45 +++++++++++++++++++++---- spec/unit/application/apply.rb | 5 ++- spec/unit/application/cert.rb | 10 +++--- spec/unit/application/describe.rb | 4 +-- spec/unit/application/doc.rb | 11 ++---- spec/unit/application/filebucket.rb | 10 +++--- spec/unit/application/resource.rb | 35 ++++--------------- spec/unit/util/command_line.rb | 50 +++++++++++++++++++--------- test/other/puppet.rb | 9 +---- 19 files changed, 130 insertions(+), 120 deletions(-) diff --git a/bin/puppet b/bin/puppet index 5e619de6e..a326ba148 100755 --- a/bin/puppet +++ b/bin/puppet @@ -77,13 +77,13 @@ usage = "Usage: puppet command " available = "Available commands are: #{builtins.sort.join(', ')}" require 'puppet/util/command_line' -$puppet_subcommand_name = Puppet::Util::CommandLine.shift_subcommand_from_argv +subcommand_name = Puppet::Util::CommandLine.subcommand_name -if $puppet_subcommand_name.nil? +if subcommand_name.nil? puts usage, available -elsif builtins.include?($puppet_subcommand_name) #subcommand - require File.join(appdir, $puppet_subcommand_name) - Puppet::Application[$puppet_subcommand_name].run +elsif builtins.include?(subcommand_name) #subcommand + require File.join(appdir, subcommand_name) + Puppet::Application[subcommand_name].run else - abort "Error: Unknown command #{$puppet_subcommand_name}.\n#{usage}\n#{available}" + abort "Error: Unknown command #{subcommand_name}.\n#{usage}\n#{available}" end diff --git a/lib/puppet/application.rb b/lib/puppet/application.rb index 1245b1006..7b404a906 100644 --- a/lib/puppet/application.rb +++ b/lib/puppet/application.rb @@ -25,7 +25,7 @@ require 'optparse' # # # dispatch is called to know to what command to call # dispatch do -# ARGV.shift +# Puppet::Util::CommandLine.args.shift # end # # option("--arg ARGUMENT") do |v| diff --git a/lib/puppet/application/apply.rb b/lib/puppet/application/apply.rb index 787ce375f..d977cf1d0 100644 --- a/lib/puppet/application/apply.rb +++ b/lib/puppet/application/apply.rb @@ -66,10 +66,10 @@ Puppet::Application.new(:apply) do command(:parseonly) do # Set our code or file to use. - if options[:code] or ARGV.length == 0 + if options[:code] or Puppet::Util::CommandLine.args.length == 0 Puppet[:code] = options[:code] || STDIN.read else - Puppet[:manifest] = ARGV.shift + Puppet[:manifest] = Puppet::Util::CommandLine.args.shift end begin Puppet::Resource::TypeCollection.new(Puppet[:environment]).perform_initial_import @@ -82,10 +82,10 @@ Puppet::Application.new(:apply) do command(:main) do # Set our code or file to use. - if options[:code] or ARGV.length == 0 + if options[:code] or Puppet::Util::CommandLine.args.length == 0 Puppet[:code] = options[:code] || STDIN.read else - Puppet[:manifest] = ARGV.shift + Puppet[:manifest] = Puppet::Util::CommandLine.args.shift end # Collect our facts. diff --git a/lib/puppet/application/cert.rb b/lib/puppet/application/cert.rb index f48e5301a..7a7784b09 100644 --- a/lib/puppet/application/cert.rb +++ b/lib/puppet/application/cert.rb @@ -44,7 +44,7 @@ Puppet::Application.new(:cert) do if @all hosts = :all else - hosts = ARGV.collect { |h| puts h; h.downcase } + hosts = Puppet::Util::CommandLine.args.collect { |h| puts h; h.downcase } end begin @ca.apply(:revoke, :to => hosts) if @mode == :destroy diff --git a/lib/puppet/application/describe.rb b/lib/puppet/application/describe.rb index d3d335496..ea4ac162c 100644 --- a/lib/puppet/application/describe.rb +++ b/lib/puppet/application/describe.rb @@ -202,7 +202,7 @@ Puppet::Application.new(:describe,"#{$0} [options] [type]") do end setup do - options[:types] = ARGV.dup + options[:types] = Puppet::Util::CommandLine.args.dup unless options[:list] || options[:types].size > 0 handle_help(nil) end diff --git a/lib/puppet/application/doc.rb b/lib/puppet/application/doc.rb index 32f9ba75c..0a11d6045 100644 --- a/lib/puppet/application/doc.rb +++ b/lib/puppet/application/doc.rb @@ -70,7 +70,7 @@ Puppet::Application.new(:doc) do files += env.modulepath files << File.dirname(env[:manifest]) end - files += ARGV + files += Puppet::Util::CommandLine.args Puppet.info "scanning: %s" % files.inspect Puppet.settings.setdefaults("puppetdoc", "document_all" => [false, "Document all resources"] @@ -167,7 +167,7 @@ Puppet::Application.new(:doc) do setup do # sole manifest documentation - if ARGV.size > 0 + if Puppet::Util::CommandLine.args.size > 0 options[:mode] = :rdoc @manifest = true end diff --git a/lib/puppet/application/filebucket.rb b/lib/puppet/application/filebucket.rb index ed67009aa..cd7c854af 100644 --- a/lib/puppet/application/filebucket.rb +++ b/lib/puppet/application/filebucket.rb @@ -12,18 +12,23 @@ Puppet::Application.new(:filebucket) do option("--remote","-r") option("--verbose","-v") + class << self + attr :args + end + dispatch do - ARGV.shift + @args = Puppet::Util::CommandLine.args + args.shift end command(:get) do - md5 = ARGV.shift + md5 = args.shift out = @client.getfile(md5) print out end command(:backup) do - ARGV.each do |file| + args.each do |file| unless FileTest.exists?(file) $stderr.puts "%s: no such file" % file next @@ -38,8 +43,8 @@ Puppet::Application.new(:filebucket) do end command(:restore) do - file = ARGV.shift - md5 = ARGV.shift + file = args.shift + md5 = args.shift @client.restore(file, md5) end diff --git a/lib/puppet/application/resource.rb b/lib/puppet/application/resource.rb index ae4349850..78aed95c5 100644 --- a/lib/puppet/application/resource.rb +++ b/lib/puppet/application/resource.rb @@ -38,11 +38,12 @@ Puppet::Application.new(:resource) do end command(:main) do - type = ARGV.shift or raise "You must specify the type to display" + args = Puppet::Util::CommandLine.args + type = args.shift or raise "You must specify the type to display" typeobj = Puppet::Type.type(type) or raise "Could not find type #{type}" - name = ARGV.shift + name = args.shift params = {} - ARGV.each do |setting| + args.each do |setting| if setting =~ /^(\w+)=(.+)$/ params[$1] = $2 else diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb index a9cb48419..66f038307 100644 --- a/lib/puppet/defaults.rb +++ b/lib/puppet/defaults.rb @@ -5,19 +5,8 @@ module Puppet conf = nil var = nil - legacy_name = Hash.new{ |h,k| k }.update({ - 'agent' => 'puppetd', - 'cert' => 'puppetca', - 'doc' => 'puppetdoc', - 'filebucket' => 'filebucket', - 'apply' => 'puppet', - 'describe' => 'pi', - 'queue' => 'puppetqd', - 'resource' => 'ralsh', - 'kick' => 'puppetrun', - 'master' => 'puppetmasterd', - }) - name = legacy_name[ $puppet_subcommand_name ] || $0.gsub(/.+#{File::SEPARATOR}/,'').sub(/\.rb$/, '') + require 'puppet/util/command_line' + name = Puppet::Util::CommandLine.legacy_executable_name # Make File.expand_path happy require 'etc' diff --git a/lib/puppet/reference/providers.rb b/lib/puppet/reference/providers.rb index d425d803e..df02178e3 100644 --- a/lib/puppet/reference/providers.rb +++ b/lib/puppet/reference/providers.rb @@ -8,8 +8,8 @@ providers = Puppet::Util::Reference.newreference :providers, :title => "Provider end types.sort! { |a,b| a.name.to_s <=> b.name.to_s } - unless ARGV.empty? - types.reject! { |type| ! ARGV.include?(type.name.to_s) } + unless Puppet::Util::CommandLine.args.empty? + types.reject! { |type| ! Puppet::Util::CommandLine.args.include?(type.name.to_s) } end ret = "Details about this host:\n\n" diff --git a/lib/puppet/util/command_line.rb b/lib/puppet/util/command_line.rb index f231ee7f8..5fe07b28d 100644 --- a/lib/puppet/util/command_line.rb +++ b/lib/puppet/util/command_line.rb @@ -1,12 +1,45 @@ module Puppet module Util module CommandLine - def self.shift_subcommand_from_argv( argv = ARGV, stdin = STDIN ) - case argv.first - when nil; "apply" unless stdin.tty? # ttys get usage info - when "--help"; nil # help should give you usage, not the help for `puppet apply` - when /^-|\.pp$|\.rb$/; "apply" - else argv.shift + def self.subcommand_name(*args) + subcommand_name, args = subcommand_and_args(*args) + return subcommand_name + end + + def self.args(*args) + subcommand_name, args = subcommand_and_args(*args) + return args + end + + LegacyName = Hash.new{|h,k| k}.update({ + 'agent' => 'puppetd', + 'cert' => 'puppetca', + 'doc' => 'puppetdoc', + 'filebucket' => 'filebucket', + 'apply' => 'puppet', + 'describe' => 'pi', + 'queue' => 'puppetqd', + 'resource' => 'ralsh', + 'kick' => 'puppetrun', + 'master' => 'puppetmasterd', + }) + + def self.legacy_executable_name(*args) + LegacyName[ subcommand_name(*args) ] + end + + def self.subcommand_and_args( zero = $0, argv = ARGV, stdin = STDIN ) + zero = zero.gsub(/.*#{File::SEPARATOR}/,'').sub(/\.rb$/, '') + + if zero == 'puppet' + case argv.first + when nil; [ stdin.tty? ? nil : "apply", argv] # ttys get usage info + when "--help"; [nil, argv] # help should give you usage, not the help for `puppet apply` + when /^-|\.pp$|\.rb$/; ["apply", argv] + else [ argv.first, argv[1..-1] ] + end + else + [ zero, argv ] end end end diff --git a/spec/unit/application/apply.rb b/spec/unit/application/apply.rb index e9a1c72a0..6fdeebfcb 100755 --- a/spec/unit/application/apply.rb +++ b/spec/unit/application/apply.rb @@ -201,7 +201,7 @@ describe "Puppet" do end it "should set the code to run from STDIN if no arguments" do - ARGV.stubs(:length).returns(0) + Puppet::Util::CommandLine.stubs(:args).returns([]) STDIN.stubs(:read).returns("code to run") Puppet.expects(:[]=).with(:code,"code to run") @@ -210,8 +210,7 @@ describe "Puppet" do end it "should set the manifest if some files are passed on command line" do - ARGV.stubs(:length).returns(1) - ARGV.stubs(:shift).returns("site.pp") + Puppet::Util::CommandLine.stubs(:args).returns(['site.pp']) Puppet.expects(:[]=).with(:manifest,"site.pp") diff --git a/spec/unit/application/cert.rb b/spec/unit/application/cert.rb index a777a8c54..8757cf36c 100644 --- a/spec/unit/application/cert.rb +++ b/spec/unit/application/cert.rb @@ -110,7 +110,7 @@ describe "PuppetCA" do @cert_app.all = false @ca = stub_everything 'ca' @cert_app.ca = @ca - ARGV.stubs(:collect).returns([]) + Puppet::Util::CommandLine.stubs(:args).returns([]) end it "should delegate to the CertificateAuthority" do @@ -128,7 +128,7 @@ describe "PuppetCA" do end it "should delegate to ca.apply with the hosts given on command line" do - ARGV.stubs(:collect).returns(["host"]) + Puppet::Util::CommandLine.stubs(:args).returns(["host"]) @ca.expects(:apply).with { |mode,to| to[:to] == ["host"]} @@ -136,7 +136,7 @@ describe "PuppetCA" do end it "should send the currently set digest" do - ARGV.stubs(:collect).returns(["host"]) + Puppet::Util::CommandLine.stubs(:args).returns(["host"]) @cert_app.handle_digest(:digest) @ca.expects(:apply).with { |mode,to| to[:digest] == :digest} @@ -146,7 +146,7 @@ describe "PuppetCA" do it "should delegate to ca.apply with current set mode" do @cert_app.mode = "currentmode" - ARGV.stubs(:collect).returns(["host"]) + Puppet::Util::CommandLine.stubs(:args).returns(["host"]) @ca.expects(:apply).with { |mode,to| mode == "currentmode" } @@ -155,7 +155,7 @@ describe "PuppetCA" do it "should revoke cert if mode is clean" do @cert_app.mode = :destroy - ARGV.stubs(:collect).returns(["host"]) + Puppet::Util::CommandLine.stubs(:args).returns(["host"]) @ca.expects(:apply).with { |mode,to| mode == :revoke } @ca.expects(:apply).with { |mode,to| mode == :destroy } diff --git a/spec/unit/application/describe.rb b/spec/unit/application/describe.rb index f9a601454..fd0c5e002 100755 --- a/spec/unit/application/describe.rb +++ b/spec/unit/application/describe.rb @@ -50,8 +50,8 @@ describe Puppet::Application[:describe] do end describe "during setup" do - it "should collect ARGV in options[:types]" do - ARGV.stubs(:dup).returns(['1','2']) + it "should collect arguments in options[:types]" do + Puppet::Util::CommandLine.stubs(:args).returns(['1','2']) @describe.run_setup @describe.options[:types].should == ['1','2'] diff --git a/spec/unit/application/doc.rb b/spec/unit/application/doc.rb index 267a70d8b..185612873 100755 --- a/spec/unit/application/doc.rb +++ b/spec/unit/application/doc.rb @@ -128,11 +128,11 @@ describe "doc" do before :each do Puppet::Log.stubs(:newdestination) - ARGV.stubs(:size).returns(0) + Puppet::Util::CommandLine.stubs(:args).returns([]) end it "should default to rdoc mode if there are command line arguments" do - ARGV.stubs(:size).returns(1) + Puppet::Util::CommandLine.stubs(:args).returns(["1"]) @doc.stubs(:setup_rdoc) @doc.options.expects(:[]=).with(:mode,:rdoc) @@ -304,12 +304,7 @@ describe "doc" do @doc.stubs(:exit) File.stubs(:expand_path).with('modules').returns('modules') File.stubs(:expand_path).with('manifests').returns('manifests') - @old = ARGV.dup - ARGV.clear - end - - after :each do - ARGV << @old + Puppet::Util::CommandLine.stubs(:args).returns([]) end it "should set document_all on --all" do diff --git a/spec/unit/application/filebucket.rb b/spec/unit/application/filebucket.rb index f78c0b7be..37cc93997 100644 --- a/spec/unit/application/filebucket.rb +++ b/spec/unit/application/filebucket.rb @@ -4,7 +4,7 @@ require File.dirname(__FILE__) + '/../../spec_helper' require 'puppet/application/filebucket' -describe "Filebucket" do +describe Puppet::Application[:filebucket] do before :each do @filebucket = Puppet::Application[:filebucket] end @@ -152,7 +152,7 @@ describe "Filebucket" do end it "should use the first non-option parameter as the dispatch" do - ARGV.stubs(:shift).returns(:get) + Puppet::Util::CommandLine.stubs(:args).returns([:get]) @filebucket.get_command.should == :get end @@ -171,7 +171,7 @@ describe "Filebucket" do it "should call the client getfile method with the given md5" do md5="DEADBEEF" - ARGV.stubs(:shift).returns(md5) + @filebucket.stubs(:args).returns([md5]) @client.expects(:getfile).with(md5) @@ -193,7 +193,7 @@ describe "Filebucket" do @filebucket.stubs(:puts) FileTest.stubs(:exists?).returns(true) FileTest.stubs(:readable?).returns(true) - ARGV.stubs(:each).multiple_yields("file1","file2") + @filebucket.stubs(:args).returns(["file1", "file2"]) @client.expects(:backup).with("file1") @client.expects(:backup).with("file2") @@ -206,7 +206,7 @@ describe "Filebucket" do it "should call the client getfile method with the given md5" do md5="DEADBEEF" file="testfile" - ARGV.stubs(:shift).returns(file,md5) + @filebucket.stubs(:args).returns([file, md5]) @client.expects(:restore).with(file,md5) diff --git a/spec/unit/application/resource.rb b/spec/unit/application/resource.rb index 6224ee083..90fd3cc66 100755 --- a/spec/unit/application/resource.rb +++ b/spec/unit/application/resource.rb @@ -133,34 +133,15 @@ describe "resource" do describe "when running" do - def set_args(args) - (ARGV.clear << args).flatten! - end - - def push_args(*args) - @args_stack ||= [] - @args_stack << ARGV.dup - set_args(args) - end - - def pop_args - set_args(@args_stack.pop) - end - before :each do @type = stub_everything 'type', :properties => [] - push_args('type') + Puppet::Util::CommandLine.stubs(:args).returns(['type']) Puppet::Type.stubs(:type).returns(@type) end - after :each do - pop_args - end - it "should raise an error if no type is given" do - push_args + Puppet::Util::CommandLine.stubs(:args).returns([]) lambda { @resource.main }.should raise_error - pop_args end it "should raise an error when editing a remote host" do @@ -192,15 +173,14 @@ describe "resource" do end it "should describe the given resource" do - push_args('type','name') + Puppet::Util::CommandLine.stubs(:args).returns(['type', 'name']) x = stub_everything 'resource' Puppet::Resource.expects(:find).with('https://host:8139/production/resources/type/name').returns(x) @resource.main - pop_args end it "should add given parameters to the object" do - push_args('type','name','param=temp') + Puppet::Util::CommandLine.stubs(:args).returns(['type','name','param=temp']) res = stub "resource" res.expects(:save).with('https://host:8139/production/resources/type/name').returns(res) @@ -209,7 +189,6 @@ describe "resource" do Puppet::Resource.expects(:new).with('type', 'name', {'param' => 'temp'}).returns(res) @resource.main - pop_args end end @@ -230,15 +209,14 @@ describe "resource" do end it "should describe the given resource" do - push_args('type','name') + Puppet::Util::CommandLine.stubs(:args).returns(['type','name']) x = stub_everything 'resource' Puppet::Resource.expects(:find).with('type/name').returns(x) @resource.main - pop_args end it "should add given parameters to the object" do - push_args('type','name','param=temp') + Puppet::Util::CommandLine.stubs(:args).returns(['type','name','param=temp']) res = stub "resource" res.expects(:save).with('type/name').returns(res) @@ -247,7 +225,6 @@ describe "resource" do Puppet::Resource.expects(:new).with('type', 'name', {'param' => 'temp'}).returns(res) @resource.main - pop_args end end diff --git a/spec/unit/util/command_line.rb b/spec/unit/util/command_line.rb index 265535b78..a07438f9e 100644 --- a/spec/unit/util/command_line.rb +++ b/spec/unit/util/command_line.rb @@ -12,67 +12,85 @@ describe Puppet::Util::CommandLine do end it "should pull off the first argument if it looks like a subcommand" do - args = %w( client --help whatever.pp ) - command = Puppet::Util::CommandLine.shift_subcommand_from_argv( args, @tty ) + command, args = Puppet::Util::CommandLine.subcommand_and_args("puppet", %w( client --help whatever.pp ), @tty ) command.should == "client" args.should == %w( --help whatever.pp ) end it "should use 'apply' if the first argument looks like a .pp file" do - args = %w( whatever.pp ) - command = Puppet::Util::CommandLine.shift_subcommand_from_argv( args, @tty ) + command, args = Puppet::Util::CommandLine.subcommand_and_args("puppet", %w( whatever.pp ), @tty ) command.should == "apply" args.should == %w( whatever.pp ) end it "should use 'apply' if the first argument looks like a .rb file" do - args = %w( whatever.rb ) - command = Puppet::Util::CommandLine.shift_subcommand_from_argv( args, @tty ) + command, args = Puppet::Util::CommandLine.subcommand_and_args("puppet", %w( whatever.rb ), @tty ) command.should == "apply" args.should == %w( whatever.rb ) end it "should use 'apply' if the first argument looks like a flag" do - args = %w( --debug ) - command = Puppet::Util::CommandLine.shift_subcommand_from_argv( args, @tty ) + command, args = Puppet::Util::CommandLine.subcommand_and_args("puppet", %w( --debug ), @tty ) command.should == "apply" args.should == %w( --debug ) end it "should use 'apply' if the first argument is -" do - args = %w( - ) - command = Puppet::Util::CommandLine.shift_subcommand_from_argv( args, @tty ) + command, args = Puppet::Util::CommandLine.subcommand_and_args("puppet", %w( - ), @tty ) command.should == "apply" args.should == %w( - ) end it "should return nil if the first argument is --help" do - args = %w( --help ) - command = Puppet::Util::CommandLine.shift_subcommand_from_argv( args, @tty ) + command, args = Puppet::Util::CommandLine.subcommand_and_args("puppet", %w( --help ), @tty ) command.should == nil end it "should return nil if there are no arguments on a tty" do - args = [] - command = Puppet::Util::CommandLine.shift_subcommand_from_argv( args, @tty ) + command, args = Puppet::Util::CommandLine.subcommand_and_args("puppet", [], @tty ) command.should == nil args.should == [] end it "should use 'apply' if there are no arguments on a pipe" do - args = [] - command = Puppet::Util::CommandLine.shift_subcommand_from_argv( args, @pipe ) + command, args = Puppet::Util::CommandLine.subcommand_and_args("puppet", [], @pipe ) command.should == "apply" args.should == [] end + it "should provide a convenience method that only returns the subcommand" do + Puppet::Util::CommandLine.expects(:subcommand_and_args).with("puppet", [], @pipe ).returns(["command", ['args']]) + command = Puppet::Util::CommandLine.subcommand_name( "puppet", [], @pipe ) + command.should == "command" + end + + it "should provide a convenience method that only returns the args" do + Puppet::Util::CommandLine.expects(:subcommand_and_args).with("puppet", [], @pipe ).returns(["command", ['args']]) + args = Puppet::Util::CommandLine.args( "puppet", [], @pipe ) + args.should == ['args'] + end + + it "should return the executable name if it is not puppet" do + command, args = Puppet::Util::CommandLine.subcommand_and_args("puppetmasterd", [], @tty ) + + command.should == "puppetmasterd" + end + + it "should translate subcommand names into their legacy equivalent" do + Puppet::Util::CommandLine.legacy_executable_name("puppet", ["master"], @tty).should == "puppetmasterd" + end + + it "should leave legacy command names alone" do + Puppet::Util::CommandLine.legacy_executable_name("puppetmasterd", [], @tty).should == "puppetmasterd" + end + end diff --git a/test/other/puppet.rb b/test/other/puppet.rb index 1839333e5..2d55c2fba 100755 --- a/test/other/puppet.rb +++ b/test/other/puppet.rb @@ -86,14 +86,7 @@ class TestPuppetModule < Test::Unit::TestCase end def test_name - # Make sure it defaults to $0 without the rb - should = $0.gsub(/.+#{File::SEPARATOR}/,'').sub(/\.rb$/, '') - - assert_equal(should, Puppet[:name], "default name was not right") - - assert_nothing_raised("Could not reset name") do - Puppet[:name] = "puppetca" - end + Puppet[:name] = "puppetca" assert_equal("puppetca", Puppet[:name], "name reset did not take") end