From 54c57febcb60076b819de75ac30ef50c6883f3b9 Mon Sep 17 00:00:00 2001
From: Quinton Miller <nicetas.c@gmail.com>
Date: Sat, 13 May 2023 04:50:17 +0800
Subject: [PATCH] Allow `File.delete` to remove read-only files on Windows

---
 spec/std/file_spec.cr            | 18 ++++++++++++++++--
 spec/support/tempfile.cr         | 22 ++--------------------
 src/crystal/system/win32/file.cr | 10 ++++++++++
 3 files changed, 28 insertions(+), 22 deletions(-)

diff --git a/spec/std/file_spec.cr b/spec/std/file_spec.cr
index 190b718e217c..09731808e459 100644
--- a/spec/std/file_spec.cr
+++ b/spec/std/file_spec.cr
@@ -220,7 +220,7 @@ describe "File" do
       other = datapath("test_file.ini")
 
       with_tempfile("test_file_symlink.txt") do |symlink|
-        File.symlink(File.real_path(file), symlink)
+        File.symlink(File.realpath(file), symlink)
 
         File.same?(file, symlink).should be_false
         File.same?(file, symlink, follow_symlinks: true).should be_true
@@ -514,7 +514,7 @@ describe "File" do
     end
   end
 
-  describe "delete" do
+  describe ".delete" do
     it "deletes a file" do
       with_tempfile("delete-file.txt") do |filename|
         File.open(filename, "w") { }
@@ -533,6 +533,20 @@ describe "File" do
       end
     end
 
+    it "deletes a read-only file" do
+      with_tempfile("delete-file-dir") do |path|
+        Dir.mkdir(path)
+        File.chmod(path, 0o755)
+
+        filename = File.join(path, "foo")
+        File.open(filename, "w") { }
+        File.exists?(filename).should be_true
+        File.chmod(filename, 0o000)
+        File.delete(filename)
+        File.exists?(filename).should be_false
+      end
+    end
+
     it "deletes? a file" do
       with_tempfile("delete-file.txt") do |filename|
         File.open(filename, "w") { }
diff --git a/spec/support/tempfile.cr b/spec/support/tempfile.cr
index 43c7df1a2ae6..a5e0c57d010a 100644
--- a/spec/support/tempfile.cr
+++ b/spec/support/tempfile.cr
@@ -31,7 +31,7 @@ def with_tempfile(*paths, file = __FILE__, &)
   ensure
     if SPEC_TEMPFILE_CLEANUP
       paths.each do |path|
-        rm_rf(path) if File.exists?(path)
+        FileUtils.rm_rf(path) if File.exists?(path)
       end
     end
   end
@@ -74,24 +74,6 @@ end
 
 if SPEC_TEMPFILE_CLEANUP
   at_exit do
-    rm_rf(SPEC_TEMPFILE_PATH) if Dir.exists?(SPEC_TEMPFILE_PATH)
-  end
-end
-
-private def rm_rf(path : String) : Nil
-  if Dir.exists?(path) && !File.symlink?(path)
-    Dir.each_child(path) do |entry|
-      src = File.join(path, entry)
-      rm_rf(src)
-    end
-    Dir.delete(path)
-  else
-    begin
-      File.delete(path)
-    rescue File::AccessDeniedError
-      # To be able to delete read-only files (e.g. ones under .git/) on Windows.
-      File.chmod(path, 0o666)
-      File.delete(path)
-    end
+    FileUtils.rm_rf(SPEC_TEMPFILE_PATH) if Dir.exists?(SPEC_TEMPFILE_PATH)
   end
 end
diff --git a/src/crystal/system/win32/file.cr b/src/crystal/system/win32/file.cr
index 21930501eaf7..0dddf9d2a417 100644
--- a/src/crystal/system/win32/file.cr
+++ b/src/crystal/system/win32/file.cr
@@ -247,11 +247,21 @@ module Crystal::System::File
       return false
     end
 
+    # Windows cannot delete read-only files, so we unset the attribute here, but
+    # restore it afterwards if deletion still failed
+    read_only_removed = false
+    if attributes.bits_set?(LibC::FILE_ATTRIBUTE_READONLY)
+      if LibC.SetFileAttributesW(win_path, attributes & ~LibC::FILE_ATTRIBUTE_READONLY) != 0
+        read_only_removed = true
+      end
+    end
+
     # all reparse point directories should be deleted like a directory, not just
     # symbolic links, so we don't care about the reparse tag here
     is_reparse_dir = attributes.bits_set?(LibC::FILE_ATTRIBUTE_REPARSE_POINT) && attributes.bits_set?(LibC::FILE_ATTRIBUTE_DIRECTORY)
     result = is_reparse_dir ? LibC._wrmdir(win_path) : LibC._wunlink(win_path)
     return true if result == 0
+    LibC.SetFileAttributesW(win_path, attributes) if read_only_removed
     raise ::File::Error.from_errno("Error deleting file", file: path)
   end