From 6f45e763193e2c6c7a7f18a75c5f44cbcbf098dd Mon Sep 17 00:00:00 2001 From: barneygale Date: Thu, 4 May 2023 23:36:20 +0100 Subject: [PATCH 1/4] GH-64978: Add `pathlib.Path.chown()` Move implementation from `shutil.chown()`. This function now calls through to pathlib. --- Doc/library/pathlib.rst | 15 +++++ Lib/pathlib.py | 32 ++++++++++ Lib/shutil.py | 32 +++------- Lib/test/test_pathlib.py | 61 +++++++++++++++++++ ...3-05-04-23-22-33.gh-issue-64978.lwBZ3C.rst | 2 + 5 files changed, 119 insertions(+), 23 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2023-05-04-23-22-33.gh-issue-64978.lwBZ3C.rst diff --git a/Doc/library/pathlib.rst b/Doc/library/pathlib.rst index 14118127835bbe..20d3a575e9529c 100644 --- a/Doc/library/pathlib.rst +++ b/Doc/library/pathlib.rst @@ -819,6 +819,19 @@ call fails (for example because the path doesn't exist). .. versionchanged:: 3.10 The *follow_symlinks* parameter was added. +.. method:: Path.chown(owner=None, group=None) + + Change the *owner* and/or *group* of the path. + + *owner* can be a system user name or a uid; the same applies to *group*. + At least one argument is required. + + .. audit-event:: pathlib.Path.chown path,owner,group pathlib.Path.chown + + .. availability:: Unix. + + .. versionadded:: 3.12 + .. method:: Path.exists(*, follow_symlinks=True) Return ``True`` if the path points to an existing file or directory. @@ -1442,6 +1455,7 @@ Below is a table mapping various :mod:`os` functions to their corresponding :func:`os.path.abspath` :meth:`Path.absolute` [#]_ :func:`os.path.realpath` :meth:`Path.resolve` :func:`os.chmod` :meth:`Path.chmod` +:func:`os.chown` :meth:`Path.chown` [#]_ :func:`os.mkdir` :meth:`Path.mkdir` :func:`os.makedirs` :meth:`Path.mkdir` :func:`os.rename` :meth:`Path.rename` @@ -1476,4 +1490,5 @@ Below is a table mapping various :mod:`os` functions to their corresponding .. rubric:: Footnotes .. [#] :func:`os.path.abspath` normalizes the resulting path, which may change its meaning in the presence of symlinks, while :meth:`Path.absolute` does not. +.. [#] :meth:`Path.chown` supports owner and group names and IDs, whereas :func:`os.chown` only supports IDs. .. [#] :meth:`PurePath.relative_to` requires ``self`` to be the subpath of the argument, but :func:`os.path.relpath` does not. diff --git a/Lib/pathlib.py b/Lib/pathlib.py index f32e1e2d822834..f688fd1d42d400 100644 --- a/Lib/pathlib.py +++ b/Lib/pathlib.py @@ -1029,6 +1029,38 @@ def lchmod(self, mode): """ self.chmod(mode, follow_symlinks=False) + if hasattr(os, 'chown'): + def chown(self, owner=None, group=None): + """Change the owner and/or group of the path. + + owner and group can be the uid/gid or the user/group names, and + in that case, they are converted to their respective uid/gid. + """ + sys.audit('pathlib.Path.chown', self, owner, group) + + if owner is None: + owner = -1 + elif isinstance(owner, str): + try: + import pwd + owner = pwd.getpwnam(owner)[2] + except (ImportError, KeyError): + raise LookupError(f"no such user: {owner!r}") from None + + if group is None: + group = -1 + elif isinstance(group, str): + try: + import grp + group = grp.getgrnam(group)[2] + except (ImportError, KeyError): + raise LookupError(f"no such group: {group!r}") from None + + if owner == -1 and group == -1: + raise ValueError("user and/or group must be set") + + os.chown(self, owner, group) + def unlink(self, missing_ok=False): """ Remove this file or link. diff --git a/Lib/shutil.py b/Lib/shutil.py index 7d1a3d00011f37..f0811bd3082aed 100644 --- a/Lib/shutil.py +++ b/Lib/shutil.py @@ -10,6 +10,7 @@ import fnmatch import collections import errno +import pathlib import warnings try: @@ -1381,29 +1382,14 @@ def chown(path, user=None, group=None): """ sys.audit('shutil.chown', path, user, group) - if user is None and group is None: - raise ValueError("user and/or group must be set") - - _user = user - _group = group - - # -1 means don't change it - if user is None: - _user = -1 - # user can either be an int (the uid) or a string (the system username) - elif isinstance(user, str): - _user = _get_uid(user) - if _user is None: - raise LookupError("no such user: {!r}".format(user)) - - if group is None: - _group = -1 - elif not isinstance(group, int): - _group = _get_gid(group) - if _group is None: - raise LookupError("no such group: {!r}".format(group)) - - os.chown(path, _user, _group) + if not isinstance(path, pathlib.Path): + path = os.fspath(path) + if isinstance(path, bytes): + path = os.fsdecode(path) + path = pathlib.Path(path) + + path.chown(user, group) + def get_terminal_size(fallback=(80, 24)): """Get the size of the terminal window. diff --git a/Lib/test/test_pathlib.py b/Lib/test/test_pathlib.py index a932e03df4236d..f96d726836b30a 100644 --- a/Lib/test/test_pathlib.py +++ b/Lib/test/test_pathlib.py @@ -3078,6 +3078,67 @@ def test_handling_bad_descriptor(self): self.fail("Bad file descriptor not handled.") raise + @unittest.skipUnless(pwd, "the pwd module is needed for this test") + @unittest.skipUnless(grp, "the grp module is needed for this test") + @unittest.skipUnless(hasattr(os, 'chown'), 'requires os.chown') + def test_chown(self): + dirname = self.cls(BASE) + filename = dirname / "fileA" + + with self.assertRaises(ValueError): + filename.chown() + + with self.assertRaises(LookupError): + filename.chown(user='non-existing username') + + with self.assertRaises(LookupError): + filename.chown(group='non-existing groupname') + + with self.assertRaises(TypeError): + filename.chown(b'spam') + + with self.assertRaises(TypeError): + filename.chown(3.14) + + uid = os.getuid() + gid = os.getgid() + + def check_chown(path, uid=None, gid=None): + s = path.stat() + if uid is not None: + self.assertEqual(uid, s.st_uid) + if gid is not None: + self.assertEqual(gid, s.st_gid) + + filename.chown(uid, gid) + check_chown(filename, uid, gid) + filename.chown(uid) + check_chown(filename, uid) + filename.chown(user=uid) + check_chown(filename, uid) + filename.chown(group=gid) + check_chown(filename, gid=gid) + + dirname.chown(uid, gid) + check_chown(dirname, uid, gid) + dirname.chown(uid) + check_chown(dirname, uid) + dirname.chown(user=uid) + check_chown(dirname, uid) + dirname.chown(group=gid) + check_chown(dirname, gid=gid) + + try: + user = pwd.getpwuid(uid)[0] + group = grp.getgrgid(gid)[0] + except KeyError: + # On some systems uid/gid cannot be resolved. + pass + else: + filename.chown(user, group) + check_chown(filename, uid, gid) + dirname.chown(user, group) + check_chown(dirname, uid, gid) @only_nt class WindowsPathTest(_BasePathTest, unittest.TestCase): diff --git a/Misc/NEWS.d/next/Library/2023-05-04-23-22-33.gh-issue-64978.lwBZ3C.rst b/Misc/NEWS.d/next/Library/2023-05-04-23-22-33.gh-issue-64978.lwBZ3C.rst new file mode 100644 index 00000000000000..9d9e04ef3d3e33 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2023-05-04-23-22-33.gh-issue-64978.lwBZ3C.rst @@ -0,0 +1,2 @@ +Add :meth:`pathlib.Path.chown`, which changes the owner and/or group of a +path. From e7dd3d7529b0049214635bfb3012dbee7ea1c51e Mon Sep 17 00:00:00 2001 From: barneygale Date: Thu, 4 May 2023 23:49:17 +0100 Subject: [PATCH 2/4] Delay urllib import --- Lib/pathlib.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/pathlib.py b/Lib/pathlib.py index f688fd1d42d400..55e669dfd243cb 100644 --- a/Lib/pathlib.py +++ b/Lib/pathlib.py @@ -17,7 +17,6 @@ from _collections_abc import Sequence from errno import ENOENT, ENOTDIR, EBADF, ELOOP from stat import S_ISDIR, S_ISLNK, S_ISREG, S_ISSOCK, S_ISBLK, S_ISCHR, S_ISFIFO -from urllib.parse import quote_from_bytes as urlquote_from_bytes __all__ = [ @@ -405,7 +404,8 @@ def as_uri(self): # It's a posix path => 'file:///etc/hosts' prefix = 'file://' path = str(self) - return prefix + urlquote_from_bytes(os.fsencode(path)) + from urllib.parse import quote_from_bytes + return prefix + quote_from_bytes(os.fsencode(path)) @property def _str_normcase(self): From c2808b46c00692f07359cd21683022989d9ffdfc Mon Sep 17 00:00:00 2001 From: barneygale Date: Thu, 4 May 2023 23:50:12 +0100 Subject: [PATCH 3/4] Fix tests --- Lib/test/test_pathlib.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Lib/test/test_pathlib.py b/Lib/test/test_pathlib.py index f96d726836b30a..d1ec418ac0c3a5 100644 --- a/Lib/test/test_pathlib.py +++ b/Lib/test/test_pathlib.py @@ -3089,7 +3089,7 @@ def test_chown(self): filename.chown() with self.assertRaises(LookupError): - filename.chown(user='non-existing username') + filename.chown(owner='non-existing username') with self.assertRaises(LookupError): filename.chown(group='non-existing groupname') @@ -3114,7 +3114,7 @@ def check_chown(path, uid=None, gid=None): check_chown(filename, uid, gid) filename.chown(uid) check_chown(filename, uid) - filename.chown(user=uid) + filename.chown(owner=uid) check_chown(filename, uid) filename.chown(group=gid) check_chown(filename, gid=gid) @@ -3123,21 +3123,21 @@ def check_chown(path, uid=None, gid=None): check_chown(dirname, uid, gid) dirname.chown(uid) check_chown(dirname, uid) - dirname.chown(user=uid) + dirname.chown(owner=uid) check_chown(dirname, uid) dirname.chown(group=gid) check_chown(dirname, gid=gid) try: - user = pwd.getpwuid(uid)[0] + owner = pwd.getpwuid(uid)[0] group = grp.getgrgid(gid)[0] except KeyError: # On some systems uid/gid cannot be resolved. pass else: - filename.chown(user, group) + filename.chown(owner, group) check_chown(filename, uid, gid) - dirname.chown(user, group) + dirname.chown(owner, group) check_chown(dirname, uid, gid) @only_nt From 156ea26e4d78647959a75d4927d23d916ce69952 Mon Sep 17 00:00:00 2001 From: barneygale Date: Fri, 5 May 2023 00:28:56 +0100 Subject: [PATCH 4/4] Fix warning --- Lib/pathlib.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Lib/pathlib.py b/Lib/pathlib.py index 55e669dfd243cb..a10f03a03daa4c 100644 --- a/Lib/pathlib.py +++ b/Lib/pathlib.py @@ -1039,6 +1039,8 @@ def chown(self, owner=None, group=None): sys.audit('pathlib.Path.chown', self, owner, group) if owner is None: + if group is None: + raise ValueError("user and/or group must be set") owner = -1 elif isinstance(owner, str): try: @@ -1056,9 +1058,6 @@ def chown(self, owner=None, group=None): except (ImportError, KeyError): raise LookupError(f"no such group: {group!r}") from None - if owner == -1 and group == -1: - raise ValueError("user and/or group must be set") - os.chown(self, owner, group) def unlink(self, missing_ok=False):