Please review pull request #759: Bug/master/14322 puppet version doesn't work opened by (daniel-pittman)
Description:
- Opened: Fri May 11 19:01:23 UTC 2012
- Based on: puppetlabs:master (80c8f0a7b3a8b944388f506861c06efe6656020a)
- Requested merge: daniel-pittman:bug/master/14322-puppet---version-doesn't-work (1357b78be3b1f3df7efe63350501271296ad5d87)
Diff follows:
diff --git a/lib/puppet/application.rb b/lib/puppet/application.rb
index e2ae01e..771d409 100644
--- a/lib/puppet/application.rb
+++ b/lib/puppet/application.rb
@@ -255,6 +255,8 @@ def run_mode( mode_name = nil)
attr_reader :options, :command_line
# Every app responds to --version
+ # See also `lib/puppet/util/command_line.rb` for some special case early
+ # handling of this.
option("--version", "-V") do |arg|
puts "#{Puppet.version}"
exit
diff --git a/lib/puppet/util/command_line.rb b/lib/puppet/util/command_line.rb
index 115ed39..4beb69f 100644
--- a/lib/puppet/util/command_line.rb
+++ b/lib/puppet/util/command_line.rb
@@ -48,21 +48,33 @@ def require_application(application)
require File.join(appdir, application)
end
- # This is the main entry point for all puppet applications / faces; it is basically where the bootstrapping
- # process / lifecycle of an app begins.
+ # This is the main entry point for all puppet applications / faces; it
+ # is basically where the bootstrapping process / lifecycle of an app
+ # begins.
def execute
+ # We support printing the global version very early, unconditionally.
+ # This doesn't replace declaring the option later, even if it means
+ # that particular bit will never trigger.
+ if @argv.include? "--version" or @argv.include? "-V"
+ puts Puppet.version
+ exit 0
+ end
+
+ # Build up our settings - we don't need that until after version check.
Puppet::Util.exit_on_fail("intialize global default settings") do
Puppet.settings.initialize_global_settings(args)
end
- # OK, now that we've processed the command line options and the config files, we should be able to say that
- # we definitively know where the libdir is... which means that we can now look for our available
+ # OK, now that we've processed the command line options and the config
+ # files, we should be able to say that we definitively know where the
+ # libdir is... which means that we can now look for our available
# applications / subcommands / faces.
if subcommand_name and available_subcommands.include?(subcommand_name) then
require_application subcommand_name
- # This will need to be cleaned up to do something that is not so application-specific
- # (i.e.. so that we can load faces). Longer-term, use the autoloader. See comments in
+ # This will need to be cleaned up to do something that is not so
+ # application-specific (i.e.. so that we can load faces).
+ # Longer-term, use the autoloader. See comments in
# #available_subcommands method above. --cprice 2012-03-06
app = Puppet::Application.find(subcommand_name).new(self)
Puppet::Plugins.on_application_initialization(:application_object => self)
@@ -99,8 +111,9 @@ def subcommand_and_args(zero, argv, stdin)
if zero == 'puppet'
case argv.first
- # if they didn't pass a command, or passed a help flag, we will fall back to showing a usage message.
- # we no longer default to 'apply'
+ # if they didn't pass a command, or passed a help flag, we will
+ # fall back to showing a usage message. we no longer default to
+ # 'apply'
when nil, "--help", "-h", /^-|\.pp$|\.rb$/
[nil, argv]
else
diff --git a/spec/unit/util/command_line_spec.rb b/spec/unit/util/command_line_spec.rb
index f608c13..53b53f5 100755
--- a/spec/unit/util/command_line_spec.rb
+++ b/spec/unit/util/command_line_spec.rb
@@ -6,97 +6,117 @@
describe Puppet::Util::CommandLine do
include PuppetSpec::Files
- before do
- @tty = stub("tty", :tty? => true )
- @pipe = stub("pipe", :tty? => false)
- end
+ let :tty do stub("tty", :tty? => true) end
+ let :pipe do stub("pipe", :tty? => false) end
- it "should pull off the first argument if it looks like a subcommand" do
- command_line = Puppet::Util::CommandLine.new("puppet", %w{ client --help whatever.pp }, @tty )
+ context "#initialize" do
+ it "should pull off the first argument if it looks like a subcommand" do
+ command_line = Puppet::Util::CommandLine.new("puppet", %w{ client --help whatever.pp }, tty)
- command_line.subcommand_name.should == "client"
- command_line.args.should == %w{ --help whatever.pp }
- end
+ command_line.subcommand_name.should == "client"
+ command_line.args.should == %w{ --help whatever.pp }
+ end
- it "should return nil if the first argument looks like a .pp file" do
- command_line = Puppet::Util::CommandLine.new("puppet", %w{ whatever.pp }, @tty )
+ it "should return nil if the first argument looks like a .pp file" do
+ command_line = Puppet::Util::CommandLine.new("puppet", %w{ whatever.pp }, tty)
- command_line.subcommand_name.should == nil
- command_line.args.should == %w{ whatever.pp }
- end
+ command_line.subcommand_name.should == nil
+ command_line.args.should == %w{ whatever.pp }
+ end
- it "should return nil if the first argument looks like a .rb file" do
- command_line = Puppet::Util::CommandLine.new("puppet", %w{ whatever.rb }, @tty )
+ it "should return nil if the first argument looks like a .rb file" do
+ command_line = Puppet::Util::CommandLine.new("puppet", %w{ whatever.rb }, tty)
- command_line.subcommand_name.should == nil
- command_line.args.should == %w{ whatever.rb }
- end
+ command_line.subcommand_name.should == nil
+ command_line.args.should == %w{ whatever.rb }
+ end
- it "should return nil if the first argument looks like a flag" do
- command_line = Puppet::Util::CommandLine.new("puppet", %w{ --debug }, @tty )
+ it "should return nil if the first argument looks like a flag" do
+ command_line = Puppet::Util::CommandLine.new("puppet", %w{ --debug }, tty)
- command_line.subcommand_name.should == nil
- command_line.args.should == %w{ --debug }
- end
+ command_line.subcommand_name.should == nil
+ command_line.args.should == %w{ --debug }
+ end
- it "should return nil if the first argument is -" do
- command_line = Puppet::Util::CommandLine.new("puppet", %w{ - }, @tty )
+ it "should return nil if the first argument is -" do
+ command_line = Puppet::Util::CommandLine.new("puppet", %w{ - }, tty)
- command_line.subcommand_name.should == nil
- command_line.args.should == %w{ - }
- end
+ command_line.subcommand_name.should == nil
+ command_line.args.should == %w{ - }
+ end
- it "should return nil if the first argument is --help" do
- command_line = Puppet::Util::CommandLine.new("puppet", %w{ --help }, @tty )
+ it "should return nil if the first argument is --help" do
+ command_line = Puppet::Util::CommandLine.new("puppet", %w{ --help }, tty)
- command_line.subcommand_name.should == nil
- end
+ command_line.subcommand_name.should == nil
+ end
- it "should return nil if there are no arguments on a tty" do
- command_line = Puppet::Util::CommandLine.new("puppet", [], @tty )
+ it "should return nil if there are no arguments on a tty" do
+ command_line = Puppet::Util::CommandLine.new("puppet", [], tty)
- command_line.subcommand_name.should == nil
- command_line.args.should == []
- end
+ command_line.subcommand_name.should == nil
+ command_line.args.should == []
+ end
- it "should return nil if there are no arguments on a pipe" do
- command_line = Puppet::Util::CommandLine.new("puppet", [], @pipe )
+ it "should return nil if there are no arguments on a pipe" do
+ command_line = Puppet::Util::CommandLine.new("puppet", [], pipe)
- command_line.subcommand_name.should == nil
- command_line.args.should == []
+ command_line.subcommand_name.should == nil
+ command_line.args.should == []
+ end
end
- describe "when dealing with puppet commands" do
+ context "#execute" do
+ %w{--version -V}.each do |arg|
+ it "should print the version and exit if #{arg} is given" do
+ expect do
+ expect do
+ described_class.new("puppet", [arg], tty).execute
+ end.to exit_with 0
+ end.to have_printed(Puppet.version.to_s)
+ end
+ end
+ end
+ describe "when dealing with puppet commands" do
it "should return the executable name if it is not puppet" do
- command_line = Puppet::Util::CommandLine.new("puppetmasterd", [], @tty )
-
+ command_line = Puppet::Util::CommandLine.new("puppetmasterd", [], tty)
command_line.subcommand_name.should == "puppetmasterd"
end
it "should translate subcommand names into their legacy equivalent" do
- command_line = Puppet::Util::CommandLine.new("puppet", ["master"], @tty)
+ command_line = Puppet::Util::CommandLine.new("puppet", ["master"], tty)
command_line.legacy_executable_name.should == :puppetmasterd
end
it "should leave legacy command names alone" do
- command_line = Puppet::Util::CommandLine.new("puppetmasterd", [], @tty)
+ command_line = Puppet::Util::CommandLine.new("puppetmasterd", [], tty)
command_line.legacy_executable_name.should == :puppetmasterd
end
describe "when the subcommand is not implemented" do
it "should find and invoke an executable with a hyphenated name" do
- commandline = Puppet::Util::CommandLine.new("puppet", ['whatever', 'argument'], @tty)
- Puppet::Util.expects(:which).with('puppet-whatever').returns('/dev/null/puppet-whatever')
- commandline.expects(:exec).with('/dev/null/puppet-whatever', 'argument')
-
- commandline.execute
+ commandline = Puppet::Util::CommandLine.new("puppet", ['whatever', 'argument'], tty)
+ Puppet::Util.expects(:which).with('puppet-whatever').
+ returns('/dev/null/puppet-whatever')
+
+ # It is important that we abort at the point exec is called, because
+ # the code (reasonably) assumes that if `exec` is called processing
+ # immediately terminates, and we are replaced by the executed process.
+ #
+ # This raise isn't a perfect simulation of that, but it is enough to
+ # validate that the system works, and ... well, if exec is broken we
+ # have two problems, y'know.
+ commandline.expects(:exec).with('/dev/null/puppet-whatever', 'argument').
+ raises(SystemExit)
+
+ expect { commandline.execute }.to raise_error SystemExit
end
describe "and an external implementation cannot be found" do
it "should abort and show the usage message" do
- commandline = Puppet::Util::CommandLine.new("puppet", ['whatever', 'argument'], @tty)
+ commandline = Puppet::Util::CommandLine.new("puppet", ['whatever', 'argument'], tty)
Puppet::Util.expects(:which).with('puppet-whatever').returns(nil)
commandline.expects(:exec).never
@@ -107,18 +127,22 @@
end
end
describe 'when loading commands' do
- before do
- @core_apps = %w{describe filebucket kick queue resource agent cert apply doc master}
- @command_line = Puppet::Util::CommandLine.new("foo", %w{ client --help whatever.pp }, @tty )
+ let :core_apps do
+ %w{describe filebucket kick queue resource agent cert apply doc master}
end
+
+ let :command_line do
+ Puppet::Util::CommandLine.new("foo", %w{ client --help whatever.pp }, tty)
+ end
+
it "should expose available_subcommands as a class method" do
- @core_apps.each do |command|
- @command_line.available_subcommands.should include command
+ core_apps.each do |command|
+ command_line.available_subcommands.should include command
end
end
it 'should be able to find all existing commands' do
- @core_apps.each do |command|
- @command_line.available_subcommands.should include command
+ core_apps.each do |command|
+ command_line.available_subcommands.should include command
end
end
describe 'when multiple paths have applications' do
@@ -130,9 +154,9 @@
$LOAD_PATH.unshift(@dir) # WARNING: MUST MATCH THE AFTER ACTIONS!
end
it 'should be able to find commands from both paths' do
- found = @command_line.available_subcommands
+ found = command_line.available_subcommands
found.should include 'foo'
- @core_apps.each { |cmd| found.should include cmd }
+ core_apps.each { |cmd| found.should include cmd }
end
after do
$LOAD_PATH.shift # WARNING: MUST MATCH THE BEFORE ACTIONS!
-- You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to [email protected].
For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
