https://github.com/python/cpython/commit/c4ee4e756a311f03633723445299bde90eb7b79c
commit: c4ee4e756a311f03633723445299bde90eb7b79c
branch: main
author: Barney Gale <[email protected]>
committer: barneygale <[email protected]>
date: 2024-08-24T15:11:39+01:00
summary:
GH-122890: Fix low-level error handling in `pathlib.Path.copy()` (#122897)
Give unique names to our low-level FD copying functions, and try each one
in turn. Handle errors appropriately for each implementation:
- `fcntl.FICLONE`: suppress `EBADF`, `EOPNOTSUPP`, `ETXTBSY`, `EXDEV`
- `posix._fcopyfile`: suppress `EBADF`, `ENOTSUP`
- `os.copy_file_range`: suppress `ETXTBSY`, `EXDEV`
- `os.sendfile`: suppress `ENOTSOCK`
files:
M Lib/pathlib/_os.py
M Lib/test/test_pathlib/test_pathlib.py
diff --git a/Lib/pathlib/_os.py b/Lib/pathlib/_os.py
index 63dbe131baea51..642b3a57c59a1d 100644
--- a/Lib/pathlib/_os.py
+++ b/Lib/pathlib/_os.py
@@ -20,7 +20,7 @@
_winapi = None
-def get_copy_blocksize(infd):
+def _get_copy_blocksize(infd):
"""Determine blocksize for fastcopying on Linux.
Hopefully the whole file will be copied in a single call.
The copying itself should be performed in a loop 'till EOF is
@@ -40,7 +40,7 @@ def get_copy_blocksize(infd):
if fcntl and hasattr(fcntl, 'FICLONE'):
- def clonefd(source_fd, target_fd):
+ def _ficlone(source_fd, target_fd):
"""
Perform a lightweight copy of two files, where the data blocks are
copied only when modified. This is known as Copy on Write (CoW),
@@ -48,18 +48,22 @@ def clonefd(source_fd, target_fd):
"""
fcntl.ioctl(target_fd, fcntl.FICLONE, source_fd)
else:
- clonefd = None
+ _ficlone = None
if posix and hasattr(posix, '_fcopyfile'):
- def copyfd(source_fd, target_fd):
+ def _fcopyfile(source_fd, target_fd):
"""
Copy a regular file content using high-performance fcopyfile(3)
syscall (macOS).
"""
posix._fcopyfile(source_fd, target_fd, posix._COPYFILE_DATA)
-elif hasattr(os, 'copy_file_range'):
- def copyfd(source_fd, target_fd):
+else:
+ _fcopyfile = None
+
+
+if hasattr(os, 'copy_file_range'):
+ def _copy_file_range(source_fd, target_fd):
"""
Copy data from one regular mmap-like fd to another by using a
high-performance copy_file_range(2) syscall that gives filesystems
@@ -67,7 +71,7 @@ def copyfd(source_fd, target_fd):
copy.
This should work on Linux >= 4.5 only.
"""
- blocksize = get_copy_blocksize(source_fd)
+ blocksize = _get_copy_blocksize(source_fd)
offset = 0
while True:
sent = os.copy_file_range(source_fd, target_fd, blocksize,
@@ -75,13 +79,17 @@ def copyfd(source_fd, target_fd):
if sent == 0:
break # EOF
offset += sent
-elif hasattr(os, 'sendfile'):
- def copyfd(source_fd, target_fd):
+else:
+ _copy_file_range = None
+
+
+if hasattr(os, 'sendfile'):
+ def _sendfile(source_fd, target_fd):
"""Copy data from one regular mmap-like fd to another by using
high-performance sendfile(2) syscall.
This should work on Linux >= 2.6.33 only.
"""
- blocksize = get_copy_blocksize(source_fd)
+ blocksize = _get_copy_blocksize(source_fd)
offset = 0
while True:
sent = os.sendfile(target_fd, source_fd, offset, blocksize)
@@ -89,7 +97,7 @@ def copyfd(source_fd, target_fd):
break # EOF
offset += sent
else:
- copyfd = None
+ _sendfile = None
if _winapi and hasattr(_winapi, 'CopyFile2'):
@@ -114,18 +122,36 @@ def copyfileobj(source_f, target_f):
else:
try:
# Use OS copy-on-write where available.
- if clonefd:
+ if _ficlone:
try:
- clonefd(source_fd, target_fd)
+ _ficlone(source_fd, target_fd)
return
except OSError as err:
if err.errno not in (EBADF, EOPNOTSUPP, ETXTBSY, EXDEV):
raise err
# Use OS copy where available.
- if copyfd:
- copyfd(source_fd, target_fd)
- return
+ if _fcopyfile:
+ try:
+ _fcopyfile(source_fd, target_fd)
+ return
+ except OSError as err:
+ if err.errno not in (EINVAL, ENOTSUP):
+ raise err
+ if _copy_file_range:
+ try:
+ _copy_file_range(source_fd, target_fd)
+ return
+ except OSError as err:
+ if err.errno not in (ETXTBSY, EXDEV):
+ raise err
+ if _sendfile:
+ try:
+ _sendfile(source_fd, target_fd)
+ return
+ except OSError as err:
+ if err.errno != ENOTSOCK:
+ raise err
except OSError as err:
# Produce more useful error messages.
err.filename = source_f.name
diff --git a/Lib/test/test_pathlib/test_pathlib.py
b/Lib/test/test_pathlib/test_pathlib.py
index 4c60f278834c9c..ad1720cdb24f0b 100644
--- a/Lib/test/test_pathlib/test_pathlib.py
+++ b/Lib/test/test_pathlib/test_pathlib.py
@@ -1,3 +1,4 @@
+import contextlib
import io
import os
import sys
@@ -22,10 +23,18 @@
from test.test_pathlib import test_pathlib_abc
from test.test_pathlib.test_pathlib_abc import needs_posix, needs_windows,
needs_symlinks
+try:
+ import fcntl
+except ImportError:
+ fcntl = None
try:
import grp, pwd
except ImportError:
grp = pwd = None
+try:
+ import posix
+except ImportError:
+ posix = None
root_in_posix = False
@@ -707,6 +716,45 @@ def test_copy_link_preserve_metadata(self):
if hasattr(source_st, 'st_flags'):
self.assertEqual(source_st.st_flags, target_st.st_flags)
+ def test_copy_error_handling(self):
+ def make_raiser(err):
+ def raiser(*args, **kwargs):
+ raise OSError(err, os.strerror(err))
+ return raiser
+
+ base = self.cls(self.base)
+ source = base / 'fileA'
+ target = base / 'copyA'
+
+ # Raise non-fatal OSError from all available fast copy functions.
+ with contextlib.ExitStack() as ctx:
+ if fcntl and hasattr(fcntl, 'FICLONE'):
+ ctx.enter_context(mock.patch('fcntl.ioctl',
make_raiser(errno.EXDEV)))
+ if posix and hasattr(posix, '_fcopyfile'):
+ ctx.enter_context(mock.patch('posix._fcopyfile',
make_raiser(errno.ENOTSUP)))
+ if hasattr(os, 'copy_file_range'):
+ ctx.enter_context(mock.patch('os.copy_file_range',
make_raiser(errno.EXDEV)))
+ if hasattr(os, 'sendfile'):
+ ctx.enter_context(mock.patch('os.sendfile',
make_raiser(errno.ENOTSOCK)))
+
+ source.copy(target)
+ self.assertTrue(target.exists())
+ self.assertEqual(source.read_text(), target.read_text())
+
+ # Raise fatal OSError from first available fast copy function.
+ if fcntl and hasattr(fcntl, 'FICLONE'):
+ patchpoint = 'fcntl.ioctl'
+ elif posix and hasattr(posix, '_fcopyfile'):
+ patchpoint = 'posix._fcopyfile'
+ elif hasattr(os, 'copy_file_range'):
+ patchpoint = 'os.copy_file_range'
+ elif hasattr(os, 'sendfile'):
+ patchpoint = 'os.sendfile'
+ else:
+ return
+ with mock.patch(patchpoint, make_raiser(errno.ENOENT)):
+ self.assertRaises(FileNotFoundError, source.copy, target)
+
@unittest.skipIf(sys.platform == "win32" or sys.platform == "wasi",
"directories are always readable on Windows and WASI")
@unittest.skipIf(root_in_posix, "test fails with root privilege")
def test_copy_dir_no_read_permission(self):
_______________________________________________
Python-checkins mailing list -- [email protected]
To unsubscribe send an email to [email protected]
https://mail.python.org/mailman3/lists/python-checkins.python.org/
Member address: [email protected]