diff options
author | andrei kulakov <andrei.avk@gmail.com> | 2021-09-21 17:53:07 -0400 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-09-21 23:53:07 +0200 |
commit | b7eac52b466f697d3e89f47508e0df0196a98970 (patch) | |
tree | f399750eb74741f8bf7d3e1035e000e83728dbe0 | |
parent | bpo-45061: Detect refcount bug on empty string singleton (GH-28504) (diff) | |
download | cpython-b7eac52b466f697d3e89f47508e0df0196a98970.tar.gz cpython-b7eac52b466f697d3e89f47508e0df0196a98970.tar.bz2 cpython-b7eac52b466f697d3e89f47508e0df0196a98970.zip |
bpo-45234: Fix FileNotFound exception raised instead of IsADirectoryError in shutil.copyfile() (GH-28421)
This was a regression from fixing BPO-43219.
-rw-r--r-- | Lib/shutil.py | 59 | ||||
-rw-r--r-- | Lib/test/test_shutil.py | 40 | ||||
-rw-r--r-- | Misc/NEWS.d/next/Library/2021-09-17-11-20-55.bpo-45234.qUcTVt.rst | 3 |
3 files changed, 73 insertions, 29 deletions
diff --git a/Lib/shutil.py b/Lib/shutil.py index 273a7d2f432..e544498fca7 100644 --- a/Lib/shutil.py +++ b/Lib/shutil.py @@ -253,36 +253,37 @@ def copyfile(src, dst, *, follow_symlinks=True): if not follow_symlinks and _islink(src): os.symlink(os.readlink(src), dst) else: - try: - with open(src, 'rb') as fsrc, open(dst, 'wb') as fdst: - # macOS - if _HAS_FCOPYFILE: - try: - _fastcopy_fcopyfile(fsrc, fdst, posix._COPYFILE_DATA) - return dst - except _GiveupOnFastCopy: - pass - # Linux - elif _USE_CP_SENDFILE: - try: - _fastcopy_sendfile(fsrc, fdst) + with open(src, 'rb') as fsrc: + try: + with open(dst, 'wb') as fdst: + # macOS + if _HAS_FCOPYFILE: + try: + _fastcopy_fcopyfile(fsrc, fdst, posix._COPYFILE_DATA) + return dst + except _GiveupOnFastCopy: + pass + # Linux + elif _USE_CP_SENDFILE: + try: + _fastcopy_sendfile(fsrc, fdst) + return dst + except _GiveupOnFastCopy: + pass + # Windows, see: + # https://github.com/python/cpython/pull/7160#discussion_r195405230 + elif _WINDOWS and file_size > 0: + _copyfileobj_readinto(fsrc, fdst, min(file_size, COPY_BUFSIZE)) return dst - except _GiveupOnFastCopy: - pass - # Windows, see: - # https://github.com/python/cpython/pull/7160#discussion_r195405230 - elif _WINDOWS and file_size > 0: - _copyfileobj_readinto(fsrc, fdst, min(file_size, COPY_BUFSIZE)) - return dst - - copyfileobj(fsrc, fdst) - - # Issue 43219, raise a less confusing exception - except IsADirectoryError as e: - if os.path.exists(dst): - raise - else: - raise FileNotFoundError(f'Directory does not exist: {dst}') from e + + copyfileobj(fsrc, fdst) + + # Issue 43219, raise a less confusing exception + except IsADirectoryError as e: + if not os.path.exists(dst): + raise FileNotFoundError(f'Directory does not exist: {dst}') from e + else: + raise return dst diff --git a/Lib/test/test_shutil.py b/Lib/test/test_shutil.py index 7bf60fd566e..7669b94ac35 100644 --- a/Lib/test/test_shutil.py +++ b/Lib/test/test_shutil.py @@ -1151,6 +1151,28 @@ class TestCopy(BaseTest, unittest.TestCase): rv = fn(src, os.path.join(dst_dir, 'bar')) self.assertEqual(rv, os.path.join(dst_dir, 'bar')) + def test_copy_dir(self): + self._test_copy_dir(shutil.copy) + + def test_copy2_dir(self): + self._test_copy_dir(shutil.copy2) + + def _test_copy_dir(self, copy_func): + src_dir = self.mkdtemp() + src_file = os.path.join(src_dir, 'foo') + dir2 = self.mkdtemp() + dst = os.path.join(src_dir, 'does_not_exist/') + write_file(src_file, 'foo') + if sys.platform == "win32": + err = PermissionError + else: + err = IsADirectoryError + self.assertRaises(err, copy_func, dir2, src_dir) + + # raise *err* because of src rather than FileNotFoundError because of dst + self.assertRaises(err, copy_func, dir2, dst) + copy_func(src_file, dir2) # should not raise exceptions + ### shutil.copyfile @os_helper.skip_unless_symlink @@ -1259,6 +1281,24 @@ class TestCopy(BaseTest, unittest.TestCase): write_file(src_file, 'foo') self.assertRaises(FileNotFoundError, shutil.copyfile, src_file, dst) + def test_copyfile_copy_dir(self): + # Issue 45234 + # test copy() and copyfile() raising proper exceptions when src and/or + # dst are directories + src_dir = self.mkdtemp() + src_file = os.path.join(src_dir, 'foo') + dir2 = self.mkdtemp() + dst = os.path.join(src_dir, 'does_not_exist/') + write_file(src_file, 'foo') + if sys.platform == "win32": + err = PermissionError + else: + err = IsADirectoryError + + self.assertRaises(err, shutil.copyfile, src_dir, dst) + self.assertRaises(err, shutil.copyfile, src_file, src_dir) + self.assertRaises(err, shutil.copyfile, dir2, src_dir) + class TestArchives(BaseTest, unittest.TestCase): diff --git a/Misc/NEWS.d/next/Library/2021-09-17-11-20-55.bpo-45234.qUcTVt.rst b/Misc/NEWS.d/next/Library/2021-09-17-11-20-55.bpo-45234.qUcTVt.rst new file mode 100644 index 00000000000..3817b5de644 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2021-09-17-11-20-55.bpo-45234.qUcTVt.rst @@ -0,0 +1,3 @@ +Fixed a regression in :func:`~shutil.copyfile`, :func:`~shutil.copy`, +:func:`~shutil.copy2` raising :exc:`FileNotFoundError` when source is a +directory, which should raise :exc:`IsADirectoryError` |