The problem is that the environment list gets cleared
when Settings#set_value is called, and it was being
called every time Settings#use was called, which is
more often than obvious but especially if reporting
is enabled.

Previously we ignored noop when running inside of Settings,
and this essentially adds that back in, so we can remove
the special noop behaviour in Settings itself.

Signed-off-by: Luke Kanies <[email protected]>
---
 lib/puppet/type.rb              |    4 ++++
 lib/puppet/util/settings.rb     |   21 ++++++---------------
 spec/unit/type_spec.rb          |   16 ++++++++++++++++
 spec/unit/util/settings_spec.rb |   23 -----------------------
 4 files changed, 26 insertions(+), 38 deletions(-)

diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index 8807110..c3855a4 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -723,6 +723,10 @@ class Type
 
   # Are we running in noop mode?
   def noop?
+    # If we're not a host_config, we're almost certainly part of
+    # Settings, and we want to ignore 'noop'
+    return false if catalog and ! catalog.host_config?
+
     if defined?(@noop)
       @noop
     else
diff --git a/lib/puppet/util/settings.rb b/lib/puppet/util/settings.rb
index cbb12a8..ca4ecda 100644
--- a/lib/puppet/util/settings.rb
+++ b/lib/puppet/util/settings.rb
@@ -157,13 +157,6 @@ class Puppet::Util::Settings
     set_value(str, value, :cli)
   end
 
-  def without_noop
-    old_noop = value(:noop,:cli) and set_value(:noop, false, :cli) if 
valid?(:noop)
-    yield
-  ensure
-    set_value(:noop, old_noop, :cli) if valid?(:noop)
-  end
-
   def include?(name)
     name = name.intern if name.is_a? String
     @config.include?(name)
@@ -635,14 +628,12 @@ if @config.include?(:run_mode)
         return
       end
 
-      without_noop do
-        catalog.host_config = false
-        catalog.apply do |transaction|
-          if transaction.any_failed?
-            report = transaction.report
-            failures = report.logs.find_all { |log| log.level == :err }
-            raise "Got #{failures.length} failure(s) while initializing: 
#{failures.collect { |l| l.to_s }.join("; ")}"
-          end
+      catalog.host_config = false
+      catalog.apply do |transaction|
+        if transaction.any_failed?
+          report = transaction.report
+          failures = report.logs.find_all { |log| log.level == :err }
+          raise "Got #{failures.length} failure(s) while initializing: 
#{failures.collect { |l| l.to_s }.join("; ")}"
         end
       end
 
diff --git a/spec/unit/type_spec.rb b/spec/unit/type_spec.rb
index 71d415d..487750e 100755
--- a/spec/unit/type_spec.rb
+++ b/spec/unit/type_spec.rb
@@ -123,6 +123,22 @@ describe Puppet::Type do
     Puppet::Type.type(:mount).new(:name => "foo").type.should == :mount
   end
 
+  it "should use any provided noop value" do
+    Puppet::Type.type(:mount).new(:name => "foo", :noop => true).must be_noop
+  end
+
+  it "should use the global noop value if none is provided" do
+    Puppet[:noop] = true
+    Puppet::Type.type(:mount).new(:name => "foo").must be_noop
+  end
+
+  it "should not be noop if in a non-host_config catalog" do
+    resource = Puppet::Type.type(:mount).new(:name => "foo")
+    catalog = Puppet::Resource::Catalog.new
+    catalog.add_resource resource
+    resource.should_not be_noop
+  end
+
   describe "when creating an event" do
     before do
       @resource = Puppet::Type.type(:mount).new :name => "foo"
diff --git a/spec/unit/util/settings_spec.rb b/spec/unit/util/settings_spec.rb
index 81cab79..7bca44b 100755
--- a/spec/unit/util/settings_spec.rb
+++ b/spec/unit/util/settings_spec.rb
@@ -1096,27 +1096,4 @@ describe Puppet::Util::Settings do
 
     it "should cache the result"
   end
-
-  describe "#without_noop" do
-    before do
-      @settings = Puppet::Util::Settings.new
-      @settings.setdefaults :main, :noop => [true, ""]
-    end
-
-    it "should set noop to false for the duration of the block" do
-      @settings.without_noop { @settings.value(:noop, :cli).should be_false }
-    end
-
-    it "should ensure that noop is returned to its previous value" do
-      @settings.without_noop { raise } rescue nil
-      @settings.value(:noop, :cli).should be_true
-    end
-
-    it "should work even if no 'noop' setting is available" do
-      settings = Puppet::Util::Settings.new
-      stuff = nil
-      settings.without_noop { stuff = "yay" }
-      stuff.should == "yay"
-    end
-  end
 end
-- 
1.6.1

-- 
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.

Reply via email to