Fix Windows Sys.rename regression from ocaml#12184 (ocaml#12320)

Author: jmid

Changes

@@ -292,7 +292,7 @@ OCaml 5.1.0
   (Daniel Bünzli, review by Jeremy Yallop, Gabriel Scherer, Wiktor Kuchta,
    Nicolás Ojeda Bär)
 
-- #12184: Sys.rename Windows fixes on directory corner cases.
+- #12184, #12320: Sys.rename Windows fixes on directory corner cases.
   (Jan Midtgaard, review by Anil Madhavapeddy)
 
 * #11565: Enable -strict-formats by default. Some incorrect format

runtime/win32.c

@@ -779,26 +779,17 @@ CAMLexport wchar_t *caml_win32_getenv(wchar_t const *lpName)
 
 int caml_win32_rename(const wchar_t * oldpath, const wchar_t * newpath)
 {
-  /* First handle corner-cases not handled by MoveFileEx:
-     - dir to empty dir - positive - should succeed
+  /* First handle corner-case not handled by MoveFileEx:
      - dir to existing file - should fail */
+  DWORD new_attribs;
   DWORD old_attribs = GetFileAttributes(oldpath);
   if ((old_attribs != INVALID_FILE_ATTRIBUTES) &&
-      (old_attribs & FILE_ATTRIBUTE_DIRECTORY) != 0 &&
-      (old_attribs & FILE_ATTRIBUTE_HIDDEN) == 0 &&
-      (old_attribs & FILE_ATTRIBUTE_SYSTEM) == 0) {
-    DWORD new_attribs = GetFileAttributes(newpath);
+      (old_attribs & FILE_ATTRIBUTE_DIRECTORY) != 0) {
+    new_attribs = GetFileAttributes(newpath);
     if ((new_attribs != INVALID_FILE_ATTRIBUTES) &&
-        (new_attribs & FILE_ATTRIBUTE_HIDDEN) == 0 &&
-        (new_attribs & FILE_ATTRIBUTE_SYSTEM) == 0) {
-      if ((new_attribs & FILE_ATTRIBUTE_DIRECTORY) != 0) {
-        /* Try to delete and fall though.
-           RemoveDirectoryW fails on non-empty dirs as intended. */
-        RemoveDirectoryW(newpath);
-      } else {
+        (new_attribs & FILE_ATTRIBUTE_DIRECTORY) == 0) {
         errno = ENOTDIR;
         return -1;
-      }
     }
   }
   /* MOVEFILE_REPLACE_EXISTING: to be closer to POSIX
@@ -812,6 +803,23 @@ int caml_win32_rename(const wchar_t * oldpath, const wchar_t * newpath)
                  MOVEFILE_COPY_ALLOWED)) {
     return 0;
   }
+
+  /* Another cornercase not handled by MoveFileEx:
+     - dir to empty dir - positive - should succeed */
+  if ((old_attribs != INVALID_FILE_ATTRIBUTES) &&
+      (old_attribs & FILE_ATTRIBUTE_DIRECTORY) != 0 &&
+      (new_attribs != INVALID_FILE_ATTRIBUTES) &&
+      (new_attribs & FILE_ATTRIBUTE_DIRECTORY) != 0) {
+    /* Try to delete: RemoveDirectoryW fails on non-empty dirs as intended.
+       Then try again. */
+    RemoveDirectoryW(newpath);
+    if (MoveFileEx(oldpath, newpath,
+                   MOVEFILE_REPLACE_EXISTING | MOVEFILE_WRITE_THROUGH |
+                   MOVEFILE_COPY_ALLOWED)) {
+      return 0;
+    }
+  }
+
   /* Modest attempt at mapping Win32 error codes to POSIX error codes.
      The __dosmaperr() function from the CRT does a better job but is
      generally not accessible. */

testsuite/tests/lib-sys/rename.ml

@@ -89,6 +89,11 @@ let _ =
   print_newline();
   safe_remove_dir "foo";
   safe_remove_dir "bar";
+  print_string "Rename existing empty directory to itself: ";
+  Sys.mkdir "foo" 0o755;
+  testrenamedir "foo" "foo";
+  print_newline();
+  safe_remove_dir "foo";
   print_string "Rename directory to existing file: ";
   Sys.mkdir "foo" 0o755;
   writefile f2 "xyz";

testsuite/tests/lib-sys/rename.reference

@@ -6,4 +6,5 @@ Rename directory to a nonexisting directory: passed
 Rename a nonexisting directory: fails as expected
 Rename directory to a non-empty directory: fails as expected
 Rename directory to existing empty directory: passed
+Rename existing empty directory to itself: source directory still exists!
 Rename directory to existing file: fails as expected