Skip to content

GH-73991: Add follow_symlinks argument to pathlib.Path.copy() #120519

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion Doc/library/pathlib.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1432,17 +1432,26 @@ Creating files and directories
Copying, renaming and deleting
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

.. method:: Path.copy(target)
.. method:: Path.copy(target, *, follow_symlinks=True)

Copy the contents of this file to the *target* file. If *target* specifies
a file that already exists, it will be replaced.

If *follow_symlinks* is false, and this file is a symbolic link, *target*
will be created as a symbolic link. If *follow_symlinks* is true and this
file is a symbolic link, *target* will be a copy of the symlink target.

.. note::
This method uses operating system functionality to copy file content
efficiently. The OS might also copy some metadata, such as file
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.
Comment on lines +1450 to +1453
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Failing to copy a symlink that targets a directory is disappointing. A symlink is never reported as a directory in Python, so code that copies a tree that contains directory symlinks is going to fail on Windows Server 2016 and Windows Server 2019 systems, with no obvious reason that a Python programmer would reasonably expect. A fallback path is needed to copy the link via os.symlink().

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should have waited for your feedback Eryk, my bad. I'll fix this in a follow-up PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As always, I defer to what you and Steve think is best.

I'm of a mixed mind regarding Python's PEP 11 support policy that requires new versions of Python to support Server releases that still have extended support from Microsoft, which sets the bar for all versions of Windows. The Server editions are released in the LTSC channel and get 10 years of support. Server 2016 (build 14393) is supported until January 2027, and Server 2019 (build 17763) is supported until January 2029. Part of me wishes we just grouped the Server releases with corresponding releases in the general availability channel (e.g. Windows 10 1607 and Windows 10 1809) because it's more work to provide and maintain fallback paths for older versions of Windows. However, it's also great for users that they can use the latest version of Python on older systems.


.. versionadded:: 3.14


Expand Down
9 changes: 7 additions & 2 deletions Lib/pathlib/_abc.py
Original file line number Diff line number Diff line change
Expand Up @@ -790,14 +790,19 @@ def mkdir(self, mode=0o777, parents=False, exist_ok=False):
"""
raise UnsupportedOperation(self._unsupported_msg('mkdir()'))

def copy(self, target):
def copy(self, target, follow_symlinks=True):
"""
Copy the contents of this file to the given target.
Copy the contents of this file to the given target. If this file is a
symlink and follow_symlinks is false, a symlink will be created at the
target.
"""
if not isinstance(target, PathBase):
target = self.with_segments(target)
if self._samefile_safe(target):
raise OSError(f"{self!r} and {target!r} are the same file")
if not follow_symlinks and self.is_symlink():
target.symlink_to(self.readlink())
return
with self.open('rb') as source_f:
try:
with target.open('wb') as target_f:
Expand Down
10 changes: 6 additions & 4 deletions Lib/pathlib/_local.py
Original file line number Diff line number Diff line change
Expand Up @@ -782,19 +782,21 @@ def mkdir(self, mode=0o777, parents=False, exist_ok=False):
raise

if copyfile:
def copy(self, target):
def copy(self, target, follow_symlinks=True):
"""
Copy the contents of this file to the given target.
Copy the contents of this file to the given target. If this file is a
symlink and follow_symlinks is false, a symlink will be created at the
target.
"""
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)
return PathBase.copy(self, target, follow_symlinks=follow_symlinks)
raise
copyfile(os.fspath(self), target)
copyfile(os.fspath(self), target, follow_symlinks)

def chmod(self, mode, *, follow_symlinks=True):
"""
Expand Down
27 changes: 24 additions & 3 deletions Lib/pathlib/_os.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

from errno import EBADF, EOPNOTSUPP, ETXTBSY, EXDEV
import os
import stat
import sys
try:
import fcntl
Expand Down Expand Up @@ -91,12 +92,32 @@ def copyfd(source_fd, target_fd):
copyfd = None


if _winapi and hasattr(_winapi, 'CopyFile2'):
def copyfile(source, target):
if _winapi and hasattr(_winapi, 'CopyFile2') and hasattr(os.stat_result, 'st_file_attributes'):
def _is_dirlink(path):
try:
st = os.lstat(path)
except (OSError, ValueError):
return False
return (st.st_file_attributes & stat.FILE_ATTRIBUTE_DIRECTORY and
st.st_reparse_tag == stat.IO_REPARSE_TAG_SYMLINK)

def copyfile(source, target, follow_symlinks):
"""
Copy from one file to another using CopyFile2 (Windows only).
"""
_winapi.CopyFile2(source, target, 0)
if follow_symlinks:
flags = 0
else:
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
flags |= _winapi.COPY_FILE_DIRECTORY
_winapi.CopyFile2(source, target, flags)
else:
copyfile = None

Expand Down
35 changes: 34 additions & 1 deletion Lib/test/test_pathlib/test_pathlib_abc.py
Original file line number Diff line number Diff line change
Expand Up @@ -1712,7 +1712,7 @@ def test_copy_directory(self):
source.copy(target)

@needs_symlinks
def test_copy_symlink(self):
def test_copy_symlink_follow_symlinks_true(self):
base = self.cls(self.base)
source = base / 'linkA'
target = base / 'copyA'
Expand All @@ -1721,6 +1721,26 @@ def test_copy_symlink(self):
self.assertFalse(target.is_symlink())
self.assertEqual(source.read_text(), target.read_text())

@needs_symlinks
def test_copy_symlink_follow_symlinks_false(self):
base = self.cls(self.base)
source = base / 'linkA'
target = base / 'copyA'
source.copy(target, follow_symlinks=False)
self.assertTrue(target.exists())
self.assertTrue(target.is_symlink())
self.assertEqual(source.readlink(), target.readlink())

@needs_symlinks
def test_copy_directory_symlink_follow_symlinks_false(self):
base = self.cls(self.base)
source = base / 'linkB'
target = base / 'copyA'
source.copy(target, follow_symlinks=False)
self.assertTrue(target.exists())
self.assertTrue(target.is_symlink())
self.assertEqual(source.readlink(), target.readlink())

def test_copy_to_existing_file(self):
base = self.cls(self.base)
source = base / 'fileA'
Expand Down Expand Up @@ -1749,6 +1769,19 @@ def test_copy_to_existing_symlink(self):
self.assertFalse(real_target.is_symlink())
self.assertEqual(source.read_text(), real_target.read_text())

@needs_symlinks
def test_copy_to_existing_symlink_follow_symlinks_false(self):
base = self.cls(self.base)
source = base / 'dirB' / 'fileB'
target = base / 'linkA'
real_target = base / 'fileA'
source.copy(target, follow_symlinks=False)
self.assertTrue(target.exists())
self.assertTrue(target.is_symlink())
self.assertTrue(real_target.exists())
self.assertFalse(real_target.is_symlink())
self.assertEqual(source.read_text(), real_target.read_text())

def test_copy_empty(self):
base = self.cls(self.base)
source = base / 'empty'
Expand Down
5 changes: 5 additions & 0 deletions Modules/_winapi.c
Original file line number Diff line number Diff line change
Expand Up @@ -3166,6 +3166,11 @@ static int winapi_exec(PyObject *m)
#define COPY_FILE_REQUEST_COMPRESSED_TRAFFIC 0x10000000
#endif
WINAPI_CONSTANT(F_DWORD, COPY_FILE_REQUEST_COMPRESSED_TRAFFIC);
#ifndef COPY_FILE_DIRECTORY
// Only defined in newer WinSDKs
#define COPY_FILE_DIRECTORY 0x00000080
#endif
WINAPI_CONSTANT(F_DWORD, COPY_FILE_DIRECTORY);

WINAPI_CONSTANT(F_DWORD, COPYFILE2_CALLBACK_CHUNK_STARTED);
WINAPI_CONSTANT(F_DWORD, COPYFILE2_CALLBACK_CHUNK_FINISHED);
Expand Down
Loading