From 9ebc00169108d0794da761ca7990e16fdb67b2d1 Mon Sep 17 00:00:00 2001 From: barneygale Date: Thu, 20 Jun 2024 19:51:27 +0100 Subject: [PATCH 1/2] GH-73991: Support copying directory symlinks on older Windows 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 | 19 +++++++++++++++++-- Lib/test/test_pathlib/test_pathlib_abc.py | 3 ++- 6 files changed, 33 insertions(+), 28 deletions(-) diff --git a/Doc/library/pathlib.rst b/Doc/library/pathlib.rst index 5bfcad0dadff6a..0a06d7447901c8 100644 --- a/Doc/library/pathlib.rst +++ b/Doc/library/pathlib.rst @@ -1447,11 +1447,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 4b3edf535a61aa..2298a249529460 100644 --- a/Lib/pathlib/__init__.py +++ b/Lib/pathlib/__init__.py @@ -5,8 +5,8 @@ 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 f1f350a196091a..6112379335150c 100644 --- a/Lib/pathlib/_abc.py +++ b/Lib/pathlib/_abc.py @@ -16,10 +16,7 @@ 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 0105ea3042422e..acb57214b81865 100644 --- a/Lib/pathlib/_local.py +++ b/Lib/pathlib/_local.py @@ -17,8 +17,8 @@ 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 @@ def copy(self, target, follow_symlinks=True): 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 bbb019b6534503..abd46bbc5845a8 100644 --- a/Lib/pathlib/_os.py +++ b/Lib/pathlib/_os.py @@ -20,6 +20,15 @@ _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. @@ -115,9 +124,15 @@ def copyfile(source, target, follow_symlinks): except OSError as err: # Check for ERROR_ACCESS_DENIED if err.winerror != 5 or not _is_dirlink(source): - raise + raise # Some other error. 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 # Some other error. + raise UnsupportedOperation(err) else: copyfile = None diff --git a/Lib/test/test_pathlib/test_pathlib_abc.py b/Lib/test/test_pathlib/test_pathlib_abc.py index cd629c01871165..ff2e921a8d2cf8 100644 --- a/Lib/test/test_pathlib/test_pathlib_abc.py +++ b/Lib/test/test_pathlib/test_pathlib_abc.py @@ -5,7 +5,8 @@ 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 From 09e360131f7faa2ae090b982f7771a26cca0e27f Mon Sep 17 00:00:00 2001 From: barneygale Date: Fri, 21 Jun 2024 19:26:59 +0100 Subject: [PATCH 2/2] Address review comments --- Lib/pathlib/_os.py | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/Lib/pathlib/_os.py b/Lib/pathlib/_os.py index abd46bbc5845a8..61923b5e410b5c 100644 --- a/Lib/pathlib/_os.py +++ b/Lib/pathlib/_os.py @@ -115,24 +115,30 @@ def copyfile(source, target, follow_symlinks): 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): - raise # Some other error. + 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 - try: - _winapi.CopyFile2(source, target, flags) - except OSError as err: - # Check for ERROR_INVALID_PARAMETER - if err.winerror != 87: - raise # Some other error. - raise UnsupportedOperation(err) + 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