https://github.com/python/cpython/commit/7f8ec523021427a5c1ab3ce0cdd6e4bb909f1dc5
commit: 7f8ec523021427a5c1ab3ce0cdd6e4bb909f1dc5
branch: main
author: Barney Gale <[email protected]>
committer: barneygale <[email protected]>
date: 2024-12-08T18:45:09Z
summary:
GH-127381: pathlib ABCs: remove `PathBase.unlink()` and `rmdir()` (#127736)
Virtual filesystems don't always make a distinction between deleting files
and empty directories, and sometimes support deleting non-empty directories
in a single operation. Here we remove `PathBase.unlink()` and `rmdir()`,
leaving `_delete()` as the sole deletion method, now made abstract. I hope
to drop the underscore prefix later on.
files:
M Lib/pathlib/_abc.py
M Lib/pathlib/_local.py
M Lib/test/test_pathlib/test_pathlib.py
M Lib/test/test_pathlib/test_pathlib_abc.py
diff --git a/Lib/pathlib/_abc.py b/Lib/pathlib/_abc.py
index 820970fcd5889b..309eab2ff855c3 100644
--- a/Lib/pathlib/_abc.py
+++ b/Lib/pathlib/_abc.py
@@ -840,6 +840,12 @@ def copy_into(self, target_dir, *, follow_symlinks=True,
dirs_exist_ok=dirs_exist_ok,
preserve_metadata=preserve_metadata)
+ def _delete(self):
+ """
+ Delete this file or directory (including all sub-directories).
+ """
+ raise UnsupportedOperation(self._unsupported_msg('_delete()'))
+
def move(self, target):
"""
Recursively move this file or directory tree to the given destination.
@@ -874,43 +880,6 @@ def lchmod(self, mode):
"""
self.chmod(mode, follow_symlinks=False)
- def unlink(self, missing_ok=False):
- """
- Remove this file or link.
- If the path is a directory, use rmdir() instead.
- """
- raise UnsupportedOperation(self._unsupported_msg('unlink()'))
-
- def rmdir(self):
- """
- Remove this directory. The directory must be empty.
- """
- raise UnsupportedOperation(self._unsupported_msg('rmdir()'))
-
- def _delete(self):
- """
- Delete this file or directory (including all sub-directories).
- """
- if self.is_symlink() or self.is_junction():
- self.unlink()
- elif self.is_dir():
- self._rmtree()
- else:
- self.unlink()
-
- def _rmtree(self):
- def on_error(err):
- raise err
- results = self.walk(
- on_error=on_error,
- top_down=False, # So we rmdir() empty directories.
- follow_symlinks=False)
- for dirpath, _, filenames in results:
- for filename in filenames:
- filepath = dirpath / filename
- filepath.unlink()
- dirpath.rmdir()
-
def owner(self, *, follow_symlinks=True):
"""
Return the login name of the file owner.
diff --git a/Lib/pathlib/_local.py b/Lib/pathlib/_local.py
index 250bc12956f5bc..f87069ce70a2de 100644
--- a/Lib/pathlib/_local.py
+++ b/Lib/pathlib/_local.py
@@ -846,10 +846,18 @@ def rmdir(self):
"""
os.rmdir(self)
- def _rmtree(self):
- # Lazy import to improve module import time
- import shutil
- shutil.rmtree(self)
+ def _delete(self):
+ """
+ Delete this file or directory (including all sub-directories).
+ """
+ if self.is_symlink() or self.is_junction():
+ self.unlink()
+ elif self.is_dir():
+ # Lazy import to improve module import time
+ import shutil
+ shutil.rmtree(self)
+ else:
+ self.unlink()
def rename(self, target):
"""
diff --git a/Lib/test/test_pathlib/test_pathlib.py
b/Lib/test/test_pathlib/test_pathlib.py
index 8c9049f15d5bf9..ce0f4748c860b1 100644
--- a/Lib/test/test_pathlib/test_pathlib.py
+++ b/Lib/test/test_pathlib/test_pathlib.py
@@ -1352,6 +1352,25 @@ def test_group_no_follow_symlinks(self):
self.assertEqual(expected_gid, gid_2)
self.assertEqual(expected_name, link.group(follow_symlinks=False))
+ def test_unlink(self):
+ p = self.cls(self.base) / 'fileA'
+ p.unlink()
+ self.assertFileNotFound(p.stat)
+ self.assertFileNotFound(p.unlink)
+
+ def test_unlink_missing_ok(self):
+ p = self.cls(self.base) / 'fileAAA'
+ self.assertFileNotFound(p.unlink)
+ p.unlink(missing_ok=True)
+
+ def test_rmdir(self):
+ p = self.cls(self.base) / 'dirA'
+ for q in p.iterdir():
+ q.unlink()
+ p.rmdir()
+ self.assertFileNotFound(p.stat)
+ self.assertFileNotFound(p.unlink)
+
@needs_symlinks
def test_delete_symlink(self):
tmp = self.cls(self.base, 'delete')
diff --git a/Lib/test/test_pathlib/test_pathlib_abc.py
b/Lib/test/test_pathlib/test_pathlib_abc.py
index bf9ae6cc8a2433..675abf30a9f13c 100644
--- a/Lib/test/test_pathlib/test_pathlib_abc.py
+++ b/Lib/test/test_pathlib/test_pathlib_abc.py
@@ -1370,8 +1370,6 @@ def test_unsupported_operation(self):
self.assertRaises(e, p.touch)
self.assertRaises(e, p.chmod, 0o755)
self.assertRaises(e, p.lchmod, 0o755)
- self.assertRaises(e, p.unlink)
- self.assertRaises(e, p.rmdir)
self.assertRaises(e, p.owner)
self.assertRaises(e, p.group)
self.assertRaises(e, p.as_uri)
@@ -1493,31 +1491,18 @@ def mkdir(self, mode=0o777, parents=False,
exist_ok=False):
self.parent.mkdir(parents=True, exist_ok=True)
self.mkdir(mode, parents=False, exist_ok=exist_ok)
- def unlink(self, missing_ok=False):
- path = str(self)
- name = self.name
- parent = str(self.parent)
- if path in self._directories:
- raise IsADirectoryError(errno.EISDIR, "Is a directory", path)
- elif path in self._files:
- self._directories[parent].remove(name)
- del self._files[path]
- elif not missing_ok:
- raise FileNotFoundError(errno.ENOENT, "File not found", path)
-
- def rmdir(self):
+ def _delete(self):
path = str(self)
if path in self._files:
- raise NotADirectoryError(errno.ENOTDIR, "Not a directory", path)
- elif path not in self._directories:
- raise FileNotFoundError(errno.ENOENT, "File not found", path)
- elif self._directories[path]:
- raise OSError(errno.ENOTEMPTY, "Directory not empty", path)
- else:
- name = self.name
- parent = str(self.parent)
- self._directories[parent].remove(name)
+ del self._files[path]
+ elif path in self._directories:
+ for name in list(self._directories[path]):
+ self.joinpath(name)._delete()
del self._directories[path]
+ else:
+ raise FileNotFoundError(errno.ENOENT, "File not found", path)
+ parent = str(self.parent)
+ self._directories[parent].remove(self.name)
class DummyPathTest(DummyPurePathTest):
@@ -2245,30 +2230,11 @@ def test_is_char_device_false(self):
self.assertIs((P / 'fileA\udfff').is_char_device(), False)
self.assertIs((P / 'fileA\x00').is_char_device(), False)
- def test_unlink(self):
- p = self.cls(self.base) / 'fileA'
- p.unlink()
- self.assertFileNotFound(p.stat)
- self.assertFileNotFound(p.unlink)
-
- def test_unlink_missing_ok(self):
- p = self.cls(self.base) / 'fileAAA'
- self.assertFileNotFound(p.unlink)
- p.unlink(missing_ok=True)
-
- def test_rmdir(self):
- p = self.cls(self.base) / 'dirA'
- for q in p.iterdir():
- q.unlink()
- p.rmdir()
- self.assertFileNotFound(p.stat)
- self.assertFileNotFound(p.unlink)
-
def test_delete_file(self):
p = self.cls(self.base) / 'fileA'
p._delete()
self.assertFileNotFound(p.stat)
- self.assertFileNotFound(p.unlink)
+ self.assertFileNotFound(p._delete)
def test_delete_dir(self):
base = self.cls(self.base)
@@ -2347,7 +2313,7 @@ def setUp(self):
def tearDown(self):
base = self.cls(self.base)
- base._rmtree()
+ base._delete()
def test_walk_topdown(self):
walker = self.walk_path.walk()
_______________________________________________
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]
