diff --git a/Doc/library/pathlib.rst b/Doc/library/pathlib.rst index f66d36a32cbd04..6df481e3005db9 100644 --- a/Doc/library/pathlib.rst +++ b/Doc/library/pathlib.rst @@ -1637,20 +1637,11 @@ Copying, renaming and deleting :meth:`Path.delete` to remove a non-empty directory. -.. method:: Path.delete(ignore_errors=False, on_error=None) +.. method:: Path.delete() Delete this file or directory. If this path refers to a non-empty directory, its files and sub-directories are deleted recursively. - If *ignore_errors* is true, errors resulting from failed deletions will be - ignored. If *ignore_errors* is false or omitted, and a callable is given as - the optional *on_error* argument, it will be called with one argument of - type :exc:`OSError` each time an exception is raised. The callable can - handle the error to continue the deletion process or re-raise it to stop. - Note that the filename is available as the :attr:`~OSError.filename` - attribute of the exception object. If neither *ignore_errors* nor - *on_error* are supplied, exceptions are propagated to the caller. - .. note:: When deleting non-empty directories on platforms that lack the necessary diff --git a/Lib/pathlib/_abc.py b/Lib/pathlib/_abc.py index 720756cac66f68..c7d2b27d01adfc 100644 --- a/Lib/pathlib/_abc.py +++ b/Lib/pathlib/_abc.py @@ -923,23 +923,13 @@ def rmdir(self): """ raise UnsupportedOperation(self._unsupported_msg('rmdir()')) - def delete(self, ignore_errors=False, on_error=None): + def delete(self): """ Delete this file or directory (including all sub-directories). - - If *ignore_errors* is true, exceptions raised from scanning the - filesystem and removing files and directories are ignored. Otherwise, - if *on_error* is set, it will be called to handle the error. If - neither *ignore_errors* nor *on_error* are set, exceptions are - propagated to the caller. """ - if ignore_errors: - def on_error(err): - pass - elif on_error is None: + if self.is_dir(follow_symlinks=False): def on_error(err): raise err - if self.is_dir(follow_symlinks=False): results = self.walk( on_error=on_error, top_down=False, # So we rmdir() empty directories. @@ -955,14 +945,9 @@ def on_error(err): dirpath.joinpath(name).rmdir() except OSError as err: on_error(err) - delete_self = self.rmdir + self.rmdir() else: - delete_self = self.unlink - try: - delete_self() - except OSError as err: - err.filename = str(self) - on_error(err) + self.unlink() delete.avoids_symlink_attacks = False def owner(self, *, follow_symlinks=True): diff --git a/Lib/pathlib/_local.py b/Lib/pathlib/_local.py index 8f5c58c16ef0d0..f7fe25685fd89d 100644 --- a/Lib/pathlib/_local.py +++ b/Lib/pathlib/_local.py @@ -824,32 +824,16 @@ def rmdir(self): """ os.rmdir(self) - def delete(self, ignore_errors=False, on_error=None): + def delete(self): """ Delete this file or directory (including all sub-directories). - - If *ignore_errors* is true, exceptions raised from scanning the - filesystem and removing files and directories are ignored. Otherwise, - if *on_error* is set, it will be called to handle the error. If - neither *ignore_errors* nor *on_error* are set, exceptions are - propagated to the caller. - """ - if self.is_dir(follow_symlinks=False): - onexc = None - if on_error: - def onexc(func, filename, err): - err.filename = filename - on_error(err) - shutil.rmtree(str(self), ignore_errors, onexc=onexc) + """ + if self.is_symlink() or self.is_junction(): + self.unlink() + elif self.is_dir(): + shutil.rmtree(str(self)) else: - try: - self.unlink() - except OSError as err: - if not ignore_errors: - if on_error: - on_error(err) - else: - raise + self.unlink() delete.avoids_symlink_attacks = shutil.rmtree.avoids_symlink_attacks diff --git a/Lib/test/test_pathlib/test_pathlib.py b/Lib/test/test_pathlib/test_pathlib.py index 4c60f278834c9c..67b0cd125c4d4b 100644 --- a/Lib/test/test_pathlib/test_pathlib.py +++ b/Lib/test/test_pathlib/test_pathlib.py @@ -715,11 +715,6 @@ def test_copy_dir_no_read_permission(self): target = base / 'copyE' self.assertRaises(PermissionError, source.copy, target) self.assertFalse(target.exists()) - errors = [] - source.copy(target, on_error=errors.append) - self.assertEqual(len(errors), 1) - self.assertIsInstance(errors[0], PermissionError) - self.assertFalse(target.exists()) def test_copy_dir_preserve_metadata(self): base = self.cls(self.base) @@ -904,10 +899,7 @@ def test_delete_unwritable(self): child_dir_path.chmod(new_mode) tmp.chmod(new_mode) - errors = [] - tmp.delete(on_error=errors.append) - # Test whether onerror has actually been called. - self.assertEqual(len(errors), 3) + self.assertRaises(PermissionError, tmp.delete) finally: tmp.chmod(old_dir_mode) child_file_path.chmod(old_child_file_mode) @@ -942,42 +934,16 @@ def test_delete_outer_junction(self): import _winapi tmp = self.cls(self.base, 'delete') tmp.mkdir() - try: - src = tmp / 'cheese' - dst = tmp / 'shop' - src.mkdir() - spam = src / 'spam' - spam.write_text('') - _winapi.CreateJunction(str(src), str(dst)) - self.assertRaises(OSError, dst.delete) - dst.delete(ignore_errors=True) - finally: - tmp.delete(ignore_errors=True) - - @needs_windows - def test_delete_outer_junction_on_error(self): - import _winapi - tmp = self.cls(self.base, 'delete') - tmp.mkdir() - dir_ = tmp / 'dir' - dir_.mkdir() - link = tmp / 'link' - _winapi.CreateJunction(str(dir_), str(link)) - try: - self.assertRaises(OSError, link.delete) - self.assertTrue(dir_.exists()) - self.assertTrue(link.exists(follow_symlinks=False)) - errors = [] - - def on_error(error): - errors.append(error) - - link.delete(on_error=on_error) - self.assertEqual(len(errors), 1) - self.assertIsInstance(errors[0], OSError) - self.assertEqual(errors[0].filename, str(link)) - finally: - os.unlink(str(link)) + src = tmp / 'cheese' + dst = tmp / 'shop' + src.mkdir() + spam = src / 'spam' + spam.write_text('') + _winapi.CreateJunction(str(src), str(dst)) + dst.delete() + self.assertFalse(dst.exists()) + self.assertTrue(spam.exists()) + self.assertTrue(src.exists()) @unittest.skipUnless(delete_use_fd_functions, "requires safe delete") def test_delete_fails_on_close(self): @@ -1003,16 +969,6 @@ def close(fd): self.assertTrue(dir2.is_dir()) self.assertEqual(close_count, 2) - close_count = 0 - errors = [] - - with swap_attr(os, 'close', close) as orig_close: - dir1.delete(on_error=errors.append) - self.assertEqual(len(errors), 2) - self.assertEqual(errors[0].filename, str(dir2)) - self.assertEqual(errors[1].filename, str(dir1)) - self.assertEqual(close_count, 2) - @unittest.skipUnless(hasattr(os, "mkfifo"), 'requires os.mkfifo()') @unittest.skipIf(sys.platform == "vxworks", "fifo requires special path on VxWorks") @@ -1028,63 +984,6 @@ def test_delete_on_named_pipe(self): p.delete() self.assertFalse(p.exists()) - @unittest.skipIf(sys.platform[:6] == 'cygwin', - "This test can't be run on Cygwin (issue #1071513).") - @os_helper.skip_if_dac_override - @os_helper.skip_unless_working_chmod - def test_delete_deleted_race_condition(self): - # bpo-37260 - # - # Test that a file or a directory deleted after it is enumerated - # by scandir() but before unlink() or rmdr() is called doesn't - # generate any errors. - def on_error(exc): - assert exc.filename - if not isinstance(exc, PermissionError): - raise - # Make the parent and the children writeable. - for p, mode in zip(paths, old_modes): - p.chmod(mode) - # Remove other dirs except one. - keep = next(p for p in dirs if str(p) != exc.filename) - for p in dirs: - if p != keep: - p.rmdir() - # Remove other files except one. - keep = next(p for p in files if str(p) != exc.filename) - for p in files: - if p != keep: - p.unlink() - - tmp = self.cls(self.base, 'delete') - tmp.mkdir() - paths = [tmp] + [tmp / f'child{i}' for i in range(6)] - dirs = paths[1::2] - files = paths[2::2] - for path in dirs: - path.mkdir() - for path in files: - path.write_text('') - - old_modes = [path.stat().st_mode for path in paths] - - # Make the parent and the children non-writeable. - new_mode = stat.S_IREAD | stat.S_IEXEC - for path in reversed(paths): - path.chmod(new_mode) - - try: - tmp.delete(on_error=on_error) - except: - # Test failed, so cleanup artifacts. - for path, mode in zip(paths, old_modes): - try: - path.chmod(mode) - except OSError: - pass - tmp.delete() - raise - def test_delete_does_not_choke_on_failing_lstat(self): try: orig_lstat = os.lstat diff --git a/Lib/test/test_pathlib/test_pathlib_abc.py b/Lib/test/test_pathlib/test_pathlib_abc.py index f222fd5b1ec082..e45fbe330d9da2 100644 --- a/Lib/test/test_pathlib/test_pathlib_abc.py +++ b/Lib/test/test_pathlib/test_pathlib_abc.py @@ -2705,14 +2705,6 @@ def test_delete_missing(self): # filename is guaranteed not to exist filename = tmp / 'foo' self.assertRaises(FileNotFoundError, filename.delete) - # test that ignore_errors option is honored - filename.delete(ignore_errors=True) - # test on_error - errors = [] - filename.delete(on_error=errors.append) - self.assertEqual(len(errors), 1) - self.assertIsInstance(errors[0], FileNotFoundError) - self.assertEqual(errors[0].filename, str(filename)) def setUpWalk(self): # Build: