summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorBarney Gale <barney.gale@gmail.com>2024-12-08 18:45:09 (GMT)
committerGitHub <noreply@github.com>2024-12-08 18:45:09 (GMT)
commit7f8ec523021427a5c1ab3ce0cdd6e4bb909f1dc5 (patch)
tree7939eb62c8eb50421ab9024e0d9b27c0e04bf90c
parent2367759212f609b8ddf3218003b3ccd8e72849ae (diff)
downloadcpython-7f8ec523021427a5c1ab3ce0cdd6e4bb909f1dc5.zip
cpython-7f8ec523021427a5c1ab3ce0cdd6e4bb909f1dc5.tar.gz
cpython-7f8ec523021427a5c1ab3ce0cdd6e4bb909f1dc5.tar.bz2
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.
-rw-r--r--Lib/pathlib/_abc.py43
-rw-r--r--Lib/pathlib/_local.py16
-rw-r--r--Lib/test/test_pathlib/test_pathlib.py19
-rw-r--r--Lib/test/test_pathlib/test_pathlib_abc.py56
4 files changed, 48 insertions, 86 deletions
diff --git a/Lib/pathlib/_abc.py b/Lib/pathlib/_abc.py
index 820970f..309eab2 100644
--- a/Lib/pathlib/_abc.py
+++ b/Lib/pathlib/_abc.py
@@ -840,6 +840,12 @@ class PathBase(PurePathBase):
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 @@ class PathBase(PurePathBase):
"""
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 250bc12..f87069c 100644
--- a/Lib/pathlib/_local.py
+++ b/Lib/pathlib/_local.py
@@ -846,10 +846,18 @@ class Path(PathBase, PurePath):
"""
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 8c9049f..ce0f474 100644
--- a/Lib/test/test_pathlib/test_pathlib.py
+++ b/Lib/test/test_pathlib/test_pathlib.py
@@ -1352,6 +1352,25 @@ class PathTest(test_pathlib_abc.DummyPathTest, PurePathTest):
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 bf9ae6c..675abf3 100644
--- a/Lib/test/test_pathlib/test_pathlib_abc.py
+++ b/Lib/test/test_pathlib/test_pathlib_abc.py
@@ -1370,8 +1370,6 @@ class PathBaseTest(PurePathBaseTest):
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 @@ class DummyPath(PathBase):
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 @@ class DummyPathTest(DummyPurePathTest):
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 @@ class DummyPathWalkTest(unittest.TestCase):
def tearDown(self):
base = self.cls(self.base)
- base._rmtree()
+ base._delete()
def test_walk_topdown(self):
walker = self.walk_path.walk()