[Puppet-dev] (puppetlabs/puppet) (#10417) Disable show diffs on Windows by default

[wearetherobots]

Please review pull request #393: (#10417) Disable show diffs on Windows by default opened by (joshcooper)

Description:

Previously, anyone running puppet apply or puppet agent with
either the --test, --noop, or --show_diffs option, would see a
CreateProcess exception while trying to execute the diff
command. This was made worse by the recent change that causes puppet
to diff its last run summary file.

On Windows, diff is not present, by default. There is fc.exe (file
compare), but it is rather brain-dead, especially if you diff a binary
file.

This commit makes Puppet::Util::Diff.diff return '' by default on
Windows. However, if a diff command has been specified in puppet.conf,
then puppet will use it, as before.

• Opened: Mon Jan 23 18:23:07 UTC 2012
• Based on: puppetlabs:2.7.x (0a4d48fd26bf0aadb533a53fcff9d55ae9883f5f)
• Requested merge: joshcooper:ticket/2.7.x/10417-no-diff-on-windows (95979a3f17a9a51f9d6579ee45d08584f8e95324)

Diff follows:

diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb
index 26ee43c..e4c11e5 100644
--- a/lib/puppet/defaults.rb
+++ b/lib/puppet/defaults.rb
@@ -108,7 +108,10 @@ module Puppet
       we know nothing about."
     },
     :diff_args => ["-u", "Which arguments to pass to the diff command when printing differences between files."],
-    :diff => ["diff", "Which diff command to use when printing differences between files."],
+    :diff => {
+      :default => (Puppet.features.microsoft_windows? ? "" : "diff"),
+      :desc    => "Which diff command to use when printing differences between files.",
+    },
     :show_diff => [false, "Whether to log and report a contextual diff when files are being replaced.  This causes
       partial file contents to pass through Puppet's normal logging and reporting system, so this setting should be
       used with caution if you are sending Puppet's reports to an insecure destination.
diff --git a/lib/puppet/util/diff.rb b/lib/puppet/util/diff.rb
index 131241b..e89af7c 100644
--- a/lib/puppet/util/diff.rb
+++ b/lib/puppet/util/diff.rb
@@ -4,6 +4,8 @@ module Puppet::Util::Diff
   require 'tempfile'
 
   def diff(old, new)
+    return '' if Puppet[:diff].empty?
+
     command = [Puppet[:diff]]
     if args = Puppet[:diff_args] and args != ""
       command << args
diff --git a/spec/integration/defaults_spec.rb b/spec/integration/defaults_spec.rb
index 02dbf96..b641b20 100755
--- a/spec/integration/defaults_spec.rb
+++ b/spec/integration/defaults_spec.rb
@@ -309,4 +309,14 @@
       end
     end
   end
+
+  describe "diff" do
+    it "should default to 'diff' on POSIX", :unless => Puppet.features.microsoft_windows? do
+      Puppet.settings[:diff].should == 'diff'
+    end
+
+    it "should default to '' on Windows", :if => Puppet.features.microsoft_windows? do
+      Puppet.settings[:diff].should == ''
+    end
+  end
 end
diff --git a/spec/unit/util/diff_spec.rb b/spec/unit/util/diff_spec.rb
new file mode 100755
index 0000000..75b2c03
--- /dev/null
+++ b/spec/unit/util/diff_spec.rb
@@ -0,0 +1,30 @@
+#!/usr/bin/env rspec
+require 'spec_helper'
+require 'puppet/util/diff'
+
+describe Puppet::Util::Diff do
+  describe ".diff" do
+    it "should execute the diff command with arguments" do
+      Puppet.stubs(:[]).with(:diff).returns('foo')
+      Puppet.stubs(:[]).with(:diff_args).returns('bar')
+
+      subject.expects(:execute).with(['foo', 'bar', 'a', 'b'], {:failonfail => false}).returns('baz')
+      subject.diff('a', 'b').should == 'baz'
+    end
+
+    it "should omit diff arguments if none are specified" do
+      Puppet.stubs(:[]).with(:diff).returns('foo')
+      Puppet.stubs(:[]).with(:diff_args).returns(nil)
+
+      subject.expects(:execute).with(['foo', 'a', 'b'], {:failonfail => false}).returns('baz')
+      subject.diff('a', 'b').should == 'baz'
+    end
+
+    it "should not execute the diff command if no diff command specified" do
+      Puppet.stubs(:[]).with(:diff).returns('')
+
+      subject.expects(:execute).never
+      subject.diff('a', 'b').should == ''
+    end
+  end
+end

    

--
You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To post to this group, send email to puppet-dev@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.