Merge branch 'mr/rm_and_symlinks' into 'master'

Ensure fallback function for windows unlink handles links correctly

See merge request it/e3-core!66

src/e3/os/windows/fs.py

@@ -19,6 +19,8 @@
     Access,
     FileAttribute,
     FileInfo,
+    FindData,
+    IOReparseTag,
     IOStatusBlock,
     NTException,
     ObjectAttributes,
@@ -217,6 +219,30 @@ def uid(self) -> int:
 
         return result.index_number
 
+    @property
+    def reparse_tag(self) -> int:
+        """Find the reparse point tag for a given file.
+
+        :return: the tag as int. 0 is returned if not a reparse point
+        """
+        if not self.is_reparse_point:
+            return 0
+
+        result = FindData()
+        find_first_file: Callable = NT.FindFirstFile  # type: ignore
+        find_close: Callable = NT.FindClose  # type: ignore
+
+        handle = find_first_file(c_wchar_p(self.path), pointer(result))
+        if not handle:
+            raise NTException(
+                status=handle,
+                message=f"cannot find volume for {self.path}",
+                origin="NTFile.reparse_tag",
+            )
+        find_close(handle)
+
+        return result.reserved0
+
     def read_attributes_internal(self) -> None:
         """Retrieve file basic attributes (internal function).
 
@@ -305,6 +331,22 @@ def is_readonly(self) -> bool:
         """
         return self.basic_info.file_attributes.attr & FileAttribute.READONLY > 0
 
+    @property
+    def is_reparse_point(self) -> bool:
+        """Check if a given file is a reparse point.
+
+        :return: True if the file is a reparse point, False otherwise
+        """
+        return self.basic_info.file_attributes.attr & FileAttribute.REPARSE_POINT > 0
+
+    @property
+    def is_symlink(self) -> bool:
+        """Check whether a given file is a symlink or not.
+
+        :return: return True for all kind of symlinks (native and WSL).
+        """
+        return self.reparse_tag in (IOReparseTag.SYMLINK, IOReparseTag.WSL_SYMLINK)
+
     @property
     def trash_path(self) -> str:
         """Return path in which the file can move safely for deletion.
@@ -503,6 +545,10 @@ def unlink(self) -> None:
             self.basic_info.file_attributes.attr &= ~FileAttribute.READONLY
             self.write_attributes()
 
+        if self.is_symlink:
+            # If this is a symlink ensure that we don't delete the target
+            open_options |= OpenOptions.OPEN_REPARSE_POINT
+
         # set our access modes
         desired_access = Access.DELETE
         shared_access = Share.DELETE

src/e3/os/windows/native_api.py

@@ -18,6 +18,7 @@
     LPWSTR,
     ULONG,
     USHORT,
+    WORD,
 )
 from datetime import datetime
 
@@ -58,6 +59,24 @@ def __str__(self) -> str:
         return ",".join(result)
 
 
+class IOReparseTag:
+    """Reparse Point Tag constants.
+
+    This is important to note that symbolic links on Windows are always implemented
+    using reparse points. Nevertheless a reparse point is a more general concept not
+    always associated with the concept of symbolic links. In the present code we are
+    only interested in checking whether a reparse point is a symbolic link or not.
+
+    Currenly Windows supports two kinds of symbolic links. One for Win32 apps (SYMLINK)
+    and one for WSL subsystem (WSL_SYMLINK). Note that Cygwin now uses the second one
+    to implement symbolic links. Note that WSL symbolic links are not handled correctly
+    by the Python runtime (for example os.path.islink will return False).
+    """
+
+    SYMLINK = 0xA000000C
+    WSL_SYMLINK = 0xA000001D
+
+
 class Access:
     """Desired Access constants."""
 
@@ -95,6 +114,7 @@ class OpenOptions:
     BACKUP_INTENT = 0x00004000
     SYNCHRONOUS_IO_NON_ALERT = 0x00000020
     DELETE_ON_CLOSE = 0x00001000
+    OPEN_REPARSE_POINT = 0x00200000
 
 
 class Wait:
@@ -160,37 +180,113 @@ def __len__(self) -> int:
         return self.length
 
 
+# Offset in seconds between Windows and Linux epoch. Windows use January 1st 1601
+# whereas Posix systems are using January 1st 1970.
+W32_EPOCH_OFFSET = 11_644_473_600
+
+
 class FileTime(Structure):
     """Map FILETIME structure."""
 
+    # Note: in a previous implementation that structure was directly mapped to
+    # LARGE_INTEGER Doing that is wrong as it force an implicit 8 byte alignment.
+    # Some structures do not respect that alignment (See FindData for example).
+    _fields_ = [("filetime_low", DWORD), ("filetime_high", DWORD)]
+
+    def __init__(self, t: datetime) -> None:
+        # Transform date to Windows timestamp
+        timestamp = (t - datetime(1970, 1, 1)).total_seconds()
+
+        # Windows use hundreds of ns as unit
+        timestamp = (int(timestamp) + W32_EPOCH_OFFSET) * 10_000_000
+
+        Structure.__init__(self, timestamp % 2**32, timestamp // 2**32)
+
+    @property
+    def filetime(self) -> int:
+        return self.filetime_low + self.filetime_high * 2**32
+
+    @property
+    def as_datetime(self) -> datetime:
+        try:
+            return datetime.fromtimestamp(
+                self.filetime // 10_000_000 - W32_EPOCH_OFFSET
+            )
+        except ValueError as err:  # defensive code
+            # Add some information to ease debugging
+            raise ValueError(f"filetime '{self.filetime}' failed with {err}") from err
+
+    def __str__(self) -> str:
+        try:
+            return str(time.ctime(self.filetime // 10_000_000 - W32_EPOCH_OFFSET))
+        except ValueError:  # defensive code
+            return "none"
+
+
+class LargeFileTime(Structure):
+    """Map filetime implemented using LARGE_INTEGER."""
+
+    # Contrary to WIN32 API, Native API use LARGE_INTEGER instead of a tuple
+    # of DWORD. This means that there is an implicit alignment constraint of
+    # 8 bytes. As consequence even if similar, this should not be merged with
+    # FileTime.
     _fields_ = [("filetime", LARGE_INTEGER)]
 
     def __init__(self, t: datetime) -> None:
+        # Transform date to Windows timestamp
         timestamp = (t - datetime(1970, 1, 1)).total_seconds()
-        timestamp = (int(timestamp) + 11644473600) * 10000000
+
+        # Windows use hundreds of ns as unit
+        timestamp = (int(timestamp) + W32_EPOCH_OFFSET) * 10_000_000
         Structure.__init__(self, timestamp)
 
     @property
     def as_datetime(self) -> datetime:
         try:
-            return datetime.fromtimestamp(self.filetime // 10000000 - 11644473600)
+            return datetime.fromtimestamp(
+                self.filetime // 10_000_000 - W32_EPOCH_OFFSET
+            )
         except ValueError as err:  # defensive code
             # Add some information to ease debugging
             raise ValueError(f"filetime '{self.filetime}' failed with {err}") from err
 
     def __str__(self) -> str:
         try:
-            return str(time.ctime(self.filetime // 10000000 - 11644473600))
+            return str(time.ctime(self.filetime // 10_000_000 - W32_EPOCH_OFFSET))
         except ValueError:  # defensive code
             return "none"
 
 
+class FindData(Structure):
+    _fields_ = [
+        ("file_attributes", FileAttribute),
+        ("creation_time", FileTime),
+        ("last_access_time", FileTime),
+        ("last_write_time", FileTime),
+        ("file_size0", DWORD),
+        ("file_size1", DWORD),
+        # When the file is a reparse point, reserved0 field contains the reparse point
+        # tag (i.e: the reparse point kind).
+        ("reserved0", DWORD),
+        ("reserved1", DWORD),
+        ("filename", ctypes.c_wchar * 260),
+        ("dos_filename", ctypes.c_wchar * 14),
+        ("unused0", DWORD),
+        ("unused1", DWORD),
+        ("unused2", WORD),
+    ]
+
+
 class FileInfo:
     """Declaration of structures returned by QueryInformationFile."""
 
     class Names:
         class_id = 12
 
+    class ReparsePoint(Structure):
+        _fields_ = [("file_reference", LARGE_INTEGER), ("tag", ULONG)]
+        class_id = 33
+
     class Disposition(Structure):
         _fields_ = [("delete_file", BOOLEAN)]
         class_id = 13
@@ -205,10 +301,10 @@ class Rename(Structure):
 
     class Basic(Structure):
         _fields_ = [
-            ("creation_time", FileTime),
-            ("last_access_time", FileTime),
-            ("last_write_time", FileTime),
-            ("change_time", FileTime),
+            ("creation_time", LargeFileTime),
+            ("last_access_time", LargeFileTime),
+            ("last_write_time", LargeFileTime),
+            ("change_time", LargeFileTime),
             ("file_attributes", FileAttribute),
         ]
         class_id = 4
@@ -286,6 +382,8 @@ def __init__(self, name: UnicodeString, parent: HANDLE | None = None):
 
 # Declare the Win32 functions return types and signature
 class NT:
+    FindFirstFile = None
+    FindClose = None
     Sleep = None
     GetVolumePathName = None
     SetInformationFile = None
@@ -305,6 +403,14 @@ def init_api(cls) -> None:
             kernel32 = ctypes.windll.kernel32
             ntdll = ctypes.windll.ntdll
 
+            cls.FindFirstFile = kernel32.FindFirstFileW
+            cls.FindFirstFile.restype = HANDLE
+            cls.FindFirstFile.argtypes = [c_wchar_p, POINTER(FindData)]
+
+            cls.FindClose = kernel32.FindClose
+            cls.FindClose.restype = BOOL
+            cls.FindClose.argtypes = [HANDLE]
+
             cls.GetVolumePathName = kernel32.GetVolumePathNameW
             cls.GetVolumePathName.restype = BOOL
             cls.GetVolumePathName.argtypes = [c_wchar_p, c_wchar_p, DWORD]

tests/tests_e3/fs/main_test.py

@@ -442,12 +442,23 @@ def test_rm_list():
     assert not os.path.exists("b")
 
 
-@pytest.mark.skipif(sys.platform == "win32", reason="test using symlink")
-def test_rm_symlink_to_dir():
+def test_rm_symlink():
     e3.fs.mkdir("a")
-    os.symlink("a", "b")
+    try:
+        os.symlink("a", "b")
+    except Exception:
+        # This means symlinks are not supported on that system or not allowed
+        return
+
     e3.fs.rm("b", recursive=True)
     assert not os.path.exists("b")
+    assert os.path.exists("a")
+
+    e3.os.fs.touch("d")
+    os.symlink("d", "e")
+    e3.fs.rm("e", recursive=True)
+    assert not os.path.exists("e")
+    assert os.path.exists("d")
 
 
 def test_safe_copy():

tests/tests_e3/os/windows/fs/main_test.py

@@ -13,7 +13,7 @@
     from e3.os.windows.fs import NTFile
     from e3.os.windows.native_api import (
         Access,
-        FileTime,
+        LargeFileTime,
         NTException,
         Share,
         FileAttribute,
@@ -60,14 +60,16 @@ def test_write_attributes():
     ntfile = NTFile(test_file_path)
     ntfile.read_attributes()
     ntfile.open(Access.READ_ATTRS)
-    ntfile.basic_info.change_time = FileTime(datetime.now() - timedelta(seconds=3600))
+    ntfile.basic_info.change_time = LargeFileTime(
+        datetime.now() - timedelta(seconds=3600)
+    )
     assert str(time.localtime().tm_year) in str(ntfile.basic_info.change_time)
     try:
         with pytest.raises(NTException):
             ntfile.write_attributes()
     finally:
         ntfile.close()
-    ntfile.basic_info.change_time = FileTime(datetime.now() - timedelta(days=3))
+    ntfile.basic_info.change_time = LargeFileTime(datetime.now() - timedelta(days=3))
     ntfile.write_attributes()
     assert datetime.now() - ntfile.basic_info.change_time.as_datetime > timedelta(
         seconds=3000