Please review pull request #402: (#12114) Qualify usages of ::File to avoid conflict with File face opened by (joshcooper)
Description:
Previously, if the file face had been loaded, then subsequent calls
to Puppet::Application.find would attempt to callPuppet::Application::File.join, and fail since it should be calling::File.join.
In addition, if spec/unit/application_spec.rb is tested first, then
several other order-dependent failures occurred, for the same reason
as above.
This commit uses the '::' scope resolution operator to refer to ruby's
File class and adds a spec test.
In general, if you add an application to lib/puppet/application/,
then you should fully qualify uses of ::File, otherwise, if the file
face is loaded first, your application may fail.
- Opened: Tue Jan 24 22:36:49 UTC 2012
- Based on: puppetlabs:2.7.x (dd6bef5446a3ce892b503ac19830f810791560ad)
- Requested merge: joshcooper:ticket/2.7.x/12114-file-face-help (b0c8d2c22345a34cd637640c7f50e46ad1dc1c6d)
Diff follows:
diff --git a/lib/puppet/application.rb b/lib/puppet/application.rb
index f6dac39..c940d06 100644
--- a/lib/puppet/application.rb
+++ b/lib/puppet/application.rb
@@ -119,7 +119,7 @@ class Application
require 'puppet/util'
include Puppet::Util
- DOCPATTERN = File.expand_path(File.dirname(__FILE__) + "/util/command_line/*" )
+ DOCPATTERN = ::File.expand_path(::File.dirname(__FILE__) + "/util/command_line/*" )
class << self
include Puppet::Util
@@ -216,7 +216,7 @@ def find(name)
klass = name.to_s.capitalize
begin
- require File.join('puppet', 'application', name.to_s.downcase)
+ require ::File.join('puppet', 'application', name.to_s.downcase)
rescue LoadError => e
puts "Unable to find application '#{name}'. #{e}"
Kernel::exit(1)
diff --git a/lib/puppet/application/apply.rb b/lib/puppet/application/apply.rb
index e9b4e7a..3e2be48 100644
--- a/lib/puppet/application/apply.rb
+++ b/lib/puppet/application/apply.rb
@@ -154,7 +154,7 @@ def apply
if options[:catalog] == "-"
text = $stdin.read
else
- text = File.read(options[:catalog])
+ text = ::File.read(options[:catalog])
end
begin
@@ -177,7 +177,7 @@ def main
Puppet[:code] = options[:code] || STDIN.read
else
manifest = command_line.args.shift
- raise "Could not find file #{manifest}" unless File.exist?(manifest)
+ raise "Could not find file #{manifest}" unless ::File.exist?(manifest)
Puppet.warning("Only one file can be applied per run. Skipping #{command_line.args.join(', ')}") if command_line.args.size > 0
Puppet[:manifest] = manifest
end
@@ -208,7 +208,7 @@ def main
$stderr.puts "#{file} is not readable"
exit(63)
end
- node.classes = File.read(file).split(/[\s\n]+/)
+ node.classes = ::File.read(file).split(/[\s\n]+/)
end
end
diff --git a/lib/puppet/application/device.rb b/lib/puppet/application/device.rb
index d0b9387..7a602f2 100644
--- a/lib/puppet/application/device.rb
+++ b/lib/puppet/application/device.rb
@@ -171,8 +171,8 @@ def main
Puppet.info "starting applying configuration to #{device.name} at #{device.url}"
# override local $vardir and $certname
- Puppet.settings.set_value(:confdir, File.join(Puppet[:devicedir], device.name), :cli)
- Puppet.settings.set_value(:vardir, File.join(Puppet[:devicedir], device.name), :cli)
+ Puppet.settings.set_value(:confdir, ::File.join(Puppet[:devicedir], device.name), :cli)
+ Puppet.settings.set_value(:vardir, ::File.join(Puppet[:devicedir], device.name), :cli)
Puppet.settings.set_value(:certname, device.name, :cli)
# this will reload and recompute default settings and create the devices sub vardir, or we hope so :-)
diff --git a/lib/puppet/application/doc.rb b/lib/puppet/application/doc.rb
index 6c23a7a..d2683b0 100644
--- a/lib/puppet/application/doc.rb
+++ b/lib/puppet/application/doc.rb
@@ -167,7 +167,7 @@ def rdoc
unless @manifest
env = Puppet::Node::Environment.new
files += env.modulepath
- files << File.dirname(env[:manifest])
+ files << ::File.dirname(env[:manifest])
end
files += command_line.args
Puppet.info "scanning: #{files.inspect}"
@@ -252,7 +252,7 @@ def setup_rdoc(dummy_argument=:work_arround_for_ruby_GC_bug)
@unknown_args.each do |option|
# force absolute path for modulepath when passed on commandline
if option[:opt]=="--modulepath" or option[:opt] == "--manifestdir"
- option[:arg] = option[:arg].split(File::PATH_SEPARATOR).collect { |p| File.expand_path(p) }.join(File::PATH_SEPARATOR)
+ option[:arg] = option[:arg].split(::File::PATH_SEPARATOR).collect { |p| ::File.expand_path(p) }.join(::File::PATH_SEPARATOR)
end
Puppet.settings.handlearg(option[:opt], option[:arg])
end
diff --git a/lib/puppet/application/inspect.rb b/lib/puppet/application/inspect.rb
index 6737128..ad31d8d 100644
--- a/lib/puppet/application/inspect.rb
+++ b/lib/puppet/application/inspect.rb
@@ -164,7 +164,7 @@ def run_command
end
if Puppet[:archive_files] and ral_resource.type == :file and audited_attributes.include?(:content)
path = ral_resource[:path]
- if File.readable?(path)
+ if ::File.readable?(path)
begin
dipper.backup(path)
rescue StandardError => detail
diff --git a/spec/unit/application_spec.rb b/spec/unit/application_spec.rb
index 591358e..efd5b46 100755
--- a/spec/unit/application_spec.rb
+++ b/spec/unit/application_spec.rb
@@ -38,6 +38,12 @@
expect { @klass.find("ThisShallNeverEverEverExist") }.to exit_with 1
end
+
+ it "#12114: should prevent File namespace collisions" do
+ # have to require the file face once, then the second time around it would fail
+ @klass.find("File").should == Puppet::Application::File
+ @klass.find("File").should == Puppet::Application::File
+ end
end
describe ".run_mode" do
-- 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.
