Greetings!
Please review the pull request #57: (#8491) Prevent repeated loading of fact files opened by (adrienthebo)
Some more information about the pull request:
- Opened: Wed Sep 14 23:44:36 UTC 2011
- Based on: puppetlabs:1.6rc (cba8ebc3e4cd0a61471bdac62995328776db92bc)
- Requested merge: adrienthebo:ticket/master/8491-remove_multiple_fact_loading (4d937456e793be0830e55128f091cc5a81a77721)
Description:
Fact loading could recurse indefinitely if a fact file attempted to call
Fact#value on a fact that was not yet defined before the current file.
If Fact#value was called outside of a setcode block, it would be
evaluated at load time and the loader would rescan the fact path from
the beginning and would reenter the current file, continuing until the
stack was full. This is a byproduct of the more exhaustive fact
searching introduced in 2255abe.
The resolution for this is to track the files that have been loaded and
ignore subsequent attempts to load them, emulating the behavior of
Kernel.require. However, since facts can be legitimately refreshed
over the life of a ruby process using Facter, Facter.clear will reset
the list of loaded files by destroying the fact collection, and
subsequently the loader.
Currently puppet agent will reload all facts preceeding a run, so normal
puppet agent behavior will remain as expected. However, the facter facts
terminus manually loads fact files itself and bypasses facter's search
path and standard loading mechanism. While it will benefit from the
recursion protection, it currently does not have a way to reset the
loaded file list.
Thanks!
The Pull Request Bot
Diff follows:
diff --git a/lib/facter/util/loader.rb b/lib/facter/util/loader.rb
index a52012c..29a1de0 100644
--- a/lib/facter/util/loader.rb
+++ b/lib/facter/util/loader.rb
@@ -2,6 +2,11 @@ require 'facter'
# Load facts on demand.
class Facter::Util::Loader
+
+ def initialize
+ @loaded = []
+ end
+
# Load all resolutions for a single fact.
def load(fact)
# Now load from the search path
@@ -68,10 +73,16 @@ class Facter::Util::Loader
end
def load_file(file)
+ return if @loaded.include? file
# We have to specify Kernel.load, because we have a load method.
begin
+ # Store the file path so we don't try to reload it
+ @loaded << file
Kernel.load(file)
rescue ScriptError => detail
+ # Don't store the path if the file can't be loaded
+ # in case it's loadable later on.
+ @loaded.delete(file)
warn "Error loading fact #{file} #{detail}"
end
end
diff --git a/spec/fixtures/unit/util/loader/nosuchfact.rb b/spec/fixtures/unit/util/loader/nosuchfact.rb
new file mode 100644
index 0000000..f0ba60a
--- /dev/null
+++ b/spec/fixtures/unit/util/loader/nosuchfact.rb
@@ -0,0 +1 @@
+Facter.value(:nosuchfact)
diff --git a/spec/unit/util/loader_spec.rb b/spec/unit/util/loader_spec.rb
index 1bc909f..3fb1f04 100755
--- a/spec/unit/util/loader_spec.rb
+++ b/spec/unit/util/loader_spec.rb
@@ -280,4 +280,12 @@ describe Facter::Util::Loader do
@loader.load_all
end
end
+
+ it "should load facts on the facter search path only once" do
+ facterlibdir = File.expand_path(File.dirname(__FILE__) + '../../../fixtures/unit/util/loader')
+ with_env 'FACTERLIB' => facterlibdir do
+ Facter::Util::Loader.new.load_all
+ Facter.value(:nosuchfact).should be_nil
+ end
+ end
end
-- 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.
