-
-
Couldn't load subscription status.
- Fork 33.3k
gh-128432: Add umask context manager to shutil
#128433
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 6 commits
45fb826
fa40444
c665655
9438fa1
943d0c9
e8c824f
d53cf30
dc1dcc0
0ff7997
df59fde
12ceaac
bb8955c
786dddf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -877,6 +877,50 @@ Querying the size of the output terminal | |
| The ``fallback`` values are also used if :func:`os.get_terminal_size` | ||
| returns zeroes. | ||
|
|
||
| .. _shutil-context-managers: | ||
|
|
||
| Context managers to modify process execution environments | ||
| --------------------------------------------------------- | ||
|
|
||
| High-level :term:`context managers <context manager>` for changing a process's execution environment. | ||
|
|
||
| .. warning:: | ||
|
|
||
| These may change process-wide states, such as the current working directory, | ||
| and as such are not suitable for use in most threaded or async contexts. | ||
| They are also not suitable for most non-linear code execution, like generators, | ||
| where the program execution is temporarily relinquished. Unless explicitly | ||
| desired, you should not yield within these context managers. | ||
|
|
||
| .. class:: umask(mask) | ||
jb2170 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| This is a simple wrapper around :func:`os.umask`. It changes the process's | ||
jb2170 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| umask upon entering and restores the old one on exit. | ||
|
|
||
| This context manager is :ref:`reentrant <reentrant-cms>`. | ||
|
|
||
| .. versionadded:: 3.14 | ||
jb2170 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| In this example, we use a :class:`umask` context manager, within which we | ||
jb2170 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| create a file that only the user can access:: | ||
|
|
||
| >>> from shutil import umask | ||
| >>> private_umask = umask(0o077) | ||
| >>> with private_umask: | ||
jb2170 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| ... with open("my-secret-file", "w") as f: | ||
| ... f.write("I ate all the cake!\n") | ||
jb2170 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| The file's permissions are empty for anyone but the user: | ||
|
|
||
| .. code-block:: shell-session | ||
|
|
||
| $ ls -l my-secret-file | ||
| -rw------- 1 jb2170 jb2170 20 Jan 2 01:23 my-secret-file | ||
jb2170 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| Using :class:`umask` like this is better practice than first creating the file, | ||
jb2170 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| and later changing its permissions with :func:`~os.chmod`, between which a | ||
| period of time exists in which the file may have too lenient permissions. | ||
|
||
|
|
||
| .. _`fcopyfile`: | ||
| http://www.manpagez.com/man/3/copyfile/ | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -10,6 +10,7 @@ | |||||||
| import fnmatch | ||||||||
| import collections | ||||||||
| import errno | ||||||||
| from contextlib import AbstractContextManager | ||||||||
|
|
||||||||
| try: | ||||||||
| import zlib | ||||||||
|
|
@@ -61,7 +62,7 @@ | |||||||
| "get_unpack_formats", "register_unpack_format", | ||||||||
| "unregister_unpack_format", "unpack_archive", | ||||||||
| "ignore_patterns", "chown", "which", "get_terminal_size", | ||||||||
| "SameFileError"] | ||||||||
| "SameFileError", "umask"] | ||||||||
jb2170 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||
| # disk_usage is added later, if available on the platform | ||||||||
|
|
||||||||
| class Error(OSError): | ||||||||
|
|
@@ -1581,6 +1582,21 @@ def which(cmd, mode=os.F_OK | os.X_OK, path=None): | |||||||
| return name | ||||||||
| return None | ||||||||
|
|
||||||||
|
|
||||||||
| class umask(AbstractContextManager): | ||||||||
|
||||||||
| @classmethod |
So we can skip the import and explicit inheritance from the ABC.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| class umask(AbstractContextManager): | |
| class temp_umask: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Partially resolved by dc1dcc0. I think we should keep inheriting from the ABC; isn't that sort of what it's for?
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,7 +21,7 @@ | |
| get_archive_formats, Error, unpack_archive, | ||
| register_unpack_format, RegistryError, | ||
| unregister_unpack_format, get_unpack_formats, | ||
| SameFileError, _GiveupOnFastCopy) | ||
| SameFileError, _GiveupOnFastCopy, umask) | ||
jb2170 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| import tarfile | ||
| import zipfile | ||
| try: | ||
|
|
@@ -3458,13 +3458,65 @@ def test_module_all_attribute(self): | |
| 'unregister_archive_format', 'get_unpack_formats', | ||
| 'register_unpack_format', 'unregister_unpack_format', | ||
| 'unpack_archive', 'ignore_patterns', 'chown', 'which', | ||
| 'get_terminal_size', 'SameFileError'] | ||
| 'get_terminal_size', 'SameFileError', 'umask'] | ||
jb2170 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| if hasattr(os, 'statvfs') or os.name == 'nt': | ||
| target_api.append('disk_usage') | ||
| self.assertEqual(set(shutil.__all__), set(target_api)) | ||
| with self.assertWarns(DeprecationWarning): | ||
| from shutil import ExecError | ||
|
|
||
|
|
||
| @unittest.skipIf(os.name != "posix" or support.is_wasi or support.is_emscripten, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tangent: we probably need a separate issue to update the |
||
| "need proper os.umask()") | ||
| class TestUmask(unittest.TestCase): | ||
| # make target masks in here sufficiently exotic, away from 0o022 | ||
|
|
||
| mask_private = 0o777 | ||
| mask_public = 0o000 | ||
|
|
||
| def get_mask(self): | ||
| os.umask(mask := os.umask(0)) | ||
| return mask | ||
|
|
||
| def test_simple(self): | ||
| old_mask = self.get_mask() | ||
| target_mask = self.mask_private | ||
| self.assertNotEqual(old_mask, target_mask) | ||
|
|
||
| with umask(target_mask): | ||
jb2170 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| self.assertEqual(self.get_mask(), target_mask) | ||
| self.assertEqual(self.get_mask(), old_mask) | ||
|
|
||
| def test_reentrant(self): | ||
| old_mask = self.get_mask() | ||
| target_mask_1 = self.mask_private | ||
| target_mask_2 = self.mask_public | ||
| self.assertNotIn(old_mask, (target_mask_1, target_mask_2)) | ||
| umask1, umask2 = umask(target_mask_1), umask(target_mask_2) | ||
jb2170 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| with umask1: | ||
| self.assertEqual(self.get_mask(), target_mask_1) | ||
| with umask2: | ||
| self.assertEqual(self.get_mask(), target_mask_2) | ||
| with umask1: | ||
| self.assertEqual(self.get_mask(), target_mask_1) | ||
| self.assertEqual(self.get_mask(), target_mask_2) | ||
| self.assertEqual(self.get_mask(), target_mask_1) | ||
| self.assertEqual(self.get_mask(), old_mask) | ||
|
|
||
| def test_exception(self): | ||
| old_mask = self.get_mask() | ||
| target_mask = self.mask_private | ||
| self.assertNotEqual(old_mask, target_mask) | ||
|
|
||
| try: | ||
| with umask(target_mask): | ||
jb2170 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| self.assertEqual(self.get_mask(), target_mask) | ||
| raise RuntimeError("boom") | ||
| except RuntimeError as re: | ||
| self.assertEqual(str(re), "boom") | ||
| self.assertEqual(self.get_mask(), old_mask) | ||
|
|
||
|
|
||
| if __name__ == '__main__': | ||
| unittest.main() | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| Add a :class:`~shutil.umask` context manager to :mod:`shutil`, to provide a | ||
jb2170 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| stdlib context manager for changing a process's umask | ||
Uh oh!
There was an error while loading. Please reload this page.