From f09d184821efd9438d092643881e28bdf8de4de5 Mon Sep 17 00:00:00 2001 From: Barney Gale Date: Wed, 3 Jul 2024 04:30:29 +0100 Subject: [PATCH] GH-73991: Support copying directory symlinks on older Windows (#120807) Check for `ERROR_INVALID_PARAMETER` when calling `_winapi.CopyFile2()` and raise `UnsupportedOperation`. In `Path.copy()`, handle this exception and fall back to the `PathBase.copy()` implementation. --- Doc/library/pathlib.rst | 5 ----- Lib/pathlib/__init__.py | 4 ++-- Lib/pathlib/_abc.py | 11 +-------- Lib/pathlib/_local.py | 19 +++++++++------- Lib/pathlib/_os.py | 27 ++++++++++++++++++++--- Lib/test/test_pathlib/test_pathlib_abc.py | 3 ++- 6 files changed, 40 insertions(+), 29 deletions(-) diff --git a/Doc/library/pathlib.rst b/Doc/library/pathlib.rst index 0918bbb47e9..d7fd56f4c4f 100644 --- a/Doc/library/pathlib.rst +++ b/Doc/library/pathlib.rst @@ -1554,11 +1554,6 @@ Copying, renaming and deleting permissions. After the copy is complete, users may wish to call :meth:`Path.chmod` to set the permissions of the target file. - .. warning:: - On old builds of Windows (before Windows 10 build 19041), this method - raises :exc:`OSError` when a symlink to a directory is encountered and - *follow_symlinks* is false. - .. versionadded:: 3.14 diff --git a/Lib/pathlib/__init__.py b/Lib/pathlib/__init__.py index 4b3edf535a6..2298a249529 100644 --- a/Lib/pathlib/__init__.py +++ b/Lib/pathlib/__init__.py @@ -5,8 +5,8 @@ paths with operations that have semantics appropriate for different operating systems. """ -from ._abc import * +from ._os import * from ._local import * -__all__ = (_abc.__all__ + +__all__ = (_os.__all__ + _local.__all__) diff --git a/Lib/pathlib/_abc.py b/Lib/pathlib/_abc.py index 71973913921..b5f903ec1f0 100644 --- a/Lib/pathlib/_abc.py +++ b/Lib/pathlib/_abc.py @@ -16,10 +16,7 @@ import operator import posixpath from glob import _GlobberBase, _no_recurse_symlinks from stat import S_ISDIR, S_ISLNK, S_ISREG, S_ISSOCK, S_ISBLK, S_ISCHR, S_ISFIFO -from ._os import copyfileobj - - -__all__ = ["UnsupportedOperation"] +from ._os import UnsupportedOperation, copyfileobj @functools.cache @@ -27,12 +24,6 @@ def _is_case_sensitive(parser): return parser.normcase('Aa') == 'Aa' -class UnsupportedOperation(NotImplementedError): - """An exception that is raised when an unsupported operation is called on - a path object. - """ - pass - class ParserBase: """Base class for path parsers, which do low-level path manipulation. diff --git a/Lib/pathlib/_local.py b/Lib/pathlib/_local.py index 0105ea30424..acb57214b81 100644 --- a/Lib/pathlib/_local.py +++ b/Lib/pathlib/_local.py @@ -17,8 +17,8 @@ try: except ImportError: grp = None -from ._abc import UnsupportedOperation, PurePathBase, PathBase -from ._os import copyfile +from ._os import UnsupportedOperation, copyfile +from ._abc import PurePathBase, PathBase __all__ = [ @@ -791,12 +791,15 @@ class Path(PathBase, PurePath): try: target = os.fspath(target) except TypeError: - if isinstance(target, PathBase): - # Target is an instance of PathBase but not os.PathLike. - # Use generic implementation from PathBase. - return PathBase.copy(self, target, follow_symlinks=follow_symlinks) - raise - copyfile(os.fspath(self), target, follow_symlinks) + if not isinstance(target, PathBase): + raise + else: + try: + copyfile(os.fspath(self), target, follow_symlinks) + return + except UnsupportedOperation: + pass # Fall through to generic code. + PathBase.copy(self, target, follow_symlinks=follow_symlinks) def chmod(self, mode, *, follow_symlinks=True): """ diff --git a/Lib/pathlib/_os.py b/Lib/pathlib/_os.py index bbb019b6534..61923b5e410 100644 --- a/Lib/pathlib/_os.py +++ b/Lib/pathlib/_os.py @@ -20,6 +20,15 @@ except ImportError: _winapi = None +__all__ = ["UnsupportedOperation"] + + +class UnsupportedOperation(NotImplementedError): + """An exception that is raised when an unsupported operation is attempted. + """ + pass + + def get_copy_blocksize(infd): """Determine blocksize for fastcopying on Linux. Hopefully the whole file will be copied in a single call. @@ -106,18 +115,30 @@ if _winapi and hasattr(_winapi, 'CopyFile2') and hasattr(os.stat_result, 'st_fil Copy from one file to another using CopyFile2 (Windows only). """ if follow_symlinks: - flags = 0 + _winapi.CopyFile2(source, target, 0) else: + # Use COPY_FILE_COPY_SYMLINK to copy a file symlink. flags = _winapi.COPY_FILE_COPY_SYMLINK try: _winapi.CopyFile2(source, target, flags) return except OSError as err: # Check for ERROR_ACCESS_DENIED - if err.winerror != 5 or not _is_dirlink(source): + if err.winerror == 5 and _is_dirlink(source): + pass + else: raise + + # Add COPY_FILE_DIRECTORY to copy a directory symlink. flags |= _winapi.COPY_FILE_DIRECTORY - _winapi.CopyFile2(source, target, flags) + try: + _winapi.CopyFile2(source, target, flags) + except OSError as err: + # Check for ERROR_INVALID_PARAMETER + if err.winerror == 87: + raise UnsupportedOperation(err) from None + else: + raise else: copyfile = None diff --git a/Lib/test/test_pathlib/test_pathlib_abc.py b/Lib/test/test_pathlib/test_pathlib_abc.py index ad692e872ed..28c9664cc90 100644 --- a/Lib/test/test_pathlib/test_pathlib_abc.py +++ b/Lib/test/test_pathlib/test_pathlib_abc.py @@ -5,7 +5,8 @@ import errno import stat import unittest -from pathlib._abc import UnsupportedOperation, ParserBase, PurePathBase, PathBase +from pathlib._os import UnsupportedOperation +from pathlib._abc import ParserBase, PurePathBase, PathBase import posixpath from test.support import is_wasi