[#4182] show_diff was broken for streamed file contents

show_diff was written assuming that a file's contents would be loaded
into memory. That's no longer true, for perfomance reasons.
This patch streams the file to a temporary file to take the
diff.
As a consequence, it means that when show_diff is on, files may get
streamed twice.

2 files changed, 20 insertions, 3 deletions

diff --git a/lib/puppet/type/file/content.rb b/lib/puppet/type/file/content.rb
index 7d541645c..d7bb022a3 100755
--- a/lib/puppet/type/file/content.rb
+++ b/lib/puppet/type/file/content.rb
@@ -1,5 +1,6 @@
 require 'net/http'
 require 'uri'
+require 'tempfile'
 
 require 'puppet/util/checksums'
 require 'puppet/network/http/api/v1'
@@ -101,7 +102,9 @@ module Puppet
             result = super
 
             if ! result and Puppet[:show_diff]
-                string_file_diff(@resource[:path], actual_content)
+                write_temporarily do |path|
+                    print diff(@resource[:path], path)
+                end
             end
             return result
         end
@@ -137,6 +140,19 @@ module Puppet
             return return_event
         end
 
+        def write_temporarily
+            tempfile = Tempfile.new("puppet-file")
+            tempfile.open
+
+            write(tempfile)
+
+            tempfile.close
+
+            yield tempfile.path
+
+            tempfile.delete
+        end
+
         def write(file)
             resource.parameter(:checksum).sum_stream { |sum|
                 each_chunk_from(actual_content || resource.parameter(:source)) { |chunk|
diff --git a/spec/unit/type/file/content_spec.rb b/spec/unit/type/file/content_spec.rb
index f1001aa8c..2c00e50dc 100755
--- a/spec/unit/type/file/content_spec.rb
+++ b/spec/unit/type/file/content_spec.rb
@@ -175,14 +175,15 @@ describe content do
 
                 it "should display a diff if the current contents are different from the desired content" do 
                     @content.should = "some content"
-                    @content.expects(:string_file_diff).once
+                    @content.expects(:diff).returns("my diff").once
+                    @content.expects(:print).with("my diff").once
 
                     @content.insync?("other content")
                 end
 
                 it "should not display a diff if the sum for the current contents is the same as the sum for the desired content" do 
                     @content.should = "some content"
-                    @content.expects(:string_file_diff).never
+                    @content.expects(:diff).never
 
                     @content.insync?("{md5}" + Digest::MD5.hexdigest("some content"))
                 end