Skip to content

Commit

Permalink
(PUP-5018) Cannot unlink dangling Windows symlinks
Browse files Browse the repository at this point in the history
 - When a symlink is created to point at a directory, then that target
   directory is removed, Windows was previously generating
   `Errno::EACCES: Permission denied @ unlink_internal` because the
   underlying Ruby File.unlink was being called, which fails on Windows.

   When the target no longer exists, Ruby's File.unlink only works
   properly on symlinks pointed at files and not symlinks pointed at
   directories.

   Given the lstat only specifies 'link' and does not describe the
   target, there is no way of knowing whether the former target was a
   file or directory.  Thefore, when the target stat does not exist,
   first attempt to File.unlink, and if that fails call Dir.rmdir.
   Ruby's Dir.rmdir only works on links that were targeted at
   directories, and not links that were targeted at files.
  • Loading branch information
Iristyle committed Aug 12, 2015
1 parent bc196fe commit 579d5ea
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 1 deletion.
4 changes: 3 additions & 1 deletion lib/puppet/file_system/windows.rb
Expand Up @@ -62,7 +62,9 @@ def unlink(*file_names)
stat = Puppet::Util::Windows::File.stat(file_name) rescue nil

# sigh, Ruby + Windows :(
if stat && stat.ftype == 'directory'
if !stat
::File.unlink(file_name) rescue Dir.rmdir(file_name)
elsif stat.ftype == 'directory'
if Puppet::Util::Windows::File.symlink?(file_name)
Dir.rmdir(file_name)
else
Expand Down
20 changes: 20 additions & 0 deletions spec/unit/file_system_spec.rb
Expand Up @@ -446,6 +446,26 @@ def increment_counter_in_multiple_processes(file, num_procs, options)
expect(Puppet::FileSystem.readlink(symlink)).to eq(missing_file.to_s)
end

it "should be able to unlink a dangling symlink pointed at a file" do
symlink = tmpfile("somefile_link")
Puppet::FileSystem.symlink(file, symlink)
::File.delete(file)
Puppet::FileSystem.unlink(symlink)

expect(Puppet::FileSystem).to_not be_exist(file)
expect(Puppet::FileSystem).to_not be_exist(symlink)
end

it "should be able to unlink a dangling symlink pointed at a directory" do
symlink = tmpfile("somedir_link")
Puppet::FileSystem.symlink(dir, symlink)
Dir.rmdir(dir)
Puppet::FileSystem.unlink(symlink)

expect(Puppet::FileSystem).to_not be_exist(dir)
expect(Puppet::FileSystem).to_not be_exist(symlink)
end

it "should delete only the symlink and not the target when calling unlink instance method" do
[file, dir].each do |target|
symlink = tmpfile("#{Puppet::FileSystem.basename(target)}_link")
Expand Down

0 comments on commit 579d5ea

Please sign in to comment.