Fixed #3393 - Updates to Facter for MS Windows

This patch is originally by Daniel Berger <djberg96@gmail.com>, I
changed using Facter.value instead of repeatedly testing
Config['host_os'], removed Resolution::which, and fixed the specs.

Thanks to Paul Nasrat for helping with cross-platform debugging.

Signed-off-by: David Schmitt <david@dasz.at>

lib/facter/ipaddress.rb

@@ -111,30 +111,25 @@
 
 Facter.add(:ipaddress) do
     confine :kernel => %w{windows}
-    setcode do
-        ip = nil
-        output = %x{ipconfig}
-
-        output.split(/^\S/).each { |str|
-            if str =~ /IP Address.*: ([0-9]+\.[0-9]+\.[0-9]+\.[0-9]+)/
-                tmp = $1
-                unless tmp =~ /^127\./
-                    ip = tmp
-                    break
-                end
-            end
-        }
-        ip
-    end
+    require 'socket'
+    IPSocket.getaddress(Socket.gethostname)
 end
 
 Facter.add(:ipaddress, :ldapname => "iphostnumber", :timeout => 2) do
     setcode do
-        require 'resolv'
-
+        if Facter.value(:kernel) == 'windows'
+            require 'win32/resolv'
+        else
+            require 'resolv'
+        end
+
         begin
             if hostname = Facter.value(:hostname)
-                ip = Resolv.getaddress(hostname)
+                if Facter.value(:kernel) == 'windows'
+                    ip = Win32::Resolv.get_resolv_info.last[0]
+                else
+                    ip = Resolv.getaddress(hostname)                
+                end
                 unless ip == "127.0.0.1"
                     ip
                 end

lib/facter/kernel.rb

@@ -2,8 +2,10 @@
     setcode do
         require 'rbconfig'
         case Config::CONFIG['host_os']
-        when /mswin/i; 'windows'
-        else Facter::Util::Resolution.exec("uname -s")
+        when /mswin|win32|dos|cygwin|mingw/i
+            'windows'
+        else
+            Facter::Util::Resolution.exec("uname -s")
         end
     end
 end

lib/facter/macaddress.rb

@@ -67,13 +67,26 @@
 Facter.add(:macaddress) do
     confine :kernel => %w(windows)
     setcode do
-        ether = []
-        output = %x{ipconfig /all}
-        output.split(/\r\n/).each do |str|
-            if str =~  /.*Physical Address.*: (\w{1,2}-\w{1,2}-\w{1,2}-\w{1,2}-\w{1,2}-\w{1,2})/
-                ether.push($1.gsub(/-/, ":"))
-            end
-        end
-        ether[0]
+        require 'win32ole'
+        require 'socket'
+
+        ether = nil
+        host = Socket.gethostname
+        connect_string = "winmgmts://#{host}/root/cimv2"
+
+        wmi = WIN32OLE.connect(connect_string)
+
+        query = %Q{
+          select *
+          from Win32_NetworkAdapterConfiguration
+          where IPEnabled = True
+        }
+
+        wmi.ExecQuery(query).each{ |nic|
+          ether = nic.MacAddress
+          break
+        }
+
+        ether
     end
 end

lib/facter/util/resolution.rb

@@ -11,9 +11,13 @@
 class Facter::Util::Resolution
     attr_accessor :interpreter, :code, :name, :timeout
 
+    WINDOWS = Config::CONFIG['host_os'] =~ /mswin|win32|dos|mingw|cygwin/i
+
+    INTERPRETER = WINDOWS ? 'cmd.exe' : '/bin/sh'
+
     def self.have_which
         if ! defined?(@have_which) or @have_which.nil?
-            if Config::CONFIG['host_os'] =~ /mswin/
+            if Facter.value(:kernel) == 'windows'
                 @have_which = false
             else
                 %x{which which >/dev/null 2>&1}
@@ -23,31 +27,50 @@ def self.have_which
         @have_which
     end
 
-    # Execute a chunk of code.
-    def self.exec(code, interpreter = "/bin/sh")
-        raise ArgumentError, "non-sh interpreters are not currently supported" unless interpreter == "/bin/sh"
-        binary = code.split(/\s+/).shift
-
-        if have_which
+    # Execute a program and return the output of that program.
+    #
+    # Returns nil if the program can't be found, or if there is a problem
+    # executing the code.
+    #
+    def self.exec(code, interpreter = INTERPRETER)
+        raise ArgumentError, "invalid interpreter" unless interpreter == INTERPRETER
+
+        # Try to guess whether the specified code can be executed by looking at the
+        # first word. If it cannot be found on the PATH defer on resolving the fact
+        # by returning nil.
+        # This only fails on shell built-ins, most of which are masked by stuff in 
+        # /bin or of dubious value anyways. In the worst case, "sh -c 'builtin'" can
+        # be used to work around this limitation
+        #
+        # Windows' %x{} throws Errno::ENOENT when the command is not found, so we 
+        # can skip the check there. This is good, since builtins cannot be found 
+        # elsewhere.
+        if have_which and !WINDOWS
             path = nil
-            if binary !~ /^\//
+            binary = code.split.first
+            if code =~ /^\//
+                path = binary
+            else
                 path = %x{which #{binary} 2>/dev/null}.chomp
                 # we don't have the binary necessary
                 return nil if path == "" or path.match(/Command not found\./)
-            else
-                path = binary
             end
 
             return nil unless FileTest.exists?(path)
         end
 
         out = nil
+
         begin
             out = %x{#{code}}.chomp
+        rescue Errno::ENOENT => detail
+            # command not found on Windows
+            return nil
         rescue => detail
             $stderr.puts detail
             return nil
         end
+
         if out == ""
             return nil
         else
@@ -86,7 +109,7 @@ def limit
     def setcode(string = nil, interp = nil, &block)
         if string
             @code = string
-            @interpreter = interp || "/bin/sh"
+            @interpreter = interp || INTERPRETER
         else
             unless block_given?
                 raise ArgumentError, "You must pass either code or a block"

spec/unit/util/resolution.rb

@@ -44,9 +44,10 @@
             @resolve = Facter::Util::Resolution.new("yay")
         end
 
-        it "should default to /bin/sh as the interpreter if a string is provided" do
+        it "should default to the detected interpreter if a string is provided" do
+            Facter::Util::Resolution::INTERPRETER = "/bin/bar"
             @resolve.setcode "foo"
-            @resolve.interpreter.should == "/bin/sh"
+            @resolve.interpreter.should == "/bin/bar"
         end
 
         it "should set the code to any provided string" do
@@ -87,17 +88,44 @@
         end
 
         describe "and the code is a string" do
-            it "should return the result of executing the code with the interpreter" do
-                @resolve.setcode "/bin/foo"
-                Facter::Util::Resolution.expects(:exec).with("/bin/foo", "/bin/sh").returns "yup"
-
-                @resolve.value.should == "yup"
+            describe "on windows" do
+                before do
+                    Facter::Util::Resolution::WINDOWS = true
+                    Facter::Util::Resolution::INTERPRETER = "cmd.exe"
+                end
+
+                it "should return the result of executing the code with the interpreter" do
+                    @resolve.setcode "/bin/foo"
+                    Facter::Util::Resolution.expects(:exec).once.with("/bin/foo", "cmd.exe").returns "yup"
+
+                    @resolve.value.should == "yup"
+                end
+
+                it "should return nil if the value is an empty string" do
+                    @resolve.setcode "/bin/foo"
+                    Facter::Util::Resolution.expects(:exec).once.returns ""
+                    @resolve.value.should be_nil
+                end
             end
 
-            it "should return nil if the value is an empty string" do
-                @resolve.setcode "/bin/foo"
-                Facter::Util::Resolution.stubs(:exec).returns ""
-                @resolve.value.should be_nil
+            describe "on non-windows systems" do
+                before do
+                    Facter::Util::Resolution::WINDOWS = false
+                    Facter::Util::Resolution::INTERPRETER = "/bin/sh"
+                end
+
+                it "should return the result of executing the code with the interpreter" do
+                    @resolve.setcode "/bin/foo"
+                    Facter::Util::Resolution.expects(:exec).once.with("/bin/foo", "/bin/sh").returns "yup"
+
+                    @resolve.value.should == "yup"
+                end
+
+                it "should return nil if the value is an empty string" do
+                    @resolve.setcode "/bin/foo"
+                    Facter::Util::Resolution.expects(:exec).once.returns ""
+                    @resolve.value.should be_nil
+                end
             end
         end
 
@@ -233,5 +261,9 @@
         it "should fail if any interpreter other than /bin/sh is requested" do
             lambda { Facter::Util::Resolution.exec("/something", "/bin/perl") }.should raise_error(ArgumentError)
         end
+
+        it "should execute the binary" do
+            Facter::Util::Resolution.exec("echo foo").should == "foo"
+        end
     end
 end