Fix #1757 Change file mode representation to octal

This patch changes the internal representation of a file's mode to a
string instead of an integer. This simplifies the problem of displaying
the value consistently throughout all of puppet.

7 files changed, 22 insertions, 52 deletions

diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb
index f35a26408..6523c99a0 100644
--- a/lib/puppet/type/file.rb
+++ b/lib/puppet/type/file.rb
@@ -718,8 +718,9 @@ Puppet::Type.newtype(:file) do
 
     mode = self.should(:mode) # might be nil
     umask = mode ? 000 : 022
+    mode_int = mode ? mode.to_i(8) : nil
 
-    content_checksum = Puppet::Util.withumask(umask) { File.open(path, 'w', mode) { |f| write_content(f) } }
+    content_checksum = Puppet::Util.withumask(umask) { File.open(path, 'w', mode_int ) { |f| write_content(f) } }
 
     # And put our new file in place
     if use_temporary_file # This is only not true when our file is empty.
diff --git a/lib/puppet/type/file/ensure.rb b/lib/puppet/type/file/ensure.rb
index 967e06aee..4a68551ee 100755
--- a/lib/puppet/type/file/ensure.rb
+++ b/lib/puppet/type/file/ensure.rb
@@ -66,7 +66,7 @@ module Puppet
       end
       if mode
         Puppet::Util.withumask(000) do
-          Dir.mkdir(@resource[:path],mode)
+          Dir.mkdir(@resource[:path], mode.to_i(8))
         end
       else
         Dir.mkdir(@resource[:path])
diff --git a/lib/puppet/type/file/mode.rb b/lib/puppet/type/file/mode.rb
index 1ce56c843..2acd8b359 100755
--- a/lib/puppet/type/file/mode.rb
+++ b/lib/puppet/type/file/mode.rb
@@ -25,60 +25,26 @@ module Puppet
 
     @event = :file_changed
 
-    # Our modes are octal, so make sure they print correctly.  Other
-    # valid values are symbols, basically
-    def is_to_s(currentvalue)
-      case currentvalue
-      when Integer
-        return "%o" % currentvalue
-      when Symbol
-        return currentvalue
-      else
-        raise Puppet::DevError, "Invalid current value for mode: #{currentvalue.inspect}"
-      end
-    end
-
-    def should_to_s(newvalue = @should)
-      case newvalue
-      when Integer
-        return "%o" % newvalue
-      when Symbol
-        return newvalue
-      else
-        raise Puppet::DevError, "Invalid 'should' value for mode: #{newvalue.inspect}"
-      end
-    end
-
     munge do |should|
-      # this is pretty hackish, but i need to make sure the number is in
-      # octal, yet the number can only be specified as a string right now
-      value = should
-      if value.is_a?(String)
-        unless value =~ /^\d+$/
-          raise Puppet::Error, "File modes can only be numbers, not #{value.inspect}"
-        end
-        # Make sure our number looks like octal.
-        unless value =~ /^0/
-          value = "0#{value}"
-        end
-        old = value
-        begin
-          value = Integer(value)
-        rescue ArgumentError => detail
-          raise Puppet::DevError, "Could not convert #{old.inspect} to integer"
+      if should.is_a?(String)
+        unless should =~ /^[0-7]+$/
+          raise Puppet::Error, "File modes can only be octal numbers, not #{should.inspect}"
         end
+        should.to_i(8).to_s(8)
+      else
+        should.to_s(8)
       end
-
-      return value
     end
 
     # If we're a directory, we need to be executable for all cases
     # that are readable.  This should probably be selectable, but eh.
     def dirmask(value)
       if FileTest.directory?(@resource[:path])
+        value = value.to_i(8)
         value |= 0100 if value & 0400 != 0
         value |= 010 if value & 040 != 0
         value |= 01 if value & 04 != 0
+        value = value.to_s(8)
       end
 
       value
@@ -101,7 +67,7 @@ module Puppet
         unless defined?(@fixed)
           @should &&= @should.collect { |s| self.dirmask(s) }
         end
-        return stat.mode & 007777
+        return (stat.mode & 007777).to_s(8)
       else
         return :absent
       end
@@ -111,7 +77,7 @@ module Puppet
       mode = self.should
 
       begin
-        File.chmod(mode, @resource[:path])
+        File.chmod(mode.to_i(8), @resource[:path])
       rescue => detail
         error = Puppet::Error.new("failed to chmod #{@resource[:path]}: #{detail.message}")
         error.set_backtrace detail.backtrace
diff --git a/spec/unit/transaction/resource_harness_spec.rb b/spec/unit/transaction/resource_harness_spec.rb
index 255481ae4..4611409d5 100755
--- a/spec/unit/transaction/resource_harness_spec.rb
+++ b/spec/unit/transaction/resource_harness_spec.rb
@@ -46,7 +46,7 @@ describe Puppet::Transaction::ResourceHarness do
       @harness.cache(@resource, :mode, "755")
       @harness.copy_audited_parameters(@resource, {}).should == [:mode]
 
-      @resource[:mode].should == 0755
+      @resource[:mode].should == "755"
     end
 
     it "should cache and log the current value if no cached values are present" do
@@ -169,7 +169,7 @@ describe Puppet::Transaction::ResourceHarness do
       @resource[:audit] = :mode
       @harness.cache(@resource, :mode, "755")
       @harness.changes_to_perform(@status, @resource)
-      @resource[:mode].should == 0755
+      @resource[:mode].should == "755"
     end
 
     it "should mark changes created as a result of auditing as auditing changes" do
@@ -242,7 +242,7 @@ describe Puppet::Transaction::ResourceHarness do
         @resource[:ensure] = :present
         @resource[:mode] = "755"
         @current_state[:ensure] = :present
-        @current_state[:mode] = 0755
+        @current_state[:mode] = "755"
         @harness.changes_to_perform(@status, @resource).should == []
       end
     end
diff --git a/spec/unit/type/file/source_spec.rb b/spec/unit/type/file/source_spec.rb
index a45a1f74e..522ae1f0e 100755
--- a/spec/unit/type/file/source_spec.rb
+++ b/spec/unit/type/file/source_spec.rb
@@ -154,7 +154,7 @@ describe Puppet::Type.type(:file).attrclass(:source) do
 
         @resource[:owner].must == 100
         @resource[:group].must == 200
-        @resource[:mode].must == 123
+        @resource[:mode].must == "173"
 
         # Metadata calls it checksum, we call it content.
         @resource[:content].must == @metadata.checksum
@@ -170,7 +170,7 @@ describe Puppet::Type.type(:file).attrclass(:source) do
 
         @resource[:owner].must == 1
         @resource[:group].must == 2
-        @resource[:mode].must == 3
+        @resource[:mode].must == "3"
         @resource[:content].should_not == @metadata.checksum
       end
 
diff --git a/test/language/snippets.rb b/test/language/snippets.rb
index 51c5e23fe..a10e8e870 100755
--- a/test/language/snippets.rb
+++ b/test/language/snippets.rb
@@ -37,6 +37,9 @@ class TestSnippets < Test::Unit::TestCase
   end
 
   def assert_mode_equal(mode, path)
+    if mode.is_a? Integer
+      mode = mode.to_s(8)
+    end
     unless file = @catalog.resource(:file, path)
       raise "Could not find file #{path}"
     end
diff --git a/test/ral/type/file.rb b/test/ral/type/file.rb
index 6322529cf..386c3ca1b 100755
--- a/test/ral/type/file.rb
+++ b/test/ral/type/file.rb
@@ -612,7 +612,7 @@ class TestFile < Test::Unit::TestCase
         
       :mode => "0777"
     )
-    assert_equal(0777, file.should(:mode), "Mode did not get set correctly")
+    assert_equal("777", file.should(:mode), "Mode did not get set correctly")
     assert_apply(file)
     assert_equal(0777, File.stat(path).mode & 007777, "file mode is incorrect")
     File.unlink(path)