On Apr 3, 2010, at 9:54 AM, Markus Roberts wrote:
Initial thoughts (I'll be back for more later):
Looks Ok at a first read, though I have question/comments (some
noted below) and parts I'll need to go back to with more coffee in
me to make sure I understand.
diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb
index e13c796..93d1735 100644
--- a/lib/puppet/type/file.rb
+++ b/lib/puppet/type/file.rb
@@ -706,10 +706,10 @@ module Puppet
# Write out the file. Requires the content to be written,
# the property name for logging, and the checksum for
validation.
- def write(content, property)
+ def write(property)
It appears that the comment here is now at least two refactors out
of date, as we are nolonger passing in the content or the checksum.
This might be a good argument for removing the comment altogether as
it is both a) wrong and b) being consistently ignored.
remove_existing(:file)
- use_temporary_file = (content.length != 0)
+ use_temporary_file = write_temporary_file?
if use_temporary_file
path = "#{self[:path]}.puppettmp_#{rand(10000)}"
while File.exists?(path) or File.symlink?(path)
@@ -722,14 +722,17 @@ module Puppet
mode = self.should(:mode) # might be nil
umask = mode ? 000 : 022
+ content_checksum = nil
Puppet::Util.withumask(umask) do
- File.open(path, File::CREAT|File::WRONLY|
File::TRUNC, mode) { |f| f.print content }
+ File.open(path, File::CREAT|File::WRONLY|
File::TRUNC, mode) do |f|
+ content_checksum = write_content(f)
+ end
end
This could be reduced to:
content_checksum = Puppet::Util.withumask(umask) do
File.open(path, File::CREAT|File::WRONLY|
File::TRUNC, mode) { |f| write_content(f) }
end
Or if I'm not mistaken:
content_checksum = Puppet::Util.withumask(umask)
{ File.open(path,'w' , mode) { |f| write_content(f) } }
I don't think 'w' does FILE::TRUNC - that is, it appends rather than
overwrites, and we specifically always want to overwrite.
# And put our new file in place
if use_temporary_file # This is only not true when our
file is empty.
begin
- fail_if_checksum_is_wrong(path, content)
+ fail_if_checksum_is_wrong(path,
content_checksum) if validate_checksum?
File.rename(path, self[:path])
rescue => detail
fail "Could not rename temporary file %s to %s:
%s" % [path, self[:path], detail]
@@ -743,17 +746,35 @@ module Puppet
property_fix
end
- private
+ # Should we validate the checksum of the file we're writing?
+ def validate_checksum?
+ self[:checksum] !~ /time/
+ end
# Make sure the file we wrote out is what we think it is.
- def fail_if_checksum_is_wrong(path, content)
- # Use the appropriate checksum type -- md5, md5lite, etc.
- checksum = parameter(:checksum).sum(content)
-
+ def fail_if_checksum_is_wrong(path, content_checksum)
newsum = parameter(:checksum).sum_file(path)
- return if [:absent, nil, checksum].include?(newsum)
+ return if [:absent, nil, content_checksum].include?
(newsum)
+
+ self.fail "File written to disk did not match checksum;
discarding changes (%s vs %s)" % [content_checksum, newsum]
+ end
- self.fail "File written to disk did not match checksum;
discarding changes (#{checksum} vs #{newsum})"
+ def write_content(file)
+ checksum = nil
+ if content = property(:content)
+ checksum = content.write(file)
+ else # we have a sole ensure property
+ file.print ""
+ end
+ checksum
+ end
What is the purpose of the
file.print ""
That is, opening a file and then closing it without writing anything
still creates it in all OSes, right? Isn't this routine doing the
same thing as:
def write_content(file)
(content = property(:content)) && content.write(file)l
end
If you say 'ensure => file' but don't specify content, we need to just
create an empty file, and this seemed the easiest way to do that. I
suppose :ensure could instead set 'content' to "", but I don't know
that that's particularly cleaner.
--
The most dangerous strategy is to jump a chasm in two leaps.
-- Benjamin Disraeli
---------------------------------------------------------------------
Luke Kanies -|- http://puppetlabs.com -|- +1(615)594-8199
--
You received this message because you are subscribed to the Google Groups "Puppet
Developers" group.
To post to this group, send email to puppet-...@googlegroups.com.
To unsubscribe from this group, send email to
puppet-dev+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/puppet-dev?hl=en.
