Make link tests work on OS X, enable link.mode on OS X

chef/lib/chef/file_access_control/unix.rb

@@ -29,7 +29,7 @@ module Unix
       def set_all
         set_owner
         set_group
-        set_mode unless resource.instance_of?(Chef::Resource::Link)
+        set_mode
       end
 
       # Workaround the fact that Ruby's Etc module doesn't believe in negative
@@ -87,32 +87,39 @@ def set_group
         end
       end
 
-      # TODO rename this to a more generic target_permissions
       def target_mode
         return nil if resource.mode.nil?
         (resource.mode.respond_to?(:oct) ? resource.mode.oct : resource.mode.to_i) & 007777
       end
 
-      # TODO rename this to a more generic set_permissions
       def set_mode
         if (mode = target_mode) && (mode != (stat.mode & 007777))
-          File.chmod(target_mode, file)
+          chmod(target_mode, file)
           Chef::Log.info("#{log_string} mode changed to #{mode.to_s(8)}")
           modified
         end
       end
 
       def stat
-        if resource.instance_of?(Chef::Resource::Link)
-          @stat ||= ::File.lstat(file)
+        if File.symlink?(file)
+          @stat ||= File.lstat(file)
         else
-          @stat ||= ::File.stat(file)
+          @stat ||= File.stat(file)
         end
       end
 
       private
+
+      def chmod(mode, file)
+        if File.symlink?(file)
+          File.lchmod(mode, file)
+        else
+          File.chmod(mode, file)
+        end
+      end
+
       def chown(uid, gid, file)
-        if resource.instance_of?(Chef::Resource::Link)
+        if ::File.symlink?(file)
           File.lchown(uid, gid, file)
         else
           File.chown(uid, gid, file)

chef/lib/chef/resource/link.rb

@@ -62,22 +62,6 @@ def link_type(arg=nil)
         )
       end
 
-      def group(arg=nil)
-        set_or_return(
-          :group,
-          arg,
-          :regex => Chef::Config[:group_valid_regex]
-        )
-      end
-
-      def owner(arg=nil)
-        set_or_return(
-          :owner,
-          arg,
-          :regex => Chef::Config[:user_valid_regex]
-        )
-      end
-
     end
   end
 end

chef/spec/functional/resource/link_spec.rb

@@ -18,16 +18,15 @@
 
 require 'spec_helper'
 
-if Chef::Platform.windows?
+if windows?
   require 'chef/win32/file' #probably need this in spec_helper
 end
 
 describe Chef::Resource::Link do
-
   let(:file_base) { "file_spec" }
 
   let(:base_dir) do
-    if Chef::Platform.windows?
+    if windows?
       Chef::Win32::File.get_long_path_name(Dir.tmpdir.gsub('/', '\\'))
     else
       Dir.tmpdir
@@ -53,31 +52,31 @@
   end
 
   def canonicalize(path)
-    Chef::Platform.windows? ? path.gsub('/', '\\') : path
+    windows? ? path.gsub('/', '\\') : path
   end
   def symlink(a, b)
-    if Chef::Platform.windows?
+    if windows?
       Chef::Win32::File.symlink(a, b)
     else
       File.symlink(a, b)
     end
   end
   def symlink?(file)
-    if Chef::Platform.windows?
+    if windows?
       Chef::Win32::File.symlink?(file)
     else
       File.symlink?(file)
     end
   end
   def readlink(file)
-    if Chef::Platform.windows?
+    if windows?
       Chef::Win32::File.readlink(file)
     else
       File.readlink(file)
     end
   end
   def link(a, b)
-    if Chef::Platform.windows?
+    if windows?
       Chef::Win32::File.link(a, b)
     else
       File.link(a, b)
@@ -311,7 +310,13 @@ def create_resource
           Dir.mkdir(target_file)
         end
         it 'create errors out' do
-          lambda { resource.run_action(:create) }.should raise_error(Chef::Platform.windows? ? Errno::EACCES : Errno::EISDIR)
+          if windows?
+            lambda { resource.run_action(:create) }.should raise_error(Errno::EACCES)
+          elsif os_x?
+            lambda { resource.run_action(:create) }.should raise_error(Errno::EPERM)
+          else
+            lambda { resource.run_action(:create) }.should raise_error(Errno::EISDIR)
+          end
         end
         it_behaves_like 'delete errors out'
       end
@@ -460,19 +465,25 @@ def denied_acl(sid, expected_perms)
           Dir.mkdir(target_file)
         end
         it 'errors out' do
-          lambda { resource.run_action(:create) }.should raise_error(Chef::Platform.windows? ? Errno::EACCES : Errno::EISDIR)
+          if windows?
+            lambda { resource.run_action(:create) }.should raise_error(Errno::EACCES)
+          elsif os_x?
+            lambda { resource.run_action(:create) }.should raise_error(Errno::EPERM)
+          else
+            lambda { resource.run_action(:create) }.should raise_error(Errno::EISDIR)
+          end
         end
         it_behaves_like 'delete errors out'
       end
       context "and the link already exists and is not writeable to this user", :pending do
       end
       context "and specifies security attributes" do
         before(:each) do
-          resource.owner(Chef::Platform.windows? ? 'Guest' : 'nobody')
+          resource.owner(windows? ? 'Guest' : 'nobody')
         end
         it 'ignores them' do
           resource.run_action(:create)
-          if Chef::Platform.windows?
+          if windows?
             Chef::Win32::Security.get_named_security_info(target_file).owner.should_not == SID.Guest
           else
             File.lstat(target_file).uid.should_not == Etc.getpwnam('nobody').uid
@@ -486,7 +497,7 @@ def denied_acl(sid, expected_perms)
       end
       context 'and the link does not yet exist' do
         it 'create errors out' do
-          lambda { resource.run_action(:create) }.should raise_error(Chef::Platform.windows? ? Chef::Exceptions::Win32APIError : Errno::EPERM)
+          lambda { resource.run_action(:create) }.should raise_error(windows? ? Chef::Exceptions::Win32APIError : Errno::EPERM)
         end
         include_context 'delete is noop'
       end
@@ -507,8 +518,11 @@ def denied_acl(sid, expected_perms)
           it 'links to the target file' do
             resource.run_action(:create)
             File.exists?(target_file).should be_true
-            symlink?(target_file).should be_true
-            readlink(target_file).should == @other_target
+            # OS X gets angry about this sort of link.  Bug in OS X, IMO.
+            pending('OS X symlink? and readlink working on hard links to symlinks', :if => os_x?) do
+              symlink?(target_file).should be_true
+              readlink(target_file).should == @other_target
+            end
           end
           include_context 'delete is noop'
         end
@@ -522,15 +536,17 @@ def denied_acl(sid, expected_perms)
         end
         context 'and the link does not yet exist' do
           it 'links to the target file' do
-            resource.run_action(:create)
-            # Windows and Unix have different definitions of exists? here, and that's OK.
-            if Chef::Platform.windows?
-              File.exists?(target_file).should be_true
-            else
-              File.exists?(target_file).should be_false
+            pending('OS X fails to create hardlinks to broken symlinks', :if => os_x?) do
+              resource.run_action(:create)
+              # Windows and Unix have different definitions of exists? here, and that's OK.
+              if windows?
+                File.exists?(target_file).should be_true
+              else
+                File.exists?(target_file).should be_false
+              end
+              symlink?(target_file).should be_true
+              readlink(target_file).should == @other_target
             end
-            symlink?(target_file).should be_true
-            readlink(target_file).should == @other_target
           end
           include_context 'delete is noop'
         end

chef/spec/support/platform_helpers.rb

@@ -16,4 +16,8 @@ def unix?
   !windows?
 end
 
+def os_x?
+  !!(RUBY_PLATFORM =~ /darwin/)
+end
+
 DEV_NULL = windows? ? 'NUL' : '/dev/null'

chef/spec/support/shared/functional/securable_resource.rb

@@ -46,17 +46,21 @@
     end
 
     it "should set permissions in string form as an octal number" do
-      mode_string = '777'
-      resource.mode mode_string
-      resource.run_action(:create)
-      (File.lstat(path).mode & 007777).should == (mode_string.oct & 007777)
+      pending('Linux does not support lchmod', :if => resource.instance_of?(Chef::Resource::Link) && !os_x?) do
+        mode_string = '776'
+        resource.mode mode_string
+        resource.run_action(:create)
+        (File.lstat(path).mode & 007777).should == (mode_string.oct & 007777)
+      end
     end
 
     it "should set permissions in numeric form as a ruby-interpreted octal" do
-      mode_integer = 0777
-      resource.mode mode_integer
-      resource.run_action(:create)
-      (File.lstat(path).mode & 007777).should == (mode_integer & 007777)
+      pending('Linux does not support lchmod', :if => resource.instance_of?(Chef::Resource::Link) && !os_x?) do
+        mode_integer = 0776
+        resource.mode mode_integer
+        resource.run_action(:create)
+        (File.lstat(path).mode & 007777).should == (mode_integer & 007777)
+      end
     end
   end