Fix for #3366 - --tags '' treated as boolean 'true'

This is the patch from Mike Pountney <Mike.Pountney@gmail.com> off
the list with the additional test Luke requested.

2 files changed, 21 insertions, 4 deletions

diff --git a/lib/puppet/util/settings.rb b/lib/puppet/util/settings.rb
index 532fce960..002bc0b1a 100644
--- a/lib/puppet/util/settings.rb
+++ b/lib/puppet/util/settings.rb
@@ -147,6 +147,7 @@ class Puppet::Util::Settings
         @cache.clear
         value = munge_value(value) if value
         str = opt.sub(/^--/,'')
+
         bool = true
         newstr = str.sub(/^no-/, '')
         if newstr != str
@@ -155,8 +156,10 @@ class Puppet::Util::Settings
         end
         str = str.intern
 
-        if value == "" or value.nil?
-            value = bool
+        if @config[str].is_a?(Puppet::Util::Settings::BooleanSetting)
+            if value == "" or value.nil?
+                value = bool
+            end
         end
 
         set_value(str, value, :cli)
diff --git a/spec/unit/util/settings.rb b/spec/unit/util/settings.rb
index 645b54366..1487f19d2 100755
--- a/spec/unit/util/settings.rb
+++ b/spec/unit/util/settings.rb
@@ -79,6 +79,7 @@ describe Puppet::Util::Settings do
         end
 
         it "should support a getopt-specific mechanism for turning booleans off" do
+            @settings[:bool] = true
             @settings.handlearg("--no-bool", "")
             @settings[:bool].should == false
         end
@@ -97,20 +98,33 @@ describe Puppet::Util::Settings do
             @settings[:bool].should == true
         end
 
-        it "should consider a cli setting with an empty string as an argument to be a boolean" do
+        it "should consider a cli setting with an empty string as an argument to be a boolean, if the setting itself is a boolean" do
             # Turn it off first
             @settings[:bool] = false
             @settings.handlearg("--bool", "")
             @settings[:bool].should == true
         end
 
+        it "should consider a cli setting with an empty string as an argument to be an empty argument, if the setting itself is not a boolean" do
+            @settings[:myval] = "bob"
+            @settings.handlearg("--myval", "")
+            @settings[:myval].should == ""
+        end
+
         it "should consider a cli setting with a boolean as an argument to be a boolean" do
             # Turn it off first
             @settings[:bool] = false
-            @settings.handlearg("--bool", true)
+            @settings.handlearg("--bool", "true")
             @settings[:bool].should == true
         end
 
+       it "should not consider a cli setting of a non boolean with a boolean as an argument to be a boolean" do
+            # Turn it off first
+            @settings[:myval] = "bob"
+            @settings.handlearg("--no-myval", "")
+            @settings[:myval].should == ""
+        end
+
         it "should clear the cache when setting getopt-specific values" do
             @settings.setdefaults :mysection, :one => ["whah", "yay"], :two => ["$one yay", "bah"]
             @settings[:two].should == "whah yay"