From 89068025e43da426ec1c715d0a0d346df53e1192 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaspar=20L=C3=B6chte?= Date: Tue, 8 Feb 2022 12:13:12 +0100 Subject: [PATCH 1/7] bpo-20779 add chown() and lchown() to Path() in pathlib No support for WindowsPath --- Lib/pathlib.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/Lib/pathlib.py b/Lib/pathlib.py index 7f4210e2b80c9b..77abae46edfe9a 100644 --- a/Lib/pathlib.py +++ b/Lib/pathlib.py @@ -1127,6 +1127,19 @@ def lchmod(self, mode): """ self.chmod(mode, follow_symlinks=False) + def chown(self, uid, gid, *, dir_fd=None, follow_symlinks=True): + """ + Change the owner and group id of path to the numeric uid and gid, like os.chown(). + """ + os.chown(self, uid, gid, dir_fd=dir_fd, follow_symlinks=follow_symlinks) + + def lchown(self, uid, gid, *, dir_fd=None): + """ + Like chown(), except if the path points to a symlink, the symlink's + permissions are changed, rather than its target's. + """ + self.chown(uid, gid, dir_fd=dir_fd, follow_symlinks=False) + def unlink(self, missing_ok=False): """ Remove this file or link. @@ -1393,3 +1406,9 @@ class WindowsPath(Path, PureWindowsPath): def is_mount(self): raise NotImplementedError("Path.is_mount() is unsupported on this system") + + def chown(self, uid, gid, *, dir_fd=None, follow_symlinks=True): + raise NotImplementedError("Path.chown() is unsupported on this system") + + def lchown(self, uid, gid, *, dir_fd=None): + raise NotImplementedError("Path.lchown() is unsupported on this system") From 5bb6b63358228c25bb43354972fcabfb1aee2cff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaspar=20L=C3=B6chte?= Date: Tue, 8 Feb 2022 12:13:46 +0100 Subject: [PATCH 2/7] Add tests for chown() and lchown() for pathlib.Path() --- Lib/test/test_pathlib.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/Lib/test/test_pathlib.py b/Lib/test/test_pathlib.py index ec2baca18fd817..4ba3f741ec4be3 100644 --- a/Lib/test/test_pathlib.py +++ b/Lib/test/test_pathlib.py @@ -1874,6 +1874,34 @@ def test_chmod(self): p.chmod(new_mode) self.assertEqual(p.stat().st_mode, new_mode) + def test_chown(self): + p = self.cls(BASE) / 'fileA' + uid = p.stat().st_uid + gid = p.stat().st_gid + new_uid = 2000 + new_gid = 2000 + p.chown(uid=new_uid, gid=new_gid) + self.assertEqual(p.stat().st_uid, new_uid) + self.assertEqual(p.stat().st_gid, new_gid) + # Set back + p.chown(uid=uid, gid=gid) + self.assertEqual(p.stat().st_uid, uid) + self.assertEqual(p.stat().st_gid, gid) + + def test_lchown(self): + p = self.cls(BASE) / 'fileA' + uid = p.stat().st_uid + gid = p.stat().st_gid + new_uid = 2000 + new_gid = 2000 + p.lchown(uid=new_uid, gid=new_gid) + self.assertEqual(p.stat().st_uid, new_uid) + self.assertEqual(p.stat().st_gid, new_gid) + # Set back + p.lchown(uid=uid, gid=gid) + self.assertEqual(p.stat().st_uid, uid) + self.assertEqual(p.stat().st_gid, gid) + # On Windows, os.chmod does not follow symlinks (issue #15411) @only_posix def test_chmod_follow_symlinks_true(self): From dbdbbbe7436fffd31a26e0840d1aea3c32e5ce68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaspar=20L=C3=B6chte?= Date: Tue, 8 Feb 2022 12:30:12 +0100 Subject: [PATCH 3/7] Add patch entry to Misc/NEWS.d/next --- .../next/Library/2022-02-08-12-26-20.bpo-20779.Gha5Fa.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2022-02-08-12-26-20.bpo-20779.Gha5Fa.rst diff --git a/Misc/NEWS.d/next/Library/2022-02-08-12-26-20.bpo-20779.Gha5Fa.rst b/Misc/NEWS.d/next/Library/2022-02-08-12-26-20.bpo-20779.Gha5Fa.rst new file mode 100644 index 00000000000000..bedceeacb74840 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2022-02-08-12-26-20.bpo-20779.Gha5Fa.rst @@ -0,0 +1,2 @@ +Add :func:`pathlib.Path.chown` and :func:`pathlib.Path.lchown` +to :class:`pathlib.Path`. Patch by Jaspar Stach. From d4070a39ab701157d74c67d459c773de62ddbd6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaspar=20L=C3=B6chte?= Date: Tue, 8 Feb 2022 12:47:04 +0100 Subject: [PATCH 4/7] Add documentation for chown and lchown, fix typo in lchmod --- Doc/library/pathlib.rst | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/Doc/library/pathlib.rst b/Doc/library/pathlib.rst index 7ab603fd133b86..4f14ca2206eed3 100644 --- a/Doc/library/pathlib.rst +++ b/Doc/library/pathlib.rst @@ -755,6 +755,30 @@ call fails (for example because the path doesn't exist). .. versionchanged:: 3.10 The *follow_symlinks* parameter was added. +.. method:: Path.chown(uid, gid, *, dir_fd=None, follow_symlinks=True) + + Change the file ownership, like :func:`os.chown`. + + This method normally follows symlinks. Some Unix flavours support changing + permissions on the symlink itself; on these platforms you may add the + argument ``follow_symlinks=False``, or use :meth:`~Path.lchown`. + + :: + + >>> p = Path('setup.py') + >>> p.stat().st_uid + 1000 + >>> p.stat().st_gid + 1000 + >>> p.chown(1, 20) + >>> p.stat().st_uid + 1 + >>> p.stat().st_gid + 20 + + .. availability:: Unix. + + .. method:: Path.exists() Whether the path points to an existing file or directory:: @@ -923,9 +947,17 @@ call fails (for example because the path doesn't exist). symbolic link's mode is changed rather than its target's. +.. method:: Path.lchown(uid, gid, *, dir_fd=None) + + Like :meth:`Path.chown`, but if the path points to a symbolic link, the + symbolic link's mode is changed rather than its target's. + + .. availability:: Unix. + + .. method:: Path.lstat() - Like :meth:`Path.stat` but, if the path points to a symbolic link, return + Like :meth:`Path.stat`, but if the path points to a symbolic link, return the symbolic link's information rather than its target's. @@ -1260,6 +1292,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` From f9a611b3165362a08563ffa7da1c39a7c0d26551 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaspar=20L=C3=B6chte?= Date: Tue, 8 Feb 2022 14:23:23 +0100 Subject: [PATCH 5/7] Improve and extend the tests --- Lib/test/test_pathlib.py | 87 ++++++++++++++++++++++++++++++---------- 1 file changed, 66 insertions(+), 21 deletions(-) diff --git a/Lib/test/test_pathlib.py b/Lib/test/test_pathlib.py index 4ba3f741ec4be3..95336ac9c234ac 100644 --- a/Lib/test/test_pathlib.py +++ b/Lib/test/test_pathlib.py @@ -21,6 +21,15 @@ except ImportError: grp = pwd = None +try: + import pwd + all_users = [u.pw_uid for u in pwd.getpwall()] +except (ImportError, AttributeError): + all_users = [] + +root_in_posix = False +if hasattr(os, 'geteuid'): + root_in_posix = (os.geteuid() == 0) class _BaseFlavourTest(object): @@ -1874,33 +1883,69 @@ def test_chmod(self): p.chmod(new_mode) self.assertEqual(p.stat().st_mode, new_mode) - def test_chown(self): + @unittest.skipUnless(root_in_posix and len(all_users) > 1, + "test needs root privilege and more than one user") + def test_chown_with_root(self): + # original uid and gid p = self.cls(BASE) / 'fileA' uid = p.stat().st_uid gid = p.stat().st_gid - new_uid = 2000 - new_gid = 2000 - p.chown(uid=new_uid, gid=new_gid) - self.assertEqual(p.stat().st_uid, new_uid) - self.assertEqual(p.stat().st_gid, new_gid) - # Set back + + # get users and groups for testing + uid_1, uid_2 = all_users[:2] + groups = os.getgroups() + if len(groups) < 2: + self.skipTest("test needs at least 2 groups") + gid_1, gid_2 = groups[:2] + + p.chown(uid=uid_1, gid=gid_1) + self.assertEqual(p.stat().st_uid, uid_1) + self.assertEqual(p.stat().st_gid, gid_1) + p.chown(uid=uid_2, gid=gid_2) + self.assertEqual(p.stat().st_uid, uid_2) + self.assertEqual(p.stat().st_gid, gid_2) + + # Set back to original p.chown(uid=uid, gid=gid) - self.assertEqual(p.stat().st_uid, uid) - self.assertEqual(p.stat().st_gid, gid) - def test_lchown(self): + @unittest.skipUnless(not root_in_posix and len(all_users) > 1, + "test needs non-root account and more than one user") + def test_chown_without_permission(self): p = self.cls(BASE) / 'fileA' - uid = p.stat().st_uid - gid = p.stat().st_gid - new_uid = 2000 - new_gid = 2000 - p.lchown(uid=new_uid, gid=new_gid) - self.assertEqual(p.stat().st_uid, new_uid) - self.assertEqual(p.stat().st_gid, new_gid) - # Set back - p.lchown(uid=uid, gid=gid) - self.assertEqual(p.stat().st_uid, uid) - self.assertEqual(p.stat().st_gid, gid) + + new_uid = 503 + new_gid = 503 + with self.assertRaises(PermissionError): + p.chown(uid=new_uid, gid=new_gid) + + @only_nt + def test_chown_windows(self): + p = self.cls(BASE) / 'fileA' + + new_uid = 503 + new_gid = 503 + with self.assertRaises(NotImplementedError): + p.chown(uid=new_uid, gid=new_gid) + + @only_posix + @mock.patch('pathlib.Path.chown') + def test_lchown(self, chown_mock): + new_uid = 503 + new_gid = 503 + + p = self.cls(BASE) / 'fileA' + + p.lchown(new_uid, new_gid) + chown_mock.assert_called_with(new_uid, new_gid, dir_fd=None, follow_symlinks=False) + + @only_nt + def test_lchown_windows(self): + p = self.cls(BASE) / 'fileA' + + new_uid = 503 + new_gid = 503 + with self.assertRaises(NotImplementedError): + p.lchown(uid=new_uid, gid=new_gid) # On Windows, os.chmod does not follow symlinks (issue #15411) @only_posix From 06aebeb0c1139c205140b14745d7cd135feab3b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaspar=20L=C3=B6chte?= Date: Tue, 3 May 2022 07:44:16 +0200 Subject: [PATCH 6/7] Remove `lchown()`, don't expose `dir_fd` --- Doc/library/pathlib.rst | 12 ++--------- Lib/pathlib.py | 16 +++------------ Lib/test/test_pathlib.py | 20 ------------------- .../2022-02-08-12-26-20.bpo-20779.Gha5Fa.rst | 3 +-- 4 files changed, 6 insertions(+), 45 deletions(-) diff --git a/Doc/library/pathlib.rst b/Doc/library/pathlib.rst index 4f14ca2206eed3..f01af0f9efe4c5 100644 --- a/Doc/library/pathlib.rst +++ b/Doc/library/pathlib.rst @@ -755,13 +755,13 @@ call fails (for example because the path doesn't exist). .. versionchanged:: 3.10 The *follow_symlinks* parameter was added. -.. method:: Path.chown(uid, gid, *, dir_fd=None, follow_symlinks=True) +.. method:: Path.chown(uid, gid, *, follow_symlinks=True) Change the file ownership, like :func:`os.chown`. This method normally follows symlinks. Some Unix flavours support changing permissions on the symlink itself; on these platforms you may add the - argument ``follow_symlinks=False``, or use :meth:`~Path.lchown`. + argument ``follow_symlinks=False``. :: @@ -947,14 +947,6 @@ call fails (for example because the path doesn't exist). symbolic link's mode is changed rather than its target's. -.. method:: Path.lchown(uid, gid, *, dir_fd=None) - - Like :meth:`Path.chown`, but if the path points to a symbolic link, the - symbolic link's mode is changed rather than its target's. - - .. availability:: Unix. - - .. method:: Path.lstat() Like :meth:`Path.stat`, but if the path points to a symbolic link, return diff --git a/Lib/pathlib.py b/Lib/pathlib.py index 77abae46edfe9a..18acf587e8bfda 100644 --- a/Lib/pathlib.py +++ b/Lib/pathlib.py @@ -1127,18 +1127,11 @@ def lchmod(self, mode): """ self.chmod(mode, follow_symlinks=False) - def chown(self, uid, gid, *, dir_fd=None, follow_symlinks=True): + def chown(self, uid, gid, *, follow_symlinks=True): """ Change the owner and group id of path to the numeric uid and gid, like os.chown(). """ - os.chown(self, uid, gid, dir_fd=dir_fd, follow_symlinks=follow_symlinks) - - def lchown(self, uid, gid, *, dir_fd=None): - """ - Like chown(), except if the path points to a symlink, the symlink's - permissions are changed, rather than its target's. - """ - self.chown(uid, gid, dir_fd=dir_fd, follow_symlinks=False) + os.chown(self, uid, gid, follow_symlinks=follow_symlinks) def unlink(self, missing_ok=False): """ @@ -1407,8 +1400,5 @@ class WindowsPath(Path, PureWindowsPath): def is_mount(self): raise NotImplementedError("Path.is_mount() is unsupported on this system") - def chown(self, uid, gid, *, dir_fd=None, follow_symlinks=True): + def chown(self, uid, gid, *, follow_symlinks=True): raise NotImplementedError("Path.chown() is unsupported on this system") - - def lchown(self, uid, gid, *, dir_fd=None): - raise NotImplementedError("Path.lchown() is unsupported on this system") diff --git a/Lib/test/test_pathlib.py b/Lib/test/test_pathlib.py index 95336ac9c234ac..56362e6182dd7f 100644 --- a/Lib/test/test_pathlib.py +++ b/Lib/test/test_pathlib.py @@ -1926,26 +1926,6 @@ def test_chown_windows(self): new_gid = 503 with self.assertRaises(NotImplementedError): p.chown(uid=new_uid, gid=new_gid) - - @only_posix - @mock.patch('pathlib.Path.chown') - def test_lchown(self, chown_mock): - new_uid = 503 - new_gid = 503 - - p = self.cls(BASE) / 'fileA' - - p.lchown(new_uid, new_gid) - chown_mock.assert_called_with(new_uid, new_gid, dir_fd=None, follow_symlinks=False) - - @only_nt - def test_lchown_windows(self): - p = self.cls(BASE) / 'fileA' - - new_uid = 503 - new_gid = 503 - with self.assertRaises(NotImplementedError): - p.lchown(uid=new_uid, gid=new_gid) # On Windows, os.chmod does not follow symlinks (issue #15411) @only_posix diff --git a/Misc/NEWS.d/next/Library/2022-02-08-12-26-20.bpo-20779.Gha5Fa.rst b/Misc/NEWS.d/next/Library/2022-02-08-12-26-20.bpo-20779.Gha5Fa.rst index bedceeacb74840..c94d9eee72ff0a 100644 --- a/Misc/NEWS.d/next/Library/2022-02-08-12-26-20.bpo-20779.Gha5Fa.rst +++ b/Misc/NEWS.d/next/Library/2022-02-08-12-26-20.bpo-20779.Gha5Fa.rst @@ -1,2 +1 @@ -Add :func:`pathlib.Path.chown` and :func:`pathlib.Path.lchown` -to :class:`pathlib.Path`. Patch by Jaspar Stach. +Add :func:`pathlib.Path.chown` to :class:`pathlib.Path`. Patch by Jaspar Stach. From cfae2d41d109bd3840d4aa3c577f665f44ebf3a5 Mon Sep 17 00:00:00 2001 From: Jaspar S Date: Sat, 7 May 2022 08:28:04 +0200 Subject: [PATCH 7/7] Apply suggestions from code review Co-authored-by: Barney Gale --- Doc/library/pathlib.rst | 2 ++ .../next/Library/2022-02-08-12-26-20.bpo-20779.Gha5Fa.rst | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/Doc/library/pathlib.rst b/Doc/library/pathlib.rst index f01af0f9efe4c5..eb526d3437c8da 100644 --- a/Doc/library/pathlib.rst +++ b/Doc/library/pathlib.rst @@ -778,6 +778,8 @@ call fails (for example because the path doesn't exist). .. availability:: Unix. + .. versionadded:: 3.12 + .. method:: Path.exists() diff --git a/Misc/NEWS.d/next/Library/2022-02-08-12-26-20.bpo-20779.Gha5Fa.rst b/Misc/NEWS.d/next/Library/2022-02-08-12-26-20.bpo-20779.Gha5Fa.rst index c94d9eee72ff0a..d5e52de5ae15e7 100644 --- a/Misc/NEWS.d/next/Library/2022-02-08-12-26-20.bpo-20779.Gha5Fa.rst +++ b/Misc/NEWS.d/next/Library/2022-02-08-12-26-20.bpo-20779.Gha5Fa.rst @@ -1 +1,2 @@ -Add :func:`pathlib.Path.chown` to :class:`pathlib.Path`. Patch by Jaspar Stach. +Add :meth:`pathlib.Path.chown` method that changes file ownership. +Patch by Jaspar Stach.